-
Notifications
You must be signed in to change notification settings - Fork 3k
Do not change configured JWT authentication token audience value #51144
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
| .audience(oidcConfig.credentials().jwt().audience().isPresent() | ||
| ? removeLastPathSeparator(oidcConfig.credentials().jwt().audience().get()) | ||
| : tokenRequestUri) | ||
| .audience(oidcConfig.credentials().jwt().audience().orElse(tokenRequestUri)) |
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.
What if there is a trailing slash in the application.properties? Will that break an existing app?
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.
Hi @gastaldi,
Unfortunately, it might some, yes, it is a fairly narrow space, must be Keycloak, a rather specific client authentication mechanism where a token wrapping various info instead of typical client id and secret is generated, and users having an unnecessary trailing slash occasionally in the KC realm endpoint.
I added a breaking change label
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.
Let me look a bit more into it
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.
No, it can't be worked around, I thought KC specific path could be found, but in the test, JWT audience is initialized with a Keycloak realm URL...
So let it be a breaking change, I hope no one will ever notice it in practice, but if they do, the cost of the addressing it will be straightforward - just remove a trailing slash if it ever was relevant, via the same env var that sets quarkus.oidc.credentials.jwt.audience.
Introducing a property to control whether the audience should have the trailing slash removed is an option that might be considered, but I'd be hesitating with doing it now, to add a property to control the buggy OIDC behavior to an already massive set of existing OIDC props :-)
This comment has been minimized.
This comment has been minimized.
michalvavrik
left a comment
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.
to add a property to control the buggy OIDC behavior to an already massive set of existing OIDC props :-)
Personally, I'd call it undocumented behavior. I understand that #51133 has API that accepts aud with the ending slash. And we should give them way to end it with slash other than to add two slashes. I tried to look it up and it seems to be common to have the URI without ending slash.
This comment has been minimized.
This comment has been minimized.
|
Hi Michal, @michalvavrik
Right, but users who do not need a slash already have an option not to add it, instead of adding it and expecting it be removed... When it comes to configuring an audience, the configured value, whatever it is, is a user provided value - users have already expressed a requirement with something like I'm not aware of any OIDC spec text that would require that the JWT audience claim value must not end with Sigh... having said all of that, I think with a property this PR can become bacportable... Let me think about this property |
1649db9 to
6ec9ca6
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🎊 PR Preview 809be8f has been successfully built and deployed to https://quarkus-pr-main-51144-preview.surge.sh/version/main/guides/
|
michalvavrik
left a comment
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.
+1
6ec9ca6 to
91da575
Compare
|
Thanks @michalvavrik, @gastaldi, so I added a property to allow keeping the trailing slash (default is false as on |
Status for workflow
|
Status for workflow
|
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ❌ | Native Tests - Security2 | Build |
Failures | Logs | Raw logs | 🔍 |
| ❌ | JVM Integration Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
| ✔️ | JVM Integration Tests - JDK 17 Windows | Logs | Raw logs | 🔍 | ||
| ❌ | JVM Integration Tests - JDK 21 | Build |
Failures | Logs | Raw logs | 🔍 |
| ❌ | JVM Integration Tests - JDK 25 | Build |
Failures | Logs | Raw logs | 🔍 |
Full information is available in the Build summary check run.
You can consult the Develocity build scans.
Failures
⚙️ Native Tests - Security2 #
- Failing: integration-tests/oidc-tenancy
📦 integration-tests/oidc-tenancy
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationInGraalITCase.testJwtTokenIntrospectionOnlyAndUserInfo - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Expected status code <200> but was <401>.
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:481)
at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)
at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:108)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationInGraalITCase.testNoUserInfoCallIfTokenIsInvalid - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Response body doesn't match expectation.
Expected: "1"
Actual: 0
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:481)
⚙️ JVM Integration Tests - JDK 17 #
- Failing: integration-tests/oidc-tenancy
📦 integration-tests/oidc-tenancy
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest. - History - More details - Source on GitHub
org.junit.jupiter.engine.execution.ConditionEvaluationException: Failed to evaluate condition [io.quarkus.test.junit.QuarkusTestExtension]: Internal error: Test class was loaded with an unexpected classloader (QuarkusClassLoader:Quarkus Base Runtime ClassLoader: TEST for JUnitQuarkusTest-no-profile (QuarkusTest)@32f308c6) or the thread context classloader (jdk.internal.loader.ClassLoaders$AppClassLoader@5ffd2b27) was incorrect.
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1602)
at java.base/java.util.stream.Referen...
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testCodeFlowRefreshTokensWhenIdTokenIsExpired line 213 - History - More details - Source on GitHub
org.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/tenant-refresh/tenant-web-app-refresh/api/user?state=347c5115-75bb-4784-9d05-20a189841d3a&session_state=732226ab-e531-b9de-d64d-3ab76ad1c3db&iss=http%3A%2F%2Flocalhost%3A32795%2Frealms%2Fquarkus-webapp&code=986cb65d-5ef4-7c2a-c18f-94c7d82a38cc.732226ab-e531-b9de-d64d-3ab76ad1c3db.624d9e56-5200-4087-9932-43de640f3722
at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:762)
at org.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2870)
at org.htmlunit.html.DomElement.click(DomElement.java:1173)
at org.htmlunit.html.DomElement.click(DomElement.java:1094)
at org.htmlunit.html.DomElement.click(DomElement.java:987)
at org.htmlunit.html.DomElement.click(DomElement.java:964)
at org.htmlunit.html.DomElement.click(DomElement.java:941)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testCodeFlowRefreshTokensWhileIdTokenIsValid line 272 - History - More details - Source on GitHub
org.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/tenant-refresh/tenant-web-app-refresh/api/user?state=d6e5683c-65f0-480a-bc7c-c44d4058e216&session_state=acea6dd5-ecbf-b1c3-be9f-cfa851b9ad85&iss=http%3A%2F%2Flocalhost%3A32795%2Frealms%2Fquarkus-webapp&code=223b4d2b-26fb-bdde-52a0-fec22a921aea.acea6dd5-ecbf-b1c3-be9f-cfa851b9ad85.624d9e56-5200-4087-9932-43de640f3722
at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:762)
at org.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2870)
at org.htmlunit.html.DomElement.click(DomElement.java:1173)
at org.htmlunit.html.DomElement.click(DomElement.java:1094)
at org.htmlunit.html.DomElement.click(DomElement.java:987)
at org.htmlunit.html.DomElement.click(DomElement.java:964)
at org.htmlunit.html.DomElement.click(DomElement.java:941)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testJwtTokenIntrospectionOnlyAndUserInfo line 682 - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Expected status code <200> but was <401>.
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testNoUserInfoCallIfTokenIsInvalid line 723 - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Response body doesn't match expectation.
Expected: "1"
Actual: 0
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
⚙️ JVM Integration Tests - JDK 21 #
- Failing: integration-tests/oidc-tenancy
📦 integration-tests/oidc-tenancy
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest. - History - More details - Source on GitHub
org.junit.jupiter.engine.execution.ConditionEvaluationException: Failed to evaluate condition [io.quarkus.test.junit.QuarkusTestExtension]: Internal error: Test class was loaded with an unexpected classloader (QuarkusClassLoader:Quarkus Base Runtime ClassLoader: TEST for JUnitQuarkusTest-no-profile (QuarkusTest)@51e94b7d) or the thread context classloader (jdk.internal.loader.ClassLoaders$AppClassLoader@c387f44) was incorrect.
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1685)
at java.base/java.util.stream.Referenc...
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testCodeFlowRefreshTokensWhenIdTokenIsExpired line 213 - History - More details - Source on GitHub
org.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/tenant-refresh/tenant-web-app-refresh/api/user?state=53d55276-cad8-42c5-8c08-d019d94279db&session_state=8796e664-d8ea-acfc-f0d5-9debb496cdbb&iss=http%3A%2F%2Flocalhost%3A32794%2Frealms%2Fquarkus-webapp&code=4c927abe-6a39-6a1d-9215-bdddbe622323.8796e664-d8ea-acfc-f0d5-9debb496cdbb.09747264-b3c3-4121-970a-3d0da6ddab87
at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:762)
at org.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2870)
at org.htmlunit.html.DomElement.click(DomElement.java:1173)
at org.htmlunit.html.DomElement.click(DomElement.java:1094)
at org.htmlunit.html.DomElement.click(DomElement.java:987)
at org.htmlunit.html.DomElement.click(DomElement.java:964)
at org.htmlunit.html.DomElement.click(DomElement.java:941)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testCodeFlowRefreshTokensWhileIdTokenIsValid line 272 - History - More details - Source on GitHub
org.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/tenant-refresh/tenant-web-app-refresh/api/user?state=ef1f4ba2-1657-4727-8e27-7f614f33b7d6&session_state=bc3fa6c1-b393-c4ee-c743-344957f00d49&iss=http%3A%2F%2Flocalhost%3A32794%2Frealms%2Fquarkus-webapp&code=303507af-8681-f03a-411f-93fcd1f04a3d.bc3fa6c1-b393-c4ee-c743-344957f00d49.09747264-b3c3-4121-970a-3d0da6ddab87
at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:762)
at org.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2870)
at org.htmlunit.html.DomElement.click(DomElement.java:1173)
at org.htmlunit.html.DomElement.click(DomElement.java:1094)
at org.htmlunit.html.DomElement.click(DomElement.java:987)
at org.htmlunit.html.DomElement.click(DomElement.java:964)
at org.htmlunit.html.DomElement.click(DomElement.java:941)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testJwtTokenIntrospectionOnlyAndUserInfo line 682 - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Expected status code <200> but was <401>.
at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486)
at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testNoUserInfoCallIfTokenIsInvalid line 723 - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Response body doesn't match expectation.
Expected: "1"
Actual: 0
at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
⚙️ JVM Integration Tests - JDK 25 #
- Failing: integration-tests/oidc-tenancy
📦 integration-tests/oidc-tenancy
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest. - History - More details - Source on GitHub
org.junit.jupiter.engine.execution.ConditionEvaluationException: Failed to evaluate condition [io.quarkus.test.junit.QuarkusTestExtension]: Internal error: Test class was loaded with an unexpected classloader (QuarkusClassLoader:Quarkus Base Runtime ClassLoader: TEST for JUnitQuarkusTest-no-profile (QuarkusTest)@40a8a26f) or the thread context classloader (jdk.internal.loader.ClassLoaders$AppClassLoader@5bc2b487) was incorrect.
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1693)
at java.base/java.util.stream.Referen...
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testCodeFlowRefreshTokensWhenIdTokenIsExpired line 213 - History - More details - Source on GitHub
org.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/tenant-refresh/tenant-web-app-refresh/api/user?state=fdb9c841-f554-4ac9-9111-a738df2e177f&session_state=7e38532a-e2f7-357e-6846-033357077cf4&iss=http%3A%2F%2Flocalhost%3A32794%2Frealms%2Fquarkus-webapp&code=ba03f92c-1b38-7947-8366-0004bd6c2ddd.7e38532a-e2f7-357e-6846-033357077cf4.a58504c9-3396-424d-9a0e-8ebd183a5c0b
at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:762)
at org.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2870)
at org.htmlunit.html.DomElement.click(DomElement.java:1173)
at org.htmlunit.html.DomElement.click(DomElement.java:1094)
at org.htmlunit.html.DomElement.click(DomElement.java:987)
at org.htmlunit.html.DomElement.click(DomElement.java:964)
at org.htmlunit.html.DomElement.click(DomElement.java:941)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testCodeFlowRefreshTokensWhileIdTokenIsValid line 272 - History - More details - Source on GitHub
org.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/tenant-refresh/tenant-web-app-refresh/api/user?state=554f808c-9e23-4bee-8e22-1ec3d9859763&session_state=8f322b0f-2bf0-3ea7-cd2a-1ea37d744b43&iss=http%3A%2F%2Flocalhost%3A32794%2Frealms%2Fquarkus-webapp&code=5e33060c-63f4-175f-67d5-e772b062181e.8f322b0f-2bf0-3ea7-cd2a-1ea37d744b43.a58504c9-3396-424d-9a0e-8ebd183a5c0b
at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:762)
at org.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2870)
at org.htmlunit.html.DomElement.click(DomElement.java:1173)
at org.htmlunit.html.DomElement.click(DomElement.java:1094)
at org.htmlunit.html.DomElement.click(DomElement.java:987)
at org.htmlunit.html.DomElement.click(DomElement.java:964)
at org.htmlunit.html.DomElement.click(DomElement.java:941)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testJwtTokenIntrospectionOnlyAndUserInfo line 682 - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Expected status code <200> but was <401>.
at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:483)
at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)
❌ io.quarkus.it.keycloak.BearerTokenAuthorizationTest.testNoUserInfoCallIfTokenIsInvalid line 723 - History - More details - Source on GitHub
java.lang.AssertionError:
1 expectation failed.
Response body doesn't match expectation.
Expected: "1"
Actual: 0
at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)

Fixes #51133.
I did a mistake 4 years ago in 4a42b44 where the audience property had its trailing slash, if present, removed in
OidcCommonUtils#signJwtWithKey, because this is what was necessary to get the Keycloak test passing (Keycloak realm endpoint URL is used as an audience by default), but it is really inapplication.propertieswhere the users should deal with having the trailing slash present or not.There is also
quarkus.credentials.jwt.audienceproperty that can always be used to customize it.