-
Notifications
You must be signed in to change notification settings - Fork 940
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
Application Integration: OIDC endpoints #869
base: main
Are you sure you want to change the base?
Conversation
Implements the core storage interface for the Zitadel OIDC implementation. Intended to be used with applications that use Hanko for authentication. This is NOT a generic OIDC implementation for login to Hanko.
First full runthrough is implemented as a test. Most functionality should work now. Major errors are eliminated.
|
||
var opKeys []op.Key | ||
for _, key := range keys { | ||
opKeys = append(opKeys, &key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G601: Implicit memory aliasing in for loop.
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
|
||
storage := oidc.NewStorage(persister) | ||
for _, client := range cfg.OIDC.Clients { | ||
err := storage.AddClient(&client) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G601: Implicit memory aliasing in for loop.
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Hi Ferdinand, We are trying to solve Authentication for first party applications and one of our core values is developer experience. With OIDC this would suffer a lot. Not only do you as a user need to do the whole redirect, save state, callback dance there is also the issue of styling. The Login screen and components would suddenly have to be hosted at the hanko-backend and not in your application. On top of that we currently do not have the capacities to overview and maintain the huge code surface OIDC/OAuth would put on our shoulders. We are currently looking into a session like system offered as an alternative to the stateless JWT session we currently have and think that this will be the way to go in the future. Regardless, thanks for your effort. If you have any more input on that matter feel free to respond. |
Thank you @like-a-bause for the reply! I fully understand the reasoning behind it. Do you happen to have some more information on the session system you are planning? I'm expecting you will introduce a custom endpoint to validate the (session) token against and then just pass the (session) token via Cookie or Header? |
We recently had a talk about this and imagined a somehow hybrid approach: Give out a server stored session/refresh token and a short lived JWT session. This way relaying-party backend implementations don't have to do anything but we will get the benefit of being able to cancel a session server side. |
Can we move this to a discussion? For my downstream systems to work I need id_tokens. Thats why I like OIDC. I don't need care about the access_tokens hanko may generate as long as I get my id_token. I don't care about sessions, etc. Just proof of life and how that was achieved. |
Description
This PR implements OIDC provider endpoints for Hanko using the zitadel/oidc package, briefly also described in this discussion post. Applications can then be integrated with Hanko as their identity provider using whatever standard OAuth 2.0 / OIDC client the language provides.
The PR could serve as an alternative integration pattern for applications instead of the currently used JWT based cookie / Header authentication. Because OAuth 2.0 / OIDC is a fairly widespread and standardized protocol - and most languages implement OAuth 2.0 / OIDC clients - integration of Hanko and therefore adoption would be much easier than implementing a custom JWT system.
References:
Implementation
Used the excellent zitadel/oidc package that handles all OIDC related tasks. Tried to keep the storage backend as close to the current Hanko implementation as possible.
There is one issue in the current setup that Hanko doesn't provide automated redirects on successful login, so some Client side work is needed to get this working properly.
Besides that it should fit quite well with the existing Hanko program flow.
Tests
Currently only the happy path is tested. OIDC implementation itself is tested in the underlying package.
Planning on adding more tests if/when there is a general consensus if such implementation would get merged or not.
Todos
Additional context
This PR is in DRAFT stage and the implementation is not finished. I requested it to get an early 👍 👎 if there is a chance this lands in Hanko or not - and understand where I need to adhere better to Hanko principles.