Enable JS 2.0 (authenticated) extensions to work with classic notebook servers #1221
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1221 +/- ##
==========================================
+ Coverage 79.88% 80.46% +0.57%
==========================================
Files 69 69
Lines 8299 8301 +2
Branches 1607 1607
==========================================
+ Hits 6630 6679 +49
+ Misses 1226 1198 -28
+ Partials 443 424 -19
... and 6 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
|
||
| # If there is no identity provider set, load the default. If using | ||
| # a classic notebook server, adding extensions that inherit | ||
| # from JupyterHandler will use a mix of new+old authentication log. |
There was a problem hiding this comment.
| # from JupyterHandler will use a mix of new+old authentication log. | |
| # from JupyterHandler will use a mix of new+old authentication logic. |
jupyter_server/auth/identity.py
Outdated
| @@ -539,7 +540,8 @@ def process_login_form(self, handler: JupyterHandler) -> User | None: | |||
| return self.generate_anonymous_user(handler) | |||
|
|
|||
| if self.token and self.token == typed_password: | |||
| return self.user_for_token(typed_password) # type:ignore[attr-defined] | |||
| # type:ignore[attr-defined] | |||
There was a problem hiding this comment.
moved type annotation? Does this work when it's not on the assignment line?
There was a problem hiding this comment.
Weird. This was caused by my local linter for some reason. Let me update...
|
How related is this to #1245? |
Zsailer
force-pushed
the
binder
branch
2 times, most recently
from
a48f3c6
to
46e24b0
Compare
| config=self.settings.get("config", None), | ||
| # For backwards compatibility, pass the token | ||
| # from the webapp settings. | ||
| token=self.settings.get("token", "<generated>"), |
There was a problem hiding this comment.
If the token's not set, this will actually set the token to the value "<generated>" and accept requests authenticated with that string.
I believe this will also fix #1245 I think this implementation does need to change a bit, because it will result in simultaneous credential cookies for the two components, which can become out of sync. For example, logging out of a notebook will not log out of the new handlers. Instead, I think the LegacyIdentityProvider needs to work with both cookies, so it actually checks the original cookie (or calling the upstream LoginHandler.get_user) before looking in the new cookie for the model. That way, invalidation of the upstream cookie logs out of all endpoints. |
Fixes #1038
If a server extension inherits from
JupyterHandlerin Jupyter Server 2.0, it won't currently work with the classic notebook server (from jupyter/notebook) because they use different+competing authentication logic.Why? The extension looks for an identity provider that doesn't exist, so it creates a default one. Meanwhile, all other endpoints use the old/classic way of authenticating requests. The main reason this fails is that the classic auth flow and the identity provider over-write one another's login cookie because they are using the same cookie name by default. (Further, other issues arise if folks try to configure other settings on the login cookie...).
This PR allows both auth flows to coexist and ensures their settings+results are consistent. There are two key changes here
model-. I can't think of any particular reason why this would be problematic...cc @fcollonval