From b6f497a953da089a6f6fffe7ccd04523bcc6010f Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Wed, 20 Mar 2024 11:31:50 +0300 Subject: [PATCH 1/8] feat: return only service errors Signed-off-by: WashingtonKK Remove repoerr from things service Signed-off-by: WashingtonKK Remove repoerr in services Signed-off-by: WashingtonKK Fix ci Signed-off-by: WashingtonKK Add servive error check Signed-off-by: WashingtonKK Wrap permission list error Signed-off-by: WashingtonKK Wrap permission list error Signed-off-by: WashingtonKK Modify tests and keep internal errors Signed-off-by: WashingtonKK Fix tests Signed-off-by: WashingtonKK fix: enhance error encoding Signed-off-by: WashingtonKK fix: update api docs Signed-off-by: WashingtonKK feat: update checkdomain and checkpolicy Signed-off-by: WashingtonKK fix: errors on service layer Signed-off-by: WashingtonKK fix: tests Signed-off-by: WashingtonKK fix: tests Signed-off-by: WashingtonKK update tests Signed-off-by: WashingtonKK fix: property based tests Signed-off-by: WashingtonKK Remove repo error from service layer Signed-off-by: WashingtonKK fix test' Signed-off-by: WashingtonKK refactor errors: Signed-off-by: WashingtonKK fix: property based tests Signed-off-by: WashingtonKK fix: ci Signed-off-by: WashingtonKK fix ci Signed-off-by: WashingtonKK enhance errors Signed-off-by: WashingtonKK fix: tests Signed-off-by: WashingtonKK remove unused error Signed-off-by: WashingtonKK update bootstrap and certs tests Signed-off-by: WashingtonKK fix ci Signed-off-by: WashingtonKK Update docs Signed-off-by: WashingtonKK update bootstrap test Signed-off-by: WashingtonKK fix auth tests Signed-off-by: WashingtonKK fix typo: Signed-off-by: WashingtonKK fix ci Signed-off-by: WashingtonKK --- api/openapi/auth.yml | 2 + api/openapi/bootstrap.yml | 2 + api/openapi/users.yml | 2 + auth/api/http/domains/endpoint_test.go | 12 +- auth/api/http/keys/endpoint_test.go | 2 +- auth/service.go | 35 ++-- auth/service_test.go | 278 ++++++++++++++++++++++--- bootstrap/api/endpoint_test.go | 8 +- bootstrap/service.go | 2 +- certs/service.go | 5 +- internal/api/common.go | 90 ++++---- internal/api/common_test.go | 2 +- internal/groups/service.go | 7 +- invitations/api/endpoint_test.go | 1 - pkg/errors/service/types.go | 25 ++- pkg/sdk/go/certs_test.go | 21 +- 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 | 68 +++--- users/service_test.go | 8 +- 27 files changed, 526 insertions(+), 250 deletions(-) diff --git a/api/openapi/auth.yml b/api/openapi/auth.yml index 90d9c8c96c..42dba996a6 100644 --- a/api/openapi/auth.yml +++ b/api/openapi/auth.yml @@ -152,6 +152,8 @@ paths: responses: "200": $ref: "#/components/responses/DomainPermissionRes" + "400": + description: Malformed entity specification. "401": description: Missing or invalid access token provided. "404": 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..cd182df185 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": 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..1e31977557 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)) @@ -186,14 +186,7 @@ func (svc service) Authorize(ctx context.Context, pr PolicyReq) error { func (svc service) checkPolicy(ctx context.Context, pr PolicyReq) error { // Domain status is required for if user sent authorization request on things, channels, groups and domains if pr.SubjectType == UserType && (pr.ObjectType == GroupType || pr.ObjectType == ThingType || pr.ObjectType == DomainType) { - domainID := pr.Domain - if domainID == "" { - if pr.ObjectType != DomainType { - return svcerr.ErrDomainAuthorization - } - domainID = pr.Object - } - if err := svc.checkDomain(ctx, pr.SubjectType, pr.Subject, domainID); err != nil { + if err := svc.checkDomain(ctx, pr.SubjectType, pr.Subject, pr.Domain); err != nil { return err } } @@ -204,6 +197,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 +534,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 +583,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 +594,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 +612,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 +622,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 +645,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 +768,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..a0281045eb 100644 --- a/auth/service_test.go +++ b/auth/service_test.go @@ -141,6 +141,7 @@ func TestIssue(t *testing.T) { saveErr error checkPolicyRequest auth.PolicyReq checkPolicyRequest1 auth.PolicyReq + checkPolicyRequest2 auth.PolicyReq checkPolicyErr error checkPolicyErr1 error retreiveByIDErr error @@ -161,6 +162,11 @@ func TestIssue(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyRequest1: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: accessToken, err: nil, }, @@ -180,6 +186,11 @@ func TestIssue(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyRequest1: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: accessToken, err: nil, }, @@ -200,6 +211,11 @@ func TestIssue(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyRequest1: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr: repoerr.ErrNotFound, retrieveByIDResponse: auth.Domain{}, retreiveByIDErr: repoerr.ErrNotFound, @@ -231,9 +247,14 @@ func TestIssue(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, + checkPolicyRequest2: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr: svcerr.ErrAuthorization, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, - err: nil, + err: svcerr.ErrAuthorization, }, { desc: "issue login key with membership permission", @@ -261,10 +282,15 @@ func TestIssue(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, + checkPolicyRequest2: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr: svcerr.ErrAuthorization, checkPolicyErr1: nil, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, - err: nil, + err: svcerr.ErrAuthorization, }, { desc: "issue login key with membership permission with failed to authorize", @@ -292,6 +318,11 @@ func TestIssue(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, + checkPolicyRequest2: auth.PolicyReq{ + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr: svcerr.ErrAuthorization, checkPolicyErr1: svcerr.ErrAuthorization, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, @@ -303,12 +334,14 @@ func TestIssue(t *testing.T) { repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest).Return(tc.checkPolicyErr) repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest1).Return(tc.checkPolicyErr1) repoCall3 := drepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(tc.retrieveByIDResponse, tc.retreiveByIDErr) + repoCall4 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest2).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 +397,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 + checkPolicyRequest1 auth.PolicyReq + checkPolicyErr error + retrieveByIDErr error + err error }{ { desc: "issue refresh key", @@ -405,6 +439,14 @@ func TestIssue(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyRequest1: auth.PolicyReq{ + Subject: "mgx_test@example.com", + SubjectType: auth.UserType, + SubjectKind: "", + ObjectKind: "", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: refreshToken, checkPolicyErr: svcerr.ErrAuthorization, retrieveByIDErr: repoerr.ErrNotFound, @@ -416,6 +458,14 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, + checkPolicyRequest1: auth.PolicyReq{ + Subject: "mgx_test@example.com", + SubjectType: auth.UserType, + SubjectKind: "", + ObjectKind: "", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: accessToken, err: errIssueUser, }, @@ -425,6 +475,14 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, + checkPolicyRequest1: auth.PolicyReq{ + Subject: "mgx_test@example.com", + SubjectType: auth.UserType, + SubjectKind: "", + ObjectKind: "", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, token: "", err: errRetrieve, }, @@ -462,19 +520,26 @@ func TestIssue(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyRequest1: auth.PolicyReq{ + SubjectType: auth.UserType, + 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.checkPolicyRequest1).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() } } @@ -763,6 +828,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: nil, }, { @@ -783,7 +854,14 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.GroupType, Permission: auth.AdminPermission, }, - err: svcerr.ErrDomainAuthorization, + checkPolicyReq1: 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", @@ -798,18 +876,23 @@ func TestAuthorize(t *testing.T) { checkPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, - SubjectKind: auth.TokenKind, - Object: validID, ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, checkPolicyReq1: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + SubjectKind: auth.TokenKind, Permission: auth.AdminPermission, Object: validID, ObjectType: auth.DomainType, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -831,18 +914,23 @@ func TestAuthorize(t *testing.T) { checkPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, - SubjectKind: auth.TokenKind, - Object: validID, ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, checkPolicyReq1: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + SubjectKind: auth.TokenKind, Permission: auth.AdminPermission, Object: validID, ObjectType: auth.DomainType, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -877,6 +965,12 @@ func TestAuthorize(t *testing.T) { Object: auth.MagistralaObject, ObjectType: auth.PlatformType, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -910,6 +1004,12 @@ func TestAuthorize(t *testing.T) { Object: auth.MagistralaObject, ObjectType: auth.PlatformType, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -944,6 +1044,12 @@ func TestAuthorize(t *testing.T) { Object: auth.MagistralaObject, ObjectType: auth.PlatformType, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, retrieveDomainRes: auth.Domain{ ID: validID, @@ -971,6 +1077,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrAuthentication, }, { @@ -989,6 +1101,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrAuthentication, }, { @@ -1007,6 +1125,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformKind, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrDomainAuthorization, }, { @@ -1026,6 +1150,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: nil, }, { @@ -1044,6 +1174,12 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: svcerr.ErrDomainAuthorization, }, } @@ -1051,13 +1187,15 @@ func TestAuthorize(t *testing.T) { repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq).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) + repoCall3 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq2).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() } cases2 := []struct { @@ -1796,7 +1934,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 +2006,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 +2085,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 +2095,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", @@ -2061,6 +2199,7 @@ func TestAssignUsers(t *testing.T) { checkPolicyReq auth.PolicyReq checkPolicyReq1 auth.PolicyReq checkPolicyReq2 auth.PolicyReq + checkPolicyReq3 auth.PolicyReq checkpolicyErr error checkPolicyErr1 error checkPolicyErr2 error @@ -2104,6 +2243,13 @@ func TestAssignUsers(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq3: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, err: nil, }, @@ -2169,6 +2315,13 @@ func TestAssignUsers(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.ViewPermission, }, + checkPolicyReq3: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, checkPolicyErr1: svcerr.ErrAuthorization, err: svcerr.ErrAuthorization, }, @@ -2207,8 +2360,15 @@ func TestAssignUsers(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq3: 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", @@ -2245,6 +2405,13 @@ func TestAssignUsers(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq3: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, addPoliciesErr: svcerr.ErrAuthorization, err: errAddPolicies, }, @@ -2283,6 +2450,13 @@ func TestAssignUsers(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq3: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, savePoliciesErr: repoerr.ErrCreateEntity, err: errAddPolicies, }, @@ -2321,8 +2495,15 @@ func TestAssignUsers(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.MembershipPermission, }, + checkPolicyReq3: 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, }, } @@ -2332,9 +2513,10 @@ func TestAssignUsers(t *testing.T) { 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) + repoCall4 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3).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 +2526,7 @@ func TestAssignUsers(t *testing.T) { repoCall4.Unset() repoCall5.Unset() repoCall6.Unset() + repoCall7.Unset() } } @@ -2356,6 +2539,7 @@ func TestUnassignUsers(t *testing.T) { domainID string checkPolicyReq auth.PolicyReq checkPolicyReq1 auth.PolicyReq + checkPolicyReq2 auth.PolicyReq checkPolicyErr error checkPolicyErr1 error deletePoliciesErr error @@ -2367,6 +2551,13 @@ func TestUnassignUsers(t *testing.T) { token: accessToken, domainID: validID, checkPolicyReq: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, + checkPolicyReq1: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, @@ -2374,9 +2565,9 @@ func TestUnassignUsers(t *testing.T) { Object: validID, ObjectKind: "", ObjectType: auth.DomainType, - Permission: auth.SharePermission, + Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkPolicyReq2: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, @@ -2384,7 +2575,7 @@ func TestUnassignUsers(t *testing.T) { Object: validID, ObjectKind: "", ObjectType: auth.DomainType, - Permission: auth.AdminPermission, + Permission: auth.SharePermission, }, err: nil, }, @@ -2438,11 +2629,18 @@ func TestUnassignUsers(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, + checkPolicyReq2: 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{ @@ -2465,6 +2663,13 @@ func TestUnassignUsers(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, deletePoliciesErr: errors.ErrMalformedEntity, err: errors.ErrMalformedEntity, }, @@ -2492,6 +2697,13 @@ func TestUnassignUsers(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, + checkPolicyReq2: auth.PolicyReq{ + Subject: id, + SubjectType: auth.UserType, + Object: "mgx", + ObjectType: auth.DomainType, + Permission: auth.MembershipPermission, + }, deletePoliciesErr1: errors.ErrMalformedEntity, err: errors.ErrMalformedEntity, }, @@ -2501,8 +2713,9 @@ func TestUnassignUsers(t *testing.T) { 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) + repoCall3 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq2).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 +2723,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..cee19f88c8 100644 --- a/bootstrap/service.go +++ b/bootstrap/service.go @@ -400,7 +400,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..e324efbd06 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.ErrNotFound, err) } vcert, err := cs.pki.Read(serialID) diff --git a/internal/api/common.go b/internal/api/common.go index 17703ab8f9..de2dc950b0 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -106,66 +106,78 @@ 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.ErrMissingCertData), errors.Contains(err, apiutil.ErrInvalidCertData), errors.Contains(err, apiutil.ErrInvalidContact), errors.Contains(err, apiutil.ErrInvalidTopic), - errors.Contains(err, apiutil.ErrInvalidQueryParams): + 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 +192,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..8a5525b73e 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,23 @@ 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..700d22c3b5 100644 --- a/pkg/sdk/go/certs_test.go +++ b/pkg/sdk/go/certs_test.go @@ -230,8 +230,14 @@ func TestViewCertByThing(t *testing.T) { { desc: "get existing cert", thingID: thingID, + token: authmocks.InvalidValue, + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), + }, + { + desc: "revoke non-existing cert", + thingID: "2", token: token, - page: certs.Page{Certs: []certs.Cert{c}}, + err: errors.NewSDKErrorWithStatus(certs.ErrFailedCertRevocation, http.StatusNotFound), }, { desc: "get non-existent cert", @@ -248,7 +254,18 @@ func TestViewCertByThing(t *testing.T) { token: "", page: certs.Page{Certs: []certs.Cert{}}, err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrBearerToken), http.StatusUnauthorized), - svcerr: errors.Wrap(svcerr.ErrAuthentication, apiutil.ErrBearerToken), + }, + { + desc: "revoke existing cert", + thingID: thingID, + token: token, + err: nil, + }, + { + desc: "revoke deleted cert", + thingID: thingID, + token: token, + err: errors.NewSDKErrorWithStatus(certs.ErrFailedToRemoveCertFromDB, http.StatusNotFound), }, } for _, tc := range cases { 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..0617669df5 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.ErrViewEntity, http.StatusBadRequest), }, } 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..957262e5d9 100644 --- a/users/service.go +++ b/users/service.go @@ -19,6 +19,20 @@ import ( "golang.org/x/sync/errgroup" ) +var ( + + // ErrFailedUpdateRole indicates a failure to update user role. + ErrFailedUpdateRole = errors.New("failed to update user role") + + // ErrIssueToken indicates a failure to issue token. + ErrIssueToken = errors.New("failed to issue token") + + errUserNotSignedUp = errors.New("user not signed up") + errFailedPermissionsList = errors.New("failed to list permissions") + errAddPolicies = errors.New("failed to add policies") + errRecoveryToken = errors.New("failed to generate password recovery token") +) + type service struct { clients postgres.Repository idProvider magistrala.IDProvider @@ -59,7 +73,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 +99,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 } @@ -260,7 +274,7 @@ 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 + return svcerr.ErrNotFound } issueReq := &magistrala.IssueReq{ UserId: client.ID, @@ -268,7 +282,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) @@ -284,11 +298,11 @@ func (svc service) ResetSecret(ctx context.Context, resetToken, secret string) e return errors.Wrap(svcerr.ErrViewEntity, err) } if c.Credentials.Identity == "" { - return repoerr.ErrNotFound + return svcerr.ErrNotFound } secret, err = svc.hasher.Hash(secret) if err != nil { - return err + return errors.Wrap(svcerr.ErrUpdateEntity, err) } c = mgclients.Client{ ID: c.ID, @@ -315,11 +329,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{}, errors.Wrap(ErrIssueToken, 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 +369,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 +404,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 @@ -399,10 +413,10 @@ func (svc service) DisableClient(ctx context.Context, token, id string) (mgclien func (svc service) changeClientStatus(ctx context.Context, token string, client mgclients.Client) (mgclients.Client, error) { tokenUserID, err := svc.Identify(ctx, token) if err != nil { - return mgclients.Client{}, err + return mgclients.Client{}, errors.Wrap(svcerr.ErrAuthentication, err) } if err := svc.checkSuperAdmin(ctx, tokenUserID); err != nil { - return mgclients.Client{}, err + return mgclients.Client{}, errors.Wrap(svcerr.ErrAuthorization, err) } dbClient, err := svc.clients.RetrieveByID(ctx, client.ID) if err != nil { @@ -417,7 +431,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 +512,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 +526,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 +576,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.ErrNotFound, err) } claims := &magistrala.IssueReq{ UserId: rclient.ID, @@ -575,9 +589,9 @@ 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, err) } - return &magistrala.Token{}, err + return &magistrala.Token{}, errors.Wrap(svcerr.ErrCreateEntity, err) } claims := &magistrala.IssueReq{ UserId: rclient.ID, @@ -619,7 +633,7 @@ func (svc service) addClientPolicy(ctx context.Context, userID string, role mgcl } resp, err := svc.auth.AddPolicies(ctx, &policies) if err != nil { - return errors.Wrap(svcerr.ErrAddPolicies, err) + return errors.Wrap(errAddPolicies, err) } if !resp.Added { return svcerr.ErrAuthorization @@ -652,7 +666,7 @@ func (svc service) addClientPolicyRollback(ctx context.Context, userID string, r return errors.Wrap(svcerr.ErrDeletePolicies, err) } if !resp.Deleted { - return svcerr.ErrAuthorization + return svcerr.ErrDeletePolicies } return nil } @@ -668,10 +682,10 @@ func (svc service) updateClientPolicy(ctx context.Context, userID string, role m Object: auth.MagistralaObject, }) if err != nil { - return errors.Wrap(svcerr.ErrAddPolicies, err) + return errors.Wrap(errAddPolicies, err) } if !resp.Added { - return svcerr.ErrAuthorization + return errors.Wrap(svcerr.ErrUpdateEntity, errAddPolicies) } return nil case mgclients.UserRole: @@ -685,10 +699,10 @@ func (svc service) updateClientPolicy(ctx context.Context, userID string, role m Object: auth.MagistralaObject, }) if err != nil { - return errors.Wrap(svcerr.ErrDeletePolicies, err) + return errors.Wrap(svcerr.ErrUpdateEntity, err) } if !resp.Deleted { - return svcerr.ErrAuthorization + return svcerr.ErrUpdateEntity } return nil } diff --git a/users/service_test.go b/users/service_test.go index 5b0a82f757..5abf05a47d 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -1024,7 +1024,7 @@ func TestUpdateClientRole(t *testing.T) { authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, addPolicyResponse: &magistrala.AddPolicyRes{Added: false}, token: validToken, - err: svcerr.ErrAuthorization, + err: svcerr.ErrAddPolicies, }, { desc: "update client role with failed to add policy", @@ -1053,7 +1053,7 @@ func TestUpdateClientRole(t *testing.T) { deletePolicyResponse: &magistrala.DeletePolicyRes{Deleted: false}, updateRoleResponse: mgclients.Client{}, token: validToken, - err: svcerr.ErrFailedPolicyUpdate, + err: svcerr.ErrUpdateEntity, }, { desc: "update client role to user role with failed to delete policy with error", @@ -1063,7 +1063,7 @@ func TestUpdateClientRole(t *testing.T) { updateRoleResponse: mgclients.Client{}, token: validToken, deletePolicyErr: svcerr.ErrMalformedEntity, - err: svcerr.ErrDeletePolicies, + err: svcerr.ErrUpdateEntity, }, { desc: "Update client with failed repo update and roll back", @@ -2547,7 +2547,7 @@ func TestOAuthCallback(t *testing.T) { saveResponse: mgclients.Client{}, saveErr: repoerr.ErrConflict, issueResponse: &magistrala.Token{}, - err: errors.New("user already exists"), + err: svcerr.ErrConflict, }, } From 5efad4d9421b648e3b49310a0c4cefa855ebe4a3 Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Thu, 18 Apr 2024 19:54:51 +0300 Subject: [PATCH 2/8] update auth api docs Signed-off-by: WashingtonKK refactor errors Signed-off-by: WashingtonKK refactor errors Signed-off-by: WashingtonKK --- api/openapi/auth.yml | 2 ++ certs/service.go | 2 +- pkg/errors/service/types.go | 1 + pkg/sdk/go/certs_test.go | 2 +- users/service.go | 20 ++++++-------------- users/service_test.go | 4 ++-- 6 files changed, 13 insertions(+), 18 deletions(-) diff --git a/api/openapi/auth.yml b/api/openapi/auth.yml index 42dba996a6..f46872ef0c 100644 --- a/api/openapi/auth.yml +++ b/api/openapi/auth.yml @@ -156,6 +156,8 @@ paths: 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/certs/service.go b/certs/service.go index e324efbd06..191b328594 100644 --- a/certs/service.go +++ b/certs/service.go @@ -187,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(svcerr.ErrNotFound, err) + return Cert{}, errors.Wrap(svcerr.ErrViewEntity, err) } vcert, err := cs.pki.Read(serialID) diff --git a/pkg/errors/service/types.go b/pkg/errors/service/types.go index 8a5525b73e..a19f4b0f25 100644 --- a/pkg/errors/service/types.go +++ b/pkg/errors/service/types.go @@ -63,6 +63,7 @@ var ( // 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") diff --git a/pkg/sdk/go/certs_test.go b/pkg/sdk/go/certs_test.go index 700d22c3b5..aeb457eeb9 100644 --- a/pkg/sdk/go/certs_test.go +++ b/pkg/sdk/go/certs_test.go @@ -181,7 +181,7 @@ func TestViewCert(t *testing.T) { certID: "43", token: token, cRes: c, - err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, svcerr.ErrNotFound), http.StatusNotFound), + err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, svcerr.ErrViewEntity), http.StatusBadRequest), svcerr: errors.Wrap(svcerr.ErrNotFound, repoerr.ErrNotFound), }, { diff --git a/users/service.go b/users/service.go index 957262e5d9..ea914ad380 100644 --- a/users/service.go +++ b/users/service.go @@ -20,13 +20,7 @@ import ( ) var ( - - // ErrFailedUpdateRole indicates a failure to update user role. - ErrFailedUpdateRole = errors.New("failed to update user role") - - // ErrIssueToken indicates a failure to issue token. - ErrIssueToken = errors.New("failed to issue token") - + errIssueToken = errors.New("failed to issue token") errUserNotSignedUp = errors.New("user not signed up") errFailedPermissionsList = errors.New("failed to list permissions") errAddPolicies = errors.New("failed to add policies") @@ -273,8 +267,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 svcerr.ErrNotFound + if err != nil { + return errors.Wrap(svcerr.ErrViewEntity, err) } issueReq := &magistrala.IssueReq{ UserId: client.ID, @@ -297,9 +291,7 @@ 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 svcerr.ErrNotFound - } + secret, err = svc.hasher.Hash(secret) if err != nil { return errors.Wrap(svcerr.ErrUpdateEntity, err) @@ -329,7 +321,7 @@ 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(ErrIssueToken, err) + return mgclients.Client{}, errors.Wrap(errIssueToken, err) } newSecret, err = svc.hasher.Hash(newSecret) if err != nil { @@ -589,7 +581,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.Wrap(svcerr.ErrConflict, err) + return &magistrala.Token{}, errors.New("user already exists") } return &magistrala.Token{}, errors.Wrap(svcerr.ErrCreateEntity, err) } diff --git a/users/service_test.go b/users/service_test.go index 5abf05a47d..806e49dfc0 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -2349,7 +2349,7 @@ func TestResetSecret(t *testing.T) { Identity: "", }, }, - err: repoerr.ErrNotFound, + err: nil, }, { desc: "reset secret with failed to update secret", @@ -2547,7 +2547,7 @@ func TestOAuthCallback(t *testing.T) { saveResponse: mgclients.Client{}, saveErr: repoerr.ErrConflict, issueResponse: &magistrala.Token{}, - err: svcerr.ErrConflict, + err: errors.New("user already exists"), }, } From a343321f1e182ad7ad2c974c5fe6f03b8a540502 Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Tue, 23 Apr 2024 16:10:41 +0300 Subject: [PATCH 3/8] fix: certs and twins tests Signed-off-by: WashingtonKK fix: certs and twins tests Signed-off-by: WashingtonKK --- pkg/sdk/go/certs_test.go | 31 ++++++++----------------------- users/service.go | 2 +- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/pkg/sdk/go/certs_test.go b/pkg/sdk/go/certs_test.go index aeb457eeb9..d3a7c8ca45 100644 --- a/pkg/sdk/go/certs_test.go +++ b/pkg/sdk/go/certs_test.go @@ -175,13 +175,15 @@ func TestViewCert(t *testing.T) { certID: validID, token: token, cRes: c, + err: nil, + svcerr: nil, }, { desc: "get non-existent cert", certID: "43", token: token, cRes: c, - err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, svcerr.ErrViewEntity), http.StatusBadRequest), + err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, svcerr.ErrNotFound), http.StatusNotFound), svcerr: errors.Wrap(svcerr.ErrNotFound, repoerr.ErrNotFound), }, { @@ -230,14 +232,8 @@ func TestViewCertByThing(t *testing.T) { { desc: "get existing cert", thingID: thingID, - token: authmocks.InvalidValue, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), - }, - { - desc: "revoke non-existing cert", - thingID: "2", token: token, - err: errors.NewSDKErrorWithStatus(certs.ErrFailedCertRevocation, http.StatusNotFound), + page: certs.Page{Certs: []certs.Cert{c}}, }, { desc: "get non-existent cert", @@ -254,18 +250,7 @@ func TestViewCertByThing(t *testing.T) { token: "", page: certs.Page{Certs: []certs.Cert{}}, err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrBearerToken), http.StatusUnauthorized), - }, - { - desc: "revoke existing cert", - thingID: thingID, - token: token, - err: nil, - }, - { - desc: "revoke deleted cert", - thingID: thingID, - token: token, - err: errors.NewSDKErrorWithStatus(certs.ErrFailedToRemoveCertFromDB, http.StatusNotFound), + svcerr: errors.Wrap(svcerr.ErrAuthentication, apiutil.ErrBearerToken), }, } for _, tc := range cases { @@ -308,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), }, { @@ -316,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), }, { @@ -338,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/users/service.go b/users/service.go index ea914ad380..6b096d5de2 100644 --- a/users/service.go +++ b/users/service.go @@ -294,7 +294,7 @@ func (svc service) ResetSecret(ctx context.Context, resetToken, secret string) e secret, err = svc.hasher.Hash(secret) if err != nil { - return errors.Wrap(svcerr.ErrUpdateEntity, err) + return errors.Wrap(svcerr.ErrMalformedEntity, err) } c = mgclients.Client{ ID: c.ID, From e862650de94ba7f8eff0c5d99725c5435719f9ae Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Wed, 24 Apr 2024 23:45:00 +0300 Subject: [PATCH 4/8] refactor errors in users service layer Signed-off-by: WashingtonKK fix: issue token Signed-off-by: WashingtonKK refactor error handling and fix tests: Signed-off-by: WashingtonKK fix tests and remove redundant error Signed-off-by: WashingtonKK Update users/service.go Co-authored-by: Arvindh <30824765+arvindh123@users.noreply.github.com> Signed-off-by: WashingtonKK modify delete policies error Signed-off-by: WashingtonKK revert error types Signed-off-by: WashingtonKK fix: ci Signed-off-by: WashingtonKK revert policy rollback error Signed-off-by: WashingtonKK revert domain check Signed-off-by: WashingtonKK remove debug logs Signed-off-by: WashingtonKK fix bootstrap property based tests Signed-off-by: WashingtonKK refactor bootstrap service error Signed-off-by: WashingtonKK --- api/openapi/users.yml | 2 ++ auth/service.go | 9 ++++++++- auth/service_test.go | 24 ++++++++++++++++++++---- bootstrap/service.go | 6 ++++-- internal/api/common.go | 2 +- pkg/sdk/go/tokens_test.go | 2 +- users/service.go | 33 +++++++++++++++++++-------------- users/service_test.go | 6 +++--- 8 files changed, 58 insertions(+), 26 deletions(-) diff --git a/api/openapi/users.yml b/api/openapi/users.yml index cd182df185..f09e93076a 100644 --- a/api/openapi/users.yml +++ b/api/openapi/users.yml @@ -519,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/service.go b/auth/service.go index 1e31977557..fa57c1aba3 100644 --- a/auth/service.go +++ b/auth/service.go @@ -186,7 +186,14 @@ func (svc service) Authorize(ctx context.Context, pr PolicyReq) error { func (svc service) checkPolicy(ctx context.Context, pr PolicyReq) error { // Domain status is required for if user sent authorization request on things, channels, groups and domains if pr.SubjectType == UserType && (pr.ObjectType == GroupType || pr.ObjectType == ThingType || pr.ObjectType == DomainType) { - if err := svc.checkDomain(ctx, pr.SubjectType, pr.Subject, pr.Domain); err != nil { + domainID := pr.Domain + if domainID == "" { + if pr.ObjectType != DomainType { + return svcerr.ErrDomainAuthorization + } + domainID = pr.Object + } + if err := svc.checkDomain(ctx, pr.SubjectType, pr.Subject, domainID); err != nil { return err } } diff --git a/auth/service_test.go b/auth/service_test.go index a0281045eb..b700044bdd 100644 --- a/auth/service_test.go +++ b/auth/service_test.go @@ -215,6 +215,7 @@ func TestIssue(t *testing.T) { SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, + Object: groupName, }, checkPolicyErr: repoerr.ErrNotFound, retrieveByIDResponse: auth.Domain{}, @@ -253,6 +254,7 @@ func TestIssue(t *testing.T) { Permission: auth.MembershipPermission, }, checkPolicyErr: svcerr.ErrAuthorization, + checkPolicyErr1: svcerr.ErrAuthorization, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, err: svcerr.ErrAuthorization, }, @@ -288,7 +290,7 @@ func TestIssue(t *testing.T) { Permission: auth.MembershipPermission, }, checkPolicyErr: svcerr.ErrAuthorization, - checkPolicyErr1: nil, + checkPolicyErr1: svcerr.ErrAuthorization, retrieveByIDResponse: auth.Domain{Status: auth.EnabledStatus}, err: svcerr.ErrAuthorization, }, @@ -443,6 +445,7 @@ func TestIssue(t *testing.T) { Subject: "mgx_test@example.com", SubjectType: auth.UserType, SubjectKind: "", + Object: groupName, ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -522,6 +525,7 @@ func TestIssue(t *testing.T) { }, checkPolicyRequest1: auth.PolicyReq{ SubjectType: auth.UserType, + Object: groupName, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -876,8 +880,9 @@ func TestAuthorize(t *testing.T) { checkPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, - Permission: auth.AdminPermission, + Permission: auth.MembershipPermission, }, checkPolicyReq1: auth.PolicyReq{ Subject: id, @@ -890,8 +895,9 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, - Permission: auth.MembershipPermission, + Permission: auth.AdminPermission, }, retrieveDomainRes: auth.Domain{ @@ -928,6 +934,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -968,6 +975,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1007,6 +1015,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1047,6 +1056,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1080,6 +1090,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1104,6 +1115,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1128,6 +1140,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1153,6 +1166,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1177,6 +1191,7 @@ func TestAuthorize(t *testing.T) { checkPolicyReq2: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, + Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -1184,6 +1199,7 @@ func TestAuthorize(t *testing.T) { }, } for _, tc := range cases { + fmt.Println(tc.desc) repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq).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) @@ -1197,7 +1213,7 @@ func TestAuthorize(t *testing.T) { repoCall3.Unset() repoCall4.Unset() } - + fmt.Println("Cases 1 end") cases2 := []struct { desc string policyReq auth.PolicyReq diff --git a/bootstrap/service.go b/bootstrap/service.go index cee19f88c8..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 diff --git a/internal/api/common.go b/internal/api/common.go index de2dc950b0..af5db9dab0 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -149,10 +149,10 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(err, svcerr.ErrViewEntity), errors.Contains(err, apiutil.ErrBootstrapState), errors.Contains(err, apiutil.ErrMissingCertData), - errors.Contains(err, apiutil.ErrMissingCertData), errors.Contains(err, apiutil.ErrInvalidCertData), errors.Contains(err, apiutil.ErrInvalidContact), errors.Contains(err, apiutil.ErrInvalidTopic), + errors.Contains(err, bootstrap.ErrAddBootstrap), errors.Contains(err, apiutil.ErrInvalidCertData): err = unwrap(err) w.WriteHeader(http.StatusBadRequest) diff --git a/pkg/sdk/go/tokens_test.go b/pkg/sdk/go/tokens_test.go index 0617669df5..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(svcerr.ErrViewEntity, http.StatusBadRequest), + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), }, } for _, tc := range cases { diff --git a/users/service.go b/users/service.go index 6b096d5de2..7a14e1e334 100644 --- a/users/service.go +++ b/users/service.go @@ -23,7 +23,6 @@ var ( errIssueToken = errors.New("failed to issue token") errUserNotSignedUp = errors.New("user not signed up") errFailedPermissionsList = errors.New("failed to list permissions") - errAddPolicies = errors.New("failed to add policies") errRecoveryToken = errors.New("failed to generate password recovery token") ) @@ -101,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) @@ -111,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) { @@ -321,7 +326,7 @@ 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(errIssueToken, err) + return mgclients.Client{}, err } newSecret, err = svc.hasher.Hash(newSecret) if err != nil { @@ -405,10 +410,10 @@ func (svc service) DisableClient(ctx context.Context, token, id string) (mgclien func (svc service) changeClientStatus(ctx context.Context, token string, client mgclients.Client) (mgclients.Client, error) { tokenUserID, err := svc.Identify(ctx, token) if err != nil { - return mgclients.Client{}, errors.Wrap(svcerr.ErrAuthentication, err) + return mgclients.Client{}, err } if err := svc.checkSuperAdmin(ctx, tokenUserID); err != nil { - return mgclients.Client{}, errors.Wrap(svcerr.ErrAuthorization, err) + return mgclients.Client{}, err } dbClient, err := svc.clients.RetrieveByID(ctx, client.ID) if err != nil { @@ -581,9 +586,9 @@ 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{}, errors.Wrap(svcerr.ErrCreateEntity, err) + return &magistrala.Token{}, err } claims := &magistrala.IssueReq{ UserId: rclient.ID, @@ -625,7 +630,7 @@ func (svc service) addClientPolicy(ctx context.Context, userID string, role mgcl } resp, err := svc.auth.AddPolicies(ctx, &policies) if err != nil { - return errors.Wrap(errAddPolicies, err) + return errors.Wrap(svcerr.ErrAddPolicies, err) } if !resp.Added { return svcerr.ErrAuthorization @@ -658,7 +663,7 @@ func (svc service) addClientPolicyRollback(ctx context.Context, userID string, r return errors.Wrap(svcerr.ErrDeletePolicies, err) } if !resp.Deleted { - return svcerr.ErrDeletePolicies + return svcerr.ErrAuthorization } return nil } @@ -674,10 +679,10 @@ func (svc service) updateClientPolicy(ctx context.Context, userID string, role m Object: auth.MagistralaObject, }) if err != nil { - return errors.Wrap(errAddPolicies, err) + return errors.Wrap(svcerr.ErrAddPolicies, err) } if !resp.Added { - return errors.Wrap(svcerr.ErrUpdateEntity, errAddPolicies) + return svcerr.ErrAuthorization } return nil case mgclients.UserRole: @@ -691,10 +696,10 @@ func (svc service) updateClientPolicy(ctx context.Context, userID string, role m Object: auth.MagistralaObject, }) if err != nil { - return errors.Wrap(svcerr.ErrUpdateEntity, err) + return errors.Wrap(svcerr.ErrDeletePolicies, err) } if !resp.Deleted { - return svcerr.ErrUpdateEntity + return svcerr.ErrAuthorization } return nil } diff --git a/users/service_test.go b/users/service_test.go index 806e49dfc0..d06d238a81 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -1024,7 +1024,7 @@ func TestUpdateClientRole(t *testing.T) { authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, addPolicyResponse: &magistrala.AddPolicyRes{Added: false}, token: validToken, - err: svcerr.ErrAddPolicies, + err: svcerr.ErrAuthorization, }, { desc: "update client role with failed to add policy", @@ -1053,7 +1053,7 @@ func TestUpdateClientRole(t *testing.T) { deletePolicyResponse: &magistrala.DeletePolicyRes{Deleted: false}, updateRoleResponse: mgclients.Client{}, token: validToken, - err: svcerr.ErrUpdateEntity, + err: svcerr.ErrAuthorization, }, { desc: "update client role to user role with failed to delete policy with error", @@ -1063,7 +1063,7 @@ func TestUpdateClientRole(t *testing.T) { updateRoleResponse: mgclients.Client{}, token: validToken, deletePolicyErr: svcerr.ErrMalformedEntity, - err: svcerr.ErrUpdateEntity, + err: svcerr.ErrDeletePolicies, }, { desc: "Update client with failed repo update and roll back", From b527df3019296026fdeb31c99b934877e2837b78 Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Tue, 30 Apr 2024 07:44:10 +0300 Subject: [PATCH 5/8] fix ci Signed-off-by: WashingtonKK --- internal/api/common.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/api/common.go b/internal/api/common.go index af5db9dab0..8d675d0189 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -149,7 +149,6 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { 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, bootstrap.ErrAddBootstrap), From 4012c94dd55d5e459c497fdba33e6591059c4b4b Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Tue, 30 Apr 2024 08:19:04 +0300 Subject: [PATCH 6/8] refactor test cases Signed-off-by: WashingtonKK --- auth/service_test.go | 411 +++++++++++++++++-------------------------- 1 file changed, 165 insertions(+), 246 deletions(-) diff --git a/auth/service_test.go b/auth/service_test.go index b700044bdd..93e8ed6e92 100644 --- a/auth/service_test.go +++ b/auth/service_test.go @@ -133,19 +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 - checkPolicyRequest2 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 + checkPolicyReq3uest auth.PolicyReq + checkPlatformPolicyReq auth.PolicyReq + checkDomainPolicyReq auth.PolicyReq + checkPolicyErr error + checkPolicyErr1 error + retreiveByIDErr error + err error }{ { desc: "issue login key", @@ -153,16 +153,13 @@ func TestIssue(t *testing.T) { Type: auth.AccessKey, IssuedAt: time.Now(), }, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -177,16 +174,13 @@ func TestIssue(t *testing.T) { IssuedAt: time.Now(), Domain: groupName, }, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -202,16 +196,13 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ + checkPlatformPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -230,25 +221,19 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ 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, }, - checkPolicyRequest2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -266,25 +251,19 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ 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, }, - checkPolicyRequest2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -302,25 +281,19 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ 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, }, - checkPolicyRequest2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, @@ -333,10 +306,10 @@ 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) + repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3uest).Return(tc.checkPolicyErr) + 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.checkPolicyRequest2).Return(tc.checkPolicyErr) + 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() @@ -399,14 +372,14 @@ func TestIssue(t *testing.T) { } cases4 := []struct { - desc string - key auth.Key - token string - checkPolicyRequest auth.PolicyReq - checkPolicyRequest1 auth.PolicyReq - checkPolicyErr error - retrieveByIDErr error - err error + desc string + key auth.Key + token string + checkPolicyReq3uest auth.PolicyReq + checkDOmainPolicyReq auth.PolicyReq + checkPolicyErr error + retrieveByIDErr error + err error }{ { desc: "issue refresh key", @@ -414,12 +387,10 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, - checkPolicyRequest: auth.PolicyReq{ + checkPolicyReq3uest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, @@ -432,21 +403,17 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, - checkPolicyRequest: auth.PolicyReq{ + checkPolicyReq3uest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ + checkDOmainPolicyReq: auth.PolicyReq{ Subject: "mgx_test@example.com", SubjectType: auth.UserType, - SubjectKind: "", Object: groupName, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -461,11 +428,9 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, - checkPolicyRequest1: auth.PolicyReq{ + checkDOmainPolicyReq: auth.PolicyReq{ Subject: "mgx_test@example.com", SubjectType: auth.UserType, - SubjectKind: "", - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -478,11 +443,9 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, - checkPolicyRequest1: auth.PolicyReq{ + checkDOmainPolicyReq: auth.PolicyReq{ Subject: "mgx_test@example.com", SubjectType: auth.UserType, - SubjectKind: "", - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, @@ -495,12 +458,10 @@ func TestIssue(t *testing.T) { Type: auth.InvitationKey, IssuedAt: time.Now(), }, - checkPolicyRequest: auth.PolicyReq{ + checkPolicyReq3uest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, @@ -514,16 +475,13 @@ func TestIssue(t *testing.T) { IssuedAt: time.Now(), Domain: groupName, }, - checkPolicyRequest: auth.PolicyReq{ - Subject: "", + checkPolicyReq3uest: auth.PolicyReq{ SubjectType: auth.UserType, - SubjectKind: "", Object: auth.MagistralaObject, - ObjectKind: "", ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyRequest1: auth.PolicyReq{ + checkDOmainPolicyReq: auth.PolicyReq{ SubjectType: auth.UserType, Object: groupName, ObjectType: auth.DomainType, @@ -536,9 +494,9 @@ func TestIssue(t *testing.T) { }, } for _, tc := range cases4 { - repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest).Return(tc.checkPolicyErr) + repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3uest).Return(tc.checkPolicyErr) repoCall1 := drepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(auth.Domain{}, tc.retrieveByIDErr) - repoCall2 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest1).Return(tc.checkPolicyErr) + 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() @@ -802,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", @@ -823,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, @@ -832,7 +790,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.PlatformType, Permission: auth.AdminPermission, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, ObjectType: auth.DomainType, @@ -850,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, @@ -858,7 +816,7 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.GroupType, Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, ObjectType: auth.DomainType, @@ -877,14 +835,14 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, ObjectType: auth.DomainType, Permission: auth.MembershipPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, SubjectKind: auth.TokenKind, @@ -892,7 +850,7 @@ func TestAuthorize(t *testing.T) { Object: validID, ObjectType: auth.DomainType, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -917,13 +875,13 @@ func TestAuthorize(t *testing.T) { ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq: auth.PolicyReq{ + checkPolicyReq3: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, ObjectType: auth.DomainType, Permission: auth.AdminPermission, }, - checkPolicyReq1: auth.PolicyReq{ + checkAdminPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, SubjectKind: auth.TokenKind, @@ -931,7 +889,7 @@ func TestAuthorize(t *testing.T) { Object: validID, ObjectType: auth.DomainType, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -957,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, @@ -965,14 +923,14 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -997,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, @@ -1005,14 +963,14 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1038,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, @@ -1046,14 +1004,14 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1079,15 +1037,14 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1106,13 +1063,13 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1131,13 +1088,13 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1149,21 +1106,20 @@ func TestAuthorize(t *testing.T) { { 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1182,13 +1138,13 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: validID, @@ -1200,10 +1156,10 @@ func TestAuthorize(t *testing.T) { } for _, tc := range cases { fmt.Println(tc.desc) - repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq).Return(tc.checkPolicyErr) + 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 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq2).Return(tc.checkPolicyErr1) + 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)) @@ -1222,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, @@ -2207,22 +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 - checkPolicyReq3 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", @@ -2230,36 +2185,32 @@ 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, }, - checkPolicyReq3: auth.PolicyReq{ + checkPolicyReq33: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2275,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, }, @@ -2311,27 +2258,25 @@ 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, }, - checkPolicyReq3: auth.PolicyReq{ + checkPolicyReq33: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2347,36 +2292,32 @@ 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, }, - checkPolicyReq3: auth.PolicyReq{ + checkPolicyReq33: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2392,36 +2333,32 @@ 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, }, - checkPolicyReq3: auth.PolicyReq{ + checkPolicyReq33: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2437,36 +2374,32 @@ 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, }, - checkPolicyReq3: auth.PolicyReq{ + checkPolicyReq33: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2482,36 +2415,32 @@ 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, }, - checkPolicyReq3: auth.PolicyReq{ + checkPolicyReq33: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2526,10 +2455,10 @@ func TestAssignUsers(t *testing.T) { 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("CheckPolicy", mock.Anything, tc.checkPolicyReq3).Return(tc.checkPolicyErr2) + 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) @@ -2550,46 +2479,44 @@ func TestUnassignUsers(t *testing.T) { svc, accessToken := newService() cases := []struct { - desc string - token string - domainID string - checkPolicyReq auth.PolicyReq - checkPolicyReq1 auth.PolicyReq - checkPolicyReq2 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, }, - 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Domain: groupName, Subject: "testID", SubjectType: auth.UserType, SubjectKind: auth.TokenKind, Object: validID, - ObjectKind: "", ObjectType: auth.DomainType, Permission: auth.SharePermission, }, @@ -2599,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, }, @@ -2625,27 +2550,25 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2659,27 +2582,25 @@ func TestUnassignUsers(t *testing.T) { 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2693,27 +2614,25 @@ 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, }, - checkPolicyReq2: auth.PolicyReq{ + checkDomainPolicyReq: auth.PolicyReq{ Subject: id, SubjectType: auth.UserType, Object: "mgx", @@ -2727,9 +2646,9 @@ 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("CheckPolicy", mock.Anything, tc.checkPolicyReq2).Return(tc.checkPolicyErr1) + 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) From 7d3230235340ec21684e20c0e86b13f0d7602a87 Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Tue, 30 Apr 2024 08:23:39 +0300 Subject: [PATCH 7/8] fix: ci Signed-off-by: WashingtonKK --- auth/service_test.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/auth/service_test.go b/auth/service_test.go index 93e8ed6e92..c47e53ec69 100644 --- a/auth/service_test.go +++ b/auth/service_test.go @@ -139,7 +139,7 @@ func TestIssue(t *testing.T) { retrieveByIDResponse auth.Domain token string saveErr error - checkPolicyReq3uest auth.PolicyReq + checkPolicyRequest auth.PolicyReq checkPlatformPolicyReq auth.PolicyReq checkDomainPolicyReq auth.PolicyReq checkPolicyErr error @@ -153,7 +153,7 @@ func TestIssue(t *testing.T) { Type: auth.AccessKey, IssuedAt: time.Now(), }, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -174,7 +174,7 @@ func TestIssue(t *testing.T) { IssuedAt: time.Now(), Domain: groupName, }, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -196,7 +196,7 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -221,7 +221,7 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -251,7 +251,7 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -281,7 +281,7 @@ func TestIssue(t *testing.T) { Domain: groupName, }, token: accessToken, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -306,7 +306,7 @@ 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.checkPolicyReq3uest).Return(tc.checkPolicyErr) + repoCall1 := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyRequest).Return(tc.checkPolicyErr) 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) @@ -375,7 +375,7 @@ func TestIssue(t *testing.T) { desc string key auth.Key token string - checkPolicyReq3uest auth.PolicyReq + checkPolicyRequest auth.PolicyReq checkDOmainPolicyReq auth.PolicyReq checkPolicyErr error retrieveByIDErr error @@ -387,7 +387,7 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, Object: auth.MagistralaObject, @@ -403,7 +403,7 @@ func TestIssue(t *testing.T) { Type: auth.RefreshKey, IssuedAt: time.Now(), }, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, Object: auth.MagistralaObject, @@ -458,7 +458,7 @@ func TestIssue(t *testing.T) { Type: auth.InvitationKey, IssuedAt: time.Now(), }, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ Subject: email, SubjectType: auth.UserType, Object: auth.MagistralaObject, @@ -475,7 +475,7 @@ func TestIssue(t *testing.T) { IssuedAt: time.Now(), Domain: groupName, }, - checkPolicyReq3uest: auth.PolicyReq{ + checkPolicyRequest: auth.PolicyReq{ SubjectType: auth.UserType, Object: auth.MagistralaObject, ObjectType: auth.PlatformType, @@ -494,7 +494,7 @@ func TestIssue(t *testing.T) { }, } for _, tc := range cases4 { - repoCall := prepo.On("CheckPolicy", mock.Anything, tc.checkPolicyReq3uest).Return(tc.checkPolicyErr) + 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) @@ -763,7 +763,7 @@ func TestAuthorize(t *testing.T) { desc string policyReq auth.PolicyReq retrieveDomainRes auth.Domain - checkPolicyReq3 auth.PolicyReq + checkPolicyReq3 auth.PolicyReq checkAdminPolicyReq auth.PolicyReq checkDomainPolicyReq auth.PolicyReq checkPolicyErr error @@ -2167,10 +2167,10 @@ func TestAssignUsers(t *testing.T) { domainID string userIDs []string relation string - checkPolicyReq3 auth.PolicyReq + checkPolicyReq3 auth.PolicyReq checkAdminPolicyReq auth.PolicyReq checkDomainPolicyReq auth.PolicyReq - checkPolicyReq33 auth.PolicyReq + checkPolicyReq33 auth.PolicyReq checkpolicyErr error checkPolicyErr1 error checkPolicyErr2 error @@ -2482,7 +2482,7 @@ func TestUnassignUsers(t *testing.T) { desc string token string domainID string - checkPolicyReq3 auth.PolicyReq + checkPolicyReq3 auth.PolicyReq checkAdminPolicyReq auth.PolicyReq checkDomainPolicyReq auth.PolicyReq checkPolicyErr error From c2176d9edb72e0be1f0ad9b2c00d37ed54fcb4d7 Mon Sep 17 00:00:00 2001 From: WashingtonKK Date: Tue, 30 Apr 2024 15:59:04 +0300 Subject: [PATCH 8/8] update oauth error Signed-off-by: WashingtonKK --- users/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/users/service.go b/users/service.go index 7a14e1e334..7a3215d042 100644 --- a/users/service.go +++ b/users/service.go @@ -575,7 +575,7 @@ func (svc service) OAuthCallback(ctx context.Context, state mgoauth2.State, clie if errors.Contains(err, repoerr.ErrNotFound) { return &magistrala.Token{}, errors.Wrap(svcerr.ErrNotFound, errUserNotSignedUp) } - return &magistrala.Token{}, errors.Wrap(svcerr.ErrNotFound, err) + return &magistrala.Token{}, errors.Wrap(svcerr.ErrViewEntity, err) } claims := &magistrala.IssueReq{ UserId: rclient.ID,