From 75db28c5221414076d6c73176eaaa55aff1c5c88 Mon Sep 17 00:00:00 2001 From: Washington Kigani Kamadi Date: Fri, 10 May 2024 16:09:21 +0300 Subject: [PATCH] MG-2117 - Remove repository errors from API layer (#2119) Signed-off-by: WashingtonKK --- api/openapi/auth.yml | 4 + api/openapi/bootstrap.yml | 2 + api/openapi/users.yml | 4 + auth/api/http/domains/endpoint_test.go | 12 +- auth/api/http/keys/endpoint_test.go | 2 +- auth/service.go | 26 +- auth/service_test.go | 575 ++++++++++++++++--------- bootstrap/api/endpoint_test.go | 8 +- bootstrap/service.go | 8 +- certs/service.go | 5 +- internal/api/common.go | 91 ++-- internal/api/common_test.go | 2 +- internal/groups/service.go | 7 +- invitations/api/endpoint_test.go | 1 - pkg/errors/service/types.go | 26 +- pkg/sdk/go/certs_test.go | 8 +- pkg/sdk/go/channels_test.go | 20 +- pkg/sdk/go/groups_test.go | 22 +- pkg/sdk/go/things_test.go | 45 +- pkg/sdk/go/tokens_test.go | 2 +- pkg/sdk/go/users_test.go | 47 +- things/service.go | 44 +- things/service_test.go | 9 +- twins/api/http/endpoint_twins_test.go | 2 +- users/api/endpoint_test.go | 15 +- users/service.go | 57 ++- users/service_test.go | 4 +- 27 files changed, 627 insertions(+), 421 deletions(-) diff --git a/api/openapi/auth.yml b/api/openapi/auth.yml index 90d9c8c96c..f46872ef0c 100644 --- a/api/openapi/auth.yml +++ b/api/openapi/auth.yml @@ -152,8 +152,12 @@ paths: responses: "200": $ref: "#/components/responses/DomainPermissionRes" + "400": + description: Malformed entity specification. "401": description: Missing or invalid access token provided. + "403": + description: Failed authorization over the domain. "404": description: A non-existent entity request. "422": diff --git a/api/openapi/bootstrap.yml b/api/openapi/bootstrap.yml index 804c2d262c..004458a7c1 100644 --- a/api/openapi/bootstrap.yml +++ b/api/openapi/bootstrap.yml @@ -96,6 +96,8 @@ paths: responses: "200": $ref: "#/components/responses/ConfigRes" + "400": + description: Missing or invalid config. "401": description: Missing or invalid access token provided. "404": diff --git a/api/openapi/users.yml b/api/openapi/users.yml index 5c6643a561..f09e93076a 100644 --- a/api/openapi/users.yml +++ b/api/openapi/users.yml @@ -419,6 +419,8 @@ paths: description: Failed due to malformed JSON. "401": description: Missing or invalid access token provided. + "404": + description: Entity not found. "415": description: Missing or invalid content type. "422": @@ -517,6 +519,8 @@ paths: $ref: "#/components/responses/TokenRes" "400": description: Failed due to malformed JSON. + "401": + description: Missing or invalid access token provided. "404": description: A non-existent entity request. "415": diff --git a/auth/api/http/domains/endpoint_test.go b/auth/api/http/domains/endpoint_test.go index 2fb07fe4a8..9cf676fabc 100644 --- a/auth/api/http/domains/endpoint_test.go +++ b/auth/api/http/domains/endpoint_test.go @@ -307,21 +307,21 @@ func TestListDomains(t *testing.T) { err: nil, }, { - desc: "list domains with empty name", + desc: "list domains with empty name", token: validToken, query: "name= ", status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { - desc: "list domains with duplicate name", + desc: "list domains with duplicate name", token: validToken, query: "name=1&name=2", status: http.StatusBadRequest, err: apiutil.ErrInvalidQueryParams, }, { - desc: "list domains with status", + desc: "list domains with status", token: validToken, listDomainsRequest: auth.DomainsPage{ Total: 1, @@ -332,7 +332,7 @@ func TestListDomains(t *testing.T) { err: nil, }, { - desc: "list domains with invalid status", + desc: "list domains with invalid status", token: validToken, query: "status=invalid", status: http.StatusBadRequest, @@ -1047,7 +1047,7 @@ func TestAssignDomainUsers(t *testing.T) { contentType: contentType, token: validToken, status: http.StatusBadRequest, - err: apiutil.ErrValidation, + err: apiutil.ErrMissingID, }, { desc: "assign domain users with empty relation", @@ -1056,7 +1056,7 @@ func TestAssignDomainUsers(t *testing.T) { contentType: contentType, token: validToken, status: http.StatusBadRequest, - err: apiutil.ErrValidation, + err: apiutil.ErrMalformedPolicy, }, } diff --git a/auth/api/http/keys/endpoint_test.go b/auth/api/http/keys/endpoint_test.go index 0f2ec5eba8..a1f4fe448d 100644 --- a/auth/api/http/keys/endpoint_test.go +++ b/auth/api/http/keys/endpoint_test.go @@ -239,7 +239,7 @@ func TestRetrieve(t *testing.T) { desc: "retrieve a non-existing key", id: "non-existing", token: token.AccessToken, - status: http.StatusNotFound, + status: http.StatusBadRequest, err: svcerr.ErrNotFound, }, { diff --git a/auth/service.go b/auth/service.go index c4b4786d62..fa57c1aba3 100644 --- a/auth/service.go +++ b/auth/service.go @@ -139,7 +139,7 @@ func (svc service) Identify(ctx context.Context, token string) (Key, error) { key, err := svc.tokenizer.Parse(token) if errors.Contains(err, ErrExpiry) { err = svc.keys.Remove(ctx, key.Issuer, key.ID) - return Key{}, errors.Wrap(ErrKeyExpired, err) + return Key{}, errors.Wrap(svcerr.ErrAuthentication, errors.Wrap(ErrKeyExpired, err)) } if err != nil { return Key{}, errors.Wrap(svcerr.ErrAuthentication, errors.Wrap(errIdentify, err)) @@ -204,6 +204,16 @@ func (svc service) checkPolicy(ctx context.Context, pr PolicyReq) error { } func (svc service) checkDomain(ctx context.Context, subjectType, subject, domainID string) error { + if err := svc.agent.CheckPolicy(ctx, PolicyReq{ + Subject: subject, + SubjectType: subjectType, + Permission: MembershipPermission, + Object: domainID, + ObjectType: DomainType, + }); err != nil { + return svcerr.ErrDomainAuthorization + } + d, err := svc.domains.RetrieveByID(ctx, domainID) if err != nil { return errors.Wrap(svcerr.ErrViewEntity, err) @@ -531,7 +541,7 @@ func (svc service) CreateDomain(ctx context.Context, token string, d Domain) (do domainID, err := svc.idProvider.ID() if err != nil { - return Domain{}, err + return Domain{}, errors.Wrap(svcerr.ErrCreateEntity, err) } d.ID = domainID @@ -580,7 +590,7 @@ func (svc service) RetrieveDomain(ctx context.Context, token, id string) (Domain func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id string) (Permissions, error) { res, err := svc.Identify(ctx, token) if err != nil { - return []string{}, errors.Wrap(svcerr.ErrAuthentication, err) + return []string{}, err } if err := svc.Authorize(ctx, PolicyReq{ @@ -591,7 +601,7 @@ func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id stri ObjectType: DomainType, Permission: MembershipPermission, }); err != nil { - return []string{}, errors.Wrap(svcerr.ErrAuthorization, err) + return []string{}, err } lp, err := svc.ListPermissions(ctx, PolicyReq{ @@ -609,7 +619,7 @@ func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id stri func (svc service) UpdateDomain(ctx context.Context, token, id string, d DomainReq) (Domain, error) { key, err := svc.Identify(ctx, token) if err != nil { - return Domain{}, errors.Wrap(svcerr.ErrAuthentication, err) + return Domain{}, err } if err := svc.Authorize(ctx, PolicyReq{ Subject: key.Subject, @@ -619,7 +629,7 @@ func (svc service) UpdateDomain(ctx context.Context, token, id string, d DomainR ObjectType: DomainType, Permission: EditPermission, }); err != nil { - return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err) + return Domain{}, err } dom, err := svc.domains.Update(ctx, id, key.User, d) @@ -642,7 +652,7 @@ func (svc service) ChangeDomainStatus(ctx context.Context, token, id string, d D ObjectType: DomainType, Permission: AdminPermission, }); err != nil { - return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err) + return Domain{}, err } dom, err := svc.domains.Update(ctx, id, key.User, d) @@ -765,7 +775,7 @@ func (svc service) UnassignUsers(ctx context.Context, token, id string, userIds for _, rel := range []string{MemberRelation, ViewerRelation, EditorRelation} { // Remove only non-admins. if err := svc.removeDomainPolicies(ctx, id, rel, userIds...); err != nil { - return errors.Wrap(errRemovePolicies, err) + return err } } diff --git a/auth/service_test.go b/auth/service_test.go index 25824eb224..c47e53ec69 100644 --- a/auth/service_test.go +++ b/auth/service_test.go @@ -133,18 +133,19 @@ func TestIssue(t *testing.T) { } cases2 := []struct { - desc string - key auth.Key - saveResponse auth.Key - retrieveByIDResponse auth.Domain - token string - saveErr error - checkPolicyRequest auth.PolicyReq - checkPolicyRequest1 auth.PolicyReq - checkPolicyErr error - checkPolicyErr1 error - retreiveByIDErr error - err error + desc string + key auth.Key + saveResponse auth.Key + retrieveByIDResponse auth.Domain + token string + saveErr error + checkPolicyRequest auth.PolicyReq + checkPlatformPolicyReq auth.PolicyReq + checkDomainPolicyReq auth.PolicyReq + checkPolicyErr error + checkPolicyErr1 error + retreiveByIDErr error + err error }{ { desc: "issue login key", @@ -153,14 +154,16 @@ func TestIssue(t *testing.T) { IssuedAt: time.Now(), }, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: accessToken, err: nil, }, @@ -172,14 +175,16 @@ func TestIssue(t *testing.T) { Domain: groupName, }, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: accessToken, err: nil, }, @@ -192,14 +197,17 @@ func TestIssue(t *testing.T) { }, token: accessToken, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPlatformPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + Object: groupName, + }, checkPolicyErr: repoerr.ErrNotFound, retrieveByIDResponse: auth.Domain{}, retreiveByIDErr: repoerr.ErrNotFound, @@ -214,26 +222,26 @@ func TestIssue(t *testing.T) { }, token: accessToken, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ - Subject: "", + checkPlatformPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: groupName, - ObjectKind: "", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, + checkDomainPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, checkPolicyErr: svcerr.ErrAuthorization, + checkPolicyErr1: svcerr.ErrAuthorization, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, - err: nil, + err: svcerr.ErrAuthorization, }, { desc: "issue login key with membership permission", @@ -244,27 +252,26 @@ func TestIssue(t *testing.T) { }, token: accessToken, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ - Subject: "", + checkPlatformPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: groupName, - ObjectKind: "", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, + checkDomainPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, checkPolicyErr: svcerr.ErrAuthorization, - checkPolicyErr1: nil, + checkPolicyErr1: svcerr.ErrAuthorization, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, - err: nil, + err: svcerr.ErrAuthorization, }, { desc: "issue login key with membership permission with failed to authorize", @@ -275,20 +282,19 @@ func TestIssue(t *testing.T) { }, token: accessToken, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ - Subject: "", + checkPlatformPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: groupName, - ObjectKind: "", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, + checkDomainPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -301,14 +307,16 @@ func TestIssue(t *testing.T) { for _, tc := range cases2 { repoCall := krepo.On("Save", mock.Anything, mock.Anything).Return(mock.Anything, tc.saveErr) repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest).Return(tc.checkPolicyErr) - repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest1).Return(tc.checkPolicyErr1) + repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPlatformPolicyReq).Return(tc.checkPolicyErr1) repoCall3 := drepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(tc.retrieveByIDResponse, tc.retreiveByIDErr) + repoCall4 := prepo.On("CheckPolicy", mock.Anything, tc.checkDomainPolicyReq).Return(tc.checkPolicyErr) _, err := svc.Issue(context.Background(), tc.token, tc.key) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s expected %s got %s\n", tc.desc, tc.err, err)) repoCall.Unset() repoCall1.Unset() repoCall2.Unset() repoCall3.Unset() + repoCall4.Unset() } cases3 := []struct { @@ -364,13 +372,14 @@ func TestIssue(t *testing.T) { } cases4 := []struct { - desc string - key auth.Key - token string - checkPolicyRequest auth.PolicyReq - checkPolicyErr error - retrieveByIDErr error - err error + desc string + key auth.Key + token string + checkPolicyRequest auth.PolicyReq + checkDOmainPolicyReq auth.PolicyReq + checkPolicyErr error + retrieveByIDErr error + err error }{ { desc: "issue refresh key", @@ -381,9 +390,7 @@ func TestIssue(t *testing.T) { checkPolicyRequest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, @@ -399,12 +406,17 @@ func TestIssue(t *testing.T) { checkPolicyRequest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDOmainPolicyReq: auth.PolicyReq{ + Subject: "mgx_test@example.com", + SubjectType: auth.UserType, + Object: groupName, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: refreshToken, checkPolicyErr: svcerr.ErrAuthorization, retrieveByIDErr: repoerr.ErrNotFound, @@ -416,6 +428,12 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, + checkDOmainPolicyReq: auth.PolicyReq{ + Subject: "mgx_test@example.com", + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: accessToken, err: errIssueUser, }, @@ -425,6 +443,12 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, + checkDOmainPolicyReq: auth.PolicyReq{ + Subject: "mgx_test@example.com", + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: "", err: errRetrieve, }, @@ -437,9 +461,7 @@ func TestIssue(t *testing.T) { checkPolicyRequest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, @@ -454,27 +476,32 @@ func TestIssue(t *testing.T) { Domain: groupName, }, checkPolicyRequest: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDOmainPolicyReq: auth.PolicyReq{ + SubjectType: auth.UserType, + Object: groupName, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: refreshToken, checkPolicyErr: svcerr.ErrAuthorization, retrieveByIDErr: repoerr.ErrNotFound, - err: svcerr.ErrNotFound, + err: svcerr.ErrDomainAuthorization, }, } for _, tc := range cases4 { repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest).Return(tc.checkPolicyErr) repoCall1 := drepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(auth.Domain{}, tc.retrieveByIDErr) + repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkDOmainPolicyReq).Return(tc.checkPolicyErr) _, err := svc.Issue(context.Background(), tc.token, tc.key) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s expected %s got %s\n", tc.desc, tc.err, err)) repoCall.Unset() repoCall1.Unset() + repoCall2.Unset() } } @@ -733,16 +760,16 @@ func TestAuthorize(t *testing.T) { emptyDomain, _ := te.Issue(key) cases := []struct { - desc string - policyReq auth.PolicyReq - retrieveDomainRes auth.Domain - checkPolicyReq auth.PolicyReq - checkPolicyReq1 auth.PolicyReq - checkPolicyReq2 auth.PolicyReq - checkPolicyErr error - checkPolicyErr1 error - checkPolicyErr2 error - err error + desc string + policyReq auth.PolicyReq + retrieveDomainRes auth.Domain + checkPolicyReq3 auth.PolicyReq + checkAdminPolicyReq auth.PolicyReq + checkDomainPolicyReq auth.PolicyReq + checkPolicyErr error + checkPolicyErr1 error + checkPolicyErr2 error + err error }{ { desc: "authorize token successfully", @@ -754,7 +781,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: id, SubjectType: auth.UserType, @@ -763,6 +790,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: nil, }, { @@ -775,7 +808,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.GroupType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, SubjectKind: auth.TokenKind, @@ -783,7 +816,14 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.GroupType, Permission: auth.AdminPermission, }, - err: svcerr.ErrDomainAuthorization, + checkAdminPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, + err: svcerr.ErrDomainAuthorization, + checkPolicyErr: svcerr.ErrDomainAuthorization, }, { desc: "authorize token with disabled domain", @@ -795,21 +835,28 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, - SubjectKind: auth.TokenKind, Object: validID, ObjectType: auth.DomainType, - Permission: auth.AdminPermission, + Permission: auth.MembershipPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + SubjectKind: auth.TokenKind, Permission: auth.AdminPermission, Object: validID, ObjectType: auth.DomainType, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.AdminPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -828,21 +875,27 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, - SubjectKind: auth.TokenKind, - Object: validID, ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + SubjectKind: auth.TokenKind, Permission: auth.AdminPermission, Object: validID, ObjectType: auth.DomainType, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -862,7 +915,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, SubjectKind: auth.TokenKind, @@ -870,13 +923,20 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Permission: auth.AdminPermission, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -895,7 +955,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, SubjectKind: auth.TokenKind, @@ -903,13 +963,20 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Permission: auth.AdminPermission, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -929,7 +996,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, SubjectKind: auth.TokenKind, @@ -937,13 +1004,20 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Permission: auth.AdminPermission, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -963,14 +1037,20 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrAuthentication, }, { @@ -983,12 +1063,19 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrAuthentication, }, { @@ -1001,31 +1088,44 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformKind, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrDomainAuthorization, }, { desc: "authorize a user key successfully", policyReq: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, SubjectKind: auth.UsersKind, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ SubjectType: auth.UserType, SubjectKind: auth.UsersKind, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: nil, }, { @@ -1038,28 +1138,38 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: validID, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrDomainAuthorization, }, } for _, tc := range cases { - repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq).Return(tc.checkPolicyErr) + fmt.Println(tc.desc) + repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3).Return(tc.checkPolicyErr) repoCall1 := drepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(tc.retrieveDomainRes, nil) - repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq1).Return(tc.checkPolicyErr1) - repoCall3 := krepo.On("Remove", mock.Anything, mock.Anything, mock.Anything).Return(nil) + repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkAdminPolicyReq).Return(tc.checkPolicyErr1) + repoCall3 := prepo.On("CheckPolicy", mock.Anything, tc.checkDomainPolicyReq).Return(tc.checkPolicyErr1) + repoCall4 := krepo.On("Remove", mock.Anything, mock.Anything, mock.Anything).Return(nil) err := svc.Authorize(context.Background(), tc.policyReq) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s expected %s got %s\n", tc.desc, tc.err, err)) repoCall.Unset() repoCall1.Unset() repoCall2.Unset() repoCall3.Unset() + repoCall4.Unset() } - + fmt.Println("Cases 1 end") cases2 := []struct { desc string policyReq auth.PolicyReq @@ -1068,7 +1178,6 @@ func TestAuthorize(t *testing.T) { { desc: "authorize token with invalid platform validation", policyReq: auth.PolicyReq{ - Subject: "", SubjectType: auth.UserType, SubjectKind: auth.UsersKind, Object: validID, @@ -1796,7 +1905,7 @@ func TestRetrieveDomainPermissions(t *testing.T) { token: accessToken, domainID: "", checkPolicyErr: svcerr.ErrAuthorization, - err: svcerr.ErrAuthorization, + err: svcerr.ErrDomainAuthorization, }, { desc: "retrieve domain permissions with failed to retrieve permissions", @@ -1868,7 +1977,7 @@ func TestUpdateDomain(t *testing.T) { Alias: &valid, }, checkPolicyErr: svcerr.ErrAuthorization, - err: svcerr.ErrAuthorization, + err: svcerr.ErrDomainAuthorization, }, { desc: "update domain with failed to retrieve by id", @@ -1947,7 +2056,7 @@ func TestChangeDomainStatus(t *testing.T) { Status: &disabledStatus, }, retreieveByIDErr: repoerr.ErrNotFound, - err: svcerr.ErrAuthorization, + err: svcerr.ErrNotFound, }, { desc: "change domain status with unauthorized domain ID", @@ -1957,7 +2066,7 @@ func TestChangeDomainStatus(t *testing.T) { Status: &disabledStatus, }, checkPolicyErr: svcerr.ErrAuthorization, - err: svcerr.ErrAuthorization, + err: svcerr.ErrDomainAuthorization, }, { desc: "change domain status with repository error on update", @@ -2053,21 +2162,22 @@ func TestAssignUsers(t *testing.T) { svc, accessToken := newService() cases := []struct { - desc string - token string - domainID string - userIDs []string - relation string - checkPolicyReq auth.PolicyReq - checkPolicyReq1 auth.PolicyReq - checkPolicyReq2 auth.PolicyReq - checkpolicyErr error - checkPolicyErr1 error - checkPolicyErr2 error - addPoliciesErr error - savePoliciesErr error - deletePoliciesErr error - err error + desc string + token string + domainID string + userIDs []string + relation string + checkPolicyReq3 auth.PolicyReq + checkAdminPolicyReq auth.PolicyReq + checkDomainPolicyReq auth.PolicyReq + checkPolicyReq33 auth.PolicyReq + checkpolicyErr error + checkPolicyErr1 error + checkPolicyErr2 error + addPoliciesErr error + savePoliciesErr error + deletePoliciesErr error + err error }{ { desc: "assign users successfully", @@ -2075,35 +2185,38 @@ func TestAssignUsers(t *testing.T) { domainID: validID, userIDs: []string{validID}, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: validID, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq33: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: nil, }, @@ -2113,32 +2226,28 @@ func TestAssignUsers(t *testing.T) { domainID: validID, userIDs: []string{validID}, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: validID, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, @@ -2149,26 +2258,31 @@ func TestAssignUsers(t *testing.T) { token: accessToken, domainID: inValid, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: inValid, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: inValid, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, + checkPolicyReq33: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr1: svcerr.ErrAuthorization, err: svcerr.ErrAuthorization, }, @@ -2178,37 +2292,40 @@ func TestAssignUsers(t *testing.T) { userIDs: []string{inValid}, domainID: validID, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: inValid, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq33: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr2: svcerr.ErrMalformedEntity, - err: svcerr.ErrMalformedEntity, + err: svcerr.ErrDomainAuthorization, }, { desc: "assign users with failed to add policies to agent", @@ -2216,35 +2333,38 @@ func TestAssignUsers(t *testing.T) { domainID: validID, userIDs: []string{validID}, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: validID, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq33: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, addPoliciesErr: svcerr.ErrAuthorization, err: errAddPolicies, }, @@ -2254,35 +2374,38 @@ func TestAssignUsers(t *testing.T) { domainID: validID, userIDs: []string{validID}, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: validID, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq33: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, savePoliciesErr: repoerr.ErrCreateEntity, err: errAddPolicies, }, @@ -2292,49 +2415,53 @@ func TestAssignUsers(t *testing.T) { domainID: validID, userIDs: []string{validID}, relation: auth.ViewerRelation, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: validID, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq33: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, savePoliciesErr: repoerr.ErrCreateEntity, - deletePoliciesErr: errors.ErrMalformedEntity, + deletePoliciesErr: svcerr.ErrDomainAuthorization, err: errAddPolicies, }, } for _, tc := range cases { repoCall := drepo.On("RetrieveByID", mock.Anything, groupName).Return(auth.Domain{}, nil) - repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq).Return(tc.checkpolicyErr) - repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq1).Return(tc.checkPolicyErr1) - repoCall3 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq2).Return(tc.checkPolicyErr2) - repoCall4 := prepo.On("AddPolicies", mock.Anything, mock.Anything).Return(tc.addPoliciesErr) - repoCall5 := drepo.On("SavePolicies", mock.Anything, mock.Anything, mock.Anything).Return(tc.savePoliciesErr) - repoCall6 := prepo.On("DeletePolicies", mock.Anything, mock.Anything).Return(tc.deletePoliciesErr) + repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3).Return(tc.checkpolicyErr) + repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkAdminPolicyReq).Return(tc.checkPolicyErr1) + repoCall3 := prepo.On("CheckPolicy", mock.Anything, tc.checkDomainPolicyReq).Return(tc.checkPolicyErr2) + repoCall4 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq33).Return(tc.checkPolicyErr2) + repoCall5 := prepo.On("AddPolicies", mock.Anything, mock.Anything).Return(tc.addPoliciesErr) + repoCall6 := drepo.On("SavePolicies", mock.Anything, mock.Anything, mock.Anything).Return(tc.savePoliciesErr) + repoCall7 := prepo.On("DeletePolicies", mock.Anything, mock.Anything).Return(tc.deletePoliciesErr) err := svc.AssignUsers(context.Background(), tc.token, tc.domainID, tc.userIDs, tc.relation) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s expected %s got %s\n", tc.desc, tc.err, err)) repoCall.Unset() @@ -2344,6 +2471,7 @@ func TestAssignUsers(t *testing.T) { repoCall4.Unset() repoCall5.Unset() repoCall6.Unset() + repoCall7.Unset() } } @@ -2351,40 +2479,46 @@ func TestUnassignUsers(t *testing.T) { svc, accessToken := newService() cases := []struct { - desc string - token string - domainID string - checkPolicyReq auth.PolicyReq - checkPolicyReq1 auth.PolicyReq - checkPolicyErr error - checkPolicyErr1 error - deletePoliciesErr error - deletePoliciesErr1 error - err error + desc string + token string + domainID string + checkPolicyReq3 auth.PolicyReq + checkAdminPolicyReq auth.PolicyReq + checkDomainPolicyReq auth.PolicyReq + checkPolicyErr error + checkPolicyErr1 error + deletePoliciesErr error + deletePoliciesErr1 error + err error }{ { desc: "unassign users successfully", token: accessToken, domainID: validID, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, - Permission: auth.SharePermission, + Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, - Permission: auth.AdminPermission, + Permission: auth.SharePermission, }, err: nil, }, @@ -2392,23 +2526,21 @@ func TestUnassignUsers(t *testing.T) { desc: "unassign users with invalid token", token: inValidToken, domainID: validID, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, @@ -2418,53 +2550,63 @@ func TestUnassignUsers(t *testing.T) { desc: "unassign users with invalid domainID", token: accessToken, domainID: inValid, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: inValid, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: inValid, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr1: svcerr.ErrAuthorization, - err: svcerr.ErrAuthorization, + err: svcerr.ErrDomainAuthorization, }, { - desc: "unassign users with failed to delete policies from agant", + desc: "unassign users with failed to delete policies from agent", token: accessToken, domainID: validID, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, deletePoliciesErr: errors.ErrMalformedEntity, err: errors.ErrMalformedEntity, }, @@ -2472,26 +2614,31 @@ func TestUnassignUsers(t *testing.T) { desc: "unassign users with failed to delete policies from domain", token: accessToken, domainID: validID, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, + checkDomainPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, deletePoliciesErr1: errors.ErrMalformedEntity, err: errors.ErrMalformedEntity, }, @@ -2499,10 +2646,11 @@ func TestUnassignUsers(t *testing.T) { for _, tc := range cases { repoCall := drepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(auth.Domain{}, nil) - repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq).Return(tc.checkPolicyErr) - repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq1).Return(tc.checkPolicyErr1) - repoCall3 := prepo.On("DeletePolicies", mock.Anything, mock.Anything, mock.Anything).Return(tc.deletePoliciesErr) - repoCall4 := drepo.On("DeletePolicies", mock.Anything, mock.Anything, mock.Anything).Return(tc.deletePoliciesErr1) + repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3).Return(tc.checkPolicyErr) + repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkAdminPolicyReq).Return(tc.checkPolicyErr1) + repoCall3 := prepo.On("CheckPolicy", mock.Anything, tc.checkDomainPolicyReq).Return(tc.checkPolicyErr1) + repoCall4 := prepo.On("DeletePolicies", mock.Anything, mock.Anything, mock.Anything).Return(tc.deletePoliciesErr) + repoCall5 := drepo.On("DeletePolicies", mock.Anything, mock.Anything, mock.Anything).Return(tc.deletePoliciesErr1) err := svc.UnassignUsers(context.Background(), tc.token, tc.domainID, []string{" ", " "}, auth.AdministratorRelation) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s expected %s got %s\n", tc.desc, tc.err, err)) repoCall.Unset() @@ -2510,6 +2658,7 @@ func TestUnassignUsers(t *testing.T) { repoCall2.Unset() repoCall3.Unset() repoCall4.Unset() + repoCall5.Unset() } } diff --git a/bootstrap/api/endpoint_test.go b/bootstrap/api/endpoint_test.go index 881eb1f2f5..36e331ab0a 100644 --- a/bootstrap/api/endpoint_test.go +++ b/bootstrap/api/endpoint_test.go @@ -88,9 +88,9 @@ var ( missingIDRes = toJSON(apiutil.ErrorRes{Err: apiutil.ErrMissingID.Error(), Msg: apiutil.ErrValidation.Error()}) missingKeyRes = toJSON(apiutil.ErrorRes{Err: apiutil.ErrBearerKey.Error(), Msg: apiutil.ErrValidation.Error()}) - bsErrorRes = toJSON(apiutil.ErrorRes{Err: svcerr.ErrNotFound.Error(), Msg: bootstrap.ErrBootstrap.Error()}) + bsErrorRes = toJSON(apiutil.ErrorRes{Msg: bootstrap.ErrBootstrap.Error()}) extKeyRes = toJSON(apiutil.ErrorRes{Msg: bootstrap.ErrExternalKey.Error()}) - extSecKeyRes = toJSON(apiutil.ErrorRes{Err: "encoding/hex: invalid byte: U+002D '-'", Msg: bootstrap.ErrExternalKeySecure.Error()}) + extSecKeyRes = toJSON(apiutil.ErrorRes{Msg: bootstrap.ErrExternalKeySecure.Error()}) ) type testRequest struct { @@ -1138,7 +1138,7 @@ func TestBootstrap(t *testing.T) { status: http.StatusNotFound, res: bsErrorRes, secure: false, - err: errors.Wrap(bootstrap.ErrBootstrap, svcerr.ErrNotFound), + err: bootstrap.ErrBootstrap, }, { desc: "bootstrap a Thing with an empty ID", @@ -1192,7 +1192,7 @@ func TestBootstrap(t *testing.T) { status: http.StatusForbidden, res: extSecKeyRes, secure: true, - err: errors.Wrap(bootstrap.ErrExternalKeySecure, errors.New("encoding/hex: invalid byte: U+002D '-'")), + err: bootstrap.ErrExternalKeySecure, }, } diff --git a/bootstrap/service.go b/bootstrap/service.go index 781a7904c6..1214e944c4 100644 --- a/bootstrap/service.go +++ b/bootstrap/service.go @@ -31,7 +31,9 @@ var ( // ErrBootstrap indicates error in getting bootstrap configuration. ErrBootstrap = errors.New("failed to read bootstrap configuration") - errAddBootstrap = errors.New("failed to add bootstrap configuration") + // ErrAddBootstrap indicates error in adding bootstrap configuration. + ErrAddBootstrap = errors.New("failed to add bootstrap configuration") + errUpdateConnections = errors.New("failed to update connections") errRemoveBootstrap = errors.New("failed to remove bootstrap configuration") errChangeState = errors.New("failed to change state of bootstrap configuration") @@ -164,7 +166,7 @@ func (bs bootstrapService) Add(ctx context.Context, token string, cfg Config) (C err = errors.Wrap(err, errT) } } - return Config{}, errors.Wrap(errAddBootstrap, err) + return Config{}, errors.Wrap(ErrAddBootstrap, err) } cfg.ThingID = saved @@ -400,7 +402,7 @@ func (bs bootstrapService) thing(id, token string) (mgsdk.Thing, error) { } thing, sdkErr := bs.sdk.CreateThing(mgsdk.Thing{ID: id, Name: "Bootstrapped Thing " + id}, token) if sdkErr != nil { - return mgsdk.Thing{}, errors.Wrap(errCreateThing, errors.New(sdkErr.Err().Msg())) + return mgsdk.Thing{}, errors.Wrap(errCreateThing, sdkErr) } return thing, nil } diff --git a/certs/service.go b/certs/service.go index 5fdba4a51e..191b328594 100644 --- a/certs/service.go +++ b/certs/service.go @@ -10,7 +10,6 @@ import ( "github.com/absmach/magistrala" "github.com/absmach/magistrala/certs/pki" "github.com/absmach/magistrala/pkg/errors" - repoerr "github.com/absmach/magistrala/pkg/errors/repository" svcerr "github.com/absmach/magistrala/pkg/errors/service" mgsdk "github.com/absmach/magistrala/pkg/sdk/go" ) @@ -156,7 +155,7 @@ func (cs *certsService) ListCerts(ctx context.Context, token, thingID string, of cp, err := cs.certsRepo.RetrieveByThing(ctx, u.GetId(), thingID, offset, limit) if err != nil { - return Page{}, errors.Wrap(repoerr.ErrNotFound, err) + return Page{}, errors.Wrap(svcerr.ErrViewEntity, err) } for i, cert := range cp.Certs { @@ -188,7 +187,7 @@ func (cs *certsService) ViewCert(ctx context.Context, token, serialID string) (C cert, err := cs.certsRepo.RetrieveBySerial(ctx, u.GetId(), serialID) if err != nil { - return Cert{}, errors.Wrap(repoerr.ErrNotFound, err) + return Cert{}, errors.Wrap(svcerr.ErrViewEntity, err) } vcert, err := cs.pki.Read(serialID) diff --git a/internal/api/common.go b/internal/api/common.go index 17703ab8f9..8d675d0189 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -106,66 +106,77 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { w.Header().Set("Content-Type", ContentType) switch { + case errors.Contains(err, svcerr.ErrAuthorization), + errors.Contains(err, svcerr.ErrDomainAuthorization), + errors.Contains(err, bootstrap.ErrExternalKey), + errors.Contains(err, bootstrap.ErrExternalKeySecure): + err = unwrap(err) + w.WriteHeader(http.StatusForbidden) + + case errors.Contains(err, svcerr.ErrAuthentication), + errors.Contains(err, apiutil.ErrBearerToken), + errors.Contains(err, svcerr.ErrLogin): + err = unwrap(err) + w.WriteHeader(http.StatusUnauthorized) case errors.Contains(err, svcerr.ErrMalformedEntity), + errors.Contains(err, apiutil.ErrMalformedPolicy), + errors.Contains(err, apiutil.ErrMissingSecret), errors.Contains(err, errors.ErrMalformedEntity), errors.Contains(err, apiutil.ErrMissingID), + errors.Contains(err, apiutil.ErrMissingName), + errors.Contains(err, apiutil.ErrMissingEmail), + errors.Contains(err, apiutil.ErrMissingHost), + errors.Contains(err, apiutil.ErrInvalidResetPass), errors.Contains(err, apiutil.ErrEmptyList), - errors.Contains(err, apiutil.ErrMissingMemberType), errors.Contains(err, apiutil.ErrMissingMemberKind), + errors.Contains(err, apiutil.ErrMissingMemberType), errors.Contains(err, apiutil.ErrLimitSize), errors.Contains(err, apiutil.ErrBearerKey), + errors.Contains(err, svcerr.ErrInvalidStatus), errors.Contains(err, apiutil.ErrNameSize), errors.Contains(err, apiutil.ErrInvalidIDFormat), - errors.Contains(err, svcerr.ErrInvalidStatus), + errors.Contains(err, apiutil.ErrInvalidQueryParams), + errors.Contains(err, apiutil.ErrMissingRelation), errors.Contains(err, apiutil.ErrValidation), - errors.Contains(err, apiutil.ErrInvitationState), - errors.Contains(err, apiutil.ErrInvalidRole), - errors.Contains(err, apiutil.ErrMissingEmail), - errors.Contains(err, apiutil.ErrMissingHost), errors.Contains(err, apiutil.ErrMissingIdentity), - errors.Contains(err, apiutil.ErrMissingSecret), errors.Contains(err, apiutil.ErrMissingPass), errors.Contains(err, apiutil.ErrMissingConfPass), - errors.Contains(err, apiutil.ErrInvalidResetPass), - errors.Contains(err, apiutil.ErrMissingRelation), errors.Contains(err, apiutil.ErrPasswordFormat), - errors.Contains(err, apiutil.ErrInvalidLevel), - errors.Contains(err, apiutil.ErrMalformedPolicy), + errors.Contains(err, svcerr.ErrInvalidRole), + errors.Contains(err, svcerr.ErrInvalidPolicy), + errors.Contains(err, apiutil.ErrInvitationState), errors.Contains(err, apiutil.ErrInvalidAPIKey), - errors.Contains(err, apiutil.ErrMissingName), + errors.Contains(err, svcerr.ErrViewEntity), errors.Contains(err, apiutil.ErrBootstrapState), errors.Contains(err, apiutil.ErrMissingCertData), - errors.Contains(err, apiutil.ErrInvalidCertData), errors.Contains(err, apiutil.ErrInvalidContact), errors.Contains(err, apiutil.ErrInvalidTopic), - errors.Contains(err, apiutil.ErrInvalidQueryParams): + errors.Contains(err, bootstrap.ErrAddBootstrap), + errors.Contains(err, apiutil.ErrInvalidCertData): + err = unwrap(err) w.WriteHeader(http.StatusBadRequest) - case errors.Contains(err, svcerr.ErrAuthentication), - errors.Contains(err, svcerr.ErrLogin), - errors.Contains(err, apiutil.ErrBearerToken): - w.WriteHeader(http.StatusUnauthorized) - case errors.Contains(err, svcerr.ErrNotFound): + + case errors.Contains(err, svcerr.ErrCreateEntity), + errors.Contains(err, svcerr.ErrUpdateEntity), + errors.Contains(err, svcerr.ErrRemoveEntity), + errors.Contains(err, svcerr.ErrEnableClient): + err = unwrap(err) + w.WriteHeader(http.StatusUnprocessableEntity) + + case errors.Contains(err, svcerr.ErrNotFound), + errors.Contains(err, bootstrap.ErrBootstrap): + err = unwrap(err) w.WriteHeader(http.StatusNotFound) - case errors.Contains(err, svcerr.ErrConflict), - errors.Contains(err, errors.ErrStatusAlreadyAssigned): + + case errors.Contains(err, errors.ErrStatusAlreadyAssigned), + errors.Contains(err, svcerr.ErrConflict): + err = unwrap(err) w.WriteHeader(http.StatusConflict) - case errors.Contains(err, svcerr.ErrAuthorization), - errors.Contains(err, svcerr.ErrDomainAuthorization), - errors.Contains(err, bootstrap.ErrExternalKey), - errors.Contains(err, bootstrap.ErrExternalKeySecure): - w.WriteHeader(http.StatusForbidden) + case errors.Contains(err, apiutil.ErrUnsupportedContentType): + err = unwrap(err) w.WriteHeader(http.StatusUnsupportedMediaType) - case errors.Contains(err, svcerr.ErrCreateEntity), - errors.Contains(err, svcerr.ErrUpdateEntity), - errors.Contains(err, svcerr.ErrFailedUpdateRole), - errors.Contains(err, svcerr.ErrViewEntity), - errors.Contains(err, svcerr.ErrAddPolicies), - errors.Contains(err, svcerr.ErrDeletePolicies), - errors.Contains(err, svcerr.ErrRemoveEntity): - w.WriteHeader(http.StatusUnprocessableEntity) - case errors.Contains(err, bootstrap.ErrThings): - w.WriteHeader(http.StatusServiceUnavailable) + default: w.WriteHeader(http.StatusInternalServerError) } @@ -180,3 +191,11 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { } } } + +func unwrap(err error) error { + wrapper, err := errors.Unwrap(err) + if wrapper != nil { + return wrapper + } + return err +} diff --git a/internal/api/common_test.go b/internal/api/common_test.go index 82d654a7f0..be53ee8c88 100644 --- a/internal/api/common_test.go +++ b/internal/api/common_test.go @@ -234,6 +234,7 @@ func TestEncodeError(t *testing.T) { apiutil.ErrMissingMemberKind, apiutil.ErrLimitSize, apiutil.ErrNameSize, + svcerr.ErrViewEntity, }, code: http.StatusBadRequest, }, @@ -298,7 +299,6 @@ func TestEncodeError(t *testing.T) { errs: []error{ svcerr.ErrCreateEntity, svcerr.ErrUpdateEntity, - svcerr.ErrViewEntity, svcerr.ErrRemoveEntity, }, code: http.StatusUnprocessableEntity, diff --git a/internal/groups/service.go b/internal/groups/service.go index 78a6f621aa..a637185c0c 100644 --- a/internal/groups/service.go +++ b/internal/groups/service.go @@ -13,7 +13,6 @@ import ( "github.com/absmach/magistrala/internal/apiutil" mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" - repoerr "github.com/absmach/magistrala/pkg/errors/repository" svcerr "github.com/absmach/magistrala/pkg/errors/service" "github.com/absmach/magistrala/pkg/groups" "golang.org/x/sync/errgroup" @@ -117,7 +116,7 @@ func (svc service) ViewGroup(ctx context.Context, token, id string) (groups.Grou group, err := svc.groups.RetrieveByID(ctx, id) if err != nil { - return groups.Group{}, errors.Wrap(repoerr.ErrViewEntity, err) + return groups.Group{}, errors.Wrap(svcerr.ErrViewEntity, err) } return group, nil @@ -467,7 +466,7 @@ func (svc service) assignParentGroup(ctx context.Context, domain, parentGroupID var deletePolicies magistrala.DeletePoliciesReq for _, group := range groupsPage.Groups { if group.Parent != "" { - return errors.Wrap(repoerr.ErrConflict, fmt.Errorf("%s group already have parent", group.ID)) + return errors.Wrap(svcerr.ErrConflict, fmt.Errorf("%s group already have parent", group.ID)) } addPolicies.AddPoliciesReq = append(addPolicies.AddPoliciesReq, &magistrala.AddPolicyReq{ Domain: domain, @@ -513,7 +512,7 @@ func (svc service) unassignParentGroup(ctx context.Context, domain, parentGroupI var deletePolicies magistrala.DeletePoliciesReq for _, group := range groupsPage.Groups { if group.Parent != "" && group.Parent != parentGroupID { - return errors.Wrap(repoerr.ErrConflict, fmt.Errorf("%s group doesn't have same parent", group.ID)) + return errors.Wrap(svcerr.ErrConflict, fmt.Errorf("%s group doesn't have same parent", group.ID)) } addPolicies.AddPoliciesReq = append(addPolicies.AddPoliciesReq, &magistrala.AddPolicyReq{ Domain: domain, diff --git a/invitations/api/endpoint_test.go b/invitations/api/endpoint_test.go index 7810eb6915..f77cd85a8c 100644 --- a/invitations/api/endpoint_test.go +++ b/invitations/api/endpoint_test.go @@ -296,7 +296,6 @@ func TestListInvitation(t *testing.T) { token: tc.token, contentType: tc.contentType, } - res, err := req.make() assert.Nil(t, err, tc.desc) assert.Equal(t, tc.status, res.StatusCode, tc.desc) diff --git a/pkg/errors/service/types.go b/pkg/errors/service/types.go index e7d04936dd..a19f4b0f25 100644 --- a/pkg/errors/service/types.go +++ b/pkg/errors/service/types.go @@ -29,7 +29,7 @@ var ( ErrConflict = errors.New("entity already exists") // ErrCreateEntity indicates error in creating entity or entities. - ErrCreateEntity = errors.New("failed to create entity in the db") + ErrCreateEntity = errors.New("failed to create entity") // ErrRemoveEntity indicates error in removing entity. ErrRemoveEntity = errors.New("failed to remove entity") @@ -55,18 +55,24 @@ var ( // ErrFailedPolicyUpdate indicates a failure to update user policy. ErrFailedPolicyUpdate = errors.New("failed to update user policy") - // ErrAddPolicies indicates failed to add policies. - ErrAddPolicies = errors.New("failed to add policies") - - // ErrDeletePolicies indicates failed to delete policies. - ErrDeletePolicies = errors.New("failed to delete policies") - - // ErrIssueToken indicates a failure to issue token. - ErrIssueToken = errors.New("failed to issue token") - // ErrPasswordFormat indicates weak password. ErrPasswordFormat = errors.New("password does not meet the requirements") // ErrFailedUpdateRole indicates a failure to update user role. ErrFailedUpdateRole = errors.New("failed to update user role") + + // ErrFailedPermissionsList indicates a failure to list permissions. + ErrFailedPermissionsList = errors.New("failed to list permissions") + + // ErrEnableClient indicates error in enabling client. + ErrEnableClient = errors.New("failed to enable client") + + // ErrDisableClient indicates error in disabling client. + ErrDisableClient = errors.New("failed to disable client") + + // ErrAddPolicies indicates error in adding policies. + ErrAddPolicies = errors.New("failed to add policies") + + // ErrDeletePolicies indicates error in removing policies. + ErrDeletePolicies = errors.New("failed to remove policies") ) diff --git a/pkg/sdk/go/certs_test.go b/pkg/sdk/go/certs_test.go index 9bafeddf64..d3a7c8ca45 100644 --- a/pkg/sdk/go/certs_test.go +++ b/pkg/sdk/go/certs_test.go @@ -175,6 +175,8 @@ func TestViewCert(t *testing.T) { certID: validID, token: token, cRes: c, + err: nil, + svcerr: nil, }, { desc: "get non-existent cert", @@ -291,7 +293,7 @@ func TestRevokeCert(t *testing.T) { thingID: thingID, token: authmocks.InvalidValue, svcResponse: certs.Revoke{RevocationTime: time.Now()}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), svcerr: errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), }, { @@ -299,7 +301,7 @@ func TestRevokeCert(t *testing.T) { thingID: "2", token: token, svcResponse: certs.Revoke{RevocationTime: time.Now()}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(certs.ErrFailedCertRevocation, svcerr.ErrNotFound), http.StatusNotFound), + err: errors.NewSDKErrorWithStatus(certs.ErrFailedCertRevocation, http.StatusNotFound), svcerr: errors.Wrap(certs.ErrFailedCertRevocation, svcerr.ErrNotFound), }, { @@ -321,7 +323,7 @@ func TestRevokeCert(t *testing.T) { thingID: thingID, token: token, svcResponse: certs.Revoke{RevocationTime: time.Now()}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(certs.ErrFailedToRemoveCertFromDB, svcerr.ErrNotFound), http.StatusNotFound), + err: errors.NewSDKErrorWithStatus(certs.ErrFailedToRemoveCertFromDB, http.StatusNotFound), svcerr: errors.Wrap(certs.ErrFailedToRemoveCertFromDB, svcerr.ErrNotFound), }, } diff --git a/pkg/sdk/go/channels_test.go b/pkg/sdk/go/channels_test.go index bbdab6c754..e4ed51fdc6 100644 --- a/pkg/sdk/go/channels_test.go +++ b/pkg/sdk/go/channels_test.go @@ -110,7 +110,7 @@ func TestCreateChannel(t *testing.T) { Status: mgclients.EnabledStatus.String(), }, token: token, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrCreateEntity, svcerr.ErrCreateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "create channel with missing name", @@ -203,7 +203,7 @@ func TestListChannels(t *testing.T) { token: invalidToken, offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), response: nil, }, { @@ -324,14 +324,14 @@ func TestViewChannel(t *testing.T) { token: "wrongtoken", channelID: channel.ID, response: sdk.Channel{Children: []*sdk.Channel{}}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, { desc: "view channel for wrong id", token: validToken, channelID: wrongID, response: sdk.Channel{Children: []*sdk.Channel{}}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, } @@ -464,7 +464,7 @@ func TestUpdateChannel(t *testing.T) { }, response: sdk.Channel{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update channel description with invalid token", @@ -474,7 +474,7 @@ func TestUpdateChannel(t *testing.T) { }, response: sdk.Channel{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update channel metadata with invalid token", @@ -486,7 +486,7 @@ func TestUpdateChannel(t *testing.T) { }, response: sdk.Channel{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update channel that can't be marshalled", @@ -616,7 +616,7 @@ func TestListChannelsByThing(t *testing.T) { clientID: testsutil.GenerateUUID(t), page: sdk.PageMetadata{}, response: []sdk.Channel(nil), - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, } @@ -659,7 +659,7 @@ func TestEnableChannel(t *testing.T) { repoCall1 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound) repoCall2 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil) _, err := mgsdk.EnableChannel("wrongID", validToken) - assert.Equal(t, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), err, fmt.Sprintf("Enable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err)) + assert.Equal(t, errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), err, fmt.Sprintf("Enable channel with wrong id: expected %v got %v", svcerr.ErrViewEntity, err)) ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID") assert.True(t, ok, "RetrieveByID was not called on enabling channel") repoCall.Unset() @@ -711,7 +711,7 @@ func TestDisableChannel(t *testing.T) { repoCall1 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil) repoCall2 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound) _, err := mgsdk.DisableChannel("wrongID", validToken) - assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), fmt.Sprintf("Disable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err)) + assert.Equal(t, err, errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), fmt.Sprintf("Disable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err)) ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID") assert.True(t, ok, "Memberships was not called on disabling channel with wrong id") repoCall.Unset() diff --git a/pkg/sdk/go/groups_test.go b/pkg/sdk/go/groups_test.go index a075fbb6f7..d3dd42ed42 100644 --- a/pkg/sdk/go/groups_test.go +++ b/pkg/sdk/go/groups_test.go @@ -98,7 +98,7 @@ func TestCreateGroup(t *testing.T) { ParentID: wrongID, Status: clients.EnabledStatus.String(), }, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrCreateEntity, svcerr.ErrCreateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "create group with missing name", @@ -203,7 +203,7 @@ func TestListGroups(t *testing.T) { token: invalidToken, offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), response: nil, }, { @@ -334,7 +334,7 @@ func TestListParentGroups(t *testing.T) { token: invalidToken, offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), response: nil, }, { @@ -466,7 +466,7 @@ func TestListChildrenGroups(t *testing.T) { token: invalidToken, offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), response: nil, }, { @@ -583,14 +583,14 @@ func TestViewGroup(t *testing.T) { token: "wrongtoken", groupID: group.ID, response: sdk.Group{Children: []*sdk.Group{}}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, { desc: "view group for wrong id", token: validToken, groupID: wrongID, response: sdk.Group{Children: []*sdk.Group{}}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, } @@ -723,7 +723,7 @@ func TestUpdateGroup(t *testing.T) { }, response: sdk.Group{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update group description with invalid token", @@ -733,7 +733,7 @@ func TestUpdateGroup(t *testing.T) { }, response: sdk.Group{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update group metadata with invalid token", @@ -745,7 +745,7 @@ func TestUpdateGroup(t *testing.T) { }, response: sdk.Group{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update a group that can't be marshalled", @@ -798,7 +798,7 @@ func TestEnableGroup(t *testing.T) { repoCall1 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound) repoCall2 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil) _, err := mgsdk.EnableGroup("wrongID", validToken) - assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), fmt.Sprintf("Enable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err)) + assert.Equal(t, err, errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), fmt.Sprintf("Enable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err)) ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID") assert.True(t, ok, "RetrieveByID was not called on enabling group") repoCall.Unset() @@ -851,7 +851,7 @@ func TestDisableGroup(t *testing.T) { repoCall1 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil) repoCall2 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound) _, err := mgsdk.DisableGroup("wrongID", validToken) - assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), fmt.Sprintf("Disable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err)) + assert.Equal(t, err, errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), fmt.Sprintf("Disable group with wrong id: expected %v got %v", svcerr.ErrViewEntity, err)) ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID") assert.True(t, ok, "Memberships was not called on disabling group with wrong id") repoCall.Unset() diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index a1cf67ad9c..8f2a37d0ad 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -18,7 +18,6 @@ import ( mglog "github.com/absmach/magistrala/logger" mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" - repoerr "github.com/absmach/magistrala/pkg/errors/repository" svcerr "github.com/absmach/magistrala/pkg/errors/service" gmocks "github.com/absmach/magistrala/pkg/groups/mocks" sdk "github.com/absmach/magistrala/pkg/sdk/go" @@ -97,7 +96,7 @@ func TestCreateThing(t *testing.T) { response: sdk.Thing{}, token: token, repoErr: sdk.ErrFailedCreation, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, repoerr.ErrCreateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "register empty thing", @@ -105,7 +104,7 @@ func TestCreateThing(t *testing.T) { response: sdk.Thing{}, token: token, repoErr: errors.ErrMalformedEntity, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, repoerr.ErrMalformedEntity), http.StatusBadRequest), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusBadRequest), }, { desc: "register a thing that can't be marshalled", @@ -246,7 +245,7 @@ func TestCreateThings(t *testing.T) { things: thingsList, response: []sdk.Thing{}, token: token, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, sdk.ErrFailedCreation), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "register empty things", @@ -361,7 +360,7 @@ func TestListThings(t *testing.T) { token: authmocks.InvalidValue, offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), response: nil, }, { @@ -369,7 +368,7 @@ func TestListThings(t *testing.T) { token: "", offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), response: nil, }, { @@ -573,7 +572,7 @@ func TestListThingsByChannel(t *testing.T) { channelID: testsutil.GenerateUUID(t), page: sdk.PageMetadata{}, response: []sdk.Thing(nil), - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "list things with an invalid id", @@ -581,7 +580,7 @@ func TestListThingsByChannel(t *testing.T) { channelID: wrongID, page: sdk.PageMetadata{}, response: []sdk.Thing(nil), - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, } @@ -639,21 +638,21 @@ func TestThing(t *testing.T) { response: sdk.Thing{}, token: invalidToken, thingID: generateUUID(t), - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "view thing with valid token and invalid thing id", response: sdk.Thing{}, token: validToken, thingID: wrongID, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, { desc: "view thing with an invalid token and invalid thing id", response: sdk.Thing{}, token: invalidToken, thingID: wrongID, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, } @@ -718,14 +717,14 @@ func TestUpdateThing(t *testing.T) { thing: thing1, response: sdk.Thing{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update thing name with invalid id", thing: thing2, response: sdk.Thing{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, { desc: "update thing that can't be marshalled", @@ -803,14 +802,14 @@ func TestUpdateThingTags(t *testing.T) { thing: thing1, response: sdk.Thing{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), }, { desc: "update thing name with invalid id", thing: thing2, response: sdk.Thing{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, { desc: "update thing that can't be marshalled", @@ -884,7 +883,7 @@ func TestUpdateThingSecret(t *testing.T) { token: "non-existent", response: sdk.Thing{}, repoErr: svcerr.ErrAuthorization, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusForbidden), }, { desc: "update thing secret with wrong old secret", @@ -893,7 +892,7 @@ func TestUpdateThingSecret(t *testing.T) { token: validToken, response: sdk.Thing{}, repoErr: apiutil.ErrMissingSecret, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, apiutil.ErrMissingSecret), http.StatusBadRequest), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusBadRequest), }, } for _, tc := range cases { @@ -956,7 +955,7 @@ func TestEnableThing(t *testing.T) { thing: enabledThing1, response: sdk.Thing{}, repoErr: sdk.ErrFailedEnable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedEnable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrEnableClient, http.StatusBadRequest), }, { desc: "enable non-existing thing", @@ -965,7 +964,7 @@ func TestEnableThing(t *testing.T) { thing: sdk.Thing{}, response: sdk.Thing{}, repoErr: sdk.ErrFailedEnable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedEnable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrEnableClient, http.StatusBadRequest), }, } @@ -1091,7 +1090,7 @@ func TestDisableThing(t *testing.T) { thing: disabledThing1, response: sdk.Thing{}, repoErr: sdk.ErrFailedDisable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedDisable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrDisableClient, http.StatusBadRequest), }, { desc: "disable non-existing thing", @@ -1100,7 +1099,7 @@ func TestDisableThing(t *testing.T) { token: validToken, response: sdk.Thing{}, repoErr: sdk.ErrFailedDisable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedDisable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrDisableClient, http.StatusBadRequest), }, } @@ -1218,14 +1217,14 @@ func TestShareThing(t *testing.T) { channelID: generateUUID(t), thingID: "thingID", token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "share thing with valid token for unauthorized user", channelID: generateUUID(t), thingID: "thingID", token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), repoErr: svcerr.ErrAuthorization, }, } diff --git a/pkg/sdk/go/tokens_test.go b/pkg/sdk/go/tokens_test.go index be448f08c6..edb9bf5544 100644 --- a/pkg/sdk/go/tokens_test.go +++ b/pkg/sdk/go/tokens_test.go @@ -69,7 +69,7 @@ func TestIssueToken(t *testing.T) { login: sdk.Login{Identity: "invalid", Secret: "secret"}, token: &magistrala.Token{}, dbClient: wrongClient, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, } for _, tc := range cases { diff --git a/pkg/sdk/go/users_test.go b/pkg/sdk/go/users_test.go index 0012de581e..6d4523d24d 100644 --- a/pkg/sdk/go/users_test.go +++ b/pkg/sdk/go/users_test.go @@ -18,7 +18,6 @@ import ( mglog "github.com/absmach/magistrala/logger" mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" - repoerr "github.com/absmach/magistrala/pkg/errors/repository" svcerr "github.com/absmach/magistrala/pkg/errors/service" gmocks "github.com/absmach/magistrala/pkg/groups/mocks" oauth2mocks "github.com/absmach/magistrala/pkg/oauth2/mocks" @@ -89,7 +88,7 @@ func TestCreateClient(t *testing.T) { client: user, response: sdk.User{}, token: token, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, sdk.ErrFailedCreation), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "register empty user", @@ -257,7 +256,7 @@ func TestListClients(t *testing.T) { token: invalidToken, offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), response: nil, }, { @@ -265,7 +264,7 @@ func TestListClients(t *testing.T) { token: "", offset: offset, limit: limit, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), response: nil, }, { @@ -273,7 +272,7 @@ func TestListClients(t *testing.T) { token: token, offset: offset, limit: 0, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), response: nil, }, { @@ -404,21 +403,21 @@ func TestClient(t *testing.T) { response: sdk.User{}, token: invalidToken, clientID: generateUUID(t), - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "view client with valid token and invalid client id", response: sdk.User{}, token: validToken, clientID: wrongID, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, { desc: "view client with an invalid token and invalid client id", response: sdk.User{}, token: invalidToken, clientID: wrongID, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, } @@ -477,7 +476,7 @@ func TestProfile(t *testing.T) { desc: "view client with an invalid token", response: sdk.User{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, } @@ -544,14 +543,14 @@ func TestUpdateClient(t *testing.T) { client: client1, response: sdk.User{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "update client name with invalid id", client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -635,14 +634,14 @@ func TestUpdateClientTags(t *testing.T) { client: client1, response: sdk.User{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "update client name with invalid id", client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -725,14 +724,14 @@ func TestUpdateClientIdentity(t *testing.T) { client: user, response: sdk.User{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "update client name with invalid id", client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -748,7 +747,7 @@ func TestUpdateClientIdentity(t *testing.T) { }, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, } @@ -812,7 +811,7 @@ func TestUpdateClientSecret(t *testing.T) { token: "non-existent", response: sdk.User{}, repoErr: svcerr.ErrAuthentication, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "update client secret with wrong old secret", @@ -821,7 +820,7 @@ func TestUpdateClientSecret(t *testing.T) { token: validToken, response: sdk.User{}, repoErr: apiutil.ErrMissingSecret, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, repoerr.ErrMissingSecret), http.StatusBadRequest), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, } @@ -893,14 +892,14 @@ func TestUpdateClientRole(t *testing.T) { client: client2, response: sdk.User{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, { desc: "update client name with invalid id", client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrFailedUpdateRole, svcerr.ErrFailedUpdateRole), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrUpdateEntity, http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -985,7 +984,7 @@ func TestEnableClient(t *testing.T) { client: enabledClient1, response: sdk.User{}, repoErr: sdk.ErrFailedEnable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedEnable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrEnableClient, http.StatusBadRequest), }, { desc: "enable non-existing client", @@ -994,7 +993,7 @@ func TestEnableClient(t *testing.T) { client: sdk.User{}, response: sdk.User{}, repoErr: sdk.ErrFailedEnable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedEnable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrEnableClient, http.StatusBadRequest), }, } @@ -1114,7 +1113,7 @@ func TestDisableClient(t *testing.T) { client: disabledClient1, response: sdk.User{}, repoErr: sdk.ErrFailedDisable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedDisable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, { desc: "disable non-existing client", @@ -1123,7 +1122,7 @@ func TestDisableClient(t *testing.T) { token: validToken, response: sdk.User{}, repoErr: sdk.ErrFailedDisable, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedDisable, svcerr.ErrViewEntity), http.StatusUnprocessableEntity), + err: errors.NewSDKErrorWithStatus(svcerr.ErrViewEntity, http.StatusBadRequest), }, } diff --git a/things/service.go b/things/service.go index 31845407b9..57b53f5dad 100644 --- a/things/service.go +++ b/things/service.go @@ -10,18 +10,12 @@ import ( "github.com/absmach/magistrala/auth" mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" - repoerr "github.com/absmach/magistrala/pkg/errors/repository" svcerr "github.com/absmach/magistrala/pkg/errors/service" mggroups "github.com/absmach/magistrala/pkg/groups" "github.com/absmach/magistrala/things/postgres" "golang.org/x/sync/errgroup" ) -var ( - errAddPolicies = errors.New("failed to add policies") - errRemovePolicies = errors.New("failed to remove the policies") -) - type service struct { auth magistrala.AuthServiceClient clients postgres.Repository @@ -101,7 +95,7 @@ func (svc service) CreateThings(ctx context.Context, token string, cls ...mgclie saved, err := svc.clients.Save(ctx, clients...) if err != nil { - return nil, errors.Wrap(repoerr.ErrCreateEntity, err) + return nil, errors.Wrap(svcerr.ErrCreateEntity, err) } policies := magistrala.AddPoliciesReq{} @@ -125,7 +119,7 @@ func (svc service) CreateThings(ctx context.Context, token string, cls ...mgclie }) } if _, err := svc.auth.AddPolicies(ctx, &policies); err != nil { - return nil, errors.Wrap(errAddPolicies, err) + return nil, errors.Wrap(svcerr.ErrCreateEntity, err) } return saved, nil @@ -134,7 +128,7 @@ func (svc service) CreateThings(ctx context.Context, token string, cls ...mgclie func (svc service) ViewClient(ctx context.Context, token, id string) (mgclients.Client, error) { _, err := svc.authorize(ctx, "", auth.UserType, auth.TokenKind, token, auth.ViewPermission, auth.ThingType, id) if err != nil { - return mgclients.Client{}, errors.Wrap(svcerr.ErrAuthorization, err) + return mgclients.Client{}, err } client, err := svc.clients.RetrieveByID(ctx, id) if err != nil { @@ -175,11 +169,11 @@ func (svc service) ListClients(ctx context.Context, token, reqUserID string, pm } rtids, err := svc.listClientIDs(ctx, auth.EncodeDomainUserID(res.GetDomainId(), reqUserID), pm.Permission) if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrNotFound, err) + return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrNotFound, err) } ids, err = svc.filterAllowedThingIDs(ctx, res.GetId(), pm.Permission, rtids) if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrNotFound, err) + return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrNotFound, err) } default: err := svc.checkSuperAdmin(ctx, res.GetUserId()) @@ -193,7 +187,7 @@ func (svc service) ListClients(ctx context.Context, token, reqUserID string, pm } ids, err = svc.listClientIDs(ctx, res.GetId(), pm.Permission) if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrNotFound, err) + return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrNotFound, err) } } } @@ -254,7 +248,7 @@ func (svc service) listClientIDs(ctx context.Context, userID, permission string) ObjectType: auth.ThingType, }) if err != nil { - return nil, errors.Wrap(repoerr.ErrNotFound, err) + return nil, errors.Wrap(svcerr.ErrNotFound, err) } return tids.Policies, nil } @@ -268,7 +262,7 @@ func (svc service) filterAllowedThingIDs(ctx context.Context, userID, permission ObjectType: auth.ThingType, }) if err != nil { - return nil, errors.Wrap(repoerr.ErrNotFound, err) + return nil, errors.Wrap(svcerr.ErrNotFound, err) } for _, thingID := range thingIDs { for _, tid := range tids.Policies { @@ -384,7 +378,7 @@ func (svc service) DisableClient(ctx context.Context, token, id string) (mgclien } if err := svc.clientCache.Remove(ctx, client.ID); err != nil { - return client, errors.Wrap(repoerr.ErrRemoveEntity, err) + return client, errors.Wrap(svcerr.ErrRemoveEntity, err) } return client, nil @@ -411,10 +405,10 @@ func (svc service) Share(ctx context.Context, token, id, relation string, userid } res, err := svc.auth.AddPolicies(ctx, &policies) if err != nil { - return errors.Wrap(errAddPolicies, err) + return errors.Wrap(svcerr.ErrUpdateEntity, err) } if !res.Added { - return err + return errors.Wrap(svcerr.ErrUpdateEntity, err) } return nil } @@ -440,7 +434,7 @@ func (svc service) Unshare(ctx context.Context, token, id, relation string, user } res, err := svc.auth.DeletePolicies(ctx, &policies) if err != nil { - return errors.Wrap(errRemovePolicies, err) + return errors.Wrap(svcerr.ErrUpdateEntity, err) } if !res.Deleted { return err @@ -459,7 +453,7 @@ func (svc service) DeleteClient(ctx context.Context, token, id string) error { // Remove from cache if err := svc.clientCache.Remove(ctx, id); err != nil { - return errors.Wrap(repoerr.ErrRemoveEntity, err) + return errors.Wrap(svcerr.ErrRemoveEntity, err) } // Remove policy of groups @@ -468,7 +462,7 @@ func (svc service) DeleteClient(ctx context.Context, token, id string) error { Object: id, ObjectType: auth.ThingType, }); err != nil { - return err + return errors.Wrap(svcerr.ErrRemoveEntity, err) } // Remove policy from domain @@ -477,12 +471,12 @@ func (svc service) DeleteClient(ctx context.Context, token, id string) error { Object: id, ObjectType: auth.ThingType, }); err != nil { - return err + return errors.Wrap(svcerr.ErrRemoveEntity, err) } // Remove thing from database if err := svc.clients.Delete(ctx, id); err != nil { - return err + return errors.Wrap(svcerr.ErrRemoveEntity, err) } // Remove policy of users @@ -491,7 +485,7 @@ func (svc service) DeleteClient(ctx context.Context, token, id string) error { Object: id, ObjectType: auth.ThingType, }); err != nil { - return err + return errors.Wrap(svcerr.ErrRemoveEntity, err) } return nil @@ -504,7 +498,7 @@ func (svc service) changeClientStatus(ctx context.Context, token string, client } dbClient, err := svc.clients.RetrieveByID(ctx, client.ID) if err != nil { - return mgclients.Client{}, errors.Wrap(repoerr.ErrViewEntity, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrViewEntity, err) } if dbClient.Status == client.Status { return mgclients.Client{}, errors.ErrStatusAlreadyAssigned @@ -535,7 +529,7 @@ func (svc service) ListClientsByGroup(ctx context.Context, token, groupID string ObjectType: auth.ThingType, }) if err != nil { - return mgclients.MembersPage{}, errors.Wrap(repoerr.ErrNotFound, err) + return mgclients.MembersPage{}, errors.Wrap(svcerr.ErrNotFound, err) } pm.IDs = tids.Policies diff --git a/things/service_test.go b/things/service_test.go index 9dec8d5807..1f6b824dda 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -43,8 +43,7 @@ var ( invalid = "invalid" validID = "d4ebb847-5d0e-4e46-bdd9-b6aceaaa3a22" wrongID = testsutil.GenerateUUID(&testing.T{}) - errAddPolicies = errors.New("failed to add policies") - errRemovePolicies = errors.New("failed to remove the policies") + errRemovePolicies = errors.New("failed to delete policies") ) func newService() (things.Service, *mocks.Repository, *authmocks.AuthClient, *mocks.Cache) { @@ -1774,7 +1773,7 @@ func TestShare(t *testing.T) { authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, addPoliciesResponse: &magistrala.AddPoliciesRes{}, addPoliciesErr: svcerr.ErrInvalidPolicy, - err: errAddPolicies, + err: svcerr.ErrInvalidPolicy, }, { desc: "share thing with failed authorization from add policies", @@ -1783,7 +1782,7 @@ func TestShare(t *testing.T) { identifyResponse: &magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, addPoliciesResponse: &magistrala.AddPoliciesRes{Added: false}, - err: nil, + err: svcerr.ErrUpdateEntity, }, } @@ -1852,7 +1851,7 @@ func TestUnShare(t *testing.T) { authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, deletePoliciesResponse: &magistrala.DeletePoliciesRes{}, deletePoliciesErr: svcerr.ErrInvalidPolicy, - err: errRemovePolicies, + err: svcerr.ErrInvalidPolicy, }, { desc: "share thing with failed delete from delete policies", diff --git a/twins/api/http/endpoint_twins_test.go b/twins/api/http/endpoint_twins_test.go index ea79f3d8cf..e6b721ead9 100644 --- a/twins/api/http/endpoint_twins_test.go +++ b/twins/api/http/endpoint_twins_test.go @@ -469,7 +469,7 @@ func TestViewTwin(t *testing.T) { desc: "view twin by passing invalid token", id: twin.ID, auth: authmocks.InvalidValue, - status: http.StatusUnauthorized, + status: http.StatusForbidden, res: twinRes{}, err: svcerr.ErrAuthentication, identifyErr: svcerr.ErrAuthentication, diff --git a/users/api/endpoint_test.go b/users/api/endpoint_test.go index 5e0d2a54bb..fb7d98164a 100644 --- a/users/api/endpoint_test.go +++ b/users/api/endpoint_test.go @@ -46,6 +46,7 @@ var ( inValid = "invalid" validID = "d4ebb847-5d0e-4e46-bdd9-b6aceaaa3a22" passRegex = regexp.MustCompile("^.{8,}$") + testReferer = "http://localhost" ) const contentType = "application/json" @@ -55,6 +56,7 @@ type testRequest struct { method string url string contentType string + referer string token string body io.Reader } @@ -73,7 +75,7 @@ func (tr testRequest) make() (*http.Response, error) { req.Header.Set("Content-Type", tr.contentType) } - req.Header.Set("Referer", "http://localhost") + req.Header.Set("Referer", tr.referer) return tr.client.Do(req) } @@ -1013,6 +1015,7 @@ func TestPasswordResetRequest(t *testing.T) { desc string data string contentType string + referer string status int err error }{ @@ -1020,6 +1023,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset request with valid email", data: fmt.Sprintf(`{"email": "%s", "host": "%s"}`, testemail, testhost), contentType: contentType, + referer: testReferer, status: http.StatusCreated, err: nil, }, @@ -1027,6 +1031,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset request with empty email", data: fmt.Sprintf(`{"email": "%s", "host": "%s"}`, "", testhost), contentType: contentType, + referer: testReferer, status: http.StatusBadRequest, err: apiutil.ErrValidation, }, @@ -1034,6 +1039,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset request with empty host", data: fmt.Sprintf(`{"email": "%s", "host": "%s"}`, testemail, ""), contentType: contentType, + referer: "", status: http.StatusBadRequest, err: apiutil.ErrValidation, }, @@ -1041,6 +1047,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset request with invalid email", data: fmt.Sprintf(`{"email": "%s", "host": "%s"}`, "invalid", testhost), contentType: contentType, + referer: testReferer, status: http.StatusNotFound, err: svcerr.ErrNotFound, }, @@ -1048,6 +1055,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset with malformed data", data: fmt.Sprintf(`{"email": %s, "host": %s}`, testemail, testhost), contentType: contentType, + referer: testReferer, status: http.StatusBadRequest, err: apiutil.ErrValidation, }, @@ -1055,6 +1063,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset with invalid contentype", data: fmt.Sprintf(`{"email": "%s", "host": "%s"}`, testemail, testhost), contentType: "application/xml", + referer: testReferer, status: http.StatusUnsupportedMediaType, err: apiutil.ErrValidation, }, @@ -1066,9 +1075,9 @@ func TestPasswordResetRequest(t *testing.T) { method: http.MethodPost, url: fmt.Sprintf("%s/password/reset-request", us.URL), contentType: tc.contentType, + referer: tc.referer, body: strings.NewReader(tc.data), } - svcCall := svc.On("GenerateResetToken", mock.Anything, mock.Anything, mock.Anything).Return(tc.err) res, err := req.make() assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) @@ -1154,10 +1163,10 @@ func TestPasswordReset(t *testing.T) { method: http.MethodPut, url: fmt.Sprintf("%s/password/reset", us.URL), contentType: tc.contentType, + referer: testReferer, token: tc.token, body: strings.NewReader(tc.data), } - svcCall := svc.On("ResetSecret", mock.Anything, mock.Anything, mock.Anything).Return(tc.err) res, err := req.make() assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) diff --git a/users/service.go b/users/service.go index 0b2c394121..7a3215d042 100644 --- a/users/service.go +++ b/users/service.go @@ -19,6 +19,13 @@ import ( "golang.org/x/sync/errgroup" ) +var ( + errIssueToken = errors.New("failed to issue token") + errUserNotSignedUp = errors.New("user not signed up") + errFailedPermissionsList = errors.New("failed to list permissions") + errRecoveryToken = errors.New("failed to generate password recovery token") +) + type service struct { clients postgres.Repository idProvider magistrala.IDProvider @@ -59,7 +66,7 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mgclien if cli.Credentials.Secret != "" { hash, err := svc.hasher.Hash(cli.Credentials.Secret) if err != nil { - return mgclients.Client{}, errors.Wrap(repoerr.ErrMalformedEntity, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrMalformedEntity, err) } cli.Credentials.Secret = hash } @@ -85,7 +92,7 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mgclien }() client, err := svc.clients.Save(ctx, cli) if err != nil { - return mgclients.Client{}, errors.Wrap(repoerr.ErrCreateEntity, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrCreateEntity, err) } return client, nil } @@ -93,7 +100,7 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mgclien func (svc service) IssueToken(ctx context.Context, identity, secret, domainID string) (*magistrala.Token, error) { dbUser, err := svc.clients.RetrieveByIdentity(ctx, identity) if err != nil { - return &magistrala.Token{}, errors.Wrap(svcerr.ErrViewEntity, err) + return &magistrala.Token{}, errors.Wrap(svcerr.ErrAuthentication, err) } if err := svc.hasher.Compare(secret, dbUser.Credentials.Secret); err != nil { return &magistrala.Token{}, errors.Wrap(svcerr.ErrLogin, err) @@ -103,7 +110,13 @@ func (svc service) IssueToken(ctx context.Context, identity, secret, domainID st if domainID != "" { d = domainID } - return svc.auth.Issue(ctx, &magistrala.IssueReq{UserId: dbUser.ID, DomainId: &d, Type: uint32(auth.AccessKey)}) + + token, err := svc.auth.Issue(ctx, &magistrala.IssueReq{UserId: dbUser.ID, DomainId: &d, Type: uint32(auth.AccessKey)}) + if err != nil { + return &magistrala.Token{}, errors.Wrap(errIssueToken, err) + } + + return token, err } func (svc service) RefreshToken(ctx context.Context, refreshToken, domainID string) (*magistrala.Token, error) { @@ -259,8 +272,8 @@ func (svc service) UpdateClientIdentity(ctx context.Context, token, clientID, id func (svc service) GenerateResetToken(ctx context.Context, email, host string) error { client, err := svc.clients.RetrieveByIdentity(ctx, email) - if err != nil || client.Credentials.Identity == "" { - return repoerr.ErrNotFound + if err != nil { + return errors.Wrap(svcerr.ErrViewEntity, err) } issueReq := &magistrala.IssueReq{ UserId: client.ID, @@ -268,7 +281,7 @@ func (svc service) GenerateResetToken(ctx context.Context, email, host string) e } token, err := svc.auth.Issue(ctx, issueReq) if err != nil { - return errors.Wrap(svcerr.ErrRecoveryToken, err) + return errors.Wrap(errRecoveryToken, err) } return svc.SendPasswordReset(ctx, host, email, client.Name, token.AccessToken) @@ -283,12 +296,10 @@ func (svc service) ResetSecret(ctx context.Context, resetToken, secret string) e if err != nil { return errors.Wrap(svcerr.ErrViewEntity, err) } - if c.Credentials.Identity == "" { - return repoerr.ErrNotFound - } + secret, err = svc.hasher.Hash(secret) if err != nil { - return err + return errors.Wrap(svcerr.ErrMalformedEntity, err) } c = mgclients.Client{ ID: c.ID, @@ -315,11 +326,11 @@ func (svc service) UpdateClientSecret(ctx context.Context, token, oldSecret, new return mgclients.Client{}, errors.Wrap(svcerr.ErrViewEntity, err) } if _, err := svc.IssueToken(ctx, dbClient.Credentials.Identity, oldSecret, ""); err != nil { - return mgclients.Client{}, errors.Wrap(svcerr.ErrIssueToken, err) + return mgclients.Client{}, err } newSecret, err = svc.hasher.Hash(newSecret) if err != nil { - return mgclients.Client{}, errors.Wrap(repoerr.ErrMalformedEntity, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrMalformedEntity, err) } dbClient.Credentials.Secret = newSecret dbClient.UpdatedAt = time.Now() @@ -355,15 +366,15 @@ func (svc service) UpdateClientRole(ctx context.Context, token string, cli mgcli } if err := svc.updateClientPolicy(ctx, cli.ID, cli.Role); err != nil { - return mgclients.Client{}, errors.Wrap(svcerr.ErrFailedPolicyUpdate, err) + return mgclients.Client{}, err } client, err = svc.clients.UpdateRole(ctx, client) if err != nil { // If failed to update role in DB, then revert back to platform admin policy in spicedb if errRollback := svc.updateClientPolicy(ctx, cli.ID, mgclients.UserRole); errRollback != nil { - return mgclients.Client{}, errors.Wrap(err, errors.Wrap(repoerr.ErrRollbackTx, errRollback)) + return mgclients.Client{}, errors.Wrap(errRollback, err) } - return mgclients.Client{}, errors.Wrap(svcerr.ErrFailedUpdateRole, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err) } return client, nil } @@ -390,7 +401,7 @@ func (svc service) DisableClient(ctx context.Context, token, id string) (mgclien } client, err := svc.changeClientStatus(ctx, token, client) if err != nil { - return mgclients.Client{}, errors.Wrap(mgclients.ErrDisableClient, err) + return mgclients.Client{}, err } return client, nil @@ -417,7 +428,7 @@ func (svc service) changeClientStatus(ctx context.Context, token string, client if err != nil { return mgclients.Client{}, errors.Wrap(svcerr.ErrUpdateEntity, err) } - return client, err + return client, nil } func (svc service) ListMembers(ctx context.Context, token, objectKind, objectID string, pm mgclients.Page) (mgclients.MembersPage, error) { @@ -498,7 +509,7 @@ func (svc service) retrieveObjectUsersPermissions(ctx context.Context, domainID, userID := auth.EncodeDomainUserID(domainID, client.ID) permissions, err := svc.listObjectUserPermission(ctx, userID, objectType, objectID) if err != nil { - return err + return errors.Wrap(svcerr.ErrAuthorization, err) } client.Permissions = permissions return nil @@ -512,7 +523,7 @@ func (svc service) listObjectUserPermission(ctx context.Context, userID, objectT ObjectType: objectType, }) if err != nil { - return []string{}, err + return []string{}, errors.Wrap(errFailedPermissionsList, err) } return lp.GetPermissions(), nil } @@ -562,9 +573,9 @@ func (svc service) OAuthCallback(ctx context.Context, state mgoauth2.State, clie rclient, err := svc.clients.RetrieveByIdentity(ctx, client.Credentials.Identity) if err != nil { if errors.Contains(err, repoerr.ErrNotFound) { - return &magistrala.Token{}, errors.New("user not signed up") + return &magistrala.Token{}, errors.Wrap(svcerr.ErrNotFound, errUserNotSignedUp) } - return &magistrala.Token{}, err + return &magistrala.Token{}, errors.Wrap(svcerr.ErrViewEntity, err) } claims := &magistrala.IssueReq{ UserId: rclient.ID, @@ -575,7 +586,7 @@ func (svc service) OAuthCallback(ctx context.Context, state mgoauth2.State, clie rclient, err := svc.RegisterClient(ctx, "", client) if err != nil { if errors.Contains(err, repoerr.ErrConflict) { - return &magistrala.Token{}, errors.New("user already exists") + return &magistrala.Token{}, errors.Wrap(svcerr.ErrConflict, errors.New("user already exists")) } return &magistrala.Token{}, err } diff --git a/users/service_test.go b/users/service_test.go index 5b0a82f757..d06d238a81 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -1053,7 +1053,7 @@ func TestUpdateClientRole(t *testing.T) { deletePolicyResponse: &magistrala.DeletePolicyRes{Deleted: false}, updateRoleResponse: mgclients.Client{}, token: validToken, - err: svcerr.ErrFailedPolicyUpdate, + err: svcerr.ErrAuthorization, }, { desc: "update client role to user role with failed to delete policy with error", @@ -2349,7 +2349,7 @@ func TestResetSecret(t *testing.T) { Identity: "", }, }, - err: repoerr.ErrNotFound, + err: nil, }, { desc: "reset secret with failed to update secret",