openid / AppAuth-JS

JavaScript client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
Apache License 2.0
977 stars 162 forks source link

completeAuthorizationRequest does not work without hash routing #114

Closed shockron22 closed 5 years ago

shockron22 commented 5 years ago

Expected Behavior

I would expect that completeAuthorizationRequest would be able to get me the code_verifier when I am not using hash routing.

[REQUIRED] Describe expected behavior

Describe the problem

https://github.com/openid/AppAuth-JS/blob/master/src/redirect_based_handler.ts#L100

your parse function is hardcoded to use look in location.hash not location.search https://github.com/openid/AppAuth-JS/blob/master/src/query_string_utils.ts#L30

this means for me completeAuthorizationRequest will NEVER run my AuthorizationListener

[REQUIRED] Actual Behavior

my AuthorizationListener never runs.

[REQUIRED] Steps to reproduce the behavior

Attempt to use an AuthorizationListener with an application that does not use location.hash

[REQUIRED] Environment

tikurahul commented 5 years ago

You can always override BasicQueryStringUtils and override the method to always use false if you want. You can then plumb that implementation through to RedirectBasedHandler using the constructor.

https://github.com/openid/AppAuth-JS/blob/master/src/redirect_based_handler.ts#L51

We did this specifically in the sample because users using AppAuth-JS need to consider the downsides of using 3P JavaScript libraries (which have access to the redirect URI).

thardyman commented 1 year ago

Thanks for this. I have been scratching my head as to why my listener wouldn't work. Took me ages to track down that line of code where the default listener is using location.hash rather than location.search. It would be good if there was some documentation around how to override the default behaviour, why you might need to, and the risks associated. I'm not sure I fully understand.

Here's my code snippet that seems to fix the issue, not sure if this is the best way to create the custom util function, but it seems to work: -


    // We need to use a custom querystring parser because the default one
    // assumes hash-based routing, which we don't use.
    const regularQueryStringUtils = new BasicQueryStringUtils()
    const myQueryStringUtils = new BasicQueryStringUtils()
    myQueryStringUtils.parse = (input: Location) =>
      regularQueryStringUtils.parse(input, false)
    const authHandler = new RedirectRequestHandler(
      new LocalStorageBackend(),
      myQueryStringUtils,
      window.location,
      new DefaultCrypto()
    )