Don't hardcode `useHash` when completing authorization requests

[lilioid]

Expected Behavior

[REQUIRED] Describe expected behavior

When being redirected from the issuer to an app url like http://localhost:8080/auth/callback?state=…&code=… I called completeAuthorizationRequestIfPossible on my RedirectRequestHandler with a configured notifier. I expected to get a notification since the response parameters are present in the current location.

Describe the problem

[REQUIRED] Actual Behavior

I did not get notified of a response and instead only found the following log messages:

Checking to see if there is an authorization response to be delivered.
Potential authorization request  http://localhost:8080/auth/callback  {  } undefined  undefined
Mismatched request (state and request_uri) dont match.
No result is available yet.

[REQUIRED] Steps to reproduce the behavior

I have created a gist with the auth code I currently use.

In my app I do the following: 1.) User visits /auth/login which triggers auth.doAuthorization("http://localhost:8080/auth/callback") 2.) User visits OpenId issuer and logs in 3.) User gets redirected to /auth/callback which triggers auth.doAuthorizationCallback

Discovered Workaround

While reading the existing code I dicsovered that the RedirectRequestHandler always passes useHash=true to its QueryStringUtils therefore ignoring all get parameters of the current location: https://github.com/openid/AppAuth-JS/blob/cf6bb68dfe630c5d5f415bdcc76ea2581f041d8f/src/redirect_based_handler.ts#L100

By knowing this I was able to construct my RedirectRequestHandler in the following way. I do not like to do this and would wish for this library to either automatically discover whether useHash should be true or let me pass it in as a parameter.

new RedirectRequestHandler(new LocalStorageBackend(), new BasicQueryStringUtils(), { ...window.location, hash: window.location.search })

[REQUIRED] Environment

• AppAuth-JS version: 1.3.0
• AppAuth-JS Environment (Node, Browser (UserAgent), ...): Browser (Firefox 86)
• Source code snippts (inline or JSBin)

[tikurahul]

The reason why we don't do this out of the box is this that query parameters in the URLs are not as secure as query fragments. This is because parameters end up in logs for proxy servers and other intermediaries.

This is why the client does the secure thing. If you want to make it less secure by default, that is a choice you can make - but that has to be an opt in and not automatic.

[lilioid]

Ahh I understand, thanks for clarifying!

I would still like to configure it somehow or at least have some sort of documentation around it (because I was really confused at first) but understand that you closed the issue.

[derTobsch]

Thanks for this issue @ftsell, we searched the whole day. After debugging deeper and depper we found the hard coded true on useHash. It would be really nice if there are docs or examples. And maybe someone can tell my why whis is less secure? I thought with the 'code flow with PKCS' (Keycloak) the parameters should be used.

see https://christianlydemann.com/implicit-flow-vs-code-flow-with-pkce/

Help me to learn something new :)

[derTobsch]

The reason why we don't do this out of the box is this that query parameters in the URLs are not as secure as query fragments. This is because parameters end up in logs for proxy servers and other intermediaries.

This is why the client does the secure thing. If you want to make it less secure by default, that is a choice you can make - but that has to be an opt in and not automatic.

Is there a way to tell keycloak that we want it in the query fragment?

[lilioid]

Is there a way to tell keycloak that we want it in the query fragment?

I've just digged a bit and found that the OpenId Connect spec defines the response_mode parameter of which keycloak seems to support query, fragment and form_post. See response_modes_supported in your OpenId configuration (here is mine). The spec also shows an example here.

I haven't tested this yet but from what I can see this seems to be the way to tell Keycloak.

[derTobsch]

Is there a way to tell keycloak that we want it in the query fragment?

I've just digged a bit and found that the OpenId Connect spec defines the response_mode parameter of which keycloak seems to support query, fragment and form_post. See response_modes_supported in your OpenId configuration (here is mine). The spec also shows an example here.

I haven't tested this yet but from what I can see this seems to be the way to tell Keycloak.

Thanks for your reply @ftsell - just in case other people will search for this here are other issues of this case #167 #57