Skip to content

OIDC client/filter issues after migrating from previous to latest LTS #47232

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

Closed
DeMol-EE opened this issue Apr 8, 2025 · 26 comments · Fixed by #47984
Closed

OIDC client/filter issues after migrating from previous to latest LTS #47232

DeMol-EE opened this issue Apr 8, 2025 · 26 comments · Fixed by #47984
Labels
Milestone

Comments

@DeMol-EE
Copy link
Contributor

DeMol-EE commented Apr 8, 2025

Describe the bug

Migrating from the previous LTS to 3.20, we are getting issues with our OIDC clients/filters. More specifically, we get:

Configuration has 2 different id values: 'client' and '...'

While searching the documentation for an answer, I noticed that there are two properties which I assume must be colliding when specified as environment variables (all caps and underscores):

  • quarkus.oidc-client.id
  • quarkus.oidc.client-id

I further noticed that the errors disappears when changing QUARKUS_OIDC_CLIENT_CLIENT_ID to QUARKUS_OIDC_CLIENT_ID, but then the application is not behaving as expected anymore, as we get 403s.

As these properties were already there in the previous LTS, perhaps this is not the cause of our issues, but I figured I could point it out either way.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

Darwin Kernel Version 24.3.0: Thu Jan 2 20:24:16 PST 2025; root:xnu-11215.81.4~3/RELEASE_ARM64_T6000 arm64

Output of java -version

OpenJDK 64-Bit Server VM Temurin-21.0.6+7 (build 21.0.6+7-LTS, mixed mode, sharing)

Quarkus version or git rev

3.20

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937)

Additional information

No response

@DeMol-EE DeMol-EE added the kind/bug Something isn't working label Apr 8, 2025
@quarkus-bot quarkus-bot bot added the area/oidc label Apr 8, 2025
Copy link

quarkus-bot bot commented Apr 8, 2025

/cc @pedroigor (oidc), @sberyozkin (oidc)

@sberyozkin
Copy link
Member

@radcortez Hi Roberto, can you please comment here ?

@DeMol-EE
Copy link
Contributor Author

DeMol-EE commented Apr 8, 2025

An update from our side: a workaround is to name our OIDC clients (which also ensures that the above 2 properties no longer collide, as they become quarkus.oidc."tenant".client-id and quarkus.oidc-client."id".id).

@sberyozkin
Copy link
Member

@DeMol-EE Can you please clarify again what exactly is the problem. I don't think we have QUARKUS_OIDC_CLIENT_CLIENT_ID that you mention in the description.

Which 2 environment properties did you use in 3.15 to init quarkus.oidc-client.id and quarkus.oidc.client-id that no longer work in 3.20 ?

@YassinHajaj
Copy link

YassinHajaj commented Apr 8, 2025

Hello @sberyozkin ,

Sorry for my previous message (published too fast).

We're colleagues with @DeMol-EE so I can answer your questions.

So basically this was our configuration (environment variables) until before the upgrade to 3.20.

QUARKUS_OIDC_CLIENT_AUTH_SERVER_URL: "${auth.base-url}/realms/foo"
QUARKUS_OIDC_CLIENT_CLIENT_ID: bar
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_METHOD: POST
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_VALUE: myclientsecret

And it worked flawlessly.


After upgrading to 3.20, the app didn't start anymore arguing that two clients were defined : 'client' and 'bar'

When changing QUARKUS_OIDC_CLIENT_CLIENT_ID to QUARKUS_OIDC_CLIENT_ID the app started but the client was ignored as it was deemed disabled.

The error was

Client is disabled, acquiring and propagating the token is not necessary


So what we did is replace @RegisterProvider(OidcClientRequestReactiveFilter.class) on our REST clients by @OidcClientFilter("bar")

And make the following changes to our configuration (environment variables)

QUARKUS_OIDC_CLIENT__BAR__AUTH_SERVER_URL: "${auth.base-url}/realms/foo"
QUARKUS_OIDC_CLIENT__BAR__CLIENT_ID: bar
QUARKUS_OIDC_CLIENT__BAR__CLIENT_NAME: bar
QUARKUS_OIDC_CLIENT__BAR__CREDENTIALS_CLIENT_SECRET_METHOD: POST
QUARKUS_OIDC_CLIENT__BAR__CLIENT_ENABLED: "true"
QUARKUS_OIDC_CLIENT__BAR__CREDENTIALS_CLIENT_SECRET_VALUE: myclientsecret

