From 4b028f108ca6bb8570ac55ed5b1364521154dcd9 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 6 Dec 2023 06:05:52 +0100 Subject: [PATCH 1/6] Discovery: opaque timestamp --- discovery/interface.go | 45 ++++++- discovery/mock.go | 4 +- discovery/module.go | 24 ++-- discovery/module_test.go | 26 ++-- discovery/store.go | 114 +++++++++++++----- discovery/store_test.go | 53 +++++--- .../sql_migrations/001_discoveryservice.sql | 14 ++- 7 files changed, 205 insertions(+), 75 deletions(-) diff --git a/discovery/interface.go b/discovery/interface.go index 81ffc78d19..9c39bbb2b4 100644 --- a/discovery/interface.go +++ b/discovery/interface.go @@ -21,15 +21,56 @@ package discovery import ( "errors" "github.com/nuts-foundation/go-did/vc" + "math" + "strconv" + "strings" ) -// Timestamp is value that references a point in the list. +// Tag is value that references a point in the list. // It is used by clients to request new entries since their last query. +// It is opaque for clients: they should not try to interpret it. +// The server who issued the tag can interpret it as Lamport timestamp. +type Tag string + +// Timestamp decodes the Tag into a Timestamp, which is a monotonically increasing integer value (Lamport timestamp). +// Tags should only be decoded by the server who issued it, so the server should provide the stored tag prefix. +// The tag prefix is a random value that is generated when the service is created. +// It is not a secret; it only makes sure clients receive the complete presentation list when they switch servers for a specific Discovery Service: +// servers return the complete list when the client passes a timestamp the server can't decode. +func (t Tag) Timestamp(tagPrefix string) *Timestamp { + trimmed := strings.TrimPrefix(string(t), tagPrefix) + if len(trimmed) == len(string(t)) { + // Invalid tag prefix + return nil + } + result, err := strconv.ParseUint(trimmed, 10, 64) + if err != nil { + // Not a number + return nil + } + if result < 0 || result > math.MaxUint64 { + // Invalid uint64 + return nil + } + lamport := Timestamp(result) + return &lamport +} + +// Timestamp is the interpreted Tag. // It's implemented as lamport timestamp (https://en.wikipedia.org/wiki/Lamport_timestamp); // it is incremented when a new entry is added to the list. // Pass 0 to start at the beginning of the list. type Timestamp uint64 +// Tag returns the Timestamp as Tag. +func (l Timestamp) Tag(serviceSeed string) Tag { + return Tag(serviceSeed + strconv.FormatUint(uint64(l), 10)) +} + +func (l Timestamp) Increment() Timestamp { + return l + 1 +} + // ErrServiceNotFound is returned when a service (ID) is not found in the discovery service. var ErrServiceNotFound = errors.New("discovery service not found") @@ -43,7 +84,7 @@ type Server interface { // If the presentation is not valid or it does not conform to the Service ServiceDefinition, it returns an error. Add(serviceID string, presentation vc.VerifiablePresentation) error // Get retrieves the presentations for the given service, starting at the given timestamp. - Get(serviceID string, startAt Timestamp) ([]vc.VerifiablePresentation, *Timestamp, error) + Get(serviceID string, startAt *Tag) ([]vc.VerifiablePresentation, *Tag, error) } // Client defines the API for Discovery Clients. diff --git a/discovery/mock.go b/discovery/mock.go index 335c3bcd0d..ade7b84915 100644 --- a/discovery/mock.go +++ b/discovery/mock.go @@ -53,11 +53,11 @@ func (mr *MockServerMockRecorder) Add(serviceID, presentation any) *gomock.Call } // Get mocks base method. -func (m *MockServer) Get(serviceID string, startAt Timestamp) ([]vc.VerifiablePresentation, *Timestamp, error) { +func (m *MockServer) Get(serviceID string, startAt *Tag) ([]vc.VerifiablePresentation, *Tag, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Get", serviceID, startAt) ret0, _ := ret[0].([]vc.VerifiablePresentation) - ret1, _ := ret[1].(*Timestamp) + ret1, _ := ret[1].(*Tag) ret2, _ := ret[2].(error) return ret0, ret1, ret2 } diff --git a/discovery/module.go b/discovery/module.go index f9c5286349..f7994d249a 100644 --- a/discovery/module.go +++ b/discovery/module.go @@ -88,7 +88,7 @@ func (m *Module) Configure(_ core.ServerConfig) error { func (m *Module) Start() error { var err error - m.store, err = newSQLStore(m.storageInstance.GetSQLDatabase(), m.services) + m.store, err = newSQLStore(m.storageInstance.GetSQLDatabase(), m.services, m.serverDefinitions) if err != nil { return err } @@ -210,17 +210,7 @@ func (m *Module) validateRetraction(serviceID string, presentation vc.Verifiable return nil } -// validateAudience checks if the given audience of the presentation matches the service ID. -func validateAudience(service ServiceDefinition, audience []string) error { - for _, audienceID := range audience { - if audienceID == service.ID { - return nil - } - } - return errors.New("aud claim is missing or invalid") -} - -func (m *Module) Get(serviceID string, startAt Timestamp) ([]vc.VerifiablePresentation, *Timestamp, error) { +func (m *Module) Get(serviceID string, startAt *Tag) ([]vc.VerifiablePresentation, *Tag, error) { if _, exists := m.services[serviceID]; !exists { return nil, nil, ErrServiceNotFound } @@ -253,3 +243,13 @@ func loadDefinitions(directory string) (map[string]ServiceDefinition, error) { } return result, nil } + +// validateAudience checks if the given audience of the presentation matches the service ID. +func validateAudience(service ServiceDefinition, audience []string) error { + for _, audienceID := range audience { + if audienceID == service.ID { + return nil + } + } + return errors.New("aud claim is missing or invalid") +} diff --git a/discovery/module_test.go b/discovery/module_test.go index dde222e5fe..25448a81d1 100644 --- a/discovery/module_test.go +++ b/discovery/module_test.go @@ -58,9 +58,10 @@ func Test_Module_Add(t *testing.T) { err := m.Add(testServiceID, vpAlice) require.EqualError(t, err, "presentation verification failed: failed") - _, timestamp, err := m.Get(testServiceID, 0) + _, tag, err := m.Get(testServiceID, nil) require.NoError(t, err) - assert.Equal(t, Timestamp(0), *timestamp) + expectedTag := tagForTimestamp(t, m.store, testServiceID, 0) + assert.Equal(t, expectedTag, *tag) }) t.Run("already exists", func(t *testing.T) { m, presentationVerifier := setupModule(t, storageEngine) @@ -117,9 +118,9 @@ func Test_Module_Add(t *testing.T) { err := m.Add(testServiceID, vpAlice) require.NoError(t, err) - _, timestamp, err := m.Get(testServiceID, 0) + _, tag, err := m.Get(testServiceID, nil) require.NoError(t, err) - assert.Equal(t, Timestamp(1), *timestamp) + assert.Equal(t, "1", string(*tag)[tagPrefixLength:]) }) t.Run("valid longer than its credentials", func(t *testing.T) { m, _ := setupModule(t, storageEngine) @@ -144,8 +145,8 @@ func Test_Module_Add(t *testing.T) { err := m.Add(testServiceID, otherVP) require.ErrorContains(t, err, "presentation does not fulfill Presentation ServiceDefinition") - _, timestamp, _ := m.Get(testServiceID, 0) - assert.Equal(t, Timestamp(0), *timestamp) + _, tag, _ := m.Get(testServiceID, nil) + assert.Equal(t, "0", string(*tag)[tagPrefixLength:]) }) }) t.Run("retraction", func(t *testing.T) { @@ -205,14 +206,21 @@ func Test_Module_Get(t *testing.T) { t.Run("ok", func(t *testing.T) { m, _ := setupModule(t, storageEngine) require.NoError(t, m.store.add(testServiceID, vpAlice, nil)) - presentations, timestamp, err := m.Get(testServiceID, 0) + presentations, tag, err := m.Get(testServiceID, nil) assert.NoError(t, err) assert.Equal(t, []vc.VerifiablePresentation{vpAlice}, presentations) - assert.Equal(t, Timestamp(1), *timestamp) + assert.Equal(t, "1", string(*tag)[tagPrefixLength:]) + }) + t.Run("ok - retrieve delta", func(t *testing.T) { + m, _ := setupModule(t, storageEngine) + require.NoError(t, m.store.add(testServiceID, vpAlice, nil)) + presentations, _, err := m.Get(testServiceID, nil) + require.NoError(t, err) + require.Len(t, presentations, 1) }) t.Run("service unknown", func(t *testing.T) { m, _ := setupModule(t, storageEngine) - _, _, err := m.Get("unknown", 0) + _, _, err := m.Get("unknown", nil) assert.ErrorIs(t, err, ErrServiceNotFound) }) } diff --git a/discovery/store.go b/discovery/store.go index ecac207569..58ecfbd032 100644 --- a/discovery/store.go +++ b/discovery/store.go @@ -29,15 +29,19 @@ import ( "gorm.io/gorm" "gorm.io/gorm/clause" "gorm.io/gorm/schema" + "math/rand" "strconv" "strings" "sync" "time" ) +const tagPrefixLength = 5 + type serviceRecord struct { - ID string `gorm:"primaryKey"` - LamportTimestamp uint64 + ID string `gorm:"primaryKey"` + LastTag Tag + TagPrefix string } func (s serviceRecord) TableName() string { @@ -103,12 +107,16 @@ type sqlStore struct { writeLock sync.Mutex } -func newSQLStore(db *gorm.DB, definitions map[string]ServiceDefinition) (*sqlStore, error) { - // Creates entries in the discovery service table with initial timestamp, if they don't exist yet - for _, definition := range definitions { +func newSQLStore(db *gorm.DB, clientDefinitions map[string]ServiceDefinition, serverDefinitions map[string]ServiceDefinition) (*sqlStore, error) { + // Creates entries in the discovery service table, if they don't exist yet + for _, definition := range clientDefinitions { currentList := serviceRecord{ ID: definition.ID, } + // If the node is server for this discovery service, make sure the timestamp prefix is set. + if _, isServer := serverDefinitions[definition.ID]; isServer { + currentList.TagPrefix = generatePrefix() + } if err := db.FirstOrCreate(¤tList, "id = ?", definition.ID).Error; err != nil { return nil, err } @@ -120,10 +128,10 @@ func newSQLStore(db *gorm.DB, definitions map[string]ServiceDefinition) (*sqlSto } // Add adds a presentation to the list of presentations. -// Timestamp should be passed if the presentation was received from a remote Discovery Server, then it is stored alongside the presentation. +// Tag should be passed if the presentation was received from a remote Discovery Server, then it is stored alongside the presentation. // If the local node is the Discovery Server and thus is responsible for the timestamping, // nil should be passed to let the store determine the right value. -func (s *sqlStore) add(serviceID string, presentation vc.VerifiablePresentation, timestamp *Timestamp) error { +func (s *sqlStore) add(serviceID string, presentation vc.VerifiablePresentation, tag *Tag) error { credentialSubjectID, err := credential.PresentationSigner(presentation) if err != nil { return err @@ -142,7 +150,7 @@ func (s *sqlStore) add(serviceID string, presentation vc.VerifiablePresentation, return err } return s.db.Transaction(func(tx *gorm.DB) error { - newTimestamp, err := s.updateTimestamp(tx, serviceID, timestamp) + newTimestamp, err := s.updateTag(tx, serviceID, tag) if err != nil { return err } @@ -166,7 +174,7 @@ func (s *sqlStore) add(serviceID string, presentation vc.VerifiablePresentation, // - presentationRecord // - presentationRecord.Credentials with credentialRecords of the credentials in the presentation // - presentationRecord.Credentials.Properties of the credentialSubject properties of the credential (for s -func createPresentationRecord(serviceID string, timestamp Timestamp, presentation vc.VerifiablePresentation) (*presentationRecord, error) { +func createPresentationRecord(serviceID string, timestamp *Timestamp, presentation vc.VerifiablePresentation) (*presentationRecord, error) { credentialSubjectID, err := credential.PresentationSigner(presentation) if err != nil { return nil, err @@ -175,12 +183,14 @@ func createPresentationRecord(serviceID string, timestamp Timestamp, presentatio newPresentation := presentationRecord{ ID: uuid.NewString(), ServiceID: serviceID, - LamportTimestamp: uint64(timestamp), CredentialSubjectID: credentialSubjectID.String(), PresentationID: presentation.ID.String(), PresentationRaw: presentation.Raw(), PresentationExpiration: presentation.JWT().Expiration().Unix(), } + if timestamp != nil { + newPresentation.LamportTimestamp = uint64(*timestamp) + } for _, currCred := range presentation.VerifiableCredential { var credentialType *string @@ -221,16 +231,29 @@ func createPresentationRecord(serviceID string, timestamp Timestamp, presentatio return &newPresentation, nil } -// get returns all presentations, registered on the given service, starting after the given timestamp. -// It also returns the latest timestamp of the returned presentations. -// This timestamp can then be used next time to only retrieve presentations that were added after that timestamp. -func (s *sqlStore) get(serviceID string, startAt Timestamp) ([]vc.VerifiablePresentation, *Timestamp, error) { +// get returns all presentations, registered on the given service, starting after the given tag. +// It also returns the latest tag of the returned presentations. +// This tag can then be used next time to only retrieve presentations that were added after that tag. +func (s *sqlStore) get(serviceID string, tag *Tag) ([]vc.VerifiablePresentation, *Tag, error) { + var service serviceRecord + if err := s.db.Find(&service, "id = ?", serviceID).Error; err != nil { + return nil, nil, fmt.Errorf("query service '%s': %w", serviceID, err) + } + var startAfter uint64 + if tag != nil { + // Decode tag + lamportTimestamp := tag.Timestamp(service.TagPrefix) + if lamportTimestamp != nil { + startAfter = uint64(*lamportTimestamp) + } + } + var rows []presentationRecord - err := s.db.Order("lamport_timestamp ASC").Find(&rows, "service_id = ? AND lamport_timestamp > ?", serviceID, int(startAt)).Error + err := s.db.Order("lamport_timestamp ASC").Find(&rows, "service_id = ? AND lamport_timestamp > ?", serviceID, startAfter).Error if err != nil { return nil, nil, fmt.Errorf("query service '%s': %w", serviceID, err) } - timestamp := startAt + highestLamportClock := startAfter presentations := make([]vc.VerifiablePresentation, 0, len(rows)) for _, row := range rows { presentation, err := vc.ParseVerifiablePresentation(row.PresentationRaw) @@ -238,9 +261,10 @@ func (s *sqlStore) get(serviceID string, startAt Timestamp) ([]vc.VerifiablePres return nil, nil, fmt.Errorf("parse presentation '%s' of service '%s': %w", row.PresentationID, serviceID, err) } presentations = append(presentations, *presentation) - timestamp = Timestamp(row.LamportTimestamp) + highestLamportClock = row.LamportTimestamp } - return presentations, ×tamp, nil + newTag := Timestamp(highestLamportClock).Tag(service.TagPrefix) + return presentations, &newTag, nil } // search searches for presentations, registered on the given service, matching the given query. @@ -303,29 +327,44 @@ func (s *sqlStore) search(serviceID string, query map[string]string) ([]vc.Verif return results, nil } -// updateTimestamp updates the timestamp of the given service. -// Clients should pass the timestamp they received from the server (which simply sets it). -// Servers should pass nil (since they "own" the timestamp), which causes it to be incremented. -func (s *sqlStore) updateTimestamp(tx *gorm.DB, serviceID string, newTimestamp *Timestamp) (Timestamp, error) { - var result serviceRecord +// updateTag updates the tag of the given service. +// Clients should pass the tag they received from the server (which simply sets it). +// Servers should pass nil (since they "own" the tag), which causes it to be incremented. +// It returns +func (s *sqlStore) updateTag(tx *gorm.DB, serviceID string, newTimestamp *Tag) (*Timestamp, error) { + var service serviceRecord // Lock (SELECT FOR UPDATE) discovery_service row to prevent concurrent updates to the same list, which could mess up the lamport timestamp. if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). Where(serviceRecord{ID: serviceID}). - Find(&result). + Find(&service). Error; err != nil { - return 0, err + return nil, err } - result.ID = serviceID + service.ID = serviceID + var result *Timestamp if newTimestamp == nil { - // Increment timestamp - result.LamportTimestamp++ + // Update tag: decode current timestamp, increment it, encode it again. + currTimestamp := Timestamp(0) + if service.LastTag != "" { + // If LastTag is empty, it means the service was just created and no presentations were added yet. + ts := service.LastTag.Timestamp(service.TagPrefix) + if ts == nil { + // would be very weird + return nil, fmt.Errorf("invalid tag '%s'", service.LastTag) + } + currTimestamp = *ts + } + ts := currTimestamp.Increment() + result = &ts + service.LastTag = ts.Tag(service.TagPrefix) } else { - result.LamportTimestamp = uint64(*newTimestamp) + // Set tag: just store it + service.LastTag = *newTimestamp } - if err := tx.Save(&result).Error; err != nil { - return 0, err + if err := tx.Save(service).Error; err != nil { + return nil, err } - return Timestamp(result.LamportTimestamp), nil + return result, nil } // exists checks whether a presentation of the given subject is registered on a service. @@ -382,3 +421,14 @@ func indexJSONObject(target map[string]interface{}, jsonPaths []string, stringVa } return jsonPaths, stringValues } + +// generatePrefix generates a random seed for a service, consisting of 5 uppercase letters. +func generatePrefix() string { + result := make([]byte, tagPrefixLength) + lower := int('A') + upper := int('Z') + for i := 0; i < len(result); i++ { + result[i] = byte(lower + rand.Intn(upper-lower)) + } + return string(result) +} diff --git a/discovery/store_test.go b/discovery/store_test.go index 11b1af52c1..835f04b036 100644 --- a/discovery/store_test.go +++ b/discovery/store_test.go @@ -130,47 +130,54 @@ func Test_sqlStore_get(t *testing.T) { storageEngine := storage.NewTestStorageEngine(t) require.NoError(t, storageEngine.Start()) - t.Run("empty list, empty timestamp", func(t *testing.T) { + t.Run("empty list, empty tag", func(t *testing.T) { m := setupStore(t, storageEngine.GetSQLDatabase()) - presentations, timestamp, err := m.get(testServiceID, 0) + presentations, tag, err := m.get(testServiceID, nil) assert.NoError(t, err) assert.Empty(t, presentations) - assert.Empty(t, timestamp) + expectedTag := tagForTimestamp(t, m, testServiceID, 0) + assert.Equal(t, expectedTag, *tag) }) - t.Run("1 entry, empty timestamp", func(t *testing.T) { + t.Run("1 entry, empty tag", func(t *testing.T) { m := setupStore(t, storageEngine.GetSQLDatabase()) require.NoError(t, m.add(testServiceID, vpAlice, nil)) - presentations, timestamp, err := m.get(testServiceID, 0) + presentations, tag, err := m.get(testServiceID, nil) assert.NoError(t, err) assert.Equal(t, []vc.VerifiablePresentation{vpAlice}, presentations) - assert.Equal(t, Timestamp(1), *timestamp) + expectedTag := tagForTimestamp(t, m, testServiceID, 1) + assert.Equal(t, expectedTag, *tag) }) - t.Run("2 entries, empty timestamp", func(t *testing.T) { + t.Run("2 entries, empty tag", func(t *testing.T) { m := setupStore(t, storageEngine.GetSQLDatabase()) require.NoError(t, m.add(testServiceID, vpAlice, nil)) require.NoError(t, m.add(testServiceID, vpBob, nil)) - presentations, timestamp, err := m.get(testServiceID, 0) + presentations, tag, err := m.get(testServiceID, nil) assert.NoError(t, err) assert.Equal(t, []vc.VerifiablePresentation{vpAlice, vpBob}, presentations) - assert.Equal(t, Timestamp(2), *timestamp) + expectedTS := tagForTimestamp(t, m, testServiceID, 2) + assert.Equal(t, expectedTS, *tag) }) t.Run("2 entries, start after first", func(t *testing.T) { m := setupStore(t, storageEngine.GetSQLDatabase()) require.NoError(t, m.add(testServiceID, vpAlice, nil)) require.NoError(t, m.add(testServiceID, vpBob, nil)) - presentations, timestamp, err := m.get(testServiceID, 1) + ts := tagForTimestamp(t, m, testServiceID, 1) + presentations, tag, err := m.get(testServiceID, &ts) assert.NoError(t, err) assert.Equal(t, []vc.VerifiablePresentation{vpBob}, presentations) - assert.Equal(t, Timestamp(2), *timestamp) + expectedTS := tagForTimestamp(t, m, testServiceID, 2) + assert.Equal(t, expectedTS, *tag) }) t.Run("2 entries, start after end", func(t *testing.T) { m := setupStore(t, storageEngine.GetSQLDatabase()) require.NoError(t, m.add(testServiceID, vpAlice, nil)) require.NoError(t, m.add(testServiceID, vpBob, nil)) - presentations, timestamp, err := m.get(testServiceID, 2) + expectedTag := tagForTimestamp(t, m, testServiceID, 2) + presentations, tag, err := m.get(testServiceID, &expectedTag) assert.NoError(t, err) assert.Equal(t, []vc.VerifiablePresentation{}, presentations) - assert.Equal(t, Timestamp(2), *timestamp) + expectedTag = tagForTimestamp(t, m, testServiceID, 0) + assert.Equal(t, expectedTag, *tag) }) } @@ -351,7 +358,8 @@ func Test_sqlStore_search(t *testing.T) { func setupStore(t *testing.T, db *gorm.DB) *sqlStore { resetStore(t, db) - store, err := newSQLStore(db, testDefinitions()) + defs := testDefinitions() + store, err := newSQLStore(db, defs, defs) require.NoError(t, err) return store } @@ -371,3 +379,20 @@ func sliceToMap(slice []credentialPropertyRecord) map[string]string { } return result } + +func Test_generateSeed(t *testing.T) { + for i := 0; i < 100; i++ { + seed := generatePrefix() + assert.Len(t, seed, 5) + for _, char := range seed { + assert.True(t, char >= 'A' && char <= 'Z') + } + } +} + +func tagForTimestamp(t *testing.T, store *sqlStore, serviceID string, ts int) Tag { + t.Helper() + var service serviceRecord + require.NoError(t, store.db.Find(&service, "id = ?", serviceID).Error) + return Timestamp(ts).Tag(service.TagPrefix) +} diff --git a/storage/sql_migrations/001_discoveryservice.sql b/storage/sql_migrations/001_discoveryservice.sql index 21c4a498e5..15117948e1 100644 --- a/storage/sql_migrations/001_discoveryservice.sql +++ b/storage/sql_migrations/001_discoveryservice.sql @@ -1,10 +1,14 @@ -- migrate:up --- discovery contains the known discovery services and the highest timestamp +-- discovery contains the known discovery services and the associated tags. create table discovery_service ( -- id is the unique identifier for the service. It comes from the service definition. - id varchar(200) not null primary key, - lamport_timestamp integer not null + id varchar(200) not null primary key, + -- tag is the latest tag pointing to the last presentation registered on the service. + last_tag varchar(40) null, + -- tag_prefix is used to prefix the tag of the presentations of the service. + -- It is only populated if the node is server for this service. + tag_prefix varchar(5) null ); -- discovery_presentation contains the presentations of the discovery services @@ -12,7 +16,9 @@ create table discovery_presentation ( id varchar(36) not null primary key, service_id varchar(36) not null, - lamport_timestamp integer not null, + -- lamport_timestamp is the lamport clock of the presentation, converted to a tag and then returned to the client. + -- It is only populated if the node is server for this service. + lamport_timestamp integer null, credential_subject_id varchar not null, presentation_id varchar not null, presentation_raw varchar not null, From 29822c1b915483633b23823b7a08d0b39b8da15f Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 7 Dec 2023 09:22:41 +0100 Subject: [PATCH 2/6] fixtest --- discovery/interface.go | 5 +++ discovery/interface_test.go | 61 +++++++++++++++++++++++++++++++++++++ discovery/store.go | 10 +++--- discovery/store_test.go | 3 +- 4 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 discovery/interface_test.go diff --git a/discovery/interface.go b/discovery/interface.go index 9c39bbb2b4..d308eb91a3 100644 --- a/discovery/interface.go +++ b/discovery/interface.go @@ -56,6 +56,11 @@ func (t Tag) Timestamp(tagPrefix string) *Timestamp { return &lamport } +// Empty returns true if the Tag is empty. +func (t Tag) Empty() bool { + return len(t) == 0 +} + // Timestamp is the interpreted Tag. // It's implemented as lamport timestamp (https://en.wikipedia.org/wiki/Lamport_timestamp); // it is incremented when a new entry is added to the list. diff --git a/discovery/interface_test.go b/discovery/interface_test.go new file mode 100644 index 0000000000..3ac1a0ac75 --- /dev/null +++ b/discovery/interface_test.go @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2023 Nuts community + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package discovery + +import ( + "github.com/stretchr/testify/assert" + "testing" +) + +func TestTag_Empty(t *testing.T) { + t.Run("empty", func(t *testing.T) { + assert.True(t, Tag("").Empty()) + }) + t.Run("not empty", func(t *testing.T) { + assert.False(t, Tag("not empty").Empty()) + }) +} + +func TestTag_Timestamp(t *testing.T) { + t.Run("invalid tag prefix", func(t *testing.T) { + assert.Nil(t, Tag("invalid tag prefix").Timestamp("tag prefix")) + }) + t.Run("not a number", func(t *testing.T) { + assert.Nil(t, Tag("tag prefix").Timestamp("tag prefixnot a number")) + }) + t.Run("invalid uint64", func(t *testing.T) { + assert.Nil(t, Tag("tag prefix").Timestamp("tag prefix")) + }) + t.Run("valid (small number)", func(t *testing.T) { + assert.Equal(t, Timestamp(1), *Tag("tag prefix1").Timestamp("tag prefix")) + }) + t.Run("valid (large number)", func(t *testing.T) { + assert.Equal(t, Timestamp(1234567890), *Tag("tag prefix1234567890").Timestamp("tag prefix")) + }) +} + +func TestTimestamp_Tag(t *testing.T) { + assert.Equal(t, Tag("tag prefix1"), Timestamp(1).Tag("tag prefix")) +} + +func TestTimestamp_Increment(t *testing.T) { + assert.Equal(t, Timestamp(1), Timestamp(0).Increment()) + assert.Equal(t, Timestamp(2), Timestamp(1).Increment()) + assert.Equal(t, Timestamp(1234567890), Timestamp(1234567889).Increment()) +} diff --git a/discovery/store.go b/discovery/store.go index 58ecfbd032..d3208cfbba 100644 --- a/discovery/store.go +++ b/discovery/store.go @@ -253,7 +253,6 @@ func (s *sqlStore) get(serviceID string, tag *Tag) ([]vc.VerifiablePresentation, if err != nil { return nil, nil, fmt.Errorf("query service '%s': %w", serviceID, err) } - highestLamportClock := startAfter presentations := make([]vc.VerifiablePresentation, 0, len(rows)) for _, row := range rows { presentation, err := vc.ParseVerifiablePresentation(row.PresentationRaw) @@ -261,10 +260,13 @@ func (s *sqlStore) get(serviceID string, tag *Tag) ([]vc.VerifiablePresentation, return nil, nil, fmt.Errorf("parse presentation '%s' of service '%s': %w", row.PresentationID, serviceID, err) } presentations = append(presentations, *presentation) - highestLamportClock = row.LamportTimestamp } - newTag := Timestamp(highestLamportClock).Tag(service.TagPrefix) - return presentations, &newTag, nil + lastTag := service.LastTag + if lastTag.Empty() { + // Make sure we don't return an empty string for the tag, instead return tag indicating the beginning of the list. + lastTag = Timestamp(0).Tag(service.TagPrefix) + } + return presentations, &lastTag, nil } // search searches for presentations, registered on the given service, matching the given query. diff --git a/discovery/store_test.go b/discovery/store_test.go index 835f04b036..938c9eade5 100644 --- a/discovery/store_test.go +++ b/discovery/store_test.go @@ -168,7 +168,7 @@ func Test_sqlStore_get(t *testing.T) { expectedTS := tagForTimestamp(t, m, testServiceID, 2) assert.Equal(t, expectedTS, *tag) }) - t.Run("2 entries, start after end", func(t *testing.T) { + t.Run("2 entries, start at end", func(t *testing.T) { m := setupStore(t, storageEngine.GetSQLDatabase()) require.NoError(t, m.add(testServiceID, vpAlice, nil)) require.NoError(t, m.add(testServiceID, vpBob, nil)) @@ -176,7 +176,6 @@ func Test_sqlStore_get(t *testing.T) { presentations, tag, err := m.get(testServiceID, &expectedTag) assert.NoError(t, err) assert.Equal(t, []vc.VerifiablePresentation{}, presentations) - expectedTag = tagForTimestamp(t, m, testServiceID, 0) assert.Equal(t, expectedTag, *tag) }) } From e79528931735722e62c48ff15eebd6131868d8be Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 8 Dec 2023 08:47:57 +0100 Subject: [PATCH 3/6] feedback --- discovery/module.go | 15 ++++++--------- discovery/module_test.go | 14 +++++--------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/discovery/module.go b/discovery/module.go index f7994d249a..3685e7025c 100644 --- a/discovery/module.go +++ b/discovery/module.go @@ -109,11 +109,8 @@ func (m *Module) Config() interface{} { func (m *Module) Add(serviceID string, presentation vc.VerifiablePresentation) error { // First, simple sanity checks - definition, serviceExists := m.services[serviceID] - if !serviceExists { - return ErrServiceNotFound - } - if _, isMaintainer := m.serverDefinitions[serviceID]; !isMaintainer { + definition, isServer := m.serverDefinitions[serviceID] + if !isServer { return ErrServerModeDisabled } if presentation.Format() != vc.JWTPresentationProofFormat { @@ -210,11 +207,11 @@ func (m *Module) validateRetraction(serviceID string, presentation vc.Verifiable return nil } -func (m *Module) Get(serviceID string, startAt *Tag) ([]vc.VerifiablePresentation, *Tag, error) { - if _, exists := m.services[serviceID]; !exists { - return nil, nil, ErrServiceNotFound +func (m *Module) Get(serviceID string, tag *Tag) ([]vc.VerifiablePresentation, *Tag, error) { + if _, exists := m.serverDefinitions[serviceID]; !exists { + return nil, nil, ErrServerModeDisabled } - return m.store.get(serviceID, startAt) + return m.store.get(serviceID, tag) } func loadDefinitions(directory string) (map[string]ServiceDefinition, error) { diff --git a/discovery/module_test.go b/discovery/module_test.go index 25448a81d1..6c32c28e6b 100644 --- a/discovery/module_test.go +++ b/discovery/module_test.go @@ -45,7 +45,7 @@ func Test_Module_Add(t *testing.T) { storageEngine := storage.NewTestStorageEngine(t) require.NoError(t, storageEngine.Start()) - t.Run("not a maintainer", func(t *testing.T) { + t.Run("not a server", func(t *testing.T) { m, _ := setupModule(t, storageEngine) err := m.Add("other", vpAlice) @@ -77,6 +77,7 @@ func Test_Module_Add(t *testing.T) { def := m.services[testServiceID] def.PresentationMaxValidity = 1 m.services[testServiceID] = def + m.serverDefinitions[testServiceID] = def err := m.Add(testServiceID, vpAlice) assert.EqualError(t, err, "presentation is valid for too long (max 1s)") @@ -104,11 +105,6 @@ func Test_Module_Add(t *testing.T) { err := m.Add(testServiceID, vc.VerifiablePresentation{}) assert.EqualError(t, err, "only JWT presentations are supported") }) - t.Run("service unknown", func(t *testing.T) { - m, _ := setupModule(t, storageEngine) - err := m.Add("unknown", vpAlice) - assert.ErrorIs(t, err, ErrServiceNotFound) - }) t.Run("registration", func(t *testing.T) { t.Run("ok", func(t *testing.T) { @@ -218,10 +214,10 @@ func Test_Module_Get(t *testing.T) { require.NoError(t, err) require.Len(t, presentations, 1) }) - t.Run("service unknown", func(t *testing.T) { + t.Run("not a server for this service ID", func(t *testing.T) { m, _ := setupModule(t, storageEngine) - _, _, err := m.Get("unknown", nil) - assert.ErrorIs(t, err, ErrServiceNotFound) + _, _, err := m.Get("other", nil) + assert.ErrorIs(t, err, ErrServerModeDisabled) }) } From 08ce1aae70f3961e7fa43bd8ebc34e16b9514e37 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 8 Dec 2023 09:11:11 +0100 Subject: [PATCH 4/6] comment --- discovery/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discovery/store.go b/discovery/store.go index d3208cfbba..dcca397c4c 100644 --- a/discovery/store.go +++ b/discovery/store.go @@ -351,7 +351,7 @@ func (s *sqlStore) updateTag(tx *gorm.DB, serviceID string, newTimestamp *Tag) ( // If LastTag is empty, it means the service was just created and no presentations were added yet. ts := service.LastTag.Timestamp(service.TagPrefix) if ts == nil { - // would be very weird + // would be very weird: can't decode it, although it's our own tag return nil, fmt.Errorf("invalid tag '%s'", service.LastTag) } currTimestamp = *ts From 1c64059346221924951cd65da20acd1d8cf60863 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 8 Dec 2023 09:27:09 +0100 Subject: [PATCH 5/6] OpenAPI --- docs/_static/discovery/v1.yaml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/_static/discovery/v1.yaml b/docs/_static/discovery/v1.yaml index a79302af01..84be706c9e 100644 --- a/docs/_static/discovery/v1.yaml +++ b/docs/_static/discovery/v1.yaml @@ -18,9 +18,9 @@ paths: get: summary: Retrieves the presentations of a discovery service. description: | - An API provided by the discovery server to retrieve the presentations of a discovery service, starting at the given timestamp. - The client should provide the timestamp it was returned in the last response. - If no timestamp is given, it will return all presentations. + An API provided by the discovery server to retrieve the presentations of a discovery service, starting at the given tag. + The client should provide the tag it was returned in the last response. + If no tag is given, it will return all presentations. error returns: * 404 - unknown service ID @@ -28,22 +28,22 @@ paths: tags: - discovery parameters: - - name: timestamp + - name: tag in: query schema: type: string responses: "200": - description: Presentations are returned, alongside the timestamp which should be provided at the next query. + description: Presentations are returned, alongside the tag which should be provided at the next query. content: application/json: schema: type: object required: - - timestamp + - tag - entries properties: - timestamp: + tag: type: string entries: type: array From 1fe7014cb03d663bbf75bfdeced84e008ab9fe71 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Sat, 9 Dec 2023 06:40:45 +0100 Subject: [PATCH 6/6] more descriptive error message --- discovery/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discovery/store.go b/discovery/store.go index dcca397c4c..fc2dcb779b 100644 --- a/discovery/store.go +++ b/discovery/store.go @@ -352,7 +352,7 @@ func (s *sqlStore) updateTag(tx *gorm.DB, serviceID string, newTimestamp *Tag) ( ts := service.LastTag.Timestamp(service.TagPrefix) if ts == nil { // would be very weird: can't decode it, although it's our own tag - return nil, fmt.Errorf("invalid tag '%s'", service.LastTag) + return nil, fmt.Errorf("can't decode tag '%s', did someone alter 'service.tag_prefix' or 'service.last_tag' in the database?", service.LastTag) } currTimestamp = *ts }