Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MG-2117 - Remove repository errors from API layer #2119

Merged
merged 8 commits into from
May 10, 2024
Merged

MG-2117 - Remove repository errors from API layer #2119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b6f497a
feat: return only service errors
WashingtonKK Mar 20, 2024
5efad4d
update auth api docs
WashingtonKK Apr 18, 2024
a343321
fix: certs and twins tests
WashingtonKK Apr 23, 2024
e862650
refactor errors in users service layer
WashingtonKK Apr 24, 2024
b527df3
fix ci
WashingtonKK Apr 30, 2024
4012c94
refactor test cases
WashingtonKK Apr 30, 2024
7d32302
fix: ci
WashingtonKK Apr 30, 2024
c2176d9
update oauth error
WashingtonKK Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/openapi/auth.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,12 @@ paths:
responses:
"200":
$ref: "#/components/responses/DomainPermissionRes"
"400":
description: Malformed entity specification.
"401":
description: Missing or invalid access token provided.
"403":
description: Failed authorization over the domain.
"404":
description: A non-existent entity request.
"422":
Expand Down
2 changes: 2 additions & 0 deletions api/openapi/bootstrap.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
4 changes: 4 additions & 0 deletions api/openapi/users.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down Expand Up @@ -517,6 +519,8 @@ paths:
$ref: "#/components/responses/TokenRes"
"400":
description: Failed due to malformed JSON.
"401":
description: Missing or invalid access token provided.
"404":
description: A non-existent entity request.
"415":
Expand Down
12 changes: 6 additions & 6 deletions auth/api/http/domains/endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -1056,7 +1056,7 @@ func TestAssignDomainUsers(t *testing.T) {
contentType: contentType,
token: validToken,
status: http.StatusBadRequest,
err: apiutil.ErrValidation,
err: apiutil.ErrMalformedPolicy,
},
}

Expand Down
2 changes: 1 addition & 1 deletion auth/api/http/keys/endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand Down
26 changes: 18 additions & 8 deletions auth/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -204,6 +204,16 @@ func (svc service) checkPolicy(ctx context.Context, pr PolicyReq) error {
}

func (svc service) checkDomain(ctx context.Context, subjectType, subject, domainID string) error {
if err := svc.agent.CheckPolicy(ctx, PolicyReq{
Subject: subject,
SubjectType: subjectType,
Permission: MembershipPermission,
Object: domainID,
ObjectType: DomainType,
}); err != nil {
return svcerr.ErrDomainAuthorization
}

d, err := svc.domains.RetrieveByID(ctx, domainID)
if err != nil {
return errors.Wrap(svcerr.ErrViewEntity, err)
Expand Down Expand Up @@ -531,7 +541,7 @@ func (svc service) CreateDomain(ctx context.Context, token string, d Domain) (do

domainID, err := svc.idProvider.ID()
if err != nil {
return Domain{}, err
return Domain{}, errors.Wrap(svcerr.ErrCreateEntity, err)
}
d.ID = domainID

Expand Down Expand Up @@ -580,7 +590,7 @@ func (svc service) RetrieveDomain(ctx context.Context, token, id string) (Domain
func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id string) (Permissions, error) {
res, err := svc.Identify(ctx, token)
if err != nil {
return []string{}, errors.Wrap(svcerr.ErrAuthentication, err)
return []string{}, err
}

if err := svc.Authorize(ctx, PolicyReq{
Expand All @@ -591,7 +601,7 @@ func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id stri
ObjectType: DomainType,
Permission: MembershipPermission,
}); err != nil {
return []string{}, errors.Wrap(svcerr.ErrAuthorization, err)
return []string{}, err
}

lp, err := svc.ListPermissions(ctx, PolicyReq{
Expand All @@ -609,7 +619,7 @@ func (svc service) RetrieveDomainPermissions(ctx context.Context, token, id stri
func (svc service) UpdateDomain(ctx context.Context, token, id string, d DomainReq) (Domain, error) {
key, err := svc.Identify(ctx, token)
if err != nil {
return Domain{}, errors.Wrap(svcerr.ErrAuthentication, err)
return Domain{}, err
}
if err := svc.Authorize(ctx, PolicyReq{
Subject: key.Subject,
Expand All @@ -619,7 +629,7 @@ func (svc service) UpdateDomain(ctx context.Context, token, id string, d DomainR
ObjectType: DomainType,
Permission: EditPermission,
}); err != nil {
return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err)
return Domain{}, err
}

dom, err := svc.domains.Update(ctx, id, key.User, d)
Expand All @@ -642,7 +652,7 @@ func (svc service) ChangeDomainStatus(ctx context.Context, token, id string, d D
ObjectType: DomainType,
Permission: AdminPermission,
}); err != nil {
return Domain{}, errors.Wrap(svcerr.ErrAuthorization, err)
return Domain{}, err
}

dom, err := svc.domains.Update(ctx, id, key.User, d)
Expand Down Expand Up @@ -765,7 +775,7 @@ func (svc service) UnassignUsers(ctx context.Context, token, id string, userIds
for _, rel := range []string{MemberRelation, ViewerRelation, EditorRelation} {
// Remove only non-admins.
if err := svc.removeDomainPolicies(ctx, id, rel, userIds...); err != nil {
return errors.Wrap(errRemovePolicies, err)
return err
}
}

Expand Down
Loading
Loading