mirror of
https://github.com/BookStackApp/BookStack.git
synced 2026-02-05 00:29:48 +03:00
Proposal for API Authentication #1163
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @ssddanbrown on GitHub (May 5, 2019).
Originally assigned to: @ssddanbrown on GitHub.
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
Authorization: Token <token_val>header.Implementation Considerations
These are non-proposal items we'll need to cover with tests on implementation
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.@timmkroe commented on GitHub (May 6, 2019):
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.
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)
@cnfw commented on GitHub (May 7, 2019):
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.
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.
@cnfw commented on GitHub (May 7, 2019):
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 commented on GitHub (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.
@cnfw commented on GitHub (May 10, 2019):
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 commented on GitHub (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?
@DeftNerd commented on GitHub (Jul 3, 2019):
Laravel has a method of creating signed routes (with optional expirations) using HMAC that might provide some inspiration.
You could issue tokens with a signature. Clients could pass both the token and the Signature in the same Authentication header.
Authentication: Token deadb33f01234abcd:103836719When passed to the server, you can verify the token is signed correctly, and isn't expired before even hitting the database to verify the token is still enabled.
In theory, the token wouldn't even ever have to be saved to the DB, but then it would be impossible to invalidate a token once issued.
@Lev-Zabudko commented on GitHub (Jul 26, 2019):
“Last used” can be pretty good, but you can add it later if you really need it. Now it is much easier to log to the file.
By the way, we really want to use bookstack instead of xwiki, but we need API to put content to bookstack from CI. Pls, do some API implementation for books and shelves. As MVP we may use basic auth.
Thanks!
@ssddanbrown commented on GitHub (Dec 30, 2019):
Scary how fast time moves, Can't believe it was in May that I opened this.
Anyway, Thanks everyone for your input. The proposal has now been implemented as part of #1826. Seems to work nicely as a start. No audit or "Last used" for now but it's something I'll probably look to add down the line. Think I need to take a step back and think about audit & reporting at a wider scale.
An unexpectedly tricky part of this implementation has been ensuring the API can viewed in the browser using the standard session auth, in addition to the propose token auth. Was not mandatory but I found it makes things so much easier if I can quickly navigate an API in the browser without having to set up auth etc..
Have left some cookie-based middleware on the API routes and created a light-wrapper around the session middleware to only boot up the Laravel session system if a session cookie is found on the request. Should allow the intended behaviour without much of a performance hit.
Since this is in #1826 I'll close this off, Thank again everyone.