openid / federation

4 stars 3 forks source link

Should we (always) return `error` and `error_description` in case of automatic registration error (to the RP redirect_uri)? #69

Open cicnavi opened 1 week ago

cicnavi commented 1 week ago

In OpenID Federation regarding 12.1.2. Authentication Error Response it is stated:

If the OP fails to establish trust with the RP, it SHOULD use an appropriate error code and an error_description that aids the RP in understanding what is wrong.

In addition to the error codes contained in the IANA "OAuth Extensions Error Registry" registry [IANA.OAuth.Parameters], this specification also defines the following error codes:

The description seems a bit vague and things seem to be implied. Only in the example we can see that this is actually a redirect response (and we can only guess that it is a RP redirect_uri). Is this supposed to be returned as per OAuth2 spec to the redirect_uri supplied in authorization request? If so, OAuth2 has some restrictions on when the redirect_uri can be used to return error with redirect and when we should only "inform" about error: (https://www.rfc-editor.org/rfc/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:

Similarly, in OpenID Core:

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.) Unless the Redirection URI is invalid, the Authorization Server returns the Client to the Redirection URI specified in the Authorization Request with the appropriate error and state parameters. Other parameters SHOULD NOT be returned. If the Redirection URI is invalid, the Authorization Server MUST NOT redirect the User Agent to the invalid Redirection URI.

In case of OAuth2 / OIDC it is implied that the client is already registered so we can check if the redirect_uri is valid. This brings me to the question - for example, in case of OpenID Federation Automatic Registration error, and the reason for error being trust chain validation failure, we won't have resolved RP metadata at all. Is it ok, in that case, to trust redirect_uri parameter supplied in authorization request and return the error to it?

Either way, I suggest being a bit more clearer in the description about error response, for example there is no mention that the error should be returned to RP redirect_uri or similar.

vdzhuvinov commented 1 week ago

As cited from RFC 6749, the authorisation endpoint of an AS / OP must not act on the redirect_uri when:

When we take these two rules and apply them to Federation, it follows that authorisation requests from a party that is not trusted must be rejected. Since the party is not trusted, the supplied redirect_uri must not be used either, meaning there is no automatic redirect from the OP to the redirect_uri with an error code.

The two Federation specific error codes defined for automatic registration error responses - missing_trust_anchor and validation_failed will get transmitted back to the caller only when we have PAR, i.e. a backchannel request to the OP. For a regular (front-channel) authentication request you may display them to the user (this is what we chose to do in the c2id implementation).

https://openid.net/specs/openid-federation-1_0.html#section-12.1.2

My proposal is to update the spec text along these lines, i.e. that the redirect_uri of a party that isn't trusted must not be used.

The naming of the validation_failed error could also be improved. We already have a bunch of OAuth and OIDC error codes and it would help to have a name that makes the Federation relation clear. With a code like trust_chain_validation_failed for instance.