From b0b5b97a278591f257df243efce35c45a58fb579 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Fri, 24 Jan 2025 09:03:14 -0600 Subject: [PATCH 1/9] Initial version with most @RequiredPermission annotations moved to the services implementations. --- .../action_item/ActionItemController.java | 7 -- .../action_item/ActionItemServicesImpl.java | 7 ++ .../agenda_item/AgendaItemController.java | 8 +- .../agenda_item/AgendaItemServicesImpl.java | 8 +- .../CertificationController.java | 4 - .../CertificationServiceImpl.java | 3 + .../checkin_notes/CheckinNoteController.java | 8 +- .../CheckinNoteServicesImpl.java | 8 +- .../CheckinDocumentController.java | 8 +- .../CheckinDocumentServicesImpl.java | 37 ++++++ .../services/checkins/CheckInController.java | 6 - .../checkins/CheckInServicesImpl.java | 8 +- .../services/document/DocumentController.java | 6 - .../document/DocumentServiceImpl.java | 6 + .../services/email/EmailController.java | 4 - .../services/email/EmailServicesImpl.java | 3 + .../EmployeeHoursController.java | 3 - .../EmployeeHoursServicesImpl.java | 3 + .../FeedbackAnswerController.java | 4 - .../FeedbackAnswerServicesImpl.java | 23 +++- .../FeedbackRequestController.java | 6 - .../FeedbackRequestServicesImpl.java | 41 +++++-- .../services/file/FileServicesBaseImpl.java | 5 +- .../services/kudos/KudosController.java | 5 - .../services/kudos/KudosServicesImpl.java | 4 + .../skillsreport/SkillsReportController.java | 3 - .../SkillsReportServicesImpl.java | 5 +- .../MemberProfileController.java | 4 - .../MemberProfileServicesImpl.java | 3 + .../AnniversaryReportController.java | 3 - .../AnniversaryReportServicesImpl.java | 3 + .../birthday/BirthDayController.java | 3 - .../birthday/BirthDayServicesImpl.java | 3 + .../MemberProfileReportController.java | 6 +- .../MemberProfileReportServicesImpl.java | 4 + .../currentuser/CurrentUserServicesImpl.java | 6 +- .../RetentionReportController.java | 3 - .../RetentionReportServicesImpl.java | 3 + .../services/permissions/Permission.java | 2 + .../permissions/RequiredPermission.java | 8 ++ .../RequiredPermissionInterceptor.java | 34 ++++++ .../private_notes/PrivateNoteController.java | 8 +- .../PrivateNoteServicesImpl.java | 6 + .../services/reports/MarkdownGeneration.java | 11 +- .../reports/ReportDataController.java | 4 - .../reports/ReportDataServicesImpl.java | 3 + .../reviews/ReviewAssignmentController.java | 8 -- .../reviews/ReviewAssignmentServicesImpl.java | 8 ++ .../reviews/ReviewPeriodController.java | 9 +- .../reviews/ReviewPeriodServicesImpl.java | 14 ++- .../services/settings/SettingOption.java | 5 + .../services/settings/SettingsController.java | 8 -- .../settings/SettingsServicesImpl.java | 12 ++ .../skill_record/SkillRecordController.java | 6 +- .../skill_record/SkillRecordServicesImpl.java | 4 + .../SkillCategoryController.java | 7 -- .../SkillCategoryServicesImpl.java | 7 ++ .../SkillCategorySkillController.java | 4 - .../SkillCategorySkillServicesImpl.java | 5 + .../services/skills/SkillController.java | 4 - .../services/skills/SkillServicesImpl.java | 4 + .../VolunteeringOrganizationController.java | 3 - .../volunteering/VolunteeringServiceImpl.java | 6 +- .../resources/db/dev/R__Load_testing_data.sql | 10 ++ .../CurrentUserServicesReplacement.java | 34 +++++- .../CheckinDocumentServiceImplTest.java | 11 +- .../document/DocumentationControllerTest.java | 27 +++-- .../services/email/EmailControllerTest.java | 6 +- .../feedback_request/FeedbackRequestTest.java | 14 ++- .../services/file/FileControllerTest.java | 13 ++- .../services/file/FileServicesImplTest.java | 38 +++---- .../services/fixture/PermissionFixture.java | 2 + .../checkins/services/guild/GuildTest.java | 41 +++---- .../SkillsReportServicesImplTest.java | 27 ++++- .../memberprofile/MemberProfileTest.java | 11 +- .../MemberProfileReportServicesImplTest.java | 28 ++++- .../services/pulse/PulseServicesTest.java | 6 + .../reviews/ReviewPeriodControllerTest.java | 106 ++++++++++-------- .../SkillCategoryControllerTest.java | 7 +- .../SkillCategorySkillControllerTest.java | 14 ++- 80 files changed, 550 insertions(+), 309 deletions(-) create mode 100644 server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermissionInterceptor.java diff --git a/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemController.java b/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemController.java index f84e45817b..a9f78dca4c 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.action_item; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -35,7 +33,6 @@ public ActionItemController(ActionItemServices actionItemServices) { * @return {@link HttpResponse } */ @Post - @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public HttpResponse createActionItem(@Body @Valid ActionItemCreateDTO actionItem, HttpRequest request) { ActionItem newActionItem = actionItemServices.save(new ActionItem(actionItem.getCheckinid(), @@ -53,7 +50,6 @@ public HttpResponse createActionItem(@Body @Valid ActionItemCreateDT * @return {@link HttpResponse< ActionItem >} */ @Put - @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public HttpResponse updateActionItem(@Body @Valid ActionItem actionItem, HttpRequest request) { ActionItem updatedActionItem = actionItemServices.update(actionItem); return HttpResponse @@ -70,7 +66,6 @@ public HttpResponse updateActionItem(@Body @Valid ActionItem actionItem, Http * @param id, id of {@link ActionItem} to delete */ @Delete("/{id}") - @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public HttpResponse deleteActionItem(UUID id) { actionItemServices.delete(id); return HttpResponse @@ -84,7 +79,6 @@ public HttpResponse deleteActionItem(UUID id) { * @return {@link ActionItem} */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public ActionItem readActionItem(UUID id) { return actionItemServices.read(id); } @@ -97,7 +91,6 @@ public ActionItem readActionItem(UUID id) { * @return {@link List < CheckIn > list of checkins} */ @Get("/{?checkinid,createdbyid}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findActionItems(@Nullable UUID checkinid, @Nullable UUID createdbyid) { return actionItemServices.findByFields(checkinid, createdbyid); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemServicesImpl.java index 23e6e305e3..850332b2a4 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/action_item/ActionItemServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.action_item; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.validate.crud.CRUDValidator; import jakarta.inject.Named; import jakarta.inject.Singleton; @@ -24,6 +26,7 @@ public ActionItemServicesImpl(ActionItemRepository actionItemRepo, this.crudValidator = crudValidator; } + @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public ActionItem save(@Valid @NotNull ActionItem actionItem) { ActionItem actionItemRet; @@ -45,6 +48,7 @@ public ActionItem save(@Valid @NotNull ActionItem actionItem) { } + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public ActionItem read(@NotNull UUID id) { ActionItem actionItemResult = actionItemRepo.findById(id).orElse(null); @@ -56,6 +60,7 @@ public ActionItem read(@NotNull UUID id) { } + @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public ActionItem update(@Valid @NotNull ActionItem actionItem) { ActionItem actionItemRet = null; @@ -68,6 +73,7 @@ public ActionItem update(@Valid @NotNull ActionItem actionItem) { } + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findByFields(UUID checkinid, UUID createdbyid) { crudValidator.validatePermissionsFindByFields(checkinid, createdbyid); @@ -79,6 +85,7 @@ public Set findByFields(UUID checkinid, UUID createdbyid) { } + @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public void delete(@NotNull UUID id) { ActionItem actionItemResult = actionItemRepo.findById(id).orElse(null); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemController.java b/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemController.java index 64224b4fbc..cc76d271fb 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemController.java @@ -2,8 +2,6 @@ import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.services.checkins.CheckIn; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; @@ -46,7 +44,6 @@ class AgendaItemController { * @return {@link HttpResponse } */ @Post("/") - @RequiredPermission(Permission.CAN_CREATE_CHECKINS) HttpResponse createAgendaItem(@Body @Valid AgendaItemCreateDTO agendaItem) { AgendaItem createAgendaItem = agendaItemServices.save(new AgendaItem(agendaItem.getCheckinid(), agendaItem.getCreatedbyid(), agendaItem.getDescription())); URI location = UriBuilder.of(PATH).path(createAgendaItem.getId().toString()).build(); @@ -61,7 +58,6 @@ HttpResponse createAgendaItem(@Body @Valid AgendaItemCreateDTO agend * @return {@link HttpResponse} */ @Put("/") - @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) HttpResponse updateAgendaItem(@Body @Valid AgendaItem agendaItem) { if (agendaItem == null) { return HttpResponse.ok(); @@ -81,7 +77,6 @@ HttpResponse updateAgendaItem(@Body @Valid AgendaItem agendaItem) { * @return a Set of {@link CheckIn} */ @Get("/{?checkinid,createdbyid}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) Set findAgendaItems(@Nullable UUID checkinid, @Nullable UUID createdbyid) { return agendaItemServices.findByFields(checkinid, createdbyid); } @@ -93,7 +88,6 @@ Set findAgendaItems(@Nullable UUID checkinid, @Nullable UUID created * @return {@link AgendaItem} */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) AgendaItem readAgendaItem(UUID id) { AgendaItem read = agendaItemServices.read(id); if (read == null) { @@ -112,4 +106,4 @@ AgendaItem readAgendaItem(UUID id) { void deleteAgendaItem(UUID id) { agendaItemServices.delete(id); } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemServicesImpl.java index e677f60770..73d0df824c 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/agenda_item/AgendaItemServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.agenda_item; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.services.checkins.CheckIn; @@ -44,6 +46,7 @@ public AgendaItemServicesImpl(CheckInRepository checkinRepo, } // todo remove manual validations throughout class in favor of jakarta validations at api level. @Override + @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public AgendaItem save(AgendaItem agendaItem) { AgendaItem agendaItemRet = null; if (agendaItem != null) { @@ -82,6 +85,7 @@ public AgendaItem save(AgendaItem agendaItem) { } @Override + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public AgendaItem read(@NotNull UUID id) { final UUID currentUserId = currentUserServices.getCurrentUser().getId(); boolean canViewAllCheckins = checkInServices.canViewAllCheckins(currentUserId); @@ -102,6 +106,7 @@ public AgendaItem read(@NotNull UUID id) { @Override + @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public AgendaItem update(AgendaItem agendaItem) { AgendaItem agendaItemRet = null; @@ -138,6 +143,7 @@ public AgendaItem update(AgendaItem agendaItem) { } @Override + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findByFields(@Nullable UUID checkinId, @Nullable UUID createdById) { MemberProfile currentUser = currentUserServices.getCurrentUser(); if(!checkInServices.doesUserHaveViewAccess(currentUser.getId(), checkinId, createdById)){ @@ -174,4 +180,4 @@ private void validate(boolean isError, String message, Object... args) { throw new BadArgException(String.format(message, args)); } } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationController.java b/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationController.java index a39deff8e9..ea8d807462 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.certification; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpStatus; import io.micronaut.http.annotation.Body; import io.micronaut.http.annotation.Controller; @@ -68,7 +66,6 @@ Certification create(@Body @Valid CertificationDTO certification) { * @return the updated {@link Certification} */ @Put("/{id}") - @RequiredPermission(Permission.CAN_MANAGE_CERTIFICATIONS) Certification update(@NotNull UUID id, @Body @Valid CertificationDTO certification) { return certificationService.updateCertification(new Certification( id, @@ -86,7 +83,6 @@ Certification update(@NotNull UUID id, @Body @Valid CertificationDTO certificati * @return the merged {@link Certification} */ @Post("/merge") - @RequiredPermission(Permission.CAN_MANAGE_CERTIFICATIONS) Certification mergeCertifications(@Valid @Body CertificationMergeDTO certificationMergeDTO) { return certificationService.mergeCertifications(certificationMergeDTO.getSourceId(), certificationMergeDTO.getTargetId()); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationServiceImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationServiceImpl.java index a1af17ac41..888ff48fee 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationServiceImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/certification/CertificationServiceImpl.java @@ -4,6 +4,7 @@ import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.role.role_permissions.RolePermissionServices; import io.micronaut.core.annotation.Nullable; import jakarta.transaction.Transactional; @@ -59,6 +60,7 @@ public Certification saveCertification(Certification certification) { } @Override + @RequiredPermission(Permission.CAN_MANAGE_CERTIFICATIONS) public Certification updateCertification(Certification certification) { // Fail if a certification with the same name already exists (but it's not this one) validate(certificationRepository.getByName(certification.getName()) @@ -109,6 +111,7 @@ public void deleteEarnedCertification(UUID id) { @Override @Transactional + @RequiredPermission(Permission.CAN_MANAGE_CERTIFICATIONS) public Certification mergeCertifications(UUID sourceId, UUID targetId) { Optional target = certificationRepository.findById(targetId); Optional source = certificationRepository.findById(sourceId); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteController.java b/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteController.java index d3361b774b..c78e09d4a5 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.checkin_notes; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -38,7 +36,6 @@ public CheckinNoteController(CheckinNoteServices checkinNoteServices) { * @return */ @Post - @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public HttpResponse createCheckinNote(@Body @Valid CheckinNoteCreateDTO checkinNote, HttpRequest request) { CheckinNote newCheckinNote = checkinNoteServices.save(new CheckinNote(checkinNote.getCheckinid(), checkinNote.getCreatedbyid() , checkinNote.getDescription())); @@ -55,7 +52,6 @@ public HttpResponse createCheckinNote(@Body @Valid CheckinNoteCreat * @return */ @Put - @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public HttpResponse updateCheckinNote(@Body @Valid CheckinNote checkinNote, HttpRequest request) { CheckinNote updateCheckinNote = checkinNoteServices.update(checkinNote); return HttpResponse.ok().headers(headers -> headers.location( @@ -72,7 +68,6 @@ public HttpResponse updateCheckinNote(@Body @Valid CheckinNote chec * @return */ @Get("/{?checkinid,createdbyid}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findCheckinNote(@Nullable UUID checkinid, @Nullable UUID createdbyid) { return checkinNoteServices.findByFields(checkinid, createdbyid); } @@ -84,9 +79,8 @@ public Set findCheckinNote(@Nullable UUID checkinid, @Nullable UUID * @return */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public CheckinNote readCheckinNote(@NotNull UUID id) { return checkinNoteServices.read(id); } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteServicesImpl.java index 2ccfe94cb3..27c6be80c5 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/checkin_notes/CheckinNoteServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.checkin_notes; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -42,6 +44,7 @@ public CheckinNoteServicesImpl(CheckInRepository checkinRepo, CheckInServices ch // todo remove manual validations throughout class in favor of jakarta validations at api level. @Override + @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public CheckinNote save(@NotNull CheckinNote checkinNote) { validate(checkinNote.getId() != null, "Found unexpected id %s for check in note", checkinNote.getId()); @@ -71,6 +74,7 @@ public CheckinNote save(@NotNull CheckinNote checkinNote) { } @Override + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public CheckinNote read(@NotNull UUID id) { final UUID currentUserId = currentUserServices.getCurrentUser().getId(); CheckinNote checkInNoteResult = checkinNoteRepository.findById(id).orElse(null); @@ -90,6 +94,7 @@ public CheckinNote read(@NotNull UUID id) { } @Override + @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public CheckinNote update(@NotNull CheckinNote checkinNote) { final UUID id = checkinNote.getId(); validate(id == null || checkinNoteRepository.findById(id).isEmpty(), "Unable to locate checkin note to update with id %s", checkinNote.getId()); @@ -124,6 +129,7 @@ public CheckinNote update(@NotNull CheckinNote checkinNote) { } @Override + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findByFields(@Nullable UUID checkinId, @Nullable UUID createById) { final UUID currentUserId = currentUserServices.getCurrentUser().getId(); if(!checkinServices.doesUserHaveViewAccess(currentUserId, checkinId, createById)){ @@ -138,4 +144,4 @@ private void validate(boolean isError, String message, Object... args) { throw new BadArgException(String.format(message, args)); } } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentController.java b/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentController.java index 129dfb6fbc..dcf904ca07 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.checkindocument; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.role.RoleType; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpResponse; @@ -38,7 +36,6 @@ class CheckinDocumentController { */ @Get("/{?checkinsId}") - @RequiredPermission(Permission.CAN_VIEW_CHECKIN_DOCUMENT) Set findCheckinDocument(@Nullable UUID checkinsId) { return checkinDocumentService.read(checkinsId); } @@ -51,7 +48,6 @@ Set findCheckinDocument(@Nullable UUID checkinsId) { */ @Post - @RequiredPermission(Permission.CAN_CREATE_CHECKIN_DOCUMENT) HttpResponse createCheckinDocument(@Body @Valid CheckinDocumentCreateDTO checkinDocument) { CheckinDocument createdCheckinDocument = checkinDocumentService.save(new CheckinDocument(checkinDocument.getCheckinsId(), checkinDocument.getUploadDocId())); URI location = UriBuilder.of(PATH).path(createdCheckinDocument.getId().toString()).build(); @@ -65,7 +61,6 @@ HttpResponse createCheckinDocument(@Body @Valid CheckinDocument * @return {@link HttpResponse} */ @Put - @RequiredPermission(Permission.CAN_UPDATE_CHECKIN_DOCUMENT) HttpResponse update(@Body @Valid CheckinDocument checkinDocument) { if (checkinDocument == null) { return HttpResponse.ok(); @@ -83,9 +78,8 @@ HttpResponse update(@Body @Valid CheckinDocument checkinDocument) { * @param checkinsId, id of the checkins record you wish to delete */ @Delete("/{checkinsId}") - @RequiredPermission(Permission.CAN_DELETE_CHECKIN_DOCUMENT) @Status(HttpStatus.NO_CONTENT) void delete(UUID checkinsId) { checkinDocumentService.deleteByCheckinId(checkinsId); } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServicesImpl.java index 28211968c4..3516dc35a1 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServicesImpl.java @@ -1,5 +1,10 @@ package com.objectcomputing.checkins.services.checkindocument; +import com.objectcomputing.checkins.services.checkins.CheckIn; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; +import com.objectcomputing.checkins.exceptions.PermissionException; +import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.checkins.CheckInRepository; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; @@ -11,19 +16,24 @@ import java.util.Set; import java.util.UUID; +import static com.objectcomputing.checkins.services.validate.PermissionsValidation.NOT_AUTHORIZED_MSG; + @Singleton public class CheckinDocumentServicesImpl implements CheckinDocumentServices { private final CheckinDocumentRepository checkinDocumentRepo; private final CheckInRepository checkinRepo; + private final CurrentUserServices currentUserServices; public CheckinDocumentServicesImpl(CheckinDocumentRepository checkinDocumentRepo, CheckInRepository checkinRepo, CurrentUserServices currentUserServices) { this.checkinDocumentRepo = checkinDocumentRepo; this.checkinRepo = checkinRepo; + this.currentUserServices = currentUserServices; } + @RequiredPermission(Permission.CAN_VIEW_CHECKIN_DOCUMENT) public Set read(UUID checkinsId) { Set checkinDocument = new HashSet<>(); @@ -39,9 +49,14 @@ public CheckinDocument getFindByUploadDocId(@NotNull String uploadDocId) { if(cd.isEmpty()) { throw new BadArgException(String.format("CheckinDocument with document id %s does not exist", uploadDocId)); } + + checkPermission(Permission.CAN_VIEW_CHECKIN_DOCUMENT, + cd.get().getCheckinsId()); + return cd.get(); } + @RequiredPermission(Permission.CAN_CREATE_CHECKIN_DOCUMENT) public CheckinDocument save(CheckinDocument checkinDocument) { CheckinDocument newCheckinDocument = null; @@ -63,6 +78,7 @@ public CheckinDocument save(CheckinDocument checkinDocument) { return newCheckinDocument; } + @RequiredPermission(Permission.CAN_UPDATE_CHECKIN_DOCUMENT) public CheckinDocument update(CheckinDocument checkinDocument) { CheckinDocument updatedCheckinDocument = null; @@ -83,6 +99,7 @@ public CheckinDocument update(CheckinDocument checkinDocument) { return updatedCheckinDocument; } + @RequiredPermission(Permission.CAN_DELETE_CHECKIN_DOCUMENT) public void deleteByCheckinId(@NotNull UUID checkinsId) { if(!checkinDocumentRepo.existsByCheckinsId(checkinsId)) { @@ -97,7 +114,27 @@ public void deleteByUploadDocId(@NotNull String uploadDocId) { if(!checkinDocumentRepo.existsByUploadDocId(uploadDocId)) { throw new BadArgException(String.format("CheckinDocument with uploadDocId %s does not exist", uploadDocId)); } else { + Optional cd = + checkinDocumentRepo.findByUploadDocId(uploadDocId); + checkPermission(Permission.CAN_DELETE_CHECKIN_DOCUMENT, + cd.get().getCheckinsId()); + checkinDocumentRepo.deleteByUploadDocId(uploadDocId); } } + + private void checkPermission(Permission permission, UUID checkinId) { + if (currentUserServices.hasPermission(permission)) { + return; + } + + Optional checkIn = checkinRepo.findById(checkinId); + MemberProfile currentUser = currentUserServices.getCurrentUser(); + if (checkIn.isPresent() && currentUser != null && + checkIn.get().getPdlId().equals(currentUser.getId())) { + return; + } + + throw new PermissionException(NOT_AUTHORIZED_MSG); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInController.java b/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInController.java index 246b8193c2..b25bb8f5ed 100755 --- a/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.checkins; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -42,7 +40,6 @@ public CheckInController(CheckInServices checkInServices) { * @return */ @Get("/{?teamMemberId,pdlId,completed}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findCheckIns(@Nullable UUID teamMemberId, @Nullable UUID pdlId, @Nullable Boolean completed) { return checkInServices.findByFields(teamMemberId, pdlId, completed); } @@ -54,7 +51,6 @@ public Set findCheckIns(@Nullable UUID teamMemberId, @Nullable UUID pdl * @return {@link HttpResponse} */ @Post - @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public HttpResponse createCheckIn(@Body @Valid CheckInCreateDTO checkIn, HttpRequest request) { CheckIn createdCheckIn = checkInServices.save(new CheckIn(checkIn.getTeamMemberId(), checkIn.getPdlId(), checkIn.getCheckInDate(), checkIn.isCompleted())); return HttpResponse.created(createdCheckIn) @@ -68,7 +64,6 @@ public HttpResponse createCheckIn(@Body @Valid CheckInCreateDTO checkIn * @return {@link HttpResponse} */ @Put - @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public HttpResponse update(@Body @Valid @NotNull CheckIn checkIn, HttpRequest request) { CheckIn updatedCheckIn = checkInServices.update(checkIn); return HttpResponse.ok(updatedCheckIn) @@ -80,7 +75,6 @@ public HttpResponse update(@Body @Valid @NotNull CheckIn checkIn, HttpR * @return {@link CheckIn} the check-in */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public CheckIn readCheckIn(@NotNull UUID id) { return checkInServices.read(id); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInServicesImpl.java index 00801fb840..ae3e514185 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/checkins/CheckInServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.checkins; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; @@ -90,6 +92,7 @@ public boolean doesUserHaveViewAccess(UUID currentUserId, UUID checkinId, UUID c } @Override + @RequiredPermission(Permission.CAN_CREATE_CHECKINS) public CheckIn save(@NotNull CheckIn checkIn) { validate(checkIn.getId() != null, "Found unexpected id for checkin %s", checkIn.getId()); @@ -116,6 +119,7 @@ public CheckIn save(@NotNull CheckIn checkIn) { } @Override + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public CheckIn read(@NotNull UUID checkinId) { UUID currentUserId = currentUserServices.getCurrentUser().getId(); @@ -125,6 +129,7 @@ public CheckIn read(@NotNull UUID checkinId) { } @Override + @RequiredPermission(Permission.CAN_UPDATE_CHECKINS) public CheckIn update(@NotNull CheckIn checkIn) { final UUID id = checkIn.getId(); validate(id == null, "Unable to find checkin record with id %s", checkIn.getId()); @@ -157,6 +162,7 @@ public CheckIn update(@NotNull CheckIn checkIn) { } @Override + @RequiredPermission(Permission.CAN_VIEW_CHECKINS) public Set findByFields(UUID teamMemberId, UUID pdlId, Boolean completed) { MemberProfile currentUser = currentUserServices.getCurrentUser(); final UUID currentUserId = currentUser.getId(); @@ -211,4 +217,4 @@ public boolean canViewAllCheckins(UUID memberId) { public boolean canUpdateAllCheckins(UUID memberId) { return hasPermission(memberId, Permission.CAN_UPDATE_ALL_CHECKINS); } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentController.java b/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentController.java index fc914f12ff..2595485584 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.document; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpStatus; import io.micronaut.http.annotation.Body; import io.micronaut.http.annotation.Controller; @@ -61,7 +59,6 @@ List listDocumentsForRole(UUID roleId) { * @return the created document */ @Post - @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) DocumentResponseDTO createDocument(@Body @Valid DocumentCreateDTO document) { return documentService.create(document.toDocument()); } @@ -74,7 +71,6 @@ DocumentResponseDTO createDocument(@Body @Valid DocumentCreateDTO document) { * @return the updated document */ @Put("/{documentId}") - @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) DocumentResponseDTO updateDocument(UUID documentId, @Body @Valid DocumentCreateDTO document) { return documentService.update(document.toDocument(documentId)); } @@ -86,7 +82,6 @@ DocumentResponseDTO updateDocument(UUID documentId, @Body @Valid DocumentCreateD */ @Delete("/{documentId}") @Status(HttpStatus.NO_CONTENT) - @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) void deleteDocument(UUID documentId) { documentService.deleteDocument(documentId); } @@ -101,7 +96,6 @@ void deleteDocument(UUID documentId) { * @return The list of documents saved to the role */ @Post("/{roleId}") - @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) List saveDocumentsToRoles(UUID roleId, @Body List documentIds) { return documentService.saveDocumentsToRoles(roleId, new LinkedHashSet<>(documentIds)); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentServiceImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentServiceImpl.java index 06024ef158..85e66d7981 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentServiceImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/document/DocumentServiceImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.document; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.role.RoleRepository; import io.micronaut.core.annotation.Nullable; @@ -36,6 +38,7 @@ class DocumentServiceImpl implements DocumentService { } @Override + @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) public DocumentResponseDTO create(Document document) { if (document.getId() != null) { return update(document); @@ -52,6 +55,7 @@ public DocumentResponseDTO create(Document document) { } @Override + @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) public DocumentResponseDTO update(Document document) { Optional documentByName = documentRepo.findByName(document.getName()); Optional documentByUrl = documentRepo.findByUrl(document.getUrl()); @@ -65,6 +69,7 @@ public DocumentResponseDTO update(Document document) { } @Override + @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) public void deleteDocument(UUID documentId) { if (roleDocumentationRepo.documentIsStillReferenced(documentId)) { throw new BadArgException("Document is still referenced by a role"); @@ -87,6 +92,7 @@ public List listDocuments(@Nullable UUID roleId) { @Override @Transactional + @RequiredPermission(Permission.CAN_ADMINISTER_DOCUMENTATION) public List saveDocumentsToRoles(UUID roleId, SequencedSet documentIds) { // Check the Role exists if (!roleRepo.existsById(roleId)) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/email/EmailController.java b/server/src/main/java/com/objectcomputing/checkins/services/email/EmailController.java index 49d449e4b2..dfbf9bb6e7 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/email/EmailController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/email/EmailController.java @@ -1,8 +1,5 @@ package com.objectcomputing.checkins.services.email; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; - import io.micronaut.http.HttpStatus; import io.micronaut.http.annotation.Controller; import io.micronaut.http.annotation.Post; @@ -27,7 +24,6 @@ public EmailController(EmailServices emailServices) { @Post @Status(HttpStatus.CREATED) - @RequiredPermission(Permission.CAN_SEND_EMAIL) public List sendEmail(String subject, String content, boolean html, String... recipients) { return emailServices.sendAndSaveEmail(subject, content, html, recipients); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/email/EmailServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/email/EmailServicesImpl.java index cc7bed68c5..9fca64745c 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/email/EmailServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/email/EmailServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.email; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.notifications.email.EmailSender; import com.objectcomputing.checkins.notifications.email.MailJetFactory; @@ -44,6 +46,7 @@ public EmailServicesImpl(@Named(MailJetFactory.HTML_FORMAT) EmailSender htmlEmai } @Override + @RequiredPermission(Permission.CAN_SEND_EMAIL) public List sendAndSaveEmail(String subject, String content, boolean html, String... recipients) { List sentEmails = new ArrayList<>(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursController.java b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursController.java index b6826d5dc4..94791e2bcb 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.employee_hours; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.NotFoundException; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.MediaType; @@ -47,7 +45,6 @@ public Set findEmployeeHours(@Nullable String employeeId) { * @{@link HttpResponse} */ @Post(uri="/upload" , consumes = MediaType.MULTIPART_FORM_DATA) - @RequiredPermission(Permission.CAN_UPLOAD_HOURS) public EmployeeHoursResponseDTO upload(CompletedFileUpload file){ return employeeHoursServices.save(file); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java index 24758c90fa..c63b3ce651 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.employee_hours; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; @@ -34,6 +36,7 @@ public EmployeeHoursServicesImpl(CurrentUserServices currentUserServices, @Override + @RequiredPermission(Permission.CAN_UPLOAD_HOURS) public EmployeeHoursResponseDTO save(CompletedFileUpload file) { MemberProfile currentUser = currentUserServices.getCurrentUser(); boolean isAdmin = currentUserServices.isAdmin(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerController.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerController.java index 6a198fe840..0d02bef4c6 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.feedback_answer; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpResponse; import io.micronaut.http.annotation.Body; @@ -64,7 +62,6 @@ public HttpResponse update(@Body @Valid @NotNull Feed * @return {@link FeedbackAnswerResponseDTO} */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_ANSWER) public HttpResponse getById(UUID id) { FeedbackAnswer savedAnswer = feedbackAnswerServices.getById(id); return HttpResponse.ok(fromEntity(savedAnswer)) @@ -80,7 +77,6 @@ public HttpResponse getById(UUID id) { * @return {@link FeedbackAnswerResponseDTO} */ @Get("/{?questionId,requestId}") - @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_ANSWER) public List findByValues(@Nullable UUID questionId, @Nullable UUID requestId) { return feedbackAnswerServices.findByValues(questionId, requestId) .stream() diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerServicesImpl.java index f2eab96940..ac11506823 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.feedback_answer; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -43,7 +45,6 @@ public FeedbackAnswerServicesImpl(FeedbackAnswerRepository feedbackAnswerReposit @Override public FeedbackAnswer save(FeedbackAnswer feedbackAnswer) { - // Ensure that related question exists templateQuestionServices.getById(feedbackAnswer.getQuestionId()); @@ -82,6 +83,10 @@ public FeedbackAnswer update(FeedbackAnswer feedbackAnswer) { } @Override + // This method cannot have + // @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_ANSWER) because regular + // members need to be able to find their answers. This permission is + // manually checked inside getIsPermitted(). public FeedbackAnswer getById(UUID id) { final Optional feedbackAnswer = feedbackAnswerRepository.findById(id); if (feedbackAnswer.isEmpty()) { @@ -98,6 +103,10 @@ public FeedbackAnswer getById(UUID id) { } @Override + // This method cannot have + // @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_ANSWER) because regular + // members need to be able to find their answers. This permission is + // manually checked below. public List findByValues(@Nullable UUID questionId, @Nullable UUID requestId) { List response = new ArrayList<>(); FeedbackRequest feedbackRequest; @@ -113,7 +122,12 @@ public List findByValues(@Nullable UUID questionId, @Nullable UU boolean isRequesteesSupervisor = requesteeId != null ? memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId())) : false; MemberProfile requestee = memberProfileServices.getById(requesteeId); final UUID requesteePDL = requestee.getPdlId(); - if (currentUserServices.isAdmin() || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || recipientId.equals(currentUserId) || feedbackRequestServices.selfRevieweeIsCurrentUserReviewee(feedbackRequest, currentUserId)) { + if (currentUserServices.hasPermission(Permission.CAN_VIEW_FEEDBACK_ANSWER) || + currentUserId.equals(requesteePDL) || isRequesteesSupervisor || + requestCreatorId.equals(currentUserId) || + recipientId.equals(currentUserId) || + feedbackRequestServices.selfRevieweeIsCurrentUserReviewee(feedbackRequest, currentUserId)) { + // All checks passed... response.addAll(feedbackAnswerRepository.getByQuestionIdAndRequestId(Util.nullSafeUUIDToString(questionId), Util.nullSafeUUIDToString(requestId))); return response; } @@ -145,8 +159,9 @@ public boolean updateIsPermitted(FeedbackRequest feedbackRequest) { } public boolean getIsPermitted(FeedbackRequest feedbackRequest) { - // Admins can always get questions and answers. - if (currentUserServices.isAdmin()) { + // Users with the CAN_ADMINISTER_FEEDBACK_ANSWER permission can always + // get questions and answers. + if (currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_ANSWER)) { return true; } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java index 1e2fa3a278..754e4f79c9 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.feedback_request; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.core.convert.format.Format; import io.micronaut.http.HttpResponse; @@ -46,7 +44,6 @@ public FeedbackRequestController(FeedbackRequestServices feedbackReqServices) { * @param requestBody {@link FeedbackRequestCreateDTO} New feedback request to create * @return {@link FeedbackRequestResponseDTO} */ - @RequiredPermission(Permission.CAN_CREATE_FEEDBACK_REQUEST) @Post public HttpResponse save(@Body @Valid @NotNull FeedbackRequestCreateDTO requestBody) { FeedbackRequest savedFeedbackRequest = feedbackReqServices.save(fromDTO(requestBody)); @@ -74,7 +71,6 @@ public HttpResponse update(@Body @Valid @NotNull Fee * @return {@link HttpResponse} */ @Delete("/{id}") - @RequiredPermission(Permission.CAN_DELETE_FEEDBACK_REQUEST) @Status(HttpStatus.OK) public void delete(@NotNull UUID id) { feedbackReqServices.delete(id); @@ -87,7 +83,6 @@ public void delete(@NotNull UUID id) { * @return {@link FeedbackRequestResponseDTO} */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_REQUEST) public HttpResponse getById(UUID id) { FeedbackRequest savedFeedbackRequest = feedbackReqServices.getById(id); return savedFeedbackRequest == null ? HttpResponse.notFound() : HttpResponse.ok(fromEntity(savedFeedbackRequest)) @@ -104,7 +99,6 @@ public HttpResponse getById(UUID id) { * @param oldestDate The date that filters out any requests that were made before that date * @return list of {@link FeedbackRequestResponseDTO} */ - @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_REQUEST) @Get("/{?creatorId,requesteeId,recipientId,oldestDate,reviewPeriodId,templateId,requesteeIds}") public List findByValues(@Nullable UUID creatorId, @Nullable UUID requesteeId, @Nullable UUID recipientId, @Nullable @Format("yyyy-MM-dd") LocalDate oldestDate, @Nullable UUID reviewPeriodId, @Nullable UUID templateId, @Nullable List requesteeIds) { return feedbackReqServices.findByValues(creatorId, requesteeId, recipientId, oldestDate, reviewPeriodId, templateId, requesteeIds) diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java index 4d07ab3778..8cd1c13300 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.feedback_request; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.configuration.CheckInsConfiguration; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; @@ -101,6 +103,7 @@ private void validateMembers(FeedbackRequest feedbackRequest) { } @Override + @RequiredPermission(Permission.CAN_CREATE_FEEDBACK_REQUEST) public FeedbackRequest save(FeedbackRequest feedbackRequest) { validateMembers(feedbackRequest); if (!createIsPermitted(feedbackRequest.getRequesteeId())) { @@ -253,6 +256,7 @@ public FeedbackRequest update(FeedbackRequestUpdateDTO feedbackRequestUpdateDTO) } @Override + @RequiredPermission(Permission.CAN_DELETE_FEEDBACK_REQUEST) public void delete(UUID id) { final Optional feedbackReq = feedbackReqRepository.findById(id); if (feedbackReq.isEmpty()) { @@ -267,6 +271,10 @@ public void delete(UUID id) { } @Override + // This method cannot have + // @RequiredPermission(Permission.CAN_ADMINISTER_FEEDBACK_REQUEST) because + // regular members need to be able to get feedback requests. This + // permission is manually checked elsewhere. public FeedbackRequest getById(UUID id) { final Optional feedbackReq = feedbackReqRepository.findById(id); if (feedbackReq.isEmpty()) { @@ -281,6 +289,10 @@ public FeedbackRequest getById(UUID id) { } @Override + // This method cannot have + // @RequiredPermission(Permission.CAN_ADMINISTER_FEEDBACK_REQUEST) because + // regular members need to be able to get feedback requests. This + // permission is manually checked elsewhere. public List findByValues(UUID creatorId, UUID requesteeId, UUID recipientId, LocalDate oldestDate, UUID reviewPeriodId, UUID templateId, List requesteeIds) { final UUID currentUserId = currentUserServices.getCurrentUser().getId(); if (currentUserId == null) { @@ -298,7 +310,7 @@ public List findByValues(UUID creatorId, UUID requesteeId, UUID feedbackReqList = feedbackReqList.stream().filter((FeedbackRequest request) -> { boolean visible = false; - if (currentUserServices.isAdmin()) { + if (currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_REQUEST)) { visible = true; } else if (request != null) { if (currentUserId.equals(request.getCreatorId()) || @@ -337,14 +349,17 @@ public boolean selfRevieweeIsCurrentUserReviewee(FeedbackRequest request, } private boolean createIsPermitted(UUID requesteeId) { - final boolean isAdmin = currentUserServices.isAdmin(); + if (currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_REQUEST)) { + return true; + } + final UUID currentUserId = currentUserServices.getCurrentUser().getId(); MemberProfile requestee = memberProfileServices.getById(requesteeId); boolean isRequesteesSupervisor = isSupervisor(requesteeId, currentUserId); final UUID requesteePDL = requestee.getPdlId(); //a PDL may create a request for a user who is assigned to them - return isAdmin || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || currentUserId.equals(requesteeId); + return currentUserId.equals(requesteePDL) || isRequesteesSupervisor || currentUserId.equals(requesteeId); } private boolean getIsPermitted(FeedbackRequest feedbackReq) { @@ -365,23 +380,29 @@ private boolean getIsPermitted(FeedbackRequest feedbackReq) { } private boolean updateDueDateIsPermitted(FeedbackRequest feedbackRequest) { - return isCurrentUserAdminOrOwner(feedbackRequest); + return currentUserCanAdministerOrOwner(feedbackRequest); } private boolean reassignIsPermitted(FeedbackRequest feedbackRequest) { - return isCurrentUserAdminOrOwner(feedbackRequest) && !feedbackRequest.getStatus().equals("submitted"); + return currentUserCanAdministerOrOwner(feedbackRequest) && !feedbackRequest.getStatus().equals("submitted"); } - private boolean isCurrentUserAdminOrOwner(FeedbackRequest feedbackRequest) { - boolean isAdmin = currentUserServices.isAdmin(); + private boolean currentUserCanAdministerOrOwner(FeedbackRequest feedbackRequest) { + if (currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_REQUEST)) { + return true; + } + UUID currentUserId = currentUserServices.getCurrentUser().getId(); - return isAdmin || currentUserId.equals(feedbackRequest.getCreatorId()); + return currentUserId.equals(feedbackRequest.getCreatorId()); } private boolean updateSubmitDateIsPermitted(FeedbackRequest feedbackRequest) { - boolean isAdmin = currentUserServices.isAdmin(); + if (currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_REQUEST)) { + return true; + } + UUID currentUserId = currentUserServices.getCurrentUser().getId(); - if (isAdmin || (currentUserId.equals(feedbackRequest.getCreatorId()) && feedbackRequest.getSubmitDate() != null)) { + if (currentUserId.equals(feedbackRequest.getCreatorId()) && feedbackRequest.getSubmitDate() != null) { return true; } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java index 28e841c1a4..2149d531bf 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.file; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.checkindocument.CheckinDocument; import com.objectcomputing.checkins.services.checkindocument.CheckinDocumentServices; import com.objectcomputing.checkins.services.checkins.CheckIn; @@ -149,13 +150,13 @@ public FileInfoDTO uploadFile(@NotNull UUID checkInID, @NotNull CompletedFileUpl @Override public boolean deleteFile(@NotNull String uploadDocId) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canDelete = currentUserServices.hasPermission(Permission.CAN_DELETE_CHECKIN_DOCUMENT); CheckinDocument cd = checkinDocumentServices.getFindByUploadDocId(uploadDocId); validate(cd == null, String.format("Unable to find record with id %s", uploadDocId)); CheckIn associatedCheckin = checkInServices.read(cd.getCheckinsId()); - if(!isAdmin) { + if(!canDelete) { validate((!currentUser.getId().equals(associatedCheckin.getTeamMemberId()) && !currentUser.getId().equals(associatedCheckin.getPdlId())), NOT_AUTHORIZED_MSG); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosController.java b/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosController.java index e8591d57e5..72eead812a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.kudos; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpStatus; import io.micronaut.http.annotation.Body; @@ -36,13 +34,11 @@ public KudosController(KudosServices kudosServices) { @Post @Status(HttpStatus.CREATED) - @RequiredPermission(Permission.CAN_CREATE_KUDOS) public Kudos create(@Body @Valid KudosCreateDTO kudos) { return kudosServices.save(kudos); } @Put - @RequiredPermission(Permission.CAN_ADMINISTER_KUDOS) public Kudos approve(@Body @Valid Kudos kudos) { return kudosServices.approve(kudos); } @@ -64,7 +60,6 @@ public List get(@Nullable UUID recipientId, @Nullable UUID sen @Delete("/{id}") @Status(HttpStatus.NO_CONTENT) - @RequiredPermission(Permission.CAN_ADMINISTER_KUDOS) public void delete(@NotNull UUID id) { kudosServices.delete(id); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosServicesImpl.java index 948b0ca6db..27de524e42 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/kudos/KudosServicesImpl.java @@ -1,6 +1,7 @@ package com.objectcomputing.checkins.services.kudos; import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.configuration.CheckInsConfiguration; import com.objectcomputing.checkins.notifications.email.EmailSender; import com.objectcomputing.checkins.notifications.email.MailJetFactory; @@ -90,6 +91,7 @@ private enum NotificationType { @Override @Transactional + @RequiredPermission(Permission.CAN_CREATE_KUDOS) public Kudos save(KudosCreateDTO kudosDTO) { UUID senderId = kudosDTO.getSenderId(); if (memberProfileRetrievalServices.getById(senderId).isEmpty()) { @@ -121,6 +123,7 @@ public Kudos save(KudosCreateDTO kudosDTO) { } @Override + @RequiredPermission(Permission.CAN_ADMINISTER_KUDOS) public Kudos approve(Kudos kudos) { UUID kudosId = kudos.getId(); Kudos existingKudos = kudosRepository.findById(kudosId).orElseThrow(() -> @@ -169,6 +172,7 @@ public KudosResponseDTO getById(UUID id) { } @Override + @RequiredPermission(Permission.CAN_ADMINISTER_KUDOS) public void delete(UUID id) { Kudos kudos = kudosRepository.findById(id).orElseThrow(() -> new NotFoundException(KUDOS_DOES_NOT_EXIST_MSG.formatted(id))); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportController.java b/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportController.java index 6a5d4ac668..291e43e4a0 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.member_skill.skillsreport; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.annotation.Body; @@ -35,7 +33,6 @@ public SkillsReportController(SkillsReportServices skillsReportServices) { * @return {@link SkillsReportResponseDTO} Returned skills report */ @Post - @RequiredPermission(Permission.CAN_VIEW_SKILLS_REPORT) public HttpResponse reportSkills(@Body @Valid @NotNull SkillsReportRequestDTO requestBody, HttpRequest request) { SkillsReportResponseDTO responseBody = skillsReportServices.report(requestBody); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImpl.java index b0637005b3..ef2c12ac63 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.member_skill.skillsreport; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.member_skill.MemberSkill; import com.objectcomputing.checkins.services.member_skill.MemberSkillRepository; @@ -33,7 +35,8 @@ public SkillsReportServicesImpl(MemberSkillRepository memberSkillRepo, this.skillRepo = skillRepo; } - + @Override + @RequiredPermission(Permission.CAN_VIEW_SKILLS_REPORT) public @NotNull SkillsReportResponseDTO report(@NotNull SkillsReportRequestDTO request) { final List skills = request.getSkills(); final Set members = request.getMembers(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileController.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileController.java index 14ae54588c..c79727b6ba 100755 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.memberprofile; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; @@ -96,7 +94,6 @@ public List findByValue(@Nullable String firstName, * @return {@link MemberProfileResponseDTO} The created member profile */ @Post - @RequiredPermission(Permission.CAN_CREATE_ORGANIZATION_MEMBERS) public HttpResponse save(@Body @Valid MemberProfileCreateDTO memberProfile) { MemberProfile savedProfile = memberProfileServices.saveProfile(fromDTO(memberProfile)); return HttpResponse.created(fromEntity(savedProfile)) @@ -122,7 +119,6 @@ public HttpResponse update(@Body @Valid MemberProfileU * @param id {@link UUID} Member unique id */ @Delete("/{id}") - @RequiredPermission(Permission.CAN_DELETE_ORGANIZATION_MEMBERS) @Status(HttpStatus.OK) public void delete(@NotNull UUID id) { memberProfileServices.deleteProfile(id); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java index 1bdb662a14..61cd5c896f 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java @@ -1,6 +1,7 @@ package com.objectcomputing.checkins.services.memberprofile; import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; @@ -100,6 +101,7 @@ public Set findByValues(@Nullable String firstName, @Override @CacheInvalidate(cacheNames = {"member-cache"}) + @RequiredPermission(Permission.CAN_CREATE_ORGANIZATION_MEMBERS) public MemberProfile saveProfile(MemberProfile memberProfile) { MemberProfile emailProfile = memberProfileRepository.findByWorkEmail(memberProfile.getWorkEmail()).orElse(null); @@ -149,6 +151,7 @@ public void emailAssignment(MemberProfile member, boolean isPDL) { @Override @CacheInvalidate(cacheNames = {"member-cache"}) + @RequiredPermission(Permission.CAN_DELETE_ORGANIZATION_MEMBERS) public boolean deleteProfile(@NotNull UUID id) { MemberProfile memberProfile = memberProfileRepository.findById(id).orElse(null); Set userRoles = (memberProfile != null) ? roleServices.findUserRoles(memberProfile.getId()) : Collections.emptySet(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportController.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportController.java index 3c3b05c4e7..787da7f986 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.memberprofile.anniversaryreport; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.annotation.Controller; import io.micronaut.http.annotation.Get; @@ -32,7 +30,6 @@ public AnniversaryReportController(AnniversaryServices anniversaryServices) { * @return list of anniversaries */ @Get("/{?month}") - @RequiredPermission(Permission.CAN_VIEW_ANNIVERSARY_REPORT) public List findByValue(@Nullable String[] month) { return anniversaryServices.findByValue(month); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportServicesImpl.java index 4ea9ac7fed..a4cc0f0297 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/anniversaryreport/AnniversaryReportServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.memberprofile.anniversaryreport; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; @@ -23,6 +25,7 @@ public AnniversaryReportServicesImpl(MemberProfileServices memberProfileServices } @Override + @RequiredPermission(Permission.CAN_VIEW_ANNIVERSARY_REPORT) public List findByValue(@Nullable String[] months) { List memberProfileAll = new ArrayList<>(); Set memberProfiles = memberProfileServices.findByValues(null, null, null, null, null, null, diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayController.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayController.java index f3f7646be8..688fba16ff 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.memberprofile.birthday; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.annotation.Controller; import io.micronaut.http.annotation.Get; @@ -32,7 +30,6 @@ public BirthDayController(BirthDayServices birthDayServices) { * @return list of birthdays */ @Get("/{?month,dayOfMonth}") - @RequiredPermission(Permission.CAN_VIEW_BIRTHDAY_REPORT) public List findByValue(@Nullable String[] month, @Nullable Integer[] dayOfMonth) { return birthDayServices.findByValue(month, dayOfMonth); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayServicesImpl.java index d241cd3786..015bb88559 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/birthday/BirthDayServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.memberprofile.birthday; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; @@ -20,6 +22,7 @@ public BirthDayServicesImpl(MemberProfileServices memberProfileServices) { } @Override + @RequiredPermission(Permission.CAN_VIEW_BIRTHDAY_REPORT) public List findByValue(String[] months, Integer[] daysOfMonth) { Set memberProfiles = memberProfileServices.findByValues(null, null, null, null, null, null, false); List memberProfileAll = new ArrayList<>(memberProfiles); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportController.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportController.java index 290a964386..11dfd97746 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.memberprofile.csvreport; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpHeaders; import io.micronaut.http.HttpResponse; @@ -17,6 +15,7 @@ import org.slf4j.LoggerFactory; import java.io.File; +import java.io.IOException; @Controller(MemberProfileReportController.PATH) @ExecuteOn(TaskExecutors.BLOCKING) @@ -36,14 +35,13 @@ class MemberProfileReportController { * @return HTTP response with the CSV file */ @Post(produces = MediaType.TEXT_CSV) - @RequiredPermission(Permission.CAN_VIEW_PROFILE_REPORT) HttpResponse getCsvFile(@Nullable @Body MemberProfileReportQueryDTO dto) { try { File file = memberProfileReportServices.generateFile(dto); return HttpResponse .ok(file) .header(HttpHeaders.CONTENT_DISPOSITION, String.format("attachment; filename=%s", file.getName())); - } catch (Exception error) { + } catch (IOException error) { LOG.error("Something went terribly wrong during export... ", error); return HttpResponse.serverError(); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImpl.java index 052aea4e48..41cded768a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImpl.java @@ -1,5 +1,8 @@ package com.objectcomputing.checkins.services.memberprofile.csvreport; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; + import jakarta.inject.Singleton; import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVPrinter; @@ -25,6 +28,7 @@ public MemberProfileReportServicesImpl(MemberProfileReportRepository memberProfi } @Override + @RequiredPermission(Permission.CAN_VIEW_PROFILE_REPORT) public File generateFile(MemberProfileReportQueryDTO queryDTO) throws IOException { List memberRecords = new ArrayList<>(); if (queryDTO == null || queryDTO.getMemberIds() == null) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServicesImpl.java index 600a419158..84af6e3cb2 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServicesImpl.java @@ -57,8 +57,12 @@ public boolean hasRole(RoleType role) { @Override public boolean hasPermission(Permission permission) { + MemberProfile currentUser = getCurrentUser(); + if (currentUser == null) { + return false; + } List userPermissions = - rolePermissionServices.findUserPermissions(getCurrentUser().getId()); + rolePermissionServices.findUserPermissions(currentUser.getId()); return userPermissions.stream().map(Permission::name) .anyMatch(str -> str.equals(permission.name())); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportController.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportController.java index 14047dd112..3bb8671cee 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.memberprofile.retentionreport; import com.objectcomputing.checkins.exceptions.BadArgException; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.annotation.Body; @@ -37,7 +35,6 @@ public RetentionReportController(RetentionReportServices retentionReportServices * @return {@link RetentionReportResponseDTO} Returned retention report */ @Post - @RequiredPermission(Permission.CAN_VIEW_RETENTION_REPORT) public HttpResponse reportRetention(@Body @Valid @NotNull RetentionReportRequestDTO requestBody, HttpRequest request) { if (requestBody.getStartDate().isAfter(requestBody.getEndDate()) || diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java index ea8e5f5259..321d016a28 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.memberprofile.retentionreport; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; @@ -23,6 +25,7 @@ public RetentionReportServicesImpl(MemberProfileServices memberProfileServices, } @Override + @RequiredPermission(Permission.CAN_VIEW_RETENTION_REPORT) public RetentionReportResponseDTO report(RetentionReportDTO request) { if (!currentUserServices.isAdmin()) { throw new PermissionException("Requires admin privileges"); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java index 66dd7a365a..d2c867d029 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java @@ -10,9 +10,11 @@ public enum Permission { CAN_VIEW_FEEDBACK_REQUEST("View feedback requests", "Feedback"), CAN_CREATE_FEEDBACK_REQUEST("Create feedback requests", "Feedback"), CAN_DELETE_FEEDBACK_REQUEST("Delete feedback requests", "Feedback"), + CAN_ADMINISTER_FEEDBACK_REQUEST("Administer feedback requests", "Feedback"), CAN_CREATE_KUDOS("Create kudos", "Feedback"), CAN_ADMINISTER_KUDOS("Administer kudos", "Feedback"), CAN_VIEW_FEEDBACK_ANSWER("View feedback answers", "Feedback"), + CAN_ADMINISTER_FEEDBACK_ANSWER("Administer feedback answers", "Feedback"), CAN_SEND_EMAIL("Send email", "Feedback"), CAN_EDIT_ALL_ORGANIZATION_MEMBERS("Edit all member profiles", "User Management"), CAN_DELETE_ORGANIZATION_MEMBERS("Delete organization members", "User Management"), diff --git a/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermission.java b/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermission.java index c772642c53..0ac134361e 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermission.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermission.java @@ -1,6 +1,14 @@ package com.objectcomputing.checkins.services.permissions; +import io.micronaut.aop.Around; +import java.lang.annotation.*; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +@Documented +@Retention(RUNTIME) +@Target({METHOD}) +@Around public @interface RequiredPermission { /** diff --git a/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermissionInterceptor.java b/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermissionInterceptor.java new file mode 100644 index 0000000000..025ab327f1 --- /dev/null +++ b/server/src/main/java/com/objectcomputing/checkins/services/permissions/RequiredPermissionInterceptor.java @@ -0,0 +1,34 @@ +package com.objectcomputing.checkins.services.permissions; + +import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; +import com.objectcomputing.checkins.exceptions.PermissionException; + +import io.micronaut.aop.InterceptorBean; +import io.micronaut.aop.MethodInterceptor; +import io.micronaut.aop.MethodInvocationContext; + +import jakarta.inject.Singleton; + +import java.util.Optional; + +@Singleton +@InterceptorBean(RequiredPermission.class) +public class RequiredPermissionInterceptor implements MethodInterceptor { + private final CurrentUserServices currentUserServices; + RequiredPermissionInterceptor(CurrentUserServices currentUserServices) { + this.currentUserServices = currentUserServices; + } + + @Override + public Object intercept(MethodInvocationContext context) { + Optional permission = + context.getValue(RequiredPermission.class, Permission.class); + if (permission.isPresent() && + currentUserServices.hasPermission(permission.get())) { + return context.proceed(); + } else { + // Throw with this message, as this is what is expected by many tests. + throw new PermissionException("Forbidden"); + } + } +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteController.java b/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteController.java index 0cd80224b6..eb07b1ded2 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.private_notes; import com.objectcomputing.checkins.exceptions.NotFoundException; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -40,7 +38,6 @@ class PrivateNoteController { * @return */ @Post - @RequiredPermission(Permission.CAN_CREATE_PRIVATE_NOTE) HttpResponse createPrivateNote(@Body @Valid PrivateNoteCreateDTO privateNote, HttpRequest request) { PrivateNote createPrivateNote = privateNoteServices.save(new PrivateNote(privateNote.getCheckinid(), privateNote.getCreatedbyid(), privateNote.getDescription())); URI location = UriBuilder.of(PATH).path(createPrivateNote.getId().toString()).build(); @@ -56,7 +53,6 @@ HttpResponse createPrivateNote(@Body @Valid PrivateNoteCreateDTO pr * @return */ @Put - @RequiredPermission(Permission.CAN_UPDATE_PRIVATE_NOTE) HttpResponse updatePrivateNote(@Body @Valid PrivateNote privateNote, HttpRequest request) { if (privateNote == null) { return HttpResponse.ok(); @@ -75,7 +71,6 @@ HttpResponse updatePrivateNote(@Body @Valid PrivateNote privateNote * @return */ @Get("/{?checkinid,createdbyid}") - @RequiredPermission(Permission.CAN_VIEW_PRIVATE_NOTE) public Set findPrivateNote(@Nullable UUID checkinid, @Nullable UUID createdbyid) { return privateNoteServices.findByFields(checkinid, createdbyid); @@ -88,7 +83,6 @@ public Set findPrivateNote(@Nullable UUID checkinid, * @return */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_PRIVATE_NOTE) public PrivateNote readPrivateNote(UUID id) { PrivateNote result = privateNoteServices.read(id); if (result == null) { @@ -96,4 +90,4 @@ public PrivateNote readPrivateNote(UUID id) { } return result; } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteServicesImpl.java index 48a8cbaac2..817fecee9d 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/private_notes/PrivateNoteServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.private_notes; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -36,6 +38,7 @@ public PrivateNoteServicesImpl(CheckInServices checkinServices, } @Override + @RequiredPermission(Permission.CAN_CREATE_PRIVATE_NOTE) public PrivateNote save(@NotNull PrivateNote privateNote) { validate(privateNote.getId() != null, "Found unexpected id %s for private note", privateNote.getId()); @@ -69,6 +72,7 @@ public PrivateNote save(@NotNull PrivateNote privateNote) { } @Override + @RequiredPermission(Permission.CAN_VIEW_PRIVATE_NOTE) public PrivateNote read(@NotNull UUID id) { final UUID currentUserId = currentUserServices.getCurrentUser().getId(); @@ -97,6 +101,7 @@ public PrivateNote read(@NotNull UUID id) { } @Override + @RequiredPermission(Permission.CAN_UPDATE_PRIVATE_NOTE) public PrivateNote update(@NotNull PrivateNote privateNote) { validate(privateNote.getId() == null, "No private note id %s found for updating", privateNote.getId()); @@ -130,6 +135,7 @@ public PrivateNote update(@NotNull PrivateNote privateNote) { } @Override + @RequiredPermission(Permission.CAN_VIEW_PRIVATE_NOTE) public Set findByFields(@Nullable UUID checkinId, @Nullable UUID createById) { final UUID currentUserId = currentUserServices.getCurrentUser().getId(); if(!checkinServices.doesUserHaveViewAccess(currentUserId, checkinId, createById)){ diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reports/MarkdownGeneration.java b/server/src/main/java/com/objectcomputing/checkins/services/reports/MarkdownGeneration.java index 76315cc82c..8f489dfc09 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reports/MarkdownGeneration.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reports/MarkdownGeneration.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.reports; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.MemberProfileUtils; @@ -120,7 +122,8 @@ public MarkdownGeneration(ReportDataServices reportDataServices, this.fileServices = fileServices; } - void upload(List memberIds, UUID reviewPeriodId) { + @RequiredPermission(Permission.CAN_CREATE_MERIT_REPORT) + public void upload(List memberIds, UUID reviewPeriodId) { for (UUID memberId : memberIds) { ReportDataCollation data = new ReportDataCollation( memberId, reviewPeriodId, @@ -138,12 +141,12 @@ void upload(List memberIds, UUID reviewPeriodId) { } } - void generateAndStore(ReportDataCollation data) { + private void generateAndStore(ReportDataCollation data) { final String markdown = generate(data); store(data, markdown); } - String generate(ReportDataCollation data) { + private String generate(ReportDataCollation data) { StringBuilder sb = new StringBuilder(); title(data, sb); currentInfo(data, sb); @@ -159,7 +162,7 @@ String generate(ReportDataCollation data) { return sb.toString(); } - void store(ReportDataCollation data, String markdown) { + private void store(ReportDataCollation data, String markdown) { // Send this text over to be uploaded to the google drive. fileServices.uploadDocument(directory, data.getMemberProfile().getWorkEmail(), diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataController.java b/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataController.java index 1a5a15626b..a493a825ac 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.reports; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.kudos.KudosRepository; import com.objectcomputing.checkins.services.kudos.kudos_recipient.KudosRecipientRepository; import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository; @@ -83,7 +81,6 @@ public ReportDataController(ReportDataServices reportDataServices, } @Post(uri="/upload", consumes = MediaType.MULTIPART_FORM_DATA) - @RequiredPermission(Permission.CAN_CREATE_MERIT_REPORT) public Mono>> uploadDataFiles( @Part("comp") Publisher comp, @Part("curr") Publisher curr, @@ -124,7 +121,6 @@ private String uploadHelper(ReportDataServices.DataType dataType, } @Post(uri="/generate") - @RequiredPermission(Permission.CAN_CREATE_MERIT_REPORT) public HttpStatus generate(@Body @Valid ReportDataDTO dto) { MarkdownGeneration markdown = new MarkdownGeneration(reportDataServices, diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java index b4c70a9359..824b872bfd 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.reports; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; @@ -51,6 +53,7 @@ public ReportDataServicesImpl(CurrentUserServices currentUserServices) { // Synchronized since we can upload multiple files at one time and there // is an expiration timer modifying the map too. @Override + @RequiredPermission(Permission.CAN_CREATE_MERIT_REPORT) public synchronized void store(DataType dataType, CompletedFileUpload file) throws IOException { MemberProfile currentUser = currentUserServices.getCurrentUser(); boolean isAdmin = currentUserServices.isAdmin(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentController.java b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentController.java index b6059367fe..7f1ead7be2 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.reviews; import com.objectcomputing.checkins.exceptions.NotFoundException; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -47,7 +45,6 @@ public ReviewAssignmentController(ReviewAssignmentServices reviewAssignmentServi * @return a streamable response containing the stored {@link ReviewAssignment} */ @Post - @RequiredPermission(Permission.CAN_CREATE_REVIEW_ASSIGNMENTS) public HttpResponse createReviewAssignment(@Body @Valid ReviewAssignmentDTO assignment, HttpRequest request) { ReviewAssignment reviewAssignment = reviewAssignmentServices.save(assignment.convertToEntity()); return HttpResponse.created(reviewAssignment) @@ -63,7 +60,6 @@ public HttpResponse createReviewAssignment(@Body @Valid Review * @return a streamable response containing the list of stored {@link ReviewAssignment} */ @Post("/{reviewPeriodId}") - @RequiredPermission(Permission.CAN_CREATE_REVIEW_ASSIGNMENTS) @Status(HttpStatus.CREATED) public List createReviewAssignment(@NotNull UUID reviewPeriodId, @Body List<@Valid ReviewAssignmentDTO> assignments) { @@ -77,7 +73,6 @@ public List createReviewAssignment(@NotNull UUID reviewPeriodI * @param id {@link UUID} of the review assignment * @return a streamable response containing the found {@link ReviewAssignment} with the given ID */ - @RequiredPermission(Permission.CAN_VIEW_REVIEW_ASSIGNMENTS) @Get("/{id}") public ReviewAssignment getById(@NotNull UUID id) { ReviewAssignment result = reviewAssignmentServices.findById(id); @@ -87,7 +82,6 @@ public ReviewAssignment getById(@NotNull UUID id) { return result; } - @RequiredPermission(Permission.CAN_VIEW_REVIEW_ASSIGNMENTS) @Get("/period/{reviewPeriodId}{?reviewerId}") public Set findAssignmentsByPeriodId(@NotNull UUID reviewPeriodId, @Nullable @QueryValue UUID reviewerId) { return reviewAssignmentServices.findAllByReviewPeriodIdAndReviewerId(reviewPeriodId, reviewerId); @@ -99,7 +93,6 @@ public Set findAssignmentsByPeriodId(@NotNull UUID reviewPerio * @param reviewAssignment the updated {@link ReviewAssignment} * @return a streamable response containing the stored {@link ReviewAssignment} */ - @RequiredPermission(Permission.CAN_UPDATE_REVIEW_ASSIGNMENTS) @Put public HttpResponse update(@Body @Valid ReviewAssignment reviewAssignment, HttpRequest request) { ReviewAssignment updatedReviewAssignment = reviewAssignmentServices.update(reviewAssignment); @@ -112,7 +105,6 @@ public HttpResponse update(@Body @Valid ReviewAssignment revie * * @param id the id of the review assignment to be deleted to delete */ - @RequiredPermission(Permission.CAN_DELETE_REVIEW_ASSIGNMENTS) @Delete("/{id}") @Status(HttpStatus.OK) public void deleteReviewAssignment(@NotNull UUID id) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentServicesImpl.java index 3fae5ff53c..05ec5ff417 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewAssignmentServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.reviews; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository; import io.micronaut.core.annotation.Nullable; @@ -25,6 +27,7 @@ public ReviewAssignmentServicesImpl(ReviewAssignmentRepository reviewAssignmentR } @Override + @RequiredPermission(Permission.CAN_CREATE_REVIEW_ASSIGNMENTS) public ReviewAssignment save(ReviewAssignment reviewAssignment) { ReviewAssignment newAssignment = null; if (reviewAssignment != null) { @@ -48,6 +51,7 @@ public ReviewAssignment save(ReviewAssignment reviewAssignment) { // to avoid multiple calls from the client-side overlapping and attempting // to create the same review assignments multiple times. @Override + @RequiredPermission(Permission.CAN_CREATE_REVIEW_ASSIGNMENTS) public synchronized List saveAll(UUID reviewPeriodId, List reviewAssignments, boolean deleteExisting) { if(deleteExisting) { @@ -71,12 +75,14 @@ public synchronized List saveAll(UUID reviewPeriodId, List findAllByReviewPeriodIdAndReviewerId(UUID reviewPeriodId, @Nullable UUID reviewerId) { Set reviewAssignments; @@ -91,6 +97,7 @@ public Set findAllByReviewPeriodIdAndReviewerId(UUID reviewPer } @Override + @RequiredPermission(Permission.CAN_UPDATE_REVIEW_ASSIGNMENTS) public ReviewAssignment update(ReviewAssignment reviewAssignment) { LOG.info("Updating entity {}", reviewAssignment); if (reviewAssignment.getId() != null && reviewAssignmentRepository.findById(reviewAssignment.getId()).isPresent()) { @@ -101,6 +108,7 @@ public ReviewAssignment update(ReviewAssignment reviewAssignment) { } @Override + @RequiredPermission(Permission.CAN_DELETE_REVIEW_ASSIGNMENTS) public void delete(UUID id) { if (id != null && reviewAssignmentRepository.findById(id).isPresent()) { reviewAssignmentRepository.deleteById(id); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodController.java b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodController.java index 66262a3622..e6f16caffb 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.reviews; import com.objectcomputing.checkins.exceptions.NotFoundException; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -47,7 +45,6 @@ public ReviewPeriodController(ReviewPeriodServices reviewPeriodServices, ReviewA * @return a streamable response containing the stored {@link ReviewPeriod} */ @Post - @RequiredPermission(Permission.CAN_CREATE_REVIEW_PERIOD) public HttpResponse createReviewPeriod(@Body @Valid ReviewPeriodCreateDTO period, HttpRequest request) { HttpResponse httpResponse; Set reviewAssignments; @@ -69,7 +66,6 @@ public HttpResponse createReviewPeriod(@Body @Valid ReviewPeriodCr * @return a streamable response containing the found {@link ReviewPeriod} with the given ID */ @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_REVIEW_PERIOD) public ReviewPeriod getById(@NotNull UUID id) { ReviewPeriod result = reviewPeriodServices.findById(id); if (result == null) { @@ -86,7 +82,6 @@ public ReviewPeriod getById(@NotNull UUID id) { * @return a streamable response containing a {@link Set} of {@link ReviewPeriod}s that match the given criteria */ @Get("/{?name,reviewStatus}") - @RequiredPermission(Permission.CAN_VIEW_REVIEW_PERIOD) public Set findByValue(@Nullable String name, @Nullable ReviewStatus reviewStatus) { return reviewPeriodServices.findByValue(name, reviewStatus); } @@ -98,7 +93,6 @@ public Set findByValue(@Nullable String name, @Nullable ReviewStat * @return a streamable response containing the stored {@link ReviewPeriod} */ @Put - @RequiredPermission(Permission.CAN_UPDATE_REVIEW_PERIOD) public HttpResponse update(@Body @Valid ReviewPeriod reviewPeriod, HttpRequest request) { ReviewPeriod updatedReviewPeriod = reviewPeriodServices.update(reviewPeriod); return HttpResponse.ok(updatedReviewPeriod) @@ -113,9 +107,8 @@ public HttpResponse update(@Body @Valid ReviewPeriod reviewPeriod, * @param id the id of the review period to be deleted to delete */ @Delete("/{id}") - @RequiredPermission(Permission.CAN_DELETE_REVIEW_PERIOD) @Status(HttpStatus.OK) public void deleteReviewPeriod(@NotNull UUID id) { reviewPeriodServices.delete(id); } -} \ No newline at end of file +} diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodServicesImpl.java index 2710f8b44f..e29aa727d2 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.reviews; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.Environments; import com.objectcomputing.checkins.configuration.CheckInsConfiguration; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; @@ -84,10 +86,12 @@ private enum SelfReviewDate { LAUNCH, THREE_DAYS, ONE_DAY } this.automatedEmailRepository = automatedEmailRepository; } - void setEmailSender(EmailSender emailSender) { + void setEmailSender(EmailSender emailSender) { this.emailSender = emailSender; } + @Override + @RequiredPermission(Permission.CAN_CREATE_REVIEW_PERIOD) public ReviewPeriod save(ReviewPeriod reviewPeriod) { ReviewPeriod newPeriod = null; if (reviewPeriod != null) { @@ -105,10 +109,14 @@ public ReviewPeriod save(ReviewPeriod reviewPeriod) { return newPeriod; } + @Override + @RequiredPermission(Permission.CAN_VIEW_REVIEW_PERIOD) public ReviewPeriod findById(@NotNull UUID id) { return reviewPeriodRepository.findById(id).orElse(null); } + @Override + @RequiredPermission(Permission.CAN_VIEW_REVIEW_PERIOD) public Set findByValue(String name, ReviewStatus reviewStatus) { Set reviewPeriods = new HashSet<>(); @@ -125,6 +133,8 @@ public Set findByValue(String name, ReviewStatus reviewStatus) { return reviewPeriods; } + @Override + @RequiredPermission(Permission.CAN_DELETE_REVIEW_PERIOD) public void delete(@NotNull UUID id) { if (!feedbackRequestServices.findByValues(null, null, null, null, id, null, null).isEmpty()) { throw new BadArgException(String.format("Review Period %s has associated feedback requests and cannot be deleted", id)); @@ -137,6 +147,8 @@ protected List findByNameLike(String name) { return reviewPeriodRepository.findByNameIlike(wildcard); } + @Override + @RequiredPermission(Permission.CAN_UPDATE_REVIEW_PERIOD) public ReviewPeriod update(@NotNull ReviewPeriod reviewPeriod) { LOG.info("Updating entity {}", reviewPeriod); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java index e0c61ee403..69bb398222 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingOption.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.settings; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; @@ -38,16 +40,19 @@ public enum SettingOption { this.values = values; } + @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public static List getOptions(){ return Arrays.asList(SettingOption.values()); } + @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public static boolean isValidOption(String name){ return Stream.of(SettingOption.values()) .anyMatch(option -> option.name().equalsIgnoreCase(name)); } @JsonCreator + @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public static SettingOption fromName(String name) { for (SettingOption option : values()) { if (option.name().equalsIgnoreCase(name)) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java index e33a612dd8..a96c185a3e 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.settings; import com.objectcomputing.checkins.exceptions.NotFoundException; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; import io.micronaut.http.annotation.Body; @@ -47,7 +45,6 @@ public SettingsController(SettingsServices settingsServices) { */ @ExecuteOn(TaskExecutors.BLOCKING) @Get - @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public List findAllSettings() { return settingsServices.findAllSettings().stream() .map(this::fromEntity).toList(); @@ -61,7 +58,6 @@ public List findAllSettings() { */ @ExecuteOn(TaskExecutors.BLOCKING) @Get("/{name}") - @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public SettingsResponseDTO findByName(@PathVariable @NotNull String name) { return fromEntity(settingsServices.findByName(name)); } @@ -72,7 +68,6 @@ public SettingsResponseDTO findByName(@PathVariable @NotNull String name) { * @return {@link } Returned setting options */ @Get("/options") - @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public List getOptions() { List options = SettingOption.getOptions(); return options.stream().map(option -> { @@ -100,7 +95,6 @@ public List getOptions() { */ @ExecuteOn(TaskExecutors.BLOCKING) @Post - @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) public HttpResponse save(@Body @Valid SettingsDTO settingDTO) { Setting savedSetting = settingsServices.save(fromDTO(settingDTO)); URI location = UriBuilder.of(PATH).path(savedSetting.getId().toString()).build(); @@ -115,7 +109,6 @@ public HttpResponse save(@Body @Valid SettingsDTO settingDT */ @Put @ExecuteOn(TaskExecutors.BLOCKING) - @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) public HttpResponse update(@Body @Valid SettingsDTO settingsDTO) { Setting savedSetting = settingsServices.update(settingsDTO.getName(), settingsDTO.getValue()); SettingsResponseDTO settingsResponseDTO = fromEntity(savedSetting); @@ -132,7 +125,6 @@ public HttpResponse update(@Body @Valid SettingsDTO setting */ @Delete("/{id}") @ExecuteOn(TaskExecutors.BLOCKING) - @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) public HttpStatus delete(UUID id) { return settingsServices.delete(id) ? HttpStatus.OK : HttpStatus.UNPROCESSABLE_ENTITY; } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java index 656416a004..3491b9145f 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/settings/SettingsServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.settings; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import jakarta.inject.Singleton; @@ -17,6 +19,8 @@ public SettingsServicesImpl(SettingsRepository settingsRepository) { this.settingsRepository = settingsRepository; } + @Override + @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) public Setting save(Setting setting) { if (setting.getId() != null) { throw new BadArgException("Setting ID is autogenerated by the server upon creation, and should not be provided."); @@ -25,6 +29,8 @@ public Setting save(Setting setting) { return settingsRepository.save(setting); } + @Override + @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) public Setting update(String name, String value) { validateSettingOption(name); Setting setting = settingsRepository.findByName(name) @@ -33,15 +39,21 @@ public Setting update(String name, String value) { return settingsRepository.update(setting); } + @Override + @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public Setting findByName(@NotNull String name) { return settingsRepository.findByName(name) .orElseThrow(() -> new NotFoundException("Setting with name " + name + " not found.")); } + @Override + @RequiredPermission(Permission.CAN_VIEW_SETTINGS) public List findAllSettings() { return settingsRepository.findAll(); } + @Override + @RequiredPermission(Permission.CAN_ADMINISTER_SETTINGS) public boolean delete(@NotNull UUID id) { if (!settingsRepository.existsById(id)) { throw new NotFoundException("No setting with id " + id); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordController.java b/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordController.java index c06c830554..05fa5ad5aa 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.skill_record; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpHeaders; import io.micronaut.http.HttpResponse; import io.micronaut.http.MediaType; @@ -15,6 +13,7 @@ import org.slf4j.LoggerFactory; import java.io.File; +import java.io.IOException; @Controller("/services/skills/records") @ExecuteOn(TaskExecutors.BLOCKING) @@ -27,14 +26,13 @@ class SkillRecordController { this.skillRecordServices = skillRecordServices; } - @RequiredPermission(Permission.CAN_VIEW_SKILL_CATEGORIES) @Get(value = "/csv", produces = MediaType.TEXT_CSV) HttpResponse generateCsv() { try { File file = skillRecordServices.generateFile(); return HttpResponse.ok(file) .header(HttpHeaders.CONTENT_DISPOSITION, String.format("attachment; filename=%s", file.getName())); - } catch (Exception error) { + } catch (IOException error) { LOG.error("Something went terribly wrong during export... ", error); return HttpResponse.serverError(); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImpl.java index 40dc840aad..588e246dc1 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImpl.java @@ -1,5 +1,8 @@ package com.objectcomputing.checkins.services.skill_record; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; + import jakarta.inject.Singleton; import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVPrinter; @@ -22,6 +25,7 @@ public SkillRecordServicesImpl(SkillRecordRepository skillRecordRepository, Skil } @Override + @RequiredPermission(Permission.CAN_VIEW_SKILL_CATEGORIES) public File generateFile() throws IOException { List skillRecords = skillRecordRepository.findAll(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryController.java b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryController.java index 1d6303821d..17b6f48442 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryController.java @@ -1,8 +1,6 @@ package com.objectcomputing.checkins.services.skillcategory; import com.objectcomputing.checkins.exceptions.NotFoundException; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; @@ -38,7 +36,6 @@ public SkillCategoryController(SkillCategoryServices skillCategoryServices) { } @Post - @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public HttpResponse create(@Body @Valid SkillCategoryCreateDTO dto, HttpRequest request) { SkillCategory skillCategory = new SkillCategory(dto.getName(), dto.getDescription()); SkillCategory createdSkillCategory = skillCategoryServices.save(skillCategory); @@ -47,7 +44,6 @@ public HttpResponse create(@Body @Valid SkillCategoryCreateDTO dt } @Put - @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public HttpResponse update(@Body @Valid SkillCategoryUpdateDTO dto, HttpRequest request) { SkillCategory skillCategory = new SkillCategory(dto.getId(), dto.getName(), dto.getDescription()); SkillCategory update = skillCategoryServices.update(skillCategory); @@ -56,7 +52,6 @@ public HttpResponse update(@Body @Valid SkillCategoryUpdateDTO dt } @Get("/{id}") - @RequiredPermission(Permission.CAN_VIEW_SKILL_CATEGORIES) public SkillCategoryResponseDTO getById(@NotNull UUID id) { SkillCategoryResponseDTO result = skillCategoryServices.read(id); if (result == null) { @@ -66,13 +61,11 @@ public SkillCategoryResponseDTO getById(@NotNull UUID id) { } @Get("/with-skills") - @RequiredPermission(Permission.CAN_VIEW_SKILL_CATEGORIES) public List findAllWithSkills() { return skillCategoryServices.findAllWithSkills(); } @Delete("/{id}") - @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) @Status(HttpStatus.OK) public void delete(@NotNull UUID id) { skillCategoryServices.delete(id); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryServicesImpl.java index b13273d25d..c5963dba0a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.skillcategory; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; @@ -26,6 +28,7 @@ public SkillCategoryServicesImpl(SkillCategoryRepository skillCategoryRepository } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public SkillCategory save(SkillCategory skillCategory) { if (skillCategoryRepository.findByName(skillCategory.getName()).isPresent()) { throw new AlreadyExistsException(skillCategory.getName()); @@ -34,6 +37,7 @@ public SkillCategory save(SkillCategory skillCategory) { } @Override + @RequiredPermission(Permission.CAN_VIEW_SKILL_CATEGORIES) public SkillCategoryResponseDTO read(@NotNull UUID id) { SkillCategory skillCategory = skillCategoryRepository.findById(id).orElseThrow(() -> new NotFoundException("Category not found") @@ -43,6 +47,7 @@ public SkillCategoryResponseDTO read(@NotNull UUID id) { } @Override + @RequiredPermission(Permission.CAN_VIEW_SKILL_CATEGORIES) public List findAllWithSkills() { List categoriesWithSkills = new ArrayList<>(); List categories = skillCategoryRepository.findAllOrderByName(); @@ -55,6 +60,7 @@ public List findAllWithSkills() { } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public SkillCategory update(SkillCategory skillCategory) { if (skillCategoryRepository.findById(skillCategory.getId()).isEmpty()) { throw new BadArgException(String.format("Category with %s does not exist", skillCategory.getId())); @@ -64,6 +70,7 @@ public SkillCategory update(SkillCategory skillCategory) { @Transactional @Override + @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public void delete(UUID id) { skillCategorySkillServices.deleteAllByCategoryId(id); skillCategoryRepository.deleteById(id); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillController.java b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillController.java index 3b63448e13..0c34e04d47 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.skillcategory.skillcategory_skill; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; @@ -32,7 +30,6 @@ public SkillCategorySkillController(SkillCategorySkillServices skillCategorySkil } @Post - @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public HttpResponse create(@Body @Valid SkillCategorySkillId dto, HttpRequest request) { SkillCategorySkill skillCategorySkill = skillCategorySkillServices.save(dto); return HttpResponse.created(skillCategorySkill) @@ -40,7 +37,6 @@ public HttpResponse create(@Body @Valid SkillCategorySkillId } @Delete - @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) @Status(HttpStatus.OK) public void delete(@Body @Valid SkillCategorySkillId dto) { skillCategorySkillServices.delete(dto); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillServicesImpl.java index 773bf32430..8214fc4c1a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.skillcategory.skillcategory_skill; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.skills.Skill; import jakarta.inject.Singleton; import jakarta.validation.Valid; @@ -30,12 +32,14 @@ public List findSkillsBySkillCategoryId(String skillCategoryId) { } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public SkillCategorySkill save(@Valid SkillCategorySkillId dto) { SkillCategorySkill skillCategorySkill = new SkillCategorySkill(dto); return skillCategorySkillRepository.save(skillCategorySkill); } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public void delete(SkillCategorySkillId dto) { skillCategorySkillRepository.deleteByIds( dto.getSkillCategoryId().toString(), @@ -44,6 +48,7 @@ public void delete(SkillCategorySkillId dto) { } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILL_CATEGORIES) public void deleteAllByCategoryId(UUID categoryId) { skillCategorySkillRepository.deleteBySkillCategoryId(categoryId); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillController.java b/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillController.java index 9bf351cde8..9519135bdc 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.skills; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.NotFoundException; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; @@ -86,7 +84,6 @@ public Set findByValue(@Nullable String name, @Nullable Boolean pending) * @return {@link HttpResponse} */ @Put - @RequiredPermission(Permission.CAN_EDIT_SKILLS) public HttpResponse update(@Body @Valid Skill skill, HttpRequest request) { Skill updatedSkill = skillServices.update(skill); return HttpResponse.ok(updatedSkill) @@ -100,7 +97,6 @@ public HttpResponse update(@Body @Valid Skill skill, HttpRequest reque */ @Delete("/{id}") @Status(HttpStatus.OK) - @RequiredPermission(Permission.CAN_EDIT_SKILLS) public void deleteSkill(@NotNull UUID id) { skillServices.delete(id); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillServicesImpl.java index 0a47dda442..1998e8af49 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skills/SkillServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.skills; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; @@ -65,6 +67,7 @@ public Set findByValue(String name, Boolean pending) { } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILLS) public void delete(@NotNull UUID id) { skillRepository.deleteById(id); } @@ -75,6 +78,7 @@ protected List findByNameLike(String name) { } @Override + @RequiredPermission(Permission.CAN_EDIT_SKILLS) public Skill update(@NotNull Skill skill) { if (skill.getId() != null && skillRepository.findById(skill.getId()).isPresent()) { return skillRepository.update(skill); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringOrganizationController.java b/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringOrganizationController.java index e6ff6df0ee..a04dde6f95 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringOrganizationController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringOrganizationController.java @@ -1,7 +1,5 @@ package com.objectcomputing.checkins.services.volunteering; -import com.objectcomputing.checkins.services.permissions.Permission; -import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.annotation.Body; import io.micronaut.http.annotation.Controller; @@ -70,7 +68,6 @@ VolunteeringOrganization create(@Valid @Body VolunteeringOrganizationDTO organiz * @return the updated {@link VolunteeringOrganization} */ @Put("/{id}") - @RequiredPermission(Permission.CAN_ADMINISTER_VOLUNTEERING_ORGANIZATIONS) VolunteeringOrganization update(@NotNull UUID id, @Valid @Body VolunteeringOrganizationDTO organization) { return volunteeringService.update(new VolunteeringOrganization( id, diff --git a/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringServiceImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringServiceImpl.java index 184e1ba0c2..950c9c1692 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringServiceImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/volunteering/VolunteeringServiceImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.volunteering; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; @@ -94,14 +96,12 @@ public VolunteeringRelationship create(VolunteeringRelationship relationship) { @Override public VolunteeringEvent create(VolunteeringEvent event) { - if (event.getId() != null) { - return update(event); - } validateEvent(event, "create"); return eventRepo.save(event); } @Override + @RequiredPermission(Permission.CAN_ADMINISTER_VOLUNTEERING_ORGANIZATIONS) public VolunteeringOrganization update(VolunteeringOrganization organization) { // Fail if an organization with the same name already exists (but it's not this one) validate(organizationRepo.getByName(organization.getName()) diff --git a/server/src/main/resources/db/dev/R__Load_testing_data.sql b/server/src/main/resources/db/dev/R__Load_testing_data.sql index 95722469b3..9af6a8afc3 100644 --- a/server/src/main/resources/db/dev/R__Load_testing_data.sql +++ b/server/src/main/resources/db/dev/R__Load_testing_data.sql @@ -668,11 +668,21 @@ insert into role_permissions values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_DELETE_FEEDBACK_REQUEST'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_ADMINISTER_FEEDBACK_REQUEST'); + insert into role_permissions (roleid, permission) values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_VIEW_FEEDBACK_ANSWER'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_ADMINISTER_FEEDBACK_ANSWER'); + insert into role_permissions (roleid, permission) values diff --git a/server/src/test/java/com/objectcomputing/checkins/services/CurrentUserServicesReplacement.java b/server/src/test/java/com/objectcomputing/checkins/services/CurrentUserServicesReplacement.java index 99b958725b..af957f943f 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/CurrentUserServicesReplacement.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/CurrentUserServicesReplacement.java @@ -1,13 +1,19 @@ package com.objectcomputing.checkins.services; import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.role.role_permissions.RolePermission; +import com.objectcomputing.checkins.services.role.Role; +import com.objectcomputing.checkins.services.role.RoleRepository; +import com.objectcomputing.checkins.services.role.role_permissions.RolePermissionRepository; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; import com.objectcomputing.checkins.services.role.RoleType; import java.util.List; +import jakarta.inject.Inject; import jakarta.inject.Singleton; + import io.micronaut.core.util.StringUtils; import io.micronaut.context.env.Environment; import io.micronaut.context.annotation.Replaces; @@ -18,8 +24,12 @@ @Requires(property = "replace.currentuserservices", value = StringUtils.TRUE) public class CurrentUserServicesReplacement implements CurrentUserServices { public MemberProfile currentUser; - public List roles; - public List permissions; + + @Inject + RoleRepository roleRepository; + + @Inject + RolePermissionRepository rolePermissionRepository; @Override public MemberProfile findOrSaveUser(String firstName, @@ -30,12 +40,28 @@ public MemberProfile findOrSaveUser(String firstName, @Override public boolean hasRole(RoleType role) { - return roles == null ? false : roles.contains(role); + if (currentUser != null) { + for(Role has : roleRepository.findUserRoles(currentUser.getId())) { + if (has.getRole().equals(role.toString())) { + return true; + } + } + } + return false; } @Override public boolean hasPermission(Permission permission) { - return permissions == null ? false : permissions.contains(permission); + if (currentUser != null) { + for(Role role : roleRepository.findUserRoles(currentUser.getId())) { + for(RolePermission perm : rolePermissionRepository.findByRole(role.getRole())) { + if (perm.getPermission() == permission) { + return true; + } + } + } + } + return false; } @Override diff --git a/server/src/test/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServiceImplTest.java b/server/src/test/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServiceImplTest.java index 1068d52b52..fe63466300 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServiceImplTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/checkindocument/CheckinDocumentServiceImplTest.java @@ -2,9 +2,11 @@ import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.TestContainersSuite; +import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; import com.objectcomputing.checkins.services.fixture.CheckInDocumentFixture; import com.objectcomputing.checkins.services.fixture.CheckInFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.checkins.CheckIn; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.role.RoleType; @@ -25,8 +27,12 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +@Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class CheckinDocumentServiceImplTest extends TestContainersSuite - implements MemberProfileFixture, CheckInFixture, CheckInDocumentFixture { + implements MemberProfileFixture, CheckInFixture, CheckInDocumentFixture, RoleFixture { + @Inject + CurrentUserServicesReplacement currentUserServices; + @Inject private CheckinDocumentServicesImpl services; @@ -39,6 +45,9 @@ void reset() { pdl = createADefaultMemberProfile(); member = createADefaultMemberProfileForPdl(pdl); checkIn = createADefaultCheckIn(member, pdl); + currentUserServices.currentUser = member; + createAndAssignRoles(); + assignAdminRole(member); } @Test diff --git a/server/src/test/java/com/objectcomputing/checkins/services/document/DocumentationControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/document/DocumentationControllerTest.java index e54e94e15d..61977f6042 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/document/DocumentationControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/document/DocumentationControllerTest.java @@ -25,7 +25,6 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; -import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -42,9 +41,6 @@ @SuppressWarnings("OptionalGetWithoutIsPresent") // Will throw an exception, which is fine for testing class DocumentationControllerTest extends TestContainersSuite implements MemberProfileFixture, RoleFixture, DocumentationFixture { - @Inject - DocumentService documentService; - @Inject DocumentationClient client; @@ -84,19 +80,22 @@ void createTestDocumentation() { testDocument = createDocument("Test Document", "https://test.com", "This is a test document"); howToDealWithPdlsDocument = createDocument("How to deal with PDLs", "/how-to-deal-with-pdls.pdf", "just for members"); + http = rawClient.toBlocking(); + // Assign the member documents so members start with 3 documents - documentService.saveDocumentsToRoles( - memberRole.getId(), - new LinkedHashSet<>(List.of(testDocument.getId(), twoRolesDocument.getId(), howToDealWithPdlsDocument.getId())) - ); + HttpRequest request = HttpRequest.POST( + String.format("/%s", memberRole.getId()), + List.of(testDocument.getId(), twoRolesDocument.getId(), + howToDealWithPdlsDocument.getId()) + ).basicAuth(admin.getWorkEmail(), ADMIN_ROLE); + http.exchange(request); // Assign the pdl documents so pdls start with 2 documents - documentService.saveDocumentsToRoles( - pdlRole.getId(), - new LinkedHashSet<>(List.of(twoRolesDocument.getId(), pdlDocument.getId())) - ); - - http = rawClient.toBlocking(); + request = HttpRequest.POST( + String.format("/%s", pdlRole.getId()), + List.of(twoRolesDocument.getId(), pdlDocument.getId()) + ).basicAuth(admin.getWorkEmail(), ADMIN_ROLE); + http.exchange(request); } @Test diff --git a/server/src/test/java/com/objectcomputing/checkins/services/email/EmailControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/email/EmailControllerTest.java index 1a7474341a..eea14eccea 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/email/EmailControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/email/EmailControllerTest.java @@ -92,14 +92,16 @@ void testSendAndSaveHtmlEmail() { assertEquals(email.get("content"), firstEmailRes.getContents()); assertEquals(admin.getId(), firstEmailRes.getSentBy()); assertEquals(recipient1.getId(), firstEmailRes.getRecipient()); - assertTrue(firstEmailRes.getTransmissionDate().isAfter(firstEmailRes.getSendDate())); + assertTrue(firstEmailRes.getTransmissionDate().isAfter(firstEmailRes.getSendDate()) || + firstEmailRes.getTransmissionDate().isEqual(firstEmailRes.getSendDate())); Email secondEmailRes = response.getBody().get().get(1); assertEquals(email.get("subject"), secondEmailRes.getSubject()); assertEquals(email.get("content"), secondEmailRes.getContents()); assertEquals(admin.getId(), secondEmailRes.getSentBy()); assertEquals(recipient2.getId(), secondEmailRes.getRecipient()); - assertTrue(secondEmailRes.getTransmissionDate().isAfter(secondEmailRes.getSendDate())); + assertTrue(secondEmailRes.getTransmissionDate().isAfter(secondEmailRes.getSendDate()) || + secondEmailRes.getTransmissionDate().isEqual(secondEmailRes.getSendDate())); assertEquals(1, htmlEmailSender.events.size()); assertEquals( diff --git a/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestTest.java b/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestTest.java index 0f83752c7c..5f3ffefebb 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestTest.java @@ -8,6 +8,7 @@ import com.objectcomputing.checkins.services.TestContainersSuite; import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import com.objectcomputing.checkins.services.role.RoleType; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.fixture.FeedbackRequestFixture; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; import com.objectcomputing.checkins.services.fixture.ReviewPeriodFixture; @@ -40,7 +41,7 @@ @Property(name = "replace.mailjet.factory", value = StringUtils.TRUE) @Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class FeedbackRequestTest extends TestContainersSuite - implements FeedbackRequestFixture, MemberProfileFixture, ReviewPeriodFixture, ReviewAssignmentFixture { + implements FeedbackRequestFixture, MemberProfileFixture, ReviewPeriodFixture, ReviewAssignmentFixture, RoleFixture { @Inject private CurrentUserServicesReplacement currentUserServices; @@ -56,6 +57,7 @@ class FeedbackRequestTest extends TestContainersSuite @BeforeEach void setUp() { + createAndAssignRoles(); emailSender.reset(); } @@ -74,8 +76,7 @@ void testUpdateFeedbackRequest() { // We need the current user to be logged in and admin. currentUserServices.currentUser = creator; - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(creator); FeedbackRequestUpdateDTO updateDTO = new FeedbackRequestUpdateDTO(); updateDTO.setId(feedbackRequest.getId()); @@ -92,8 +93,9 @@ void testUpdateFeedbackRequest() { @Test void testUpdateFeedbackRequest_NotFound() { + MemberProfile creator = createADefaultMemberProfile(); UUID feedbackRequestId = UUID.randomUUID(); - UUID creatorId = UUID.randomUUID(); + UUID creatorId = creator.getId(); UUID recipientId = UUID.randomUUID(); UUID requesteeId = UUID.randomUUID(); @@ -108,6 +110,10 @@ void testUpdateFeedbackRequest_NotFound() { FeedbackRequestUpdateDTO updateDTO = new FeedbackRequestUpdateDTO(); updateDTO.setId(feedbackRequest.getId()); + // We need the creator to be logged in. + currentUserServices.currentUser = creator; + assignMemberRole(creator); + assertThrows(NotFoundException.class, () -> feedbackRequestServices.update(updateDTO)); assertEquals(0, emailSender.events.size()); } diff --git a/server/src/test/java/com/objectcomputing/checkins/services/file/FileControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/file/FileControllerTest.java index e547735ead..89eec1f0bf 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/file/FileControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/file/FileControllerTest.java @@ -5,6 +5,7 @@ import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; import com.objectcomputing.checkins.services.fixture.CheckInFixture; import com.objectcomputing.checkins.services.fixture.CheckInDocumentFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.checkins.CheckIn; import com.objectcomputing.checkins.services.checkindocument.CheckinDocument; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; @@ -35,6 +36,7 @@ import java.util.UUID; import static com.objectcomputing.checkins.services.role.RoleType.Constants.MEMBER_ROLE; +import static com.objectcomputing.checkins.services.role.RoleType.Constants.PDL_ROLE; import static com.objectcomputing.checkins.services.role.RoleType.Constants.ADMIN_ROLE; import static io.micronaut.http.MediaType.MULTIPART_FORM_DATA; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -44,7 +46,7 @@ @Property(name = "replace.fileservicesimpl", value = StringUtils.TRUE) class FileControllerTest extends TestContainersSuite - implements MemberProfileFixture, CheckInFixture, CheckInDocumentFixture { + implements MemberProfileFixture, CheckInFixture, CheckInDocumentFixture, RoleFixture { @Inject @Client("/services/files") @@ -62,6 +64,7 @@ class FileControllerTest extends TestContainersSuite @BeforeEach void reset() { + createAndAssignRoles(); pdl = createADefaultMemberProfile(); member = createADefaultMemberProfileForPdl(pdl); checkIn = createADefaultCheckIn(member, pdl); @@ -110,7 +113,7 @@ void testFindByCheckinId() { String fileId = "some.id"; FileInfoDTO testFileInfoDto = createUploadedDocument(fileId); - final HttpRequest request = HttpRequest.GET(String.format("?id=%s", checkIn.getId())).basicAuth(member.getWorkEmail(), MEMBER_ROLE); + final HttpRequest request = HttpRequest.GET(String.format("?id=%s", checkIn.getId())).basicAuth(pdl.getWorkEmail(), PDL_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(FileInfoDTO.class)); assertNotNull(response); @@ -128,7 +131,7 @@ void testDownloadDocument() { FileInfoDTO testFileInfoDto = createUploadedDocument(uploadDocId); final HttpRequest request= HttpRequest.GET(String.format("/%s/download", uploadDocId)) - .basicAuth(member.getWorkEmail(), MEMBER_ROLE); + .basicAuth(pdl.getWorkEmail(), PDL_ROLE); final HttpResponse response = client.toBlocking().exchange(request, File.class); assertNotNull(response); @@ -139,7 +142,7 @@ void testDownloadDocument() { void testUploadEndpoint() { final HttpRequest request = HttpRequest.POST(String.format("/%s", checkIn.getId()), MultipartBody.builder() .addPart("file", testFile).build()) - .basicAuth(member.getWorkEmail(), MEMBER_ROLE) + .basicAuth(pdl.getWorkEmail(), PDL_ROLE) .contentType(MULTIPART_FORM_DATA); final HttpResponse response = client.toBlocking().exchange(request, FileInfoDTO.class); @@ -173,7 +176,7 @@ void testDeleteEndpoint() { FileInfoDTO testFileInfoDto = createUploadedDocument(uploadDocId); final HttpRequest request = HttpRequest.DELETE(String.format("/%s", uploadDocId)) - .basicAuth(member.getWorkEmail(), MEMBER_ROLE); + .basicAuth(pdl.getWorkEmail(), PDL_ROLE); final HttpResponse response = client.toBlocking().exchange(request); assertNotNull(response); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/file/FileServicesImplTest.java b/server/src/test/java/com/objectcomputing/checkins/services/file/FileServicesImplTest.java index 1a6e6e4ed6..2e55b051b3 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/file/FileServicesImplTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/file/FileServicesImplTest.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.file; +import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.security.GoogleServiceConfiguration; @@ -135,10 +136,12 @@ void reset() { createAndAssignRoles(); pdl = createADefaultMemberProfile(); + assignPdlRole(pdl); member = createADefaultMemberProfileForPdl(pdl); + assignMemberRole(member); currentUserServices.currentUser = createASecondDefaultMemberProfile(); - currentUserServices.roles = null; + assignMemberRole(currentUserServices.currentUser); checkIn = createADefaultCheckIn(member, pdl); } @@ -148,8 +151,7 @@ void testFindFilesForFindAll() { String fileId = "some.id"; services.addFile(fileId, new byte[0]); - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); final Set result = services.findFiles(null); @@ -171,7 +173,8 @@ void testFindFilesForFindByCheckinId() { services.addFile(fileId, new byte[0]); CheckinDocument cd = createACustomCheckInDocument(checkIn, fileId); - currentUserServices.currentUser = member; + // Must be a PDL to even ask for the files from a check-in. + currentUserServices.currentUser = pdl; final Set result = services.findFiles(checkIn.getId()); assertNotNull(result); @@ -180,7 +183,8 @@ void testFindFilesForFindByCheckinId() { @Test void testFindByCheckinIdWhenNoDocsAreUploadedForCheckinId() { - currentUserServices.currentUser = member; + // Must be a PDL to even ask for the files from a check-in. + currentUserServices.currentUser = pdl; final Set result = services.findFiles(checkIn.getId()); assertNotNull(result); @@ -200,8 +204,7 @@ void testFindAllFilesThrowsException() { String fileId = "some.id"; services.addFile(fileId, new byte[0]); - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); services.shouldThrow = true; @@ -227,7 +230,7 @@ void testDownloadFiles() { long byteCount = 50; services.addFile(uploadDocId, new byte[(int)byteCount]); - currentUserServices.currentUser = member; + currentUserServices.currentUser = pdl; CheckinDocument cd = createACustomCheckInDocument(checkIn, uploadDocId); final java.io.File resultFile = services.downloadFiles(uploadDocId); @@ -240,8 +243,6 @@ void testDownloadFilesAdminCanAccess() { long byteCount = 50; services.addFile(uploadDocId, new byte[(int)byteCount]); - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); // The CheckInsServicesImpl checks with the rolePermissionServices. assignAdminRole(currentUserServices.currentUser); @@ -269,7 +270,7 @@ void testDownloadFilesUnauthorizedUser() { CheckinDocument cd = createACustomCheckInDocument(checkIn, uploadDocId); // This exception actually comes from the CheckinDocumentServices. - final BadArgException responseException = assertThrows(BadArgException.class, () -> + final PermissionException responseException = assertThrows(PermissionException.class, () -> services.downloadFiles(uploadDocId)); assertEquals(NOT_AUTHORIZED_MSG, responseException.getMessage()); @@ -280,7 +281,7 @@ void testDownloadFilesThrowsException() { String uploadDocId = "some.test.id"; services.addFile(uploadDocId, new byte[0]); - currentUserServices.currentUser = member; + currentUserServices.currentUser = pdl; CheckinDocument cd = createACustomCheckInDocument(checkIn, uploadDocId); services.shouldThrow = true; @@ -295,7 +296,7 @@ void testDeleteFile() { services.addFile(uploadDocId, new byte[0]); CheckinDocument cd = createACustomCheckInDocument(checkIn, uploadDocId); - currentUserServices.currentUser = member; + currentUserServices.currentUser = pdl; Boolean result = services.deleteFile(uploadDocId); assertTrue(result); } @@ -306,8 +307,6 @@ void testDeleteFilesAdminCanAccess() { services.addFile(uploadDocId, new byte[0]); CheckinDocument cd = createACustomCheckInDocument(checkIn, uploadDocId); - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); // The CheckInsServicesImpl checks with the rolePermissionServices. assignAdminRole(currentUserServices.currentUser); @@ -332,7 +331,7 @@ void testDeleteFileByUnauthorizedUser() { services.addFile(uploadDocId, new byte[0]); CheckinDocument cd = createACustomCheckInDocument(checkIn, uploadDocId); - final BadArgException responseException = assertThrows(BadArgException.class, () -> + final PermissionException responseException = assertThrows(PermissionException.class, () -> services.deleteFile(uploadDocId)); assertEquals(NOT_AUTHORIZED_MSG, responseException.getMessage()); @@ -348,7 +347,7 @@ void testDeleteFileThrowsException() { //act currentUserServices.currentUser = member; - final FileRetrievalException responseException = assertThrows(FileRetrievalException.class, + final PermissionException responseException = assertThrows(PermissionException.class, () -> services.deleteFile(uploadDocId)); } @@ -357,7 +356,8 @@ void testUploadFilesByCreatingNewDirectory() { CompletedFileUpload fileToUpload = new SimpleUploadFile("file.name"); //act - currentUserServices.currentUser = member; + // Must be a PDL to upload files for a check-in. + currentUserServices.currentUser = pdl; FileInfoDTO result = services.uploadFile(checkIn.getId(), fileToUpload); //assert @@ -369,8 +369,6 @@ void testUploadFilesByCreatingNewDirectory() { void testFileUploadForAdminByUploadingFileToExistingDirectory() { CompletedFileUpload fileToUpload = new SimpleUploadFile("file.name"); - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); // The CheckInsServicesImpl checks with the rolePermissionServices. assignAdminRole(currentUserServices.currentUser); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java index 91859c2c8c..e79aeb7f45 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java @@ -51,7 +51,9 @@ public interface PermissionFixture extends RolePermissionFixture { Permission.CAN_VIEW_FEEDBACK_REQUEST, Permission.CAN_CREATE_FEEDBACK_REQUEST, Permission.CAN_DELETE_FEEDBACK_REQUEST, + Permission.CAN_ADMINISTER_FEEDBACK_REQUEST, Permission.CAN_VIEW_FEEDBACK_ANSWER, + Permission.CAN_ADMINISTER_FEEDBACK_ANSWER, Permission.CAN_EDIT_ALL_ORGANIZATION_MEMBERS, Permission.CAN_DELETE_ORGANIZATION_MEMBERS, Permission.CAN_CREATE_ORGANIZATION_MEMBERS, diff --git a/server/src/test/java/com/objectcomputing/checkins/services/guild/GuildTest.java b/server/src/test/java/com/objectcomputing/checkins/services/guild/GuildTest.java index deae25f885..4a645db1b8 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/guild/GuildTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/guild/GuildTest.java @@ -8,6 +8,7 @@ import com.objectcomputing.checkins.services.role.RoleType; import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.TestContainersSuite; import com.objectcomputing.checkins.services.guild.member.GuildMember; import com.objectcomputing.checkins.services.guild.member.GuildMemberHistoryRepository; @@ -46,7 +47,7 @@ @Property(name = "replace.mailjet.factory", value = StringUtils.TRUE) @Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class GuildTest extends TestContainersSuite - implements MemberProfileFixture { + implements MemberProfileFixture, RoleFixture { @Inject private Validator validator; @@ -66,6 +67,11 @@ class GuildTest extends TestContainersSuite @BeforeEach void setUp() { + createAndAssignRoles(); + + currentUserServices.currentUser = createADefaultMemberProfile(); + assignMemberRole(currentUserServices.currentUser); + emailSender.reset(); } @@ -218,7 +224,7 @@ void testSaveGuildWithValidData() { guildDTO.setLink(link); guildDTO.setCommunity(true); - MemberProfile memberProfile = createADefaultMemberProfile(); + MemberProfile memberProfile = createASecondDefaultMemberProfile(); GuildCreateDTO.GuildMemberCreateDTO guildMemberCreateDTO = new GuildCreateDTO.GuildMemberCreateDTO(); guildMemberCreateDTO.setMemberId(memberProfile.getId()); guildMemberCreateDTO.setLead(true); @@ -252,7 +258,7 @@ GuildResponseDTO createGuild(String name, MemberProfile lead) { @Test void testSaveGuildWithExistingName() { - MemberProfile memberProfile = createADefaultMemberProfile(); + MemberProfile memberProfile = createASecondDefaultMemberProfile(); createGuild("Existing Guild", memberProfile); emailSender.reset(); @@ -292,7 +298,6 @@ void testSaveGuildWithNullDTO() { @Test void testUpdateGuildWithNewGuildLeaders() { // Create members involved. - MemberProfile currentUser = createADefaultMemberProfile(); MemberProfile memberProfile1 = createASecondDefaultMemberProfile(); MemberProfile memberProfile2 = createAThirdDefaultMemberProfile(); @@ -321,9 +326,7 @@ void testUpdateGuildWithNewGuildLeaders() { guildDTO.setGuildMembers(Arrays.asList(guildMember1, guildMember2)); // We need the current user to be logged in and admin. - currentUserServices.currentUser = currentUser; - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); GuildResponseDTO response = guildServices.update(guildDTO); assertEquals(2, emailSender.events.size()); @@ -343,7 +346,6 @@ void testUpdateGuildWithNewGuildLeaders() { @Test void testUpdateGuildWithNoNewGuildLeaders() { // Create members involved. - MemberProfile currentUser = createADefaultMemberProfile(); MemberProfile memberProfile1 = createASecondDefaultMemberProfile(); MemberProfile memberProfile2 = createAThirdDefaultMemberProfile(); @@ -372,9 +374,7 @@ void testUpdateGuildWithNoNewGuildLeaders() { guildDTO.setGuildMembers(Arrays.asList(guildMember1, guildMember2)); // We need the current user to be logged in and admin. - currentUserServices.currentUser = currentUser; - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); GuildResponseDTO response = guildServices.update(guildDTO); assertEquals(1, emailSender.events.size()); @@ -393,10 +393,7 @@ void testUpdateGuildWithNonExistentGuildId() { guildDTO.setId(UUID.randomUUID()); // Non-existent Guild ID // We need the current user to be logged in and admin. - MemberProfile currentUser = createADefaultMemberProfile(); - currentUserServices.currentUser = currentUser; - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); assertThrows(BadArgException.class, () -> guildServices.update(guildDTO)); @@ -415,10 +412,7 @@ void testUpdateGuildWithNoGuildLeads() { guildDTO.setGuildMembers(Collections.emptyList()); // No guild members // We need the current user to be logged in and admin. - MemberProfile currentUser = createADefaultMemberProfile(); - currentUserServices.currentUser = currentUser; - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); assertThrows(BadArgException.class, () -> guildServices.update(guildDTO)); @@ -436,10 +430,6 @@ void testUpdateGuildWithUnauthorizedUser() { guildDTO.setId(existing.getId()); guildDTO.setGuildMembers(Collections.emptyList()); // No guild members - // We need the current user to be logged in and *not* admin. - MemberProfile currentUser = createADefaultMemberProfile(); - currentUserServices.currentUser = currentUser; - assertThrows(PermissionException.class, () -> { guildServices.update(guildDTO); }); @@ -459,10 +449,7 @@ void testUpdateGuildWithInvalidLink() { guildDTO.setLink("invalid-link"); // Invalid link // We need the current user to be logged in and admin. - MemberProfile currentUser = createADefaultMemberProfile(); - currentUserServices.currentUser = currentUser; - currentUserServices.roles = new ArrayList(); - currentUserServices.roles.add(RoleType.ADMIN); + assignAdminRole(currentUserServices.currentUser); assertThrows(BadArgException.class, () -> guildServices.update(guildDTO)); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImplTest.java b/server/src/test/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImplTest.java index fb2e77cfff..b815befe0f 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImplTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/member_skill/skillsreport/SkillsReportServicesImplTest.java @@ -5,6 +5,7 @@ import com.objectcomputing.checkins.services.fixture.SkillFixture; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; import com.objectcomputing.checkins.services.fixture.MemberSkillFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.skills.Skill; import com.objectcomputing.checkins.services.member_skill.skillsreport.SkillLevel; import com.objectcomputing.checkins.services.member_skill.MemberSkill; @@ -13,6 +14,11 @@ import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; import com.objectcomputing.checkins.services.skills.SkillRepository; +import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; + +import io.micronaut.context.annotation.Property; +import io.micronaut.core.util.StringUtils; + import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -32,11 +38,20 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +@Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class SkillsReportServicesImplTest extends TestContainersSuite - implements MemberProfileFixture, MemberSkillFixture, SkillFixture { + implements MemberProfileFixture, MemberSkillFixture, SkillFixture, RoleFixture { + @Inject + CurrentUserServicesReplacement currentUserServices; + @Inject private SkillsReportServicesImpl skillsReportServices; + @BeforeEach + void setUp() { + createAndAssignRoles(); + } + @Test void testReportSkillNotExist() { final SkillLevelDTO dto = new SkillLevelDTO(); @@ -48,6 +63,8 @@ void testReportSkillNotExist() { final SkillsReportRequestDTO request = new SkillsReportRequestDTO(); request.setSkills(skills); + currentUserServices.currentUser = createADefaultMemberProfile(); + assignAdminRole(currentUserServices.currentUser); assertThrows(BadArgException.class, () -> skillsReportServices.report(request)); } @@ -63,6 +80,9 @@ void testReportMemberProfileNotExist() { final Set members = new HashSet<>(); members.add(UUID.randomUUID()); request.setMembers(members); + + currentUserServices.currentUser = createADefaultMemberProfile(); + assignAdminRole(currentUserServices.currentUser); assertThrows(BadArgException.class, () -> skillsReportServices.report(request)); } @@ -70,6 +90,8 @@ void testReportMemberProfileNotExist() { void testReportEmptyRequestedSkillsList() { final SkillsReportRequestDTO request = new SkillsReportRequestDTO(); request.setSkills(new ArrayList<>()); + currentUserServices.currentUser = createADefaultMemberProfile(); + assignAdminRole(currentUserServices.currentUser); final SkillsReportResponseDTO response = skillsReportServices.report(request); assertNotNull(response); assertEquals(0, response.getTeamMembers().size()); @@ -121,6 +143,9 @@ void testReport() { requestedSkills1.add(dto2); requestedSkills1.add(dto3); + currentUserServices.currentUser = member1; + assignAdminRole(currentUserServices.currentUser); + // Any member with at least 1 satisfying skill is returned final SkillsReportRequestDTO request1 = new SkillsReportRequestDTO(); request1.setSkills(requestedSkills1); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTest.java b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTest.java index 93e96a9ae6..c74478a861 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTest.java @@ -2,6 +2,7 @@ import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.notifications.email.MailJetFactory; import com.objectcomputing.checkins.services.MailJetFactoryReplacement; @@ -35,7 +36,7 @@ @Property(name = "replace.mailjet.factory", value = StringUtils.TRUE) @Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class MemberProfileTest extends TestContainersSuite - implements MemberProfileFixture { + implements MemberProfileFixture, RoleFixture { @Inject protected Validator validator; @@ -52,6 +53,7 @@ class MemberProfileTest extends TestContainersSuite @BeforeEach void setUp() { + createAndAssignRoles(); emailSender.reset(); } @@ -184,6 +186,9 @@ void testSaveProfileWithExistingEmail() { String workEmail = existingProfile.getWorkEmail(); MemberProfile newProfile = new MemberProfile(UUID.randomUUID(), "John", null, "Smith", null, null, null, null, workEmail, null, null, null, null, null, null, null, null, null); + // The current user must have permission to create new members. + currentUserServices.currentUser = existingProfile; + assignAdminRole(existingProfile); assertThrows(AlreadyExistsException.class, () -> memberProfileServices.saveProfile(newProfile)); } @@ -200,6 +205,10 @@ void testSaveProfileWithNewEmail() { supervisorProfile.getId(), null, null, null, null, LocalDate.now()); + // Need permission to create new profiles. + currentUserServices.currentUser = supervisorProfile; + assignAdminRole(supervisorProfile); + MemberProfile savedProfile = memberProfileServices.saveProfile(newProfile); // Assertions to verify the saved profile is not null and is equal to the newProfile diff --git a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImplTest.java b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImplTest.java index f8310b8fcb..7f8cf00bcd 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImplTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/csvreport/MemberProfileReportServicesImplTest.java @@ -2,8 +2,14 @@ import com.objectcomputing.checkins.services.TestContainersSuite; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices; +import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; + +import io.micronaut.context.annotation.Property; +import io.micronaut.core.util.StringUtils; + import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -26,14 +32,23 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +@Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class MemberProfileReportServicesImplTest extends TestContainersSuite - implements MemberProfileFixture { + implements MemberProfileFixture, RoleFixture { + @Inject + CurrentUserServicesReplacement currentUserServices; + @Inject private MemberProfileServices memberProfileServices; @Inject private MemberProfileReportServicesImpl memberProfileReportServices; + @BeforeEach + void setUp() { + createAndAssignRoles(); + } + @Test void testGenerateFileWithAllMemberProfiles() throws IOException { List expectedRecords = createSampleRecords(); @@ -128,13 +143,16 @@ MemberProfileRecord from(MemberProfile profile) { } private List createSampleRecords() { + // A user must have the CAN_VIEW_PROFILE_REPORT to create this report. + currentUserServices.currentUser = createAThirdDefaultMemberProfile(); + assignAdminRole(currentUserServices.currentUser); + // The createADefaultMemberProfileForPdl() method actually sets both // the PDL and Supervisor to the id of the member profile passed in. + MemberProfileRecord record1 = from(currentUserServices.currentUser); MemberProfile pdl = createADefaultMemberProfile(); - MemberProfileRecord record1 = from(pdl); - MemberProfileRecord record2 = from(createADefaultMemberProfileForPdl(pdl)); - MemberProfileRecord record3 = from(createAThirdDefaultMemberProfile()); - + MemberProfileRecord record2 = from(pdl); + MemberProfileRecord record3 = from(createADefaultMemberProfileForPdl(pdl)); return List.of(record1, record2, record3); } } diff --git a/server/src/test/java/com/objectcomputing/checkins/services/pulse/PulseServicesTest.java b/server/src/test/java/com/objectcomputing/checkins/services/pulse/PulseServicesTest.java index f3de9f012f..e4ee9faa39 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/pulse/PulseServicesTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/pulse/PulseServicesTest.java @@ -11,6 +11,7 @@ import com.objectcomputing.checkins.services.settings.Setting; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.EmailHelper; +import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import io.micronaut.core.util.StringUtils; import io.micronaut.context.annotation.Property; @@ -34,7 +35,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue; @Property(name = "replace.mailjet.factory", value = StringUtils.TRUE) +@Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class PulseServicesTest extends TestContainersSuite implements TeamFixture, RoleFixture { + @Inject + CurrentUserServicesReplacement currentUserServices; + @Inject @Named(MailJetFactory.MJML_FORMAT) private MailJetFactoryReplacement.MockEmailSender emailSender; @@ -74,6 +79,7 @@ void setUp() { admin = createAThirdDefaultMemberProfile(); assignAdminRole(admin); + currentUserServices.currentUser = admin; List recipientsEmail = List.of(member.getWorkEmail(), other.getWorkEmail(), diff --git a/server/src/test/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodControllerTest.java index 261501737c..892a4dfaf4 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodControllerTest.java @@ -399,19 +399,6 @@ void testPOSTCreateANullReviewPeriod() { assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus()); } - @Test - void testPOSTCreateANullReviewPeriodForbiddenWithoutPermission() { - ReviewPeriodCreateDTO reviewPeriodCreateDTO = new ReviewPeriodCreateDTO(); - - final HttpRequest request = HttpRequest. - POST("/", reviewPeriodCreateDTO).basicAuth(MEMBER_ROLE, MEMBER_ROLE); - HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, - () -> client.toBlocking().exchange(request, Map.class)); - - assertNotNull(responseException.getResponse()); - assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus()); - } - @Test void testUpdateReviewPeriodAsAdmin() { MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); @@ -706,6 +693,8 @@ void testOpenAReviewPeriodWithBadLaunchTime() { LocalDateTime closeDate = selfReviewCloseDate.plusDays(1); LocalDateTime startDate = launchDate.minusDays(30); LocalDateTime endDate = closeDate.minusDays(1); + + MemberProfile supervisor = createADefaultSupervisor(); ReviewPeriod period = getReviewPeriodRepository().save( new ReviewPeriod("Good Times, Bad Times", ReviewStatus.AWAITING_APPROVAL, null, null, @@ -713,11 +702,12 @@ void testOpenAReviewPeriodWithBadLaunchTime() { startDate, endDate)); period.setReviewStatus(ReviewStatus.OPEN); - assertThrows( - BadArgException.class, - () -> reviewPeriodServices.update(period), - "Expected ReviewPeriodServices.update() to throw, but it didn't" - ); + + final HttpRequest request = HttpRequest. + PUT("/", period).basicAuth(supervisor.getWorkEmail(), ADMIN_ROLE); + HttpClientResponseException exception = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request)); } @Test @@ -727,15 +717,19 @@ void testCreateAReviewPeriodWithBadSelfReviewCloseDate() { LocalDateTime closeDate = selfReviewCloseDate.plusDays(1); LocalDateTime startDate = launchDate.minusDays(30); LocalDateTime endDate = closeDate.minusDays(1); - BadArgException exception = assertThrows( - BadArgException.class, - () -> reviewPeriodServices.save( + + MemberProfile supervisor = createADefaultSupervisor(); + ReviewPeriod period = new ReviewPeriod("Good Times, Bad Times", ReviewStatus.AWAITING_APPROVAL, null, null, launchDate, selfReviewCloseDate, closeDate, - startDate, endDate)), - "Expected ReviewPeriodServices.save() to throw, but it didn't" - ); + startDate, endDate); + + final HttpRequest request = HttpRequest. + POST("/", period).basicAuth(supervisor.getWorkEmail(), ADMIN_ROLE); + HttpClientResponseException exception = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request)); assertTrue(exception.getMessage() .contains("self-review close date must be after")); } @@ -747,15 +741,18 @@ void testCreateAReviewPeriodWithBadCloseDate() { LocalDateTime closeDate = launchDate.minusDays(1); LocalDateTime startDate = launchDate.minusDays(30); LocalDateTime endDate = closeDate.minusDays(1); - BadArgException exception = assertThrows( - BadArgException.class, - () -> reviewPeriodServices.save( + + MemberProfile supervisor = createADefaultSupervisor(); + ReviewPeriod period = new ReviewPeriod("Good Times, Bad Times", ReviewStatus.AWAITING_APPROVAL, null, null, launchDate, selfReviewCloseDate, closeDate, - startDate, endDate)), - "Expected ReviewPeriodServices.save() to throw, but it didn't" - ); + startDate, endDate); + final HttpRequest request = HttpRequest. + POST("/", period).basicAuth(supervisor.getWorkEmail(), ADMIN_ROLE); + HttpClientResponseException exception = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request)); assertTrue(exception.getMessage() .contains("close date must be after")); } @@ -767,15 +764,19 @@ void testCreateAReviewPeriodWithBadStartDate() { LocalDateTime closeDate = selfReviewCloseDate.plusDays(1); LocalDateTime startDate = launchDate; LocalDateTime endDate = closeDate.minusDays(1); - BadArgException exception = assertThrows( - BadArgException.class, - () -> reviewPeriodServices.save( + + MemberProfile supervisor = createADefaultSupervisor(); + ReviewPeriod period = new ReviewPeriod("Good Times, Bad Times", ReviewStatus.AWAITING_APPROVAL, null, null, launchDate, selfReviewCloseDate, closeDate, - startDate, endDate)), - "Expected ReviewPeriodServices.save() to throw, but it didn't" - ); + startDate, endDate); + + final HttpRequest request = HttpRequest. + POST("/", period).basicAuth(supervisor.getWorkEmail(), ADMIN_ROLE); + HttpClientResponseException exception = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request)); assertTrue(exception.getMessage() .contains("start date must be before")); } @@ -787,15 +788,19 @@ void testCreateAReviewPeriodWithBadEndDate1() { LocalDateTime closeDate = selfReviewCloseDate.plusDays(1); LocalDateTime startDate = launchDate.minusDays(30); LocalDateTime endDate = startDate; - BadArgException exception = assertThrows( - BadArgException.class, - () -> reviewPeriodServices.save( + + MemberProfile supervisor = createADefaultSupervisor(); + ReviewPeriod period = new ReviewPeriod("Good Times, Bad Times", ReviewStatus.AWAITING_APPROVAL, null, null, launchDate, selfReviewCloseDate, closeDate, - startDate, endDate)), - "Expected ReviewPeriodServices.save() to throw, but it didn't" - ); + startDate, endDate); + + final HttpRequest request = HttpRequest. + POST("/", period).basicAuth(supervisor.getWorkEmail(), ADMIN_ROLE); + HttpClientResponseException exception = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request)); assertTrue(exception.getMessage() .contains("end date must be after")); } @@ -807,15 +812,19 @@ void testCreateAReviewPeriodWithBadEndDate2() { LocalDateTime closeDate = selfReviewCloseDate.plusDays(1); LocalDateTime startDate = launchDate.minusDays(30); LocalDateTime endDate = closeDate.plusDays(1); - BadArgException exception = assertThrows( - BadArgException.class, - () -> reviewPeriodServices.save( + + MemberProfile supervisor = createADefaultSupervisor(); + ReviewPeriod period = new ReviewPeriod("Good Times, Bad Times", ReviewStatus.AWAITING_APPROVAL, null, null, launchDate, selfReviewCloseDate, closeDate, - startDate, endDate)), - "Expected ReviewPeriodServices.save() to throw, but it didn't" - ); + startDate, endDate); + + final HttpRequest request = HttpRequest. + POST("/", period).basicAuth(supervisor.getWorkEmail(), ADMIN_ROLE); + HttpClientResponseException exception = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request)); assertTrue(exception.getMessage() .contains("end date must be on or before")); } @@ -918,4 +927,5 @@ private void checkSelfReviewEmail(LocalDateTime launchDate, member.getWorkEmail(), emailSender.events.get(2)); } + } diff --git a/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryControllerTest.java index 94d49387c2..e331e6684b 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/SkillCategoryControllerTest.java @@ -69,7 +69,9 @@ void testCreateSkillCategoryAlreadyExists() { @Test void testCreateNotAllowed() { + SkillCategory existingCategory = createDefaultSkillCategory(); SkillCategoryCreateDTO createDTO = new SkillCategoryCreateDTO(); + createDTO.setName(existingCategory.getName()); final HttpRequest request = HttpRequest .POST("/", createDTO) @@ -99,7 +101,10 @@ void testUpdateSkillCategory() { @Test void testUpdateNotAllowed() { + SkillCategory existingCategory = createDefaultSkillCategory(); SkillCategoryUpdateDTO updateDTO = new SkillCategoryUpdateDTO(); + updateDTO.setId(existingCategory.getId()); + updateDTO.setName(existingCategory.getName()); final HttpRequest request = HttpRequest .PUT("/", updateDTO) @@ -296,4 +301,4 @@ void testDeleteNotAllowed() { assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus()); } -} \ No newline at end of file +} diff --git a/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillControllerTest.java index 0184053ef4..fb6616fed7 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/skillcategory/skillcategory_skill/SkillCategorySkillControllerTest.java @@ -84,8 +84,11 @@ void testCreateNullIds() { @Test void testCreateNotAllowed() { + SkillCategory defaultSkillCategory = createDefaultSkillCategory(); + Skill aDefaultSkill = createADefaultSkill(); + SkillCategorySkill skillCategorySkill = new SkillCategorySkill(defaultSkillCategory.getId(),aDefaultSkill.getId()); HttpRequest httpRequest = HttpRequest - .POST("/", new SkillCategorySkillId()) + .POST("/", skillCategorySkill.getSkillCategorySkillId()) .basicAuth(MEMBER_ROLE, MEMBER_ROLE); final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(httpRequest, Map.class)); @@ -111,7 +114,7 @@ void testDelete() { @Test - void testDeleteDontExist() { + void testDeleteDoesntExist() { SkillCategorySkillId skillCategorySkillId = new SkillCategorySkillId(UUID.randomUUID(),UUID.randomUUID()); HttpRequest httpRequest = HttpRequest .DELETE("/", skillCategorySkillId) @@ -123,12 +126,15 @@ void testDeleteDontExist() { @Test void testDeleteNotAllowed() { + SkillCategory defaultSkillCategory = createDefaultSkillCategory(); + Skill aDefaultSkill = createADefaultSkill(); + SkillCategorySkill skillCategorySkill = createSkillCategorySkill(defaultSkillCategory.getId(), aDefaultSkill.getId()); HttpRequest httpRequest = HttpRequest - .DELETE("/", new SkillCategorySkillId()) + .DELETE("/", skillCategorySkill.getSkillCategorySkillId()) .basicAuth(MEMBER_ROLE, MEMBER_ROLE); final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(httpRequest, Map.class)); assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus()); } -} \ No newline at end of file +} From 5116b3d73c58310d2f1e6d7557f6cc6c181aefc4 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Fri, 24 Jan 2025 12:33:46 -0600 Subject: [PATCH 2/9] Fixed incorrect references to "role". --- .../services/pulseresponse/PulseResponseController.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/services/pulseresponse/PulseResponseController.java b/server/src/main/java/com/objectcomputing/checkins/services/pulseresponse/PulseResponseController.java index 5a1a96167d..cb8839385f 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/pulseresponse/PulseResponseController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/pulseresponse/PulseResponseController.java @@ -83,11 +83,11 @@ public HttpResponse update(@Body @Valid @NotNull PulseResponse pu * @return */ @Get("/{id}") - public PulseResponse readRole(@NotNull UUID id) { + public PulseResponse readPulse(@NotNull UUID id) { PulseResponse result = pulseResponseServices.read(id); if (result == null) { - throw new NotFoundException("No role item for UUID"); + throw new NotFoundException("No pulse item for UUID"); } return result; } -} \ No newline at end of file +} From 886d421ea4ac3d49efde45f1108474ff30451b78 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 27 Jan 2025 07:57:51 -0600 Subject: [PATCH 3/9] Added permissions to replace calls to isAdmin() on the current user services. --- .../EmployeeHoursServicesImpl.java | 7 +++--- .../FeedbackTemplateServicesImpl.java | 13 ++++++---- .../TemplateQuestionServicesImpl.java | 5 ++-- .../services/file/FileServicesBaseImpl.java | 18 +++++++------ .../services/file/FileServicesImpl.java | 3 +-- .../services/guild/GuildServicesImpl.java | 13 +++++++--- .../guild/member/GuildMemberServicesImpl.java | 15 +++++++---- .../KudosRecipientServicesImpl.java | 4 +-- .../MemberProfileServicesImpl.java | 12 ++++++--- .../RetentionReportServicesImpl.java | 4 --- .../services/permissions/Permission.java | 11 +++++--- .../reports/ReportDataServicesImpl.java | 5 +--- .../services/team/TeamServicesImpl.java | 13 +++++----- .../team/member/TeamMemberServicesImpl.java | 23 ++++++++++------- .../resources/db/dev/R__Load_testing_data.sql | 25 +++++++++++++++++++ .../EmployeeHoursControllerTest.java | 21 ++++++++-------- .../FeedbackTemplateControllerTest.java | 1 + .../services/fixture/PermissionFixture.java | 7 +++++- .../CheckServicesImplTest.java | 9 ++++++- 19 files changed, 135 insertions(+), 74 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java index c63b3ce651..f728b2770a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServicesImpl.java @@ -39,7 +39,6 @@ public EmployeeHoursServicesImpl(CurrentUserServices currentUserServices, @RequiredPermission(Permission.CAN_UPLOAD_HOURS) public EmployeeHoursResponseDTO save(CompletedFileUpload file) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); List employeeHoursList = new ArrayList<>(); Set employeeHours = new HashSet<>(); EmployeeHoursResponseDTO responseDTO = new EmployeeHoursResponseDTO(); @@ -63,17 +62,17 @@ public EmployeeHoursResponseDTO save(CompletedFileUpload file) { @Override public Set findByFields(String employeeId) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canViewAll = currentUserServices.hasPermission(Permission.CAN_VIEW_ALL_UPLOADED_HOURS); Set employeeHours = new HashSet<>(); employeehourRepo.findAll().forEach(employeeHours::add); if(employeeId !=null) { - validate((!isAdmin && currentUser!=null&& !currentUser.getEmployeeId().equals(employeeId)), + validate((!canViewAll && currentUser!=null && !currentUser.getEmployeeId().equals(employeeId)), NOT_AUTHORIZED_MSG); employeeHours.retainAll(employeehourRepo.findByEmployeeId(employeeId)); } else { - validate(!isAdmin, NOT_AUTHORIZED_MSG); + validate(!canViewAll, NOT_AUTHORIZED_MSG); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateServicesImpl.java index ed180269df..0f92edd053 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateServicesImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.feedback_template; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -93,12 +94,12 @@ public FeedbackTemplate getById(UUID id) { @Override public List findByFields(@Nullable UUID creatorId, @Nullable String title) { UUID currentUserId = currentUserServices.getCurrentUser().getId(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); List allTemplates = feedbackTemplateRepository.searchByValues(Util.nullSafeUUIDToString(creatorId), title); return allTemplates .stream() - .filter(template -> template.getIsPublic() || isAdmin || template.getCreatorId().equals(currentUserId)) - .filter(template -> !template.getIsReview() || isAdmin) + .filter(template -> template.getIsPublic() || canAdminister || template.getCreatorId().equals(currentUserId)) + .filter(template -> !template.getIsReview() || canAdminister) .toList(); } @@ -113,12 +114,14 @@ public boolean setAdHocInactiveByCreator(@Nullable UUID creatorId) { public boolean updateIsPermitted(UUID creatorId) { UUID currentUserId = currentUserServices.getCurrentUser().getId(); - boolean isAdmin = currentUserServices.isAdmin(); - return isAdmin || currentUserId.equals(creatorId); + return hasAdministerPermission() || currentUserId.equals(creatorId); } public boolean deleteIsPermitted(UUID creatorId) { return updateIsPermitted(creatorId); } + private boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_TEMPLATES); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/template_question/TemplateQuestionServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/template_question/TemplateQuestionServicesImpl.java index 6d89beac04..1b41c0fa9e 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/template_question/TemplateQuestionServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_template/template_question/TemplateQuestionServicesImpl.java @@ -1,5 +1,5 @@ package com.objectcomputing.checkins.services.feedback_template.template_question; - +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -139,9 +139,8 @@ public List findByFields(UUID templateId) { // only admins or the creator of the template can add questions to it public boolean createIsPermitted(UUID templateCreatorId) { - boolean isAdmin = currentUserServices.isAdmin(); UUID currentUserId = currentUserServices.getCurrentUser().getId(); - return currentUserId != null && (isAdmin || currentUserId.equals(templateCreatorId)); + return currentUserId != null && (currentUserServices.hasPermission(Permission.CAN_ADMINISTER_FEEDBACK_TEMPLATES) || currentUserId.equals(templateCreatorId)); } public boolean updateIsPermitted(UUID templateCreatorId) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java index 2149d531bf..ec4d9ceca0 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesBaseImpl.java @@ -58,12 +58,12 @@ abstract protected FileInfoDTO uploadSingleFile( @Override public Set findFiles(@Nullable UUID checkInID) { - boolean isAdmin = currentUserServices.isAdmin(); - validate(checkInID == null && !isAdmin, NOT_AUTHORIZED_MSG); + boolean canAdminister = hasAdministerPermission(); + validate(checkInID == null && !canAdminister, NOT_AUTHORIZED_MSG); try { Set result = new HashSet<>(); - if (checkInID == null && isAdmin) { + if (checkInID == null && canAdminister) { getCheckinDocuments(result, Collections.emptySet()); } else if (checkInID != null) { validate(!checkInServices.accessGranted(checkInID, currentUserServices.getCurrentUser().getId()), @@ -89,14 +89,14 @@ public Set findFiles(@Nullable UUID checkInID) { @Override public java.io.File downloadFiles(@NotNull String uploadDocId) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); CheckinDocument cd = checkinDocumentServices.getFindByUploadDocId(uploadDocId); validate(cd == null, String.format("Unable to find record with id %s", uploadDocId)); CheckIn associatedCheckin = checkInServices.read(cd.getCheckinsId()); - if(!isAdmin) { + if(!canAdminister) { validate((!currentUser.getId().equals(associatedCheckin.getTeamMemberId()) && !currentUser.getId().equals(associatedCheckin.getPdlId())), NOT_AUTHORIZED_MSG); } try { @@ -120,12 +120,12 @@ public java.io.File downloadFiles(@NotNull String uploadDocId) { @Override public FileInfoDTO uploadFile(@NotNull UUID checkInID, @NotNull CompletedFileUpload file) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); validate((file.getFilename() == null || file.getFilename().equals("")), "Please select a valid file before uploading."); CheckIn checkIn = checkInServices.read(checkInID); validate(checkIn == null, "Unable to find checkin record with id %s", checkInID); - if(!isAdmin) { + if(!canAdminister) { validate((!currentUser.getId().equals(checkIn.getTeamMemberId()) && !currentUser.getId().equals(checkIn.getPdlId())), "You are not authorized to perform this operation"); validate(checkIn.isCompleted(), NOT_AUTHORIZED_MSG); } @@ -175,4 +175,8 @@ protected void validate(boolean isError, String message, Object... args) { throw new FileRetrievalException(String.format(message, args)); } } + + protected boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_CHECKIN_DOCUMENTS); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesImpl.java index 38654da92f..3193a24c6a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/file/FileServicesImpl.java @@ -147,8 +147,7 @@ protected FileInfoDTO uploadSingleFile(CompletedFileUpload file, String director public FileInfoDTO uploadDocument(String directoryName, String name, String text) { final String GOOGLE_DOC_TYPE = "application/vnd.google-apps.document"; MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); - validate(!isAdmin, "You are not authorized to perform this operation"); + validate(!hasAdministerPermission(), "You are not authorized to perform this operation"); try { Drive drive = googleApiAccess.getDrive(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/guild/GuildServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/guild/GuildServicesImpl.java index 7408414e0f..697e6c01f4 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/guild/GuildServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/guild/GuildServicesImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.guild; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.Environments; import com.objectcomputing.checkins.configuration.CheckInsConfiguration; import com.objectcomputing.checkins.exceptions.BadArgException; @@ -135,8 +136,8 @@ public GuildResponseDTO read(@NotNull UUID guildId) { public GuildResponseDTO update(GuildUpdateDTO guildDTO) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); - if (isAdmin || (currentUser != null && + boolean canAdminister = hasAdministerPermission(); + if (canAdminister || (currentUser != null && !guildMemberServices.findByFields(guildDTO.getId(), currentUser.getId(), true).isEmpty())) { // Guild newGuildEntity = null; GuildResponseDTO updated= null; @@ -236,9 +237,9 @@ public Set findByFields(String name, UUID memberId) { public boolean delete(@NotNull UUID id) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); - if (isAdmin || (currentUser != null && !guildMemberRepo.search(nullSafeUUIDToString(id), nullSafeUUIDToString(currentUser.getId()), true).isEmpty())) { + if (canAdminister || (currentUser != null && !guildMemberRepo.search(nullSafeUUIDToString(id), nullSafeUUIDToString(currentUser.getId()), true).isEmpty())) { guildMemberHistoryRepository.deleteByGuildId(id); guildMemberRepo.deleteByGuildId(id.toString()); guildsRepo.deleteById(id); @@ -341,4 +342,8 @@ public void emailGuildLeaders(Set guildLeadersEmails, Guild guild) { String body = "Congratulations, you have been assigned as a guild leader of " + guild.getName(); emailSender.sendEmail(null, null, subject, body, guildLeadersEmails.toArray(new String[0])); } + + private boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_GUILDS); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberServicesImpl.java index a3fbb5e5d1..f436a462f1 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/guild/member/GuildMemberServicesImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.guild.member; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.configuration.CheckInsConfiguration; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; @@ -76,11 +77,11 @@ public GuildMember save(@Valid @NotNull GuildMember guildMember, boolean sendEma throw new BadArgException(String.format("Member %s already exists in guild %s", memberId, guildId)); } // only allow admins to create guild leads - else if (!currentUserServices.isAdmin() && Boolean.TRUE.equals(guildMember.getLead())) { + else if (!hasAdministerPermission() && Boolean.TRUE.equals(guildMember.getLead())) { throw new BadArgException(NOT_AUTHORIZED_MSG); } // only admins and leads can add members to guilds unless a user adds themself - else if (!currentUserServices.isAdmin() && !guildMember.getMemberId().equals(currentUser.getId()) && !isLead){ + else if (!hasAdministerPermission() && !guildMember.getMemberId().equals(currentUser.getId()) && !isLead){ throw new PermissionException(NOT_AUTHORIZED_MSG); } @@ -103,7 +104,7 @@ public GuildMember read(@NotNull UUID id) { public GuildMember update(@NotNull @Valid GuildMember guildMember) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); final UUID id = guildMember.getId(); final UUID guildId = guildMember.getGuildId(); @@ -122,7 +123,7 @@ public GuildMember update(@NotNull @Valid GuildMember guildMember) { throw new BadArgException(String.format("Member %s doesn't exist", memberId)); } else if (guildMemberRepo.findByGuildIdAndMemberId(guildMember.getGuildId(), guildMember.getMemberId()).isEmpty()) { throw new BadArgException(String.format("Member %s is not part of guild %s", memberId, guildId)); - } else if (!isAdmin && guildLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { + } else if (!canAdminister && guildLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { throw new BadArgException(NOT_AUTHORIZED_MSG); } GuildMember guildMemberUpdate = guildMemberRepo.update(guildMember); @@ -161,7 +162,7 @@ public void delete(@NotNull UUID id, boolean sendEmail) { throw new BadArgException("At least one guild lead must be present in the guild at all times"); } // if the current user is not an admin, is not the same as the member in the request, and is not a lead in the guild -> don't delete - if (!currentUserServices.isAdmin() && !guildMember.getMemberId().equals(currentUser.getId()) && !currentUserIsLead) { + if (!hasAdministerPermission() && !guildMember.getMemberId().equals(currentUser.getId()) && !currentUserIsLead) { throw new PermissionException(NOT_AUTHORIZED_MSG); } @@ -211,4 +212,8 @@ private String constructEmailContent (GuildMember guildMember, boolean isAdded){ private static GuildMemberHistory buildGuildMemberHistory(UUID guildId, UUID memberId, String change, LocalDateTime date) { return new GuildMemberHistory(guildId,memberId,change,date); } + + private boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_GUILDS); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/kudos/kudos_recipient/KudosRecipientServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/kudos/kudos_recipient/KudosRecipientServicesImpl.java index 167495e7ec..90dc2d5403 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/kudos/kudos_recipient/KudosRecipientServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/kudos/kudos_recipient/KudosRecipientServicesImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.kudos.kudos_recipient; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.notifications.email.EmailSender; import com.objectcomputing.checkins.notifications.email.MailJetFactory; import com.objectcomputing.checkins.exceptions.BadArgException; @@ -54,8 +55,7 @@ public KudosRecipient save(KudosRecipient kudosRecipient) { new NotFoundException("No kudos with id %s")); boolean isKudosCreator = currentUserServices.getCurrentUser().getId().equals(kudos.getSenderId()); - boolean isAdmin = currentUserServices.isAdmin(); - if (!isAdmin && !isKudosCreator) { + if (!currentUserServices.hasPermission(Permission.CAN_ADMINISTER_KUDOS) && !isKudosCreator) { throw new PermissionException(NOT_AUTHORIZED_MSG); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java index 61cd5c896f..d2d112480a 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServicesImpl.java @@ -67,7 +67,7 @@ public MemberProfile getById(@NotNull UUID id) { throw new NotFoundException("No member profile for id " + id); } MemberProfile memberProfile = optional.get(); - if (!currentUserServices.isAdmin()) { + if (!hasAdministerPermission()) { memberProfile.clearBirthYear(); } return memberProfile; @@ -91,7 +91,7 @@ public Set findByValues(@Nullable String firstName, @Nullable Boolean terminated) { Set memberProfiles = new HashSet<>(memberProfileRepository.search(firstName, null, lastName, null, title, nullSafeUUIDToString(pdlId), workEmail, nullSafeUUIDToString(supervisorId), terminated)); - if (!currentUserServices.isAdmin()) { + if (!hasAdministerPermission()) { for (MemberProfile memberProfile : memberProfiles) { memberProfile.clearBirthYear(); } @@ -199,7 +199,7 @@ public List findAll() { @Cacheable public List getSupervisorsForId(UUID id) { List supervisorsForId = memberProfileRepository.findSupervisorsForId(id); - if (!currentUserServices.isAdmin()) { + if (!hasAdministerPermission()) { for (MemberProfile memberProfile : supervisorsForId) { memberProfile.clearBirthYear(); } @@ -211,7 +211,7 @@ public List getSupervisorsForId(UUID id) { @Cacheable public List getSubordinatesForId(UUID id) { List subordinatesForId = memberProfileRepository.findSubordinatesForId(id); - if (!currentUserServices.isAdmin()) { + if (!hasAdministerPermission()) { for (MemberProfile memberProfile : subordinatesForId) { memberProfile.clearBirthYear(); } @@ -270,4 +270,8 @@ public void updateLastSeen(UUID id) { memberProfileRepository.update(memberProfile); } } + + private boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_EDIT_ALL_ORGANIZATION_MEMBERS); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java index 321d016a28..58a7402dd2 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/retentionreport/RetentionReportServicesImpl.java @@ -27,10 +27,6 @@ public RetentionReportServicesImpl(MemberProfileServices memberProfileServices, @Override @RequiredPermission(Permission.CAN_VIEW_RETENTION_REPORT) public RetentionReportResponseDTO report(RetentionReportDTO request) { - if (!currentUserServices.isAdmin()) { - throw new PermissionException("Requires admin privileges"); - } - RetentionReportResponseDTO response; List memberProfiles = memberProfileServices.findAll(); LocalDate periodStartDate = getIntervalStartDate(request.getStartDate(), request.getFrequency()); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java index d2c867d029..ba203ca9ae 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java @@ -15,6 +15,7 @@ public enum Permission { CAN_ADMINISTER_KUDOS("Administer kudos", "Feedback"), CAN_VIEW_FEEDBACK_ANSWER("View feedback answers", "Feedback"), CAN_ADMINISTER_FEEDBACK_ANSWER("Administer feedback answers", "Feedback"), + CAN_ADMINISTER_FEEDBACK_TEMPLATES("Administer feedback templates", "Feedback"), CAN_SEND_EMAIL("Send email", "Feedback"), CAN_EDIT_ALL_ORGANIZATION_MEMBERS("Edit all member profiles", "User Management"), CAN_DELETE_ORGANIZATION_MEMBERS("Delete organization members", "User Management"), @@ -32,6 +33,7 @@ public enum Permission { CAN_VIEW_CHECKINS_REPORT("View checkins report", "Reporting"), CAN_CREATE_MERIT_REPORT("Create Merit Reports", "Reporting"), CAN_UPLOAD_HOURS("Upload Hours", "Reporting"), + CAN_VIEW_ALL_UPLOADED_HOURS("View all uploaded hours", "Reporting"), CAN_CREATE_CHECKINS("Create check-ins", "Check-ins"), CAN_VIEW_CHECKINS("View check-ins", "Check-ins"), CAN_UPDATE_CHECKINS("Update check-ins", "Check-ins"), @@ -40,6 +42,7 @@ public enum Permission { CAN_UPDATE_PRIVATE_NOTE("Update check-ins private notes", "Check-ins"), CAN_CREATE_PRIVATE_NOTE("Create check-ins private notes", "Check-ins"), CAN_VIEW_CHECKIN_DOCUMENT("View check-ins document", "Check-ins"), + CAN_ADMINISTER_CHECKIN_DOCUMENTS("Administer check-ins documents", "Check-ins"), CAN_UPDATE_CHECKIN_DOCUMENT("Update check-ins document", "Check-ins"), CAN_CREATE_CHECKIN_DOCUMENT("Create check-ins document", "Check-ins"), CAN_DELETE_CHECKIN_DOCUMENT("Delete check-ins document", "Check-ins"), @@ -62,9 +65,11 @@ public enum Permission { CAN_VIEW_ALL_PULSE_RESPONSES("View pulse responses", "Reporting"), CAN_MANAGE_CERTIFICATIONS("Manage certifications", "Certifications"), CAN_MANAGE_EARNED_CERTIFICATIONS("Manage earned certifications", "Certifications"), - CAN_ADMINISTER_VOLUNTEERING_ORGANIZATIONS("Update volunteering organizations", "Volunteering"), - CAN_ADMINISTER_VOLUNTEERING_RELATIONSHIPS("Update volunteering relationships", "Volunteering"), - CAN_ADMINISTER_VOLUNTEERING_EVENTS("Update volunteering events", "Volunteering"), + CAN_ADMINISTER_VOLUNTEERING_ORGANIZATIONS("Administer volunteering organizations", "Volunteering"), + CAN_ADMINISTER_VOLUNTEERING_RELATIONSHIPS("Administer volunteering relationships", "Volunteering"), + CAN_ADMINISTER_VOLUNTEERING_EVENTS("Administer volunteering events", "Volunteering"), + CAN_ADMINISTER_GUILDS("Administer guilds", "Guilds"), + CAN_ADMINISTER_TEAMS("Administer teams", "Teams"), CAN_ADMINISTER_DOCUMENTATION("Administer documentation and role documentation", "Documentation"); private final String description; diff --git a/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java index 824b872bfd..50c4277a58 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/reports/ReportDataServicesImpl.java @@ -56,8 +56,6 @@ public ReportDataServicesImpl(CurrentUserServices currentUserServices) { @RequiredPermission(Permission.CAN_CREATE_MERIT_REPORT) public synchronized void store(DataType dataType, CompletedFileUpload file) throws IOException { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); - validate(!isAdmin, NOT_AUTHORIZED_MSG); // Get the map for the current user. Stored perUser; @@ -78,10 +76,9 @@ public synchronized void store(DataType dataType, CompletedFileUpload file) thro } @Override + @RequiredPermission(Permission.CAN_CREATE_MERIT_REPORT) public ByteBuffer get(DataType dataType) throws NotFoundException { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); - validate(!isAdmin, NOT_AUTHORIZED_MSG); UUID id = currentUser.getId(); if (storedUploads.containsKey(id)) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/team/TeamServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/team/TeamServicesImpl.java index b2bfa95d60..c63ed2aeb6 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/team/TeamServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/team/TeamServicesImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.team; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -83,9 +84,7 @@ public TeamResponseDTO read(@NotNull UUID teamId) { public TeamResponseDTO update(TeamUpdateDTO teamDTO) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); - - if (isAdmin || (currentUser != null && + if (hasAdministerPermission() || (currentUser != null && !teamMemberServices.findByFields(teamDTO.getId(), currentUser.getId(), true).isEmpty())) { TeamResponseDTO updated = null; @@ -149,9 +148,7 @@ public Set findByFields(String name, UUID memberid) { public boolean delete(@NotNull UUID id) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); - - if (isAdmin || (currentUser != null && !teamMemberServices.findByFields(id, currentUser.getId(), true).isEmpty())) { + if (hasAdministerPermission() || (currentUser != null && !teamMemberServices.findByFields(id, currentUser.getId(), true).isEmpty())) { teamMemberServices.deleteByTeam(id); teamsRepo.deleteById(id); } else { @@ -202,4 +199,8 @@ private TeamMemberResponseDTO fromMemberEntity(TeamMember teamMember, MemberProf return new TeamMemberResponseDTO(teamMember.getId(), memberProfile.getFirstName(), memberProfile.getLastName(), memberProfile.getId(), teamMember.isLead()); } + + private boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_TEAMS); + } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/team/member/TeamMemberServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/team/member/TeamMemberServicesImpl.java index 85fffb96b4..f0fde3a7f9 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/team/member/TeamMemberServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/team/member/TeamMemberServicesImpl.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.team.member; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; import com.objectcomputing.checkins.exceptions.PermissionException; @@ -50,7 +51,7 @@ private static MemberHistory buildMemberHistory(UUID teamId, UUID memberId, Stri public TeamMember save(@Valid @NotNull TeamMember teamMember) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); final UUID teamId = teamMember.getTeamId(); final UUID memberId = teamMember.getMemberId(); @@ -67,7 +68,7 @@ public TeamMember save(@Valid @NotNull TeamMember teamMember) { throw new BadArgException(String.format("Member %s doesn't exist", memberId)); } else if (teamMemberRepo.findByTeamIdAndMemberId(teamMember.getTeamId(), teamMember.getMemberId()).isPresent()) { throw new BadArgException(String.format("Member %s already exists in team %s", memberId, teamId)); - } else if (!isAdmin && teamLeads.size() > 0 && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { + } else if (!canAdminister && teamLeads.size() > 0 && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { throw new BadArgException(NOT_AUTHORIZED_MSG); } @@ -83,7 +84,7 @@ public TeamMember read(@NotNull UUID id) { public TeamMember update(@NotNull @Valid TeamMember teamMember) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); final UUID id = teamMember.getId(); final UUID teamId = teamMember.getTeamId(); @@ -102,7 +103,7 @@ public TeamMember update(@NotNull @Valid TeamMember teamMember) { throw new BadArgException(String.format("Member %s doesn't exist", memberId)); } else if (teamMemberRepo.findByTeamIdAndMemberId(teamMember.getTeamId(), teamMember.getMemberId()).isEmpty()) { throw new BadArgException(String.format("Member %s is not part of team %s", memberId, teamId)); - } else if (!isAdmin && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { + } else if (!canAdminister && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { throw new BadArgException(NOT_AUTHORIZED_MSG); } @@ -130,13 +131,13 @@ public Set findByFields(@Nullable UUID teamId, @Nullable UUID member public void delete(@NotNull UUID id) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); TeamMember teamMember = teamMemberRepo.findById(id).orElse(null); if (teamMember != null) { Set teamLeads = this.findByFields(teamMember.getTeamId(), null, true); - if (!isAdmin && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { + if (!canAdminister && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { throw new PermissionException(NOT_AUTHORIZED_MSG); } else { teamMemberRepo.deleteById(id); @@ -151,13 +152,13 @@ public void delete(@NotNull UUID id) { public void deleteByTeam(@NotNull UUID id) { MemberProfile currentUser = currentUserServices.getCurrentUser(); - boolean isAdmin = currentUserServices.isAdmin(); + boolean canAdminister = hasAdministerPermission(); List teamMembers = teamMemberRepo.findByTeamId(id); if (teamMembers != null) { List teamLeads = teamMembers.stream().filter(TeamMember::isLead).toList(); - if (!isAdmin && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { + if (!canAdminister && teamLeads.stream().noneMatch(o -> o.getMemberId().equals(currentUser.getId()))) { throw new PermissionException(NOT_AUTHORIZED_MSG); } else { teamMembers.forEach(member -> { @@ -169,4 +170,8 @@ public void deleteByTeam(@NotNull UUID id) { throw new NotFoundException(String.format("Unable to locate team with id %s", id)); } } -} \ No newline at end of file + + private boolean hasAdministerPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_TEAMS); + } +} diff --git a/server/src/main/resources/db/dev/R__Load_testing_data.sql b/server/src/main/resources/db/dev/R__Load_testing_data.sql index 9af6a8afc3..3c3cb5d642 100644 --- a/server/src/main/resources/db/dev/R__Load_testing_data.sql +++ b/server/src/main/resources/db/dev/R__Load_testing_data.sql @@ -918,11 +918,36 @@ insert into role_permissions values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_UPLOAD_HOURS'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_VIEW_ALL_UPLOADED_HOURS'); + insert into role_permissions (roleid, permission) values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_SEND_EMAIL'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_ADMINISTER_FEEDBACK_TEMPLATES'); + +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_ADMINISTER_CHECKIN_DOCUMENTS'); + +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_ADMINISTER_GUILDS'); + +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_ADMINISTER_TEAMS'); + -- PDL Permissions insert into role_permissions (roleid, permission) diff --git a/server/src/test/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursControllerTest.java index 8913ed74ed..b85231e089 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursControllerTest.java @@ -17,6 +17,7 @@ import io.micronaut.http.client.multipart.MultipartBody; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.BeforeEach; import java.io.File; import java.io.IOException; @@ -40,6 +41,14 @@ class EmployeeHoursControllerTest extends TestContainersSuite implements MemberP @Client("/services/employee/hours") HttpClient client; + private MemberProfile user; + + @BeforeEach + void setUp() { + user = createAnUnrelatedUser(); + createAndAssignAdminRole(user); + } + @Test void testCreateEmployeeHours() { File file = new File("src/test/java/com/objectcomputing/checkins/services/employee_hours/test.csv"); @@ -50,9 +59,6 @@ void testCreateEmployeeHours() { MemberProfile memberProfile=createADefaultMemberProfile(); createADefaultMemberProfileForPdl(memberProfile); - MemberProfile user = createAnUnrelatedUser(); - createAndAssignAdminRole(user); - final HttpRequest request = HttpRequest.POST("/upload", multipartBody).basicAuth(user.getWorkEmail(),ADMIN_ROLE).contentType(MediaType.MULTIPART_FORM_DATA); final HttpResponse response = client.toBlocking().exchange(request, EmployeeHoursResponseDTO.class); @@ -71,9 +77,6 @@ void testCreateEmployeeHoursWithNoTeamMemberId() { .addPart("file","test.csv",new MediaType("text/csv"),file) .build(); - MemberProfile user = createAnUnrelatedUser(); - createAndAssignAdminRole(user); - final HttpRequest request = HttpRequest.POST("/upload", multipartBody).basicAuth(user.getWorkEmail(),ADMIN_ROLE).contentType(MediaType.MULTIPART_FORM_DATA); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -100,8 +103,6 @@ void testCreateEmployeeHoursWithNoAdminRole() { void testFindAllRecordsWithAdminRole() throws IOException { MemberProfile memberProfile=createADefaultMemberProfile(); createADefaultMemberProfileForPdl(memberProfile); - MemberProfile user = createAnUnrelatedUser(); - createAndAssignAdminRole(user); createEmployeeHours(); final HttpRequest request = HttpRequest.GET("/").basicAuth(user.getWorkEmail(),ADMIN_ROLE); @@ -118,7 +119,7 @@ void testFindRecordsWithEmployeeId() throws IOException { MemberProfile memberProfile=createADefaultMemberProfile(); createADefaultMemberProfileForPdl(memberProfile); List employeeHoursList = createEmployeeHours(); - final HttpRequest request = HttpRequest.GET(String.format("/?employeeId=%s",employeeHoursList.get(0).getEmployeeId())).basicAuth(ADMIN_ROLE,ADMIN_ROLE); + final HttpRequest request = HttpRequest.GET(String.format("/?employeeId=%s",employeeHoursList.get(0).getEmployeeId())).basicAuth(user.getWorkEmail(), ADMIN_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(EmployeeHours.class)); assertEquals(Set.of(employeeHoursList.get(0)),response.body()); assertEquals(HttpStatus.OK,response.getStatus()); @@ -145,7 +146,7 @@ void testFindAllRecordsWithNonAdminRole() throws IOException { @Test void testFindEmployeeHoursNotFound() { - final HttpRequest request = HttpRequest.GET("/?employeeId=invalid_id").basicAuth(ADMIN_ROLE,ADMIN_ROLE); + final HttpRequest request = HttpRequest.GET("/?employeeId=invalid_id").basicAuth(user.getWorkEmail(), ADMIN_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(EmployeeHours.class)); Set emptySet = Collections.emptySet(); assertEquals(emptySet,response.body()); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateControllerTest.java index 749b1f3648..9932977217 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplateControllerTest.java @@ -380,6 +380,7 @@ void testFindDoesNotGrabReviewsForNonAdmin() { @Test void testFindReviewsForAdmin() { final MemberProfile memberOne = createADefaultMemberProfile(); + createAndAssignAdminRole(memberOne); final FeedbackTemplate template = saveDefaultFeedbackTemplate(memberOne.getId()); final FeedbackTemplate templateTwo = saveAnotherDefaultFeedbackTemplate(memberOne.getId()); final FeedbackTemplate templateThree = saveReviewFeedbackTemplate(memberOne.getId()); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java index e79aeb7f45..d376df2b60 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java @@ -104,7 +104,12 @@ public interface PermissionFixture extends RolePermissionFixture { Permission.CAN_IMPERSONATE_MEMBERS, Permission.CAN_SEND_EMAIL, Permission.CAN_CREATE_MERIT_REPORT, - Permission.CAN_UPLOAD_HOURS + Permission.CAN_UPLOAD_HOURS, + Permission.CAN_VIEW_ALL_UPLOADED_HOURS, + Permission.CAN_ADMINISTER_FEEDBACK_TEMPLATES, + Permission.CAN_ADMINISTER_GUILDS, + Permission.CAN_ADMINISTER_TEAMS, + Permission.CAN_ADMINISTER_CHECKIN_DOCUMENTS ); default void setPermissionsForAdmin(UUID roleID) { diff --git a/server/src/test/java/com/objectcomputing/checkins/services/request_notifications/CheckServicesImplTest.java b/server/src/test/java/com/objectcomputing/checkins/services/request_notifications/CheckServicesImplTest.java index debee294cf..ae589d9729 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/request_notifications/CheckServicesImplTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/request_notifications/CheckServicesImplTest.java @@ -5,12 +5,14 @@ import com.objectcomputing.checkins.services.feedback_template.FeedbackTemplate; import com.objectcomputing.checkins.services.fixture.FeedbackRequestFixture; import com.objectcomputing.checkins.services.fixture.FeedbackTemplateFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.pulse.PulseServices; import com.objectcomputing.checkins.services.reviews.ReviewPeriodServices; import com.objectcomputing.checkins.notifications.email.MailJetFactory; import com.objectcomputing.checkins.services.MailJetFactoryReplacement; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; +import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -27,8 +29,11 @@ import static org.junit.jupiter.api.Assertions.assertEquals; @Property(name = "replace.mailjet.factory", value = StringUtils.TRUE) +@Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class CheckServicesImplTest extends TestContainersSuite - implements FeedbackTemplateFixture, FeedbackRequestFixture, MemberProfileFixture { + implements FeedbackTemplateFixture, FeedbackRequestFixture, MemberProfileFixture, RoleFixture { + @Inject + CurrentUserServicesReplacement currentUserServices; @Inject @Named(MailJetFactory.MJML_FORMAT) @@ -39,6 +44,8 @@ class CheckServicesImplTest extends TestContainersSuite @BeforeEach void resetTest() { + currentUserServices.currentUser = createAThirdDefaultMemberProfile(); + createAndAssignAdminRole(currentUserServices.currentUser); emailSender.reset(); } From 06b02527f586ce18fddf19b0447d201517f7b2d7 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 27 Jan 2025 09:39:24 -0600 Subject: [PATCH 4/9] Removed isAdmin() from this service implementation. --- .../skills/combineskills/CombineSkillServicesImpl.java | 6 +++--- .../skills/combineskills/CombineSkillsControllerTest.java | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillServicesImpl.java index c27908bb93..198534dae0 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillServicesImpl.java @@ -1,5 +1,7 @@ package com.objectcomputing.checkins.services.skills.combineskills; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; import com.objectcomputing.checkins.services.member_skill.MemberSkill; import com.objectcomputing.checkins.services.member_skill.MemberSkillServices; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; @@ -32,10 +34,8 @@ public CombineSkillServicesImpl(SkillServices skillServices, this.currentUserServices = currentUserServices; } + @RequiredPermission(Permission.CAN_EDIT_SKILLS) public Skill combine(@NotNull @Valid CombineSkillsDTO skillDTO) { - final boolean isAdmin = currentUserServices.isAdmin(); - permissionsValidation.validatePermissions(!isAdmin); - Set existingSkills = skillServices.findByValue(skillDTO.getName(), null); for (Skill existingSkill : existingSkills) { if (existingSkill.getName().equals(skillDTO.getName())) { diff --git a/server/src/test/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillsControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillsControllerTest.java index 5e2e7e8fc3..a5c7af4957 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillsControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/skills/combineskills/CombineSkillsControllerTest.java @@ -28,7 +28,6 @@ import static com.objectcomputing.checkins.services.role.RoleType.Constants.ADMIN_ROLE; import static com.objectcomputing.checkins.services.role.RoleType.Constants.MEMBER_ROLE; -import static com.objectcomputing.checkins.services.validate.PermissionsValidation.NOT_AUTHORIZED_MSG; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -135,7 +134,7 @@ void testPOSTCombine2SkillsIntoOneNonAdmin() { String href = Objects.requireNonNull(body).get("_links").get("self").get("href").asText(); assertEquals(request.getPath(), href); - assertEquals(NOT_AUTHORIZED_MSG, error); + assertEquals("Forbidden", error); } @@ -305,4 +304,4 @@ void testPOSTCombine2SkillsIntoOneCheckMemberSkills() { } -} \ No newline at end of file +} From a78bedb0fb81391c71a53b33547759c28882e18e Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Thu, 30 Jan 2025 10:49:26 -0600 Subject: [PATCH 5/9] Replaced selectIsAdmin and checking for ADMIN role with permissions. --- web-ui/src/components/admin/users/Users.jsx | 3 ++- .../components/checkin/documents/CheckinDocs.jsx | 6 +++--- .../member-directory/AdminMemberCard.jsx | 5 ++++- .../member-directory/MemberSummaryCard.jsx | 7 +++++-- web-ui/src/components/menu/Menu.jsx | 4 ++-- .../src/components/private-note/PrivateNote.jsx | 4 ++-- web-ui/src/components/reviews/TeamReviews.jsx | 4 ++-- web-ui/src/components/routes/Routes.jsx | 1 - web-ui/src/context/AppContext.jsx | 3 ++- web-ui/src/context/selectors.js | 16 ++++++++++++++++ web-ui/src/pages/CheckinsPage.jsx | 8 ++++---- web-ui/src/pages/MemberProfilePage.jsx | 6 +++--- web-ui/src/pages/ViewFeedbackPage.jsx | 4 ++-- 13 files changed, 47 insertions(+), 24 deletions(-) diff --git a/web-ui/src/components/admin/users/Users.jsx b/web-ui/src/components/admin/users/Users.jsx index 785c139212..f09251288d 100644 --- a/web-ui/src/components/admin/users/Users.jsx +++ b/web-ui/src/components/admin/users/Users.jsx @@ -19,6 +19,7 @@ import { selectNormalizedMembers, selectNormalizedMembersAdmin, selectHasCreateMembersPermission, + selectCanEditAllOrganizationMembers, } from '../../../context/selectors'; import { useQueryParameters } from '../../../helpers/query-parameters'; @@ -67,7 +68,7 @@ const Users = () => { }; const normalizedMembers = - includeTerminated && selectHasCreateMembersPermission(state) + includeTerminated && selectCanEditAllOrganizationMembers(state) ? selectNormalizedMembersAdmin(state, searchText) : selectNormalizedMembers(state, searchText); diff --git a/web-ui/src/components/checkin/documents/CheckinDocs.jsx b/web-ui/src/components/checkin/documents/CheckinDocs.jsx index 353e0db5c6..464104cdf9 100644 --- a/web-ui/src/components/checkin/documents/CheckinDocs.jsx +++ b/web-ui/src/components/checkin/documents/CheckinDocs.jsx @@ -8,8 +8,8 @@ import { selectCsrfToken, selectCurrentUser, selectIsPDL, - selectIsAdmin, - selectCheckin + selectCheckin, + selectCanAdministerCheckinDocuments, } from '../../../context/selectors'; import DescriptionIcon from '@mui/icons-material/Description'; import IconButton from '@mui/material/IconButton'; @@ -34,7 +34,7 @@ const UploadDocs = () => { const [files, setFiles] = useState([]); const [fileColors, setFileColors] = useState({}); - const pdlorAdmin = selectIsPDL(state) || selectIsAdmin(state); + const pdlorAdmin = selectIsPDL(state) || selectCanAdministerCheckinDocuments(state); const canView = pdlorAdmin && currentUserId !== memberId; useEffect(() => { diff --git a/web-ui/src/components/member-directory/AdminMemberCard.jsx b/web-ui/src/components/member-directory/AdminMemberCard.jsx index dabe0b5674..4b252712dc 100644 --- a/web-ui/src/components/member-directory/AdminMemberCard.jsx +++ b/web-ui/src/components/member-directory/AdminMemberCard.jsx @@ -10,6 +10,7 @@ import { selectHasCreateMembersPermission, selectHasDeleteMembersPermission, selectHasImpersonateMembersPermission, + selectCanEditAllOrganizationMembers, } from '../../context/selectors'; import { getAvatarURL, resolve } from '../../api/api.js'; @@ -86,7 +87,8 @@ const AdminMemberCard = ({ member, index }) => { // is due to the fact that users can edit their own profiles. But, only // certain users can create new profiles. So, we associate the edit feature // with profile creation. - if (selectHasCreateMembersPermission(state)) { + if (selectHasCreateMembersPermission(state) || + selectCanEditAllOrganizationMembers(state)) { entries.push('Edit'); actionFunctions.push(handleOpen); } @@ -228,6 +230,7 @@ const AdminMemberCard = ({ member, index }) => { {(selectHasCreateMembersPermission(state) || selectHasDeleteMembersPermission(state) || + selectCanEditAllOrganizationMembers(state) || selectHasImpersonateMembersPermission(state)) && ( {options().length > 0 && diff --git a/web-ui/src/components/member-directory/MemberSummaryCard.jsx b/web-ui/src/components/member-directory/MemberSummaryCard.jsx index f8ee684c2a..202fc8d5ef 100644 --- a/web-ui/src/components/member-directory/MemberSummaryCard.jsx +++ b/web-ui/src/components/member-directory/MemberSummaryCard.jsx @@ -3,7 +3,10 @@ import { styled } from '@mui/material/styles'; import { Link } from 'react-router-dom'; import { AppContext } from '../../context/AppContext'; -import { selectIsAdmin, selectProfileMap } from '../../context/selectors'; +import { + selectProfileMap, + selectCanEditAllOrganizationMembers, +} from '../../context/selectors'; import { getAvatarURL } from '../../api/api.js'; import Avatar from '@mui/material/Avatar'; @@ -34,7 +37,7 @@ const StyledBox = styled(Box)(() => ({ const MemberSummaryCard = ({ member }) => { const { state } = useContext(AppContext); - const isAdmin = selectIsAdmin(state); + const isAdmin = selectCanEditAllOrganizationMembers(state); const { location, name, diff --git a/web-ui/src/components/menu/Menu.jsx b/web-ui/src/components/menu/Menu.jsx index e3f63f67df..442ebe8fdd 100644 --- a/web-ui/src/components/menu/Menu.jsx +++ b/web-ui/src/components/menu/Menu.jsx @@ -13,7 +13,6 @@ import { selectHasSkillsReportPermission, selectHasTeamSkillsReportPermission, selectHasViewPulseReportPermission, - selectIsAdmin, selectHasEarnedCertificationsPermission, selectHasMeritReportPermission, selectHasVolunteeringEventsPermission, @@ -26,6 +25,7 @@ import { selectHasCreateMembersPermission, selectHasDeleteMembersPermission, selectHasImpersonateMembersPermission, + selectCanEditAllOrganizationMembers, selectHasUploadHoursPermission, selectHasPermissionAssignmentPermission, } from '../../context/selectors'; @@ -110,7 +110,6 @@ function Menu({ children }) { const csrf = selectCsrfToken(state); const { id, workEmail } = userProfile && userProfile.memberProfile ? userProfile.memberProfile : {}; - const isAdmin = selectIsAdmin(state); const hasReportPermission = selectHasReportPermission(state); const canViewFeedbackAnswer = selectCanViewFeedbackAnswerPermission(state); const canViewFeedbackRequest = selectCanViewFeedbackRequestPermission(state); @@ -144,6 +143,7 @@ function Menu({ children }) { ['/admin/roles', 'Roles', () => selectCanEditMemberRolesPermission(state)], ['/admin/users', 'Users', () => selectHasCreateMembersPermission(state) || selectHasDeleteMembersPermission(state) || + selectCanEditAllOrganizationMembers(state) || selectHasImpersonateMembersPermission(state) ], ['/admin/email', 'Send Email', () => selectHasSendEmailPermission(state)], diff --git a/web-ui/src/components/private-note/PrivateNote.jsx b/web-ui/src/components/private-note/PrivateNote.jsx index 6a975cf3fb..5f41f278bc 100644 --- a/web-ui/src/components/private-note/PrivateNote.jsx +++ b/web-ui/src/components/private-note/PrivateNote.jsx @@ -10,12 +10,12 @@ import { selectCsrfToken, selectCurrentUser, selectIsPDL, - selectIsAdmin, selectCheckin, selectProfile, selectCanViewPrivateNotesPermission, selectCanCreatePrivateNotesPermission, selectCanUpdatePrivateNotesPermission, + selectCanAdministerCheckinDocuments, } from '../../context/selectors'; import { UPDATE_TOAST } from '../../context/actions'; import { debounce } from 'lodash/function'; @@ -42,7 +42,7 @@ const PrivateNote = () => { const currentCheckin = selectCheckin(state, checkinId); const currentMember = selectProfile(state, memberId); const pdlId = currentMember?.pdlId; - const isAdmin = selectIsAdmin(state); + const isAdmin = selectCanAdministerCheckinDocuments(state); const noteRef = useRef([]); const [note, setNote] = useState(); diff --git a/web-ui/src/components/reviews/TeamReviews.jsx b/web-ui/src/components/reviews/TeamReviews.jsx index 418518416d..ef70a54d5e 100644 --- a/web-ui/src/components/reviews/TeamReviews.jsx +++ b/web-ui/src/components/reviews/TeamReviews.jsx @@ -55,7 +55,6 @@ import { } from '../../context/actions'; import { AppContext } from '../../context/AppContext'; import { - selectIsAdmin, selectCsrfToken, selectCurrentMembers, selectCurrentUser, @@ -70,6 +69,7 @@ import { selectHasCreateReviewAssignmentsPermission, selectHasDeleteReviewAssignmentsPermission, selectHasUpdateReviewAssignmentsPermission, + selectCanAdministerFeedbackRequests, } from '../../context/selectors'; import MemberSelector from '../member_selector/MemberSelector'; @@ -126,7 +126,7 @@ const ReviewStatus = { const TeamReviews = ({ onBack, periodId }) => { const { state, dispatch } = useContext(AppContext); const location = useLocation(); - const isAdmin = selectIsAdmin(state); + const isAdmin = selectCanAdministerFeedbackRequests(state); const [openMode, setOpenMode] = useState(false); const [approvalState, setApprovalState] = useState(false); const [assignments, setAssignments] = useState([]); diff --git a/web-ui/src/components/routes/Routes.jsx b/web-ui/src/components/routes/Routes.jsx index 25bc64ee75..c289c6abbc 100644 --- a/web-ui/src/components/routes/Routes.jsx +++ b/web-ui/src/components/routes/Routes.jsx @@ -28,7 +28,6 @@ import MeritReportPage from '../../pages/MeritReportPage'; import Users from '../admin/users/Users'; import VolunteerReportPage from '../../pages/VolunteerReportPage'; -import { selectIsAdmin } from '../../context/selectors'; import FeedbackRequestConfirmation from '../feedback_request_confirmation/FeedbackRequestConfirmation'; import FeedbackRequestPage from '../../pages/FeedbackRequestPage'; import ViewFeedbackPage from '../../pages/ViewFeedbackPage'; diff --git a/web-ui/src/context/AppContext.jsx b/web-ui/src/context/AppContext.jsx index bd08fc7dc2..5dd1752607 100644 --- a/web-ui/src/context/AppContext.jsx +++ b/web-ui/src/context/AppContext.jsx @@ -23,6 +23,7 @@ import { } from '../api/member'; import { selectCanViewCheckinsPermission, + selectCanEditAllOrganizationMembers, } from './selectors'; import { getAllRoles, getAllUserRoles } from '../api/roles'; import { getMemberSkills } from '../api/memberskill'; @@ -190,7 +191,7 @@ const AppContextProvider = props => { if (csrf && userProfile && !memberProfiles) { dispatch({ type: UPDATE_PEOPLE_LOADING, payload: true }); getMemberProfiles(); - if (userProfile.role?.includes('ADMIN')) { + if (selectCanEditAllOrganizationMembers(state)) { getTerminatedMembers(); } } diff --git a/web-ui/src/context/selectors.js b/web-ui/src/context/selectors.js index a724740dc5..fa203f0ba2 100644 --- a/web-ui/src/context/selectors.js +++ b/web-ui/src/context/selectors.js @@ -206,6 +206,10 @@ export const selectCanUpdateCheckinsPermission = hasPermission( 'CAN_UPDATE_CHECKINS' ); +export const selectCanUpdateAllCheckinsPermission = hasPermission( + 'CAN_UPDATE_ALL_CHECKINS' +); + export const selectCanEditSkills = hasPermission( 'CAN_EDIT_SKILLS' ); @@ -222,6 +226,18 @@ export const selectCanUpdatePrivateNotesPermission = hasPermission( 'CAN_UPDATE_PRIVATE_NOTE' ); +export const selectCanAdministerCheckinDocuments = hasPermission( + 'CAN_ADMINISTER_CHECKIN_DOCUMENTS' +); + +export const selectCanAdministerFeedbackRequests = hasPermission( + 'CAN_ADMINISTER_FEEDBACK_REQUEST' +); + +export const selectCanEditAllOrganizationMembers = hasPermission( + 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS', +); + export const selectIsPDL = createSelector( selectUserProfile, userProfile => diff --git a/web-ui/src/pages/CheckinsPage.jsx b/web-ui/src/pages/CheckinsPage.jsx index 4ac7b3c3f6..fe689998ef 100644 --- a/web-ui/src/pages/CheckinsPage.jsx +++ b/web-ui/src/pages/CheckinsPage.jsx @@ -7,7 +7,6 @@ import { AppContext } from '../context/AppContext'; import { selectMostRecentCheckin, selectCurrentUser, - selectIsAdmin, selectIsPDL, selectCsrfToken, selectCheckin, @@ -16,6 +15,7 @@ import { selectCanViewCheckinsPermission, selectCanUpdateCheckinsPermission, selectCanViewPrivateNotesPermission, + selectCanUpdateAllCheckinsPermission, } from '../context/selectors'; import { getCheckins, createNewCheckin } from '../context/thunks'; import { UPDATE_CHECKIN, UPDATE_TOAST } from '../context/actions'; @@ -89,12 +89,12 @@ const CheckinsPage = () => { }, [currentUserId, memberId, checkinId, mostRecent, history]); const currentCheckin = selectCheckin(state, checkinId); - const isAdmin = selectIsAdmin(state); + const updateAll = selectCanUpdateAllCheckinsPermission(state); const isPdl = selectIsPDL(state); const canViewPrivateNote = selectCanViewPrivateNotesPermission(state) && - (isAdmin || selectedProfile?.pdlId === currentUserId) && + (updateAll || selectedProfile?.pdlId === currentUserId) && currentUserId !== memberId; const handleOpen = () => setOpen(true); @@ -159,7 +159,7 @@ const CheckinsPage = () => { aria-describedby="checkin-tooltip-wrapper" className="create-checkin-tooltip-wrapper" > - {(isAdmin || isPdl || currentUserId === memberId) && ( + {(updateAll || isPdl || currentUserId === memberId) && (