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 f54c72988b..b6826d5dc4 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,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.NotFoundException; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.MediaType; @@ -39,25 +41,13 @@ public Set findEmployeeHours(@Nullable String employeeId) { } - /** - * @param id - * @return - */ - @Get("/{id}") - public EmployeeHours readEmployeeHours(@NotNull UUID id) { - EmployeeHours result = employeeHoursServices.read(id); - if (result == null) { - throw new NotFoundException("No employee hours for employee id"); - } - return result; - } - /** * Parse the CSV file and store it to employee hours table * @param file * @{@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/EmployeeHoursServices.java b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServices.java index 96cd4d8067..55a04f5faa 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServices.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/employee_hours/EmployeeHoursServices.java @@ -9,8 +9,6 @@ public interface EmployeeHoursServices { EmployeeHoursResponseDTO save(CompletedFileUpload file); - EmployeeHours read(UUID id); - Set findByFields(String employeeId); } 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 18326b9813..24758c90fa 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 @@ -40,7 +40,6 @@ public EmployeeHoursResponseDTO save(CompletedFileUpload file) { List employeeHoursList = new ArrayList<>(); Set employeeHours = new HashSet<>(); EmployeeHoursResponseDTO responseDTO = new EmployeeHoursResponseDTO(); - validate(!isAdmin, NOT_AUTHORIZED_MSG); responseDTO.setRecordCountDeleted(employeehourRepo.count()); employeehourRepo.deleteAll(); try { @@ -58,11 +57,6 @@ public EmployeeHoursResponseDTO save(CompletedFileUpload file) { } - @Override - public EmployeeHours read(UUID id) { - return employeehourRepo.findById(id).orElse(null); - } - @Override public Set findByFields(String employeeId) { MemberProfile currentUser = currentUserServices.getCurrentUser(); 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 64750cbba0..948b0ca6db 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,5 +1,6 @@ package com.objectcomputing.checkins.services.kudos; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.configuration.CheckInsConfiguration; import com.objectcomputing.checkins.notifications.email.EmailSender; import com.objectcomputing.checkins.notifications.email.MailJetFactory; @@ -121,10 +122,6 @@ public Kudos save(KudosCreateDTO kudosDTO) { @Override public Kudos approve(Kudos kudos) { - if (!currentUserServices.isAdmin()) { - throw new PermissionException(NOT_AUTHORIZED_MSG); - } - UUID kudosId = kudos.getId(); Kudos existingKudos = kudosRepository.findById(kudosId).orElseThrow(() -> new BadArgException(KUDOS_DOES_NOT_EXIST_MSG.formatted(kudosId))); @@ -151,7 +148,7 @@ public KudosResponseDTO getById(UUID id) { if (kudos.getDateApproved() == null) { // If not yet approved, only admins and the sender can access the kudos - if (!currentUserServices.isAdmin() && !isSender) { + if (!hasAdministerKudosPermission() && !isSender) { throw new PermissionException(NOT_AUTHORIZED_MSG); } } else { @@ -162,7 +159,7 @@ public KudosResponseDTO getById(UUID id) { .stream() .anyMatch(recipient -> recipient.getMemberId().equals(currentUserId)); - if (!currentUserServices.isAdmin() && !isSender && !isRecipient) { + if (!hasAdministerKudosPermission() && !isSender && !isRecipient) { throw new PermissionException(NOT_AUTHORIZED_MSG); } } @@ -173,10 +170,6 @@ public KudosResponseDTO getById(UUID id) { @Override public void delete(UUID id) { - if (!currentUserServices.isAdmin()) { - throw new PermissionException(NOT_AUTHORIZED_MSG); - } - Kudos kudos = kudosRepository.findById(id).orElseThrow(() -> new NotFoundException(KUDOS_DOES_NOT_EXIST_MSG.formatted(id))); @@ -196,7 +189,7 @@ public List findByValues(@Nullable UUID recipientId, @Nullable } else if (senderId != null) { return findAllFromMember(senderId); } else { - if (!currentUserServices.isAdmin()) { + if (!hasAdministerKudosPermission()) { throw new PermissionException(NOT_AUTHORIZED_MSG); } @@ -215,7 +208,7 @@ public List getRecent() { } private List findByPending(boolean isPending) { - if (!currentUserServices.isAdmin()) { + if (!hasAdministerKudosPermission()) { throw new PermissionException(NOT_AUTHORIZED_MSG); } @@ -235,10 +228,10 @@ private List findByPending(boolean isPending) { private List findAllToMember(UUID memberId) { - boolean isAdmin = currentUserServices.isAdmin(); UUID currentUserId = currentUserServices.getCurrentUser().getId(); - if (!currentUserId.equals(memberId) && !isAdmin) { + if (!currentUserId.equals(memberId) && + !hasAdministerKudosPermission()) { throw new PermissionException("You are not authorized to retrieve the kudos another user has received"); } @@ -261,10 +254,10 @@ private List findAllToMember(UUID memberId) { private List findAllFromMember(UUID senderId) { - boolean isAdmin = currentUserServices.isAdmin(); UUID currentUserId = currentUserServices.getCurrentUser().getId(); - if (!currentUserId.equals(senderId) && !isAdmin) { + if (!currentUserId.equals(senderId) && + !hasAdministerKudosPermission()) { throw new PermissionException("You are not authorized to retrieve the kudos another user has sent"); } @@ -386,4 +379,8 @@ private void slackApprovedKudos(Kudos kudos) { LOG.error("Unable to POST to Slack: " + httpResponse.reason()); } } + + private boolean hasAdministerKudosPermission() { + return currentUserServices.hasPermission(Permission.CAN_ADMINISTER_KUDOS); + } } 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 1b35173a8f..14ae54588c 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 @@ -111,7 +111,7 @@ public HttpResponse save(@Body @Valid MemberProfileCre */ @Put public HttpResponse update(@Body @Valid MemberProfileUpdateDTO memberProfile) { - MemberProfile savedProfile = memberProfileServices.saveProfile(fromDTO(memberProfile)); + MemberProfile savedProfile = memberProfileServices.updateProfile(fromDTO(memberProfile)); return HttpResponse.ok(fromEntity(savedProfile)) .headers(headers -> headers.location(location(savedProfile.getId()))); } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServices.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServices.java index 5643e30769..83932c5344 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServices.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileServices.java @@ -27,4 +27,6 @@ Set findByValues(String firstName, String lastName, String title, List getSubordinatesForId(UUID id); MemberProfile updateProfile(MemberProfile memberProfile); + + void updateLastSeen(UUID 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 667606949a..1bdb662a14 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,5 +1,6 @@ package com.objectcomputing.checkins.services.memberprofile; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.exceptions.NotFoundException; @@ -23,8 +24,10 @@ import org.slf4j.LoggerFactory; import java.util.*; +import java.time.LocalDate; import static com.objectcomputing.checkins.util.Util.nullSafeUUIDToString; +import static com.objectcomputing.checkins.services.validate.PermissionsValidation.NOT_AUTHORIZED_MSG; @Singleton @CacheConfig("member-cache") @@ -110,27 +113,9 @@ public MemberProfile saveProfile(MemberProfile memberProfile) { emailAssignment(createdMemberProfile, true); // PDL emailAssignment(createdMemberProfile, false); // Supervisor return createdMemberProfile; - } - - Optional existingProfileOpt = memberProfileRepository.findById(memberProfile.getId()); - MemberProfile updatedMemberProfile = memberProfileRepository.update(memberProfile); - if (existingProfileOpt.isEmpty()) { - LOG.error("MemberProfile with id {} not found", memberProfile.getId()); } else { - MemberProfile existingProfile = existingProfileOpt.get(); - - boolean pdlChanged = !Objects.equals(existingProfile.getPdlId(), memberProfile.getPdlId()); - boolean supervisorChanged = !Objects.equals(existingProfile.getSupervisorid(), memberProfile.getSupervisorid()); - - if (pdlChanged) { - emailAssignment(updatedMemberProfile, true); // PDL - } - if (supervisorChanged) { - emailAssignment(updatedMemberProfile, false); // Supervisor - } + throw new BadArgException("New member created with an id"); } - - return updatedMemberProfile; } public void emailAssignment(MemberProfile member, boolean isPDL) { @@ -165,9 +150,6 @@ public void emailAssignment(MemberProfile member, boolean isPDL) { @Override @CacheInvalidate(cacheNames = {"member-cache"}) public boolean deleteProfile(@NotNull UUID id) { - if (!currentUserServices.isAdmin()) { - throw new PermissionException("Requires admin privileges"); - } MemberProfile memberProfile = memberProfileRepository.findById(id).orElse(null); Set userRoles = (memberProfile != null) ? roleServices.findUserRoles(memberProfile.getId()) : Collections.emptySet(); @@ -180,7 +162,7 @@ public boolean deleteProfile(@NotNull UUID id) { } else if (!teamMemberServices.findByFields(null, id, null).isEmpty()) { throw new BadArgException(String.format("User %s cannot be deleted since TeamMember record(s) exist", MemberProfileUtils.getFullName(memberProfile))); } else if (!userRoles.isEmpty()) { - throw new BadArgException(String.format("User %s cannot be deleted since user has PDL role", MemberProfileUtils.getFullName(memberProfile))); + throw new BadArgException(String.format("User %s cannot be deleted since user has one or more roles", MemberProfileUtils.getFullName(memberProfile))); } // Update PDL ID for all associated members before termination @@ -237,6 +219,52 @@ public List getSubordinatesForId(UUID id) { @Override @CacheInvalidate(cacheNames = {"member-cache"}) public MemberProfile updateProfile(MemberProfile memberProfile) { - return memberProfileRepository.update(memberProfile); + if (memberProfile.getId() == null) { + throw new BadArgException("Null profile id in update"); + } + + MemberProfile currentUser = currentUserServices.getCurrentUser(); + if (!currentUserServices.hasPermission(Permission.CAN_EDIT_ALL_ORGANIZATION_MEMBERS) && + (currentUser == null || !currentUser.getId().equals(memberProfile.getId()))) { + throw new PermissionException(NOT_AUTHORIZED_MSG); + } + + MemberProfile emailProfile = memberProfileRepository.findByWorkEmail(memberProfile.getWorkEmail()).orElse(null); + + if (emailProfile != null && emailProfile.getId() != null && !Objects.equals(memberProfile.getId(), emailProfile.getId())) { + throw new AlreadyExistsException(String.format("Email %s already exists in database", + memberProfile.getWorkEmail())); + } + + Optional existingProfileOpt = memberProfileRepository.findById(memberProfile.getId()); + MemberProfile updatedMemberProfile = memberProfileRepository.update(memberProfile); + if (existingProfileOpt.isEmpty()) { + LOG.error("MemberProfile with id {} not found", memberProfile.getId()); + } else { + MemberProfile existingProfile = existingProfileOpt.get(); + + boolean pdlChanged = !Objects.equals(existingProfile.getPdlId(), memberProfile.getPdlId()); + boolean supervisorChanged = !Objects.equals(existingProfile.getSupervisorid(), memberProfile.getSupervisorid()); + + if (pdlChanged) { + emailAssignment(updatedMemberProfile, true); // PDL + } + if (supervisorChanged) { + emailAssignment(updatedMemberProfile, false); // Supervisor + } + } + + return updatedMemberProfile; + } + + @Override + @CacheInvalidate(cacheNames = {"member-cache"}) + public void updateLastSeen(UUID id) { + Optional profile = memberProfileRepository.findById(id); + if (profile.isPresent()) { + MemberProfile memberProfile = profile.get(); + memberProfile.setLastSeen(LocalDate.now()); + memberProfileRepository.update(memberProfile); + } } } diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserController.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserController.java index abd2207559..68d185b64f 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserController.java @@ -19,7 +19,6 @@ import io.swagger.v3.oas.annotations.tags.Tag; import java.net.URI; -import java.time.LocalDate; import java.util.List; import java.util.Set; import java.util.UUID; @@ -63,8 +62,7 @@ public HttpResponse currentUser(@Nullable Authentication authent MemberProfile user = currentUserServices.findOrSaveUser(firstName, lastName, workEmail); - user.setLastSeen(LocalDate.now()); - memberProfileServices.updateProfile(user); + memberProfileServices.updateLastSeen(user.getId()); List permissions = rolePermissionServices.findUserPermissions(user.getId()); Set roles = roleServices.findUserRoles(user.getId()); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServices.java b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServices.java index 98a3d154cb..3cff594d84 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServices.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/memberprofile/currentuser/CurrentUserServices.java @@ -1,6 +1,7 @@ package com.objectcomputing.checkins.services.memberprofile.currentuser; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.role.RoleType; public interface CurrentUserServices { @@ -9,6 +10,8 @@ public interface CurrentUserServices { boolean hasRole(RoleType role); + boolean hasPermission(Permission permission); + boolean isAdmin(); MemberProfile getCurrentUser(); 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 ae063117c1..600a419158 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 @@ -2,12 +2,14 @@ import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.exceptions.NotFoundException; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository; import com.objectcomputing.checkins.services.role.Role; import com.objectcomputing.checkins.services.role.RoleServices; import com.objectcomputing.checkins.services.role.RoleType; import com.objectcomputing.checkins.services.role.member_roles.MemberRoleServices; +import com.objectcomputing.checkins.services.role.role_permissions.RolePermissionServices; import io.micronaut.security.authentication.Authentication; import io.micronaut.security.utils.SecurityService; import jakarta.inject.Singleton; @@ -15,6 +17,7 @@ import java.time.LocalDate; import java.util.Optional; +import java.util.List; @Singleton public class CurrentUserServicesImpl implements CurrentUserServices { @@ -23,14 +26,18 @@ public class CurrentUserServicesImpl implements CurrentUserServices { private final SecurityService securityService; private final RoleServices roleServices; private final MemberRoleServices memberRoleServices; + private final RolePermissionServices rolePermissionServices; public CurrentUserServicesImpl(MemberProfileRepository memberProfileRepository, RoleServices roleServices, - SecurityService securityService, MemberRoleServices memberRoleServices) { + SecurityService securityService, + MemberRoleServices memberRoleServices, + RolePermissionServices rolePermissionServices) { this.memberProfileRepo = memberProfileRepository; this.roleServices = roleServices; this.securityService = securityService; this.memberRoleServices = memberRoleServices; + this.rolePermissionServices = rolePermissionServices; } @Override @@ -48,6 +55,14 @@ public boolean hasRole(RoleType role) { return securityService.hasRole(role.toString()); } + @Override + public boolean hasPermission(Permission permission) { + List userPermissions = + rolePermissionServices.findUserPermissions(getCurrentUser().getId()); + return userPermissions.stream().map(Permission::name) + .anyMatch(str -> str.equals(permission.name())); + } + @Override public boolean isAdmin() { return hasRole(RoleType.ADMIN); 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 29e8add4c8..66dd7a365a 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 @@ -14,11 +14,13 @@ public enum Permission { CAN_ADMINISTER_KUDOS("Administer kudos", "Feedback"), CAN_VIEW_FEEDBACK_ANSWER("View 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"), CAN_CREATE_ORGANIZATION_MEMBERS("Create organization members", "User Management"), CAN_IMPERSONATE_MEMBERS("Impersonate organization members", "Security"), CAN_VIEW_ROLE_PERMISSIONS("View role permissions", "Security"), CAN_ASSIGN_ROLE_PERMISSIONS("Assign role permissions", "Security"), + CAN_EDIT_MEMBER_ROLES("Edit member roles", "Security"), CAN_VIEW_PERMISSIONS("View all permissions", "Security"), CAN_VIEW_SKILLS_REPORT("View skills report", "Reporting"), CAN_VIEW_RETENTION_REPORT("View retention report", "Reporting"), @@ -27,6 +29,7 @@ public enum Permission { CAN_VIEW_PROFILE_REPORT("View profile report", "Reporting"), CAN_VIEW_CHECKINS_REPORT("View checkins report", "Reporting"), CAN_CREATE_MERIT_REPORT("Create Merit Reports", "Reporting"), + CAN_UPLOAD_HOURS("Upload 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"), @@ -41,6 +44,7 @@ public enum Permission { CAN_VIEW_ALL_CHECKINS("View all check-ins", "Check-ins"), CAN_UPDATE_ALL_CHECKINS("Update all check-ins, including completed check-ins", "Check-ins"), CAN_EDIT_SKILL_CATEGORIES("Edit skill categories", "Skill Categories"), + CAN_EDIT_SKILLS("Edit skills", "Skills"), CAN_CREATE_REVIEW_ASSIGNMENTS("Create review assignments", "Reviews"), CAN_VIEW_REVIEW_ASSIGNMENTS("View review assignments", "Reviews"), CAN_UPDATE_REVIEW_ASSIGNMENTS("Update review assignments", "Reviews"), diff --git a/server/src/main/java/com/objectcomputing/checkins/services/role/member_roles/MemberRoleController.java b/server/src/main/java/com/objectcomputing/checkins/services/role/member_roles/MemberRoleController.java index 6e2b5942ec..782919c025 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/role/member_roles/MemberRoleController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/role/member_roles/MemberRoleController.java @@ -1,5 +1,8 @@ package com.objectcomputing.checkins.services.role.member_roles; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; + import io.micronaut.http.HttpResponse; import io.micronaut.http.annotation.*; import io.micronaut.scheduling.TaskExecutors; @@ -29,6 +32,7 @@ HttpResponse> getAllAssignedMemberRoles() { } @Delete("/{roleId}/{memberId}") + @RequiredPermission(Permission.CAN_EDIT_MEMBER_ROLES) HttpResponse deleteMemberRole(@NotNull UUID roleId, @NotNull UUID memberId){ memberRoleServices.delete(new MemberRoleId(memberId, roleId)); return HttpResponse.ok(); @@ -36,6 +40,7 @@ HttpResponse deleteMemberRole(@NotNull UUID roleId, @NotNull UUID memberId){ @Post @CacheInvalidate(cacheNames = {"role-permission-cache"}) + @RequiredPermission(Permission.CAN_EDIT_MEMBER_ROLES) HttpResponse saveMemberRole(@NotNull @Body MemberRoleId id){ MemberRole memberRole = memberRoleServices.saveByIds(id.getMemberId(), id.getRoleId()); return HttpResponse.ok(memberRole); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionServicesImpl.java index dbc8a2c0a0..91dc348fac 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionServicesImpl.java @@ -1,9 +1,11 @@ package com.objectcomputing.checkins.services.role.role_permissions; +import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.permissions.PermissionDTO; import com.objectcomputing.checkins.services.permissions.PermissionServices; import com.objectcomputing.checkins.services.role.Role; +import com.objectcomputing.checkins.services.role.RoleType; import com.objectcomputing.checkins.services.role.RoleServices; import io.micronaut.cache.annotation.CacheConfig; import io.micronaut.cache.annotation.CacheInvalidate; @@ -70,6 +72,13 @@ public RolePermission save(UUID roleId, Permission permissionId) { @CacheInvalidate(all = true) @Override public void delete(UUID roleId, Permission permissionId) { + // Disallow removal of Assign Role Permissions from ADMIN + if (permissionId == Permission.CAN_ASSIGN_ROLE_PERMISSIONS) { + Role role = roleServices.read(roleId); + if (role.getRole().equals(RoleType.Constants.ADMIN_ROLE)) { + throw new BadArgException("Cannot remove Assign Role Permissions from ADMIN"); + } + } rolePermissionRepository.deleteByIds(roleId.toString(), permissionId); } 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 a87bb89278..9bf351cde8 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,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.NotFoundException; import io.micronaut.core.annotation.Nullable; import io.micronaut.http.HttpRequest; @@ -84,6 +86,7 @@ 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) @@ -97,7 +100,8 @@ 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); } -} \ No newline at end of file +} 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 d05c7ce29c..0a47dda442 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 @@ -2,7 +2,6 @@ import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.exceptions.BadArgException; -import com.objectcomputing.checkins.exceptions.PermissionException; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; import jakarta.inject.Singleton; import jakarta.validation.constraints.NotNull; @@ -24,6 +23,7 @@ public SkillServicesImpl(SkillRepository skillRepository, this.currentUserServices = currentUserServices; } + @Override public Skill save(Skill skill) { Skill newSkill = null; if (skill != null) { @@ -41,10 +41,12 @@ public Skill save(Skill skill) { return newSkill; } + @Override public Skill readSkill(@NotNull UUID id) { return skillRepository.findById(id).orElse(null); } + @Override public Set findByValue(String name, Boolean pending) { Set skillList = new HashSet<>(); @@ -62,10 +64,8 @@ public Set findByValue(String name, Boolean pending) { return skillList; } + @Override public void delete(@NotNull UUID id) { - if (!currentUserServices.isAdmin()) { - throw new PermissionException("You do not have permission to access this resource"); - } skillRepository.deleteById(id); } @@ -74,11 +74,8 @@ protected List findByNameLike(String name) { return skillRepository.findByNameIlike(wildcard); } + @Override public Skill update(@NotNull Skill skill) { - if (!currentUserServices.isAdmin()) { - throw new PermissionException("You do not have permission to access this resource"); - } - if (skill.getId() != null && skillRepository.findById(skill.getId()).isPresent()) { return skillRepository.update(skill); } else { 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 8c0ada2f68..95722469b3 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 @@ -633,6 +633,16 @@ VALUES ('fa2319a4-8db6-4b83-bba7-70a3e4d7670f', '2024-05-21', '1b4f99da-ef70-4a76-9b37-8bb783b749ad', PGP_SYM_ENCRYPT('internal #9','${aeskey}'), PGP_SYM_ENCRYPT('extenal #9','${aeskey}'), 4, 5); -- Admin Permissions +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_EDIT_MEMBER_ROLES'); + +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS'); + insert into role_permissions (roleid, permission) values @@ -728,6 +738,11 @@ insert into role_permissions values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_EDIT_SKILL_CATEGORIES'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_EDIT_SKILLS'); + insert into role_permissions (roleid, permission) values @@ -888,6 +903,11 @@ insert into role_permissions values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_CREATE_MERIT_REPORT'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_UPLOAD_HOURS'); + 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 60723b8d5e..99b958725b 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/CurrentUserServicesReplacement.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/CurrentUserServicesReplacement.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services; +import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; import com.objectcomputing.checkins.services.role.RoleType; @@ -18,6 +19,7 @@ public class CurrentUserServicesReplacement implements CurrentUserServices { public MemberProfile currentUser; public List roles; + public List permissions; @Override public MemberProfile findOrSaveUser(String firstName, @@ -31,6 +33,11 @@ public boolean hasRole(RoleType role) { return roles == null ? false : roles.contains(role); } + @Override + public boolean hasPermission(Permission permission) { + return permissions == null ? false : permissions.contains(permission); + } + @Override public boolean isAdmin() { return hasRole(RoleType.ADMIN); 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 f6858dab40..8913ed74ed 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 @@ -25,6 +25,7 @@ import java.util.Objects; import java.util.Set; import java.util.UUID; +import java.util.Collections; import static com.objectcomputing.checkins.services.role.RoleType.Constants.ADMIN_ROLE; import static com.objectcomputing.checkins.services.role.RoleType.Constants.MEMBER_ROLE; @@ -92,7 +93,7 @@ void testCreateEmployeeHoursWithNoAdminRole() { HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); - assertEquals(HttpStatus.BAD_REQUEST,responseException.getStatus()); + assertEquals(HttpStatus.FORBIDDEN,responseException.getStatus()); } @Test @@ -142,20 +143,13 @@ void testFindAllRecordsWithNonAdminRole() throws IOException { } @Test - void testGetByIdNotFound() { - - final HttpRequest request = HttpRequest. - GET(String.format("/%s", UUID.randomUUID().toString())).basicAuth(ADMIN_ROLE,ADMIN_ROLE); - - HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, - () -> client.toBlocking().exchange(request, Map.class)); + void testFindEmployeeHoursNotFound() { - assertNotNull(responseException.getResponse()); - assertEquals(HttpStatus.NOT_FOUND,responseException.getStatus()); + final HttpRequest request = HttpRequest.GET("/?employeeId=invalid_id").basicAuth(ADMIN_ROLE,ADMIN_ROLE); + final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(EmployeeHours.class)); + Set emptySet = Collections.emptySet(); + assertEquals(emptySet,response.body()); + assertEquals(HttpStatus.OK,response.getStatus()); } - - - - } 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 cae3a27202..91859c2c8c 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 @@ -47,10 +47,12 @@ public interface PermissionFixture extends RolePermissionFixture { // Add ADMIN Permissions here List adminPermissions = List.of( + Permission.CAN_EDIT_MEMBER_ROLES, Permission.CAN_VIEW_FEEDBACK_REQUEST, Permission.CAN_CREATE_FEEDBACK_REQUEST, Permission.CAN_DELETE_FEEDBACK_REQUEST, Permission.CAN_VIEW_FEEDBACK_ANSWER, + Permission.CAN_EDIT_ALL_ORGANIZATION_MEMBERS, Permission.CAN_DELETE_ORGANIZATION_MEMBERS, Permission.CAN_CREATE_ORGANIZATION_MEMBERS, Permission.CAN_VIEW_ROLE_PERMISSIONS, @@ -66,6 +68,7 @@ public interface PermissionFixture extends RolePermissionFixture { Permission.CAN_ASSIGN_ROLE_PERMISSIONS, Permission.CAN_VIEW_SKILL_CATEGORIES, Permission.CAN_EDIT_SKILL_CATEGORIES, + Permission.CAN_EDIT_SKILLS, Permission.CAN_VIEW_PRIVATE_NOTE, Permission.CAN_CREATE_PRIVATE_NOTE, Permission.CAN_UPDATE_PRIVATE_NOTE, @@ -98,7 +101,8 @@ public interface PermissionFixture extends RolePermissionFixture { Permission.CAN_CREATE_KUDOS, Permission.CAN_IMPERSONATE_MEMBERS, Permission.CAN_SEND_EMAIL, - Permission.CAN_CREATE_MERIT_REPORT + Permission.CAN_CREATE_MERIT_REPORT, + Permission.CAN_UPLOAD_HOURS ); default void setPermissionsForAdmin(UUID roleID) { diff --git a/server/src/test/java/com/objectcomputing/checkins/services/member_role/MemberRoleControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/member_role/MemberRoleControllerTest.java index 12418ec7b2..b24865e06d 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/member_role/MemberRoleControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/member_role/MemberRoleControllerTest.java @@ -14,6 +14,7 @@ import io.micronaut.http.HttpStatus; import io.micronaut.http.client.HttpClient; import io.micronaut.http.client.annotation.Client; +import io.micronaut.http.client.exceptions.HttpClientResponseException; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; @@ -69,6 +70,18 @@ void testDeleteMemberRole() { assertEquals(HttpStatus.OK, response.getStatus()); } + @Test + void testDeleteMemberRoleNoPermission() { + MemberProfile member = createADefaultMemberProfile(); + + Role memberRole = createAndAssignRole(RoleType.MEMBER, member); + + final HttpRequest request = HttpRequest.DELETE(String.format("/%s/%s", memberRole.getId(), member.getId())) + .basicAuth(member.getWorkEmail(), RoleType.Constants.MEMBER_ROLE); + HttpClientResponseException e = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, MemberRole.class)); + assertEquals(HttpStatus.FORBIDDEN, e.getStatus()); + } + @Test void testSaveMemberRole() { MemberProfile member = createADefaultMemberProfile(); @@ -89,4 +102,17 @@ void testSaveMemberRole() { assertEquals(adminRole.getId(), response.getBody().get().getMemberRoleId().getRoleId()); } + @Test + void testSaveMemberRoleNoPermission() { + MemberProfile member = createADefaultMemberProfile(); + Role memberRole = createAndAssignRole(RoleType.MEMBER, member); + + MemberRoleId memberRoleId = new MemberRoleId(member.getId(), memberRole.getId()); + + final HttpRequest request = HttpRequest.POST("", memberRoleId) + .basicAuth(member.getWorkEmail(), RoleType.Constants.MEMBER_ROLE); + + HttpClientResponseException e = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, MemberRole.class)); + assertEquals(HttpStatus.FORBIDDEN, e.getStatus()); + } } diff --git a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileControllerTest.java index 8b2facb550..df39d8e551 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileControllerTest.java @@ -6,6 +6,7 @@ import com.objectcomputing.checkins.services.role.RoleType; import com.objectcomputing.checkins.services.skills.Skill; import com.objectcomputing.checkins.services.team.Team; +import com.objectcomputing.checkins.util.Util; import io.micronaut.core.type.Argument; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; @@ -35,21 +36,25 @@ class MemberProfileControllerTest extends TestContainersSuite implements MemberP @Client("/services/member-profiles") private HttpClient client; - //TODO: Use Util.MAX instead of defining variable - /* - * LocalDate.Max cannot be used for end-to-end tests - * LocalDate.Max year = 999999999 - * POSTGRES supported date range = 4713 BC - 5874897 AD - */ - private final LocalDate maxDate = LocalDate.of(2099, 12, 31); + private final LocalDate maxDate = Util.MAX.toLocalDate(); private String encodeValue(String value) { return URLEncoder.encode(value, StandardCharsets.UTF_8); } + private MemberProfile member; + private MemberProfile pdl; + private MemberProfile admin; + @BeforeEach - void createRolesAndPermissions() { + void makeRoles() { createAndAssignRoles(); + pdl = createADefaultMemberProfile(); + member = createADefaultMemberProfileForPdl(pdl); + admin = createASecondDefaultMemberProfile(); + + assignAdminRole(admin); + assignPdlRole(pdl); } @Test @@ -67,12 +72,8 @@ void testGETNonExistingEndpointReturns404() { @Test void testDeleteThrowsExceptionWhenUserDoesNotExist() { - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - assignAdminRole(memberProfileOfAdmin); -// createAndAssignAdminRole(memberProfileOfAdmin); - final HttpRequest request = HttpRequest.DELETE("/01b7d769-9fa2-43ff-95c7-f3b950a27bf9") - .basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -89,22 +90,17 @@ void testDeleteThrowsExceptionWhenUserDoesNotExist() { @Test void testDeleteThrowsExceptionIfCheckinDataExists() { - MemberProfile memberProfileOfPDL = createADefaultMemberProfile(); - MemberProfile memberProfileOfMember = createADefaultMemberProfileForPdl(memberProfileOfPDL); - createADefaultCheckIn(memberProfileOfMember, memberProfileOfPDL); + createADefaultCheckIn(member, pdl); - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - assignAdminRole(memberProfileOfAdmin); - - final HttpRequest request = HttpRequest.DELETE(memberProfileOfMember.getId().toString()) - .basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + final HttpRequest request = HttpRequest.DELETE(member.getId().toString()) + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request)); assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus()); assertEquals( - String.format("User %s cannot be deleted since Checkin record(s) exist", MemberProfileUtils.getFullName(memberProfileOfMember)), + String.format("User %s cannot be deleted since Checkin record(s) exist", MemberProfileUtils.getFullName(member)), responseException.getMessage() ); } @@ -112,25 +108,18 @@ void testDeleteThrowsExceptionIfCheckinDataExists() { @Test void testDeleteThrowsExceptionIfMemberSkillExists() { - MemberProfile memberProfileOfPDL = createADefaultMemberProfile(); - MemberProfile memberProfileOfMember = createADefaultMemberProfileForPdl(memberProfileOfPDL); - Skill testSkill = createADefaultSkill(); - createMemberSkill(memberProfileOfMember, testSkill, "Pro", LocalDate.now()); + createMemberSkill(member, testSkill, "Pro", LocalDate.now()); - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - assignAdminRole(memberProfileOfAdmin); -// createAndAssignAdminRole(memberProfileOfAdmin); - - final HttpRequest request = HttpRequest.DELETE(memberProfileOfMember.getId().toString()) - .basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + final HttpRequest request = HttpRequest.DELETE(member.getId().toString()) + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request)); assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus()); assertEquals( - String.format("User %s cannot be deleted since MemberSkill record(s) exist", MemberProfileUtils.getFullName(memberProfileOfMember)), + String.format("User %s cannot be deleted since MemberSkill record(s) exist", MemberProfileUtils.getFullName(member)), responseException.getMessage() ); } @@ -138,25 +127,18 @@ void testDeleteThrowsExceptionIfMemberSkillExists() { @Test void testDeleteThrowsExceptionIfMemberIsPartOfATeam() { - MemberProfile memberProfileOfPDL = createADefaultMemberProfile(); - MemberProfile memberProfileOfMember = createADefaultMemberProfileForPdl(memberProfileOfPDL); - Team testTeam = createDefaultTeam(); - createDefaultTeamMember(testTeam, memberProfileOfMember); + createDefaultTeamMember(testTeam, member); - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - assignAdminRole(memberProfileOfAdmin); -// createAndAssignAdminRole(memberProfileOfAdmin); - - final HttpRequest request = HttpRequest.DELETE(memberProfileOfMember.getId().toString()) - .basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + final HttpRequest request = HttpRequest.DELETE(member.getId().toString()) + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request)); assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus()); assertEquals( - String.format("User %s cannot be deleted since TeamMember record(s) exist", MemberProfileUtils.getFullName(memberProfileOfMember)), + String.format("User %s cannot be deleted since TeamMember record(s) exist", MemberProfileUtils.getFullName(member)), responseException.getMessage() ); } @@ -164,23 +146,15 @@ void testDeleteThrowsExceptionIfMemberIsPartOfATeam() { @Test void testDeleteThrowsExceptionIfMemberHasPDLRole() { - MemberProfile memberProfileOfPDL = createADefaultMemberProfile(); - assignPdlRole(memberProfileOfPDL); -// createAndAssignRole(RoleType.PDL, memberProfileOfPDL); - - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - assignAdminRole(memberProfileOfAdmin); -// createAndAssignAdminRole(memberProfileOfAdmin); - - final HttpRequest request = HttpRequest.DELETE(memberProfileOfPDL.getId().toString()) - .basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + final HttpRequest request = HttpRequest.DELETE(pdl.getId().toString()) + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request)); assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus()); assertEquals( - String.format("User %s cannot be deleted since user has PDL role", MemberProfileUtils.getFullName(memberProfileOfPDL)), + String.format("User %s cannot be deleted since user has one or more roles", MemberProfileUtils.getFullName(pdl)), responseException.getMessage() ); } @@ -188,21 +162,14 @@ void testDeleteThrowsExceptionIfMemberHasPDLRole() { @Test void testDeleteHappyPath() { - MemberProfile memberProfileOfPDL = createADefaultMemberProfile(); - MemberProfile memberProfileOfMember = createADefaultMemberProfileForPdl(memberProfileOfPDL); - - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - assignAdminRole(memberProfileOfAdmin); -// createAndAssignAdminRole(memberProfileOfAdmin); - - final HttpRequest request = HttpRequest.DELETE(memberProfileOfMember.getId().toString()) - .basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + final HttpRequest request = HttpRequest.DELETE(member.getId().toString()) + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); final HttpResponse response = client.toBlocking().exchange(request); assertEquals(HttpStatus.OK, response.getStatus()); // Ensure profile is Deleted and not Terminated - final HttpRequest requestForAssertingDeletion = HttpRequest.GET(String.format("/%s", memberProfileOfMember.getId())) + final HttpRequest requestForAssertingDeletion = HttpRequest.GET(String.format("/%s", member.getId())) .basicAuth(MEMBER_ROLE, MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, @@ -214,7 +181,7 @@ void testDeleteHappyPath() { assertEquals(request.getPath(), href); assertEquals(HttpStatus.NOT_FOUND, responseException.getStatus()); - assertEquals("No member profile for id " + memberProfileOfMember.getId(), error); + assertEquals("No member profile for id " + member.getId(), error); } @Test @@ -255,27 +222,24 @@ void testGETFindByNameReturnsEmptyBody() throws UnsupportedEncodingException { @Test void testGETFindByValueName() throws UnsupportedEncodingException { - MemberProfile memberProfile = createADefaultMemberProfile(); final HttpRequest request = HttpRequest. - GET(String.format("/?firstName=%s", encodeValue(memberProfile.getFirstName()))).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/?firstName=%s", encodeValue(member.getFirstName()))).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(MemberProfile.class)); - assertEquals(Set.of(memberProfile), response.body()); + assertEquals(Set.of(member), response.body()); assertEquals(HttpStatus.OK, response.getStatus()); } @Test void testGETGetByIdHappyPath() { - MemberProfile memberProfile = createADefaultMemberProfile(); - final HttpRequest request = HttpRequest. - GET(String.format("/%s", memberProfile.getId())).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/%s", member.getId())).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse response = client.toBlocking().exchange(request, MemberProfile.class); - assertEquals(memberProfile, response.body()); + assertEquals(member, response.body()); assertEquals(HttpStatus.OK, response.getStatus()); } @@ -294,58 +258,45 @@ void testGETGetByIdNotFound() { @Test void testFindByMemberName() throws UnsupportedEncodingException { - MemberProfile memberProfile = createADefaultMemberProfile(); - - final HttpRequest request = HttpRequest.GET(String.format("/?firstName=%s", encodeValue(memberProfile.getFirstName()))) - .basicAuth(MEMBER_ROLE, MEMBER_ROLE); + final HttpRequest request = HttpRequest.GET(String.format("/?firstName=%s", encodeValue(member.getFirstName()))) + .basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(MemberProfile.class)); - assertEquals(Set.of(memberProfile), response.body()); + assertEquals(Set.of(member), response.body()); assertEquals(HttpStatus.OK, response.getStatus()); } @Test void testFindByMemberRole() throws UnsupportedEncodingException { - MemberProfile memberProfile = createADefaultMemberProfile(); - - final HttpRequest request = HttpRequest.GET(String.format("/?title=%s", encodeValue(memberProfile.getTitle()))).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + final HttpRequest request = HttpRequest.GET(String.format("/?title=%s", encodeValue(member.getTitle()))).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(MemberProfile.class)); - assertEquals(Set.of(memberProfile), response.body()); + assertEquals(Set.of(member), response.body()); assertEquals(HttpStatus.OK, response.getStatus()); } @Test void testFindByMemberPdlId() { - MemberProfile memberProfile = createADefaultMemberProfile(); - MemberProfile memberProfilePdl = createADefaultMemberProfileForPdl(memberProfile); - - final HttpRequest request = HttpRequest.GET(String.format("/?pdlId=%s", memberProfilePdl.getPdlId())).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + final HttpRequest request = HttpRequest.GET(String.format("/?pdlId=%s", member.getPdlId())).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(MemberProfile.class)); - assertEquals(Set.of(memberProfilePdl), response.body()); + assertEquals(Set.of(member), response.body()); assertEquals(HttpStatus.OK, response.getStatus()); } @Test void testFindBySupervisorId() { - MemberProfile memberProfileOfPdl = createADefaultMemberProfile(); - MemberProfile memberProfileOfUser = createADefaultMemberProfileForPdl(memberProfileOfPdl); - - final HttpRequest request = HttpRequest.GET(String.format("/?supervisorId=%s", memberProfileOfUser.getSupervisorid())) - .basicAuth(memberProfileOfUser.getWorkEmail(), MEMBER_ROLE); + final HttpRequest request = HttpRequest.GET(String.format("/?supervisorId=%s", member.getSupervisorid())) + .basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(MemberProfile.class)); - assertEquals(Set.of(memberProfileOfUser), response.body()); + assertEquals(Set.of(member), response.body()); assertEquals(HttpStatus.OK, response.getStatus()); } @Test void testPOSTCreateAMemberProfileAdmin() { - MemberProfile admin = createADefaultMemberProfile(); - assignAdminRole(admin); - MemberProfileUpdateDTO dto = mkUpdateMemberProfileDTO(); final HttpRequest request = HttpRequest. @@ -362,9 +313,6 @@ void testPOSTCreateAMemberProfileAdmin() { @Test void testPOSTCreateAMemberProfileMemberUnauthorized() { - MemberProfile member = createADefaultMemberProfile(); - assignMemberRole(member); - MemberProfileUpdateDTO dto = mkUpdateMemberProfileDTO(); final HttpRequest request = HttpRequest. @@ -378,7 +326,7 @@ void testPOSTCreateAMemberProfileMemberUnauthorized() { @Test void testPOSTCreateAMemberProfilePdlUnauthorized() { - MemberProfile pdl = createADefaultMemberProfile(); + MemberProfile pdl = createAThirdDefaultMemberProfile(); assignPdlRole(pdl); MemberProfileUpdateDTO dto = mkUpdateMemberProfileDTO(); @@ -398,9 +346,6 @@ public void assertUnauthorized(HttpClientResponseException exception) { @Test void testPOSTCreateANullMemberProfile() { - MemberProfile admin = createADefaultMemberProfile(); - assignAdminRole(admin); - MemberProfileCreateDTO memberProfileCreateDTO = new MemberProfileCreateDTO(); final HttpRequest request = HttpRequest. @@ -415,9 +360,6 @@ void testPOSTCreateANullMemberProfile() { // Find By id - when no user data exists for POST @Test void testPostValidationFailures() { - MemberProfile admin = createADefaultMemberProfile(); - assignAdminRole(admin); - final HttpRequest request = HttpRequest.POST("", new MemberProfileCreateDTO()) .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException thrown = assertThrows(HttpClientResponseException.class, @@ -432,9 +374,6 @@ void testPostValidationFailures() { @Test void testPOSTSaveMemberSameEmailThrowsException() { - MemberProfile admin = createADefaultMemberProfile(); - assignAdminRole(admin); - // create a user MemberProfileUpdateDTO firstUser = mkUpdateMemberProfileDTO(); @@ -463,10 +402,13 @@ void testPOSTSaveMemberSameEmailThrowsException() { @Test void testPUTUpdateMemberProfile() { + // Create a DTO and them update it with the "member" information. MemberProfileUpdateDTO profileUpdateDTO = mkUpdateMemberProfileDTO(); + profileUpdateDTO.setId(member.getId()); + profileUpdateDTO.setWorkEmail(member.getWorkEmail()); final HttpRequest request = HttpRequest.PUT("/", profileUpdateDTO) - .basicAuth(MEMBER_ROLE, MEMBER_ROLE); + .basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse response = client.toBlocking().exchange(request, MemberProfileResponseDTO.class); assertTrue(response.getBody().isPresent()); @@ -505,9 +447,6 @@ void testPUTUpdateNullMemberProfile() { // POST - Future Start Date @Test void testPostForFutureStartDate() { - MemberProfile admin = createADefaultMemberProfile(); - assignAdminRole(admin); - MemberProfileCreateDTO requestBody = mkCreateMemberProfileDTO(); requestBody.setStartDate(maxDate); @@ -526,9 +465,6 @@ void testPostForFutureStartDate() { // POST - NotBlank MemberProfile first name (and last name) @Test void testPostWithNullName() { - MemberProfile admin = createADefaultMemberProfile(); - assignAdminRole(admin); - MemberProfileCreateDTO requestBody = mkCreateMemberProfileDTO(); requestBody.setFirstName(null); @@ -560,13 +496,17 @@ void testPutValidationFailures() { @Test void testPutFutureStartDate() { + // Create a DTO and them update it with the "member" information. MemberProfileUpdateDTO requestBody = mkUpdateMemberProfileDTO(); + requestBody.setId(member.getId()); + requestBody.setWorkEmail(member.getWorkEmail()); requestBody.setStartDate(maxDate); final HttpResponse response = client .toBlocking() .exchange(HttpRequest.PUT("", requestBody) - .basicAuth(MEMBER_ROLE, MEMBER_ROLE), MemberProfileResponseDTO.class); + .basicAuth(member.getWorkEmail(), MEMBER_ROLE), + MemberProfileResponseDTO.class); assertEquals(HttpStatus.OK, response.getStatus()); assertProfilesEqual(requestBody, Objects.requireNonNull(response.body())); @@ -587,16 +527,14 @@ void testPutUpdateForEmptyInput() { @Test void testMemberProfileWithNullTerminationDate() { - MemberProfile memberProfile = createADefaultMemberProfile(); final HttpResponse> response = client .toBlocking() - .exchange(HttpRequest.GET("").basicAuth(memberProfile.getWorkEmail(), MEMBER_ROLE), Argument.listOf(MemberProfileResponseDTO.class)); + .exchange(HttpRequest.GET("").basicAuth(member.getWorkEmail(), MEMBER_ROLE), Argument.listOf(MemberProfileResponseDTO.class)); assertEquals(HttpStatus.OK, response.getStatus()); assertTrue(response.getBody().isPresent()); var body = response.getBody().get(); - assertEquals(1, body.size()); - assertProfilesEqual(memberProfile, Objects.requireNonNull(body.get(0))); + assertEquals(3, body.size()); } @Test @@ -610,8 +548,7 @@ void testMemberProfileWithUpcomingTerminationDate() { assertEquals(HttpStatus.OK, response.getStatus()); assertTrue(response.getBody().isPresent()); var body = response.getBody().get(); - assertEquals(1, body.size()); - assertProfilesEqual(memberProfile, Objects.requireNonNull(body.get(0))); + assertEquals(4, body.size()); } @Test @@ -625,7 +562,9 @@ void testMemberProfileWithPreviousTerminationDate() { assertEquals(HttpStatus.OK, response.getStatus()); assertTrue(response.getBody().isPresent()); var body = response.getBody().get(); - assertEquals(0, body.size()); + // There are 4 total members, but we should only see 3 since one was + // terminated prior to today. + assertEquals(3, body.size()); } @Test 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 cc43ab26a8..93e96a9ae6 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 @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.memberprofile; +import com.objectcomputing.checkins.services.CurrentUserServicesReplacement; import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; import com.objectcomputing.checkins.exceptions.AlreadyExistsException; import com.objectcomputing.checkins.notifications.email.MailJetFactory; @@ -32,6 +33,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; @Property(name = "replace.mailjet.factory", value = StringUtils.TRUE) +@Property(name = "replace.currentuserservices", value = StringUtils.TRUE) class MemberProfileTest extends TestContainersSuite implements MemberProfileFixture { @@ -42,6 +44,9 @@ class MemberProfileTest extends TestContainersSuite @Named(MailJetFactory.HTML_FORMAT) private MailJetFactoryReplacement.MockEmailSender emailSender; + @Inject + CurrentUserServicesReplacement currentUserServices; + @Inject private MemberProfileServicesImpl memberProfileServices; @@ -222,9 +227,12 @@ void testUpdateProfileWithChangedPDL() { MemberProfile pdlProfile = createASecondDefaultMemberProfile(); UUID id = existingProfile.getId(); UUID pdlId = pdlProfile.getId(); + + currentUserServices.currentUser = existingProfile; + MemberProfile updatedProfile = new MemberProfile(id, existingProfile.getFirstName(), null, existingProfile.getLastName(), null, null, pdlId, null, existingProfile.getWorkEmail(), null, null, null, null, null, null, null, null, null); - MemberProfile result = memberProfileServices.saveProfile(updatedProfile); + MemberProfile result = memberProfileServices.updateProfile(updatedProfile); assertEquals(updatedProfile, result); @@ -244,9 +252,12 @@ void testUpdateProfileWithChangedSupervisor() { MemberProfile supervisorProfile = createASecondDefaultMemberProfile(); UUID id = existingProfile.getId(); UUID supervisorId = supervisorProfile.getId(); + + currentUserServices.currentUser = existingProfile; + MemberProfile updatedProfile = new MemberProfile(id, existingProfile.getFirstName(), null, existingProfile.getLastName(), null, null, null, null, existingProfile.getWorkEmail(), null, null, null, supervisorId, null, null, null, null, null); - MemberProfile result = memberProfileServices.saveProfile(updatedProfile); + MemberProfile result = memberProfileServices.updateProfile(updatedProfile); assertEquals(updatedProfile, result); assertEquals(1, emailSender.events.size()); @@ -263,7 +274,9 @@ void testUpdateProfileWithChangedSupervisor() { void testUpdateProfileWithNoChange() { MemberProfile existingProfile = createADefaultMemberProfile(); - MemberProfile result = memberProfileServices.saveProfile(existingProfile); + currentUserServices.currentUser = existingProfile; + + MemberProfile result = memberProfileServices.updateProfile(existingProfile); assertEquals(existingProfile, result); assertEquals(0, emailSender.events.size()); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTestUtil.java b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTestUtil.java index 4d09273808..8a86defce3 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTestUtil.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/memberprofile/MemberProfileTestUtil.java @@ -15,6 +15,7 @@ public static MemberProfileCreateDTO mkCreateMemberProfileDTO() { dto.setLocation("TestLocation"); dto.setWorkEmail("TestEmail"); dto.setEmployeeId("TestEmployeeId"); + dto.setWorkEmail("testemployee@example.com"); dto.setStartDate(LocalDate.of(2019, 1, 01)); dto.setBioText("TestBio"); dto.setLastSeen(LocalDate.now()); @@ -30,6 +31,7 @@ public static MemberProfileUpdateDTO mkUpdateMemberProfileDTO() { dto.setLocation("TestLocation"); dto.setWorkEmail("TestEmail"); dto.setEmployeeId("TestEmployeeId"); + dto.setWorkEmail("testemployee@example.com"); dto.setStartDate(LocalDate.of(2019, 1, 01)); dto.setBioText("TestBio"); dto.setLastSeen(LocalDate.now()); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionsControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionsControllerTest.java index 37cb514634..9818c8b917 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionsControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/role/role_permissions/RolePermissionsControllerTest.java @@ -112,4 +112,19 @@ void testRemovePermissionFromRole() { assertEquals(HttpStatus.OK, response.getStatus()); assertEquals(0, getRolePermissionRepository().findByIds(memberRole.getId().toString(), birthdayPermission).size()); } + + @Test + void testRemoveAssignRolePermissionFromAdminRole() { + MemberProfile sender = createADefaultMemberProfile(); + assignAdminRole(sender); + + Role adminRole = getRoleRepository().findByRole(RoleType.ADMIN.name()).orElseThrow(); + Permission permission = Permission.CAN_ASSIGN_ROLE_PERMISSIONS; + + final HttpRequest request = HttpRequest.DELETE("/", new RolePermissionDTO(adminRole.getId(), permission)) + .basicAuth(sender.getWorkEmail(), RoleType.Constants.ADMIN_ROLE); + HttpClientResponseException e = assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().exchange(request, Map.class)); + assertEquals(HttpStatus.BAD_REQUEST, e.getStatus()); + } } diff --git a/server/src/test/java/com/objectcomputing/checkins/services/skills/SkillControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/skills/SkillControllerTest.java index 226e3c239f..44276ca4ed 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/skills/SkillControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/skills/SkillControllerTest.java @@ -13,6 +13,7 @@ import io.micronaut.http.client.annotation.Client; import io.micronaut.http.client.exceptions.HttpClientResponseException; import jakarta.inject.Inject; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.net.URLEncoder; @@ -31,6 +32,17 @@ class SkillControllerTest extends TestContainersSuite implements SkillFixture, M @Client("/services/skills") private HttpClient client; + private MemberProfile member; + private MemberProfile admin; + + @BeforeEach + void makeRoles() { + createAndAssignRoles(); + member = createADefaultMemberProfile(); + admin = createASecondDefaultMemberProfile(); + assignAdminRole(admin); + } + private String encodeValue(String value) { return URLEncoder.encode(value, StandardCharsets.UTF_8); } @@ -40,7 +52,7 @@ void testGETNonExistingEndpointReturns404() { HttpClientResponseException thrown = assertThrows(HttpClientResponseException.class, () -> { client.toBlocking().exchange(HttpRequest.GET("/12345678-9123-4567-abcd-123456789abc") - .basicAuth(MEMBER_ROLE, MEMBER_ROLE)); + .basicAuth(member.getWorkEmail(), MEMBER_ROLE)); }); assertNotNull(thrown.getResponse()); @@ -51,7 +63,7 @@ void testGETNonExistingEndpointReturns404() { void testGETFindByNameReturnsEmptyBody() { final HttpRequest request = HttpRequest. - GET(String.format("/?name=%s", encodeValue("dnc"))).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/?name=%s", encodeValue("dnc"))).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(Skill.class)); @@ -64,7 +76,7 @@ void testGETFindByValueName() { Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest. - GET(String.format("/?name=%s", encodeValue(skill.getName()))).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/?name=%s", encodeValue(skill.getName()))).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(Skill.class)); @@ -78,7 +90,7 @@ void testGETFindByValuePending() { Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest. - GET(String.format("/?pending=%s", encodeValue(String.valueOf(skill.isPending())))).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/?pending=%s", encodeValue(String.valueOf(skill.isPending())))).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse> response = client.toBlocking().exchange(request, Argument.setOf(Skill.class)); @@ -93,7 +105,7 @@ void testGETGetByIdHappyPath() { Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest. - GET(String.format("/%s", skill.getId())).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/%s", skill.getId())).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse response = client.toBlocking().exchange(request, Skill.class); @@ -106,7 +118,7 @@ void testGETGetByIdHappyPath() { void testGETGetByIdNotFound() { final HttpRequest request = HttpRequest. - GET(String.format("/%s", UUID.randomUUID().toString())).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + GET(String.format("/%s", UUID.randomUUID().toString())).basicAuth(member.getWorkEmail(), MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -126,7 +138,7 @@ void testPOSTCreateASkill() { skillCreateDTO.setDescription("Bring back from the dead"); final HttpRequest request = HttpRequest. - POST("/", skillCreateDTO).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + POST("/", skillCreateDTO).basicAuth(member.getWorkEmail(), MEMBER_ROLE); final HttpResponse response = client.toBlocking().exchange(request, Skill.class); assertNotNull(response); @@ -146,7 +158,7 @@ void testPOSTCreateASkillAlreadyExists() { skillCreateDTO.setPending(skill.isPending()); final HttpRequest request = HttpRequest. - POST("/", skillCreateDTO).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + POST("/", skillCreateDTO).basicAuth(member.getWorkEmail(), MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -164,7 +176,7 @@ void testPOSTCreateASkillAlreadyExistsWhenPending() { skillCreateDTO.setPending(false); final HttpRequest request = HttpRequest. - POST("/", skillCreateDTO).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + POST("/", skillCreateDTO).basicAuth(member.getWorkEmail(), MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -179,7 +191,7 @@ void testPOSTCreateANullSkill() { SkillCreateDTO skillCreateDTO = new SkillCreateDTO(); final HttpRequest request = HttpRequest. - POST("/", skillCreateDTO).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + POST("/", skillCreateDTO).basicAuth(member.getWorkEmail(), MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -191,13 +203,10 @@ void testPOSTCreateANullSkill() { @Test void testUpdateSkillAsAdmin() { - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - createAndAssignAdminRole(memberProfileOfAdmin); - Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest. - PUT("/", skill).basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + PUT("/", skill).basicAuth(admin.getWorkEmail(), ADMIN_ROLE); final HttpResponse response = client.toBlocking().exchange(request, Skill.class); @@ -212,7 +221,7 @@ void testPUTUpdateSkillNonAdmin() { Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest.PUT("/", skill) - .basicAuth(MEMBER_ROLE, MEMBER_ROLE); + .basicAuth(member.getWorkEmail(), MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -223,15 +232,12 @@ void testPUTUpdateSkillNonAdmin() { @Test void testPUTUpdateNonexistentSkill() { - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - createAndAssignAdminRole(memberProfileOfAdmin); - SkillCreateDTO skillCreateDTO = new SkillCreateDTO(); skillCreateDTO.setName("reincarnation"); skillCreateDTO.setPending(true); final HttpRequest request = HttpRequest. - PUT("/", skillCreateDTO).basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + PUT("/", skillCreateDTO).basicAuth(admin.getWorkEmail(), ADMIN_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); @@ -255,13 +261,10 @@ void testPUTUpdateNullSkill() { @Test void deleteSkillAsAdmin() { - MemberProfile memberProfileOfAdmin = createAnUnrelatedUser(); - createAndAssignAdminRole(memberProfileOfAdmin); - Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest. - DELETE(String.format("/%s", skill.getId())).basicAuth(memberProfileOfAdmin.getWorkEmail(), ADMIN_ROLE); + DELETE(String.format("/%s", skill.getId())).basicAuth(admin.getWorkEmail(), ADMIN_ROLE); final HttpResponse response = client.toBlocking().exchange(request, Skill.class); @@ -275,11 +278,11 @@ void deleteSkillNotAsAdmin() { Skill skill = createADefaultSkill(); final HttpRequest request = HttpRequest. - DELETE(String.format("/%s", skill.getId())).basicAuth(MEMBER_ROLE, MEMBER_ROLE); + DELETE(String.format("/%s", skill.getId())).basicAuth(member.getWorkEmail(), MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class)); assertNotNull(responseException.getResponse()); assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus()); } -} \ No newline at end of file +} diff --git a/web-ui/src/components/admin/permissions/Permissions.jsx b/web-ui/src/components/admin/permissions/Permissions.jsx index 7148d85511..f3af9e81a6 100644 --- a/web-ui/src/components/admin/permissions/Permissions.jsx +++ b/web-ui/src/components/admin/permissions/Permissions.jsx @@ -2,9 +2,17 @@ import { useMediaQuery } from '@mui/material'; import React from 'react'; import DesktopTable from './DesktopTable'; import MobileTable from './MobileTable'; +import { AppContext } from '../../../context/AppContext'; +import { + selectHasPermissionAssignmentPermission, + noPermission, +} from '../../../context/selectors'; export default function Permissions() { + const { state } = useContext(AppContext); const showDesktop = useMediaQuery('(min-width:650px)', { noSsr: true }); - return
{showDesktop ? : }
; + return selectHasPermissionAssignmentPermission(state) ? + (
{showDesktop ? : }
) : + (

{noPermission}

); } diff --git a/web-ui/src/components/admin/roles/Roles.jsx b/web-ui/src/components/admin/roles/Roles.jsx index 10b720787d..3145163270 100644 --- a/web-ui/src/components/admin/roles/Roles.jsx +++ b/web-ui/src/components/admin/roles/Roles.jsx @@ -13,7 +13,7 @@ import { updateRole } from '../../../api/roles'; import { - selectHasRoleAssignmentPermission, + selectCanEditMemberRolesPermission, noPermission, } from '../../../context/selectors'; import RoleUserCards from './RoleUserCards'; @@ -215,7 +215,7 @@ const Roles = () => { setEditedRole({ ...editedRole, description: event?.target?.value }); }; - return selectHasRoleAssignmentPermission(state) ? ( + return selectCanEditMemberRolesPermission(state) ? (
diff --git a/web-ui/src/components/admin/roles/Roles.test.jsx b/web-ui/src/components/admin/roles/Roles.test.jsx index 6455c50aeb..8f90beddc5 100644 --- a/web-ui/src/components/admin/roles/Roles.test.jsx +++ b/web-ui/src/components/admin/roles/Roles.test.jsx @@ -16,7 +16,7 @@ const initialState = { userProfile: { name: 'Current User', role: ['MEMBER'], - permissions: [{ permission: 'CAN_ASSIGN_ROLE_PERMISSIONS' }], + permissions: [{ permission: 'CAN_EDIT_MEMBER_ROLES' }], }, } }; diff --git a/web-ui/src/components/admin/users/Users.jsx b/web-ui/src/components/admin/users/Users.jsx index 9692289567..785c139212 100644 --- a/web-ui/src/components/admin/users/Users.jsx +++ b/web-ui/src/components/admin/users/Users.jsx @@ -66,11 +66,8 @@ const Users = () => { setIncludeTerminated(!includeTerminated); }; - const isAdmin = - userProfile && userProfile.role && userProfile.role.includes('ADMIN'); - const normalizedMembers = - isAdmin && includeTerminated + includeTerminated && selectHasCreateMembersPermission(state) ? selectNormalizedMembersAdmin(state, searchText) : selectNormalizedMembers(state, searchText); diff --git a/web-ui/src/components/edit_skills/EditSkillsCard.test.jsx b/web-ui/src/components/edit_skills/EditSkillsCard.test.jsx index 94ae952868..f5a2301ab4 100644 --- a/web-ui/src/components/edit_skills/EditSkillsCard.test.jsx +++ b/web-ui/src/components/edit_skills/EditSkillsCard.test.jsx @@ -22,7 +22,7 @@ const initialState = { firstName: 'Current', lastName: 'User', role: ['MEMBER'], - permissions: [{ permission: 'CAN_VIEW_SKILLS_REPORT' }], + permissions: [{ permission: 'CAN_EDIT_SKILLS' }], imageUrl: 'https://upload.wikimedia.org/wikipedia/commons/7/74/SNL_MrBill_Doll.jpg', memberProfile: currentUserProfile diff --git a/web-ui/src/components/member-directory/AdminMemberCard.jsx b/web-ui/src/components/member-directory/AdminMemberCard.jsx index d767c27fa4..dabe0b5674 100644 --- a/web-ui/src/components/member-directory/AdminMemberCard.jsx +++ b/web-ui/src/components/member-directory/AdminMemberCard.jsx @@ -76,30 +76,39 @@ const AdminMemberCard = ({ member, index }) => { const handleClose = () => setOpen(false); const handleCloseDeleteConfirmation = () => setOpenDelete(false); + const actionFunctions = []; + const options = () => { let entries = []; + actionFunctions.length = 0; + + // This is "Create" permission because there is no "Edit" permission. This + // 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)) { entries.push('Edit'); + actionFunctions.push(handleOpen); } if (selectHasDeleteMembersPermission(state)) { entries.push('Delete'); + actionFunctions.push(handleOpenDeleteConfirmation); } if (selectHasImpersonateMembersPermission(state)) { // If we have not already impersonated a user, we can provide that option. if (document.cookie.indexOf("OJWT=") == -1) { entries.push('Impersonate'); + actionFunctions.push(handleImpersonate); } } return entries; } const handleAction = (e, index) => { - if (index === 0) { - handleOpen(); - } else if (index === 1) { - handleOpenDeleteConfirmation(); - } else if (index === 2) { - handleImpersonate(); + if (index < actionFunctions.length) { + actionFunctions[index](); + } else { + console.warn(`${index} is out of range for the action functions`); } }; @@ -221,11 +230,13 @@ const AdminMemberCard = ({ member, index }) => { selectHasDeleteMembersPermission(state) || selectHasImpersonateMembersPermission(state)) && ( - + {options().length > 0 && + + } { + setShowHoursUpload(false); + setSelectedFile(null); + }; + + const openHoursUpload = () => { + setShowHoursUpload(true); + }; + + const adminLinks = [ + ['/admin/manage-kudos', 'Manage Kudos', () => selectHasAdministerKudosPermission(state)], + ['/admin/permissions', 'Permissions', () => selectHasPermissionAssignmentPermission(state)], + ['/admin/roles', 'Roles', () => selectCanEditMemberRolesPermission(state)], + ['/admin/users', 'Users', () => selectHasCreateMembersPermission(state) || + selectHasDeleteMembersPermission(state) || + selectHasImpersonateMembersPermission(state) + ], + ['/admin/email', 'Send Email', () => selectHasSendEmailPermission(state)], + ['/admin/edit-skills', 'Skills', () => selectCanEditSkills(state)], + ['/admin/settings', 'Settings', () => selectHasViewSettingsPermission(state)], + [openHoursUpload, 'Upload Hours', () => selectHasUploadHoursPermission(state)], + ]; + const getReportLinks = () => { const links = []; @@ -273,15 +296,6 @@ function Menu({ children }) { setFeedbackOpen(false); }; - const closeHoursUpload = () => { - setShowHoursUpload(false); - setSelectedFile(null); - }; - - const openHoursUpload = () => { - setShowHoursUpload(true); - }; - const isLinkSelected = path => { // /checkins route is special case as additional info is added to url if (path === '/checkins' && location.pathname.includes(`${path}/`)) @@ -290,38 +304,66 @@ function Menu({ children }) { }; const createLinkJsx = (path, name, isSubLink) => { - return ( - { - closeSubMenus(); - } - } - selected={isLinkSelected(path)} - > - - - ); + if (typeof(path) === "function") { + return ( + + + + ); + } else { + return ( + { + closeSubMenus(); + } + } + selected={isLinkSelected(path)} + > + + + ); + } }; const onFileSelected = e => { setSelectedFile(e.target.files[0]); }; - const createListJsx = (listArr, isSublink) => { + const hasAtLeastOnePermission = (listArr) => { + for(let listItem of listArr) { + const [path, name, permFunc] = listItem; + if (!permFunc || permFunc()) { + return true; + } + } + return false; + }; + + const createListJsx = (listArr, checkPermissions) => { return listArr.map(listItem => { - const [path, name] = listItem; - return createLinkJsx(path, name, isSublink); - }); + const [path, name, permFunc] = listItem; + if (!checkPermissions || !permFunc || permFunc()) { + return createLinkJsx(path, name, true); + } + }).filter((e) => !!e); }; const drawer = ( @@ -341,7 +383,7 @@ function Menu({ children }) { {createLinkJsx('/', 'HOME', false)} - {isAdmin && ( + {hasAtLeastOnePermission(adminLinks) && ( <> {createListJsx(adminLinks, true)} - {isAdmin && ( - - Upload Hours - - )} )} @@ -368,7 +401,7 @@ function Menu({ children }) { - {createListJsx(checkInLinks, true)} + {createListJsx(checkInLinks)} - {createListJsx(directoryLinks, true)} + {createListJsx(directoryLinks)} - {createListJsx(feedbackLinks, true)} + {createListJsx(feedbackLinks)} {hasReportPermission && ( @@ -397,7 +430,7 @@ function Menu({ children }) { - {createListJsx(getReportLinks(), true)} + {createListJsx(getReportLinks())} )} diff --git a/web-ui/src/components/menu/__snapshots__/Menu.test.jsx.snap b/web-ui/src/components/menu/__snapshots__/Menu.test.jsx.snap index 3d1edd904e..fdf4f92152 100644 --- a/web-ui/src/components/menu/__snapshots__/Menu.test.jsx.snap +++ b/web-ui/src/components/menu/__snapshots__/Menu.test.jsx.snap @@ -523,24 +523,6 @@ exports[` > renders correctly for admin 1`] = ` /> -
-
- - ADMIN - -
- -
> renders correctly for admin 1`] = ` /> -
-
- - ADMIN - -
- -
{ } ]); - return selectHasSkillsReportPermission(state) ? ( + return selectCanEditSkills(state) ? (
diff --git a/web-ui/src/pages/EditSkillsPage.test.jsx b/web-ui/src/pages/EditSkillsPage.test.jsx index c221b6f8c3..7c97ba5a1c 100644 --- a/web-ui/src/pages/EditSkillsPage.test.jsx +++ b/web-ui/src/pages/EditSkillsPage.test.jsx @@ -8,7 +8,7 @@ const userStateWithPermission = { userProfile: { name: 'john watson', role: ['MEMBER'], - permissions: [{ permission: 'CAN_VIEW_SKILLS_REPORT' }], + permissions: [{ permission: 'CAN_EDIT_SKILLS' }], } } }; diff --git a/web-ui/src/pages/ManageKudosPage.jsx b/web-ui/src/pages/ManageKudosPage.jsx index 8b9473a08f..00b45497b6 100644 --- a/web-ui/src/pages/ManageKudosPage.jsx +++ b/web-ui/src/pages/ManageKudosPage.jsx @@ -79,20 +79,24 @@ const ManageKudosPage = () => { }; const loadPendingKudos = useCallback(async () => { - setPendingKudosLoading(true); - const res = await getAllKudos(csrf, true); - if (res?.payload?.data && !res.error) { - setPendingKudosLoading(false); - return res.payload.data; + if (selectHasAdministerKudosPermission(state)) { + setPendingKudosLoading(true); + const res = await getAllKudos(csrf, true); + if (res?.payload?.data && !res.error) { + setPendingKudosLoading(false); + return res.payload.data; + } } }, [csrf, dispatch]); const loadApprovedKudos = useCallback(async () => { - setApprovedKudosLoading(true); - const res = await getAllKudos(csrf, false); - if (res?.payload?.data && !res.error) { - setApprovedKudosLoading(false); - return res.payload.data; + if (selectHasAdministerKudosPermission(state)) { + setApprovedKudosLoading(true); + const res = await getAllKudos(csrf, false); + if (res?.payload?.data && !res.error) { + setApprovedKudosLoading(false); + return res.payload.data; + } } }, [csrf, dispatch]); diff --git a/web-ui/src/pages/PermissionsPage.jsx b/web-ui/src/pages/PermissionsPage.jsx index ea65df8127..f289724466 100644 --- a/web-ui/src/pages/PermissionsPage.jsx +++ b/web-ui/src/pages/PermissionsPage.jsx @@ -18,6 +18,7 @@ import { AppContext } from '../context/AppContext'; import { selectRoles, selectHasPermissionAssignmentPermission, + selectHasViewPermissionPermission, noPermission, } from '../context/selectors'; import { useQueryParameters } from '../helpers/query-parameters'; @@ -91,10 +92,12 @@ const EditPermissionsPage = () => { useEffect(() => { const getRolePermissions = async () => { - const res = await getRolePermissionsList(csrf); - const data = res.payload?.data && !res.error ? res.payload.data : null; - if (data) { - setRolePermissionsList(data); + if (selectHasViewPermissionPermission(state)) { + const res = await getRolePermissionsList(csrf); + const data = res.payload?.data && !res.error ? res.payload.data : null; + if (data) { + setRolePermissionsList(data); + } } }; const getPermissions = async () => {