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

Allow any did to be public #2295

Merged
merged 7 commits into from
Jul 18, 2023
Merged

Conversation

mkempa
Copy link
Contributor

@mkempa mkempa commented Jul 11, 2023

Having a universal resolver set in the configuration, I want to be able to create and receive invitations containing a public DID other than did:sov (Indy DID). In order to do that I must promote the DID to public after it has been created in the wallet.

This PR adds checks of the DID format to associate ledger operations to the did:sov only in the said operation, thus allowing any DID format to be set as public.

Signed-off-by: Matus Kempa <matus.kempa@gmail.com>
Signed-off-by: Matus Kempa <matus.kempa@gmail.com>
Signed-off-by: Matus Kempa <matus.kempa@gmail.com>
Signed-off-by: Matus Kempa <matus.kempa@gmail.com>
@@ -106,7 +106,7 @@ class Meta:
example="Bob",
)
did = fields.Str(
required=False, description="DID for connection invitation", **INDY_DID
required=False, description="DID for connection invitation", **GENERIC_DID
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When making these changes I had a did:web specifically in mind. I have no experience with other formats, that's why I have doubts whether GENERIC_DID is the right choice or be more restrictive.

@mkempa mkempa marked this pull request as ready for review July 11, 2023 13:18
@dbluhm
Copy link
Member

dbluhm commented Jul 11, 2023

@chumbert Interested in your thoughts on these changes

@chumbert
Copy link
Contributor

@chumbert Interested in your thoughts on these changes

Hello ! The changes themselves are fine. My personal preference would be quite outside of the scope of this PR.

Lately people have been contributing code that got us rid of the

if this did_method:
...
elif this did_method:

logic for DIDs and verification methods. I think it is time we also consider a pluggable way to handle did registration.

Now back to reality: in our deployments we have side-stepped the publish did endpoint completely for our own did:web needs:

@docs(
    tags=[OPENAPI_TAG],
    summary="Set public DID of the wallet."
    "This is limited to the wallet, no ledger interaction.",
)
@match_info_schema(DIDSchema())
@response_schema(DIDResultSchema, 200, description="The updated did.")
async def set_public_did(request: web.Request):
    did = request.match_info.get("did")
    if not did:
        raise web.HTTPBadRequest(reason="Request query must include DID")

    context: AdminRequestContext = request["context"]

    async with context.profile.transaction() as transaction:
        did_info = await transaction.inject(BaseWallet).set_public_did(did)
        await transaction.commit()

    return web.json_response(
        data={
            "did": did_info.did,
            "verkey": did_info.verkey,
            "posture": DIDPosture.get(did_info.metadata).moniker,
            "key_type": did_info.key_type.key_type,
            "method": did_info.method.method_name,
        }
    )

I'm working internally to be able to publish and share with you our whole plugin soon. In addition to marking DIDs public, it also has an endpoint to configure wallet routing for public did in multitenancy scenarios (impossible to receive connection requests based on implicit invitations otherwise), and other things less relevant to this discussion.

@chumbert
Copy link
Contributor

Quick update on the plugin we currently use: https://github.com/sicpa-dlab/aries-acapy-plugin-did-management

@swcurran swcurran requested a review from dbluhm July 17, 2023 20:38
@swcurran
Copy link
Member

@mkempa -- could you please resolve the conflicts? @dbluhm -- could you review? Looks like @chumbert likes it, so hopefully easy to do.

dbluhm
dbluhm previously approved these changes Jul 18, 2023
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@sonarcloud
Copy link

sonarcloud bot commented Jul 18, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dbluhm dbluhm enabled auto-merge July 18, 2023 14:17
@dbluhm dbluhm merged commit d1341f5 into hyperledger:main Jul 18, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants