From 6087647895fe013921ff4fb387d61a9d8f2d7c5f Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Wed, 5 Feb 2020 15:52:00 +0800 Subject: [PATCH] Add permission check for artifact related APIs Add permission check for artifact related APIs Signed-off-by: Wenkai Yin --- api/v2.0/swagger.yaml | 2 -- src/common/rbac/const.go | 4 +++- src/common/rbac/project/util.go | 3 +++ src/common/rbac/project/visitor_role.go | 25 +++++++++++++++++++++++++ src/server/v2.0/handler/artifact.go | 18 ++++++++++++++++-- src/server/v2.0/handler/base.go | 20 +++++++++++++++++++- 6 files changed, 66 insertions(+), 6 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index ae608dcd3..eb55f8d96 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -15,8 +15,6 @@ consumes: securityDefinitions: basicAuth: type: basic -security: - - basicAuth: [] paths: /projects/{project_name}/repositories/{repository_name}/artifacts: get: diff --git a/src/common/rbac/const.go b/src/common/rbac/const.go index e7a5b8f9e..f1947ffad 100755 --- a/src/common/rbac/const.go +++ b/src/common/rbac/const.go @@ -53,7 +53,7 @@ const ( ResourceTagRetention = Resource("tag-retention") ResourceImmutableTag = Resource("immutable-tag") ResourceRepositoryLabel = Resource("repository-label") - ResourceRepositoryTag = Resource("repository-tag") + ResourceRepositoryTag = Resource("repository-tag") // TODO remove ResourceRepositoryTagLabel = Resource("repository-tag-label") ResourceRepositoryTagManifest = Resource("repository-tag-manifest") ResourceRepositoryTagScanJob = Resource("repository-tag-scan-job") // TODO: remove @@ -62,5 +62,7 @@ const ( ResourceNotificationPolicy = Resource("notification-policy") ResourceScan = Resource("scan") ResourceScanner = Resource("scanner") + ResourceArtifact = Resource("artifact") + ResourceTag = Resource("tag") ResourceSelf = Resource("") // subresource for self ) diff --git a/src/common/rbac/project/util.go b/src/common/rbac/project/util.go index 9106d9616..cdfc4756f 100644 --- a/src/common/rbac/project/util.go +++ b/src/common/rbac/project/util.go @@ -48,6 +48,9 @@ var ( {Resource: rbac.ResourceScan, Action: rbac.ActionRead}, {Resource: rbac.ResourceScanner, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceArtifact, Action: rbac.ActionRead}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionList}, } // all policies for the projects diff --git a/src/common/rbac/project/visitor_role.go b/src/common/rbac/project/visitor_role.go index b2ad6e502..ab100bdfb 100755 --- a/src/common/rbac/project/visitor_role.go +++ b/src/common/rbac/project/visitor_role.go @@ -127,6 +127,13 @@ var ( {Resource: rbac.ResourceScanner, Action: rbac.ActionRead}, {Resource: rbac.ResourceScanner, Action: rbac.ActionCreate}, + + {Resource: rbac.ResourceArtifact, Action: rbac.ActionRead}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionList}, + + {Resource: rbac.ResourceTag, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceTag, Action: rbac.ActionDelete}, }, "master": { @@ -216,6 +223,13 @@ var ( {Resource: rbac.ResourceScan, Action: rbac.ActionRead}, {Resource: rbac.ResourceScanner, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceArtifact, Action: rbac.ActionRead}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionList}, + + {Resource: rbac.ResourceTag, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceTag, Action: rbac.ActionDelete}, }, "developer": { @@ -272,6 +286,11 @@ var ( {Resource: rbac.ResourceScan, Action: rbac.ActionRead}, {Resource: rbac.ResourceScanner, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceArtifact, Action: rbac.ActionRead}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionList}, + + {Resource: rbac.ResourceTag, Action: rbac.ActionCreate}, }, "guest": { @@ -316,6 +335,9 @@ var ( {Resource: rbac.ResourceScan, Action: rbac.ActionRead}, {Resource: rbac.ResourceScanner, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceArtifact, Action: rbac.ActionRead}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionList}, }, "limitedGuest": { @@ -344,6 +366,9 @@ var ( {Resource: rbac.ResourceScan, Action: rbac.ActionRead}, {Resource: rbac.ResourceScanner, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceArtifact, Action: rbac.ActionRead}, + {Resource: rbac.ResourceArtifact, Action: rbac.ActionList}, }, } ) diff --git a/src/server/v2.0/handler/artifact.go b/src/server/v2.0/handler/artifact.go index 7a8c47033..451724a88 100644 --- a/src/server/v2.0/handler/artifact.go +++ b/src/server/v2.0/handler/artifact.go @@ -19,6 +19,7 @@ import ( "fmt" "github.com/go-openapi/runtime/middleware" "github.com/goharbor/harbor/src/api/artifact" + "github.com/goharbor/harbor/src/common/rbac" ierror "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/project" "github.com/goharbor/harbor/src/pkg/q" @@ -43,9 +44,10 @@ type artifactAPI struct { repoMgr repository.Manager } -// TODO do auth in a separate middleware - func (a *artifactAPI) ListArtifacts(ctx context.Context, params operation.ListArtifactsParams) middleware.Responder { + if err := a.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionList, rbac.ResourceArtifact); err != nil { + return a.SendError(ctx, err) + } // set query query := &q.Query{ Keywords: map[string]interface{}{}, @@ -85,6 +87,9 @@ func (a *artifactAPI) ListArtifacts(ctx context.Context, params operation.ListAr } func (a *artifactAPI) GetArtifact(ctx context.Context, params operation.GetArtifactParams) middleware.Responder { + if err := a.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionRead, rbac.ResourceArtifact); err != nil { + return a.SendError(ctx, err) + } // set option option := option(params.WithTag, params.WithImmutableStatus, params.WithLabel, params.WithScanOverview, params.WithSignature) @@ -98,6 +103,9 @@ func (a *artifactAPI) GetArtifact(ctx context.Context, params operation.GetArtif } func (a *artifactAPI) DeleteArtifact(ctx context.Context, params operation.DeleteArtifactParams) middleware.Responder { + if err := a.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionDelete, rbac.ResourceArtifact); err != nil { + return a.SendError(ctx, err) + } artifact, err := a.artCtl.GetByReference(ctx, fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName), params.Reference, nil) if err != nil { return a.SendError(ctx, err) @@ -109,6 +117,9 @@ func (a *artifactAPI) DeleteArtifact(ctx context.Context, params operation.Delet } func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagParams) middleware.Responder { + if err := a.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionCreate, rbac.ResourceTag); err != nil { + return a.SendError(ctx, err) + } art, err := a.artCtl.GetByReference(ctx, fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName), params.Reference, &artifact.Option{ WithTag: true, @@ -129,6 +140,9 @@ func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagP } func (a *artifactAPI) DeleteTag(ctx context.Context, params operation.DeleteTagParams) middleware.Responder { + if err := a.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionDelete, rbac.ResourceTag); err != nil { + return a.SendError(ctx, err) + } artifact, err := a.artCtl.GetByReference(ctx, fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName), params.Reference, &artifact.Option{ WithTag: true, diff --git a/src/server/v2.0/handler/base.go b/src/server/v2.0/handler/base.go index b9c34e7ad..8b1057b64 100644 --- a/src/server/v2.0/handler/base.go +++ b/src/server/v2.0/handler/base.go @@ -18,6 +18,8 @@ package handler import ( "context" + "errors" + ierror "github.com/goharbor/harbor/src/internal/error" "github.com/go-openapi/runtime/middleware" "github.com/goharbor/harbor/src/common/rbac" @@ -45,7 +47,7 @@ func (*BaseAPI) SendError(ctx context.Context, err error) middleware.Responder { func (*BaseAPI) HasPermission(ctx context.Context, action rbac.Action, resource rbac.Resource) bool { s, ok := security.FromContext(ctx) if !ok { - log.Warningf("security not found in the contxt") + log.Warningf("security not found in the context") return false } @@ -77,3 +79,19 @@ func (b *BaseAPI) HasProjectPermission(ctx context.Context, projectIDOrName inte resource := rbac.NewProjectNamespace(projectID).Resource(subresource...) return b.HasPermission(ctx, action, resource) } + +// RequireProjectAccess checks the permission against the resources according to the context +// An error will be returned if it doesn't meet the requirement +func (b *BaseAPI) RequireProjectAccess(ctx context.Context, projectIDOrName interface{}, action rbac.Action, subresource ...rbac.Resource) error { + if b.HasProjectPermission(ctx, projectIDOrName, action, subresource...) { + return nil + } + secCtx, ok := security.FromContext(ctx) + if !ok { + return ierror.UnauthorizedError(errors.New("security context not found")) + } + if !secCtx.IsAuthenticated() { + return ierror.UnauthorizedError(nil) + } + return ierror.ForbiddenError(nil) +}