To answer your latest question : the env QUARKUS_OIDC_CLIENT_CLIENT_ID is documented hereunder
https://quarkus.io/guides/all-config#quarkus-oidc-client_quarkus-oidc-client-client-id

Also note that I'm not sure QUARKUS_OIDC_CLIENT__BAR__CLIENT_NAME changes something, but I figured it would be good to add it there.

@sberyozkin
Copy link
Member

sberyozkin commented Apr 8, 2025

Thanks @YassinHajaj, just let me ask about the first configuration block:

QUARKUS_OIDC_CLIENT_AUTH_SERVER_URL: "${auth.base-url}/realms/foo"
QUARKUS_OIDC_CLIENT_CLIENT_ID: bar
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_METHOD: POST
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_VALUE: myclientsecret

Are you saying this configuration block causes the error related to the two clients were defined : 'client' and 'bar' ?

@YassinHajaj
Copy link

@sberyozkin this is correct indeed (only starting from the new LTS)

@sberyozkin
Copy link
Member

@YassinHajaj

To answer your latest question : the env QUARKUS_OIDC_CLIENT_CLIENT_ID is documented hereunder
https://quarkus.io/guides/all-config#quarkus-oidc-client_quarkus-oidc-client-client-id

Yes, sorry, I got confused. quarkus.oidc-client.id represents a unique OIDC client configuration block (an equivalent property in quarkus-oidc is called quarkus.oidc.tenant-id).

So, it is not clear to me at what point quarkus.oidc-client.id got involved, it is not set in:

QUARKUS_OIDC_CLIENT_AUTH_SERVER_URL: "${auth.base-url}/realms/foo"
QUARKUS_OIDC_CLIENT_CLIENT_ID: bar
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_METHOD: POST
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_VALUE: myclientsecret

@DeMol-EE
Copy link
Contributor Author

DeMol-EE commented Apr 8, 2025

quarkus.oidc-client.id is not actually used in our configuration, but it caught my eye while going through the documentation. Though it may not be related to the specific error, I thought I’d mention it cause it struck me as odd (and it might be related after all, I’m not really able to say).

@YassinHajaj
Copy link

@sberyozkin @DeMol-EE

The configuration in question was tried as part of our resolution investigation, see my previous comment.

When changing QUARKUS_OIDC_CLIENT_CLIENT_ID to QUARKUS_OIDC_CLIENT_ID the app started but the client was ignored as it was deemed disabled.
The error was
Client is disabled, acquiring and propagating the token is not necessary

But it's not part neither of the old or the new configuration. Was just something we've tried out and that could cause an overlap with another config when used using environment variables (see original comment of @DeMol-EE)

@sberyozkin
Copy link
Member

@YassinHajaj @DeMol-EE OK, so

QUARKUS_OIDC_CLIENT_AUTH_SERVER_URL: "${auth.base-url}/realms/foo"
QUARKUS_OIDC_CLIENT_CLIENT_ID: bar
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_METHOD: POST
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_VALUE: myclientsecret

causes the error about the two clients were defined : 'client' and 'bar'.

Do you have anything configured in application.properties to complement these env properties ?

@YassinHajaj
Copy link

This is correct indeed.

Here are the build time properties related to quarkus.oidc.

application.properties

quarkus.oidc.tenant-enabled=false
quarkus.oidc.realm1.auth-server-url=${auth.base-url}/realms/realm1
quarkus.oidc.realm2.auth-server-url=${auth.base-url}/realms/realm2
quarkus.oidc.realm3.auth-server-url=${auth.base-url}/realms/realm3
quarkus.oidc.foo.auth-server-url=${auth.base-url}/realms/foo
quarkus.oidc-client.early-tokens-acquisition=false

Latest change to those is in 2023, no change since.

@sberyozkin
Copy link
Member

sberyozkin commented Apr 8, 2025

OK, thanks @YassinHajaj, @DeMol-EE

@radcortez Roberto, I hoped I might be able to clarify things but looks like your help is required, please see the last two comments above, does it ring any bell ?

