RFC: Access control rules do not support separate rules for the same action

[taneliang]

Example

static accessControlRules(police: Police<UserQuery, User>) {
  police

    // Only allow a user to create/update/delete their own record. This works.
    .onCreateUpdateDelete((police) => {
      return police.denyIfUnauthenticated().allowWithRestrictedGraphView((vc, query) => {
        if (vc instanceof UserViewerContext) {
          query.whereIdEquals(vc.user.id);
        } else {
          throw new Error('Only UserViewerContext supported!');
        }
        return query;
      });
    })

    // Expectation: For every user record:
    // 1. The user should be able to see all their own fields.
    // 2. Others can only see an allowed list of fields (e.g. email,
    //    username).
    .onRead((police) => {
      // Reality: like the create/update/delete case above, we can only allow
      // users to see all fields of their own record, OR allow everyone to see
      // that allowed list of fields for all records. We can't have a
      // combination of both.
      return police.allowWithRestrictedGraphView((vc, query) => {
        query.dangerouslyBuildKnexQueryBuilder((kqb) =>
          kqb.clearSelect().select('email,username'),
        );
        return query;
      });
    });
}

Possible implementations

• Implement post-query access control rules to filter out unauthorized information.

  • Pro: should be really simple to implement and understand.
  • Cons: we'll unnecessarily fetch information before dumping them, and we need to be careful to remove the unauthorized information from all data accesses and caches (or prevent that info from being cached).
  • Facebook seems to be doing this approach with a checkCanSee method, so confidence level ↗️. Data is fetched and dumped before being cached by loader. Source: https://youtu.be/etax3aEe2dA?t=779, but as it's a public talk it may be overly simplified; I find it hard to believe that they'll fetch and throw away data on humongous data sets like comments/photos/statuses, but in Gent we mitigate this with restricted subgraph queries. Proposed API:

   class Police<QueryType extends GentQuery<Model>, Model extends GentModel> {
     ...
     allowWithRestrictedGraphView(
       queryBuilder: (vc: ViewerContext, query: QueryType) => QueryType,
       postQueryTransform?: (vc: ViewerContext, fetchedModel: Model) => Model | undefined = undefined
     ): this;
     ...
   }

   return police.allowWithRestrictedGraphView(
     (vc, query) => query,
     (vc, fetchedModel) =>
       fetchedModel.ownerId === vc.user.id
         ? fetchedModel
         : _.pick(fetchedModel, ['id', 'email', 'username']),
   );

  • Problem with the above proposal: Model may not be compatible with the transformed type, e.g. we may require name to be non-nullable in the database, but we may not want to expose name to end users. Possible solutions:
    • Just set non-nullable fields to some default value instead of outright nullifying them. Non-nullable to-one edges will still be a problem as we can't really put a default edge for these. Solutions:
      • Require all edges to be nullable (which currently makes both nullable in SQL and TS). This may be the ideal solution for a system with multiple backends, as it may be impossible to actually enforce any non-nullability. But of course, if we're backed by SQL we'll lose our ability to enforce that relationships exist. For this, we could decouple SQL and TypeScript edge nullability.
      • Define a default edge value. This seems messy.
    • Decouple database nullability from TypeScript class property nullability. This will allow us to enforce non-nullability on the database while still ensuring that the runtime model classes are typed correctly.
    • Allow Police to return another OutputModel type. This may be problematic if we still want Police to sometimes output the original type; e.g. we may want to output both User and OtherUser types. However, this approach may work if both types are compatible, or if we decouple the storage model type from the business layer's type (and eventually the exposed API type).
    • Just #wontfix

• Somehow implement intent-based access control rules, e.g. getting a list of users' info vs getting one user's info.

  • Con: I suspect this will get unmanageable quickly, since the schema will now need to be aware of actions that can be performed on the entity.

• Somehow implement query splitting, e.g. splitting the original query for all users into 2 queries, one with the query SELECT <authorized fields>... and the other SELECT * ... WHERE id=....

  • Con: additional query needed, design is unclear