search

doorkeeper-gem / doorkeeper-openid_connect

OpenID Connect extension for Doorkeeper
MIT License
175 stars 117 forks source link

Bug fix: Fragment urls for implicit flow error redirects #118

Closed joeljunstrom closed 4 years ago

joeljunstrom commented 4 years ago

We noticed that when using the promt=none option our client did not parse the returned login error.

This was due to that the computed redirect url for errors used query strings instead of fragments even if the response_type was of the id_token type.

This PR aims to ensure that the same response mode is used regardless on success or error as per https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthError

If the End-User denies the request or the End-User authentication fails, the Authorization Server MUST return the error Authorization Response in the fragment component of the Redirection URI … unless a different Response Mode was specified.

and https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#Combinations

When supplied as the value for the response_type parameter, a successful response MUST include an Access Token, an Access Token Type, and an id_token. The default Response Mode for this Response Type is the fragment encoding and the query encoding MUST NOT be used. Both successful and error responses SHOULD be returned using the supplied Response Mode, or if none is supplied, using the default Response Mode.

It does not try to fix responding to the response mode parameter as this seems to be unsupported by doorkeeper at the moment (unless I'm misstaken, it has been known to happen before).

Further refactoring could be made to ensure the error responses is not as tied to the pre_auth as it is currently but I tried to keep the change small.

Do note that this is the first time I've touched openid / oauth stuff so sorry if I misunderstood how this is supposed to work! =)