Clarify that returning errors to the client is a MUST

Issue #1023 closed
Joseph Heenan created an issue

As discussed with Mike Jones/John Bradley, the language in section 3.1.2.6 Authentication Error Response ( http://openid.net/specs/openid-connect-core-1_0.html#AuthError ) is currently:

If the End-User denies the request or the End-User authentication fails, the OP (Authorization Server) informs the RP (Client) by using the Error Response parameters defined in Section 4.1.2.1 of OAuth 2.0 [RFC6749]. (HTTP errors unrelated to RFC 6749 are returned to the User Agent using the appropriate HTTP status code.)

The language should probably be updated to a MUST, and I think the "If the End-User denies the request or the End-User authentication fails" part needs more clarity as the spec then goes on to define errors like invalid_request_object - some implementors have interpreted this to mean that they are not required to return invalid_request_object errors to the client (and can instead show an error to the user).

Comments (12)

  1. Brian Campbell

    @ve7jtb has also written text in Sec 3 of -ietf-oauth-closing-redirectors (which is referenced by -oauth-security-topics) suggesting that an HTTP 400 be returned to the browser in some cases rather than automatically returning errors to the client via redirection.

    While I do think the original intent was to return errors to the client, there is some tension between doing that and not having the AS automatically and immediately redirect in response to some invalid authorization/authentication requests.

    It is important/useful to return an error and control back to the client in cases where the client can reasonably take some action based on the condition (i.e. an error like login_required or interaction_required on a hidden request with prompt=none can be followed by a request without the prompt parameter in a visible context). However, it's not clear that errors like invalid_request_object or invalid_request_uri are things that a client can take meaningful action on at runtime.

  2. Justin Richer

    I think the basic pattern is covered pretty well in OAuth, from https://tools.ietf.org/html/rfc6749.html#section-4.1.2.1 :

    If the request fails due to a missing, invalid, or mismatching redirection URI, or if the client identifier is missing or invalid, the authorization server SHOULD inform the resource owner of the error and MUST NOT automatically redirect the user-agent to the invalid redirection URI.

    If the resource owner denies the access request or if the request fails for reasons other than a missing or invalid redirection URI, the authorization server informs the client by adding the following parameters to the query component of the redirection URI using the "application/x-www-form-urlencoded" format, per Appendix B:

    In other words, if you can't trust the redirect URI, don't send a message back. If you can trust the redirect URI, do.

  3. Joseph Heenan reporter

    https://tools.ietf.org/html/draft-ietf-oauth-closing-redirectors-00 points out that you even a registered redirect uri may be controlled by an attacker so can't be trusted, and suggests any case where a redirect might occur without user interaction you should not perform the redirect. The recommendation there has been incorporated into https://tools.ietf.org/html/draft-ietf-oauth-security-topics-05#section-3.8

    Several of the error responses defined in OIDC Core seem to fall into that same category - and requests with prompt=none seem particularly problematic as they really have to automatically redirect back to the client's registered redirect uri by design?

  4. Brian Campbell

    However, as @josephheenan alludes to, the primary use-case behind prompt=none requires that the browser be redirected back to the client without any user interaction so that the client can check if the user has a current valid session at the AS in a hidden iframe.

    After thinking about all this a bit more, I think that the "MUST NOT automatically redirect" from sec 3.8 of security-topics-05 is overreaching.

  5. Justin Richer

    With this context, I agree with @b_d_c regarding the prompt=none case. This is a situation that is specifically designed to signal a client about a specific situation without prompting the user. I'm not sure there's a good universal solution that allows this background-check style query but also prevents redirects. The front channel is inherently leaky in a lot of ways, I don't think we can fully prevent it and still use it the way we do.

  6. Brian Campbell

    FWIW, we are one of the "some implementors" that elected to treat invalid request object issues with an HTTP 400 rather than redirecting back to the client when we added support for the request object parameter. We did so largely based on the advice in Sec 3 of -closing-redirectors and not wanting to add another automatic error redirect condition while also thinking that an invalid_request_object error probably isn't something the client can recover from or take meaningful action on at run-time anyway.

    Was that the right decision? I'm honestly not sure.

    But it was intentional and not because of a misunderstanding of the text in http://openid.net/specs/openid-connect-core-1_0.html#AuthError

    So I don't know that any errata change to Connect Core is needed here.

  7. Torsten Lodderstedt

    I agree with Justin's statement: "if you can't trust the redirect URI, don't send a message back. If you can trust the redirect URI, do."

    Unfortunately, RFC 6749 does not talk about trust in the redirect URI but missing, invalid or mismatching redirect URIs and missing or invalid client ids.

  8. Michael Jones

    I’ll note that adding a “MUST” to a normative declarative sentence doesn’t change its meaning. The normative requirements are the same either way. So I don’t there’s a compelling reason to add “MUST” to either of the sentences “… the OP … informs the RP (Client) by using the Error Response parameters …“ or “… the Authorization Server returns the Client to the Redirection URI …”.

    Having reread both the pertinent spec text and the comments in this issue, I believe the spec text is already sufficiently clear and correct. Therefore, I’m increasingly inclined to agree with Brian when he wrote:

    So I don't know that any errata change to Connect Core is needed here.

    Would anyone object to us closing this one with no action? (If you do, please supply specific proposed text changes for us to consider with your objection.)

  9. Log in to comment