Skip to end of metadata
Go to start of metadata

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 29 Next »

In adding Mobile Toolbox to Bridge, we’re encountering more complex authorization. We have roles like an organization administrator, who can create users who are members of their organization (but who cannot see any other kind of accounts, including study participants in their organization’s studies), and study designers, who can edit some of the things that a developer can edit…but only in the context of the studies they have access to.

Right now these checks are spread out:

  1. Controllers usually check for roles and consent;

  2. AuthUtils methods are called in the services to check the relationship of the caller to entities like studies and organizations.

This is getting messy. The issues I feel I’m encountering

  1. Security is defined in different places, there’s not one place to see what is and is not permissible;

  2. Utility methods are difficult to compose into new requirements and despite my best attempts, the names of these things rename confusing at times;

  3. Consequently it’s hard to say we don’t have lapses in the authorization checks that are occurring.

I think we could use a more robust alternative to implement this. But first, here are the authorization checks we have implemented or want to implement in the MTB timeframe (described in terms of access to objects in the REST API, rather than through the several endpoints that are needed to expose each object in the API itself, and skipping participant-facing APIs). Then as well look at alternatives for implementation, I’ll show what modeling enrollment would look like.

Authorization Rules

Object

Assoc(1)

Role

Permissions(2)

AccountSummary

global (but filtered)

researcher

read

Study (“participant”)

researcher

read

Organization (“member”)

org admin

read

App

all

read

dev, admin

update

superadmin

create, delete

AppConfig

public

read (filtered)

dev

create, read, write, delete

AppConfigElement

dev

create, read, write, delete

Assessment

Organization (“owner”)

dev

create, read, write

admin

delete

AssessmentConfig

public

read

Organization (“owner”)

dev

write

Enrollment(Detail)

Account (“self”)

any

create, read, delete

Study

researcher

create, read, delete

Study

admin

create, read, delete

AssessmentResource

Organization (“owner”)

developer

create, read, delete

admin

delete

FileMetadata/Revision

developer

create, read, write

admin

delete

HealthDataRecord(Ex3)

participant

create

worker

write

MasterScheduleConfig

superadmin

create, read, write, delete

NotificationMessage

self, admin, researcher

create

NotificationRegistration

self, researcher, admin

read

NotificationTopic

developer

create, read, write

admin

delete

Organization

any

read

Account (“membership”)

org_admin

write

admin

create, delete

RecordExportStatusRequest

worker

write

ReportData

Study

any/public

read

Study

dev, worker

create, delete

ReportIndex

Study

any

read

Study

dev

create, write

RequestInfo

Account (“self”)

RequestInfo

Study

researcher

read

admin, worker

read

SchedulePlan

developer, researcher, worker

read

developer

create, write

admin

delete

SmsTemplate

Account

worker

create

Study

org_admin

create

Organization (“sponsors”)

org_admin

create, read, delete

StudyConsent

dev

create, read

StudyParticipant

Study (“enrolled”)

researcher

create, read, write

worker

read

Participant (“self”)

read, write

admin

create

Subpopulation

dev

create, read, write

researcher

read

admin

delete

Survey

dev, researcher, worker

read

dev

create, write

admin

delete

Tag

public

read

superadmin

create, delete

Template/TemplateRevision

dev

create, read, write

admin

delete

Upload

dev, admin, worker

read

UploadSchema

dev

create, read, write

admin

delete

(1) = association to another model object. This typically means an additional check to ensure is a member of an organization, or has access to a study through their organization, or is the target of the call as well as the caller, and so forth. If blank, then the association is to an app, because everything is scoped/tenanted to an app.

(2) C = create/write, R = read (list or detail object), U = update/write/delete logically, D = delete physically.

Here are objects from the v2 domain model that have been designed far enough to think about permissions (designer = STUDY_DESIGNER):

Object

Assoc

Role

Permissions

Protocol/Study Arms

Organization (“owner”)

designer, developer

create, read, write

Study

designer, developer

read

admin

delete

Schedule

Study (“owner”)

designer, developer

create, read, write

admin

delete

Study Arm Schedule

Organization (“owner”)

designer, developer

create, read, delete

Spring Security

This is the most complex option available, but we could use the method-based security via annotations. Our service methods could declare the security using complex expression rules.

Modeling EnrollmentService, it would look like this (ignoring the work necessary to make it work):

public class EnrollmentService {

    @Secure("principal.id == userId or hasRole('ADMIN') or " + 
      "(hasRole('RESEARCHER') and hasOrgSponsoredStudy(studyId))")
    public PagedResourceList<EnrollmentDetail> getEnrollments(...) {
    }
    
    @Secure("principal.id == userId or hasRole('ADMIN') or " + 
      "(hasRole('RESEARCHER') and hasOrgSponsoredStudy(studyId))")
    public Enrollment enroll(...) {
    }
    
    @Secure("principal.id == userId or hasRole('ADMIN') or " + 
      "(hasRole('RESEARCHER') and hasOrgSponsoredStudy(studyId))")
    public void updateEnrollment(...) {
    }
    
    @Secure("principal.id == userId or hasRole('ADMIN') or " + 
      "(hasRole('RESEARCHER') and hasOrgSponsoredStudy(studyId))")
    public Enrollment unenroll(...) {
    }
}

Pros

  1. It’s a standardized thing so other developers should have an easier time working with it;

  2. Ultimately, security would be expressed in annotations, which seems nice;

  3. One step closer to implementing a major change to something like storing access rights in a database (so far we haven’t seen anything in Bridge that requires that level of authorization, but)

Cons

  1. Our existing authentication and authorization have all been written from scratch, which means we’d need to overwrite all the core Spring Security classes before any of this would work;

  2. The result would take time and involve learning a lot more about Spring Security;

  3. Security exceptions would appear in places where we are now able to return InvalidEntityExceptions and the like. For example empty/null/imaginary values are going to throw security exceptions. (I’m fine with this).

Implement a DSL

Nothing Spring does is that hard to duplicate with a DSL that uses our existing RequestContext. I think it makes sense to call these in the services, where multiple endpoints might create multiple conditions under which a call is acceptable. It could look something like this:

public class EnrollmentService {
    private static final AuthEvaluator SELF_ADMIN_OR_STUDY_RESEARCHER = 
        AuthUtils.canAccessStudy().inRole(RESEARCHER).or()
            .inAnyRole(ADMIN, SUPERADMIN).or()
            isSelf();

    public PagedResourceList<EnrollmentDetail> getEnrollments(...) {
        SELF_ADMIN_OR_STUDY_RESEARCHER.checkAndThrow("studyId", studyId, "userId", userId);
    }
    
    public Enrollment enroll(...) {
        SELF_ADMIN_OR_STUDY_RESEARCHER.checkAndThrow("studyId", studyId, "userId", userId);
    }
    
    public void updateEnrollment(...) {
        SELF_ADMIN_OR_STUDY_RESEARCHER.checkAndThrow("studyId", studyId, "userId", userId);
    }
    
    public Enrollment unenroll(...) {
        SELF_ADMIN_OR_STUDY_RESEARCHER.checkAndThrow("studyId", studyId, "userId", userId);
    }
}

Pros:

  1. Easier to implement and understand at this point, when compared with overriding Spring Security’s implementation classes

  2. Arguably, easier to understand because it’ll only contain what it necessary for our application (as opposed to Spring which is always more complicated because it can handle anything, including future requirements).

Cons:

  1. More custom code for new developers to learn (assuming they already know Spring security which is more portable for them anyway);

  2. We’re bearing down further on a custom implementation making that much harder later if we decide to abandon it for anything else. If we’re going to switch, better to switch sooner.

Recommit to implementing authorization in the controllers

Arguably we can continue to use authorization in the controllers if we just create more endpoints that are tailored to our specific authorization scenarios.

Take the example of AuthUtils.checkSelfStudyResearcherOrAdmin check for enrollments:

public class EnrollmentService {

    @GetMapping("/v5/studies/{studyId}/enrollments")
    public PagedResourceList<EnrollmentDetail> getEnrollments(...) {
        UserSession session = getAuthenticatedSession(RESEARCHER, ADMIN);

        if (!session.isInRole(ADMIN) && !AuthUtils.canAccessStudy(studyId)) {
          throw new UnauthorizedException();
        }
    }
    
    @GetMapping("/v3/participants/self/enrollments")
    public PagedResourceList<EnrollmentDetail> getSelfEnrollments(...) {
        UserSession session = getAuthenticatedSession();
        // set userId to the caller's user Id
    }

    @PostMapping("/v5/studies/{studyId}/enrollments")
    public Enrollment enroll(...) {
        UserSession session = getAuthenticatedSession(RESEARCHER, ADMIN);

        if (!session.isInRole(ADMIN) && !AuthUtils.canAccessStudy(studyId)) {
          throw new UnauthorizedException();
        }
    }

    @PostMapping("/v3/participants/self/enrollments")
    public Enrollment enrollSelf(...) {
        UserSession session = getAuthenticatedSession();
        // set userId to the caller's user Id
    }

    @PostMapping("/v5/studies/{studyId}/enrollments/{userId}")
    public void updateEnrollment(...) {
        UserSession session = getAuthenticatedSession(RESEARCHER, ADMIN);

        if (!session.isInRole(ADMIN) && !AuthUtils.canAccessStudy(studyId)) {
          throw new UnauthorizedException();
        }
    }
    
    @DeleteMapping("/v5/studies/{studyId}/enrollments/{userId}")
    public Enrollment unenroll(...) {
        UserSession session = getAuthenticatedSession(RESEARCHER, ADMIN);

        if (!session.isInRole(ADMIN) && !AuthUtils.canAccessStudy(studyId)) {
          throw new UnauthorizedException();
        }
    }

    @DeleteMapping("/v3/participants/self/enrollments/{userId}")
    public Enrollment unenrollSelf(...) {
        UserSession session = getAuthenticatedSession();
        // set userId to the caller's user Id
    }
}

Another example are org administrators who can list, read, create, and delete administrative accounts in their organizations:

// Instead of the /v3/participants APIs:

GET /v1/organizations/{orgId}/members
POST /v1/organizations/{orgId}/members
GET /v1/organizations/{orgId}/members/{userId}
DELETE /v1/organizations/{orgId}/members/{userId}

Pros:

  1. Consistent with what we’ve done and thus the least code churn;

  2. Possibly easier for external users to understand our APIs;

  3. Possibly easier to secure down the road with Spring Security (using URL-based security rather than method-based security);

Cons

  1. Honestly I was trying to eliminate keywords like “self” that make more endpoints…that’s more to add to the SDK, more to test, more to document, etc. In the second example, all those APIs are redundant with the participant APIs except for the security differences. Putting all the security within the system is easier;

  2. Some security checks might get tedious. You might still want a DSL class even if doing this work in the controller. The delete user is a good example:

    boolean orgAdminDelete = context.isInRole(ORG_ADMIN) && !account.getRoles().isEmpty() && 
            context.getCallerOrgMembership().equals(account.getOrgMembership());
    boolean testDelete = AuthUtils.isSelfResearcherOrAdmin(context.getCallerUserId()) &&
                account.getDataGroups().contains(TEST_USER_GROUP);
    boolean adminDelete = context.isInRole(ADMIN);
    
    if (!orgAdminDelete && !testDelete && !adminDelete) {
        throw new UnauthorizedException();
    }
  • No labels