Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

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

Extend user API key capabilities #5509

Closed
asmecher opened this issue Feb 13, 2020 · 3 comments
Closed

Extend user API key capabilities #5509

asmecher opened this issue Feb 13, 2020 · 3 comments

Comments

@asmecher
Copy link
Member

Extend user API key capabilities to support

  • multiple API keys per user
  • varying levels of access per API key

This will make it safer to grant API keys from high-level accounts like editors/managers, and to avoid reuse of a single API key for multiple purposes.

From the document "Beacon Data Requirements":

Proposal: Permit multiple API keys per user account, with tools to limit rights.

  • Create new table: api_keys
    • api_key_id
    • symbolic (a symbolic name for this API key, e.g. “beacon”)
    • description (free-form text, no need for this to be multilingual)
    • date_created
    • key (the actual API key)
    • is_setuid (boolean: true if this API key effectively grants all privileges for the user)
  • Create a new table: api_key_rights
    • api_key_id (foreign key)
    • api_key_right (a symbolic string that specifies what the key allows, e.g. “describe_plugins” to describe the installed plugins in OJS

When a feature is used that requires an API key, it can guide the creation of an appropriate API key. For example, enabling the beacon creates an API key with any of the desired rights.
In the “API Key” area of the user’s profile, add tools to administer these.
Add endpoints to the API for the kinds of data the beacon wants to expose, but without explicitly coding them around the beacon concept. For example, if the user chooses to expose readership stats through the beacon, the beacon simply identifies the API endpoint URL that makes readership stats available in a general API-ish way.

@asmecher
Copy link
Member Author

@NateWr, FYI. I'm content to start working towards multiple API keys per user -- that's clear enough -- but the api_key_rights approach that assigns capabilities to API keys needs more consideration.

At a basic level we could assign role IDs to API keys and use our existing permissions capabilities, but I suspect longer-term that this may not be flexible/fine-grained enough. For example, permitting an editor role wholesale is way too powerful/risky for most API interactions.

Another option would be to explicitly align API key capabilities with API endpoints -- to allow for endpoints to be explicitly named and allowed.

@NateWr
Copy link
Contributor

NateWr commented Feb 13, 2020

Sounds good. I think this goes back to discussions we had a long time ago about distinguishing roles and permissions. Access could be granted based on permissions and roles could be assigned different sets of permissions. This would be useful more generally for things like #5504 and other similar requests.

In the past, I think you expressed concern that this would cause compounding complexity in the authorization system, which is already challenging enough to work with. It would certainly constitute a major refactor, but it's one that I think will have to come some day -- whether or not this is that day.

Another option would be to explicitly align API key capabilities with API endpoints -- to allow for endpoints to be explicitly named and allowed.

I don't think this will work as intended because many endpoints respond differently to different roles. For example, the /submissions endpoint returns different things depending on what submissions you have access to and what your assignment is on each submission.

If we don't go in for the wholesale refactor of permissions/access, I'd suggest thinking through what particular use-cases we think are needed for access in the API keys. In my experience with third-party services, there is often not a lot of fine-grained control over API key access. It's tied to a user account and given permissions based on that account. API keys are usually given out to trusted parties.

If we think there are some key permissions levels that we need to disambiguate, we may be able to do that without rethinking how permissions are granted. For example, if this is really driven by the beacon's need to access stats and get a list of plugins, but not other manager responsibilities, we could create a new role under the hood, that's never surfaced to the UI (like ROLE_ID_SITE_ADMIN), and grant that role access to the appropriate endpoints.

The database table spec you set out looks good. A couple of suggestions:

  1. Add a created_by_user_id or just user_id column describing which user created the API key.
  2. What would other symbolic values be? Would this record maybe how the key was created? beacon, user, some-other-plugin-name? It's not clear to me what we're capturing here that wouldn't be just as well served by the description.
  3. What do you think about renaming is_setuid to is_user_id or is_set_user_id? I read uid as "unique id".
  4. Do we want a logging system? This could be useful to confirm that an API key is not being misused, or that there isn't a wayward test app running daily requests from three years ago that shouldn't be running any more and has permissions that are dangerous. At the very least, we should be logging the time and source of a key's last API request to help identify which API keys are still being used.

@asmecher
Copy link
Member Author

asmecher commented Nov 2, 2021

Re: limited access to a user's capabilities, see also: #7424 (Add support for "log in as" that is scoped to the context)

@pkp pkp locked and limited conversation to collaborators Aug 18, 2022
@asmecher asmecher converted this issue into discussion #8185 Aug 18, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants