Skip to content

Commit 2429ca3

Browse files
committed
2024-10-23 - feedback - external reviewer - server-side
1 parent af123be commit 2429ca3

File tree

2 files changed

+48
-15
lines changed

2 files changed

+48
-15
lines changed

server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,14 @@ public void delete(UUID id) {
336336

337337
@Override
338338
public FeedbackRequest getById(UUID id) {
339+
MemberProfile currentUser;
340+
341+
try {
342+
currentUser = currentUserServices.getCurrentUser();
343+
} catch (NotFoundException notFoundException) {
344+
currentUser = null;
345+
}
346+
339347
final Optional<FeedbackRequest> feedbackReq = feedbackReqRepository.findById(id);
340348
if (feedbackReq.isEmpty()) {
341349
throw new NotFoundException("No feedback req with id " + id);
@@ -344,8 +352,9 @@ public FeedbackRequest getById(UUID id) {
344352
final UUID requesteeId = feedbackReq.get().getRequesteeId();
345353
final UUID recipientId = feedbackReq.get().getRecipientId();
346354
final UUID externalRecipientId = feedbackReq.get().getExternalRecipientId();
347-
if (recipientId != null) {
348-
if (!getIsPermitted(requesteeId, recipientId, sendDate)) {
355+
final UUID recipientOrExternalRecipientId = (recipientId != null) ? recipientId : externalRecipientId;
356+
if (currentUser != null) {
357+
if (!getIsPermitted(requesteeId, recipientOrExternalRecipientId, sendDate)) {
349358
throw new PermissionException(NOT_AUTHORIZED_MSG);
350359
}
351360
} else {
@@ -420,14 +429,16 @@ private boolean createIsPermitted(UUID requesteeId) {
420429

421430
private boolean getIsPermitted(UUID requesteeId, UUID recipientOrExternalRecipientId, LocalDate sendDate) {
422431
LocalDate today = LocalDate.now();
432+
boolean getIsPermittedReturn;
423433
final UUID currentUserId = currentUserServices.getCurrentUser().getId();
424434

425435
// The recipient can only access the feedback request after it has been sent
426436
if (sendDate.isAfter(today) && currentUserId.equals(recipientOrExternalRecipientId)) {
427437
throw new PermissionException("You are not permitted to access this request before the send date.");
428438
}
429439

430-
return createIsPermitted(requesteeId) || currentUserId.equals(recipientOrExternalRecipientId);
440+
getIsPermittedReturn = createIsPermitted(requesteeId) || currentUserId.equals(recipientOrExternalRecipientId);
441+
return getIsPermittedReturn;
431442
}
432443

433444
private boolean getIsPermittedForExternalRecipient(UUID requesteeId, LocalDate sendDate) {

server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestControllerTest.java

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -805,11 +805,10 @@ void testGetFeedbackRequestByUnassignedPdlToExternalRecipient() {
805805
//get feedback request
806806
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
807807
.basicAuth(unrelatedPdl.getWorkEmail(), RoleType.Constants.PDL_ROLE);
808-
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
808+
final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () ->
809+
client.toBlocking().exchange(request, Map.class));
809810

810-
assertEquals(HttpStatus.OK, response.getStatus());
811-
assertTrue(response.getBody().isPresent());
812-
assertResponseEqualsEntity(feedbackRequest, response.getBody().get());
811+
assertUnauthorized(responseException);
813812
}
814813

815814
@Test
@@ -843,11 +842,11 @@ void testGetFeedbackRequestByRequesteeToExternalRecipient() {
843842
//get feedback request
844843
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
845844
.basicAuth(memberProfile2.getWorkEmail(), RoleType.Constants.MEMBER_ROLE);
846-
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
845+
final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () ->
846+
client.toBlocking().exchange(request, Map.class));
847847

848-
assertEquals(HttpStatus.OK, response.getStatus());
849-
assertTrue(response.getBody().isPresent());
850-
assertResponseEqualsEntity(feedbackRequest, response.getBody().get());
848+
// requestee should not be able to get the feedback request about them
849+
assertUnauthorized(responseException);
851850
}
852851

853852
@Test
@@ -878,9 +877,8 @@ void testGetFeedbackRequestByExternalRecipient() {
878877
FeedbackRequest feedbackRequest = saveFeedbackRequest(pdlMemberProfile, requestee, externalRecipient);
879878

880879
//get feedback request
881-
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
882-
.basicAuth(externalRecipient.getEmail(), RoleType.Constants.MEMBER_ROLE);
883-
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
880+
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()));
881+
final HttpResponse<FeedbackRequestResponseDTO> response = clientExternalRecipient.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
884882

885883
// recipient must be able to get the feedback request
886884
assertEquals(HttpStatus.OK, response.getStatus());
@@ -2365,7 +2363,7 @@ void testRecipientGetBeforeSendDateNotAuthorizedToExternalRecipient() {
23652363
}
23662364

23672365
@Test
2368-
void testRecipientGetBeforeSendDateAsAdminAuthorized() {
2366+
void testRecipientGetBeforeSendDateAsAdminAuthorizedToRecipient() {
23692367
MemberProfile pdlMemberProfile = createADefaultMemberProfile();
23702368
MemberProfile requestee = createADefaultMemberProfileForPdl(pdlMemberProfile);
23712369
MemberProfile recipient = createADefaultRecipient();
@@ -2388,6 +2386,30 @@ void testRecipientGetBeforeSendDateAsAdminAuthorized() {
23882386
assertResponseEqualsEntity(feedbackRequest, response.getBody().get());
23892387
}
23902388

2389+
@Test
2390+
void testRecipientGetBeforeSendDateAsAdminAuthorizedToExternalRecipient() {
2391+
MemberProfile pdlMemberProfile = createADefaultMemberProfile();
2392+
MemberProfile requestee = createADefaultMemberProfileForPdl(pdlMemberProfile);
2393+
final FeedbackExternalRecipient externalRecipient01 = createADefaultFeedbackExternalRecipient();
2394+
MemberProfile adminUser = createAThirdDefaultMemberProfile();
2395+
assignAdminRole(adminUser);
2396+
2397+
// Save feedback request with send date in the future
2398+
FeedbackRequest feedbackRequest = createFeedbackRequest(pdlMemberProfile, requestee, externalRecipient01);
2399+
feedbackRequest.setSendDate(LocalDate.now().plusDays(1));
2400+
getFeedbackRequestRepository().save(feedbackRequest);
2401+
2402+
//get feedback request
2403+
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
2404+
.basicAuth(adminUser.getWorkEmail(), RoleType.Constants.ADMIN_ROLE);
2405+
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
2406+
2407+
// the sendDate must be before the sent date unless its an admin
2408+
assertEquals(HttpStatus.OK, response.getStatus());
2409+
assertTrue(response.getBody().isPresent());
2410+
assertResponseEqualsEntity(feedbackRequest, response.getBody().get());
2411+
}
2412+
23912413
@Test
23922414
void testRecipientGetBeforeSendDateAsPdlAuthorized() {
23932415
MemberProfile pdlMemberProfile = createADefaultMemberProfile();

0 commit comments

Comments
 (0)