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

Add support for multiple audiences #26

Merged
merged 3 commits into from
Jan 26, 2024
Merged

Add support for multiple audiences #26

merged 3 commits into from
Jan 26, 2024

Conversation

DerKnerd
Copy link
Contributor

The OIDC provider Zitadel returns multiple audiences. Since I use Zitadel I added the option to have more than one audience in the configuration. If none are provided the client_id is used as only valid audience.

@ctron
Copy link
Owner

ctron commented Jan 24, 2024

Thanks for submitting the PR! That's definitely useful!

I am just wondering if we could simplify this a bit. To my understanding, calling set_other_audience_verifier_fn is a check in addition to checking the client_id already. So we could rename the valid_audiences to additional_required_audiences (or something like that), give it a type of Vec<_> (instead of the additional Option, and then either omit calling set_other_audience_verifier_fn, or return true if the Vec is empty.

What do you think?

@DerKnerd
Copy link
Contributor Author

Yeah I think that sounds like a good point. I just used that way in a different server side actix code 😄

@ctron
Copy link
Owner

ctron commented Jan 26, 2024

I am taking a look at this right now, trying to get it merged.

Just for my understanding: the idea of the set_other_audience_verifier_fn is, to verify each audience present in the token, right? so actually it would not be "additional required audiences", but "additional allowed audiences"?

@ctron ctron merged commit 6c56727 into ctron:main Jan 26, 2024
1 of 3 checks passed
@DerKnerd
Copy link
Contributor Author

Yeah that is about it. That is why the client id is always required.

@ctron
Copy link
Owner

ctron commented Jan 29, 2024

Cool, so the change is released with 0.10.1, maybe you can give it a try and report back if that works for you.

@DerKnerd
Copy link
Contributor Author

I need to see when I get to it. For the current project I moved the auth into the actix server and its session.

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.

2 participants