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

Support JWT authentication #113

Merged
merged 10 commits into from
May 3, 2024
Merged

Support JWT authentication #113

merged 10 commits into from
May 3, 2024

Conversation

nineinchnick
Copy link
Member

@nineinchnick nineinchnick commented Apr 29, 2024

Based on #110.

I added certificate generation to be able to run integration tests. Statically generated certificates cannot be used, since on macOS they could only be valid for 1 year, which would require regenerating them.

I also refactored trino/integration_tls_test.go to avoid using the reverse proxy.

Fixes #59

@cla-bot cla-bot bot added the cla-signed label Apr 29, 2024
@nineinchnick nineinchnick force-pushed the jwt branch 4 times, most recently from b922098 to 76f09a6 Compare April 30, 2024 09:03
@nineinchnick nineinchnick force-pushed the jwt branch 2 times, most recently from 3d7cfda to 6f5001f Compare April 30, 2024 10:17
@nineinchnick
Copy link
Member Author

@losipiuk @wendigo PTAL, there are a lot of devex related changes here, but I made sure every commit contain single logical change.

@wendigo
Copy link
Contributor

wendigo commented Apr 30, 2024

@nineinchnick please rebase

@nineinchnick
Copy link
Member Author

@wendigo good to go, the one failure is a flaky test. I'll see if I can adress that later

@wendigo
Copy link
Contributor

wendigo commented Apr 30, 2024

@nineinchnick please rebase, there is a conflict

@nineinchnick
Copy link
Member Author

@wendigo green again

@mosabua
Copy link
Member

mosabua commented Apr 30, 2024

Thanks for running with this @nineinchnick

@nineinchnick
Copy link
Member Author

My main motivation is how quickly I can get reviews in this repo :D

@nineinchnick
Copy link
Member Author

nineinchnick commented May 2, 2024

@wendigo PTAL. After you merge this, can you do a new release? It requires pushing a new tag.

README.md Outdated
@@ -11,7 +11,7 @@ Trino, and receive the resulting data.

* Native Go implementation
* Connections over HTTP or HTTPS
* HTTP Basic and Kerberos authentication
* HTTP Basic, and Kerberos authentication
Copy link
Contributor

Choose a reason for hiding this comment

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

that looks weird

Copy link
Member Author

Choose a reason for hiding this comment

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

The Oxford comma?

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW I ran the Vale linter on this Readme, same one we use for Trino docs

@wendigo wendigo merged commit 4c19a32 into trinodb:master May 3, 2024
5 checks passed
@nineinchnick nineinchnick deleted the jwt branch May 3, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

implement 'access-token' option
3 participants