Skip to content

Commit

Permalink
Rename all Roles to BusinessUnitUserPermissions & 'BusinessUserId' to…
Browse files Browse the repository at this point in the history
… 'BusinessUnitUserId'
  • Loading branch information
RustyHMCTS committed Sep 18, 2024
1 parent 9dc9c0c commit 4e9d47c
Show file tree
Hide file tree
Showing 26 changed files with 164 additions and 132 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@PO-235 @Opal
Feature: tests for notes roles/permissions for accounts dependant on business units
Feature: tests for notes business unit users/permissions for accounts dependant on business units

Scenario: A user can add a note to a business unit it is part of
Given I am testing as the "opal-test@hmcts.net" user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ void testHandleOauthCode() throws Exception {
UserState userState = UserState.builder()
.userName("name")
.userId(123L)
.roles(Set.of(BusinessUnitUserPermissions.builder()
.businessUnitUserPermissions(Set.of(BusinessUnitUserPermissions.builder()
.businessUnitId((short) 123)
.businessUserId("BU123")
.businessUnitUserId("BU123")
.permissions(Set.of(
Permission.builder()
.permissionId(1L)
Expand All @@ -92,10 +92,14 @@ void testHandleOauthCode() throws Exception {
.andExpect(jsonPath("$.access_token").value("accessToken"))
.andExpect(jsonPath("$.user_state.user_name").value("name"))
.andExpect(jsonPath("$.user_state.user_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_unit_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_user_id").value("BU123"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_id").value("1"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_name")
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_id")
.value("123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_user_id")
.value("BU123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_id")
.value("1"))
.andExpect(
jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_name")
.value("Notes"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void testGetBusinessUnitRefData_Permission_success() throws Exception {

when(businessUnitService.getReferenceData(any())).thenReturn(singletonList(refData));
when(userStateService.getUserStateUsingAuthToken(anyString())).thenReturn(userState);
when(userState.allRolesWithPermission(any())).thenReturn(new TestUserRoles(true));
when(userState.allBusinessUnitUsersWithPermission(any())).thenReturn(new TestUserBusinessUnits(true));

mockMvc.perform(get(URL_BASE + "?permission=MANUAL_ACCOUNT_CREATION")
.header("authorization", "Bearer some_value"))
Expand All @@ -151,7 +151,7 @@ void testGetBusinessUnitRefData_Permission_empty() throws Exception {

when(businessUnitService.getReferenceData(any())).thenReturn(singletonList(refData));
when(userStateService.getUserStateUsingAuthToken(anyString())).thenReturn(userState);
when(userState.allRolesWithPermission(any())).thenReturn(new TestUserRoles(false));
when(userState.allBusinessUnitUsersWithPermission(any())).thenReturn(new TestUserBusinessUnits(false));

mockMvc.perform(get(URL_BASE + "?permission=MANUAL_ACCOUNT_CREATION")
.header("authorization", "Bearer some_value"))
Expand Down Expand Up @@ -185,10 +185,10 @@ private BusinessUnitReferenceData createBusinessUnitRefData() {
"XX", "Fines", null, null);
}

private class TestUserRoles implements UserState.UserRoles {
private class TestUserBusinessUnits implements UserState.UserBusinessUnits {
private final boolean contains;

public TestUserRoles(boolean contains) {
public TestUserBusinessUnits(boolean contains) {
this.contains = contains;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
import uk.gov.hmcts.opal.authentication.model.AccessTokenResponse;
import uk.gov.hmcts.opal.authentication.model.SecurityToken;
import uk.gov.hmcts.opal.authentication.service.AccessTokenService;
import uk.gov.hmcts.opal.authorisation.model.Permission;
import uk.gov.hmcts.opal.authorisation.model.BusinessUnitUserPermissions;
import uk.gov.hmcts.opal.authorisation.model.Permission;
import uk.gov.hmcts.opal.authorisation.model.UserState;
import uk.gov.hmcts.opal.authorisation.service.AuthorisationService;
import uk.gov.hmcts.opal.dto.AppMode;
Expand All @@ -40,9 +40,9 @@ class TestingSupportControllerTest {
private static final UserState USER_STATE = UserState.builder()
.userName("name")
.userId(123L)
.roles(Set.of(BusinessUnitUserPermissions.builder()
.businessUnitUserPermissions(Set.of(BusinessUnitUserPermissions.builder()
.businessUnitId((short) 123)
.businessUserId("BU123")
.businessUnitUserId("BU123")
.permissions(Set.of(
Permission.builder()
.permissionId(1L)
Expand Down Expand Up @@ -132,10 +132,14 @@ void testGetToken() throws Exception {
.andExpect(jsonPath("$.access_token").value("testToken"))
.andExpect(jsonPath("$.user_state.user_name").value("name"))
.andExpect(jsonPath("$.user_state.user_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_unit_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_user_id").value("BU123"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_id").value("1"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_name")
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_id")
.value("123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_user_id")
.value("BU123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_id")
.value("1"))
.andExpect(
jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_name")
.value("Notes"));

}
Expand All @@ -160,10 +164,14 @@ void testGetTokenForUser() throws Exception {
.andExpect(jsonPath("$.access_token").value("testToken"))
.andExpect(jsonPath("$.user_state.user_name").value("name"))
.andExpect(jsonPath("$.user_state.user_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_unit_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_user_id").value("BU123"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_id").value("1"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_name")
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_id")
.value("123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_user_id")
.value("BU123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_id")
.value("1"))
.andExpect(
jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_name")
.value("Notes"));
}

Expand Down Expand Up @@ -199,10 +207,14 @@ void testGetTokenForUserFailure() throws Exception {
.andExpect(jsonPath("$.access_token").value("testToken"))
.andExpect(jsonPath("$.user_state.user_name").value("name"))
.andExpect(jsonPath("$.user_state.user_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_unit_id").value("123"))
.andExpect(jsonPath("$.user_state.roles[0].business_user_id").value("BU123"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_id").value("1"))
.andExpect(jsonPath("$.user_state.roles[0].permissions[0].permission_name")
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_id")
.value("123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].business_unit_user_id")
.value("BU123"))
.andExpect(jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_id")
.value("1"))
.andExpect(
jsonPath("$.user_state.business_unit_user_permissions[0].permissions[0].permission_name")
.value("Notes"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ public Object checkAuthorization(ProceedingJoinPoint joinPoint,
Object[] args = joinPoint.getArgs();
UserState userState = userStateAspectService.getUserState(joinPoint);

BusinessUnitUserPermissions role = authorizationAspectService.getRole(args, userState);
if (checkRoleHasPermission(role, authorizedRoleHasPermission.value())) {
BusinessUnitUserPermissions businessUnitUserPermissions = authorizationAspectService.getRole(args, userState);
if (checkRoleHasPermission(businessUnitUserPermissions, authorizedRoleHasPermission.value())) {
return joinPoint.proceed();
}
throw new PermissionNotAllowedException(authorizedRoleHasPermission.value(), role);
throw new PermissionNotAllowedException(authorizedRoleHasPermission.value(), businessUnitUserPermissions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public BusinessUnitUserPermissions getRole(Object[] args, UserState userState) {
}
throw new RoleNotFoundException(format(
"Can't infer the role for user %s. "
+ "Annotated method needs to have arguments of types (Role, AddNoteDto, NoteDto).",
+ "Annotated method needs to have arguments of types"
+ " (BusinessUnitUserPermissions, AddNoteDto, NoteDto).",
userState.getUserName()
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
* The role can be one of the argument of the annotated method.
* <pre>
* &#064;AuthorizedRoleHasPermission(Permissions.ACCOUNT_ENQUIRY)
* public void businessMethod(Role role) { ... }
* public void businessMethod(BusinessUnitUserPermissions role) { ... }
* </pre>
* The role can be inferred if one of the argument is of type NoteDto, the role will be picked by matching
* businessUnitId of NoteDto argument within the userState roles.
* businessUnitId of NoteDto argument within the userState businessUnitUserPermissions.
* If this role has the permission then only execution will be allowed, otherwise PermissionNotAllowedException
* will be thrown.
* For example:
Expand All @@ -28,7 +28,7 @@
* public NoteDto saveNote(NoteDto noteDto) { .. }
* </pre>
* The role can be inferred if one of the argument is of type NoteDto, the role will be picked by matching
* businessUnitId of AddNoteDto argument within the userState roles.
* businessUnitId of AddNoteDto argument within the userState businessUnitUserPermissions.
* If this role has the permission then only execution will be allowed, otherwise PermissionNotAllowedException
* will be thrown.
* For example:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,19 @@
public class PermissionNotAllowedException extends RuntimeException {

private final Permissions permission;
private final BusinessUnitUserPermissions role;
private final BusinessUnitUserPermissions businessUnitUserPermissions;

public PermissionNotAllowedException(Permissions value) {
super(value + " permission is not allowed for the user");
this.permission = value;
this.role = null;
this.businessUnitUserPermissions = null;
}

public PermissionNotAllowedException(Permissions permission, BusinessUnitUserPermissions role) {
super(permission + " permission is not allowed for the role " + role);
public PermissionNotAllowedException(Permissions permission,
BusinessUnitUserPermissions businessUnitUserPermissions) {
super(permission + " permission is not allowed for the businessUnitUserPermissions "
+ businessUnitUserPermissions);
this.permission = permission;
this.role = role;
this.businessUnitUserPermissions = businessUnitUserPermissions;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
public class BusinessUnitUserPermissions {

@NonNull
String businessUserId;
String businessUnitUserId;

@NonNull
Short businessUnitId;
Expand All @@ -25,11 +25,11 @@ public class BusinessUnitUserPermissions {
Set<Permission> permissions;

@JsonCreator
public BusinessUnitUserPermissions(@JsonProperty("business_user_id") String businessUserId,
public BusinessUnitUserPermissions(@JsonProperty("business_unit_user_id") String businessUnitUserId,
@JsonProperty("business_unit_id") Short businessUnitId,
@JsonProperty("permissions") Set<Permission> permissions) {

this.businessUserId = businessUserId;
this.businessUnitUserId = businessUnitUserId;
this.businessUnitId = businessUnitId;
this.permissions = permissions;
}
Expand All @@ -46,8 +46,8 @@ public boolean matchesBusinessUnitId(Short roleBusinessUnitId) {
return businessUnitId.equals(roleBusinessUnitId);
}

public static class DeveloperRole extends BusinessUnitUserPermissions {
DeveloperRole() {
public static class DeveloperBusinessUnitUserPermissions extends BusinessUnitUserPermissions {
DeveloperBusinessUnitUserPermissions() {
super("", Short.MAX_VALUE, Collections.emptySet());
}

Expand Down
42 changes: 22 additions & 20 deletions src/main/java/uk/gov/hmcts/opal/authorisation/model/UserState.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.NonNull;
import uk.gov.hmcts.opal.authorisation.model.BusinessUnitUserPermissions.DeveloperRole;
import uk.gov.hmcts.opal.authorisation.model.BusinessUnitUserPermissions.DeveloperBusinessUnitUserPermissions;

import java.util.Collections;
import java.util.Optional;
Expand All @@ -24,57 +24,58 @@ public class UserState {
String userName;

@EqualsAndHashCode.Exclude
Set<BusinessUnitUserPermissions> roles;
Set<BusinessUnitUserPermissions> businessUnitUserPermissions;

@JsonCreator
public UserState(
@JsonProperty("user_id") Long userId,
@JsonProperty("user_name") String userName,
@JsonProperty("roles") Set<BusinessUnitUserPermissions> roles
@JsonProperty("business_unit_user_permissions") Set<BusinessUnitUserPermissions> businessUnitUserPermissions
) {
this.userId = userId;
this.userName = userName;
this.roles = roles;
this.businessUnitUserPermissions = businessUnitUserPermissions;
}

public boolean anyRoleHasPermission(Permissions permission) {
return roles.stream().anyMatch(r -> r.hasPermission(permission));
return businessUnitUserPermissions.stream().anyMatch(r -> r.hasPermission(permission));
}

public boolean noRoleHasPermission(Permissions permission) {
return !anyRoleHasPermission(permission);
}

public UserRoles allRolesWithPermission(Permissions permission) {
return new UserRolesImpl(
roles.stream().filter(r -> r.hasPermission(permission)).collect(Collectors.toSet()));
public UserBusinessUnits allBusinessUnitUsersWithPermission(Permissions permission) {
return new UserBusinessUnitsImpl(
businessUnitUserPermissions.stream().filter(r -> r.hasPermission(permission)).collect(Collectors.toSet()));
}

public boolean hasRoleWithPermission(short roleBusinessUnitId, Permissions permission) {
return roles.stream()
return businessUnitUserPermissions.stream()
.filter(r -> r.matchesBusinessUnitId(roleBusinessUnitId))
.findAny() // Should be either zero or one roles that match the business unit id
.findAny() // Should be either zero or one businessUnitUserPermissions that match the business unit id
.stream()
.anyMatch(r -> r.hasPermission(permission));
}

public Optional<BusinessUnitUserPermissions> getRoleForBusinessUnit(Short businessUnitId) {
return roles.stream()
return businessUnitUserPermissions.stream()
.filter(r -> r.matchesBusinessUnitId(businessUnitId))
.findFirst();
}

public static interface UserRoles {
public static interface UserBusinessUnits {
boolean containsBusinessUnit(Short businessUnitId);
}

public static class UserRolesImpl implements UserRoles {
private final Set<BusinessUnitUserPermissions> roles;
public static class UserBusinessUnitsImpl implements UserBusinessUnits {
private final Set<BusinessUnitUserPermissions> businessUnitUserPermissions;
private final Set<Short> businessUnits;

public UserRolesImpl(Set<BusinessUnitUserPermissions> roles) {
this.roles = roles;
businessUnits = roles.stream().map(r -> r.getBusinessUnitId()).collect(Collectors.toSet());
public UserBusinessUnitsImpl(Set<BusinessUnitUserPermissions> businessUnitUserPermissions) {
this.businessUnitUserPermissions = businessUnitUserPermissions;
businessUnits = businessUnitUserPermissions.stream().map(r -> r.getBusinessUnitId())
.collect(Collectors.toSet());
}

public boolean containsBusinessUnit(Short businessUnitId) {
Expand All @@ -83,7 +84,8 @@ public boolean containsBusinessUnit(Short businessUnitId) {
}

public static class DeveloperUserState extends UserState {
private static final Optional<BusinessUnitUserPermissions> DEV_ROLE = Optional.of(new DeveloperRole());
private static final Optional<BusinessUnitUserPermissions> DEV_ROLE =
Optional.of(new DeveloperBusinessUnitUserPermissions());

public DeveloperUserState() {
super(0L, "Developer_User", Collections.emptySet());
Expand All @@ -100,8 +102,8 @@ public Optional<BusinessUnitUserPermissions> getRoleForBusinessUnit(Short busine
}

@Override
public UserRoles allRolesWithPermission(Permissions permission) {
return new UserRoles() {
public UserBusinessUnits allBusinessUnitUsersWithPermission(Permissions permission) {
return new UserBusinessUnits() {
@Override
public boolean containsBusinessUnit(Short businessUnitId) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,16 @@ public ResponseEntity<NoteDto> addNote(
log.info(":POST:addNote: {}", addNote.toPrettyJson());

UserState userState = userStateService.getUserStateUsingAuthToken(authHeaderValue);
BusinessUnitUserPermissions role = getRequiredRole(userState, addNote.getBusinessUnitId());
BusinessUnitUserPermissions businessUnitUserPermissions = getRequiredRole(userState,
addNote.getBusinessUnitId());

NoteDto noteDto = NoteDto.builder()
.associatedRecordId(addNote.getAssociatedRecordId())
.noteText(addNote.getNoteText())
.associatedRecordType(NOTE_ASSOC_REC_TYPE)
.noteType("AA") // TODO - This will probably need to part of the AddNoteDto in future
.businessUnitId(addNote.getBusinessUnitId())
.postedBy(role.getBusinessUserId())
.postedBy(businessUnitUserPermissions.getBusinessUnitUserId())
.postedByUserId(userState.getUserId())
.postedDate(LocalDateTime.now())
.build();
Expand Down
Loading

0 comments on commit 4e9d47c

Please sign in to comment.