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

Add did jwk #318

Merged
merged 4 commits into from
Sep 15, 2022
Merged

Add did jwk #318

merged 4 commits into from
Sep 15, 2022

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Sep 6, 2022

docker-compose.yml Show resolved Hide resolved
@peacekeeper
Copy link
Member

peacekeeper commented Sep 6, 2022

Looks good to me! @BernhardFuchs who usually reviews new drivers is currently on vacation but should be able to merge early next week.

@OR13 OR13 marked this pull request as ready for review September 6, 2022 22:08
@OR13
Copy link
Contributor Author

OR13 commented Sep 6, 2022

ahh, I am missing some contact info requirements... from https://github.com/decentralized-identity/universal-resolver/blob/main/docs/driver-development.md

@BernhardFuchs
Copy link
Member

Just tried the driver and it throws following error:

[representationNotSupported] No consumer for application/json

@peacekeeper Could that be an issue with the resolver itself?

@peacekeeper
Copy link
Member

@OR13 I know in the past we have had different perspectives on DID document representations, @context, etc.

In this case the error is caused by the fact that your resolver returns the following:

{
  "@context": "https://w3id.org/did-resolution/v1",
  "didDocument": { ... },
  "didResolutionMetadata": {
    "contentType": "application/json",   <---
    "pattern": "^did:(?web:|jwk:|key:).+$"
  },
  "didDocumentMetadata": {}
}

DID Core says this about the contentType metadata property:

The value of this property MUST be an ASCII string that is the Media Type of the conformant representations.

So I think it needs to be application/did+ld+json or application/did+json depending on your perspective/preference, but not application/json. Can you make this change, or do you think this is a bug in our Universal Resolver?

@OR13
Copy link
Contributor Author

OR13 commented Sep 14, 2022

@peacekeeper which contentType value will prevent mutation (by the resolver) of the response from the driver?

Assuming application/did+json would be safest.

did:jwk includes an @context.... but I don't want the resolver to do any processing of it.

@peacekeeper
Copy link
Member

@OR13 I think that since there is a @context, you should return application/did+ld+json.

@OR13
Copy link
Contributor Author

OR13 commented Sep 14, 2022

@peacekeeper I asked which format will not be mutated by the resolver... are you attesting that only application/did+ld+json will be delivered without mutation to a resolver client?

I don't want the resolver to do anything other than forward the bytes from the driver... how do I accomplish that?

Are you saying that your resolver deletes @context from any did document returned as application/did+json ?

@OR13
Copy link
Contributor Author

OR13 commented Sep 14, 2022

I guess another question... is didResolutionMetadata required? or can I omit it.

@peacekeeper
Copy link
Member

peacekeeper commented Sep 15, 2022

are you attesting that only application/did+ld+json will be delivered without mutation to a resolver client?

I don't want the resolver to do anything other than forward the bytes from the driver...

It depends on the "Accept" header sent by the client. For example, if the client sends Accept: application/did+cbor, but your driver returns application/did+ld+json, then the resolver will attempt to convert from application/did+ld+json to application/did+cbor, using the JSON-LD consumption rules and CBOR production rules, instead of just forwarding the bytes that came from the driver.

This way, if in the future we see another representation such as YAML, we can just add support for that in the resolver itself without having to update >40 drivers.

If the client sends Accept: application/did+ld+json or Accept: application/ld+json, and your driver returns application/did+ld+json, then the resolver will not mutate/convert anything.

If the client sends Accept: application/did+json or Accept: application/json, and your driver returns application/did+json, then the resolver will not mutate/convert anything.

In other cases, e.g. if the client requests a JSON-LD content type, but the driver returns a plain JSON content type, or the other way round, then that's the situation where we disagreed in the past, and where I guess the behavior is still not so well understood (and our implementation may be incorrect). I think in the current situation the resolver will NOT remove @context, but I'll have to check to make sure.

If you don't want the resolver to mutate/convert anything, then the safest thing would be to examine the "Accept" header yourself in the driver and return the appropriate representation type that was requested by the client (JSON-LD, JSON, CBOR, YAML, etc.)

@peacekeeper
Copy link
Member

I guess another question... is didResolutionMetadata required? or can I omit it.

You can omit it. The resolver will add its own resolution metadata in that field and merge it with what's returned by the driver, if any.

@OR13
Copy link
Contributor Author

OR13 commented Sep 15, 2022

@peacekeeper awesome!

The current did:jwk spec only describes how to create a single JSON document, but it does support @context.. so I suppose drivers will choose which content type to return it as.

future version of the did:jwk spec, might describe production rules that are different for JSON-LD or CBOR or YAML, and then the universal resolver would not need to convert for those... but thats up to the method spec authors. cc @quartzjer

@OR13
Copy link
Contributor Author

OR13 commented Sep 15, 2022

https://hub.docker.com/layers/transmute/restricted-resolver/0.0.2/images/sha256-1317e09d8c9bdd8c8a883e6c84318efea9575f34d78c0f213f954d3b2e6a533c?context=repo

