From 7eb71000511dd9fe3c1fb04154f4ce02093c6e1c Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Thu, 27 Oct 2022 15:51:22 +0200 Subject: [PATCH 1/9] feat: add manual parameter validation --- .../caritas/cob/messageservice/Messenger.java | 8 +++ .../api/controller/MessageController.java | 21 ++++++- .../controller/MessageControllerE2EIT.java | 56 ++++++++++++++++++- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/main/java/de/caritas/cob/messageservice/Messenger.java b/src/main/java/de/caritas/cob/messageservice/Messenger.java index 4180715..8e01151 100644 --- a/src/main/java/de/caritas/cob/messageservice/Messenger.java +++ b/src/main/java/de/caritas/cob/messageservice/Messenger.java @@ -264,4 +264,12 @@ public MessageResponseDTO postAliasMessage(String rcGroupId, MessageType message aliasMessageDTO); return mapper.messageResponseOf(response); } + + public boolean deleteMessage(String messageId) { + return false; + } + + public boolean deleteAttachment(String messageId) { + return false; + } } diff --git a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java index 1c9bfb9..5bdb93f 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java +++ b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java @@ -294,7 +294,26 @@ public ResponseEntity patchMessage(String rcToken, String rcUserId, String @Override public ResponseEntity deleteMessage(String rcToken, String rcUserId, String messageId, String attachmentId) { - return MessagesApi.super.deleteMessage(rcToken, rcUserId, messageId, attachmentId); + var creatorId = messenger + .findMessage(rcToken, rcUserId, messageId) + .map(mapper::messageDtoOf) + .map(messageDto -> messageDto.getU().get_id()); + + if (creatorId.isEmpty()) { + return ResponseEntity.notFound().build(); + } + + if (!creatorId.get().equals(rcUserId)) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); + } + + if (!messenger.deleteMessage(messageId)) { + return ResponseEntity.internalServerError().build(); + } + + return nonNull(attachmentId) && !messenger.deleteAttachment(attachmentId) + ? ResponseEntity.status(HttpStatus.MULTI_STATUS).build() + : ResponseEntity.noContent().build(); } /** diff --git a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java index 21f5b4f..9f6197f 100644 --- a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java +++ b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java @@ -3,6 +3,7 @@ import static de.caritas.cob.messageservice.testhelper.TestConstants.RC_GROUP_ID; import static de.caritas.cob.messageservice.testhelper.TestConstants.RC_USER_ID; import static de.caritas.cob.messageservice.testhelper.TestConstants.createSuccessfulMessageResult; +import static java.util.Objects.nonNull; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; @@ -648,10 +649,11 @@ void patchMessageShouldRespondWithNoContent() throws Exception { @Test @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) - void patchMessageShouldRespondWithNotImplemented() throws Exception { + void deleteMessageShouldRespondWithNotFoundGivenMessageDoesNotExist() throws Exception { givenAuthenticatedUser(); givenAValidMessageId(); givenAValidAttachmentId(); + givenAGetChatMessageNotFoundResponse(messageId); mockMvc.perform( delete("/messages/{messageId}", messageId) @@ -661,7 +663,48 @@ void patchMessageShouldRespondWithNotImplemented() throws Exception { .header("rcUserId", RandomStringUtils.randomAlphabetic(16)) .queryParam("attachmentId", attachmentId) ) - .andExpect(status().isNotImplemented()); + .andExpect(status().isNotFound()); + } + + @Test + @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) + void deleteMessageShouldRespondWithForbiddenGivenMessageIsNotFromUser() throws Exception { + givenAuthenticatedUser(); + givenAValidMessageId(); + givenAValidAttachmentId(); + givenAMasterKey(); + givenMessage(messageId, true, RandomStringUtils.randomAlphabetic(16)); + + mockMvc.perform( + delete("/messages/{messageId}", messageId) + .cookie(CSRF_COOKIE) + .header(CSRF_HEADER, CSRF_VALUE) + .header("rcToken", RandomStringUtils.randomAlphabetic(16)) + .header("rcUserId", RandomStringUtils.randomAlphabetic(16)) + .queryParam("attachmentId", attachmentId) + ) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) + void deleteMessageShouldRespondWithInternalServerErrorIfDeletionFails() throws Exception { + givenAuthenticatedUser(); + givenAValidMessageId(); + givenAValidAttachmentId(); + givenAMasterKey(); + var rcUserId = RandomStringUtils.randomAlphabetic(16); + givenMessage(messageId, true, rcUserId); + + mockMvc.perform( + delete("/messages/{messageId}", messageId) + .cookie(CSRF_COOKIE) + .header(CSRF_HEADER, CSRF_VALUE) + .header("rcToken", RandomStringUtils.randomAlphabetic(16)) + .header("rcUserId", rcUserId) + .queryParam("attachmentId", attachmentId) + ) + .andExpect(status().isInternalServerError()); } @Test @@ -1072,6 +1115,11 @@ private void givenMessages() { private void givenMessage(String id, boolean full) throws JsonProcessingException, CustomCryptoException { + givenMessage(id, full, null); + } + + private void givenMessage(String id, boolean full, String userId) + throws JsonProcessingException, CustomCryptoException { var response = new MessageResponse(); response.setSuccess(true); @@ -1087,6 +1135,10 @@ private void givenMessage(String id, boolean full) message.setAlias(encodedAlias); messagesDTO = easyRandom.nextObject(MessagesDTO.class); + if (nonNull(userId)) { + messagesDTO.getU().set_id(userId); + } + var props = message.getOtherProperties(); props.put("u", messagesDTO.getU()); props.put("attachments", messagesDTO.getAttachments()); From 18e918eeb7986d3a3763e9b740a30380256afaa6 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Thu, 27 Oct 2022 17:46:54 +0200 Subject: [PATCH 2/9] feat: delete message --- .../caritas/cob/messageservice/Messenger.java | 4 +- .../api/controller/MessageController.java | 2 +- .../api/service/MessageMapper.java | 25 +++++++++++ .../api/service/RocketChatService.java | 17 ++++++++ .../api/service/dto/DeleteMessage.java | 9 ++++ .../api/service/dto/MethodMessage.java | 17 ++++++++ .../dto/StringifiedMessageResponse.java | 11 +++++ .../controller/MessageControllerE2EIT.java | 43 +++++++++++++++++++ 8 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java create mode 100644 src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java create mode 100644 src/main/java/de/caritas/cob/messageservice/api/service/dto/StringifiedMessageResponse.java diff --git a/src/main/java/de/caritas/cob/messageservice/Messenger.java b/src/main/java/de/caritas/cob/messageservice/Messenger.java index 8e01151..e2f2efc 100644 --- a/src/main/java/de/caritas/cob/messageservice/Messenger.java +++ b/src/main/java/de/caritas/cob/messageservice/Messenger.java @@ -265,8 +265,8 @@ public MessageResponseDTO postAliasMessage(String rcGroupId, MessageType message return mapper.messageResponseOf(response); } - public boolean deleteMessage(String messageId) { - return false; + public boolean deleteMessage(String rcToken, String rcUserId, String messageId) { + return rocketChatService.deleteMessage(rcToken, rcUserId, messageId); } public boolean deleteAttachment(String messageId) { diff --git a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java index 5bdb93f..5d19772 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java +++ b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java @@ -307,7 +307,7 @@ public ResponseEntity deleteMessage(String rcToken, String rcUserId, Strin return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); } - if (!messenger.deleteMessage(messageId)) { + if (!messenger.deleteMessage(rcToken, rcUserId, messageId)) { return ResponseEntity.internalServerError().build(); } diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java index f92b8cf..b2e1a3e 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java @@ -17,16 +17,21 @@ import de.caritas.cob.messageservice.api.model.jsondeserializer.AliasJsonDeserializer; import de.caritas.cob.messageservice.api.model.rocket.chat.message.MessagesDTO; import de.caritas.cob.messageservice.api.model.rocket.chat.message.SendMessageResponseDTO; +import de.caritas.cob.messageservice.api.service.dto.DeleteMessage; import de.caritas.cob.messageservice.api.service.dto.Message; +import de.caritas.cob.messageservice.api.service.dto.MethodMessage; import de.caritas.cob.messageservice.api.service.dto.UpdateMessage; import java.time.Instant; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Random; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @Service +@Slf4j @RequiredArgsConstructor public class MessageMapper { @@ -195,4 +200,24 @@ public MessageType messageTypeOf(AliasMessageDTO alias) { return null; } + + public DeleteMessage deleteMessageOf(String messageId) { + var params = Map.of("_id", messageId); + + var message = new MethodMessage(); + message.setParams(List.of(params)); + message.setId(new Random().nextInt(100)); + message.setMsg("method"); + message.setMethod("deleteMessage"); + + var deleteMessage = new DeleteMessage(); + try { + var messageString = objectMapper.writeValueAsString(message); + deleteMessage.setMessage(messageString); + } catch (JsonProcessingException e) { + log.error("Serializing {} did not work.", message); + } + + return deleteMessage; + } } diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java b/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java index 2e54f5b..9db6769 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java @@ -26,6 +26,7 @@ import de.caritas.cob.messageservice.api.model.rocket.chat.message.SendMessageWrapper; import de.caritas.cob.messageservice.api.service.dto.Message; import de.caritas.cob.messageservice.api.service.dto.MessageResponse; +import de.caritas.cob.messageservice.api.service.dto.StringifiedMessageResponse; import de.caritas.cob.messageservice.api.service.dto.UpdateMessage; import de.caritas.cob.messageservice.api.service.helper.RocketChatCredentialsHelper; import java.net.URI; @@ -54,6 +55,7 @@ public class RocketChatService { public static final String E2E_ENCRYPTION_TYPE = "e2e"; + private static final String ENDPOINT_MESSAGE_DELETE = "/method.call/deleteMessage"; private static final String ENDPOINT_MESSAGE_GET = "/chat.getMessage?msgId="; private static final String ENDPOINT_MESSAGE_UPDATE = "/chat.update"; @@ -395,6 +397,21 @@ public Message findMessage(String rcToken, String rcUserId, String messageId) { return null; } + public boolean deleteMessage(String rcToken, String rcUserId, String messageId) { + var url = baseUrl + ENDPOINT_MESSAGE_DELETE; + var deleteMessage = mapper.deleteMessageOf(messageId); + var entity = new HttpEntity<>(deleteMessage, getRocketChatHeader(rcToken, rcUserId)); + + try { + var response = restTemplate.postForEntity(url, entity, StringifiedMessageResponse.class); + return nonNull(response.getBody()) && response.getBody().getSuccess() + && !response.getBody().getMessage().contains("\"error\""); + } catch (HttpClientErrorException exception) { + log.error("Deleting message failed.", exception); + return false; + } + } + @SuppressWarnings("java:S5852") // Using slow regular expressions is security-sensitive private boolean isRcNotFoundResponse(HttpClientErrorException exception) { return HttpStatus.BAD_REQUEST.equals(exception.getStatusCode()) diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java b/src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java new file mode 100644 index 0000000..9bfc4fb --- /dev/null +++ b/src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java @@ -0,0 +1,9 @@ +package de.caritas.cob.messageservice.api.service.dto; + +import lombok.Data; + +@Data +public class DeleteMessage { + + private String message; +} diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java new file mode 100644 index 0000000..1888c93 --- /dev/null +++ b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java @@ -0,0 +1,17 @@ +package de.caritas.cob.messageservice.api.service.dto; + +import java.util.List; +import java.util.Map; +import lombok.Data; + +@Data +public class MethodMessage { + + private String msg; + + private int id; + + private String method; + + private List> params; +} diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/dto/StringifiedMessageResponse.java b/src/main/java/de/caritas/cob/messageservice/api/service/dto/StringifiedMessageResponse.java new file mode 100644 index 0000000..86a2485 --- /dev/null +++ b/src/main/java/de/caritas/cob/messageservice/api/service/dto/StringifiedMessageResponse.java @@ -0,0 +1,11 @@ +package de.caritas.cob.messageservice.api.service.dto; + +import lombok.Data; + +@Data +public class StringifiedMessageResponse { + + private String message; + + private Boolean success; +} diff --git a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java index 9f6197f..4bcff47 100644 --- a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java +++ b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java @@ -57,6 +57,7 @@ import de.caritas.cob.messageservice.api.service.RocketChatService; import de.caritas.cob.messageservice.api.service.dto.Message; import de.caritas.cob.messageservice.api.service.dto.MessageResponse; +import de.caritas.cob.messageservice.api.service.dto.StringifiedMessageResponse; import de.caritas.cob.messageservice.api.service.helper.RocketChatCredentialsHelper; import de.caritas.cob.messageservice.api.service.statistics.StatisticsService; import java.net.URI; @@ -695,6 +696,7 @@ void deleteMessageShouldRespondWithInternalServerErrorIfDeletionFails() throws E givenAMasterKey(); var rcUserId = RandomStringUtils.randomAlphabetic(16); givenMessage(messageId, true, rcUserId); + givenDeletableMessage(false); mockMvc.perform( delete("/messages/{messageId}", messageId) @@ -707,6 +709,27 @@ void deleteMessageShouldRespondWithInternalServerErrorIfDeletionFails() throws E .andExpect(status().isInternalServerError()); } + @Test + @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) + void deleteMessageShouldRespondWithNoContentIfDeletionSucceeds() throws Exception { + givenAuthenticatedUser(); + givenAValidMessageId(); + givenAValidAttachmentId(); + givenAMasterKey(); + var rcUserId = RandomStringUtils.randomAlphabetic(16); + givenMessage(messageId, true, rcUserId); + givenDeletableMessage(true); + + mockMvc.perform( + delete("/messages/{messageId}", messageId) + .cookie(CSRF_COOKIE) + .header(CSRF_HEADER, CSRF_VALUE) + .header("rcToken", RandomStringUtils.randomAlphabetic(16)) + .header("rcUserId", rcUserId) + ) + .andExpect(status().isNoContent()); + } + @Test @WithMockUser(authorities = {AuthorityValue.USER_DEFAULT}) void sendMessageShouldTransmitTypeOfMessage() throws Exception { @@ -1156,6 +1179,26 @@ private void givenMessage(String id, boolean full, String userId) eq(MessageResponse.class))).thenReturn(ResponseEntity.ok().body(response)); } + private void givenDeletableMessage(boolean success) { + var urlSuffix = "/api/v1/method.call/deleteMessage"; + var messageResponse = easyRandom.nextObject(StringifiedMessageResponse.class); + messageResponse.setSuccess(true); + if (success) { + while (messageResponse.getMessage().contains("\"error\"")) { + messageResponse.setMessage(RandomStringUtils.randomAlphanumeric(32)); + } + } else { + messageResponse.setMessage( + RandomStringUtils.randomAlphanumeric(12) + + "\"error\"" + + RandomStringUtils.randomAlphanumeric(12)); + } + + when(restTemplate.postForEntity( + endsWith(urlSuffix), any(HttpEntity.class), eq(StringifiedMessageResponse.class))) + .thenReturn(ResponseEntity.ok(messageResponse)); + } + private void givenAWronglyFormattedMessageId() { int idLength = 0; while (idLength < 1 || idLength == 17) { From 069fc0456372391ffdcdc2a7083a89997bd76d6d Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 10:15:20 +0100 Subject: [PATCH 3/9] fix: adjust to delete-attachment flag --- .../caritas/cob/messageservice/Messenger.java | 2 +- .../api/controller/MessageController.java | 10 +++++----- .../rocket/chat/message/MessagesDTO.java | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/main/java/de/caritas/cob/messageservice/Messenger.java b/src/main/java/de/caritas/cob/messageservice/Messenger.java index e2f2efc..99d8935 100644 --- a/src/main/java/de/caritas/cob/messageservice/Messenger.java +++ b/src/main/java/de/caritas/cob/messageservice/Messenger.java @@ -269,7 +269,7 @@ public boolean deleteMessage(String rcToken, String rcUserId, String messageId) return rocketChatService.deleteMessage(rcToken, rcUserId, messageId); } - public boolean deleteAttachment(String messageId) { + public boolean deleteAttachment(String attachmentId) { return false; } } diff --git a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java index 77083e7..d0cc117 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java +++ b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java @@ -294,16 +294,16 @@ public ResponseEntity patchMessage(String rcToken, String rcUserId, String @Override public ResponseEntity deleteMessage(String rcToken, String rcUserId, String messageId, Boolean deleteAttachment) { - var creatorId = messenger + var message = messenger .findMessage(rcToken, rcUserId, messageId) .map(mapper::messageDtoOf) - .map(messageDto -> messageDto.getU().get_id()); + .orElse(null); - if (creatorId.isEmpty()) { + if (isNull(message)) { return ResponseEntity.notFound().build(); } - if (!creatorId.get().equals(rcUserId)) { + if (!rcUserId.equals(message.getCreatorId())) { return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); } @@ -311,7 +311,7 @@ public ResponseEntity deleteMessage(String rcToken, String rcUserId, Strin return ResponseEntity.internalServerError().build(); } - return deleteAttachment && !messenger.deleteAttachment() + return deleteAttachment && message.hasFile() && !messenger.deleteAttachment(message.getFileId()) ? ResponseEntity.status(HttpStatus.MULTI_STATUS).build() : ResponseEntity.noContent().build(); } diff --git a/src/main/java/de/caritas/cob/messageservice/api/model/rocket/chat/message/MessagesDTO.java b/src/main/java/de/caritas/cob/messageservice/api/model/rocket/chat/message/MessagesDTO.java index b30106a..dd62d0d 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/model/rocket/chat/message/MessagesDTO.java +++ b/src/main/java/de/caritas/cob/messageservice/api/model/rocket/chat/message/MessagesDTO.java @@ -1,5 +1,8 @@ package de.caritas.cob.messageservice.api.model.rocket.chat.message; +import static java.util.Objects.isNull; +import static java.util.Objects.nonNull; + import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import de.caritas.cob.messageservice.api.model.AliasMessageDTO; @@ -11,6 +14,7 @@ import lombok.AllArgsConstructor; import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.NonNull; import lombok.Setter; /** @@ -72,4 +76,19 @@ public class MessagesDTO { @ApiModelProperty private String org; + + @JsonIgnore + public @NonNull String getCreatorId() { + return u.get_id(); + } + + @JsonIgnore + public String getFileId() { + return isNull(file) ? null : file.getId(); + } + + @JsonIgnore + public boolean hasFile() { + return nonNull(file) && nonNull(file.getId()); + } } From 6f43213d9fdd8f6bc47dabccaf01b7e4b06790b0 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 11:11:10 +0100 Subject: [PATCH 4/9] feat: delete attachment --- .../caritas/cob/messageservice/Messenger.java | 4 +- .../api/controller/MessageController.java | 10 +- .../api/service/MessageMapper.java | 29 ++++- .../api/service/RocketChatService.java | 16 +++ .../{DeleteMessage.java => MethodCall.java} | 2 +- .../dto/MethodMessageWithParamList.java | 16 +++ ...ge.java => MethodMessageWithParamMap.java} | 4 +- .../controller/MessageControllerE2EIT.java | 103 +++++++++++++++--- 8 files changed, 155 insertions(+), 29 deletions(-) rename src/main/java/de/caritas/cob/messageservice/api/service/dto/{DeleteMessage.java => MethodCall.java} (79%) create mode 100644 src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamList.java rename src/main/java/de/caritas/cob/messageservice/api/service/dto/{MethodMessage.java => MethodMessageWithParamMap.java} (74%) diff --git a/src/main/java/de/caritas/cob/messageservice/Messenger.java b/src/main/java/de/caritas/cob/messageservice/Messenger.java index 99d8935..d8e7539 100644 --- a/src/main/java/de/caritas/cob/messageservice/Messenger.java +++ b/src/main/java/de/caritas/cob/messageservice/Messenger.java @@ -269,7 +269,7 @@ public boolean deleteMessage(String rcToken, String rcUserId, String messageId) return rocketChatService.deleteMessage(rcToken, rcUserId, messageId); } - public boolean deleteAttachment(String attachmentId) { - return false; + public boolean deleteAttachment(String rcToken, String rcUserId, String attachmentId) { + return rocketChatService.deleteAttachment(rcToken, rcUserId, attachmentId); } } diff --git a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java index d0cc117..937cdb7 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java +++ b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java @@ -311,9 +311,13 @@ public ResponseEntity deleteMessage(String rcToken, String rcUserId, Strin return ResponseEntity.internalServerError().build(); } - return deleteAttachment && message.hasFile() && !messenger.deleteAttachment(message.getFileId()) - ? ResponseEntity.status(HttpStatus.MULTI_STATUS).build() - : ResponseEntity.noContent().build(); + if (deleteAttachment && message.hasFile()) { + if (!messenger.deleteAttachment(rcToken, rcUserId, message.getFileId())) { + return ResponseEntity.status(HttpStatus.MULTI_STATUS).build(); + } + } + + return ResponseEntity.noContent().build(); } /** diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java index b2e1a3e..d540f5d 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java @@ -17,9 +17,10 @@ import de.caritas.cob.messageservice.api.model.jsondeserializer.AliasJsonDeserializer; import de.caritas.cob.messageservice.api.model.rocket.chat.message.MessagesDTO; import de.caritas.cob.messageservice.api.model.rocket.chat.message.SendMessageResponseDTO; -import de.caritas.cob.messageservice.api.service.dto.DeleteMessage; +import de.caritas.cob.messageservice.api.service.dto.MethodCall; import de.caritas.cob.messageservice.api.service.dto.Message; -import de.caritas.cob.messageservice.api.service.dto.MethodMessage; +import de.caritas.cob.messageservice.api.service.dto.MethodMessageWithParamList; +import de.caritas.cob.messageservice.api.service.dto.MethodMessageWithParamMap; import de.caritas.cob.messageservice.api.service.dto.UpdateMessage; import java.time.Instant; import java.util.Date; @@ -201,16 +202,32 @@ public MessageType messageTypeOf(AliasMessageDTO alias) { return null; } - public DeleteMessage deleteMessageOf(String messageId) { + public MethodCall deleteMessageOf(String messageId) { var params = Map.of("_id", messageId); - var message = new MethodMessage(); + var message = new MethodMessageWithParamMap(); message.setParams(List.of(params)); message.setId(new Random().nextInt(100)); - message.setMsg("method"); message.setMethod("deleteMessage"); - var deleteMessage = new DeleteMessage(); + var deleteMessage = new MethodCall(); + try { + var messageString = objectMapper.writeValueAsString(message); + deleteMessage.setMessage(messageString); + } catch (JsonProcessingException e) { + log.error("Serializing {} did not work.", message); + } + + return deleteMessage; + } + + public MethodCall deleteFileOf(String fileId) { + var message = new MethodMessageWithParamList(); + message.setParams(List.of(fileId)); + message.setId(new Random().nextInt(100)); + message.setMethod("deleteFileMessage"); + + var deleteMessage = new MethodCall(); try { var messageString = objectMapper.writeValueAsString(message); deleteMessage.setMessage(messageString); diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java b/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java index 9db6769..a0522a7 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java @@ -55,6 +55,7 @@ public class RocketChatService { public static final String E2E_ENCRYPTION_TYPE = "e2e"; + private static final String ENDPOINT_FILE_DELETE = "/method.call/deleteFileMessage"; private static final String ENDPOINT_MESSAGE_DELETE = "/method.call/deleteMessage"; private static final String ENDPOINT_MESSAGE_GET = "/chat.getMessage?msgId="; private static final String ENDPOINT_MESSAGE_UPDATE = "/chat.update"; @@ -412,6 +413,21 @@ public boolean deleteMessage(String rcToken, String rcUserId, String messageId) } } + public boolean deleteAttachment(String rcToken, String rcUserId, String attachmentId) { + var url = baseUrl + ENDPOINT_FILE_DELETE; + var deleteFile = mapper.deleteFileOf(attachmentId); + var entity = new HttpEntity<>(deleteFile, getRocketChatHeader(rcToken, rcUserId)); + + try { + var response = restTemplate.postForEntity(url, entity, StringifiedMessageResponse.class); + return nonNull(response.getBody()) && response.getBody().getSuccess() + && !response.getBody().getMessage().contains("\"error\""); + } catch (HttpClientErrorException exception) { + log.error("Deleting file failed.", exception); + return false; + } + } + @SuppressWarnings("java:S5852") // Using slow regular expressions is security-sensitive private boolean isRcNotFoundResponse(HttpClientErrorException exception) { return HttpStatus.BAD_REQUEST.equals(exception.getStatusCode()) diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodCall.java similarity index 79% rename from src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java rename to src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodCall.java index 9bfc4fb..3fea4cd 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/dto/DeleteMessage.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodCall.java @@ -3,7 +3,7 @@ import lombok.Data; @Data -public class DeleteMessage { +public class MethodCall { private String message; } diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamList.java b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamList.java new file mode 100644 index 0000000..145d560 --- /dev/null +++ b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamList.java @@ -0,0 +1,16 @@ +package de.caritas.cob.messageservice.api.service.dto; + +import java.util.List; +import lombok.Data; + +@Data +public class MethodMessageWithParamList { + + private String msg = "method"; + + private int id; + + private String method; + + private List params; +} diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamMap.java similarity index 74% rename from src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java rename to src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamMap.java index 1888c93..063e14a 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessage.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/dto/MethodMessageWithParamMap.java @@ -5,9 +5,9 @@ import lombok.Data; @Data -public class MethodMessage { +public class MethodMessageWithParamMap { - private String msg; + private String msg = "method"; private int id; diff --git a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java index 3a4960a..0dbf942 100644 --- a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java +++ b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java @@ -18,6 +18,7 @@ import static org.mockito.ArgumentMatchers.endsWith; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -147,7 +148,6 @@ class MessageControllerE2EIT { private List messages; private ConsultantReassignment consultantReassignment; private String messageId; - private String attachmentId; private AliasArgs aliasArgs; private Message message; private MessagesDTO messagesDTO; @@ -650,10 +650,9 @@ void patchMessageShouldRespondWithNoContent() throws Exception { @Test @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) - void deleteMessageShouldRespondWithNotImplemented() throws Exception { + void deleteMessageShouldRespondWithNotFoundIfMessageDoesNotExist() throws Exception { givenAuthenticatedUser(); givenAValidMessageId(); - givenAValidAttachmentId(); givenAGetChatMessageNotFoundResponse(messageId); mockMvc.perform( @@ -662,17 +661,23 @@ void deleteMessageShouldRespondWithNotImplemented() throws Exception { .header(CSRF_HEADER, CSRF_VALUE) .header("rcToken", RandomStringUtils.randomAlphabetic(16)) .header("rcUserId", RandomStringUtils.randomAlphabetic(16)) - .queryParam("deleteMessage", "true") ) .andExpect(status().isNotFound()); + + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); } @Test @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) - void deleteMessageShouldRespondWithForbiddenGivenMessageIsNotFromUser() throws Exception { + void deleteMessageShouldRespondWithForbiddenIfMessageIsNotFromUser() throws Exception { givenAuthenticatedUser(); givenAValidMessageId(); - givenAValidAttachmentId(); givenAMasterKey(); givenMessage(messageId, true, RandomStringUtils.randomAlphabetic(16)); @@ -682,9 +687,16 @@ void deleteMessageShouldRespondWithForbiddenGivenMessageIsNotFromUser() throws E .header(CSRF_HEADER, CSRF_VALUE) .header("rcToken", RandomStringUtils.randomAlphabetic(16)) .header("rcUserId", RandomStringUtils.randomAlphabetic(16)) - .queryParam("attachmentId", attachmentId) ) .andExpect(status().isForbidden()); + + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); } @Test @@ -692,7 +704,6 @@ void deleteMessageShouldRespondWithForbiddenGivenMessageIsNotFromUser() throws E void deleteMessageShouldRespondWithInternalServerErrorIfDeletionFails() throws Exception { givenAuthenticatedUser(); givenAValidMessageId(); - givenAValidAttachmentId(); givenAMasterKey(); var rcUserId = RandomStringUtils.randomAlphabetic(16); givenMessage(messageId, true, rcUserId); @@ -704,21 +715,28 @@ void deleteMessageShouldRespondWithInternalServerErrorIfDeletionFails() throws E .header(CSRF_HEADER, CSRF_VALUE) .header("rcToken", RandomStringUtils.randomAlphabetic(16)) .header("rcUserId", rcUserId) - .queryParam("attachmentId", attachmentId) ) .andExpect(status().isInternalServerError()); + + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); } @Test @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) - void deleteMessageShouldRespondWithNoContentIfDeletionSucceeds() throws Exception { + void deleteMessageShouldRespondWithNoContentIfDeleteMessageSucceeds() throws Exception { givenAuthenticatedUser(); givenAValidMessageId(); - givenAValidAttachmentId(); givenAMasterKey(); var rcUserId = RandomStringUtils.randomAlphabetic(16); givenMessage(messageId, true, rcUserId); givenDeletableMessage(true); + givenDeletableFile(true); mockMvc.perform( delete("/messages/{messageId}", messageId) @@ -728,6 +746,45 @@ void deleteMessageShouldRespondWithNoContentIfDeletionSucceeds() throws Exceptio .header("rcUserId", rcUserId) ) .andExpect(status().isNoContent()); + + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); + } + + @Test + @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) + void deleteMessageShouldRespondWithNoContentIfDeleteMessageAndAttachmentSucceed() + throws Exception { + givenAuthenticatedUser(); + givenAValidMessageId(); + givenAMasterKey(); + var rcUserId = RandomStringUtils.randomAlphabetic(16); + givenMessage(messageId, true, rcUserId); + givenDeletableMessage(true); + givenDeletableFile(true); + + mockMvc.perform( + delete("/messages/{messageId}", messageId) + .cookie(CSRF_COOKIE) + .header(CSRF_HEADER, CSRF_VALUE) + .header("rcToken", RandomStringUtils.randomAlphabetic(16)) + .header("rcUserId", rcUserId) + .queryParam("deleteAttachment", "true") + ) + .andExpect(status().isNoContent()); + + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); } @Test @@ -1199,6 +1256,26 @@ private void givenDeletableMessage(boolean success) { .thenReturn(ResponseEntity.ok(messageResponse)); } + private void givenDeletableFile(boolean success) { + var urlSuffix = "/api/v1/method.call/deleteFileMessage"; + var messageResponse = easyRandom.nextObject(StringifiedMessageResponse.class); + messageResponse.setSuccess(true); + if (success) { + while (messageResponse.getMessage().contains("\"error\"")) { + messageResponse.setMessage(RandomStringUtils.randomAlphanumeric(32)); + } + } else { + messageResponse.setMessage( + RandomStringUtils.randomAlphanumeric(12) + + "\"error\"" + + RandomStringUtils.randomAlphanumeric(12)); + } + + when(restTemplate.postForEntity( + endsWith(urlSuffix), any(HttpEntity.class), eq(StringifiedMessageResponse.class))) + .thenReturn(ResponseEntity.ok(messageResponse)); + } + private void givenAWronglyFormattedMessageId() { int idLength = 0; while (idLength < 1 || idLength == 17) { @@ -1211,10 +1288,6 @@ private void givenAValidMessageId() { messageId = RandomStringUtils.randomAlphanumeric(17); } - private void givenAValidAttachmentId() { - attachmentId = RandomStringUtils.randomAlphanumeric(17); - } - private void givenAPatchSupportedReassignArg() { aliasArgs = new AliasArgs(); var status = easyRandom.nextBoolean() ? ReassignStatus.REJECTED : ReassignStatus.CONFIRMED; From 4034a60c4757075e71c16e42f74d416e0c893fd6 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 13:27:48 +0100 Subject: [PATCH 5/9] chore: extract success test --- .../api/service/RocketChatService.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java b/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java index a0522a7..2d26745 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/RocketChatService.java @@ -42,6 +42,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestClientException; @@ -405,8 +406,7 @@ public boolean deleteMessage(String rcToken, String rcUserId, String messageId) try { var response = restTemplate.postForEntity(url, entity, StringifiedMessageResponse.class); - return nonNull(response.getBody()) && response.getBody().getSuccess() - && !response.getBody().getMessage().contains("\"error\""); + return isSuccessful(response); } catch (HttpClientErrorException exception) { log.error("Deleting message failed.", exception); return false; @@ -420,14 +420,19 @@ public boolean deleteAttachment(String rcToken, String rcUserId, String attachme try { var response = restTemplate.postForEntity(url, entity, StringifiedMessageResponse.class); - return nonNull(response.getBody()) && response.getBody().getSuccess() - && !response.getBody().getMessage().contains("\"error\""); + return isSuccessful(response); } catch (HttpClientErrorException exception) { log.error("Deleting file failed.", exception); return false; } } + private boolean isSuccessful(ResponseEntity response) { + var body = response.getBody(); + + return nonNull(body) && body.getSuccess() && !body.getMessage().contains("\"error\""); + } + @SuppressWarnings("java:S5852") // Using slow regular expressions is security-sensitive private boolean isRcNotFoundResponse(HttpClientErrorException exception) { return HttpStatus.BAD_REQUEST.equals(exception.getStatusCode()) From bd5bad9bc24e472c1bef8664b412b7e1afc23ee6 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 15:17:20 +0100 Subject: [PATCH 6/9] test: delete attachment failure --- .../controller/MessageControllerE2EIT.java | 70 ++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java index 0dbf942..801ee08 100644 --- a/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java +++ b/src/test/java/de/caritas/cob/messageservice/api/controller/MessageControllerE2EIT.java @@ -787,6 +787,67 @@ void deleteMessageShouldRespondWithNoContentIfDeleteMessageAndAttachmentSucceed( ); } + @Test + @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) + void deleteMessageShouldRespondWithNoContentIfDeleteMessageSucceedsButMessageHasNoFile() + throws Exception { + givenAuthenticatedUser(); + givenAValidMessageId(); + givenAMasterKey(); + var rcUserId = RandomStringUtils.randomAlphabetic(16); + givenMessage(messageId, true, rcUserId, false); + givenDeletableMessage(true); + + mockMvc.perform( + delete("/messages/{messageId}", messageId) + .cookie(CSRF_COOKIE) + .header(CSRF_HEADER, CSRF_VALUE) + .header("rcToken", RandomStringUtils.randomAlphabetic(16)) + .header("rcUserId", rcUserId) + .queryParam("deleteAttachment", "true") + ) + .andExpect(status().isNoContent()); + + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate, never()).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); + } + + @Test + @WithMockUser(authorities = AuthorityValue.USER_DEFAULT) + void deleteMessageShouldRespondWithMultiStatusIfDeleteMessageSucceedsButDeleteAttachmentFails() + throws Exception { + givenAuthenticatedUser(); + givenAValidMessageId(); + givenAMasterKey(); + var rcUserId = RandomStringUtils.randomAlphabetic(16); + givenMessage(messageId, true, rcUserId); + givenDeletableMessage(true); + givenDeletableFile(false); + + mockMvc.perform( + delete("/messages/{messageId}", messageId) + .cookie(CSRF_COOKIE) + .header(CSRF_HEADER, CSRF_VALUE) + .header("rcToken", RandomStringUtils.randomAlphabetic(16)) + .header("rcUserId", rcUserId) + .queryParam("deleteAttachment", "true") + ) + .andExpect(status().isMultiStatus()); + + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteMessage"), any(), eq(StringifiedMessageResponse.class) + ); + verify(restTemplate).postForEntity( + endsWith("/api/v1/method.call/deleteFileMessage"), any(), + eq(StringifiedMessageResponse.class) + ); + } + @Test @WithMockUser(authorities = {AuthorityValue.USER_DEFAULT}) void sendMessageShouldTransmitTypeOfMessage() throws Exception { @@ -1199,6 +1260,11 @@ private void givenMessage(String id, boolean full) } private void givenMessage(String id, boolean full, String userId) + throws CustomCryptoException, JsonProcessingException { + givenMessage(id, full, userId, true); + } + + private void givenMessage(String id, boolean full, String userId, boolean hasFileId) throws JsonProcessingException, CustomCryptoException { var response = new MessageResponse(); response.setSuccess(true); @@ -1222,7 +1288,9 @@ private void givenMessage(String id, boolean full, String userId) var props = message.getOtherProperties(); props.put("u", messagesDTO.getU()); props.put("attachments", messagesDTO.getAttachments()); - props.put("file", messagesDTO.getFile()); + if (hasFileId) { + props.put("file", messagesDTO.getFile()); + } props.put("org", encryptionService.encrypt(message.getMsg(), message.getRid())); props.put("_updatedAt", messagesDTO.get_updatedAt()); props.put("t", messagesDTO.getT()); From fa4d0ede88359dd492bbb7791252dd68fe9f3126 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 15:51:13 +0100 Subject: [PATCH 7/9] chore: reuse random object --- .../cob/messageservice/api/service/MessageMapper.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java index d540f5d..fade657 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java @@ -36,6 +36,8 @@ @RequiredArgsConstructor public class MessageMapper { + private static final Random random = new Random(); + private final ObjectMapper objectMapper; private final EncryptionService encryptionService; @@ -202,12 +204,14 @@ public MessageType messageTypeOf(AliasMessageDTO alias) { return null; } + @SuppressWarnings("java:S2245") + // Using pseudorandom number generators (PRNGs) is security-sensitive public MethodCall deleteMessageOf(String messageId) { var params = Map.of("_id", messageId); var message = new MethodMessageWithParamMap(); message.setParams(List.of(params)); - message.setId(new Random().nextInt(100)); + message.setId(random.nextInt(100)); message.setMethod("deleteMessage"); var deleteMessage = new MethodCall(); @@ -221,10 +225,12 @@ public MethodCall deleteMessageOf(String messageId) { return deleteMessage; } + @SuppressWarnings("java:S2245") + // Using pseudorandom number generators (PRNGs) is security-sensitive public MethodCall deleteFileOf(String fileId) { var message = new MethodMessageWithParamList(); message.setParams(List.of(fileId)); - message.setId(new Random().nextInt(100)); + message.setId(random.nextInt(100)); message.setMethod("deleteFileMessage"); var deleteMessage = new MethodCall(); From 1d741e5ca75362687612ce4d849a1b4f3d6abf49 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 15:54:19 +0100 Subject: [PATCH 8/9] chore: merge if statements --- .../messageservice/api/controller/MessageController.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java index 937cdb7..5bdc509 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java +++ b/src/main/java/de/caritas/cob/messageservice/api/controller/MessageController.java @@ -311,10 +311,9 @@ public ResponseEntity deleteMessage(String rcToken, String rcUserId, Strin return ResponseEntity.internalServerError().build(); } - if (deleteAttachment && message.hasFile()) { - if (!messenger.deleteAttachment(rcToken, rcUserId, message.getFileId())) { - return ResponseEntity.status(HttpStatus.MULTI_STATUS).build(); - } + if (deleteAttachment && message.hasFile() + && !messenger.deleteAttachment(rcToken, rcUserId, message.getFileId())) { + return ResponseEntity.status(HttpStatus.MULTI_STATUS).build(); } return ResponseEntity.noContent().build(); From e91a850ec46abdc7d4c6870a390a1f214ad8e801 Mon Sep 17 00:00:00 2001 From: Torsten Krohn Date: Mon, 31 Oct 2022 15:57:24 +0100 Subject: [PATCH 9/9] chore: double-check random object --- .../cob/messageservice/api/service/MessageMapper.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java index fade657..c517409 100644 --- a/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java +++ b/src/main/java/de/caritas/cob/messageservice/api/service/MessageMapper.java @@ -36,6 +36,8 @@ @RequiredArgsConstructor public class MessageMapper { + @SuppressWarnings("java:S2245") + // Using pseudorandom number generators (PRNGs) is security-sensitive private static final Random random = new Random(); private final ObjectMapper objectMapper; @@ -204,8 +206,6 @@ public MessageType messageTypeOf(AliasMessageDTO alias) { return null; } - @SuppressWarnings("java:S2245") - // Using pseudorandom number generators (PRNGs) is security-sensitive public MethodCall deleteMessageOf(String messageId) { var params = Map.of("_id", messageId); @@ -225,8 +225,6 @@ public MethodCall deleteMessageOf(String messageId) { return deleteMessage; } - @SuppressWarnings("java:S2245") - // Using pseudorandom number generators (PRNGs) is security-sensitive public MethodCall deleteFileOf(String fileId) { var message = new MethodMessageWithParamList(); message.setParams(List.of(fileId));