jupyter_server
jupyter_server copied to clipboard
Add utility methods to User class
I feel user_to_cookie and user_from_cookie should be a part of the User class.
This allows users to use an auth handler class (which isn't an IdentityProvider) to use these methods. In the current design they will have to copy it adding to the maintainence overhead
Thank you for the suggestion, I've made the required changes.
FWIW, this is a breaking change because IdentityProviders defining their own cookie storage will no longer have any effect, they will instead have to move their implementation to a custom User subclass.
Good point. I was too focused on the idea of moving serialization to the User class, which I still agree with in principle. However, as @minrk points out, we'd then need the ability to introduce User subclasses, and there is no such support for that (and the horse has left the barn).
we'd then need the ability to introduce User subclasses
We do actually have that, by extending get_user. Subclassing User is mentioned in the get_user docstring. JupyterHub relies on it.
Ultimately, I don't think is matters so much on which class the method lives, since it's the IdentityProvider that is responsible for selecting and calling it either way - it just means the User class and the IdentityProvider are even more deeply coupled.
If the IdentityProvider wants to customize how a logged-in user is persisted (the identity provider's responsibility), it must:
- override get_user to write fully custom cookie logic (what JupyterHub does, since its cookie logic is imported from its HubOAuth class), or
- define a custom User class with serialization methods, which in turn requires overriding
get_user_cookieandget_user_tokento return the new class beforeget_userpersists it in the default cookie.
So this change currently makes it a bit more work to change user serialization. That would be mitigated if .user_class were an attribute on IdentityProvider (it should not be configurable, as the User subclass is deeply coupled to the IdentityProvider implementation), because then just registering the class with the overridden methods would be able to use the default get_user logic. That said, it is likely those IdentityProvider methods need to be overridden anyway if there's custom serialization to be done (more or different fields to persist need to come from somewhere).
While it's a breaking change in the strictest sense (removal of an API), the only case I know of (JupyterHub) that modifies exactly this behavior (how a User is persisted in a cookie), does so at a higher level, so it's unaffected by the change here. Really only cases of custom user models that want to preserve default cookie persistence should be affected, and I'm not sure they exist yet.