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:

I was expecting to be redirected to the OIDC provider login form to provide my credential.

Case 2:

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 no state query param in the current URI. My proposal here would be first to invoke method CodeAuthenticationMechanism.processRedirectFromOidc(...) only when the current URI is the OIDC redirect_uri instead of doing it for every incoming request with a the state cookie q_auth. So:

    • when the current URI is not the OIDC redirect_uri, a 302 should be send with a fresh state cookie named like q_auth_{state} where {state} is the value of the state query param sent to the OIDC provider.
    • when the current URI is the OIDC redirect_uri, the method CodeAuthenticationMechanism.processRedirectFromOidc(...) should be invoked and the state check should be performed by checking at least if any request cookie q_auth_{state} exists.
  • 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 performing state check. The proposal I made for case 1 with the dedicated q_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)

Most upvoted comments

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

It’s not about whose users are wrong and whose are not. quarkus-oidc has been around for a while and we’ve seen many confirmations it works for users in production with all sort of providers. Your request is the first time of this kind (likely in fact the 2nd as I recall a similar query on Zulip).

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.

When a user sees an authentication challenge, I do not understand, why the code has to be tweaked, to let users move away from this challenge and forget about it and then authenticate to the same application in another tab, as opposed to expecting the users complete this challenge. It is not really about some freedom of choice etc - but about a basic completion of the flow.

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:

  • I open my browser and open tab 1 on my mail box to check my mails.
  • I see that I received a notification mail from Github to let me know that you answered me.
  • I click on the Github link in the notification mail that opens a tab 2 on github challenging me for authentication.
  • The phone ring preventing me from completing the challenge. (That was not my intention to not complete the challenge but I have to take the call)
  • I talk with my friend on the phone which tell me to go check the email he just forwarded me.
  • I go back in tab 1 to read his email.
  • I finish talking with my friend and I hang up the phone
  • I’m still in tab 1 in my mail box and I notice that I had a second Github notification mail to tell me that you sent me a second anwser.
  • As a normal user I click on the link in the second notification mail to read your answer and I’m not expecting to end up on an error page in tab 3 just because I should have remember that I already started an authentication flow that I did not complete in tab 2. The error page looks like a “punishment” for not having completing the challenge in tab 2 before taking the phone call or before clicking on the second link.

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 ?

However, I’d like to keep this issue open to gather more feedback - thanks for your input so far.

You’re welcome, thanks too 👍 . Let’s wait for more input.

  • Nicolas

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 ,

Checking the state is a prerequisite which can’t be avoided for security reasons - if the state cookie exists but there is no state query param in the current URI to match it then it is 401. It must happen for every request.

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 the redirect_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.

state RECOMMENDED. An opaque value used by the client to maintain state between the request and callback.

The expected usage of state is even described in the RFC:

https://www.rfc-editor.org/rfc/rfc6749#section-10.12

The client MUST implement CSRF protection for its redirection URI. This is typically accomplished by requiring any request sent to the redirection URI endpoint to include a value that binds the request to the user-agent’s authenticated state…

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:

(step 2 in particular - so I’m about to enter my name and password but then decide to step outside due to some emergency - well if it is the case - lets just restart a browser and start again when I’m back ?).

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 :

  • restart is browser and lose everything he was doing previously
  • or first check if the link is a link to go on “quarkus app A”. If true, the user must remind that he already started the authentication process for this app, and finish it before clicking on the link right ?

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 is important from my point of view is that quarkus-oidc supports users in production environments. The early state cookie check allows to optimize things nicely.

What kind of optimization ? To me, this early state cookie check seems Quarkus specific and has no security value for a production environment.

I’ve started prototyping a fix along the lines of what you have proposed and started questioning it by asking myself - why I’m doing it.

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.

Now we have an edge case to deal with, someone sends an initial request to the callback path with a custom state query parameter.

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.

As far as the security consideration is concerned: where is a guarantee that someone is not provoking a redirect loop by affecting the redirect URI of the authenticated user - for example, by replacing /callback with /a ?

I don’t understand what you mean here, can you please explain ?