Skip to content
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

did-exchange implicit request pthid update & invitation key verification #1599

Merged
merged 30 commits into from
Apr 7, 2022

Conversation

shaangill025
Copy link
Contributor

@shaangill025 shaangill025 commented Jan 13, 2022

Signed-off-by: Shaanjot Gill [email protected]
resolve #1595
resolve #1594

Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
@shaangill025 shaangill025 changed the title did-exchange implicit request pthid update - compliant with RFC0023 did-exchange implicit request pthid update & invitation key verification Jan 18, 2022
@swcurran
Copy link
Contributor

swcurran commented Feb 8, 2022

Can we get a review of this @TimoGlastra @andrewwhitehead ? Can we merge it? Thanks!

@@ -740,7 +746,9 @@ async def accept_response(
raise DIDXManagerError("No DIDDoc attached; cannot connect to public DID")
async with self.profile.session() as session:
wallet = session.inject(BaseWallet)
conn_did_doc = await self.verify_diddoc(wallet, response.did_doc_attach)
conn_did_doc = await self.verify_diddoc(
wallet, response.did_doc_attach, conn_rec.invitation_key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks whether the did document is signed with the first verification method (code above in this file assign conn_rec.invitation_key to first verificationMethod). I think this should be signed using an authentication key. Also I'm not sure which key we should check (the RFC is not very clear), but it could be signed with the second (or third) authentication key I guess.

Maybe @andrewwhitehead has some insights on this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based upon the clarification received during Aries WG 2022/03/23, updated verify method and now using jws.header.kid to verify signature.

@andrewwhitehead andrewwhitehead merged commit 8970e5b into openwallet-foundation:main Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants