From afc0eb1edc5b0ebd83bc26372b563dbc58d0db82 Mon Sep 17 00:00:00 2001 From: Morten Svanaes Date: Fri, 2 Feb 2024 20:30:19 +0800 Subject: [PATCH 1/3] fix: make sure background threads check for null/not logged in current user --- .../java/org/hisp/dhis/message/DefaultMessageService.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java index ec7ad8212028..b973d5d765fb 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java @@ -215,7 +215,13 @@ public long sendMessage(MessageConversationParams params, UserDetails actingUser @Override @Transactional public long sendMessage(MessageConversationParams params) { - return sendMessage(params, CurrentUserUtil.getCurrentUserDetails()); + UserDetails currentUserDetails; + if (CurrentUserUtil.hasCurrentUser()) { + currentUserDetails = CurrentUserUtil.getCurrentUserDetails(); + } else { + currentUserDetails = new SystemUser(); + } + return sendMessage(params, currentUserDetails); } @Override From 44ba334ed2b7b96793ad56616bc3ef0b1c896f32 Mon Sep 17 00:00:00 2001 From: Morten Svanaes Date: Fri, 2 Feb 2024 21:17:29 +0800 Subject: [PATCH 2/3] fix: make sure background threads check for null/not logged in current user --- .../dhis/message/DefaultMessageService.java | 26 ++++++++++++------- .../hisp/dhis/message/MessageServiceTest.java | 14 ++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java index b973d5d765fb..e0b29572ce34 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java @@ -109,7 +109,7 @@ public class DefaultMessageService implements MessageService { @Override @Transactional public long sendTicketMessage(String subject, String text, String metaData) { - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); MessageConversationParams params = new MessageConversationParams.Builder() @@ -133,7 +133,7 @@ public long sendPrivateMessage( String text, String metaData, Set attachments) { - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); MessageConversationParams params = new MessageConversationParams.Builder() @@ -260,7 +260,7 @@ public void sendReply( String metaData, boolean internal, Set attachments) { - User sender = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User sender = getCurrentUserOrNull(); Message message = new Message(text, metaData, sender, internal); @@ -294,7 +294,7 @@ public long sendCompletenessMessage(CompleteDataSetRegistration registration) { UserGroup userGroup = dataSet.getNotificationRecipients(); - User sender = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User sender = getCurrentUserOrNull(); // data set completed through sms if (sender == null) { @@ -373,7 +373,7 @@ public MessageConversation getMessageConversation(String uid) { return null; } - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); mc.setFollowUp(mc.isFollowUp(currentUser)); mc.setRead(mc.isRead(currentUser)); @@ -384,7 +384,7 @@ public MessageConversation getMessageConversation(String uid) { @Override @Transactional(readOnly = true) public long getUnreadMessageConversationCount() { - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); return messageConversationStore.getUnreadUserMessageConversationCount(currentUser); } @@ -397,7 +397,7 @@ public long getUnreadMessageConversationCount(User user) { @Override @Transactional(readOnly = true) public List getMessageConversations() { - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); return messageConversationStore.getMessageConversations( currentUser, null, false, false, null, null); } @@ -405,7 +405,7 @@ public List getMessageConversations() { @Override @Transactional(readOnly = true) public List getMessageConversations(int first, int max) { - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); return messageConversationStore.getMessageConversations( currentUser, null, false, false, first, max); } @@ -443,7 +443,7 @@ public void deleteMessages(User user) { @Override @Transactional(readOnly = true) public List getLastRecipients(int first, int max) { - User currentUser = userService.getUserByUsername(CurrentUserUtil.getCurrentUsername()); + User currentUser = getCurrentUserOrNull(); return messageConversationStore.getLastRecipients(currentUser, first, max); } @@ -525,4 +525,12 @@ private Object constructUrl( String expectedBaseUrlFormat = removeAnyTrailingSlash(baseUrl); return expectedBaseUrlFormat + MESSAGE_PATH + messageType + "/" + uid; } + + private User getCurrentUserOrNull() { + if (CurrentUserUtil.hasCurrentUser()) { + String currentUsername = CurrentUserUtil.getCurrentUsername(); + return userService.getUserByUsername(currentUsername); + } + return null; + } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java index 72ddf87505d0..54010b3023fb 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java @@ -192,4 +192,18 @@ void testGetMessageConversations() { assertTrue(conversations.contains(conversationB)); assertFalse(conversations.contains(conversationC)); } + + @Test + void testSendMessageWithNoCurrentUser() { + clearSecurityContext(); + MessageConversationParams params = + new MessageConversationParams.Builder() + .withRecipients(Set.of(userA)) + .withSubject("subject") + .withText("text") + .withMessageType(MessageType.SYSTEM) + .build(); + + messageService.sendMessage(params); + } } From 34a13bc67e184accb13e11f351cd851fb0b82687 Mon Sep 17 00:00:00 2001 From: Morten Svanaes Date: Tue, 6 Feb 2024 18:32:21 +0800 Subject: [PATCH 3/3] fix: make sure background threads check for not logged in current user --- .../test/java/org/hisp/dhis/message/MessageServiceTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java index 54010b3023fb..0d1f4903c344 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/message/MessageServiceTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.message; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -204,6 +205,6 @@ void testSendMessageWithNoCurrentUser() { .withMessageType(MessageType.SYSTEM) .build(); - messageService.sendMessage(params); + assertDoesNotThrow(() -> messageService.sendMessage(params)); } }