-
Notifications
You must be signed in to change notification settings - Fork 487
[Prototype] Self Managed OIDC Authentication #34516
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
base: main
Are you sure you want to change the base?
Conversation
| include_www_authenticate_header, | ||
| }); | ||
| } | ||
| Authenticator::Oidc(oidc) => match creds { |
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.
very preliminary comment: but I think with the number of authenticators growing, we should maybe put this behind an interface, maybe make Authenticator compatible with trait Objects, but at least we could move this big match statement to the authenticator crate.
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.
not really a comment about your PR but the general state here 😅
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.
I agree here!
| let auth_session = pending().right_future(); | ||
| (session, auth_session) | ||
| } | ||
| Authenticator::Oidc(oidc) => { |
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.
same comment as above, I'm hopeful we can factor this whole match statement out into authenticator crate or maybe put behind dyn objects
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.
I was thinking that too! Definitely some common traits between this and the frontegg authenticator.
To try:
demo.mov
Motivation
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.