Skip to content
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.

Sign up for GitHub

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

[JN-1169] add BASE permission #972

Merged
merged 7 commits into from
Jul 1, 2024
Merged

[JN-1169] add BASE permission #972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9b69f3f
WIP
devonbush Jun 20, 2024
d2af028
WIP
devonbush Jun 20, 2024
63bc96e
Merge branch 'development' into db-add-any-permission
devonbush Jun 30, 2024
eb94104
adding check all annotations methods
devonbush Jun 30, 2024
f3205d9
test fixes
devonbush Jun 30, 2024
3fde15b
consolidating service tests
devonbush Jun 30, 2024
ac12a1c
Merge branch 'development' into db-add-any-permission
devonbush Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions .../src/main/java/bio/terra/pearl/api/admin/controller/forms/ConfiguredSurveyController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import bio.terra.pearl.api.admin.api.ConfiguredSurveyApi;
import bio.terra.pearl.api.admin.service.auth.AuthUtilService;
import bio.terra.pearl.api.admin.service.auth.context.PortalStudyAuthContext;
import bio.terra.pearl.api.admin.service.auth.context.PortalStudyEnvAuthContext;
import bio.terra.pearl.api.admin.service.forms.SurveyExtService;
import bio.terra.pearl.core.model.EnvironmentName;
import bio.terra.pearl.core.model.admin.AdminUser;
Expand Down Expand Up @@ -48,7 +50,10 @@ public ResponseEntity<Object> findWithNoContent(
Boolean activeVal = active != null ? Boolean.valueOf(active) : null;
List<StudyEnvironmentSurvey> studyEnvSurveys =
surveyExtService.findWithSurveyNoContent(
portalShortcode, studyShortcode, environmentName, stableId, activeVal, operator);
PortalStudyAuthContext.of(operator, portalShortcode, studyShortcode),
stableId,
environmentName,
activeVal);
return ResponseEntity.ok(studyEnvSurveys);
}

Expand All @@ -59,14 +64,16 @@ public ResponseEntity<Object> patch(
String envName,
UUID configuredSurveyId,
Object body) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
AdminUser operator = authUtilService.requireAdminUser(request);
EnvironmentName environmentName = EnvironmentName.valueOfCaseInsensitive(envName);
StudyEnvironmentSurvey configuredSurvey =
objectMapper.convertValue(body, StudyEnvironmentSurvey.class);

StudyEnvironmentSurvey savedSes =
surveyExtService.updateConfiguredSurvey(
portalShortcode, environmentName, studyShortcode, configuredSurvey, adminUser);
PortalStudyEnvAuthContext.of(
operator, portalShortcode, studyShortcode, environmentName),
configuredSurvey);
return ResponseEntity.ok(savedSes);
}

Expand All @@ -83,37 +90,38 @@ public ResponseEntity<Object> replace(
objectMapper.convertValue(body, StudyEnvironmentSurvey.class);
newStudyEnvSurvey =
surveyExtService.replace(
portalShortcode,
studyShortcode,
environmentName,
PortalStudyEnvAuthContext.of(
operator, portalShortcode, studyShortcode, environmentName),
studyEnvSurveyId,
newStudyEnvSurvey,
operator);
newStudyEnvSurvey);
return ResponseEntity.ok(newStudyEnvSurvey);
}

@Override
public ResponseEntity<Object> create(
String portalShortcode, String studyShortcode, String envName, Object body) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
AdminUser operator = authUtilService.requireAdminUser(request);
EnvironmentName environmentName = EnvironmentName.valueOfCaseInsensitive(envName);
StudyEnvironmentSurvey configuredSurvey =
objectMapper.convertValue(body, StudyEnvironmentSurvey.class);

StudyEnvironmentSurvey savedSes =
surveyExtService.createConfiguredSurvey(
portalShortcode, studyShortcode, environmentName, configuredSurvey, adminUser);
PortalStudyEnvAuthContext.of(
operator, portalShortcode, studyShortcode, environmentName),
configuredSurvey);
return ResponseEntity.ok(savedSes);
}

@Override
public ResponseEntity<Void> remove(
String portalShortcode, String studyShortcode, String envName, UUID configuredSurveyId) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
AdminUser operator = authUtilService.requireAdminUser(request);
EnvironmentName environmentName = EnvironmentName.valueOfCaseInsensitive(envName);

surveyExtService.removeConfiguredSurvey(
portalShortcode, studyShortcode, environmentName, configuredSurveyId, adminUser);
PortalStudyEnvAuthContext.of(operator, portalShortcode, studyShortcode, environmentName),
configuredSurveyId);
return ResponseEntity.noContent().build();
}
}
25 changes: 15 additions & 10 deletions api-admin/src/main/java/bio/terra/pearl/api/admin/controller/forms/SurveyController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import bio.terra.pearl.api.admin.api.SurveyApi;
import bio.terra.pearl.api.admin.service.auth.AuthUtilService;
import bio.terra.pearl.api.admin.service.auth.context.PortalAuthContext;
import bio.terra.pearl.api.admin.service.forms.SurveyExtService;
import bio.terra.pearl.core.model.admin.AdminUser;
import bio.terra.pearl.core.model.survey.Survey;
Expand Down Expand Up @@ -30,41 +31,45 @@ public SurveyController(

@Override
public ResponseEntity<Object> get(String portalShortcode, String stableId, Integer version) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
Survey survey = surveyExtService.get(portalShortcode, stableId, version, adminUser);
AdminUser operator = authUtilService.requireAdminUser(request);
Survey survey =
surveyExtService.get(PortalAuthContext.of(operator, portalShortcode), stableId, version);
return ResponseEntity.ok(survey);
}

