-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature 2825/adjust admin menu #2828
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
523015b
Added permissions to upload hours, and edit/delete skills. Partially…
ocielliottc b786c08
Fixed skill name
ocielliottc a30fd25
Added the ability to check permissions on the current user and use th…
ocielliottc 51f334a
Added an implementation of the hasPermission method.
ocielliottc fbe0b18
Added a permission for editing member roles, disallow removal of edit…
ocielliottc ae8bed0
Replace check for admin with permission check.
ocielliottc 2d8c62f
Updated tests.
ocielliottc 091554a
Updated exception message.
ocielliottc 1bc6da6
Replace admin check with a new permission and renamed a method to ref…
ocielliottc 40b92b4
Merge branch 'develop' into feature-2825/adjust-admin-menu
ocielliottc 6360bfb
Renamed and modified the unsecure update method to only update the la…
ocielliottc 29488da
Merge branch 'develop' into feature-2825/adjust-admin-menu
ocielliottc 8d4b2ef
Merge branch 'develop' into feature-2825/adjust-admin-menu
mkimberlin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(() -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same above here. Is there a permission check happening? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with the delete:
|
||
new NotFoundException(KUDOS_DOES_NOT_EXIST_MSG.formatted(id))); | ||
|
||
|
@@ -196,7 +189,7 @@ public List<KudosResponseDTO> 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<KudosResponseDTO> getRecent() { | |
} | ||
|
||
private List<KudosResponseDTO> findByPending(boolean isPending) { | ||
if (!currentUserServices.isAdmin()) { | ||
if (!hasAdministerKudosPermission()) { | ||
throw new PermissionException(NOT_AUTHORIZED_MSG); | ||
} | ||
|
||
|
@@ -235,10 +228,10 @@ private List<KudosResponseDTO> findByPending(boolean isPending) { | |
|
||
|
||
private List<KudosResponseDTO> 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<KudosResponseDTO> findAllToMember(UUID memberId) { | |
|
||
private List<KudosResponseDTO> 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); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not need to replace this with a permission check? I don't see it happening elsewhere in here, but I could be missing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The controller handles this: