Separate participant and administrative account management

Here are the current dependencies between our accounts, authorization, and consent classes:

This makes it difficult to talk about changes to the management of accounts, even for something like authorization. Here is one aspirational model where we separate Synapse-managed accounts and participant accounts, so that all the business logic around participants is separated from our code to manage admin accounts. Basically “Account" would refer to an administrative account and "Participant" would refer to a participant account most places in our code. (An alternative is to create a new thing, like AdminAccount, for the admin accounts):

There would be backwards-incompatible consequences to this refactor. The /v3/participants APIs could no longer be used to create and manage administrative accounts. I don’t know who uses these for that purpose, at least the Bridge Study Manage would need to revise the UI it has to list all admins/users in one giant list (currently under the legacy panel). That API is still useful, but it would only return true participant accounts.

Notes on refactoring…

General

• Explicitly mark administrative accounts before we migrate roles, because right now that's the way we determine these kind of accounts. We don’t want administrative accounts to turn into participant accounts if someone (by mistake or otherwise) removes the roles from the account.

AccountDao & AccountSecretDao

• In the refactor above, a number of services can directly use AccountDao rather than AccountService. AccountDao and AccountSecretDao are basically fine as is, but we could write faster custom queries to return things like ID and health code than we are currently doing in the service classes.

ParticipantVersionService

• Should monitor changes to the Accounts table that are done through ParticipantService, not AccountService.

ConsentService, EnrollmentService

• AccountService -> AccountDao

AccountService & ParticipantService

• These two could use a common abstract parent class. For example, deleteAccount() and deleteParticipant() would share a lot of code in common. Other places they will be pretty different (e.g. only AccountService needs to enforce role change logic, only participant service needs to deal with enrollment scenarios, etc.)

AccountService

• move authenticate, changePassword, reauthenticate, deleteReauthToken, verifyChannel to the AuthenticationService.

• Event services should not be used in AccountService, they should be used in ParticipantService;

• ParticipantVersionService: should be used in ParticipantVersionService

• any attempt to manipulate participant accounts through this service should fail immediately with a bad request exception;

ParticipantService

• AccountService -> AccountDao

• Any code related to roles and/or organizations can be removed to AccountService;

• any attempt to manipulate admin accounts through this service should fail immediately with a bad request exception;

• updateIdentifiers: authenticate in the controller as part of the security check so you can take these auth methods out of AccountService and ParticipantService. Also this method is more likely useful on AccountService (arguably we can just allow administrators to change participant’s phone and email address).

Auth: AuthenticationService, AccountWorkflowService, OAuthProviderService

• These are all related and could be combined, except that it would make an unwieldy service. Maybe we could have a front controller that delegates to a set of implementation classes so external classes only deal with an AuthenticationService, but we're not maintaining a monolith.

• AccountService/ParticipantService → AccountDao in all three services

UserAdminService

• Fine as is (a crazy service to make integration testing palatable).

AccountController

• add paginated API to retrieve admin accounts in the app (across all organizations…organizations have a membership API that most of our external users should be using);

• access to these methods would require app:admin or system:superadmin powers (see below);

UserManagementController

• it may be simpler and easier to create enrollment records directly in the account for testing purposes. This would remove the consent service and the enrollment service references;

• the account service user delete should delete from all those other services, so we don't have to reference userAdminService to properly delete an account.

Expanded permissions model

We’re seeking a permissions model that will cover our current security capabilities while tackling new use cases, such as the ability to manage access to studies.

• The permission model will be permission-based, assigning a role vis-a-vis a specific target model (aka a “grant”);

• These new permissions will still be assigned to a user (not their assignment to an organization; this has important consequences for managing roles);

• Organizations will principally be a means to communicate who can see whom in a multi-tenanted application. Accounts will be assignable to multiple organizations. Migrated accounts will be given permissions to the sponsored studies of that organization, and then going forward, a user will at least have the auditor role for all studies sponsored by all of their organizations (this is the only transitively assigned role we’ve identified at this time);

• Bridge roles are hierarchical. Generally a user should have only one role vis-a-vis a model object. For example, being a study designer implies that you can read information about studies, like the auditor role. Every study designer does not also have to be an auditor.

• Currently an app-scoped developer, researcher, or admin can operate on any model in the app that requires that role (or its scoped counterpart, like study designer). WHAT SHALL WE DO ABOUT THIS?

Use Cases

Secured objects/scopes

Note that membership in an organization is also directly modeled in the database right now via the Account.orgMembership field, and will be moved to an associative table. We may not need a “member” role though it may be more convenient.

Implementation

We will introduce a flat table of Permission records that can be easily retrieved by user or by target model object:

public class Permission {
  String guid; // synthetic key makes create/add/update APIs easier
  String appId; // this always has to be part of the query
  String userId;
  String role; // "admin", "developer"
  String objectType; // "study", "organization", "app", "system"
  String objectId; // "studyId", "orgId", "appId"
  boolean transitive; // e.g. true if permission comes from org membership
  
  // Suggested toString() descriptor (implicitly scoped to an app):
  // "2rkp3nU7p8fjUTDVIgjT6T is an organization:sage-bionetworks admin"
}

The service (which we’ll probably access through Spring Security, see below):

interface PermissionsService {
  Set<Permission> getPermissionsForUser(String userId, boolean includeTransitive);
  Permission addPermission(Permission permission);
  void updatePermission(Permission permission);
  void removePermission(Permission permissions);
  Set<Permission> getPermissionsForObject(ObjectType type, String id);
  
  /** Spring security will need a very focused method to check, for a given user
    * and a given object, does the user have any of the required roles to perform
    * the request. This method can fudge things like app-scoped permissions, too.
    */
  boolean isAuthorized(AccountId accountId, ObjectType type, String objectId, Role... roles);
}

There will be top-level APIs to change permissions:

Spring Security

Spring security has nice support for annotation-based authorization constraints. I would suggest we switch to it and secure the system at a request level by annotating our controller methods. Spring provides an expression language we can use to declare our constraints, and we can even implement new methods in that constraint language, and Spring will delegate to our own code to answer authorization questions. But it would allow new developers to work with a technology that they have seen before and that is documented.

Using Spring security for authorization (not authentication, at least initially) we would do the following:

1. In a filter, create a caller's Authentication object and put it in Spring Security's SecurityContext (exactly like what we've been doing with our own RequestContext; we’d store the user’s ID and app ID);

2. Add authorization annotations to all of our controller methods.
   We can basically do our security checks in these annotations, e.g. @PreAuthorize("permit('developer', #studyId)") - permit a developer for the study ID (taken from the method’s parameters) to access the controller method. Because we can implement the “permissions” method, we can carry over our specific business logic. Later we can hook in other authorization systems very cleanly this way.

3. Remove our own static method call checks in AuthUtils. Eventually consider if we can remove RequestContext since it is 90% of the time being used to do authorization checks.

Migration

Existing roles can be expressed in the new permissions table in order to make the same kind of authorization checks. This can be done independently of allowing users to be in multiple organizations. For every administrative account in the system, we’d want to create entries based on their current roles:

The steps would be:

1. Add the permissions table, service, APIs, completely separate from existing security so they are completely functional;

2. Create bridge code so that organization memberships are mired in the permissions table (only needs to be one way);

3. Create bridge code so that role changes create the counterpart permissions (only needs to be one way);

4. Migrate all existing account roles to the new permissions tables;

5. Annotate our controllers with the new permissions;

6. Switch over external tools to use permissions apis rather than account APIs to manage permissions;

7. Remove roles and bridge code from accounts.

Multiple organization membership

Once we have a permissions table, we can implement accounts being in multiple organizations. The utility of this construct will be lessened (because people can be permitted to act directly against a study) but it may still be important for the future.