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

Adding DID Document Builder #284

Merged
merged 15 commits into from
Feb 22, 2023
158 changes: 158 additions & 0 deletions did/builder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
package did

import (
"reflect"

"github.com/TBD54566975/ssi-sdk/util"
"github.com/google/uuid"
"github.com/pkg/errors"
)

// contexts and types are kept to avoid having cast to/from interface{} values
type DIDDocumentBuilder struct {
contexts []string
types []string
*DIDDocument
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have this be exposed to users of the DIDDocumentBuilder class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andresuribe87 I was following precedence from previous code implemented in the ssi-sdk: https://github.com/TBD54566975/ssi-sdk/blob/main/credential/builder.go#L24.

}

const (
DIDDocumentLDContext string = "https://w3id.org/did/v1"
DIDDocumentType string = "DIDDocument"
BuilderEmptyError string = "builder cannot be empty"
)

// Create a new DID Document Builder
func NewDIDDocumentBuilder() DIDDocumentBuilder {
contexts := []string{DIDDocumentLDContext}
types := []string{DIDDocumentType}
return DIDDocumentBuilder{
contexts: contexts,
types: types,
DIDDocument: &DIDDocument{
ID: uuid.NewString(),
Context: contexts,
},
}
}

// Builds the DID Document
func (builder *DIDDocumentBuilder) Build() (*DIDDocument, error) {
if builder.IsEmpty() {
return nil, errors.New(BuilderEmptyError)
Copy link
Contributor

Choose a reason for hiding this comment

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

Good practice for the error to be the constant, so callers can compare against it. You can also reuse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andresuribe87 understand. Following existing implementation here https://github.com/TBD54566975/ssi-sdk/blob/main/credential/builder.go#L70 to stay consistent. If we want to revisit how errors are handle in the builders I'm OK with that, but I think they should be consistent across the VC builder and the DIDDocument builder.

}

if err := builder.DIDDocument.IsValid(); err != nil {
return nil, errors.Wrap(err, "did doc not valid")
}

return builder.DIDDocument, nil
}

func (builder *DIDDocumentBuilder) IsEmpty() bool {
if builder == nil || builder.DIDDocument == nil {
return true
}
return reflect.DeepEqual(builder, &DIDDocumentBuilder{})
}

func (builder *DIDDocumentBuilder) AddContext(context interface{}) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func (builder *DIDDocumentBuilder) AddContext(context interface{}) error {
func (builder *DIDDocumentBuilder) AddContext(context any) error {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andresuribe87 as long as this works, I'm fine moving to any, but note https://github.com/TBD54566975/ssi-sdk/blob/main/credential/builder.go#L68 uses the interface{} type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Either works, filed #289

if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
res, err := util.InterfaceToStrings(context)
if err != nil {
return errors.Wrap(err, "malformed context")
}
uniqueContexts := util.MergeUniqueValues(builder.contexts, res)
builder.contexts = uniqueContexts
Copy link
Contributor

Choose a reason for hiding this comment

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

Why doe we have an outer layer with contexts as well?

Copy link
Contributor Author

@andorsk andorsk Feb 16, 2023

Choose a reason for hiding this comment

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

great question. I used https://github.com/TBD54566975/ssi-sdk/blob/main/credential/builder.go as a reference. The reason it was suggested was the following:

On : https://github.com/TBD54566975/ssi-sdk/blob/main/credential/builder.go#L25
// contexts and types are kept to avoid having cast to/from interface{} values

I made an assumption here since the context is also the same data type, this is a reasonable implementation to rely upon for did document builder. The reason I would give is similar to the above. I can put in comments if helpful

builder.Context = uniqueContexts
return nil
}

func (builder *DIDDocumentBuilder) SetID(id string) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}

builder.ID = id
return nil
}

func (builder *DIDDocumentBuilder) SetAlsoKnownAs(name string) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.AlsoKnownAs = name
return nil
}

func (builder *DIDDocumentBuilder) SetController(controller string) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.Controller = controller
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddVerificationMethod(m VerificationMethod) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.VerificationMethod = append(builder.VerificationMethod, m)
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddAuthentication(m VerificationMethodSet) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.Authentication = append(builder.Authentication, m)
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddAssertionMethod(m VerificationMethodSet) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.AssertionMethod = append(builder.AssertionMethod, m)
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddKeyAgreement(m VerificationMethodSet) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.KeyAgreement = append(builder.KeyAgreement, m)
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddCapabilityInvocation(m VerificationMethodSet) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.CapabilityInvocation = append(builder.CapabilityInvocation, m)
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddCapabilityDelgation(m VerificationMethodSet) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.CapabilityDelegation = append(builder.CapabilityDelegation, m)
return nil
}

// Note: Not thread safe
func (builder *DIDDocumentBuilder) AddService(s Service) error {
if builder.IsEmpty() {
return errors.New(BuilderEmptyError)
}
builder.Services = append(builder.Services, s)
return nil
}
138 changes: 138 additions & 0 deletions did/builder_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package did

import (
"testing"

"github.com/TBD54566975/ssi-sdk/crypto"
"github.com/stretchr/testify/assert"
)

// Exercise all builder methods
func TestDIDDocumentBuilder(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to split this test into multiple smaller ones, that exercise a single behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reasonable suggestion if we want to update that. I can put a note in for a future PR to move this into smaller tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not do it in this PR?

From a maintainability perspective, it's easier to have smaller, self contained tests.

FWIW, I want to add coding guidelines into the contribution doc. See #290

Copy link
Contributor Author

@andorsk andorsk Feb 22, 2023

Choose a reason for hiding this comment

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

@andresuribe87 again, this was all using the reference of the VC Builder code, which does tests in a similar way. https://github.com/TBD54566975/ssi-sdk/blob/main/credential/builder_test.go#L254. I was being consistent with the existing codebase. I think if we want to break things out, we should also address the VC Builder as well.

There are good reasons to have smaller tests, but IMO this shouldn't block the PR.

// https://www.w3.org/TR/did-core/#example-did-document-with-1-verification-method-type
exampleAuthenticationEntry := map[string]string{
"id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3",
"type": "Ed25519VerificationKey2020",
"controller": "did:example:123",
"publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf",
}

exampleCapabilityInvocationEntry := map[string]string{
"id": "did:example:123#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k",
"type": "Ed25519VerificationKey2020",
"controller": "did:example:123",
"publicKeyMultibase": "z4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN",
}

exampleAssertionEntry := map[string]string{
"id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomY",
"type": "Ed25519VerificationKey2020",
"controller": "did:example:123",
"publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA",
}

exampleCapabilityDelegationEntry := map[string]string{
"id": "did:example:123#z6Mkw94ByR26zMSkNdCUi6FNRsWnc2DFEeDXyBGJ5KTzSWyi",
"type": "Ed25519VerificationKey2020",
"controller": "did:example:123",
"publicKeyMultibase": "zHgo9PAmfeoxHG8Mn2XHXamxnnSwPpkyBHAMNF3VyXJCL",
}

// https://www.w3.org/TR/did-core/#example-did-document-with-many-different-key-types
exampleVerificationMethod := VerificationMethod{
ID: "did:example:123#key-0",
Type: "JsonWebKey2020",
Controller: "did:example:123",
PublicKeyJWK: &crypto.PublicKeyJWK{
KTY: "OKP",
CRV: "Ed25519",
X: "VCpo2LMLhn6iWku8MKvSLg2ZAoC-nlOyPVQaO3FxVeQ",
},
}

// https://www.w3.org/TR/did-core/#example-key-agreement-property-containing-two-verification-methods
exampleKeyAgreementEntry := map[string]string{
"id": "did:example:123#zC9ByQ8aJs8vrNXyDhPHHNNMSHPcaSgNpjjsBYpMMjsTdS",
"type": "X25519KeyAgreementKey2019",
"controller": "did:example:123",
"publicKeyMultibase": "z9hFgmPVfmBZwRvFEyniQDBkz9LmV7gDEqytWyGZLmDXE",
}

var builderBad DIDDocumentBuilder
_, err := builderBad.Build()
assert.Error(t, err)
notReadyErr := "builder cannot be empty"
assert.Contains(t, err.Error(), notReadyErr)
err = builderBad.AddContext("https://w3id.org/did/v1")
assert.Error(t, err)

builder := NewDIDDocumentBuilder()
_, err = builder.Build()
assert.NoError(t, err)
assert.False(t, builder.IsEmpty())

// default context should be set
assert.NotEmpty(t, builder.Context)

// set context of a bad type
err = builder.AddContext(4)
assert.Error(t, err)
assert.Contains(t, err.Error(), "malformed context")

// correct context
err = builder.AddContext("https://w3id.org/did/v1")
assert.NoError(t, err)

// there is a default id
assert.NotEmpty(t, builder.ID)

// set id
id := "test-id"
err = builder.SetID(id)
assert.NoError(t, err)

// set also known as
err = builder.SetAlsoKnownAs("aka")
assert.NoError(t, err)

// TODO: Fix test methods
// set controller
err = builder.SetController("controller")
assert.NoError(t, err)

// valid type as a []string
err = builder.AddAuthentication(exampleAuthenticationEntry)
assert.NoError(t, err)

// set issuer as a string
err = builder.AddAssertionMethod(exampleAssertionEntry)
assert.NoError(t, err)

// set issuer as a string
err = builder.AddKeyAgreement(exampleKeyAgreementEntry)
assert.NoError(t, err)

err = builder.AddCapabilityInvocation(exampleCapabilityInvocationEntry)
assert.NoError(t, err)

err = builder.AddCapabilityDelgation(exampleCapabilityDelegationEntry)
assert.NoError(t, err)

err = builder.AddVerificationMethod(exampleVerificationMethod)
assert.NoError(t, err)

err = builder.AddService(
Service{
ID: "did:example:123#linked-domain",
Type: "LinkedDomains",
ServiceEndpoint: "https://bar.example.com",
})

assert.NoError(t, err)

// build it and verify some values
cred, err := builder.Build()
assert.NoError(t, err)
assert.NotEmpty(t, cred)
assert.Equal(t, id, cred.ID)
}