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.

Sign up for GitHub

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

VDR: SubjectManager returns DIDs in preferred order #3291

Merged
merged 11 commits into from
Sep 13, 2024
Merged

VDR: SubjectManager returns DIDs in preferred order #3291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c50a1c0
VDR: SubjectManager returns DIDs in preferred order
reinkrul Aug 1, 2024
4cac54a
Update vdr/didsubject/manager.go
reinkrul Aug 1, 2024
17695e1
allow duplicates
reinkrul Aug 1, 2024
daa6ae0
docs
reinkrul Aug 1, 2024
79ae713
failing test
gerardsn Aug 6, 2024
9e7df4d
Merge branch 'master' into vdr/subjectmanager-stable-order
reinkrul Sep 6, 2024
aceee29
Fixed taking first DID for OpenID Configuration signing key
reinkrul Sep 6, 2024
1767f44
Merge remote-tracking branch 'origin/vdr/subjectmanager-stable-order'…
reinkrul Sep 6, 2024
13c980a
elaborated on vdr.didmethods flag
reinkrul Sep 6, 2024
d9e5d0f
make function name more precise
reinkrul Sep 6, 2024
200ab8b
PR feedback
reinkrul Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ The following options can be configured on the server:
storage.session.redis.tls.truststorefile PEM file containing the trusted CA certificate(s) for authenticating remote Redis session servers. Can only be used when connecting over TLS (use 'rediss://' as scheme in address).
storage.sql.connection Connection string for the SQL database. If not set it, defaults to a SQLite database stored inside the configured data directory. Note: using SQLite is not recommended in production environments. If using SQLite anyways, remember to enable foreign keys ('_foreign_keys=on') and the write-ahead-log ('_journal_mode=WAL').
**VDR**
vdr.didmethods [web,nuts] Comma-separated list of enabled DID methods (without did: prefix).
vdr.didmethods [web,nuts] Comma-separated list of enabled DID methods (without did: prefix). It also controls the order in which DIDs are returned by APIs, and which DID is used for signing if the verifying party does not impose restrictions on the DID method used.
**policy**
policy.directory ./config/policy Directory to read policy files from. Policy files are JSON files that contain a scope to PresentationDefinition mapping.
======================================== =================================================================================================================================================================================================================================================================================================================================================================================================================================================================== ============================================================================================================================================================================================================================================================================================================================================
Expand Down
9 changes: 6 additions & 3 deletions auth/api/iam/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,15 +628,15 @@ func (r Wrapper) OpenIDConfiguration(ctx context.Context, request OpenIDConfigur
if errors.Is(err, didsubject.ErrSubjectNotFound) {
return nil, oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "subject not found",
Description: err.Error(),
}
}
return nil, oauth.OAuth2Error{
Code: oauth.ServerError,
InternalError: err,
}
}
// resolve DID keys
// resolve DID key
set := jwk.NewSet()
var signingKey string
for _, currentDID := range dids {
Expand All @@ -657,7 +657,10 @@ func (r Wrapper) OpenIDConfiguration(ctx context.Context, request OpenIDConfigur
}
_ = jwkKey.Set(jwk.KeyIDKey, kid)
_ = set.AddKey(jwkKey)
signingKey = kid
// The DID is the preferred DID method (as configured), so take a key of the first DID as signing key
if signingKey == "" {
signingKey = kid
}
}
// we sign with a JWK, the receiving party can verify with the signature but not if the key corresponds to the DID since the DID method might not be supported.
// this is a shortcoming of the openID federation vs OpenID4VP/DID worlds
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/deployment/server_options.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
storage.session.redis.tls.truststorefile PEM file containing the trusted CA certificate(s) for authenticating remote Redis session servers. Can only be used when connecting over TLS (use 'rediss://' as scheme in address).
storage.sql.connection Connection string for the SQL database. If not set it, defaults to a SQLite database stored inside the configured data directory. Note: using SQLite is not recommended in production environments. If using SQLite anyways, remember to enable foreign keys ('_foreign_keys=on') and the write-ahead-log ('_journal_mode=WAL').
**VDR**
vdr.didmethods [web,nuts] Comma-separated list of enabled DID methods (without did: prefix).
vdr.didmethods [web,nuts] Comma-separated list of enabled DID methods (without did: prefix). It also controls the order in which DIDs are returned by APIs, and which DID is used for signing if the verifying party does not impose restrictions on the DID method used.
**policy**
policy.directory ./config/policy Directory to read policy files from. Policy files are JSON files that contain a scope to PresentationDefinition mapping.
======================================== =================================================================================================================================================================================================================================================================================================================================================================================================================================================================== ============================================================================================================================================================================================================================================================================================================================================
3 changes: 2 additions & 1 deletion vdr/cmd/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ func FlagSet() *pflag.FlagSet {

defs := vdr.DefaultConfig()

flagSet.StringSlice("vdr.didmethods", defs.DIDMethods, "Comma-separated list of enabled DID methods (without did: prefix).")
flagSet.StringSlice("vdr.didmethods", defs.DIDMethods, "Comma-separated list of enabled DID methods (without did: prefix). "+
"It also controls the order in which DIDs are returned by APIs, and which DID is used for signing if the verifying party does not impose restrictions on the DID method used.")
return flagSet
}

