From 168637a743427d9dda7e2ecd6f1a0d86cb8d05a4 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Mon, 23 Mar 2020 16:59:10 +0800 Subject: [PATCH] Add permission check for audit logs API (#11154) add a base method to require system admin permission Signed-off-by: wang yan --- src/pkg/audit/dao/dao.go | 2 +- src/pkg/audit/dao/dao_test.go | 50 +++++++++++++++++++++++++++-- src/server/v2.0/handler/auditlog.go | 34 +++++++++++++++++--- src/server/v2.0/handler/base.go | 15 +++++++++ 4 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/pkg/audit/dao/dao.go b/src/pkg/audit/dao/dao.go index 90141d4e0..19fd27de3 100644 --- a/src/pkg/audit/dao/dao.go +++ b/src/pkg/audit/dao/dao.go @@ -62,10 +62,10 @@ func (d *dao) Count(ctx context.Context, query *q.Query) (int64, error) { func (d *dao) List(ctx context.Context, query *q.Query) ([]*model.AuditLog, error) { audit := []*model.AuditLog{} qs, err := orm.QuerySetter(ctx, &model.AuditLog{}, query) - qs = qs.OrderBy("-op_time") if err != nil { return nil, err } + qs = qs.OrderBy("-op_time") if _, err = qs.All(&audit); err != nil { return nil, err } diff --git a/src/pkg/audit/dao/dao_test.go b/src/pkg/audit/dao/dao_test.go index f28e5a3ae..dfd0a47f0 100644 --- a/src/pkg/audit/dao/dao_test.go +++ b/src/pkg/audit/dao/dao_test.go @@ -41,7 +41,7 @@ func (d *daoTestSuite) SetupSuite() { artifactID, err := d.dao.Create(d.ctx, &model.AuditLog{ Operation: "Create", ResourceType: "artifact", - Resource: "library/hello-world", + Resource: "library/test-audit", Username: "admin", }) d.Require().Nil(err) @@ -59,7 +59,7 @@ func (d *daoTestSuite) TestCount() { d.True(total > 0) total, err = d.dao.Count(d.ctx, &q.Query{ Keywords: map[string]interface{}{ - "ResourceType": "artifact", + "Resource": "library/test-audit", }, }) d.Require().Nil(err) @@ -74,7 +74,7 @@ func (d *daoTestSuite) TestList() { // query by repository ID and name audits, err = d.dao.List(d.ctx, &q.Query{ Keywords: map[string]interface{}{ - "ResourceType": "artifact", + "Resource": "library/test-audit", }, }) d.Require().Nil(err) @@ -94,6 +94,50 @@ func (d *daoTestSuite) TestGet() { d.Equal(d.auditID, audit.ID) } +func (d *daoTestSuite) TestListPIDs() { + // get the non-exist tag + id1, err := d.dao.Create(d.ctx, &model.AuditLog{ + Operation: "Create", + ResourceType: "artifact", + Resource: "library/hello-world", + Username: "admin", + ProjectID: 11, + }) + d.Require().Nil(err) + id2, err := d.dao.Create(d.ctx, &model.AuditLog{ + Operation: "Create", + ResourceType: "artifact", + Resource: "library/hello-world", + Username: "admin", + ProjectID: 12, + }) + d.Require().Nil(err) + id3, err := d.dao.Create(d.ctx, &model.AuditLog{ + Operation: "Delete", + ResourceType: "artifact", + Resource: "library/hello-world", + Username: "admin", + ProjectID: 13, + }) + d.Require().Nil(err) + + // query by repository ID and name + ol := &q.OrList{} + for _, item := range []int64{11, 12, 13} { + ol.Values = append(ol.Values, item) + } + audits, err := d.dao.List(d.ctx, &q.Query{ + Keywords: map[string]interface{}{ + "ProjectID": ol, + }, + }) + d.Require().Nil(err) + d.Require().Equal(3, len(audits)) + d.dao.Delete(d.ctx, id1) + d.dao.Delete(d.ctx, id2) + d.dao.Delete(d.ctx, id3) +} + func (d *daoTestSuite) TestCreate() { // conflict audit := &model.AuditLog{ diff --git a/src/server/v2.0/handler/auditlog.go b/src/server/v2.0/handler/auditlog.go index 77d9a60a9..cb67404d0 100644 --- a/src/server/v2.0/handler/auditlog.go +++ b/src/server/v2.0/handler/auditlog.go @@ -2,8 +2,14 @@ package handler import ( "context" + "errors" + "fmt" "github.com/go-openapi/runtime/middleware" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/security" + ierror "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/audit" + "github.com/goharbor/harbor/src/pkg/q" "github.com/goharbor/harbor/src/server/v2.0/models" "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/auditlog" operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/auditlog" @@ -21,14 +27,34 @@ type auditlogAPI struct { } func (a *auditlogAPI) ListAuditLogs(ctx context.Context, params auditlog.ListAuditLogsParams) middleware.Responder { - // ToDo enable permission check - // if !a.HasPermission(ctx, rbac.ActionList, rbac.ResourceLog) { - // return a.SendError(ctx, ierror.ForbiddenError(nil)) - // } + secCtx, ok := security.FromContext(ctx) + if !ok { + return a.SendError(ctx, ierror.UnauthorizedError(errors.New("security context not found"))) + } + if !secCtx.IsAuthenticated() { + return a.SendError(ctx, ierror.UnauthorizedError(nil).WithMessage(secCtx.GetUsername())) + } query, err := a.BuildQuery(ctx, params.Q, params.Page, params.PageSize) if err != nil { return a.SendError(ctx, err) } + + if !secCtx.IsSysAdmin() { + // ToDo remove the dependency on GetMyProjects() + projects, err := secCtx.GetMyProjects() + if err != nil { + return a.SendError(ctx, fmt.Errorf( + "failed to get projects of user %s: %v", secCtx.GetUsername(), err)) + } + ol := &q.OrList{} + for _, project := range projects { + if a.HasProjectPermission(ctx, project.ProjectID, rbac.ActionList, rbac.ResourceLog) { + ol.Values = append(ol.Values, project.ProjectID) + } + } + query.Keywords["ProjectID"] = ol + } + total, err := a.auditMgr.Count(ctx, query) if err != nil { return a.SendError(ctx, err) diff --git a/src/server/v2.0/handler/base.go b/src/server/v2.0/handler/base.go index 425cb837a..a55f8bdc8 100644 --- a/src/server/v2.0/handler/base.go +++ b/src/server/v2.0/handler/base.go @@ -99,6 +99,21 @@ func (b *BaseAPI) RequireProjectAccess(ctx context.Context, projectIDOrName inte return ierror.ForbiddenError(nil) } +// RequireSysAdmin checks the system admin permission according to the security context +func (b *BaseAPI) RequireSysAdmin(ctx context.Context) error { + secCtx, ok := security.FromContext(ctx) + if !ok { + return ierror.UnauthorizedError(errors.New("security context not found")) + } + if !secCtx.IsAuthenticated() { + return ierror.UnauthorizedError(nil) + } + if !secCtx.IsSysAdmin() { + return ierror.ForbiddenError(nil).WithMessage(secCtx.GetUsername()) + } + return nil +} + // BuildQuery builds the query model according to the query string func (b *BaseAPI) BuildQuery(ctx context.Context, query *string, pageNumber, pageSize *int64) (*q.Query, error) { var (