@Override
public ResponseEntity<Object> getAllVersions(String portalShortcode, String stableId) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
return ResponseEntity.ok(surveyExtService.listVersions(portalShortcode, stableId, adminUser));
AdminUser operator = authUtilService.requireAdminUser(request);
return ResponseEntity.ok(
surveyExtService.listVersions(PortalAuthContext.of(operator, portalShortcode), stableId));
}

@Override
public ResponseEntity<Object> create(String portalShortcode, Object body) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
AdminUser operator = authUtilService.requireAdminUser(request);

Survey survey = objectMapper.convertValue(body, Survey.class);
Survey savedSurvey = surveyExtService.create(portalShortcode, survey, adminUser);
Survey savedSurvey =
surveyExtService.create(PortalAuthContext.of(operator, portalShortcode), survey);
return ResponseEntity.ok(savedSurvey);
}

@Override
public ResponseEntity<Object> newVersion(String portalShortcode, String stableId, Object body) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
AdminUser operator = authUtilService.requireAdminUser(request);
Survey survey = objectMapper.convertValue(body, Survey.class);
if (!stableId.equals(survey.getStableId())) {
throw new IllegalArgumentException("survey parameters don't match");
}
Survey savedSurvey = surveyExtService.createNewVersion(portalShortcode, survey, adminUser);
Survey savedSurvey =
surveyExtService.createNewVersion(PortalAuthContext.of(operator, portalShortcode), survey);
return ResponseEntity.ok(savedSurvey);
}

@Override
public ResponseEntity<Void> delete(String portalShortcode, String stableId) {
AdminUser adminUser = authUtilService.requireAdminUser(request);
surveyExtService.delete(portalShortcode, stableId, adminUser);
AdminUser operator = authUtilService.requireAdminUser(request);
surveyExtService.delete(PortalAuthContext.of(operator, portalShortcode), stableId);
return ResponseEntity.noContent().build();
}
}
9 changes: 8 additions & 1 deletion api-admin/src/main/java/bio/terra/pearl/api/admin/service/auth/AuthUtilService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
/** Utility service for common auth-related methods */
@Service
public class AuthUtilService {
/**
* 'BASE_PERMISSON' indicates the operation is permitted for any PortalAdminUser as they are
* authorized to access the given portal. It might include public-ish operations like viewing
* surveys, etc.
*/
public static final String BASE_PERMISSON = "BASE";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own understanding: how does the idea of a "base" permission differ from a "read"/"view" permission? Is the idea that there will be other non-read actions that we'll want to capture with the base permission without having to enumerate every piece of functionally as a new permission, and so calling it "read" wouldn't be accurate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are reads that would not be included in the BASE permission. For example, reading participant data shouldn't be in "BASE"--that should require a read_participant_data. And there might be trivial writes that don't require a permission (e.g. updating your own user preferences)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it- makes sense! I appreciate the clarification


private final AdminUserService adminUserService;
private final BearerTokenFactory bearerTokenFactory;
private final PortalService portalService;
Expand Down Expand Up @@ -77,7 +84,7 @@ public Portal authUserToPortal(AdminUser user, String portalShortcode) {
public Portal authUserToPortalWithPermission(
AdminUser user, String portalShortcode, String permission) {
Portal portal = authUserToPortal(user, portalShortcode);
if (user.isSuperuser()) {
if (user.isSuperuser() || BASE_PERMISSON.equals(permission)) {
return portal;
}
adminUserService
Expand Down
5 changes: 2 additions & 3 deletions ...th/BaseEnforcePortalPermissionAspect.java → ...e/auth/EnforcePortalPermissionAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
* Aspect to enforce portal permissions on methods annotated with EnforcePortalPermission. Inspired
* by https://stackoverflow.com/questions/50882444/can-spring-annotation-access-method-parameters
*/
public class BaseEnforcePortalPermissionAspect
public class EnforcePortalPermissionAspect
extends BaseEnforcePermissionAspect<PortalAuthContext, EnforcePortalPermission> {

private final AuthUtilService authUtilService;

public BaseEnforcePortalPermissionAspect(AuthUtilService authUtilService) {
public EnforcePortalPermissionAspect(AuthUtilService authUtilService) {
this.authUtilService = authUtilService;
}

Expand Down
4 changes: 2 additions & 2 deletions ...nforcePortalStudyEnvPermissionAspect.java → ...nforcePortalStudyEnvPermissionAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
@Component
@Aspect
@Slf4j
public class BaseEnforcePortalStudyEnvPermissionAspect
public class EnforcePortalStudyEnvPermissionAspect
extends BaseEnforcePermissionAspect<
PortalStudyEnvAuthContext, EnforcePortalStudyEnvPermission> {
private final AuthUtilService authUtilService;
private final StudyEnvironmentService studyEnvironmentService;

public BaseEnforcePortalStudyEnvPermissionAspect(
public EnforcePortalStudyEnvPermissionAspect(
AuthUtilService authUtilService, StudyEnvironmentService studyEnvironmentService) {
this.authUtilService = authUtilService;
this.studyEnvironmentService = studyEnvironmentService;
Expand Down
4 changes: 2 additions & 2 deletions ...seEnforcePortalStudyPermissionAspect.java → ...h/EnforcePortalStudyPermissionAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
@Component
@Aspect
@Slf4j
public class BaseEnforcePortalStudyPermissionAspect
public class EnforcePortalStudyPermissionAspect
extends BaseEnforcePermissionAspect<PortalStudyAuthContext, EnforcePortalStudyPermission> {
private final AuthUtilService authUtilService;

public BaseEnforcePortalStudyPermissionAspect(AuthUtilService authUtilService) {
public EnforcePortalStudyPermissionAspect(AuthUtilService authUtilService) {
this.authUtilService = authUtilService;
}

Expand Down
Loading
Loading