Expand Down
56 changes: 55 additions & 1 deletion vdr/didsubject/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ import (
"github.com/nuts-foundation/nuts-node/storage/orm"
"github.com/nuts-foundation/nuts-node/vdr/log"
"gorm.io/gorm"
"strings"
"regexp"
"sort"
"strings"
"time"
)

Expand All @@ -54,6 +55,8 @@ type Manager struct {
DB *gorm.DB
MethodManagers map[string]MethodManager
KeyStore nutsCrypto.KeyStore
// PreferredOrder is the order in which the methods are preferred, which dictates the order in which they are returned.
PreferredOrder []string
}

func (r *Manager) List(_ context.Context) (map[string][]did.DID, error) {
Expand All @@ -70,6 +73,9 @@ func (r *Manager) List(_ context.Context) (map[string][]did.DID, error) {
}
result[sqlDID.Subject] = append(result[sqlDID.Subject], *id)
}
for currentSubject := range result {
sortDIDsByMethod(result[currentSubject], r.PreferredOrder)
}
return result, nil
}

Expand All @@ -87,6 +93,7 @@ func (r *Manager) ListDIDs(_ context.Context, subject string) ([]did.DID, error)
}
result[i] = *id
}
sortDIDsByMethod(result, r.PreferredOrder)
return result, nil
}

Expand Down Expand Up @@ -193,6 +200,7 @@ func (r *Manager) Create(ctx context.Context, options CreationOptions) ([]did.Do
docs = append(docs, doc)
dids = append(dids, sqlDoc.DID.ID)
}
sortDIDDocumentsByMethod(docs, r.PreferredOrder)
log.Logger().
WithField(core.LogFieldDIDSubject, subject).
Infof("Created new subject (DIDs: [%s])", strings.Join(dids, ", "))
Expand Down Expand Up @@ -589,3 +597,49 @@ func (r *Manager) Rollback(ctx context.Context) {
log.Logger().WithError(err).Error("failed to rollback DID documents")
}
}

func sortDIDsByMethod(list []did.DID, methodOrder []string) {
sort.Slice(list, func(i, j int) bool {
// if the DIDs are the same, use string compare
if list[i] == list[j] {
return list[i].String() < list[j].String()
}

iOrder := -1
jOrder := -1
for k, v := range methodOrder {
if v == list[i].Method {
iOrder = k
}
if v == list[j].Method {
jOrder = k
}
}
// If both are -1, they are not in the preferred methodOrder list, so sort by method for stable methodOrder
if iOrder == -1 && jOrder == -1 {
return list[i].Method < list[j].Method
}
return iOrder < jOrder
})
}

// sortDIDDocumentsByMethod sorts a list of DID documents by the methods of their ID, according to the given order.
func sortDIDDocumentsByMethod(list []did.Document, methodOrder []string) {
listOfDIDs := make([]did.DID, len(list))
for i, doc := range list {
listOfDIDs[i] = doc.ID
}
sortDIDsByMethod(listOfDIDs, methodOrder)
// methodOrder list according to listOfDIDs
orderedList := make([]did.Document, len(list))
for i, id := range listOfDIDs {
inner:
for _, doc := range list {
if doc.ID == id {
orderedList[i] = doc
break inner
}
}
}
reinkrul marked this conversation as resolved.
Show resolved Hide resolved
copy(list, orderedList)
}
55 changes: 45 additions & 10 deletions vdr/didsubject/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ func TestManager_List(t *testing.T) {
t.Run("2 subjects with each 2 DIDs", func(t *testing.T) {
db := testDB(t)
m := Manager{DB: db, MethodManagers: map[string]MethodManager{
"example1": testMethod{},
"example2": testMethod{},
}}
"example1": testMethod{method: "example1"},
"example2": testMethod{method: "example2"},
}, PreferredOrder: []string{"example2", "example1"}}
_, _, err := m.Create(audit.TestContext(), DefaultCreationOptions().With(SubjectCreationOption{Subject: "subject1"}))
require.NoError(t, err)
_, _, err = m.Create(audit.TestContext(), DefaultCreationOptions().With(SubjectCreationOption{Subject: "subject2"}))
Expand All @@ -57,23 +57,35 @@ func TestManager_List(t *testing.T) {
assert.Len(t, result["subject1"], 2)
assert.Contains(t, result, "subject2")
assert.Len(t, result["subject2"], 2)

t.Run("preferred order", func(t *testing.T) {
assert.Equal(t, "example2", result["subject1"][0].Method)
assert.Equal(t, "example1", result["subject1"][1].Method)
assert.Equal(t, "example2", result["subject2"][0].Method)
assert.Equal(t, "example1", result["subject2"][1].Method)
})
})
}

