From 7dc0954f20758ebf3464346d3407eec3966b57eb Mon Sep 17 00:00:00 2001 From: Wout Slakhorst Date: Tue, 1 Oct 2024 11:23:15 +0200 Subject: [PATCH] fix http status codes for did subject creation (#3427) --- docs/_static/vdr/v2.yaml | 1 + vdr/api/v2/api.go | 18 ++++++++++-------- vdr/didsubject/interface.go | 12 ++++++++++++ vdr/didsubject/manager.go | 20 ++++++++++---------- vdr/didsubject/manager_test.go | 12 +++++++++--- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/docs/_static/vdr/v2.yaml b/docs/_static/vdr/v2.yaml index f5ef50d75..9a254a444 100644 --- a/docs/_static/vdr/v2.yaml +++ b/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: diff --git a/vdr/api/v2/api.go b/vdr/api/v2/api.go index cb68f04bc..d1e3c6950 100644 --- a/vdr/api/v2/api.go +++ b/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 } diff --git a/vdr/didsubject/interface.go b/vdr/didsubject/interface.go index fe5e28abb..0c7d17c91 100644 --- a/vdr/didsubject/interface.go +++ b/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. diff --git a/vdr/didsubject/manager.go b/vdr/didsubject/manager.go index ebdd95377..1888ac952 100644 --- a/vdr/didsubject/manager.go +++ b/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,7 +119,7 @@ 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: @@ -133,7 +127,7 @@ func (r *SqlManager) Create(ctx context.Context, options CreationOptions) ([]did 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 //}) diff --git a/vdr/didsubject/manager_test.go b/vdr/didsubject/manager_test.go index f2746a6c7..a46b4d8e9 100644 --- a/vdr/didsubject/manager_test.go +++ b/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.-]+$)") }) }) }