fix http status codes for did subject creation (#3427)

docs/_static/vdr/v2.yaml

@@ -131,6 +131,7 @@ paths:
         error returns:
           * 400 - the subject param was malformed
           * 404 - Corresponding subject could not be found
+          * 409 - The subject is already deactivated
           * 500 - An error occurred while processing the request
       operationId: "deactivate"
       tags:

vdr/api/v2/api.go

@@ -56,13 +56,16 @@ type Wrapper struct {
 // ResolveStatusCode maps errors returned by this API to specific HTTP status codes.
 func (w *Wrapper) ResolveStatusCode(err error) int {
 	return core.ResolveStatusCode(err, map[error]int{
-		didsubject.ErrSubjectNotFound:       http.StatusNotFound,
-		didsubject.ErrSubjectAlreadyExists:  http.StatusConflict,
-		resolver.ErrNotFound:                http.StatusNotFound,
-		resolver.ErrDIDNotManagedByThisNode: http.StatusForbidden,
-		did.ErrInvalidDID:                   http.StatusBadRequest,
-		didsubject.ErrInvalidService:        http.StatusBadRequest,
-		didsubject.ErrUnsupportedDIDMethod:  http.StatusBadRequest,
+		didsubject.ErrSubjectNotFound:          http.StatusNotFound,
+		didsubject.ErrSubjectAlreadyExists:     http.StatusConflict,
+		resolver.ErrNotFound:                   http.StatusNotFound,
+		resolver.ErrDIDNotManagedByThisNode:    http.StatusForbidden,
+		did.ErrInvalidDID:                      http.StatusBadRequest,
+		didsubject.ErrInvalidService:           http.StatusBadRequest,
+		didsubject.ErrUnsupportedDIDMethod:     http.StatusBadRequest,
+		didsubject.ErrKeyAgreementNotSupported: http.StatusBadRequest,
+		didsubject.ErrSubjectValidation:        http.StatusBadRequest,
+		resolver.ErrDeactivated:                http.StatusConflict,
 	})
 }
 
@@ -121,7 +124,6 @@ func (w *Wrapper) CreateSubject(ctx context.Context, request CreateSubjectReques
 	}
 
 	docs, subject, err := w.SubjectManager.Create(ctx, options)
-	// if this operation leads to an error, it may return a 500
 	if err != nil {
 		return nil, err
 	}

vdr/didsubject/interface.go

@@ -33,6 +33,18 @@ var ErrInvalidService = errors.New("invalid DID document service")
 // ErrUnsupportedDIDMethod is returned when a DID method is not supported.
 var ErrUnsupportedDIDMethod = errors.New("unsupported DID method")
 
+// ErrKeyAgreementNotSupported is returned key agreement is required for did:web.
+var ErrKeyAgreementNotSupported = errors.New("key agreement not supported for did:web")
+
+// ErrSubjectValidation is returned when the subject creation request is invalid.
+var ErrSubjectValidation = errors.New("subject creation validation error")
+
+// ErrSubjectAlreadyExists is returned when a subject already exists.
+var ErrSubjectAlreadyExists = errors.New("subject already exists")
+
+// ErrSubjectNotFound is returned when a subject is not found.
+var ErrSubjectNotFound = errors.New("subject not found")
+
 // MethodManager keeps DID method specific state in sync with the DID sql database.
 type MethodManager interface {
 	// NewDocument generates a new DID document for the given subject.

vdr/didsubject/manager.go

@@ -40,12 +40,6 @@ import (
 	"time"
 )
 
-// ErrSubjectAlreadyExists is returned when a subject already exists.
-var ErrSubjectAlreadyExists = errors.New("subject already exists")
-
-// ErrSubjectNotFound is returned when a subject is not found.
-var ErrSubjectNotFound = errors.New("subject not found")
-
 // subjectPattern is a regular expression for checking whether a subject follows the allowed pattern; a-z, 0-9, -, _, . (case insensitive)
 var subjectPattern = regexp.MustCompile(`^[a-zA-Z0-9.-]+$`)
 
@@ -125,15 +119,15 @@ func (r *SqlManager) Create(ctx context.Context, options CreationOptions) ([]did
 		switch opt := option.(type) {
 		case SubjectCreationOption:
 			if !subjectPattern.MatchString(opt.Subject) {
-				return nil, "", fmt.Errorf("invalid subject (must follow pattern: %s)", subjectPattern.String())
+				return nil, "", errors.Join(ErrSubjectValidation, fmt.Errorf("invalid subject (must follow pattern: %s)", subjectPattern.String()))
 			}
 			subject = opt.Subject
 		case EncryptionKeyCreationOption:
 			keyFlags = keyFlags | orm.EncryptionKeyUsage()
 		case NutsLegacyNamingOption:
 			nutsLegacy = true
 		default:
-			return nil, "", fmt.Errorf("unknown option: %T", option)
+			return nil, "", errors.Join(ErrSubjectValidation, fmt.Errorf("unknown option: %T", option))
 		}
 	}
 
@@ -153,6 +147,12 @@ func (r *SqlManager) Create(ctx context.Context, options CreationOptions) ([]did
 
 		// call generate on all managers
 		for method, manager := range r.MethodManagers {
+			// known limitation, check is also done within the manager, but at this point we can return a known error for the API
+			// requires update to nutsCrypto module
+			if keyFlags.Is(orm.KeyAgreementUsage) && method == "web" {
+				return nil, ErrKeyAgreementNotSupported
+			}
+
 			// save tx in context to pass all the way down to KeyStore
 			transactionContext := context.WithValue(ctx, storage.TransactionKey{}, tx)
 			sqlDoc, err := manager.NewDocument(transactionContext, keyFlags)
@@ -394,8 +394,8 @@ func (r *SqlManager) AddVerificationMethod(ctx context.Context, subject string,
 	err := r.applyToDIDDocuments(ctx, subject, func(tx *gorm.DB, id did.DID, current *orm.DidDocument) (*orm.DidDocument, error) {
 		// known limitation
 		if keyUsage.Is(orm.KeyAgreementUsage) && id.Method == "web" {
-			return nil, errors.New("key agreement not supported for did:web")
-			// todo requires update to nutsCrypto module
+			return nil, ErrKeyAgreementNotSupported
+			// requires update to nutsCrypto module
 			//verificationMethodKey, err = m.keyStore.NewRSA(ctx, func(key crypt.PublicKey) (string, error) {
 			//	return verificationMethodID.String(), nil
 			//})

vdr/didsubject/manager_test.go

@@ -141,7 +141,9 @@ func TestManager_Create(t *testing.T) {
 
 		_, _, err := m.Create(audit.TestContext(), DefaultCreationOptions().With(""))
 
-		require.EqualError(t, err, "unknown option: string")
+		require.Error(t, err)
+		assert.ErrorIs(t, err, ErrSubjectValidation)
+		assert.ErrorContains(t, err, "unknown option: string")
 	})
 	t.Run("already exists", func(t *testing.T) {
 		db := testDB(t)
@@ -163,7 +165,9 @@ func TestManager_Create(t *testing.T) {
 
 			_, _, err := m.Create(audit.TestContext(), DefaultCreationOptions().With(SubjectCreationOption{Subject: ""}))
 
-			require.EqualError(t, err, "invalid subject (must follow pattern: ^[a-zA-Z0-9.-]+$)")
+			require.Error(t, err)
+			assert.ErrorIs(t, err, ErrSubjectValidation)
+			assert.ErrorContains(t, err, "invalid subject (must follow pattern: ^[a-zA-Z0-9.-]+$)")
 		})
 		t.Run("contains illegal character (space)", func(t *testing.T) {
 			db := testDB(t)
@@ -173,7 +177,9 @@ func TestManager_Create(t *testing.T) {
 
 			_, _, err := m.Create(audit.TestContext(), DefaultCreationOptions().With(SubjectCreationOption{Subject: "subject with space"}))
 
-			require.EqualError(t, err, "invalid subject (must follow pattern: ^[a-zA-Z0-9.-]+$)")
+			require.Error(t, err)
+			assert.ErrorIs(t, err, ErrSubjectValidation)
+			assert.ErrorContains(t, err, "invalid subject (must follow pattern: ^[a-zA-Z0-9.-]+$)")
 		})
 	})
 }