func TestManager_ListDIDs(t *testing.T) {
t.Run("ok", func(t *testing.T) {
db := testDB(t)
m := Manager{DB: db, MethodManagers: map[string]MethodManager{
"example": testMethod{},
}}
"example1": testMethod{method: "example1"},
"example2": testMethod{method: "example2"},
}, PreferredOrder: []string{"example2", "example1"}}
opts := DefaultCreationOptions().With(SubjectCreationOption{Subject: "subject"})
_, subject, err := m.Create(audit.TestContext(), opts)
require.NoError(t, err)

dids, err := m.ListDIDs(audit.TestContext(), subject)

require.NoError(t, err)
assert.Len(t, dids, 1)
assert.Len(t, dids, 2)
t.Run("preferred order", func(t *testing.T) {
assert.True(t, strings.HasPrefix(dids[0].String(), "did:example2:"))
assert.True(t, strings.HasPrefix(dids[1].String(), "did:example1:"))
})
})
t.Run("unknown subject", func(t *testing.T) {
db := testDB(t)
Expand Down Expand Up @@ -107,7 +119,10 @@ func TestManager_Create(t *testing.T) {
})
t.Run("multiple methods", func(t *testing.T) {
db := testDB(t)
m := Manager{DB: db, MethodManagers: map[string]MethodManager{"example": testMethod{}, "test": testMethod{}}}
m := Manager{DB: db, MethodManagers: map[string]MethodManager{
"example": testMethod{},
"test": testMethod{method: "test"},
}, PreferredOrder: []string{"test", "example"}}

documents, _, err := m.Create(audit.TestContext(), DefaultCreationOptions())
require.NoError(t, err)
Expand All @@ -116,7 +131,7 @@ func TestManager_Create(t *testing.T) {
for i, document := range documents {
IDs[i] = document.ID.String()
}
assert.True(t, strings.HasPrefix(IDs[0], "did:example:"))
assert.True(t, strings.HasPrefix(IDs[0], "did:test:"))
assert.True(t, strings.HasPrefix(IDs[1], "did:example:"))

// test alsoKnownAs requirements
Expand Down Expand Up @@ -168,7 +183,9 @@ func TestManager_Create(t *testing.T) {

func TestManager_Services(t *testing.T) {
db := testDB(t)
m := Manager{DB: db, MethodManagers: map[string]MethodManager{"example": testMethod{}}}
m := Manager{DB: db, MethodManagers: map[string]MethodManager{
"example": testMethod{},
}}
subject := "subject"
opts := DefaultCreationOptions().With(SubjectCreationOption{Subject: subject})
documents, _, err := m.Create(audit.TestContext(), opts)
Expand Down Expand Up @@ -423,10 +440,15 @@ func TestNewIDForService(t *testing.T) {
type testMethod struct {
committed bool
error error
method string
}

func (t testMethod) NewDocument(_ context.Context, _ orm.DIDKeyFlags) (*orm.DIDDocument, error) {
id := fmt.Sprintf("did:example:%s", uuid.New().String())
method := t.method
if method == "" {
method = "example"
}
id := fmt.Sprintf("did:%s:%s", method, uuid.New().String())
return &orm.DIDDocument{DID: orm.DID{ID: id}}, t.error
}

Expand All @@ -443,3 +465,16 @@ func (t testMethod) Commit(_ context.Context, _ orm.DIDChangeLog) error {
func (t testMethod) IsCommitted(_ context.Context, _ orm.DIDChangeLog) (bool, error) {
return t.committed, t.error
}

func Test_sortDIDDocumentsByMethod(t *testing.T) {
documents := []did.Document{
{ID: did.MustParseDID("did:example:1")},
{ID: did.MustParseDID("did:test:1")},
}

sortDIDDocumentsByMethod(documents, []string{"test", "example"})

require.Len(t, documents, 2)
assert.Equal(t, "did:test:1", documents[0].ID.String())
assert.Equal(t, "did:example:1", documents[1].ID.String())
}
Loading
Loading