From 2810edca613e662caa836e52c651a842cbe9bda2 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Mon, 1 Jun 2026 11:37:26 +0300 Subject: [PATCH 1/7] fixed alarm comment permission bug --- .../controller/AlarmCommentController.java | 10 +++++ .../AlarmCommentControllerTest.java | 37 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index 8a113fb424..90a20880f6 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -31,6 +31,7 @@ import org.thingsboard.server.common.data.alarm.Alarm; import org.thingsboard.server.common.data.alarm.AlarmComment; import org.thingsboard.server.common.data.alarm.AlarmCommentInfo; import org.thingsboard.server.common.data.alarm.AlarmCommentType; +import org.thingsboard.server.common.data.exception.ThingsboardErrorCode; import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.common.data.id.AlarmCommentId; import org.thingsboard.server.common.data.id.AlarmId; @@ -77,6 +78,7 @@ public class AlarmCommentController extends BaseController { checkParameter(ALARM_ID, strAlarmId); AlarmId alarmId = new AlarmId(toUUID(strAlarmId)); Alarm alarm = checkAlarmInfoId(alarmId, Operation.WRITE); + checkUserCommentOwnership(alarmComment, Operation.WRITE); alarmComment.setAlarmId(alarmId); alarmComment.setType(AlarmCommentType.OTHER); return tbAlarmCommentService.saveAlarmComment(alarm, alarmComment, getCurrentUser()); @@ -93,6 +95,7 @@ public class AlarmCommentController extends BaseController { AlarmCommentId alarmCommentId = new AlarmCommentId(toUUID(strCommentId)); AlarmComment alarmComment = checkAlarmCommentId(alarmCommentId, alarmId); + checkUserCommentOwnership(alarmComment, Operation.DELETE); tbAlarmCommentService.deleteAlarmComment(alarm, alarmComment, getCurrentUser()); } @@ -120,4 +123,11 @@ public class AlarmCommentController extends BaseController { return checkNotNull(alarmCommentService.findAlarmComments(alarm.getTenantId(), alarmId, pageLink)); } + private void checkUserCommentOwnership(AlarmComment alarmComment, Operation operation) throws ThingsboardException { + if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(getCurrentUser().getId())) { + throw new ThingsboardException("User is not allowed to " + operation.name().toLowerCase() + " other user's comment", + ThingsboardErrorCode.PERMISSION_DENIED); + } + } + } diff --git a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java index ba86d60852..f2599d3adf 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java @@ -160,6 +160,26 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.UPDATED_COMMENT, 1, updatedAlarmComment); } + @Test + public void testUpdateOthersAlarmCommentByTenantAdmin() throws Exception { + // Tenant admins are NOT exempt from the ownership rule — even with full tenant-level + // privileges they cannot rewrite a comment authored by a different user. + loginCustomerUser(); + AlarmComment alarmComment = createAlarmComment(alarm.getId()); + + loginTenantAdmin(); + Mockito.reset(tbClusterService, auditLogService); + + JsonNode newComment = JacksonUtil.newObjectNode().set("text", new TextNode("Tenant rewrite attempt")); + alarmComment.setComment(newComment); + + doPost("/api/alarm/" + alarm.getId() + "/comment", alarmComment) + .andExpect(status().isForbidden()) + .andExpect(statusReason(containsString("User is not allowed to write other user's comment"))); + + testNotifyEntityNever(alarm.getId(), alarmComment); + } + @Test public void testUpdateAlarmViaDifferentTenant() throws Exception { loginTenantAdmin(); @@ -215,6 +235,23 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, customerUserId, CUSTOMER_USER_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); } + @Test + public void testDeleteOthersAlarmCommentByTenantAdmin() throws Exception { + // Tenant admins are NOT exempt from the ownership rule on delete either — even with full + // tenant-level privileges they cannot delete a comment authored by a different user. + loginCustomerUser(); + AlarmComment alarmComment = createAlarmComment(alarm.getId()); + + loginTenantAdmin(); + Mockito.reset(tbClusterService, auditLogService); + + doDelete("/api/alarm/" + alarm.getId() + "/comment/" + alarmComment.getId()) + .andExpect(status().isForbidden()) + .andExpect(statusReason(containsString("User is not allowed to delete other user's comment"))); + + testNotifyEntityNever(alarm.getId(), alarmComment); + } + @Test public void testDeleteAlarmViaTenant() throws Exception { loginTenantAdmin(); From ae2fdf85f2651792ce2db09e7a2066ecefb59629 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Mon, 1 Jun 2026 12:28:21 +0300 Subject: [PATCH 2/7] fixed alarm comment update flow validation --- .../server/controller/AlarmCommentController.java | 12 ++++++++++-- .../controller/AlarmCommentControllerTest.java | 5 ++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index 90a20880f6..c9d2b6fef7 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -78,7 +78,10 @@ public class AlarmCommentController extends BaseController { checkParameter(ALARM_ID, strAlarmId); AlarmId alarmId = new AlarmId(toUUID(strAlarmId)); Alarm alarm = checkAlarmInfoId(alarmId, Operation.WRITE); - checkUserCommentOwnership(alarmComment, Operation.WRITE); + if (alarmComment.getId() != null) { + AlarmComment existingAlarmComment = checkAlarmCommentId(alarmComment.getId(), alarmId); + checkUserCommentOwnership(existingAlarmComment, Operation.WRITE); + } alarmComment.setAlarmId(alarmId); alarmComment.setType(AlarmCommentType.OTHER); return tbAlarmCommentService.saveAlarmComment(alarm, alarmComment, getCurrentUser()); @@ -125,7 +128,12 @@ public class AlarmCommentController extends BaseController { private void checkUserCommentOwnership(AlarmComment alarmComment, Operation operation) throws ThingsboardException { if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(getCurrentUser().getId())) { - throw new ThingsboardException("User is not allowed to " + operation.name().toLowerCase() + " other user's comment", + String action = switch (operation) { + case WRITE -> "edit"; + case DELETE -> "delete"; + default -> "perform this operation with"; + }; + throw new ThingsboardException("User is not allowed to " + action + " other user's comment", ThingsboardErrorCode.PERMISSION_DENIED); } } diff --git a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java index f2599d3adf..e8e9412dc6 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java @@ -172,10 +172,13 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { JsonNode newComment = JacksonUtil.newObjectNode().set("text", new TextNode("Tenant rewrite attempt")); alarmComment.setComment(newComment); + // Simulate the real attack: the attacker controls the request body and would omit (or spoof) + // the userId. Ownership must be enforced against the persisted comment, not the body. + alarmComment.setUserId(null); doPost("/api/alarm/" + alarm.getId() + "/comment", alarmComment) .andExpect(status().isForbidden()) - .andExpect(statusReason(containsString("User is not allowed to write other user's comment"))); + .andExpect(statusReason(containsString("User is not allowed to edit other user's comment"))); testNotifyEntityNever(alarm.getId(), alarmComment); } From 5c796117a01a3efcaf8855dd3a094f8f51531386 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Mon, 1 Jun 2026 12:46:09 +0300 Subject: [PATCH 3/7] minor refactoring --- .../controller/AlarmCommentController.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index c9d2b6fef7..3091d6ac44 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -40,6 +40,7 @@ import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.config.annotations.ApiOperation; import org.thingsboard.server.queue.util.TbCoreComponent; import org.thingsboard.server.service.entitiy.alarm.TbAlarmCommentService; +import org.thingsboard.server.service.security.model.SecurityUser; import org.thingsboard.server.service.security.permission.Operation; import static org.thingsboard.server.controller.ControllerConstants.ALARM_COMMENT_ID_PARAM_DESCRIPTION; @@ -78,13 +79,14 @@ public class AlarmCommentController extends BaseController { checkParameter(ALARM_ID, strAlarmId); AlarmId alarmId = new AlarmId(toUUID(strAlarmId)); Alarm alarm = checkAlarmInfoId(alarmId, Operation.WRITE); + SecurityUser currentUser = getCurrentUser(); if (alarmComment.getId() != null) { AlarmComment existingAlarmComment = checkAlarmCommentId(alarmComment.getId(), alarmId); - checkUserCommentOwnership(existingAlarmComment, Operation.WRITE); + checkUserCommentOwnership(existingAlarmComment, Operation.WRITE, currentUser); } alarmComment.setAlarmId(alarmId); alarmComment.setType(AlarmCommentType.OTHER); - return tbAlarmCommentService.saveAlarmComment(alarm, alarmComment, getCurrentUser()); + return tbAlarmCommentService.saveAlarmComment(alarm, alarmComment, currentUser); } @ApiOperation(value = "Delete Alarm comment (deleteAlarmComment)", @@ -98,8 +100,9 @@ public class AlarmCommentController extends BaseController { AlarmCommentId alarmCommentId = new AlarmCommentId(toUUID(strCommentId)); AlarmComment alarmComment = checkAlarmCommentId(alarmCommentId, alarmId); - checkUserCommentOwnership(alarmComment, Operation.DELETE); - tbAlarmCommentService.deleteAlarmComment(alarm, alarmComment, getCurrentUser()); + SecurityUser currentUser = getCurrentUser(); + checkUserCommentOwnership(alarmComment, Operation.DELETE, currentUser); + tbAlarmCommentService.deleteAlarmComment(alarm, alarmComment, currentUser); } @ApiOperation(value = "Get Alarm comments (getAlarmComments)", @@ -126,14 +129,9 @@ public class AlarmCommentController extends BaseController { return checkNotNull(alarmCommentService.findAlarmComments(alarm.getTenantId(), alarmId, pageLink)); } - private void checkUserCommentOwnership(AlarmComment alarmComment, Operation operation) throws ThingsboardException { - if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(getCurrentUser().getId())) { - String action = switch (operation) { - case WRITE -> "edit"; - case DELETE -> "delete"; - default -> "perform this operation with"; - }; - throw new ThingsboardException("User is not allowed to " + action + " other user's comment", + private void checkUserCommentOwnership(AlarmComment alarmComment, Operation operation, SecurityUser securityUser) throws ThingsboardException { + if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(securityUser.getId())) { + throw new ThingsboardException("User is not allowed to " + (operation == Operation.DELETE ? "delete" : "edit") + " other user's comment", ThingsboardErrorCode.PERMISSION_DENIED); } } From ad88850eb93909d9fa262286c28b8b88b7738956 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Fri, 5 Jun 2026 17:42:19 +0300 Subject: [PATCH 4/7] restored tenant admin permissions to moderate user comments --- .../controller/AlarmCommentController.java | 3 ++ .../AlarmCommentControllerTest.java | 34 +++++++++++-------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index 3091d6ac44..0bcce4be83 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -130,6 +130,9 @@ public class AlarmCommentController extends BaseController { } private void checkUserCommentOwnership(AlarmComment alarmComment, Operation operation, SecurityUser securityUser) throws ThingsboardException { + if (securityUser.isTenantAdmin()) { + return; + } if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(securityUser.getId())) { throw new ThingsboardException("User is not allowed to " + (operation == Operation.DELETE ? "delete" : "edit") + " other user's comment", ThingsboardErrorCode.PERMISSION_DENIED); diff --git a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java index e8e9412dc6..74b3a1122d 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java @@ -162,25 +162,24 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { @Test public void testUpdateOthersAlarmCommentByTenantAdmin() throws Exception { - // Tenant admins are NOT exempt from the ownership rule — even with full tenant-level - // privileges they cannot rewrite a comment authored by a different user. + // Tenant admins may moderate comments authored by other users — the ownership rule + // applies only to non-admin users, so a tenant admin can edit someone else's comment. loginCustomerUser(); AlarmComment alarmComment = createAlarmComment(alarm.getId()); loginTenantAdmin(); Mockito.reset(tbClusterService, auditLogService); - JsonNode newComment = JacksonUtil.newObjectNode().set("text", new TextNode("Tenant rewrite attempt")); + JsonNode newComment = JacksonUtil.newObjectNode().set("text", new TextNode("Tenant rewrite")); alarmComment.setComment(newComment); - // Simulate the real attack: the attacker controls the request body and would omit (or spoof) - // the userId. Ownership must be enforced against the persisted comment, not the body. - alarmComment.setUserId(null); + AlarmComment updatedAlarmComment = saveAlarmComment(alarm.getId(), alarmComment); - doPost("/api/alarm/" + alarm.getId() + "/comment", alarmComment) - .andExpect(status().isForbidden()) - .andExpect(statusReason(containsString("User is not allowed to edit other user's comment"))); + Assert.assertNotNull(updatedAlarmComment); + Assert.assertEquals(newComment.get("text"), updatedAlarmComment.getComment().get("text")); + Assert.assertEquals("true", updatedAlarmComment.getComment().get("edited").asText()); + Assert.assertNotNull(updatedAlarmComment.getComment().get("editedOn")); - testNotifyEntityNever(alarm.getId(), alarmComment); + testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.UPDATED_COMMENT, 1, updatedAlarmComment); } @Test @@ -240,8 +239,8 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { @Test public void testDeleteOthersAlarmCommentByTenantAdmin() throws Exception { - // Tenant admins are NOT exempt from the ownership rule on delete either — even with full - // tenant-level privileges they cannot delete a comment authored by a different user. + // Tenant admins may moderate comments authored by other users — the ownership rule + // applies only to non-admin users, so a tenant admin can delete someone else's comment. loginCustomerUser(); AlarmComment alarmComment = createAlarmComment(alarm.getId()); @@ -249,10 +248,15 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { Mockito.reset(tbClusterService, auditLogService); doDelete("/api/alarm/" + alarm.getId() + "/comment/" + alarmComment.getId()) - .andExpect(status().isForbidden()) - .andExpect(statusReason(containsString("User is not allowed to delete other user's comment"))); + .andExpect(status().isOk()); - testNotifyEntityNever(alarm.getId(), alarmComment); + AlarmComment expectedAlarmComment = AlarmComment.builder() + .alarmId(alarm.getId()) + .type(AlarmCommentType.SYSTEM) + .comment(JacksonUtil.newObjectNode().put("text", String.format("User %s deleted his comment", + TENANT_ADMIN_EMAIL))) + .build(); + testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); } @Test From 88180f9009fd68b08249654e87eb674256f895cb Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Fri, 5 Jun 2026 17:58:37 +0300 Subject: [PATCH 5/7] minor refactoring --- .../server/controller/AlarmCommentController.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index 0bcce4be83..aae7670d20 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -82,7 +82,7 @@ public class AlarmCommentController extends BaseController { SecurityUser currentUser = getCurrentUser(); if (alarmComment.getId() != null) { AlarmComment existingAlarmComment = checkAlarmCommentId(alarmComment.getId(), alarmId); - checkUserCommentOwnership(existingAlarmComment, Operation.WRITE, currentUser); + checkUserCommentOwnership(existingAlarmComment, "edit", currentUser); } alarmComment.setAlarmId(alarmId); alarmComment.setType(AlarmCommentType.OTHER); @@ -101,7 +101,7 @@ public class AlarmCommentController extends BaseController { AlarmCommentId alarmCommentId = new AlarmCommentId(toUUID(strCommentId)); AlarmComment alarmComment = checkAlarmCommentId(alarmCommentId, alarmId); SecurityUser currentUser = getCurrentUser(); - checkUserCommentOwnership(alarmComment, Operation.DELETE, currentUser); + checkUserCommentOwnership(alarmComment, "delete", currentUser); tbAlarmCommentService.deleteAlarmComment(alarm, alarmComment, currentUser); } @@ -129,12 +129,12 @@ public class AlarmCommentController extends BaseController { return checkNotNull(alarmCommentService.findAlarmComments(alarm.getTenantId(), alarmId, pageLink)); } - private void checkUserCommentOwnership(AlarmComment alarmComment, Operation operation, SecurityUser securityUser) throws ThingsboardException { + private void checkUserCommentOwnership(AlarmComment alarmComment, String action, SecurityUser securityUser) throws ThingsboardException { if (securityUser.isTenantAdmin()) { return; } if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(securityUser.getId())) { - throw new ThingsboardException("User is not allowed to " + (operation == Operation.DELETE ? "delete" : "edit") + " other user's comment", + throw new ThingsboardException("User is not allowed to " + action + " other user's comment", ThingsboardErrorCode.PERMISSION_DENIED); } } From 0276d66d7238ba83bd14b4245d46197bc2a96b3d Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Mon, 8 Jun 2026 16:53:28 +0300 Subject: [PATCH 6/7] update alarm comment moderation logic: delete is allowed for all users with alarm WRITE permission, edit - only for authors --- .../controller/AlarmCommentController.java | 16 ++------ .../alarm/DefaultTbAlarmCommentService.java | 2 +- .../server/controller/AbstractWebTest.java | 23 ++++++++++- .../AlarmCommentControllerTest.java | 41 ++++++++----------- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index aae7670d20..adc29df05f 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -82,7 +82,10 @@ public class AlarmCommentController extends BaseController { SecurityUser currentUser = getCurrentUser(); if (alarmComment.getId() != null) { AlarmComment existingAlarmComment = checkAlarmCommentId(alarmComment.getId(), alarmId); - checkUserCommentOwnership(existingAlarmComment, "edit", currentUser); + if (existingAlarmComment.getUserId() != null && !existingAlarmComment.getUserId().equals(currentUser.getId())) { + throw new ThingsboardException("User is not allowed to edit other user's comment", + ThingsboardErrorCode.PERMISSION_DENIED); + } } alarmComment.setAlarmId(alarmId); alarmComment.setType(AlarmCommentType.OTHER); @@ -101,7 +104,6 @@ public class AlarmCommentController extends BaseController { AlarmCommentId alarmCommentId = new AlarmCommentId(toUUID(strCommentId)); AlarmComment alarmComment = checkAlarmCommentId(alarmCommentId, alarmId); SecurityUser currentUser = getCurrentUser(); - checkUserCommentOwnership(alarmComment, "delete", currentUser); tbAlarmCommentService.deleteAlarmComment(alarm, alarmComment, currentUser); } @@ -129,14 +131,4 @@ public class AlarmCommentController extends BaseController { return checkNotNull(alarmCommentService.findAlarmComments(alarm.getTenantId(), alarmId, pageLink)); } - private void checkUserCommentOwnership(AlarmComment alarmComment, String action, SecurityUser securityUser) throws ThingsboardException { - if (securityUser.isTenantAdmin()) { - return; - } - if (alarmComment.getUserId() != null && !alarmComment.getUserId().equals(securityUser.getId())) { - throw new ThingsboardException("User is not allowed to " + action + " other user's comment", - ThingsboardErrorCode.PERMISSION_DENIED); - } - } - } diff --git a/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmCommentService.java b/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmCommentService.java index 0fb43a4511..276ba1bedc 100644 --- a/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmCommentService.java +++ b/application/src/main/java/org/thingsboard/server/service/entitiy/alarm/DefaultTbAlarmCommentService.java @@ -60,7 +60,7 @@ public class DefaultTbAlarmCommentService extends AbstractTbEntityService implem alarmComment.setType(AlarmCommentType.SYSTEM); alarmComment.setUserId(null); alarmComment.setComment(JacksonUtil.newObjectNode().put("text", - String.format("User %s deleted his comment", + String.format("Comment was deleted by user %s", (user.getFirstName() == null || user.getLastName() == null) ? user.getName() : user.getFirstName() + " " + user.getLastName()))); AlarmComment savedAlarmComment = checkNotNull(alarmCommentService.saveAlarmComment(alarm.getTenantId(), alarmComment)); logEntityActionService.logEntityAction(alarm.getTenantId(), alarm.getId(), alarm, alarm.getCustomerId(), ActionType.DELETED_COMMENT, user, savedAlarmComment); diff --git a/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java b/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java index bbf3a3467e..2b70e04a85 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java @@ -210,6 +210,7 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { private static final String DIFFERENT_TENANT_ADMIN_PASSWORD = "difftenant"; protected static final String CUSTOMER_USER_EMAIL = "testcustomer@thingsboard.org"; + protected static final String SECOND_CUSTOMER_USER_EMAIL = "testsecondcustomer@thingsboard.org"; private static final String CUSTOMER_USER_PASSWORD = "customer"; protected static final String DIFFERENT_CUSTOMER_USER_EMAIL = "testdifferentcustomer@thingsboard.org"; @@ -247,6 +248,7 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { protected CustomerId differentTenantCustomerId; protected UserId customerUserId; + protected UserId secondCustomerUserId; protected UserId differentCustomerUserId; protected UserId differentTenantCustomerUserId; @@ -372,9 +374,17 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { customerUser.setCustomerId(savedCustomer.getId()); customerUser.setEmail(CUSTOMER_USER_EMAIL); - customerUser = createUserAndLogin(customerUser, CUSTOMER_USER_PASSWORD); + customerUser = createUserAndActivate(customerUser, CUSTOMER_USER_PASSWORD); customerUserId = customerUser.getId(); + User secondCustomerUser = new User(); + secondCustomerUser.setAuthority(Authority.CUSTOMER_USER); + secondCustomerUser.setTenantId(tenantId); + secondCustomerUser.setCustomerId(customerId); + secondCustomerUser.setEmail(SECOND_CUSTOMER_USER_EMAIL); + secondCustomerUser = createUserAndActivate(secondCustomerUser, CUSTOMER_USER_PASSWORD); + secondCustomerUserId = secondCustomerUser.getId(); + resetTokens(); log.debug("Executed web test setup"); @@ -472,6 +482,10 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { login(CUSTOMER_USER_EMAIL, CUSTOMER_USER_PASSWORD); } + protected void loginSecondCustomerUser() throws Exception { + login(SECOND_CUSTOMER_USER_EMAIL, CUSTOMER_USER_PASSWORD); + } + protected void loginUser(String userName, String password) throws Exception { login(userName, password); } @@ -586,6 +600,13 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { return savedUser; } + protected User createUserAndActivate(User user, String password) throws Exception { + User savedUser = doPost("/api/user", user, User.class); + JsonNode activateRequest = getActivateRequest(password); + doPost("/api/noauth/activate", activateRequest).andExpect(status().isOk()); + return savedUser; + } + protected User createUser(User user, String password) throws Exception { User savedUser = doPost("/api/user", user, User.class); JsonNode activateRequest = getActivateRequest(password); diff --git a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java index 74b3a1122d..9b997cd8ad 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java @@ -161,25 +161,22 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { } @Test - public void testUpdateOthersAlarmCommentByTenantAdmin() throws Exception { - // Tenant admins may moderate comments authored by other users — the ownership rule - // applies only to non-admin users, so a tenant admin can edit someone else's comment. + public void testEditOthersAlarmCommentIsProhibited() throws Exception { loginCustomerUser(); AlarmComment alarmComment = createAlarmComment(alarm.getId()); - loginTenantAdmin(); - Mockito.reset(tbClusterService, auditLogService); - - JsonNode newComment = JacksonUtil.newObjectNode().set("text", new TextNode("Tenant rewrite")); + JsonNode newComment = JacksonUtil.newObjectNode().set("text", new TextNode("Second customer rewrite")); alarmComment.setComment(newComment); - AlarmComment updatedAlarmComment = saveAlarmComment(alarm.getId(), alarmComment); - Assert.assertNotNull(updatedAlarmComment); - Assert.assertEquals(newComment.get("text"), updatedAlarmComment.getComment().get("text")); - Assert.assertEquals("true", updatedAlarmComment.getComment().get("edited").asText()); - Assert.assertNotNull(updatedAlarmComment.getComment().get("editedOn")); + loginSecondCustomerUser(); + doPost("/api/alarm/" + alarm.getId() + "/comment", alarmComment) + .andExpect(status().isForbidden()) + .andExpect(statusReason(containsString("User is not allowed to edit other user's comment"))); - testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.UPDATED_COMMENT, 1, updatedAlarmComment); + loginTenantAdmin(); + doPost("/api/alarm/" + alarm.getId() + "/comment", alarmComment) + .andExpect(status().isForbidden()) + .andExpect(statusReason(containsString("User is not allowed to edit other user's comment"))); } @Test @@ -231,20 +228,18 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { AlarmComment expectedAlarmComment = AlarmComment.builder() .alarmId(alarm.getId()) .type(AlarmCommentType.SYSTEM) - .comment(JacksonUtil.newObjectNode().put("text", String.format("User %s deleted his comment", + .comment(JacksonUtil.newObjectNode().put("text", String.format("Comment was deleted by user %s", CUSTOMER_USER_EMAIL))) .build(); testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, customerUserId, CUSTOMER_USER_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); } @Test - public void testDeleteOthersAlarmCommentByTenantAdmin() throws Exception { - // Tenant admins may moderate comments authored by other users — the ownership rule - // applies only to non-admin users, so a tenant admin can delete someone else's comment. + public void testDeleteOthersAlarmCommentIsAllowedForUserWithAlarmWritePermission() throws Exception { loginCustomerUser(); AlarmComment alarmComment = createAlarmComment(alarm.getId()); - loginTenantAdmin(); + loginSecondCustomerUser(); Mockito.reset(tbClusterService, auditLogService); doDelete("/api/alarm/" + alarm.getId() + "/comment/" + alarmComment.getId()) @@ -253,10 +248,10 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { AlarmComment expectedAlarmComment = AlarmComment.builder() .alarmId(alarm.getId()) .type(AlarmCommentType.SYSTEM) - .comment(JacksonUtil.newObjectNode().put("text", String.format("User %s deleted his comment", - TENANT_ADMIN_EMAIL))) + .comment(JacksonUtil.newObjectNode().put("text", String.format("Comment was deleted by user %s", + SECOND_CUSTOMER_USER_EMAIL))) .build(); - testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); + testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, secondCustomerUserId, SECOND_CUSTOMER_USER_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); } @Test @@ -278,13 +273,13 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { assertThat(systemComment.getId()).isEqualTo(alarmComment.getId()); assertThat(systemComment.getType()).isEqualTo(AlarmCommentType.SYSTEM); - assertThat(systemComment.getComment().get("text").asText()).isEqualTo(String.format("User %s deleted his comment", + assertThat(systemComment.getComment().get("text").asText()).isEqualTo(String.format("Comment was deleted by user %s", TENANT_ADMIN_EMAIL)); AlarmComment expectedAlarmComment = AlarmComment.builder() .alarmId(alarm.getId()) .type(AlarmCommentType.SYSTEM) - .comment(JacksonUtil.newObjectNode().put("text", String.format("User %s deleted his comment", + .comment(JacksonUtil.newObjectNode().put("text", String.format("Comment was deleted by user %s", TENANT_ADMIN_EMAIL))) .build(); testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); From 8082d60ffe163a987f25ad7ab9950dc645eb9193 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Mon, 8 Jun 2026 17:48:35 +0300 Subject: [PATCH 7/7] update alarm comment moderation logic: delete is allowed for author or tenant admin only --- .../controller/AlarmCommentController.java | 17 ++++++++++++----- .../controller/AlarmCommentControllerTest.java | 12 ++++++++---- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java index adc29df05f..998ca1cfa5 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AlarmCommentController.java @@ -81,11 +81,7 @@ public class AlarmCommentController extends BaseController { Alarm alarm = checkAlarmInfoId(alarmId, Operation.WRITE); SecurityUser currentUser = getCurrentUser(); if (alarmComment.getId() != null) { - AlarmComment existingAlarmComment = checkAlarmCommentId(alarmComment.getId(), alarmId); - if (existingAlarmComment.getUserId() != null && !existingAlarmComment.getUserId().equals(currentUser.getId())) { - throw new ThingsboardException("User is not allowed to edit other user's comment", - ThingsboardErrorCode.PERMISSION_DENIED); - } + checkUserPermission(alarmComment, alarmId, "edit", currentUser); } alarmComment.setAlarmId(alarmId); alarmComment.setType(AlarmCommentType.OTHER); @@ -104,6 +100,9 @@ public class AlarmCommentController extends BaseController { AlarmCommentId alarmCommentId = new AlarmCommentId(toUUID(strCommentId)); AlarmComment alarmComment = checkAlarmCommentId(alarmCommentId, alarmId); SecurityUser currentUser = getCurrentUser(); + if (!currentUser.isTenantAdmin()) { + checkUserPermission(alarmComment, alarmId, "delete", currentUser); + } tbAlarmCommentService.deleteAlarmComment(alarm, alarmComment, currentUser); } @@ -131,4 +130,12 @@ public class AlarmCommentController extends BaseController { return checkNotNull(alarmCommentService.findAlarmComments(alarm.getTenantId(), alarmId, pageLink)); } + private void checkUserPermission(AlarmComment alarmComment, AlarmId alarmId, String operation, SecurityUser currentUser) throws ThingsboardException { + AlarmComment existingAlarmComment = checkAlarmCommentId(alarmComment.getId(), alarmId); + if (existingAlarmComment.getUserId() != null && !existingAlarmComment.getUserId().equals(currentUser.getId())) { + throw new ThingsboardException("User is not allowed to " + operation + " other user's comment", + ThingsboardErrorCode.PERMISSION_DENIED); + } + } + } diff --git a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java index 9b997cd8ad..bebfe832e8 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AlarmCommentControllerTest.java @@ -235,7 +235,7 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { } @Test - public void testDeleteOthersAlarmCommentIsAllowedForUserWithAlarmWritePermission() throws Exception { + public void testDeleteOthersAlarmCommentIsAllowedForAuthorOrTenantAdmin() throws Exception { loginCustomerUser(); AlarmComment alarmComment = createAlarmComment(alarm.getId()); @@ -243,15 +243,19 @@ public class AlarmCommentControllerTest extends AbstractControllerTest { Mockito.reset(tbClusterService, auditLogService); doDelete("/api/alarm/" + alarm.getId() + "/comment/" + alarmComment.getId()) - .andExpect(status().isOk()); + .andExpect(status().isForbidden()) + .andExpect(statusReason(containsString("User is not allowed to delete other user's comment"))); + loginTenantAdmin(); + doDelete("/api/alarm/" + alarm.getId() + "/comment/" + alarmComment.getId()) + .andExpect(status().isOk()); AlarmComment expectedAlarmComment = AlarmComment.builder() .alarmId(alarm.getId()) .type(AlarmCommentType.SYSTEM) .comment(JacksonUtil.newObjectNode().put("text", String.format("Comment was deleted by user %s", - SECOND_CUSTOMER_USER_EMAIL))) + TENANT_ADMIN_EMAIL))) .build(); - testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, secondCustomerUserId, SECOND_CUSTOMER_USER_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); + testLogEntityActionEntityEqClass(alarm, alarm.getId(), tenantId, customerId, tenantAdminUserId, TENANT_ADMIN_EMAIL, ActionType.DELETED_COMMENT, 1, expectedAlarmComment); } @Test