❯ curl -svX GET http://localhost:8080/1.0/identifiers/did:jwk:eyJraWQiOiJ1cm46aWV0ZjpwYXJhbXM6b2F1dGg6andrLXRodW1icHJpbnQ6c2hhLTI1NjpGZk1iek9qTW1RNGVmVDZrdndUSUpqZWxUcWpsMHhqRUlXUTJxb2JzUk1NIiwia3R5IjoiT0tQIiwiY3J2IjoiRWQyNTUxOSIsImFsZyI6IkVkRFNBIiwieCI6IkFOUmpIX3p4Y0tCeHNqUlBVdHpSYnA3RlNWTEtKWFE5QVBYOU1QMWo3azQifQ
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /1.0/identifiers/did:jwk:eyJraWQiOiJ1cm46aWV0ZjpwYXJhbXM6b2F1dGg6andrLXRodW1icHJpbnQ6c2hhLTI1NjpGZk1iek9qTW1RNGVmVDZrdndUSUpqZWxUcWpsMHhqRUlXUTJxb2JzUk1NIiwia3R5IjoiT0tQIiwiY3J2IjoiRWQyNTUxOSIsImFsZyI6IkVkRFNBIiwieCI6IkFOUmpIX3p4Y0tCeHNqUlBVdHpSYnA3RlNWTEtKWFE5QVBYOU1QMWo3azQifQ HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.79.1
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/did+ld+json
< Date: Thu, 15 Sep 2022 15:21:51 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< Transfer-Encoding: chunked
< 
{
  "@context": "https://w3id.org/did-resolution/v1",
  "didDocument": {
    "@context": [
      "https://www.w3.org/ns/did/v1",
      {
        "@vocab": "https://www.iana.org/assignments/jose#"
      }
    ],
    "id": "did:jwk:eyJraWQiOiJ1cm46aWV0ZjpwYXJhbXM6b2F1dGg6andrLXRodW1icHJpbnQ6c2hhLTI1NjpGZk1iek9qTW1RNGVmVDZrdndUSUpqZWxUcWpsMHhqRUlXUTJxb2JzUk1NIiwia3R5IjoiT0tQIiwiY3J2IjoiRWQyNTUxOSIsImFsZyI6IkVkRFNBIiwieCI6IkFOUmpIX3p4Y0tCeHNqUlBVdHpSYnA3RlNWTEtKWFE5QVBYOU1QMWo3azQifQ",
    "verificationMethod": [
      {
        "id": "#0",
        "type": "JsonWebKey2020",
        "controller": "did:jwk:eyJraWQiOiJ1cm46aWV0ZjpwYXJhbXM6b2F1dGg6andrLXRodW1icHJpbnQ6c2hhLTI1NjpGZk1iek9qTW1RNGVmVDZrdndUSUpqZWxUcWpsMHhqRUlXUTJxb2JzUk1NIiwia3R5IjoiT0tQIiwiY3J2IjoiRWQyNTUxOSIsImFsZyI6IkVkRFNBIiwieCI6IkFOUmpIX3p4Y0tCeHNqUlBVdHpSYnA3RlNWTEtKWFE5QVBYOU1QMWo3azQifQ",
        "publicKeyJwk": {
          "kid": "urn:ietf:params:oauth:jwk-thumbprint:sha-256:FfMbzOjMmQ4efT6kvwTIJjelTqjl0xjEIWQ2qobsRMM",
          "kty": "OKP",
          "crv": "Ed25519",
          "alg": "EdDSA",
          "x": "ANRjH_zxcKBxsjRPUtzRbp7FSVLKJXQ9APX9MP1j7k4"
        }
      }
    ],
    "authentication": [
      "#0"
    ],
    "assertionMethod": [
      "#0"
    ],
    "capabilityInvocation": [
      "#0"
    ],
    "capabilityDelegation": [
      "#0"
    ]
  },
  "didResolutionMetadata": {
    "contentType": "application/did+ld+json",
    "pattern": "^did:(?web:|jwk:|key:).+$"
  },
  "didDocumentMetadata": {}
* Connection #0 to host localhost left intact
}

@peacekeeper
Copy link
Member

Docker image looks a bit huge..

transmute/restricted-resolver         latest        e6446664a743   2 hours ago         1.04GB

But otherwise looks great.. @BernhardFuchs could you try again and then merge?

@cre8
Copy link

cre8 commented Jun 27, 2023

@peacekeeper @OR13I reduced the size of the image by 80% by this pull: transmute-industries/restricted-resolver#3 needs a merge

@OR13
Copy link
Contributor Author

OR13 commented Jun 30, 2023

I merged, but we need we may need to push it manually, can't remember if I automated CD.

@cre8
Copy link

cre8 commented Jun 30, 2023

@OR13 according to the CD it gets triggered with a new release, not by a push:
https://github.com/transmute-industries/restricted-resolver/blob/main/.github/workflows/cd.yml#L5C13-L5C22

@acarnagey
Copy link

I went ahead and pushed manually and tagging version 0.0.3 and latest with the new docker build docker run -d -p 8080:8080 transmute/restricted-resolver should be on the newest version that has the slimmer image

@OR13
Copy link
Contributor Author

OR13 commented Jun 30, 2023

@cre8 Thank you! if you ever want to get in touch with us, feel free to ping us in DIF slack!

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.

None yet

5 participants