IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 842 forks source link

UrlUtility should throw if no fragment is present #597

Open micha149 opened 6 years ago

micha149 commented 6 years ago

I have an issue primarily caused by a bad implementation of the authorization server, but it looked like a problem in the oidc-client. A proper exception could help finding such issues a bit faster.

My authorization server passed the response values as query parameters not as uri fragment:

http://localhost:3000/callback#state=0bfb50ace55c4e01bfa5b07487701470&error=invalid_scope&error_description=An+unsupported+scope+was+requested

The UrlUtility extracts the following props:

{
  "error": "invalid_scope",
  "error_description": "An+unsupported+scope+was+requested",
  "http://localhost:3000/callback?state": "7cf0965ef5b84f9ba98a378249f12b28"
}

The resulting error is OidcClient.processSigninResponse: No state in response. Something like No hash found in url would help a bit better.

brockallen commented 6 years ago

Hmm, yea, odd. I'll look into it.

brockallen commented 6 years ago

BTW, you know you can pass a delimiter as the 2nd param to get around your wonky token server?

brockallen commented 6 years ago

Ok, I looked into this and unfortunately given how it's designed to be used internally and that a client can explicitly pass a value that's pre-processed (meaning they've already parsed the URL and possibly parsed out the delimiter), then I can't change the way this works without some behavior changes. I added this to 2.0 to look into again, though, when I can make those types of changes

micha149 commented 6 years ago

@brockallen Yes, I noticed the delimiter parameter as I digged into the problem. But I'm using the UrlUtility not directly. I came to it through UserManager.signinRedirectCallback > OidcClient. processSigninResponse > SigninResponse.constructor > UrlUtility.parseUrlFragment and there is a hard coded delimiter: https://github.com/IdentityModel/oidc-client-js/blob/dev/src/SigninResponse.js#L11

If parseUrlFragment should be usable with a pre parsed url, maybe the problem should be catched in a prior stage. For example in the SigninResponse constructor. Or in UserManager.signinRedirectCallback where the url is taken from the navigator if not passed explicitly…

brockallen commented 6 years ago

Yep, good ideas.

So to help your immediate situation then I'd suggest you parse the URL in your callback code and pass the value in as a param. That way the delimiter is moot.

micha149 commented 6 years ago

We don't need to help in my current situation. I already fixed the wrong redirect URL from the authorization service and all works fine now. This was only a suggestion to help people with similar problems in the future.

No unnecessary haste!