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

JWT auth support #442

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

JWT auth support #442

wants to merge 4 commits into from

Conversation

slvrtrn
Copy link
Contributor

@slvrtrn slvrtrn commented Dec 20, 2024

Summary

This PR adds support for JWT authentication (ClickHouse Cloud feature).

  • Add JWT to the client configuration
  • Allow to override the token

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@mshustov mshustov requested a review from genzgd December 20, 2024 16:06
@slvrtrn
Copy link
Contributor Author

slvrtrn commented Dec 20, 2024

As JWT expires over time, we need a way to override it in the client.

@genzgd, perhaps a set_access_token method on the client instance could be a good idea? An alternative is to add authorization override on all the public methods, which is probably too much and won't be too convenient to use. WDYT?

@genzgd
Copy link
Collaborator

genzgd commented Dec 20, 2024

set_access_token makes sense. If we wanted to get fancy we could make the access token constructor parameter take either a string or a function/hook that returns a string so the user could use their own implementation for refreshing?

Copy link

@mzitnik mzitnik left a comment

Choose a reason for hiding this comment

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

LGTM

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Dec 23, 2024

I added the set_access_token method for starters. We can provide more exquisite support with internal timers as a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants