remove default response_mode being fragment #389
Conversation
| @@ -340,7 +340,7 @@ The following additional considerations are given for pre-existing Authorization | |||
| : OPTIONAL. Defined in [@!RFC6749]. The Wallet MAY allow Verifiers to request presentation of Verifiable Credentials by utilizing a pre-defined scope value. See (#request_scope) for more details. | |||
|
|
|||
| `response_mode`: | |||
| : OPTIONAL. Defined in [@!OAuth.Responses]. This parameter is used (through the new Response Mode `direct_post`) to ask the Wallet to send the response to the Verifier via an HTTPS connection (see (#response_mode_post) for more details). It is also used to request signing and encrypting (see (#jarm) for more details). If the parameter is not present, the default value is `fragment`. | |||
| : REQUIRED. Defined in [@!OAuth.Responses]. This parameter is used (through the new Response Mode `direct_post`) to ask the Wallet to send the response to the Verifier via an HTTPS connection (see (#response_mode_post) for more details). It is also used to request signing and encrypting (see (#jarm) for more details). | |||
There was a problem hiding this comment.
I know what you're aiming for here (I think anyway) but does it warrant what is effect a breaking change for some?
There was a problem hiding this comment.
are you trying to say that we need to ask a wider set of implementers before making this change?
There was a problem hiding this comment.
I'm not sure what I'm saying TBH - just noting that it's a maybe more impactful change than might be apparent at first glance.
There was a problem hiding this comment.
For what it's worth, the conformance tests treat response_mode as mandatory already, but technically that is because they only support direct_post and direct_post.jwt (and we've had no requests to implement other modes).
response_mode is also already mandatory in the Browser DC API.
Is there a GitHub issue for this change? I'm not currently clear why we're making this change and we should at least state that so any verifier that does get broken knows why. But I'm currently feeling like it would be okay to make this change, my gut feeling is not too many people are using the fragment response mode, and we already have some significant breaking changes since the last ID anyway (e.g. requiring typ in request objects), so verifiers are going to have to make updates regardless and this one would be a very simple update.
There was a problem hiding this comment.
I obviously do not know all the verifiers but i know quite a few and none of them use fragment. this PR has been open for a while and we raised it in the WG few times and no one objected so i think we cleared. can also send out in the ML.
Is there a GitHub issue for this change? I'm not currently clear why we're making this change and we should at least state that so any verifier that does get broken knows why.
because this requirement does not make much sense... I think there was an issue that mentioned this that we were reviewing on the editors call.
Do we need to replace default with direct_post? personally don't think so, but wanted to clarify.
There was a problem hiding this comment.
Do we need to replace default with direct_post? personally don't think so, but wanted to clarify.
I don't have a strong opinion either way.
There was a problem hiding this comment.
While I agree with Brian that there is potential for this change to be impactful I think majority of implementers are not using this mode and therefore its not really a sensible default behaviour when the parameter is omitted so I'm in favour of the change, the risk of keeping it around is causing strange bugs and extra implementation effort
No description provided.