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

Added DID Configuration Verification Endpoint #592

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

andresuribe87
Copy link
Contributor

Overview

This PR adds the v1/did-configurations/verification endpoint, with documentation. It follows the spec in https://identity.foundation/.well-known/resources/did-configuration/#did-configuration-resource-verification

Description

Now that we have an endpoint that can create a DIDConfiguration, this PR adds another endpoint so that clients can verify they've done the correct steps when hosting it (or that any other DID Configuration is correct).

How Has This Been Tested?

Unit tests were added. Integration tests were not added, since verifying https://www.tbd.website fails with the following reason : verifying JWT credential: error getting key to verify credential<>: did<did:key:z6Mkh4A6QTNCAFi4NZfnWt8qNSGXDGnNXhxXWewcpBrzS19v> has no verification methods with kid: did:key:z6Mkh4A6QTNCAFi4NZfnWt8qNSGXDGnNXhxXWewcpBrzS19v. I.e. we need to update the JWT to use the verification method ID, instead of the DID. This is tracked in a separate issue.

Checklist

References

https://identity.foundation/.well-known/resources/did-configuration/#did-configuration-resource-verification

@codecov-commenter
Copy link

Codecov Report

Merging #592 (ac348b0) into main (6e978ce) will decrease coverage by 0.07%.
The diff coverage is 13.04%.

@@            Coverage Diff             @@
##             main     #592      +/-   ##
==========================================
- Coverage   24.05%   23.98%   -0.07%     
==========================================
  Files          45       45              
  Lines        5330     5349      +19     
==========================================
+ Hits         1282     1283       +1     
- Misses       3807     3825      +18     
  Partials      241      241              
Impacted Files Coverage Δ
pkg/server/router/did_configuration.go 0.00% <0.00%> (ø)
pkg/server/server.go 61.24% <100.00%> (+0.15%) ⬆️

//
// 1. The credentialSubject.id MUST be a DID, and the value MUST be equal to both the Subject and Issuer of the Domain Linkage Credential.
credentialSubjectID := domainLinkageCredential.Credential.CredentialSubject["id"].(string)
if !strings.HasPrefix(credentialSubjectID, "did:") {
Copy link
Member

Choose a reason for hiding this comment

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

believe there's a method IsValidDID or similar in the utils somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I searched for something similar, but couldn't find anything.

func NewDIDConfigurationService(keyStoreService *keystore.Service) *DIDConfigurationService {
return &DIDConfigurationService{keyStoreService: keyStoreService}
func NewDIDConfigurationService(keyStoreService *keystore.Service, didResolver resolution.Resolver, schema *schema.Service) (*DIDConfigurationService, error) {
client := &http.Client{Transport: otelhttp.NewTransport(http.DefaultTransport)}
Copy link
Member

Choose a reason for hiding this comment

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

should this be the default client (with timeouts, etc) and then add in the otel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DefaultClient is the same as the zero value of http.Client. Timeouts are carried by the ctx when the call is made.

We could have an httpClientProvider, but then that get's into the dependency injection and I wanted to keep this simple.

@andresuribe87 andresuribe87 merged commit 4b0e34d into TBD54566975:main Jul 13, 2023
6 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.

3 participants