quarkus: Unexpected 401 due to AuthenticationCompletionException is thrown in OIDC Authorization Code Flow
Describe the bug
Hi guys,
I have an application where Quarkus OIDC Extension is enabled to protect HTTP endpoints using OIDC Authorization Code Flow .
Case 1:
- Step 1: I open a tab in my web browser and I try to reach a protected endpoint. I’m redirected to the OIDC provider login form to provide my credential => OK
- Step 2: I don’t fill up the form and I open another tab in my web browser to reach, the same or any other protected endpoint. A 401 is returned due to AuthenticationCompletionException => KO https://github.com/quarkusio/quarkus/blob/9381f2ef1b15453f4cbc6b9819478379e03962d0/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java#L131
I was expecting to be redirected to the OIDC provider login form to provide my credential.
Case 2:
- Step 1: I’m successfully authenticated in my webapp using OIDC Authorization Code Flow.
- Step 2: I open another tab in my web browser to browse another page of my webapp. => OK
- Step3: I leave for a long lunch break and both my webapp session cookie and my OIDC provider session cookie expires. => OK
- Step4: After the break, in both tabs in my web browser, my webapp frontend automatically tries to reach “concurrently” a protected endpoint. In both tabs I’m redirected to the OIDC provider login form to provide my credential => OK
- Step 5: I fill up the form in tab 1 and a 401 is returned due to AuthenticationCompletionException => KO https://github.com/quarkusio/quarkus/blob/9381f2ef1b15453f4cbc6b9819478379e03962d0/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java#L131
I was expecting to be successfully authenticated and redirected to the protected endpoint.
Proposal:
According to me:
-
In case 1, the error occurs because state cookie
q_auth
exists but there is nostate
query param in the current URI. My proposal here would be first to invoke methodCodeAuthenticationMechanism.processRedirectFromOidc(...)
only when the current URI is the OIDCredirect_uri
instead of doing it for every incoming request with a the state cookieq_auth
. So:- when the current URI is not the OIDC
redirect_uri
, a 302 should be send with a fresh state cookie named likeq_auth_{state}
where{state}
is the value of thestate
query param sent to the OIDC provider. - when the current URI is the OIDC
redirect_uri
, the methodCodeAuthenticationMechanism.processRedirectFromOidc(...)
should be invoked and thestate
check should be performed by checking at least if any request cookieq_auth_{state}
exists.
- when the current URI is not the OIDC
-
In case 2, the error occurs because the state cookie
q_auth
created by tab1 has been overriden by the one created by tab2 leading to a value mismatch when performingstate
check. The proposal I made for case 1 with the dedicatedq_auth_{state}
cookie allows to fix this case too.
WDYT ?
Thanks
-Nicolas
Expected behavior
No response
Actual behavior
No response
How to Reproduce?
No response
Output of uname -a
or ver
No response
Output of java -version
No response
GraalVM version (if different from Java)
No response
Quarkus version or git rev
No response
Build tool (ie. output of mvnw --version
or gradlew --version
)
No response
Additional information
No response
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 4
- Comments: 26 (15 by maintainers)
Hi everyone, sorry for a delay, I was not able to prioritize due to other issues, but as far as the next major OIDC issue is concerned, this is the one I’ll be having a look at
I’m not saying it does not work for user in production. I’m telling you it works fine but it could be more flexible to handle a case like case 1 which is a realistic and reproducible production case. More, being that flexible will not even have security impact for
quarkus-oidc
.Once again, user does not move away from challenge and open the application on another tab on purpose. But there is many cases where this can happen unintentionally and it would have been nice from
quarkus-oidc
to handle it.Here is another basic application flow (the last one) closer to us. Let’s assume that Github authentication is provided by
quarkus-oidc
:To me such behavior is frustrating for user experience and, for what it worth as an justification, I suppose that also why well known application handle this little case. Handle it has no impact on security and improve user experience so why not to do it ?
You’re welcome, thanks too 👍 . Let’s wait for more input.
I could not have explained it better than @NicoNes .
We are currently experiencing the same issue with our periodic upgrades of dependencies. We are now forced to stop updating the quarkus dependency on theese projects, leaving us vulnerable in case some new vulnerability arises.
The evolution of quarkus-oidc and the check on the state parameter on the first engagement of the webapp caught us off guard. We expected a redirect to a new login process, not a 401 page with users stuck without clues.
@NicoNes Let me think about it
Hi @sberyozkin ,
The
state
check is important for security reason to prevent CSRF I know and we both agree with that. But this check is only relevant when OIDC provider send Authorization code back on theredirect_uri
. I suppose you already know the OAuth2 RFC but it is written in it :https://www.rfc-editor.org/rfc/rfc6749#section-4.1.1.
The expected usage of state is even described in the RFC:
https://www.rfc-editor.org/rfc/rfc6749#section-10.12
Most of the webapp I’ve been using behave like that that’s why Quarkus behavior on this point surprised me a bit. Try it on gitlab or spotify for example you will see my point.
About case 2, it’s a normal situation to open multiple tabs when using a webapp and being disconnected on each of them after a long break. How can my user know which tab to use to fill up the OIDC provider login form to avoid state mismatch ?
Thanks
I was too quick to conclude that q_auth was being set for an unauthenticated resource - it is not - the problem was due to the page returned accessing an authenticated resource which then sets q_auth and causes the problem.
Hi @sberyozkin
I just answer on case 1 for now to not introduce confusion and I will answer for case 2 when we are done on this one.
My answers:
So you mean that when the user comes back to his desk after being interrupted for a meeting or any emergency reason, he can’t click on a link somebody sent to him ? You mean that before clicking on the link, the user must either :
Well, any of your solution seems realistic to me. We are putting constraints on how user is supposed to use his browser or application for no reason.
What kind of optimization ? To me, this early state cookie check seems Quarkus specific and has no security value for a production environment.
I’d say your doing it because it’s the way the spec says to do it. And because checking state for every path has no security value.
Not sure to understand your point here. This is not and edge case, this is the basic case described in the spec: Whenever the callback path is called a check state must be perform. If the custom state query parameter does not match the one in the cookie, or if there is no cookie, the request processing must fail.
I don’t understand what you mean here, can you please explain ?