'unsupported_response_mode' is not a documented error code per core specifications
vivshankar opened this issue · 8 comments
Preflight checklist
- I could not find a solution in the existing issues, docs, nor discussions.
- I agree to follow this project's Code of Conduct.
- I have read and am following this repository's Contribution Guidelines.
- This issue affects my Ory Cloud project.
- I have joined the Ory Community Slack.
- I am signed up to the Ory Security Patch Newsletter.
Describe the bug
When a client is configured with specific response modes and the request contains an invalid/unsupported response mode, Fosite returns "unsupported_response_mode". This is flagged as a violation by the FAPI Advanced Final conformance suite.
It appears that this error code is not defined in both the OAuth 2.0 or Open ID Connect core specifications.
See for OAuth 2.0 - https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
See for OIDC - https://openid.net/specs/openid-connect-core-1_0.html#AuthError
While personally I think the error code is more correct, given the impact to certification, I recommend we change this to "invalid_request".
Reproducing the bug
- Configure the OAuth 2.0 client with response_modes set to "form_post".
- Perform an authorization request with response_mode=query
- The error response is "unsupported_response_mode".
Relevant log output
No response
Relevant configuration
No response
Version
0.42.2
On which operating system are you observing this issue?
Linux
In which environment are you deploying?
Docker
Additional Context
No response
Conversation from the maillist: https://lists.openid.net/pipermail/openid-specs-ab/Week-of-Mon-20140317/004680.html
Seems for whatever reason multiple OP's implemented this. But there's no clear specification so I agree.
@james-d-elliott what is your recommendation for htis issue?
Well considering ory tends to lean towards following the spec I'd probably lean towards what the author said. Without reading the relevant specs more closely I think the error parameter would most likely match the invalid_request value, with context given in the error_description parameter.
I can see there being room to implement it similarly to how other OP's implement it; but maybe allowing the implementer to choose this.. just not sure how to go about that with my current familiarity with the library. Edit: Also unsure if this is something that's been done before, and fairly sure a person implementing it could easily map this error appropriately if they wanted already.
Yes, the error can be mapped by overriding WriteAuthorizeError. Just doesn't seem worthwhile to me. Also if there is an intention to certify Hydra for FAPI Advanced Final, this will have to be done
I was thinking the solution that is most backwards compatible (i.e. not breaking) is add a method to fosite.RFC6749Error which returns a fosite.RFC6749Error after filtering the ErrorField via a method which alters unspecified error values to specified ones.
Though looking at the RFC6749Error I think this should realistically be broken into multiple parts:
- Documentation/Research (will make this process easier, and will make future endeavors easier)
- Documenting the usages of each error in (specifically which specification they come from and if there are any specific endpoints which should not return specific errors potentially) https://github.com/ory/fosite/blob/master/errors.go
- Documenting the usages of RFC6749Error outside of this (if any)
- Deciding on an action (i.e. if the above solution makes sense or another approach makes sense)
It is possible the MessageCatalog can be enhanced to compute the error and not just the error description. That way an implementer can effectively map unsupported_response_mode to invalid_request with no impact to backward compatibility. The change would effectively be in https://github.com/ory/fosite/blob/master/errors.go#L528 to invoke:
i18n.GetMessageOrDefault(e.catalog, e.ErrorField, e.lang, e.ErrorField)
With this, the translation is left to the implementer. If the implementer doesn't map the unsupported_response_mode, it will fall back to returning unsupported_response_mode.
However, this isn't exactly what MessageCatalog was meant for. It was meant to be used for globalization but it could be used for this type of translation too, I guess.
Another option is to provide a new interface called ErrorTranslator (or named differently) that offers a func similar to the MessageCatalog but meant to translate RFC6749Error into another RFC6749Error.
@aeneasr If there is no interest in this issue, I can go ahead and close it. It was a minor issue (IMO) anyway.
At least from my end this is not a priority on our roadmap :) I agree with your sentiment