[CEP] Improve granularity of API key access controls

[czue]

Abstract

CommCare does not have any granularity in our user / role permission model to properly restrict API access. This CEP is to provide a mechanism to allow more fine-grained control over API access at the token level instead of relying on user-level controls only.

Motivation

Today CommCare has the notion of permissions at the user/project level. Users are provided access to projects (domains) and project administrators control the level of access granted to those users via the Roles and Permissions framework.

Additionally, users have API keys, which can be associated with a whitelisted set of IP addresses. API keys can also be deleted (revoked). API keys do not provide any granularity on top of the user's permissions. All API keys grant access to whatever the user can access.

For a third-party integration ecosystem it's important that we can properly scope API access to the most narrow set of rights as possible. External applications should be able to access the minimum set of data needed for those applications to function, and not all data available to the user.

Specification

This CEP seeks to add more fine-grained control over APIs.

The rest of this section documents permissions in the context of an API key, but the same should apply to an Oauth application, and any future externally integratable authentication mechanisms.

Types of access

It's unclear which set of these controls we will need. At a minimum we likely need Domain-level access. The specifics of which types will be added will depend on emerging requirements and feasibility.

Identity Access

Identity access would simply let users confirm they are who they say they are, and potentially provide a few details, for example email address and name. It would not provide access to any underlying data.

Identity access would primarily be used for external authentication. For example, using something like OpenID Connect.

Domain-Level Access

API keys should be able to be scoped to particular projects. For example, a user should be able to say for a particular API key that it can access their data from one project, but not from others. This will allow users to safely scope the data permissions provided to the project spaces they seek to integrate with.

Permission-Level Access

In this model you could hook into the Roles and Permissions framework to associate the API key with a particular set of permissions. For example, you could create an API key that only provided access to application data but not report data or individual cases/forms. Or you could create roles that can read data but not modify it.

API-Level Access (likely ruled out for the first version)

This would be the most fine-grained option. In it, you could grant access to specific APIs, as well as choose the type of access (e.g. read/write). Any APIs not explicitly included would not be allowed.

Modeling

Since at a minimum we would like to share functionality between API keys and Oauth grants, these models should extend from some shared interface. We'll call it a PermissionCollection.

In YAML, this model would look something like this (this uses the "permission-level access" paradigm mentioned above, just as an example):

name: role1
domain_permissions:
  domain1: 
    - view_apps
    - view_reports
    - edit_data
  domain2:
    - view_apps

In English, this would represent: the "role1" role has the "view_apps", "view_reports", and "edit_data" permissions on domain1, and the "view_apps" permission on "domain2".

Then both API Keys (via HQApiKey) and Oauth Grants (likely via a new model) would extend this interface.

Permission Checks

Permission checks would apply everywhere that API-based authentication is supported (which is basically just APIs). We would start by building this functionality for tastypie endpoints only

Our current tastypie implementation conflates authentication and authorization, e.g. RequirePermissionAuthentication actually includes authorization checks built into the authentication workflow.

For this work it is recommended to decouple authentication from authorization. So the authentication class would simply verify the user account and the (new) authorization class would check permissions. In order to maintain authorization at the API Key level, some state will have to be maintained about how the user authenticated. The details of this need to be ironed out, but it should be possible to save the data e.g. on the request object.

Impact on users
End users who make use of APIs will be able to have finer-grained control over how their API keys grant access than they currently do.

Impact on hosting
No anticipated impact.

Backwards compatibility
To ensure backwards compatibility all existing API keys must be granted "all" access either as a default behavior or via a data migration.

Release Timeline
No concrete timeline as of yet.

Open questions and issues

• What's the right type of access to provide?
• How do we handle API permission migrations if we make permissions more restrictive?

[czue]

Currently putting this up for review to get early feedback. I haven't dived deep enough into implementation to expose any potential issues with the stated plan.

