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

OIDC tenant resolution policy #33495

Closed
sberyozkin opened this issue May 19, 2023 · 5 comments · Fixed by #39236
Closed

OIDC tenant resolution policy #33495

sberyozkin opened this issue May 19, 2023 · 5 comments · Fixed by #39236
Assignees
Labels
area/oidc kind/enhancement New feature or request
Milestone

Comments

@sberyozkin
Copy link
Member

sberyozkin commented May 19, 2023

Description

Right now, in a multi-tenant OIDC setup, users have to write custom TenantResolver (to resolve tenant configurations already set in application.properties) or TenantConfigResolver (if configuations are created dynamically).

Custom resolvers usually need to check a path or something else in the request which can be quite boilerplate.
#32864 goes some way toward simplifying it for typical cases involving multiple providers.

When tenant configurations are set in application.properties, instead of writing a custom TenantResolver, one can use annotations instead. Which is quite simple but still some code is needed.

Implementation ideas

@pedroigor has had an idea of simplifying and enhancing it further with what can be called tenant resolution policy annotations, similar to what can be done with HTTP policy configuration, for example:

@TenantIdResolver(id="tenant-1", paths="/web-app/*")

Here, a tenant-1 OIDC configuration will apply to all paths matching /web-app/*.

It would be the first step, Next steps can involve using custom rules, as well as running multiple ordered resolvers, etc

@sberyozkin sberyozkin added the kind/enhancement New feature or request label May 19, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented May 19, 2023

/cc @pedroigor (oidc)

@michalvavrik
Copy link
Member

Hey @sberyozkin , I am missing part where would be @TenantIdResolver put (on which class or method). My problem is that when you write @TenantIdResolver(id="tenant-1", paths="/web-app/*") then it seems very much like something that belongs to application.properties instead of annotation, for the fact that I can't see a link between this annotation and specific class. Can you elaborate, please?

@michalvavrik
Copy link
Member

More I'm thinking about it, less it makes sense to use an annotation. I think we have quite robust path-matching HTTP Security policies now, all we need to do is to offer OIDC-specific policy, for example:

quarkus.oidc."tenant".paths=/web-app/*

which behind the curtains will be just shared custom HTTP Security policy for path pattern /web-app/* that will select tenant-1. I'll need to do few tweaks, but nothing major.

@sberyozkin if you agree, can you adjust the issue description or provide feedback, please?

@michalvavrik
Copy link
Member

Although with proactive auth it would be cool to select the client before initial auth happened. I think I'll avoid HTTP Security policies but major question whether the quarkus.oidc."tenant".paths=/web-app/* is acceptable instead of some annotation. Or how is the annotation meant. We need to reuse path-matching that we use for the HTTP security policies anyway.

@michalvavrik michalvavrik self-assigned this Mar 4, 2024
@sberyozkin
Copy link
Member Author

Hi @michalvavrik, sorry, I've totally missed it. I've been rereading the issue. If we have a @Tenant annotation, it already selects a path indirectly via JAX-RS @Path.

So I think what you are proposing with quarkus.oidc."tenant".paths=/web-app/* is an alternative to @Tenant and indeed, IMHO it makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oidc kind/enhancement New feature or request
Projects
None yet
2 participants