diff --git a/discovery/api/v1/api.go b/discovery/api/v1/api.go index 7377f3e58..6632c4ca5 100644 --- a/discovery/api/v1/api.go +++ b/discovery/api/v1/api.go @@ -24,6 +24,7 @@ import ( "github.com/labstack/echo/v4" "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/core" + "github.com/nuts-foundation/nuts-node/core/to" "github.com/nuts-foundation/nuts-node/discovery" "github.com/nuts-foundation/nuts-node/vcr/credential" "github.com/nuts-foundation/nuts-node/vdr/didsubject" @@ -46,6 +47,8 @@ func (w *Wrapper) ResolveStatusCode(err error) int { return http.StatusNotFound case errors.Is(err, didsubject.ErrSubjectNotFound): return http.StatusNotFound + case errors.Is(err, discovery.ErrPresentationRegistrationFailed): + return http.StatusPreconditionFailed default: return http.StatusInternalServerError } @@ -108,12 +111,6 @@ func (w *Wrapper) ActivateServiceForSubject(ctx context.Context, request Activat } err := w.Client.ActivateServiceForSubject(ctx, request.ServiceID, request.SubjectID, parameters) - if errors.Is(err, discovery.ErrPresentationRegistrationFailed) { - // registration failed, but will be retried - return ActivateServiceForSubject202JSONResponse{ - Reason: err.Error(), - }, nil - } if err != nil { // other error return nil, err @@ -123,12 +120,6 @@ func (w *Wrapper) ActivateServiceForSubject(ctx context.Context, request Activat func (w *Wrapper) DeactivateServiceForSubject(ctx context.Context, request DeactivateServiceForSubjectRequestObject) (DeactivateServiceForSubjectResponseObject, error) { err := w.Client.DeactivateServiceForSubject(ctx, request.ServiceID, request.SubjectID) - if errors.Is(err, discovery.ErrPresentationRegistrationFailed) { - // deactivation succeeded, but not all Verifiable Presentation couldn't be removed from remote Discovery Server. - return DeactivateServiceForSubject202JSONResponse{ - Reason: err.Error(), - }, nil - } if err != nil { return nil, err } @@ -141,12 +132,19 @@ func (w *Wrapper) GetServices(_ context.Context, _ GetServicesRequestObject) (Ge } func (w *Wrapper) GetServiceActivation(ctx context.Context, request GetServiceActivationRequestObject) (GetServiceActivationResponseObject, error) { + response := GetServiceActivation200JSONResponse{ + Status: ServiceStatusActive, + } activated, presentations, err := w.Client.GetServiceActivation(ctx, request.ServiceID, request.SubjectID) if err != nil { - return nil, err + if !errors.As(err, &discovery.RegistrationRefreshError{}) { + return nil, err + } + response.Status = ServiceStatusError + response.Error = to.Ptr(err.Error()) } - return GetServiceActivation200JSONResponse{ - Activated: activated, - Vp: &presentations, - }, nil + response.Activated = activated + response.Vp = &presentations + + return response, nil } diff --git a/discovery/api/v1/api_test.go b/discovery/api/v1/api_test.go index 0767da1d3..578a47ddf 100644 --- a/discovery/api/v1/api_test.go +++ b/discovery/api/v1/api_test.go @@ -71,17 +71,16 @@ func TestWrapper_ActivateServiceForSubject(t *testing.T) { assert.NoError(t, err) assert.IsType(t, ActivateServiceForSubject200Response{}, response) }) - t.Run("ok, but registration failed", func(t *testing.T) { + t.Run("but registration failed", func(t *testing.T) { test := newMockContext(t) test.client.EXPECT().ActivateServiceForSubject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(discovery.ErrPresentationRegistrationFailed) - response, err := test.wrapper.ActivateServiceForSubject(nil, ActivateServiceForSubjectRequestObject{ + _, err := test.wrapper.ActivateServiceForSubject(nil, ActivateServiceForSubjectRequestObject{ ServiceID: serviceID, SubjectID: subjectID, }) - assert.NoError(t, err) - assert.IsType(t, ActivateServiceForSubject202JSONResponse{}, response) + assert.ErrorIs(t, err, discovery.ErrPresentationRegistrationFailed) }) t.Run("other error", func(t *testing.T) { test := newMockContext(t) @@ -200,6 +199,24 @@ func TestWrapper_GetServiceActivation(t *testing.T) { assert.NoError(t, err) require.IsType(t, GetServiceActivation200JSONResponse{}, response) assert.True(t, response.(GetServiceActivation200JSONResponse).Activated) + assert.Equal(t, ServiceStatusActive, string(response.(GetServiceActivation200JSONResponse).Status)) + assert.Nil(t, response.(GetServiceActivation200JSONResponse).Error) + assert.Empty(t, response.(GetServiceActivation200JSONResponse).Vp) + }) + t.Run("refresh failed", func(t *testing.T) { + test := newMockContext(t) + test.client.EXPECT().GetServiceActivation(gomock.Any(), serviceID, subjectID).Return(true, nil, discovery.RegistrationRefreshError{Underlying: assert.AnError}) + + response, err := test.wrapper.GetServiceActivation(nil, GetServiceActivationRequestObject{ + SubjectID: subjectID, + ServiceID: serviceID, + }) + + assert.NoError(t, err) + require.IsType(t, GetServiceActivation200JSONResponse{}, response) + assert.True(t, response.(GetServiceActivation200JSONResponse).Activated) + assert.Equal(t, ServiceStatusError, string(response.(GetServiceActivation200JSONResponse).Status)) + assert.NotNil(t, response.(GetServiceActivation200JSONResponse).Error) assert.Empty(t, response.(GetServiceActivation200JSONResponse).Vp) }) t.Run("error", func(t *testing.T) { diff --git a/discovery/api/v1/generated.go b/discovery/api/v1/generated.go index b733059ce..5beca050f 100644 --- a/discovery/api/v1/generated.go +++ b/discovery/api/v1/generated.go @@ -377,6 +377,12 @@ type GetServiceActivation200JSONResponse struct { // Activated Whether the Discovery Service is activated for the given subject Activated bool `json:"activated"` + // Error Error message if status is "error". + Error *string `json:"error,omitempty"` + + // Status Status of the activation. "active" or "error". + Status GetServiceActivation200JSONResponseStatus `json:"status"` + // Vp List of VPs on the Discovery Service for the subject. One per DID method registered on the Service. // The list can be empty even if activated==true if none of the DIDs of a subject is actually registered on the Discovery Service. Vp *[]VerifiablePresentation `json:"vp,omitempty"` @@ -428,19 +434,25 @@ func (response ActivateServiceForSubject200Response) VisitActivateServiceForSubj return nil } -type ActivateServiceForSubject202JSONResponse struct { - // Reason Description of why registration failed. - Reason string `json:"reason"` +type ActivateServiceForSubject400ApplicationProblemPlusJSONResponse struct { + // Detail A human-readable explanation specific to this occurrence of the problem. + Detail string `json:"detail"` + + // Status HTTP statuscode + Status float32 `json:"status"` + + // Title A short, human-readable summary of the problem type. + Title string `json:"title"` } -func (response ActivateServiceForSubject202JSONResponse) VisitActivateServiceForSubjectResponse(w http.ResponseWriter) error { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(202) +func (response ActivateServiceForSubject400ApplicationProblemPlusJSONResponse) VisitActivateServiceForSubjectResponse(w http.ResponseWriter) error { + w.Header().Set("Content-Type", "application/problem+json") + w.WriteHeader(400) return json.NewEncoder(w).Encode(response) } -type ActivateServiceForSubject400ApplicationProblemPlusJSONResponse struct { +type ActivateServiceForSubject412ApplicationProblemPlusJSONResponse struct { // Detail A human-readable explanation specific to this occurrence of the problem. Detail string `json:"detail"` @@ -451,9 +463,9 @@ type ActivateServiceForSubject400ApplicationProblemPlusJSONResponse struct { Title string `json:"title"` } -func (response ActivateServiceForSubject400ApplicationProblemPlusJSONResponse) VisitActivateServiceForSubjectResponse(w http.ResponseWriter) error { +func (response ActivateServiceForSubject412ApplicationProblemPlusJSONResponse) VisitActivateServiceForSubjectResponse(w http.ResponseWriter) error { w.Header().Set("Content-Type", "application/problem+json") - w.WriteHeader(400) + w.WriteHeader(412) return json.NewEncoder(w).Encode(response) } diff --git a/discovery/api/v1/types.go b/discovery/api/v1/types.go index ef2a6e9ab..e5db1857e 100644 --- a/discovery/api/v1/types.go +++ b/discovery/api/v1/types.go @@ -31,3 +31,13 @@ type ServiceDefinition = discovery.ServiceDefinition // VerifiableCredential is a type alias for the VerifiableCredential from the go-did library. type VerifiableCredential = vc.VerifiableCredential + +// GetServiceActivation200JSONResponseStatus is a type alias for string, generated from an enum. +type GetServiceActivation200JSONResponseStatus string + +const ( + // ServiceStatusActive is the status for an active service. + ServiceStatusActive = "active" + // ServiceStatusError is the status for an inactive service. + ServiceStatusError = "error" +) diff --git a/discovery/client.go b/discovery/client.go index 6e95c7ecf..cfefdf88d 100644 --- a/discovery/client.go +++ b/discovery/client.go @@ -92,10 +92,6 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service return fmt.Errorf("%w: %w for %s", ErrPresentationRegistrationFailed, ErrDIDMethodsNotSupported, subjectID) } - asSoonAsPossible := time.Now().Add(-10 * time.Second) // could be whatever, as long as it's a bit in the past to avoid issues with some weird clock skew scenarios when running a cluster - if err := r.store.updatePresentationRefreshTime(serviceID, subjectID, parameters, &asSoonAsPossible); err != nil { - return err - } log.Logger().Debugf("Registering Verifiable Presentation on Discovery Service (service=%s, subject=%s)", service.ID, subjectID) var registeredDIDs []string @@ -103,12 +99,7 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service for _, subjectDID := range subjectDIDs { err := r.registerPresentation(ctx, subjectDID, service, parameters) if err != nil { - if !errors.Is(err, pe.ErrNoCredentials) { // ignore missing credentials - loopErrs = append(loopErrs, fmt.Errorf("%s: %w", subjectDID.String(), err)) - } else { - // trace logging for missing credentials - log.Logger().Tracef("Missing credentials for Discovery Service (service=%s, subject=%s, did=%s): %s", service.ID, subjectID, subjectDID, err.Error()) - } + loopErrs = append(loopErrs, fmt.Errorf("%s: %w", subjectDID.String(), err)) } else { registeredDIDs = append(registeredDIDs, subjectDID.String()) } @@ -132,6 +123,10 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service if err := r.store.updatePresentationRefreshTime(serviceID, subjectID, parameters, &refreshVPAfter); err != nil { return fmt.Errorf("unable to update Verifiable Presentation refresh time: %w", err) } + // clear any previous presentationRefreshErrors here so it's triggered by both the refresh and API call + if err := r.store.setPresentationRefreshError(serviceID, subjectID, nil); err != nil { + return fmt.Errorf("unable to clear previous presentationRefreshError: %w", err) + } return nil } @@ -278,8 +273,8 @@ func (r *defaultClientRegistrationManager) refresh(ctx context.Context, now time } var loopErrs []error for _, candidate := range refreshCandidates { + var loopErr error if err = r.activate(ctx, candidate.ServiceID, candidate.SubjectID, candidate.Parameters); err != nil { - var loopErr error if errors.Is(err, ErrDIDMethodsNotSupported) { // DID method no longer supported, remove err = r.store.updatePresentationRefreshTime(candidate.ServiceID, candidate.SubjectID, nil, nil) @@ -296,9 +291,13 @@ func (r *defaultClientRegistrationManager) refresh(ctx context.Context, now time } } else { loopErr = fmt.Errorf("failed to refresh Verifiable Presentation (service=%s, subject=%s): %w", candidate.ServiceID, candidate.SubjectID, err) + if err := r.store.setPresentationRefreshError(candidate.ServiceID, candidate.SubjectID, loopErr); err != nil { + loopErr = fmt.Errorf("failed to set refresh error for Verifiable Presentation (service=%s, subject=%s): %w. Original error: %w", candidate.ServiceID, candidate.SubjectID, err, loopErr) + } } loopErrs = append(loopErrs, loopErr) } + // activate clears any presentationRefreshErrors } if len(loopErrs) > 0 { return errors.Join(loopErrs...) diff --git a/discovery/client_test.go b/discovery/client_test.go index 5fe11d969..511bf2096 100644 --- a/discovery/client_test.go +++ b/discovery/client_test.go @@ -90,6 +90,12 @@ func Test_defaultClientRegistrationManager_activate(t *testing.T) { require.ErrorIs(t, err, ErrPresentationRegistrationFailed) assert.ErrorContains(t, err, "invoker error") + + // check no refresh records are added + record, err := store.getPresentationRefreshRecord(testServiceID, aliceSubject) + + require.NoError(t, err) + assert.Nil(t, record) }) t.Run("DID method not supported", func(t *testing.T) { ctrl := gomock.NewController(t) @@ -360,7 +366,13 @@ func Test_defaultClientRegistrationManager_refresh(t *testing.T) { err := manager.refresh(audit.TestContext(), time.Now()) - assert.EqualError(t, err, "failed to refresh Verifiable Presentation (service=usecase_v1, subject=bob): registration of Verifiable Presentation on remote Discovery Service failed: did:example:bob: remote error") + errStr := "failed to refresh Verifiable Presentation (service=usecase_v1, subject=bob): registration of Verifiable Presentation on remote Discovery Service failed: did:example:bob: remote error" + assert.EqualError(t, err, errStr) + + // check for presentationRefreshError + refreshError, err := store.getPresentationRefreshError(testServiceID, bobSubject) + require.NoError(t, err) + assert.Contains(t, refreshError.Error, errStr) }) t.Run("deactivate unknown subject", func(t *testing.T) { store := setupStore(t, storageEngine.GetSQLDatabase()) @@ -376,7 +388,6 @@ func Test_defaultClientRegistrationManager_refresh(t *testing.T) { assert.EqualError(t, err, "removed unknown subject (service=usecase_v1, subject=alice)") }) - t.Run("deactivate unsupported DID method", func(t *testing.T) { store := setupStore(t, storageEngine.GetSQLDatabase()) ctrl := gomock.NewController(t) @@ -391,9 +402,38 @@ func Test_defaultClientRegistrationManager_refresh(t *testing.T) { // refresh clears the registration require.NoError(t, err) - refreshTime, err := store.getPresentationRefreshTime(unsupportedServiceID, aliceSubject) + record, err := store.getPresentationRefreshRecord(unsupportedServiceID, aliceSubject) assert.NoError(t, err) - assert.Nil(t, refreshTime) + assert.Nil(t, record) + }) + t.Run("remove presentationRefreshError on success", func(t *testing.T) { + store := setupStore(t, storageEngine.GetSQLDatabase()) + ctrl := gomock.NewController(t) + invoker := client.NewMockHTTPClient(ctrl) + gomock.InOrder( + invoker.EXPECT().Register(gomock.Any(), gomock.Any(), gomock.Any()), + ) + wallet := holder.NewMockWallet(ctrl) + mockVCR := vcr.NewMockVCR(ctrl) + mockVCR.EXPECT().Wallet().Return(wallet).AnyTimes() + mockSubjectManager := didsubject.NewMockManager(ctrl) + manager := newRegistrationManager(testDefinitions(), store, invoker, mockVCR, mockSubjectManager) + + // Alice + _ = store.setPresentationRefreshError(testServiceID, aliceSubject, assert.AnError) + _ = store.updatePresentationRefreshTime(testServiceID, aliceSubject, defaultRegistrationParams(aliceSubject), &time.Time{}) + mockSubjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil) + wallet.EXPECT().BuildPresentation(gomock.Any(), gomock.Any(), gomock.Any(), &aliceDID, false).Return(&vpAlice, nil) + wallet.EXPECT().List(gomock.Any(), aliceDID).Return([]vc.VerifiableCredential{vcAlice}, nil) + + err := manager.refresh(audit.TestContext(), time.Now()) + + require.NoError(t, err) + + // check for presentationRefreshError + refreshError, err := store.getPresentationRefreshError(testServiceID, aliceSubject) + require.NoError(t, err) + assert.Nil(t, refreshError) }) } diff --git a/discovery/interface.go b/discovery/interface.go index 3435340af..f2522d261 100644 --- a/discovery/interface.go +++ b/discovery/interface.go @@ -59,7 +59,6 @@ type Client interface { // ActivateServiceForSubject causes a subject to be registered for a Discovery Service. // Registration of all DIDs of the subject will be attempted immediately, and automatically refreshed. - // If the initial registration for all DIDs fails with ErrPresentationRegistrationFailed, registration will be retried. // If the function is called again for the same service/DID combination, it will try to refresh the registration. // parameters are added as credentialSubject to a DiscoveryRegistrationCredential holder credential. // It returns an error if the service or subject is invalid/unknown. @@ -77,6 +76,8 @@ type Client interface { // GetServiceActivation returns the activation status of a subject on a Discovery Service. // The boolean indicates whether the subject is activated on the Discovery Service (ActivateServiceForSubject() has been called). // It also returns the Verifiable Presentations for all DIDs of the subject that are registered on the Discovery Service, if any. + // It returns a refreshRecordError if the last refresh of the service failed (activation status and VPs are still returned). + // The time of the last error is added in the error message. GetServiceActivation(ctx context.Context, serviceID, subjectID string) (bool, []vc.VerifiablePresentation, error) } @@ -96,3 +97,12 @@ type presentationVerifier func(definition ServiceDefinition, presentation vc.Ver // XForwardedHostContextKey is the context key for the X-Forwarded-Host header. type XForwardedHostContextKey struct{} + +// RegistrationRefreshError is returned from GetServiceRefreshError. +type RegistrationRefreshError struct { + Underlying error +} + +func (r RegistrationRefreshError) Error() string { + return r.Underlying.Error() +} diff --git a/discovery/module.go b/discovery/module.go index cca0e842d..aec286b93 100644 --- a/discovery/module.go +++ b/discovery/module.go @@ -379,9 +379,6 @@ func (m *Module) ActivateServiceForSubject(ctx context.Context, serviceID, subje err := m.registrationManager.activate(ctx, serviceID, subjectID, parameters) if err != nil { - if errors.Is(err, ErrPresentationRegistrationFailed) { - log.Logger().WithError(err).Warnf("Presentation registration failed, will be retried later (subject=%s,service=%s)", subjectID, serviceID) - } return err } @@ -408,11 +405,11 @@ func (m *Module) Services() []ServiceDefinition { // GetServiceActivation is a Discovery Client function that retrieves the activation status of a service for a subject. // See interface.go for more information. func (m *Module) GetServiceActivation(ctx context.Context, serviceID, subjectID string) (bool, []vc.VerifiablePresentation, error) { - refreshTime, err := m.store.getPresentationRefreshTime(serviceID, subjectID) + refreshRecord, err := m.store.getPresentationRefreshRecord(serviceID, subjectID) if err != nil { return false, nil, err } - if refreshTime == nil { + if refreshRecord == nil { return false, nil, nil } // subject is activated for service @@ -431,7 +428,16 @@ func (m *Module) GetServiceActivation(ctx context.Context, serviceID, subjectID for _, vps := range vps2D { results = append(results, vps...) } - return true, results, nil + + // check if the last refresh didn't result in an error + if refreshRecord.PresentationRefreshError.Error != "" { + // format a readable time using RFC3339 + lastOccurrence := time.Unix(int64(refreshRecord.PresentationRefreshError.LastOccurrence), 0) + readableTime := lastOccurrence.Format(time.RFC3339) + err = RegistrationRefreshError{fmt.Errorf("[%s] %s", readableTime, refreshRecord.PresentationRefreshError.Error)} + } + + return true, results, err } func loadDefinitions(directory string) (map[string]ServiceDefinition, error) { diff --git a/discovery/module_test.go b/discovery/module_test.go index aa3f8f18b..1005bbb8e 100644 --- a/discovery/module_test.go +++ b/discovery/module_test.go @@ -539,7 +539,7 @@ func TestModule_ActivateServiceForSubject(t *testing.T) { wallet := holder.NewMockWallet(gomock.NewController(t)) m.vcrInstance.(*vcr.MockVCR).EXPECT().Wallet().Return(wallet).MinTimes(1) wallet.EXPECT().List(gomock.Any(), gomock.Any()).Return(nil, errors.New("failed")).MinTimes(1) - testContext.subjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil).Times(2) + testContext.subjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil) err := m.ActivateServiceForSubject(context.Background(), testServiceID, aliceSubject, nil) @@ -584,7 +584,7 @@ func TestModule_GetServiceActivation(t *testing.T) { }) t.Run("activated, with VP", func(t *testing.T) { m, testContext := setupModule(t, storageEngine) - testContext.subjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil) + testContext.subjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil).AnyTimes() next := time.Now() _ = m.store.updatePresentationRefreshTime(testServiceID, aliceSubject, nil, &next) _ = m.store.add(testServiceID, vpAlice, 0) @@ -594,5 +594,15 @@ func TestModule_GetServiceActivation(t *testing.T) { require.NoError(t, err) assert.True(t, activated) assert.NotNil(t, presentation) + + t.Run("with refresh error", func(t *testing.T) { + _ = m.store.setPresentationRefreshError(testServiceID, aliceSubject, assert.AnError) + + activated, _, err = m.GetServiceActivation(context.Background(), testServiceID, aliceSubject) + + require.Error(t, err) + assert.True(t, activated) + assert.ErrorAs(t, err, &RegistrationRefreshError{}) + }) }) } diff --git a/discovery/store.go b/discovery/store.go index 41fa07b61..dc1b4a1c3 100644 --- a/discovery/store.go +++ b/discovery/store.go @@ -88,6 +88,8 @@ type presentationRefreshRecord struct { NextRefresh int // Parameters is a serialized JSON object containing parameters that should be used when registering the subject on the service. Parameters []byte + // PresentationRefreshError is the error message that occurred during the refresh attempt. + PresentationRefreshError presentationRefreshError `gorm:"foreignKey:ServiceID,SubjectID"` } // TableName returns the table name for this DTO. @@ -95,6 +97,23 @@ func (l presentationRefreshRecord) TableName() string { return "discovery_presentation_refresh" } +// presentationRefreshError is a record of a failed refresh attempt. +type presentationRefreshError struct { + // ServiceID refers to the entry record in discovery_service + ServiceID string `gorm:"primaryKey"` + // SubjectID is the ID of the subject that should be registered on the service. + SubjectID string `gorm:"primaryKey"` + // Error is the error message that occurred during the refresh attempt. + Error string + // LastOccurrence is the timestamp of the last occurrence of this error. + LastOccurrence int +} + +// TableName returns the table name for this DTO. +func (l presentationRefreshError) TableName() string { + return "discovery_presentation_error" +} + type sqlStore struct { db *gorm.DB } @@ -332,16 +351,15 @@ func (s *sqlStore) updatePresentationRefreshTime(serviceID string, subjectID str }) } -func (s *sqlStore) getPresentationRefreshTime(serviceID string, subjectID string) (*time.Time, error) { +func (s *sqlStore) getPresentationRefreshRecord(serviceID string, subjectID string) (*presentationRefreshRecord, error) { var row presentationRefreshRecord - if err := s.db.Find(&row, "service_id = ? AND subject_id = ?", serviceID, subjectID).Error; err != nil { + if err := s.db.Preload("PresentationRefreshError").Find(&row, "service_id = ? AND subject_id = ?", serviceID, subjectID).Error; err != nil { return nil, err } if row.NextRefresh == 0 { return nil, nil } - result := time.Unix(int64(row.NextRefresh), 0) - return &result, nil + return &row, nil } // getSubjectsToBeRefreshed returns all registered subject-service combinations that are due for refreshing. @@ -366,6 +384,39 @@ func (s *sqlStore) getSubjectsToBeRefreshed(now time.Time) ([]refreshCandidate, return result, nil } +func (s *sqlStore) getPresentationRefreshError(serviceID string, subjectID string) (*presentationRefreshError, error) { + var row presentationRefreshError + if err := s.db.Find(&row, "service_id = ? AND subject_id = ?", serviceID, subjectID).Error; err != nil { + return nil, err + } + if row.LastOccurrence == 0 { + return nil, nil + } + return &row, nil +} + +func (s *sqlStore) setPresentationRefreshError(serviceID string, subjectID string, refreshErr error) error { + return s.db.Transaction(func(tx *gorm.DB) error { + if err := tx.Delete(&presentationRefreshError{}, "service_id = ? AND subject_id = ?", serviceID, subjectID).Error; err != nil { + return err + } + + if refreshErr == nil { + // a delete + return nil + } + + row := presentationRefreshError{ + ServiceID: serviceID, + SubjectID: subjectID, + Error: refreshErr.Error(), + LastOccurrence: int(time.Now().Unix()), //32bit supports stops at 03:14:07 on Tuesday, 19 January 2038 + } + + return tx.Save(&row).Error + }) +} + // refreshCandidate is a subset of presentationRefreshRecord type refreshCandidate struct { // ServiceID is the presentationRefreshRecord.ServiceID diff --git a/discovery/store_test.go b/discovery/store_test.go index 75910a431..0e6dae97c 100644 --- a/discovery/store_test.go +++ b/discovery/store_test.go @@ -22,6 +22,7 @@ import ( "github.com/lestrrat-go/jwx/v2/jwt" "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/go-did/vc" + "github.com/nuts-foundation/nuts-node/core/to" "github.com/nuts-foundation/nuts-node/storage" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -268,7 +269,7 @@ func Test_sqlStore_getPresentationRefreshTime(t *testing.T) { t.Run("no entry", func(t *testing.T) { c := setupStore(t, storageEngine.GetSQLDatabase()) - ts, err := c.getPresentationRefreshTime(testServiceID, aliceSubject) + ts, err := c.getPresentationRefreshRecord(testServiceID, aliceSubject) require.NoError(t, err) assert.Nil(t, ts) }) @@ -276,9 +277,50 @@ func Test_sqlStore_getPresentationRefreshTime(t *testing.T) { c := setupStore(t, storageEngine.GetSQLDatabase()) now := time.Now().Truncate(time.Second) require.NoError(t, c.updatePresentationRefreshTime(testServiceID, aliceSubject, nil, &now)) - ts, err := c.getPresentationRefreshTime(testServiceID, aliceSubject) + ts, err := c.getPresentationRefreshRecord(testServiceID, aliceSubject) require.NoError(t, err) - assert.Equal(t, now, *ts) + require.NotNil(t, ts) + assert.Equal(t, int(now.Unix()), ts.NextRefresh) + + t.Run("error is preloaded", func(t *testing.T) { + require.NoError(t, c.setPresentationRefreshError(testServiceID, aliceSubject, assert.AnError)) + + ts, err := c.getPresentationRefreshRecord(testServiceID, aliceSubject) + + require.NoError(t, err) + require.NotNil(t, ts) + assert.Equal(t, assert.AnError.Error(), ts.PresentationRefreshError.Error) + }) + }) +} + +func Test_sqlStore_setPresentationRefreshError(t *testing.T) { + storageEngine := storage.NewTestStorageEngine(t) + require.NoError(t, storageEngine.Start()) + + t.Run("store", func(t *testing.T) { + c := setupStore(t, storageEngine.GetSQLDatabase()) + + require.NoError(t, c.updatePresentationRefreshTime(testServiceID, aliceSubject, nil, to.Ptr(time.Now().Add(time.Second)))) + require.NoError(t, c.setPresentationRefreshError(testServiceID, aliceSubject, assert.AnError)) + + // Check if the error is stored + refreshError, err := c.getPresentationRefreshError(testServiceID, aliceSubject) + + require.NoError(t, err) + assert.Equal(t, refreshError.Error, assert.AnError.Error()) + assert.True(t, refreshError.LastOccurrence > int(time.Now().Add(-1*time.Second).Unix())) + }) + t.Run("delete", func(t *testing.T) { + c := setupStore(t, storageEngine.GetSQLDatabase()) + require.NoError(t, c.updatePresentationRefreshTime(testServiceID, aliceSubject, nil, to.Ptr(time.Now().Add(time.Second)))) + require.NoError(t, c.setPresentationRefreshError(testServiceID, aliceSubject, assert.AnError)) + require.NoError(t, c.setPresentationRefreshError(testServiceID, aliceSubject, nil)) + + refreshError, err := c.getPresentationRefreshError(testServiceID, aliceSubject) + + require.NoError(t, err) + assert.Nil(t, refreshError) }) } diff --git a/docs/_static/discovery/v1.yaml b/docs/_static/discovery/v1.yaml index 01df7884f..d56076d6f 100644 --- a/docs/_static/discovery/v1.yaml +++ b/docs/_static/discovery/v1.yaml @@ -107,7 +107,8 @@ paths: summary: Retrieves the activation status of a subject on a Discovery Service. description: | An API provided by the Discovery Client, - used to check whether the client is managing the given subject on the specified Discovery Service (service has been activated for the subject). + used to check whether the client is managing the given subject on the specified Discovery Service (service has been activated for the subject) + and the status of the activation. A refresh could have failed. It will return true after successfully calling the activateServiceForSubject API, and false after calling the deactivateServiceForSubject API. It also returns the active Verifiable Presentations, if any. operationId: getServiceActivation @@ -122,10 +123,20 @@ paths: type: object required: - activated + - status properties: activated: type: boolean description: Whether the Discovery Service is activated for the given subject + status: + type: string + description: Status of the activation. "active" or "error". + enum: + - active + - error + error: + type: string + description: Error message if status is "error". vp: description: | List of VPs on the Discovery Service for the subject. One per DID method registered on the Service. @@ -148,6 +159,7 @@ paths: error returns: * 400 - incorrect input: invalid/unknown service or subject. + * 412 - precondition failed: subject doesn't have the required credentials. operationId: activateServiceForSubject tags: - discovery @@ -160,20 +172,11 @@ paths: responses: "200": description: Activation was successful. - "202": - description: Activation was successful, but registration of the Verifiable Presentation failed (but will be automatically re-attempted later). - content: - application/json: - schema: - type: object - required: - - reason - properties: - reason: - type: string - description: Description of why registration failed. + "400": $ref: "../common/error_response.yaml" + "412": + $ref: "../common/error_response.yaml" default: $ref: "../common/error_response.yaml" delete: diff --git a/docs/pages/deployment/discovery.rst b/docs/pages/deployment/discovery.rst index 7aac70595..d9365eddd 100644 --- a/docs/pages/deployment/discovery.rst +++ b/docs/pages/deployment/discovery.rst @@ -44,9 +44,8 @@ E.g., for service ``coffeecorner`` and subject ``example``: POST /internal/discovery/v1/coffeecorner/example -The DID's wallet must contain the Verifiable Credential(s) that are required by the service definition, -otherwise registration will fail. If the wallet does not contain the credentials, -the Nuts node will retry registration periodically for all DIDs of a subject. +The wallet must contain the Verifiable Credential(s) that are required by the service definition, +otherwise registration will fail immediately. Optionally, a POST body can be provided with registration parameters, e.g.: @@ -62,6 +61,24 @@ Optionally, a POST body can be provided with registration parameters, e.g.: This can be used to provide additional information. All registration parameters are returned by the search API. The ``authServerURL`` is added automatically by the Nuts node. It's constructed as ``https:///oauth2/``. +Once registered, future refreshes will be done automatically by the Nuts node. These refreshes could fail because of various reasons. +You can check the status of the refreshes by querying the service, e.g.: + +.. code-block:: text + + GET /internal/discovery/v1/coffeecorner/example + +The result contains a ``status`` field that indicates the status of the registration (``active`` or ``error``) . If the refresh fails, the ``error`` field contains the error message. + +.. code-block:: json + + { + "activated": true, + "status": "error", + "error": "error message", + "vp": [...] + } + Servers ******* To act as server for a specific discovery service, its service ID needs to be specified in ``discovery.server.ids``, e.g.: diff --git a/e2e-tests/discovery/run-test.sh b/e2e-tests/discovery/run-test.sh index 3f721a9a4..dd88659cf 100755 --- a/e2e-tests/discovery/run-test.sh +++ b/e2e-tests/discovery/run-test.sh @@ -15,7 +15,7 @@ docker compose down echo "------------------------------------" echo "Starting Docker containers..." echo "------------------------------------" -docker compose up --wait nodeB-backend nodeB +docker compose up --wait nodeA-backend nodeA nodeB-backend nodeB echo "------------------------------------" echo "Registering care organization..." @@ -49,8 +49,7 @@ echo "---------------------------------------" echo "Registering care organization on Discovery Service..." echo "---------------------------------------" curl --insecure -s -X POST http://localhost:28081/internal/discovery/v1/dev:eOverdracht2023/${SUBJECT} -# Start Discovery Server -docker compose up --wait nodeA-backend nodeA + # Registration refresh interval is 500ms, wait some to make sure the registration is refreshed sleep 2 diff --git a/storage/sql_migrations/008_discoveryservice_client_error.sql b/storage/sql_migrations/008_discoveryservice_client_error.sql new file mode 100644 index 000000000..63e8c0e73 --- /dev/null +++ b/storage/sql_migrations/008_discoveryservice_client_error.sql @@ -0,0 +1,20 @@ +-- +goose ENVSUB ON +-- +goose Up +-- discovery_presentation_error contains errors for activated registrations that once succeeded but fail to refresh. +create table discovery_presentation_error +( + -- service_id is the ID of the Discover Service that the DID should be registered on. + -- It comes from the service definition. + service_id varchar(200) not null, + -- subject_id is the subject that should be registered on the Discovery Service. + subject_id varchar(370) not null, + -- reason contains the error message that caused the registration to fail. + error $TEXT_TYPE, + -- last_occurrence is the timestamp (seconds since Unix epoch) when the registration last failed. + last_occurrence integer not null, + primary key (service_id, subject_id), + constraint fk_discovery_presentation_error_service foreign key (service_id, subject_id) references discovery_presentation_refresh (service_id, subject_id) on delete cascade +); + +-- +goose Down +drop table discovery_presentation_error;