Skip to content

Commit 7d08d3b

Browse files
committed
Remove unrequired permissions, but check the member id still for editing/creating
1 parent 80f6881 commit 7d08d3b

File tree

5 files changed

+4
-50
lines changed

5 files changed

+4
-50
lines changed

server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ public enum Permission {
4747
CAN_DELETE_REVIEW_PERIOD("Delete review period", "Review Period"),
4848
CAN_ADMINISTER_SETTINGS("Add or edit settings", "Settings"),
4949
CAN_VIEW_SETTINGS("View settings", "Settings"),
50-
CAN_VIEW_ALL_PULSE_RESPONSES("View all pulse responses", "Pulse"),
51-
CAN_CREATE_ALL_PULSE_RESPONSES("Create pulse responses for anyone", "Pulse"),
52-
CAN_UPDATE_ALL_PULSE_RESPONSES("Update pulse responses for anyone", "Pulse");
50+
CAN_VIEW_ALL_PULSE_RESPONSES("View all pulse responses", "Pulse");
5351

5452
private final String description;
5553
private final String category;

server/src/main/java/com/objectcomputing/checkins/services/pulseresponse/PulseResponseServicesImpl.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ public PulseResponseServicesImpl(
4040
@Override
4141
public PulseResponse save(PulseResponse pulseResponse) {
4242
UUID currentUserId = currentUserServices.getCurrentUser().getId();
43-
boolean hasPermission = rolePermissionServices.findUserPermissions(currentUserId).contains(Permission.CAN_CREATE_ALL_PULSE_RESPONSES);
44-
4543
PulseResponse pulseResponseRet = null;
4644
if (pulseResponse != null) {
4745
final UUID memberId = pulseResponse.getTeamMemberId();
@@ -52,7 +50,7 @@ public PulseResponse save(PulseResponse pulseResponse) {
5250
throw new BadArgException(String.format("Member %s doesn't exists", memberId));
5351
} else if (pulseSubDate.isBefore(LocalDate.EPOCH) || pulseSubDate.isAfter(LocalDate.MAX)) {
5452
throw new BadArgException(String.format("Invalid date for pulseresponse submission date %s", memberId));
55-
} else if (!hasPermission && !currentUserId.equals(memberId) && !isSubordinateTo(pulseResponse.getTeamMemberId(), currentUserId)) {
53+
} else if (!currentUserId.equals(memberId) && !isSubordinateTo(memberId, currentUserId)) {
5654
throw new BadArgException(String.format("User %s does not have permission to create pulse response for user %s", currentUserId, memberId));
5755
}
5856
pulseResponseRet = pulseResponseRepo.save(pulseResponse);
@@ -73,8 +71,6 @@ public PulseResponse read(@NotNull UUID id) {
7371
@Override
7472
public PulseResponse update(PulseResponse pulseResponse) {
7573
UUID currentUserId = currentUserServices.getCurrentUser().getId();
76-
boolean hasPermission = rolePermissionServices.findUserPermissions(currentUserId).contains(Permission.CAN_UPDATE_ALL_PULSE_RESPONSES);
77-
7874
PulseResponse pulseResponseRet = null;
7975
if (pulseResponse != null) {
8076
final UUID id = pulseResponse.getId();
@@ -88,7 +84,7 @@ public PulseResponse update(PulseResponse pulseResponse) {
8884
throw new BadArgException(String.format("Invalid pulseresponse %s", pulseResponse));
8985
} else if (pulseSubDate.isBefore(LocalDate.EPOCH) || pulseSubDate.isAfter(LocalDate.MAX)) {
9086
throw new BadArgException(String.format("Invalid date for pulseresponse submission date %s", memberId));
91-
} else if (!hasPermission && !currentUserId.equals(memberId) && !isSubordinateTo(pulseResponse.getTeamMemberId(), currentUserId)) {
87+
} else if (!currentUserId.equals(memberId) && !isSubordinateTo(memberId, currentUserId)) {
9288
throw new BadArgException(String.format("User %s does not have permission to update pulse response for user %s", currentUserId, memberId));
9389
}
9490
pulseResponseRet = pulseResponseRepo.update(pulseResponse);

server/src/main/resources/db/dev/R__Load_testing_data.sql

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -892,16 +892,6 @@ insert into role_permissions
892892
values
893893
('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_VIEW_ALL_PULSE_RESPONSES');
894894

895-
insert into role_permissions
896-
(roleid, permission)
897-
values
898-
('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_CREATE_ALL_PULSE_RESPONSES');
899-
900-
insert into role_permissions
901-
(roleid, permission)
902-
values
903-
('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_UPDATE_ALL_PULSE_RESPONSES');
904-
905895
-- PDL Permissions
906896
insert into role_permissions
907897
(roleid, permission)

server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,7 @@ public interface PermissionFixture extends RolePermissionFixture {
8383
Permission.CAN_LAUNCH_REVIEW_PERIOD,
8484
Permission.CAN_CLOSE_REVIEW_PERIOD,
8585
Permission.CAN_DELETE_REVIEW_PERIOD,
86-
Permission.CAN_VIEW_ALL_PULSE_RESPONSES,
87-
Permission.CAN_CREATE_ALL_PULSE_RESPONSES,
88-
Permission.CAN_UPDATE_ALL_PULSE_RESPONSES
86+
Permission.CAN_VIEW_ALL_PULSE_RESPONSES
8987
);
9088

9189
default void setPermissionsForAdmin(UUID roleID) {

server/src/test/java/com/objectcomputing/checkins/services/pulseresponse/PulseResponseControllerTest.java

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,6 @@ void testCreatePulseResponseForSomeoneInHierarchy() {
130130
assertEquals(String.format("%s/%s", request.getPath(), pulseResponseResponse.getId()), response.getHeaders().get("location"));
131131
}
132132

133-
@Test
134-
void testCreatePulseResponseForSomeoneUnrelatedWithRole() {
135-
MemberProfile memberProfile = createADefaultMemberProfile();
136-
PulseResponseCreateDTO pulseResponseCreateDTO = createPulseResponseCreateDTO(id(HIERARCHY_LEAD2));
137-
138-
HttpRequest<PulseResponseCreateDTO> request = HttpRequest.POST("", pulseResponseCreateDTO).basicAuth(ADMIN_ROLE, ADMIN_ROLE);
139-
final HttpResponse<PulseResponse> response = client.toBlocking().exchange(request, PulseResponse.class);
140-
141-
PulseResponse pulseResponseResponse = response.body();
142-
143-
Assertions.assertNotNull(pulseResponseResponse);
144-
assertEquals(HttpStatus.CREATED, response.getStatus());
145-
assertEquals(pulseResponseCreateDTO.getTeamMemberId(), pulseResponseResponse.getTeamMemberId());
146-
assertEquals(String.format("%s/%s", request.getPath(), pulseResponseResponse.getId()), response.getHeaders().get("location"));
147-
}
148-
149133
@Test
150134
void testCreateANullPulseResponse() {
151135
final HttpRequest<String> request = HttpRequest.POST("", "").basicAuth(MEMBER_ROLE, MEMBER_ROLE);
@@ -348,18 +332,6 @@ void testUpdatePulseResponse() {
348332
assertEquals(String.format("%s/%s", request.getPath(), pulseResponse.getId()), response.getHeaders().get("location"));
349333
}
350334

351-
@Test
352-
void testUpdatePulseResponseForUnrelatedMemberWithRole() {
353-
PulseResponse pulseResponse = createADefaultPulseResponse(profile(HIERARCHY_LEAD2));
354-
355-
final HttpRequest<PulseResponse> request = HttpRequest.PUT("", pulseResponse).basicAuth(ADMIN_ROLE, ADMIN_ROLE);
356-
final HttpResponse<PulseResponse> response = client.toBlocking().exchange(request, PulseResponse.class);
357-
358-
assertEquals(pulseResponse, response.body());
359-
assertEquals(HttpStatus.OK, response.getStatus());
360-
assertEquals(String.format("%s/%s", request.getPath(), pulseResponse.getId()), response.getHeaders().get("location"));
361-
}
362-
363335
@Test
364336
void testUpdatePulseResponseForSubordinateMember() {
365337
PulseResponse pulseResponse = createADefaultPulseResponse(profile(HIERARCHY_LEAD2_SUB1_SUB1));

0 commit comments

Comments
 (0)