Skip to content
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

Proposal for API Authentication #1414

Open
ssddanbrown opened this issue May 5, 2019 · 6 comments

Comments

@ssddanbrown
Copy link
Member

commented May 5, 2019

With the creation of an API upcoming on the roadmap, I thought It'd be good to start planning out the authentication method for the API. I welcome advice or concerns on this, since the authentication is something I want to get right. These are my initial thoughts:

Proposal

  • A new system 'API Access' role permission is made available for admins to assign to roles.
  • Users within a role that has 'API Access' will be able to generate API tokens in their profile.
    • Tokens will be generated as a random 32 char string, bcrypted and saved to DB, The raw token will be passed back as plaintext to the user on generation, a single time only.
    • On token creation, Users will be able to provide a name to indicate usage.
    • On token creation, you will be able to set an expiry date, after which the token becomes invalid.
    • A user will be able generate multiple tokens if they wish.
    • Tokens will show their generation date.
    • Tokens can be deleted at any time.
  • Tokens will be used via a Authorization: Token <token_val> header.
  • API access will inherit the permissions of the user the token has been generated against. If a tighter permission scope is needed a new 'API-specific' user could be created with more selective permissions.
  • Users with a 'Manage Users' permission but lacking 'API Access' permission will be able to edit/delete existing tokens but not generate new tokens.
  • Users with both 'Manage Users' and 'API Access' permission will be able to generate tokens for others in addition to edit and delete existing tokens.
  • On each API request, the token expiry will be checked. If expired an error response will be returned right away with a message or header that indicates token expiry.

Implementation Considerations

These are non-proposal items we'll need to cover with tests on implementation

  • The user's 'API Access' permission should be checked on each request instead of inferring they should have the permission by having a token, since roles could dynamically change.

Questionables

  • Should someone with just a 'Manage Users' permission be able to manage someone else's tokens or should they also require 'API Access'? I'm thinking the latter.
  • Would it be worth displaying a "Last Used" date against the token? Would require an extra DB write per request, but might be worth it to identify active API usage.
@TimmKroe

This comment has been minimized.

Copy link

commented May 6, 2019

Should someone with just a 'Manage Users' permission be able to manage someone else's tokens or should they also require 'API Access'? I'm thinking the latter.

In my opinion, if a "manager" can manage the users he should also be allowed to manage the api tokens. E.g. your sysadmin couldn't reset your password.

Would it be worth displaying a "Last Used" date against the token? Would require an extra DB write per request, but might be worth it to identify active API usage.

It would be nice to have api activity logged, e.g. for troubleshooting. But on the other side, I think that these additional queries would take to much performance on large instances with high api requests. (not sure about that)

@cw1998

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

API access will inherit the permissions of the user the token has been generated against. If a tighter permission scope is needed a new 'API-specific' user could be created with more selective permissions.

My opinion on this:
I would be more for something as you have described here around API-specific users.

An example use-case would be an automated task or bot that shouldn't be able to delete anything as a safety precaution, but the parent account should still be able to delete pages/drafts etc. through the BookStack interface.

  • Tokens will show their generation date.
  • Tokens can be deleted at any time.

Can tokens also expire if desired? This can often promote good technical hygiene - rotating tokens every so often. This would be an extra date with a token record in the DB and an extra check when the API call is being handled.

@cw1998

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

Should someone with just a 'Manage Users' permission be able to manage someone else's tokens or should they also require 'API Access'? I'm thinking the latter.

I second @TimmKroe 's comment on this. A user who manages users, but does not have API access themselves will not be able to see the keys, so it would be fine to grant them full access to that user as the role describes.

Although I think I see your thinking in saying the latter. A user manager should probably not be able to create a token for another user and see that token for the one time it would appear. Perhaps they can only edit the token's purpose/description and revoke it - leaving out the create token aspect.

@ssddanbrown

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

Thanks @TimmKroe and @cw1998, I completely agree, a 'Manage Users' user should have the ability to edit/delete existing tokens but not create unless they have the 'API Access' permission. I have updated the proposal to reflect this.

@cw1998 I agree token expiry would be worthwhile, especially since no extra DB query would need to be done to check this. Have added to the proposal.

@cw1998

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

Would it be worth displaying a "Last Used" date against the token? Would require an extra DB write per request, but might be worth it to identify active API usage.

I agree here. An audit trail for API usage would be good.

Most activity is already logged. Maybe not ideal, but if we wanted to keep the number of DB writes down, then we could add another column to the activity log table with the API token that made the change (if applicable), then it will be pretty trivial to work out the last used date for a given API token from there.

@TimmKroe

This comment has been minimized.

Copy link

commented May 11, 2019

I agree to @cw1998 an audit log for the API usage would be nice to have. But what I don't know atm is if those extra queries/writes would take up much performance compared to not doing it. If it won't be performance hungry I would definitely implement it!

Also @ssddanbrown how can I help implementing an API into bookstack, because I didn't find any open issue regarding this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.