FYI, in the last comment above, the default Quarkus OIDC tenant is disabled, its quarkus.oidc.client-id property is not configured, is it right, @YassinHajaj, or do you have something like:

quarkus.oidc.tenant-enabled=false
quarkus.oidc.client-id=default-tenant-client-id

quarkus.oidc.realm1.auth-server-url=${auth.base-url}/realms/realm1
quarkus.oidc.realm1.client-id=realm1-tenant-client-id

quarkus.oidc.realm2.auth-server-url=${auth.base-url}/realms/realm2
quarkus.oidc.realm2.client-id=realm2-tenant-client-id

quarkus.oidc.realm3.auth-server-url=${auth.base-url}/realms/realm3
quarkus.oidc.realm3.client-id=realm3-tenant-client-id

quarkus.oidc.foo.auth-server-url=${auth.base-url}/realms/foo

?

@YassinHajaj
Copy link

Hi @sberyozkin ,

No we don't define quarkus.oidc.client-id anywhere, also not for each realm, which could be strange now that you ask me.

The only properties we have are those shared in my previous comment.

@sberyozkin
Copy link
Member

@YassinHajaj OK.
So, Roberto, @radcortez, here is a suggested summary:

quarkus.oidc.tenant-enabled=false
quarkus.oidc.realm1.auth-server-url=${auth.base-url}/realms/realm1

This configuration has 2 OIDC tenant configurations: default tenant and realm1 tenant. The default tenant is disabled with quarkus.oidc.tenant-enabled=false at runtime, its quarkus.oidc.client-id property is not explicitly configured.

Setting

QUARKUS_OIDC_CLIENT_AUTH_SERVER_URL: "${auth.base-url}/realms/foo"
QUARKUS_OIDC_CLIENT_CLIENT_ID: bar
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_METHOD: POST
QUARKUS_OIDC_CLIENT_CREDENTIALS_CLIENT_SECRET_VALUE: myclientsecret

causes a startup failure related to the two clients were defined : 'client' and 'bar'.

Please have a look when you get a chance

@sberyozkin
Copy link
Member

Hi @radcortez, can you please comment here

@radcortez
Copy link
Member

I'll have a look.

@radcortez
Copy link
Member

radcortez commented Apr 16, 2025

Yes, the issue here is QUARKUS_OIDC_CLIENT_CLIENT_ID clashing with quarkus.oidc-client.client-id and quarkus.oidc-client.*.id. The matching properties are contained in a Map, it probably worked before because the matching algorithm listed quarkus.oidc-client.client-id first, so the matching worked as expected here, but this is subject to the natural sorting order of the Map. It is only an issue due to the clash caused by the use of environment variables.

I can probably fix this by sorting the names, from most significant to less significant, in this case client-id first and then *.id*. On the other hand, I do feel that this is a configuration structure issue, since we can't really tell what the user expects. While not likely, what if the user really wants to use client as a Map key?

For cases where we have these clashes, our recommendation is to set the expected name in application.properties, as quarkus.oidc-client.client-id=. It can be empty, since the environment variable will override it anyway. This will provide Quarkus with enough information to know the exact name of the property you want to use, without trying to match it against the existing properties: https://quarkus.io/guides/config-reference#environment-variables

@DeMol-EE
Copy link
Contributor Author

Thanks for the feedback.

@radcortez
Copy link
Member

This should help: smallrye/smallrye-config#1346

@geoand
Copy link
Contributor

geoand commented May 14, 2025

@radcortez with smallrye/smallrye-config#1346 having been merged, what remains to be done on the Quarkus side so we can fix this?

@radcortez
Copy link
Member

This still needs to be released from the SR Config side... I'll do that this week.

@geoand
Copy link
Contributor

geoand commented May 15, 2025

👌🏽

@DeMol-EE
Copy link
Contributor Author

Thanks a lot, guys!

@andrejpetras
Copy link
Contributor

Hi @gastaldi, can we also integrate it into the next LTS version, 3.20.x? This issue prevents us from making a safe LTS-to-LTS transition.

@gastaldi
Copy link
Contributor

@andrejpetras thanks, I've marked to backport to 3.20 too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants