status codes for discovery client

discovery/api/server/client/http.go

@@ -21,7 +21,6 @@ package client
 import (
 	"bytes"
 	"context"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"github.com/nuts-foundation/go-did/vc"
@@ -35,9 +34,9 @@ import (
 )
 
 // New creates a new DefaultHTTPClient.
-func New(strictMode bool, timeout time.Duration, tlsConfig *tls.Config) *DefaultHTTPClient {
+func New(timeout time.Duration) *DefaultHTTPClient {
 	return &DefaultHTTPClient{
-		client: client.NewWithTLSConfig(timeout, tlsConfig),
+		client: client.New(timeout),
 	}
 }
 

discovery/api/server/client/http_test.go

@@ -40,7 +40,7 @@ func TestHTTPInvoker_Register(t *testing.T) {
 	t.Run("ok", func(t *testing.T) {
 		handler := &testHTTP.Handler{StatusCode: http.StatusCreated}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		err := client.Register(context.Background(), server.URL, vp)
 
@@ -51,7 +51,7 @@ func TestHTTPInvoker_Register(t *testing.T) {
 	})
 	t.Run("non-ok with problem details", func(t *testing.T) {
 		server := httptest.NewServer(&testHTTP.Handler{StatusCode: http.StatusBadRequest, ResponseData: `{"title":"missing credentials", "status":400, "detail":"could not resolve DID"}`})
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		err := client.Register(context.Background(), server.URL, vp)
 
@@ -61,7 +61,7 @@ func TestHTTPInvoker_Register(t *testing.T) {
 	})
 	t.Run("non-ok other", func(t *testing.T) {
 		server := httptest.NewServer(&testHTTP.Handler{StatusCode: http.StatusNotFound, ResponseData: `not found`})
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		err := client.Register(context.Background(), server.URL, vp)
 
@@ -84,7 +84,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 			"timestamp": 1,
 		}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		presentations, seed, timestamp, err := client.Get(context.Background(), server.URL, 0)
 
@@ -102,7 +102,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 			"timestamp": 1,
 		}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		presentations, _, timestamp, err := client.Get(context.Background(), server.URL, 1)
 
@@ -120,7 +120,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 			writer.Write([]byte("{}"))
 		}
 		server := httptest.NewServer(http.HandlerFunc(handler))
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		_, _, _, err := client.Get(context.Background(), server.URL, 0)
 
@@ -130,7 +130,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 	t.Run("server returns invalid status code", func(t *testing.T) {
 		handler := &testHTTP.Handler{StatusCode: http.StatusInternalServerError, ResponseData: `{"title":"internal server error", "status":500, "detail":"db not found"}`}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		_, _, _, err := client.Get(context.Background(), server.URL, 0)
 
@@ -142,7 +142,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 		handler := &testHTTP.Handler{StatusCode: http.StatusOK}
 		handler.ResponseData = "not json"
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		_, _, _, err := client.Get(context.Background(), server.URL, 0)
 

discovery/api/v1/api.go

@@ -121,6 +121,9 @@ 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 err != nil {
+		if errors.Is(err, discovery.ErrPresentationRegistrationFailed) {
+			return DeactivateServiceForSubject202JSONResponse{Reason: err.Error()}, nil
+		}
 		return nil, err
 	}
 	return DeactivateServiceForSubject200Response{}, nil

discovery/api/v1/api_test.go

@@ -108,6 +108,19 @@ func TestWrapper_DeactivateServiceForSubject(t *testing.T) {
 		assert.NoError(t, err)
 		assert.IsType(t, DeactivateServiceForSubject200Response{}, response)
 	})
+	t.Run("server error", func(t *testing.T) {
+		test := newMockContext(t)
+		expectedErr := errors.Join(discovery.ErrPresentationRegistrationFailed, errors.New("custom error"))
+		test.client.EXPECT().DeactivateServiceForSubject(gomock.Any(), serviceID, subjectID).Return(expectedErr)
+
+		response, err := test.wrapper.DeactivateServiceForSubject(nil, DeactivateServiceForSubjectRequestObject{
+			ServiceID: serviceID,
+			SubjectID: subjectID,
+		})
+
+		assert.NoError(t, err)
+		assert.IsType(t, DeactivateServiceForSubject202JSONResponse{Reason: expectedErr.Error()}, response)
+	})
 	t.Run("error", func(t *testing.T) {
 		test := newMockContext(t)
 		test.client.EXPECT().DeactivateServiceForSubject(gomock.Any(), serviceID, subjectID).Return(errors.New("foo"))

discovery/client.go

@@ -52,6 +52,8 @@ type clientRegistrationManager interface {
 	validate() error
 	// removeRevoked removes all revoked presentations from the store
 	removeRevoked() error
+	// getServiceAndSubject returns the service and subject, or ErrServiceNotFound / didsubject.ErrSubjectNotFound if either does not exist
+	getServiceAndSubject(ctx context.Context, serviceID, subjectID string) (ServiceDefinition, []did.DID, error)
 }
 
 var _ clientRegistrationManager = &defaultClientRegistrationManager{}
@@ -79,15 +81,11 @@ func newRegistrationManager(services map[string]ServiceDefinition, store *sqlSto
 }
 
 func (r *defaultClientRegistrationManager) activate(ctx context.Context, serviceID, subjectID string, parameters map[string]interface{}) error {
-	service, serviceExists := r.services[serviceID]
-	if !serviceExists {
-		return ErrServiceNotFound
-	}
-	subjectDIDs, err := r.subjectManager.ListDIDs(ctx, subjectID)
+	service, subjectDIDs, err := r.getServiceAndSubject(ctx, serviceID, subjectID)
 	if err != nil {
 		return err
 	}
-	// filter DIDs on DID methods supported by the service
+	// filter DIDs on DID methods supported by the service; len == 0 means all DID Methods are accepted
 	if len(service.DIDMethods) > 0 {
 		j := 0
 		for i, did := range subjectDIDs {
@@ -97,10 +95,6 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service
 			}
 		}
 		subjectDIDs = subjectDIDs[:j]
-
-		if len(subjectDIDs) == 0 {
-			return fmt.Errorf("%w: %w for %s", ErrPresentationRegistrationFailed, ErrDIDMethodsNotSupported, subjectID)
-		}
 	}
 
 	// and filter by deactivated status
@@ -116,7 +110,7 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service
 	subjectDIDs = subjectDIDs[:j]
 
 	if len(subjectDIDs) == 0 {
-		return fmt.Errorf("%w: %w for %s", ErrPresentationRegistrationFailed, didsubject.ErrSubjectNotFound, subjectID)
+		return fmt.Errorf("%w: %w for %s", ErrPresentationRegistrationFailed, ErrDIDMethodsNotSupported, subjectID)
 	}
 
 	log.Logger().Debugf("Registering Verifiable Presentation on Discovery Service (service=%s, subject=%s)", service.ID, subjectID)
@@ -163,22 +157,17 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service
 }
 
 func (r *defaultClientRegistrationManager) deactivate(ctx context.Context, serviceID, subjectID string) error {
-	service, serviceExists := r.services[serviceID]
-	if !serviceExists {
-		return ErrServiceNotFound
-	}
-	// delete DID/service combination from DB, so it won't be registered again
-	err := r.store.updatePresentationRefreshTime(serviceID, subjectID, nil, nil)
+	service, subjectDIDs, err := r.getServiceAndSubject(ctx, serviceID, subjectID)
 	if err != nil {
 		return err
 	}
-	// subject is now successfully deactivated for the service, anything after this point is best effort
-	subjectDIDs, err := r.subjectManager.ListDIDs(ctx, subjectID)
+	// delete DID/service combination from DB, so it won't be registered again
+	err = r.store.updatePresentationRefreshTime(serviceID, subjectID, nil, nil)
 	if err != nil {
-		// this could be a didsubject.ErrSubjectNotFound after the subject has been deactivated
-		// still fail in this case since we no longer have the keys to sign a retraction
 		return err
 	}
+	// subject is now successfully deactivated for the service,
+	// anything after this point is best-effort and should include ErrPresentationRegistrationFailed to trigger a 202 status code
 
 	// filter DIDs on DID methods supported by the service
 	if len(service.DIDMethods) > 0 {
@@ -224,6 +213,18 @@ func (r *defaultClientRegistrationManager) deactivate(ctx context.Context, servi
 	return nil
 }
 
+func (r *defaultClientRegistrationManager) getServiceAndSubject(ctx context.Context, serviceID, subjectID string) (ServiceDefinition, []did.DID, error) {
+	service, serviceExists := r.services[serviceID]
+	if !serviceExists {
+		return ServiceDefinition{}, nil, ErrServiceNotFound
+	}
+	subjectDIDs, err := r.subjectManager.ListDIDs(ctx, subjectID)
+	if err != nil {
+		return ServiceDefinition{}, nil, err
+	}
+	return service, subjectDIDs, nil
+}
+
 func (r *defaultClientRegistrationManager) deregisterPresentation(ctx context.Context, subjectDID did.DID, service ServiceDefinition, vp vc.VerifiablePresentation) error {
 	presentation, err := r.buildPresentation(ctx, subjectDID, service, nil, map[string]interface{}{
 		"retract_jti": vp.ID.String(),
@@ -304,6 +305,8 @@ func (r *defaultClientRegistrationManager) refresh(ctx context.Context, now time
 				err = r.store.updatePresentationRefreshTime(candidate.ServiceID, candidate.SubjectID, nil, nil)
 				if err != nil {
 					loopErr = fmt.Errorf("failed to remove subject with unsupported DID method (service=%s, subject=%s): %w", candidate.ServiceID, candidate.SubjectID, err)
+				} else {
+					loopErr = fmt.Errorf("removed subject that has no supported DID method (service=%s, subject=%s)", candidate.ServiceID, candidate.SubjectID)
 				}
 			} else if errors.Is(err, didsubject.ErrSubjectNotFound) {
 				// Subject has probably been deactivated. Remove from service or registration will be retried every refresh interval.

discovery/client_test.go

@@ -289,6 +289,13 @@ func Test_defaultClientRegistrationManager_deactivate(t *testing.T) {
 
 		assert.ErrorIs(t, err, didsubject.ErrSubjectNotFound)
 	})
+	t.Run("unknown service", func(t *testing.T) {
+		ctx := newTestContext(t)
+
+		err := ctx.manager.deactivate(audit.TestContext(), "unknown", aliceSubject)
+
+		assert.ErrorIs(t, err, ErrServiceNotFound)
+	})
 }
 
 func Test_defaultClientRegistrationManager_refresh(t *testing.T) {
@@ -342,26 +349,17 @@ func Test_defaultClientRegistrationManager_refresh(t *testing.T) {
 
 		assert.EqualError(t, err, "removed unknown subject (service=usecase_v1, subject=alice)")
 	})
-	t.Run("deactivate deactivated DID", func(t *testing.T) {
+	t.Run("deactivate unsupported DID method", func(t *testing.T) {
 		ctx := newTestContext(t)
 		ctx.subjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil)
 		ctx.didResolver.EXPECT().Resolve(aliceDID, gomock.Any()).Return(nil, nil, resolver.ErrDeactivated)
 		_ = ctx.store.updatePresentationRefreshTime(testServiceID, aliceSubject, nil, &nextRefresh)
 
 		err := ctx.manager.refresh(audit.TestContext(), time.Now())
 
-		assert.EqualError(t, err, "removed unknown subject (service=usecase_v1, subject=alice)")
-	})
-	t.Run("deactivate unsupported DID method", func(t *testing.T) {
-		ctx := newTestContext(t)
-		ctx.subjectManager.EXPECT().ListDIDs(gomock.Any(), aliceSubject).Return([]did.DID{aliceDID}, nil)
-		_ = ctx.store.updatePresentationRefreshTime(unsupportedServiceID, aliceSubject, defaultRegistrationParams(aliceSubject), &nextRefresh)
-
-		err := ctx.manager.refresh(audit.TestContext(), time.Now())
-
 		// refresh clears the registration
-		require.NoError(t, err)
-		record, err := ctx.store.getPresentationRefreshRecord(unsupportedServiceID, aliceSubject)
+		assert.EqualError(t, err, "removed subject that has no supported DID method (service=usecase_v1, subject=alice)")
+		record, err := ctx.store.getPresentationRefreshRecord(testServiceID, aliceSubject)
 		assert.NoError(t, err)
 		assert.Nil(t, record)
 	})