Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
request_uri extension #59
request_uri extension #59
Changes from 5 commits
0ef69f7
b402aa7
673fcf7
9dff282
f945d54
0a42d0f
37fbccf
b7c2570
350391d
7822756
8056fc1
9061d50
ba585be
b1ee70e
d29107b
d198019
30d59a8
63a86f7
70ac39b
e35ab32
8cb645f
0ced90a
42f6866
4f335fc
18fd478
bfc2bdc
acd0c13
1101fda
c1ee35a
357222c
0844acb
f0fa298
cfb2d36
2c49b9b
21696fd
a8f4917
2f08615
9a021bb
8719239
e7b8871
9d78904
0e58e39
af6c572
02a58d0
76098d7
8666529
396f007
b1e94aa
d9a49d9
63d1f7f
6edd8d1
e8a6cde
e1276ec
6d5601b
c4d175b
3b0a9fb
df5d0f1
f4e4236
ac1ffc0
803cbe6
95ac037
6890440
148e1bd
ae65453
e3df8ac
0c9413d
91f811f
56ef646
a21eba3
efd0722
4ea299d
3208b16
55a4faf
5a0b1af
bee5ed9
286bf67
3c298b7
4540cca
e5add86
06aef10
e437b50
4140e35
1e033f7
d167829
fb39140
674d0aa
fe8ae63
1088682
c6057d9
e937d3a
f36cfe6
cdfea43
1737a30
d2ba099
a478244
c6db7d5
d3e6894
17a60fc
ada0a45
6f9c661
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Since we had a signed authz request and presumably a secure channel then signing the presentation request object should be optional
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.
you are right, we already have authenticated the verifier at that stage.
What do others think? Should we make the signature of the presentation request optional?
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 think in the meeting last week, we agreed that having unsigned requests is a viable approach but we should create a new issue for that, after we merged this PR.
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.
Why is this a
SHOULD
? If there is a security reason for not using signed authorization requests, then I would like to see aMUST
. If this can be used withclient_id_schemes
that do not require signed authorization requests, then I would prefer to remove this sentence.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 think if the RP is registered in some trust framework (aka client_id_scheme), which the wallet can access using its own configured information, then a signed request is not needed if the wallet can determine that the asserted client_id is trustworthy and the wallet is not relying on any other unsigned information sent by the RP in the authz request i.e. the trust framework can provide the wallet with all the necessary metadata in a trustworthy manner.
i.e. it does not help a fake RP to pretend to be a trustworthy one, if the wallet sends the VP to the trustworthy RP's endpoint (except perhaps as a complex DOS attack on the trustworthy RP?)
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.
Yes this was my understanding too. If there is nothing more to it than this then I think we should remove this sentence, since it isn't helpful to say the request "SHOULD" be signed without any further context. If we haven't already we should probably add text to the effect of what you just described David, i.e. how a wallet can trust signed requests and when it can trust unsigned requests.
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.
added some context - the primary purpose of the signature is to authenticate the verifier early in the process to prevent abuse of the request URI callback for tracking.
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.
It might be worth adding some text along the lines of
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.
This seems duplicate to
wallet_metadata
and may add more difficulty for the wallet if it has to merge the metadata from two possiblities. I'm in favor of only transmitting the ata withwallet_metadata
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.
Those are two options to pass metadata.
wallet_metadata allows to send dynamically created data, issuer has the advantage of giving some binding between the issuer URL and the actual metadata. I think both are valid options (until we have substantial implementation experience).