[WIP] Remove tokens from Database #83
Conversation
- add files for cookie and database secrets - store cookie secret on disk, instead of in database - encrypt auth tokens with EncryptedType
- use PasswordType - store first 4 bytes for filtering by prefix since we can't filter by equality on the hashed value. - user.new_foo_token() returns token string, not ORM object
it's not needed - new tokens are created when spawners start
removes last need for encrypted database fields, so db_secret is removed as well.
|
Trying to work through the implications of this:
Is that right, so far? |
Yes. I don't know if there's an easy way to have separate secrets for API vs Cookie Tokens.
Currently, yes. Working on a change that would make it possible to revoke tokens for a just a specific user. Basically the idea is to do user lookups by a unique id rather than the user's name. Then, changing that unique id is enough to invalidate all tokens for that user.
Yes, with the same caveat as above.
Yes. I don't think there's a straightforward way around that with this design. |
|
1df0abd, coupled with some sort of cache-invalidation for the SU Server, should make per-user access revocation possible. If the user's auth_id is changed, then _user_from_auth_id will return |
|
I had GitHub's API in mind when putting together the token stuff, where one can create tokens for access to the API. The reason being that one could create tokens for external services that monitor and amend behavior of the Hub (e.g. shutting down idle servers, which I don't plan to implement in the Hub itself). I don't really want to force everything that a user has granted access to the API to share the same token. In fact, I had planned to take the Token stuff further, adding auth scopes, etc. I do like the simplicity here, I'm just trying to figure out if I'm okay with losing the planned functionality. Especially since I wasn't using it, yet. |
|
@ssanderson here's a use case: Can you 'log out' a user by invalidating their cookies without forcing their server to restart because the server's API token has also been invalidated? |
|
Since I'm not sure that we'll be able to get away with no encrypted data in the db forever, I'd actually prefer to leave those digressions in the git history, even if we don't use them. |
|
My current leaning is that getting rid of cookie tokens is good, but I'm not so sure about getting rid of API tokens. I'll keep thinking about that. |
You couldn't do that with what's currently implemented, but it would be trivial to add a new unique key to |
|
|
||
| PASSWORD_SCHEMES = ['pbkdf2_sha512'] |
There was a problem hiding this comment.
This is left over from the rebase from #82.
There was a problem hiding this comment.
Also, for my own ref while reviewing, a security stackexhange post with some discussion of HMAC and PBKDF2.
I think that's the way it will end up going. |
|
Ok, I should probably review this for the forest instead of the trees since this is currently a WIP. @minrk has thoroughly convinced me of why we should keep API tokens (but not the cookie tokens), to enable a way to authorize applications to communicate with JupyterHub. This helps us in that:
I'll have to come back to this later after getting some more time to dig in. |
Restructure introduction and demark divisions of interest
Alternative implementation of #81 for discussion in #80.
Removes the APIToken and CookieToken fields from the ORM in favor of relying on Tornado's cookie encryption as the source
of truth for authentication.
This is currently rebased off of @minrk's work on #81 to steal his refactoring of the cookie_secret and proxy auth token handling. If we decide to go this route, it should be rebased some more to not include the detour through DIY encryption-land.