-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 extension does not work with Azure AD after upgrading to Quarkus 1.7.0.FINAL #11418
Comments
/cc @sberyozkin this one looks like a regression. Could you prioritize it? Thanks! |
I see that something changed with the principal and user information, see 8562266#diff-c34b979eed4ac8884d4a875479eca3cf I tried setting https://quarkus.io/guides/security-openid-connect-web-authentication#quarkus-oidc_quarkus.oidc.authentication.user-info-required and https://quarkus.io/guides/security-openid-connect-web-authentication#quarkus-oidc_quarkus.oidc.roles.source however this also didn't solve my problem. |
I also did some debugging to see what requests Quarkus is making to Azure AD. It seems that it successfully exchanged a code for a set of ID and access tokens. It also fetches the OpenID config and certs correctly on startup. I did this with Proxyman running on my Mac intercepting and decrypting SSL traffic. With the following Quarkus config:
|
@wjglerum The linked changes are not related at all to the problem so please do not enhance the existing configuration for now. I'm nearly 100% sure it is not a Quarkus OIDC issue. The reason I think it is not related is to do with your comment : |
Thanks for your reply @sberyozkin. I was going through the changes in the OIDC extension and this came across, so I gave it a try. It's not part of the original bug report and sample project. I tried the simplest setup as possible with my sample project, with little to no config. I also tried debugging and checking what is going on. I ruled out the calls to Azure AD from Quarkus as these are similar in both versions (checked with Proxyman). When double checking the request flows, I found the weird bit. When Quarkus successfully requested the tokens from Azure AD with the code and redirects the user back to the application it sets a weird cookie: Request URL:
This is not what I would expect. In 1.6.0.FINAL it sets the following cookies instead:
There is nothing in the logs, even with debugging logging. Any pointers were we could look into? For now I'm pretty stuck on this. |
Hi @wjglerum The fact you see this strange Can you set a breakpoint here please: This is where the return leg of the code flow is processed. And here: |
@wjglerum Hi, did you have a chance to look into it ?
Can you try |
@wjglerum I've started creating an Azure tenant and I have to admit I don't know what exactly I'm doing there, different types are possible, etc. Can you please create some test tenantid which I can use ? |
Hi @sberyozkin, finally got some time to properly debug this. Had a quick look yesterday evening but couldn't find anything useful. Thanks to your suggestions for the breakpoints I now did manage to find an exception. Which does not happen when we use Keycloak as the OIDC provider. The troubled bit seems to be here: https://github.com/quarkusio/quarkus/blob/master/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java#L325 It fails with the following message: With the following stacktrace (from the IntelliJ IDEA debug console):
It looks like it cannot read or parse a certain part of the response from Azure AD correctly. Which is weird as it does work in Quarkus 1.6.x. I can share the client id + secret with you for the test App registration I created on Azure AD. How can I best sent you this (sensitive) information? |
After some more debugging related to the "Invalid path" exception from above I found some more information. This error seems to come from the fetch method from the OAuth2API from Vert.x Auth, see https://github.com/vert-x3/vertx-auth/blob/3.9.2/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/impl/OAuth2API.java#L47 Using Keycloak:
Using Azure AD
Any idea why is this second fetch is happening? |
Alright I think I found it! This works for our project and my sample project in combination with Azure AD:
The second fetch was for the access token I think. Somehow Quarkus/SmallRye JWT/Vert.x Auth doesn't pick up the right path or Azure AD doesn't expose the right configuration on their discovery endpoint for verifying access tokens. Also see So I think this change was indeed introduced in the commit I mentioned, 8562266#diff-c34b979eed4ac8884d4a875479eca3cf Not sure if we need a change on the Quarkus side though. See https://docs.microsoft.com/en-us/azure/active-directory/develop/access-tokens#validating-tokens for more information on validating tokens. It should be possible to validate the access token locally right? |
Hi @wjglerum Thanks for spending the time on debugging this issue. Hmm... Something strange is going on. OK,
I did introduce this check (true by default) because until However it does look like it causes side-effects. Specifically, if the discovery document does not return an introspection endpoint address and the the access token is opaque (binary) or JWT but is signed by a key which is not available in the fetched JWK set - which is why you are seeing an extra fetch and the ultimate failure due to the missing introspection path. So what I will do is I will set this property to
Thanks again, and sorry for this regression - even though IMHO it is a good improvement from the security point of view. I'll take care of it now. |
@wjglerum the same issue is here, #11460 (comment) |
The reason it is all working with KC is because its access token is also JWT and besides, it also reports the introspection path in the discovery doc, hence the Quarkus tests are passing |
Thanks for your explanation and follow up @sberyozkin! It would be nice if we could somehow validate the access token coming from OIDC providers like Azure AD. This should be possible right? Because the access token returned by Azure AD is a JWT which has a
|
@wjglerum Are you sure you are referring to the access token as opposed to ID token ? |
@wjglerum see also this comment, #11460 (comment), if you can try to debug even more than it will clarify things. |
@wjglerum similarly to Google, Azure does not have an introspection endpoint in this discovery doc. |
@sberyozkin thanks for changing the default for verifying the access tokens! Here are the decoded headers from both tokens:
Seeing the headers I do think that Quarkus OIDC should be able to verify the access token. As the access token uses the same I'll try to do some more debugging tomorrow on why it is not using the keys to verify the access token. |
Alright, I had some more time to look at the validation of the access token. It seems it's not possible with Azure AD. Or at least not with the specified Double checked the docs for access tokens from Azure AD and they also mention that we should treat them as opaque tokens:
I guess it makes sense to keep the default to not enforce validation of access tokens. |
* it turned out that 1.7.0.Final had a bug in it which resulted in an endless loop of authenitcation when using google as OpenId provider. Issue is described here: quarkusio/quarkus#11418. Issue was fixed in 1.7.1, hence quarkus upgrade was required * application has to provide two more properties through the environment: `quarkus.oidc.client-id` and `quarkus.oidc.credentials.secret`
Describe the bug
We have a webapp where we delegate the login to Azure AD with the Quarkus OIDC extension. When we upgrade to Quarkus 1.7.0.FINAL the login through our OIDC provider is broken. Locally everything works with Keycloak as the OIDC provider. However when using Azure AD as the OIDC provider we get in a loop during login.
Expected behavior
Logging in with Azure AD as the OIDC provider works.
Actual behavior
After logging in with Azure AD we end up with an infinite loop. Flow of requests:
http://localhost:8080/
Response headers:
Request URL:
https://login.microsoftonline.com/<<<tenant_id>>>/oauth2/v2.0/authorize?redirect_uri=http%3A%2F%2Flocalhost%3A8080%2F&state=9a9c65c4-5005-4480-8648-bd727c27cc09&scope=openid&response_type=code&client_id=<<<client_id>>>
Login flow with Azure AD
Request URL:
https://login.microsoftonline.com/kmsi
Response headers:
http://localhost:8080/?code=<<<code>>>&state=9a9c65c4-5005-4480-8648-bd727c27cc09&session_state=c6f3b182-acc3-43f5-a997-193762b28332
Response headers:
From the logs with DEBUG level set for the OIDC plugin:
The weird bit here is the only on the last request it warns us about the fact that the state cookie does not match.
Quarkus 1.6
Compare this with the output when using Quarkus 1.6.0.FINAL
http://localhost:8080/
Response headers:
Request URL:
https://login.microsoftonline.com/<<<tenant_id>>>/oauth2/v2.0/authorize?redirect_uri=http%3A%2F%2Flocalhost%3A8080%2F&state=e27702af-ba2a-49d5-9c4d-d6ce5e621bc5&scope=openid&response_type=code&client_id=<<<client_id>>>
Login flow with Azure AD
Request URL:
https://login.microsoftonline.com/kmsi
Response headers:
http://localhost:8080/?code=<<<code>>>&state=e27702af-ba2a-49d5-9c4d-d6ce5e621bc5&session_state=8192f17c-fc9c-4293-95f7-d0789239dc2e
Response headers:
http://localhost:8080/
And user is logged in successfully and cookies are set properly.
With the following logs:
Everything looks good here.
To Reproduce
Steps to reproduce the behavior:
quarkus-oidc
enabled, see https://github.com/wjglerum/quarkus-oidc-azuremvn quarkus:dev
and try to loginConfiguration
Screenshots
(If applicable, add screenshots to help explain your problem.)
Environment (please complete the following information):
uname -a
orver
:java -version
:N/A
1.7.0.FINAL
mvnw --version
orgradlew --version
):Additional context
Let me know if I can provide more information. Also, did anything major change in the OIDC extension this release?
The text was updated successfully, but these errors were encountered: