Add redirect_uri to (Deferred) Credential (Error) Response to continue interaction with end user#723
Add redirect_uri to (Deferred) Credential (Error) Response to continue interaction with end user#723
Conversation
paulbastian
left a comment
There was a problem hiding this comment.
Some suggestions, but generally looks good to me.
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
fkj
left a comment
There was a problem hiding this comment.
I have some editorial suggestions, but the direction look good to me.
Co-authored-by: Frederik Krogsdal Jacobsen <fkj@users.noreply.github.com>
| * `transaction_id`: OPTIONAL. String identifying a Deferred Issuance transaction. This parameter is contained in the response if the Credential Issuer cannot immediately issue the Credential. The value is subsequently used to obtain the respective Credential with the Deferred Credential Endpoint (see (#deferred-credential-issuance)). It MUST not be used if the `credentials` parameter is present. It MUST be invalidated after the Credential for which it was meant has been obtained by the Wallet. | ||
| * `interval`: REQUIRED if `transaction_id` is present. Contains a positive number that represents the minimum amount of time in seconds that the Wallet SHOULD wait after receiving the response before sending a new request to the Deferred Credential Endpoint. It MUST NOT be used if the `credentials` parameter is present. | ||
| * `notification_id`: OPTIONAL. String identifying one or more Credentials issued in one Credential Response. It MUST be included in the Notification Request as defined in (#notification). It MUST not be used if the `credentials` parameter is not present. | ||
| * `redirect_uri`: OPTIONAL. String containing a URI. When this parameter is present, the Wallet SHOULD suggest the End-User to redirect the user agent to this URI once the Credential issuance is completed, been deferred or has failed. This allows the Issuer to continue the interaction with the End-User. If the Wallet sends multiple consecutive Credential Requests and receives multiple `redirect_uri` values, the Wallet SHOULD provide the option to redirect to at least one of them after the last response has been processed. See implementing considerations in (#redirect-uri-ambiguity) to resolve ambiguity. |
There was a problem hiding this comment.
General question ons redirect_uri from a security perspective:
- Should this be restricted to the same origin of the issuer / issuance flow? Imho a good idea
- Restrict to only https
- We should probably add wording along the lines, that the wallet should display the URI when asking for user consent?
There was a problem hiding this comment.
Maybe not display the whole URI (which could be long), but at least the origin? I agree we should restrict to https, and probably to issuer origin.
There was a problem hiding this comment.
I think a restriction to the same origin (as credential_issuer) is too restrictive and does not really improve security.
If the legitimate issuer wants to redirect the user to a non‑same‑origin URL, they could do so anyway via a browser redirect.
If an attacker is able to tamper with TLS‑protected traffic, then fixing the origin of redirect_uri would probably not provide a significant security gain.
If we restrict it to https, could this prevent some legitimate app‑to‑app scenarios?
Showing the origin to the user seems fine to me.
There was a problem hiding this comment.
I agree with @mickrau , we assume that redirect_uri cannot be tampered with, otherwise we have much bigger problems, so I believe restricting to origin of credential_issuer_url doesn't provide benefits.
Restricting to https, would enforce issuers that want to to redirect to a native app to use claimed URLs, correct? Should be okay though..
There was a problem hiding this comment.
Claimed URLs are safer since they provide more protection against spoofed apps through assetlinks.json/apple-app-site-association. One thing that we should probably allow from a UX perspective is for the wallet to either display the origin or the app that is going to open the link (i.e. the associated app). I'm not up to date on what's possible on the platforms here wrt. getting this info up front.
After more consideration, I agree that restricting to issuer origin is does not provide any security and might be a bit annoying in some cases.
There was a problem hiding this comment.
- ok, i think we all agree to restrict to
https(i will update the PR)
Regarding the points related to the origin, there isn’t a compelling argument yet.
Co-authored-by: Christian Bormann <chris.bormann@gmx.de>
| ## Redirect URI Ambiguity {#redirect-uri-ambiguity} | ||
|
|
||
| The `redirect_uri` parameter as defined in (#credential-response) and used in Credential (Error) Response and Deferred Credential (Error) Response enables the Credential Issuer to interact with the End-User after issuance is completed, has been deferred or has failed. If an access token contains authorization for multiple Credential Configurations or multiple Credential Datasets, the wallet may send multiple Credential Requests. | ||
| However, the Credential Issuer may not be able to anticipate how many Credential Requests will be received. To eliminate any ambiguity for the wallet about which `redirect_uri` to use in such multi‑credential issuance scenarios, the Credential Issuer should either use the same `redirect_uri` for all requests that share the same Access Token, or split the process into several single‑credential issuance flows. In any case, the Credential Issuer must not rely on the End-User opening the `redirect_uri`, since the End-User may choose not to do so. |
There was a problem hiding this comment.
In my opinion this doesn't make any sense and shows the problem with the current proposal.
There should be sufficient semantic information included with the redirect_uri for the Wallet to understand the purpose. Without this, it is impossible to reasonably render this to the user. Overloading a single 'redirect_uri' parameter provides none of that context and will lead to broken user experiences (or it being simply ignored).
To give some examples:
Error case:
- Is this a url relevant to a transient error message?
- Is this a url relevant to the state of the credential?
Deferred:
This is the most well defined, but being explicit that this is about the status of the deferred credential request. But knowing what this means when the user has existing successfully provisioned credentials is necessary.
Success:
Is this a one-off warm welcome? Does it get shown once to the user then never again? What about subsequent responses? In general, this would better belong in well-defined field in per-instance credential metadata rather than in the credential response.
Regarding the problem of credential datasets and whether this is the same or different: if we provided more semantic structure we could include an indicator in the response as to whether this is a 'top-level' usable redirect (applying to the access token and applicable to all credentials) or if it is specific to this instance. Making the user go through multiple issuance processes to allow for granular issuer redirects seems pretty broken.
closes #61