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

Run Gateway token renewers even if the auth token is empty. #1340

Merged

Conversation

ojarjur
Copy link
Contributor

@ojarjur ojarjur commented Oct 20, 2023

Prior to the 2.8.0 release, Gateway token renewers would be run regardless of the value of the initial auth token configured with the Gateway client.

However, that initial value could potentially be None, and token renewers are not required to support that as a value for their auth_token parameter. This meant that token renewers might be called with a value they do not support.

The 2.8.0 release fixed that issue by only calling token renewers if the auth_token value was truthy. However, this introduced a regression because the default value for auth tokens is the empty string, which is not truthy but which is a valid string value (and, as such, is expected to be supported by token renewers).

This change fixes that by calling token renewers as long as the auth_token value is not None. This is a compromise between the pre-2.8.0 behavior and the 2.8.0 behavior, and is believed to be the least disruptive option.

This fixes #1339

ojarjur and others added 2 commits October 19, 2023 17:03
Prior to the `2.8.0` release, Gateway token renewers would be run
regardless of the value of the initial auth token configured with
the Gateway client.

However, that initial value could potentially be `None`, and token
renewers are not required to support that as a value for their
`auth_token` parameter. This meant that token renewers might be
called with a value they do not support.

The `2.8.0` release fixed that issue by only calling token renewers
if the `auth_token` value was truthy. However, this introduced a
regression because the default value for auth tokens is the empty
string, which is not truthy but which *is* a valid string value
(and, as such, is expected to be supported by token renewers).

This change fixes that by calling token renewers as long as the
`auth_token` value is not `None`. This is a compromise between
the pre-2.8.0 behavior and the 2.8.0 behavior, and is believed
to be the least disruptive option.

This fixes jupyter-server#1339
@ojarjur
Copy link
Contributor Author

ojarjur commented Oct 20, 2023

@Zsailer Here's the PR that we discussed at today's community meeting.

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ojarjur - These changes look good (and straightforward) to me. Thank you for dealing with this!

@blink1073 blink1073 merged commit 393c108 into jupyter-server:main Oct 23, 2023
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GatewayTokenRenewer classes stopped being called with the 2.8.0 release of the jupyter_server package.
3 participants