A few potentially interested parties (feel free to bring in others) @snopoke @millerdev @calellowitz @ctsims @kaapstorm @esoergel

[snopoke]

I'm definitely in favour of being able to scope API keys so thanks for this CEP!

If API keys can only be used to access the APIs then why would they need permissions like view_apps?

At the moment all API keys are tied to specific users. Have you considered the option of having domain level keys that are tied to a domain instead of a user? My view on this is that project level integrations should make use of project level keys and user keys should be used for things that a specific user might want to do. This has often been an issue for us when we don't know who a key belongs to for some service (perhaps this should be a separate CEP).

I'm not sure about the API-level permissions. Having multiple different 'types' of permission seems like it could be confusing. e.g. If I did not have access to the "application API" for example (the API key did not have the application API access permission) I would not be able to access applications via the API even if I had the view_apps permission.
Is the issue with our current permissions that they don't map nicely to APIs? Or perhaps that they aren't applied to APIs very well?

[czue]

Thanks for the feedback!

If API keys can only be used to access the APIs then why would they need permissions like view_apps?

This may have been a bad example, but the idea was that in configuration (and code) you could tie it to the same permission classes we use for web roles. So e.g. if you had "view apps" then you'd be able to view the Application Structure API. I think this would allow us to leverage a lot of the code we currently have to map APIs to permissions.

At the moment all API keys are tied to specific users. Have you considered the option of having domain level keys that are tied to a domain instead of a user? My view on this is that project level integrations should make use of project level keys and user keys should be used for things that a specific user might want to do. This has often been an issue for us when we don't know who a key belongs to for some service (perhaps this should be a separate CEP).

I didn't think of this. However, I don't think it's a good fit for the use case I'm working on which is specifically tied to Oauth and user identity. But I do see the value and like the idea of it being a separate piece of work / CEP.

I'm not sure about the API-level permissions. Having multiple different 'types' of permission seems like it could be confusing. e.g. If I did not have access to the "application API" for example (the API key did not have the application API access permission) I would not be able to access applications via the API even if I had the view_apps permission.

I agree with this. Happy to scrap them as I think it does lead to confusion and unnecessary additional complexity.

Is the issue with our current permissions that they don't map nicely to APIs? Or perhaps that they aren't applied to APIs very well?

