-
Notifications
You must be signed in to change notification settings - Fork 36
Adds the ability to continue after redirect_to_web interaction on IAE #589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
wg discussion:
|
wg discussion:
|
Co-authored-by: Daniel Fett <[email protected]>
@Sakurann FYI this isn't an option for v1.1 as it would create a breaking change when redirecting back to the wallet. That is why it needs to be included in v1.0 |
Co-authored-by: Daniel Fett <[email protected]>
I think the thought was that a new type could be defined that supported this; for example "redirect_to_web_version2" (though I'm sure we can come up with a better name). The slight downside of how it is currently in this PR is that the wallet isn't permitted to opt in to the additional complexity. (Though maybe the additional complexity is small.) I think the biggest worry is putting this in at the last minute (the vote announcement for VCI goes out on Thursday next week) and potentially missing security issues that mean we have to make breaking changes later - e.g. #595 only just came up for IAR in general. |
I think this PR should be 1.0. The additional complexity is marginal if the wallet is already planning to support redirect_to_web. The only thing I think maybe should be changed is that the From the security perspective, I don't see this being any more vulnerable than a typical authorization endpoint response and existing best practice mitigations should be sufficient. |
Me and Andy-lim. Like the general direction of this PR .i.e. a mechanism for wallet to do a follow up request so that it can send multiple creds until Issuer is satisfied, before issuing it new creds. Andy raised a good point that should there be some statement that talks about the case when code_verifier does not verify against the code_challenge. Maybe something like:
|
I support adding this to 1.0. We've encountered use cases in the past around workplace credentials where flows like these are very useful. Having a path to handle these (complex) interactions in a standardised way is good. I also agree with other commenters that allowing this doesn't seem to add more risk then was already there in the first place. |
I've made the following changes to this PR based on feedback
Why auth_session should always be unique IMO the original language in the follow up request implied this value could rotate in a given session, so to formalise this I've clarified that and also made it clear the value needs to be unique in each response. By making auth_session unique there are several benefits.
|
I don't think this is a viable solution, we just end up largely duplicating this mechanism here with 2% difference, thats likely to lead to its own security challenges.
To the contrary I don't think the wallet should be able to opt out here. Thats needless fragmentation which will just hurt interoperability. The complexity is also very low, a wallet supporting IAE will already know how to send a follow up request, all this PR does is detail how the wallet does this when receiving a new auth_session parameter from the Authorisation response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the new additions. They make things more clear.
I made one very minor suggestion to differentiate between follow-up request and the (last) follow-up request.
Co-authored-by: Jan Vereecken <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the 2 comments I've mentioned, which can be addressed editorially, this PR looks good to me.
Co-authored-by: Joseph Heenan <[email protected]>
Discussed on today's WG call - consensus to merge once above issues resolved. Joseph to check to see if Stuttgart team could have a very quick check. |
Spun out of #509, this PR adds the ability to continue an authorisation process at the IAE after a redirect_to_web interaction.
Following feedback on the WG call i've added the requirement for PKCE in the event it was used in the initial request.