I think both of these may be true to some extent but probably more the latter (I haven't dug into this too much, but I have the sense no one has thought hard about how most APIs are scoped). I think we could address that as part of this work and it'd be good to improve so long as we can ensure backwards compatibility (possibly via a version bump if necessary).

[esoergel]

I'm definitely in favor of this sort of work.

If we expect this to be constrained solely to TastyPie APIs, I imagine that would significantly simplify the design. Do you think this should be designed with the idea that we'll eventually extend its reach to standard Django views as well?

I do also think it makes sense to re-use Permissions, if possible. It sounds like the main goal is for users to be able to associate API keys with a subset of their role's permissions. If we can associate an API key with a permission object, then somehow swap out permissions accessors to use that instead of the user's role's permission, I think the rest should just work fine. We'd have to be confident that the switch happens reliably, though.

My main concern with this would be future-proofing it. If it's easy to make an endpoint available with an API key, but you have to do something special to check the API key's scope in addition to the user's, it's likely we'll end up with holes. One way to approach this would be to make defining scope part of the process for making an endpoint available via API key. For example, if API access is provided by a decorator, the scope could be passed as an arg, and auth would only succeed if the permission is in scope to the API key:

@api_auth(edit_commcare_users=True)

One more big picture thought that's probably out of scope. I liked your framing of authentication vs authorization, and splitting them out makes sense for tastypie. It might similarly make sense to split them up elsewhere, and to attempt a unification of the places where they're each implemented. As I understand it, authentication happens partly in middleware, but sometimes in a decorator, and sometimes in a tastypie resource meta. Authorization is harder, as it often happens in the body of views too.

[snopoke]

I think both of these may be true to some extent but probably more the latter (I haven't dug into this too much, but I have the sense no one has thought hard about how most APIs are scoped). I think we could address that as part of this work and it'd be good to improve so long as we can ensure backwards compatibility (possibly via a version bump if necessary).

This makes sense to me but I do wonder about the backwards compatibility of it - if old version don't have permissions checks then it defeats the purpose of adding them. Perhaps a better approach would be to grandfather old API keys to have all permissions.

[czue]

This makes sense to me but I do wonder about the backwards compatibility of it - if old version don't have permissions checks then it defeats the purpose of adding them.

Hmm, yeah good point. We definitely can't make the permissions more strict and not update the old APIs or that's just wrong. Unfortunately it looks like the default is to require "access_apis" which appears to be True by default for all built-in roles.

Perhaps a better approach would be to grandfather old API keys to have all permissions.

Agreed we have to do this (it's in the "backwards compatibility" section of the original CEP). One thing I was thinking about was whether if we roll out a stricter role-based requirement and a user doesn't have that permission, their APIs would break. In that scenario it doesn't matter whether the key is scoped to all (their) permissions, and I don't think we should allow them to have a key with more permissions than they have access to.

Guessing we would just need to announce the change and be willing to break integrations if people don't update their roles? I don't really see any other viable alternative that doesn't make the security bit a joke - as you pointed out. If we wanted to be careful we could have a soft assert / notify workflow, though I suspect that most API connections are tied to admin users.

[czue]

If we expect this to be constrained solely to TastyPie APIs, I imagine that would significantly simplify the design. Do you think this should be designed with the idea that we'll eventually extend its reach to standard Django views as well?

Assume you mean Django API views using api_auth, yeah? I definitely agree we should support that - just wasn't sure whether it'd be easy to include in the first attempt at tackling this as I hadn't looked at it yet.

One way to approach this would be to make defining scope part of the process for making an endpoint available via API key. For example, if API access is provided by a decorator, the scope could be passed as an arg, and auth would only succeed if the permission is in scope to the API key:

@api_auth(edit_commcare_users=True)

I hadn't thought through the exact interface, but this sounds right to me and I think would go a long way towards "future-proofing". And generally I agree with your plan of swapping permissions at the object/check level but reusing the majority of the code.

One more big picture thought that's probably out of scope. I liked your framing of authentication vs authorization, and splitting them out makes sense for tastypie. It might similarly make sense to split them up elsewhere, and to attempt a unification of the places where they're each implemented. As I understand it, authentication happens partly in middleware, but sometimes in a decorator, and sometimes in a tastypie resource meta. Authorization is harder, as it often happens in the body of views too.

This does sound nice and also my guess is I'd just try to bite off what seemed natural and feasible through the course of doing the work but not aim to reconcile everything across HQ. So I guess I agree it's mostly out of scope 🙂

[czue]

I'm actually going to backpedal on the authentication / authorization issue. I think Tastypie is not set up at all to split them out if you're not using Django models/APIs and also I think having a successful "authentication" but an unsuccessful "authorization" actually does a fair amount of work - e.g. retrieving the objects from the DB.

[ctsims]

First off, thanks for tackling, Cory, this has been on our backlog for USH projects for a while and the scope of it kept knocking it back. I do agree that doing this in a useful way (A matrix of much more granular permissions) seems like the best approach, but it's a big lift to make that happen that we haven't been able to bite off.

I don't think we should tackle the "Project" wide API auth sets for now, it has a lot of awkward implications for the projects we've been managing, and the best practices for using project API keys v. PAT's seem like they could require even further deep changes.

@czue I shared a doc with you that we wrote up a couple months ago about a few general thoughts about API key improvements. You've probably already considered everything in it, but didn't want to overlook just in case.

One thing I think is clear from the way most tools I checked (Github, Sentry, Datadog, and a few others) use API key permissions is that they are very endpoint centric (IE: "Access Repositories -> Releases") so adding that as a permissions layer even if it's complementary to the existing more semantic Authorization permissions ("Data in Location X") seems consistent with other practices

[czue]

@czue I shared a doc with you that we wrote up a couple months ago about a few general thoughts about API key improvements. You've probably already considered everything in it, but didn't want to overlook just in case.

Wow, this would have been good to know existed prior to starting this effort! Though happily I think we reached a lot of similar conclusions which hopefully means they are reasonable ones. 🙂

There are a number of interesting points raised in that doc I hadn't thought of (e.g. using api keys as passwords, lifecycle management / cycling, etc.), which I think I'll mostly declare out of scope. But interesting to keep them in mind in case they affect implementation at all.

One thing I think is clear from the way most tools I checked (Github, Sentry, Datadog, and a few others) use API key permissions is that they are very endpoint centric (IE: "Access Repositories -> Releases") so adding that as a permissions layer even if it's complementary to the existing more semantic Authorization permissions ("Data in Location X") seems consistent with other practices

I can't tell if this is making a case for breaking out of the existing Roles/Permissions framework? Or adding a layer below it? I think it's a complicated choice, but likely the right long term answer is to build all this into the roles/permissions stuff, but not do that on the first pass, which I think is consistent with the conclusion in the doc.

Also pinging @orangejenny who I missed on the first pass but saw in the doc.

[orangejenny]

Backwards compatibility does seem tough. This approach seems right:

Guessing we would just need to announce the change and be willing to break integrations if people don't update their roles?

I also agree with @esoergel that we'd benefit from using the existing roles and permissions framework. I'd imagine API permissions would eventually get more granular than our existing permissions, possibly to the point of granting access to specific endpoints, but I think ideally that granularity would be an additional layer that uses most of the same code but isn't visible in the UI, rather than a totally separate framework. That'd hopefully let us share code, and it also seems better for users to only have one permissions model to comprehend.

[ctsims]

Guessing we would just need to announce the change and be willing to break integrations if people don't update their roles?

We could talk to the product managers, but honestly this feels off the table to me. We don't have super broad API usage, but API's are a critical point of functionality, and plenty of live impactful projects are in "technically headless" run and maintain mode and no longer have dedicated technical resources who can rebuild their APIs.

From my perspective, API's are among the most serious "won't change" aspects of a technical system. Our own technical teams are super frustrated anytime our SMS Gateways or random downstream providers change the API contract.

[czue]

@ctsims I agree with you in principle, but I think the scenario I'm talking about is more appropriately thought of as a security patch. The only scenario where we would need to introduce breaking changes is if we are exposing API data to users who do not have permissions to access the underlying data based on the stated behavior of the permission system. Weighing that security issue versus the "api contract" issue it's not clear to me that maintaining the contract is more important than maintaining the agreement with our clients that "the roles you give users accurately reflect what they can access/do in the system".

Also, this discussion may be moot since it's not clear that any such APIs exist today and I haven't personally done an audit/review.

Finally, I think we could roll out breaking changes with enough time and analytics such that we could make a very good faith effort to reach out to anyone who would break while keeping their APIs functioning for some period if time (e.g. by adding a soft assert any time someone accesses data they shouldn't have).

[ctsims]

Fair point about the permissions notion and that we don't quite have enough evidence of what kind of configurations may exist. There certainly might be cases where users clearly have more access than their current permissions should permit, and I shouldn't rule that possibility out.

I like the soft assert idea, that does seem like a good say to understand scope while also getting a clearer context on whether updates are occurring and we are reaching those teams with appropriate comms.

[czue]

Initial PR: #28420

[czue]

I'm working on the UI for this. Basically adding (optional) "domain" and "role" selection boxes to the API key UI. Does anyone want to make the case that those options shouldn't be generally available? Thinking I won't bother with a feature flag.

cc @dimagi/product