From c3432320810c35a6c7ab89e84c3957ab94c05999 Mon Sep 17 00:00:00 2001 From: Chenyu Zhang Date: Wed, 22 Jun 2022 18:30:06 +0800 Subject: [PATCH] fix: revise the process of policy update (#17021) Signed-off-by: chlins --- src/server/v2.0/handler/immutable.go | 33 +++++++++++++++++++-- src/server/v2.0/handler/notification_job.go | 25 ++++++++++++++++ src/server/v2.0/handler/preheat.go | 27 +++++++++++++++++ src/server/v2.0/handler/retention.go | 29 ++++++++++++++++-- 4 files changed, 109 insertions(+), 5 deletions(-) diff --git a/src/server/v2.0/handler/immutable.go b/src/server/v2.0/handler/immutable.go index 61462f8b9..280e659f9 100644 --- a/src/server/v2.0/handler/immutable.go +++ b/src/server/v2.0/handler/immutable.go @@ -2,19 +2,20 @@ package handler import ( "context" - "errors" "fmt" + "strings" + "github.com/go-openapi/runtime/middleware" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/controller/immutable" "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/immutable/model" handler_model "github.com/goharbor/harbor/src/server/v2.0/handler/model" "github.com/goharbor/harbor/src/server/v2.0/models" operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/immutable" - "strings" ) func newImmutableAPI() *immutableAPI { @@ -62,6 +63,15 @@ func (ia *immutableAPI) DeleteImmuRule(ctx context.Context, params operation.Del return ia.SendError(ctx, err) } + projectID, err := ia.getProjectID(ctx, projectNameOrID) + if err != nil { + return ia.SendError(ctx, err) + } + + if err := ia.requireRuleAccess(ctx, projectID, params.ImmutableRuleID); err != nil { + return ia.SendError(ctx, err) + } + if err := ia.immuCtl.DeleteImmutableRule(ctx, params.ImmutableRuleID); err != nil { return ia.SendError(ctx, err) } @@ -86,6 +96,10 @@ func (ia *immutableAPI) UpdateImmuRule(ctx context.Context, params operation.Upd } metadata.ProjectID = projectID + if err = ia.requireRuleAccess(ctx, projectID, metadata.ID); err != nil { + return ia.SendError(ctx, err) + } + if err := ia.immuCtl.UpdateImmutableRule(ctx, projectID, &metadata); err != nil { return ia.SendError(ctx, err) } @@ -146,3 +160,18 @@ func (ia *immutableAPI) getProjectID(ctx context.Context, projectNameOrID interf } return 0, errors.New("unknown project identifier type") } + +// requireRuleAccess checks whether the project has the permission to the +// immutable rule. +func (ia *immutableAPI) requireRuleAccess(ctx context.Context, projectID, metadataID int64) error { + rule, err := ia.immuCtl.GetImmutableRule(ctx, metadataID) + if err != nil { + return err + } + // if input project id does not equal projectID in db return err + if rule.ProjectID != projectID { + return errors.NotFoundError(errors.Errorf("project id %d does not match", projectID)) + } + + return nil +} diff --git a/src/server/v2.0/handler/notification_job.go b/src/server/v2.0/handler/notification_job.go index 7ce31d91d..fa7130a81 100644 --- a/src/server/v2.0/handler/notification_job.go +++ b/src/server/v2.0/handler/notification_job.go @@ -2,10 +2,15 @@ package handler import ( "context" + "github.com/go-openapi/runtime/middleware" "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/pkg" "github.com/goharbor/harbor/src/pkg/notification/job" "github.com/goharbor/harbor/src/pkg/notification/policy" + policyModel "github.com/goharbor/harbor/src/pkg/notification/policy/model" + "github.com/goharbor/harbor/src/pkg/project" "github.com/goharbor/harbor/src/server/v2.0/handler/model" "github.com/goharbor/harbor/src/server/v2.0/models" "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/webhookjob" @@ -16,6 +21,7 @@ func newNotificationJobAPI() *notificationJobAPI { return ¬ificationJobAPI{ webhookjobMgr: job.Mgr, webhookPolicyMgr: policy.Mgr, + projectMgr: pkg.ProjectMgr, } } @@ -23,6 +29,7 @@ type notificationJobAPI struct { BaseAPI webhookjobMgr job.Manager webhookPolicyMgr policy.Manager + projectMgr project.Manager } func (n *notificationJobAPI) ListWebhookJobs(ctx context.Context, params webhookjob.ListWebhookJobsParams) middleware.Responder { @@ -36,6 +43,10 @@ func (n *notificationJobAPI) ListWebhookJobs(ctx context.Context, params webhook return n.SendError(ctx, err) } + if err := n.requirePolicyAccess(ctx, projectNameOrID, policy); err != nil { + return n.SendError(ctx, err) + } + query, err := n.BuildQuery(ctx, params.Q, params.Sort, params.Page, params.PageSize) if err != nil { return n.SendError(ctx, err) @@ -65,3 +76,17 @@ func (n *notificationJobAPI) ListWebhookJobs(ctx context.Context, params webhook WithLink(n.Links(ctx, params.HTTPRequest.URL, total, query.PageNumber, query.PageSize).String()). WithPayload(results) } + +// requirePolicyAccess checks whether the project has the permission to the policy. +func (n *notificationJobAPI) requirePolicyAccess(ctx context.Context, projectNameIrID interface{}, policy *policyModel.Policy) error { + p, err := n.projectMgr.Get(ctx, projectNameIrID) + if err != nil { + return err + } + // check the projectID whether match with the projectID in policy + if p.ProjectID != policy.ProjectID { + return errors.NotFoundError(errors.Errorf("project id %d does not match", p.ProjectID)) + } + + return nil +} diff --git a/src/server/v2.0/handler/preheat.go b/src/server/v2.0/handler/preheat.go index 9c36ae38e..2c200f3db 100644 --- a/src/server/v2.0/handler/preheat.go +++ b/src/server/v2.0/handler/preheat.go @@ -278,6 +278,17 @@ func (api *preheatAPI) UpdatePolicy(ctx context.Context, params operation.Update return api.SendError(ctx, err) } + project, err := api.projectCtl.GetByName(ctx, params.ProjectName) + if err != nil { + return api.SendError(ctx, err) + } + // override project ID + policy.ProjectID = project.ProjectID + + if err := api.requirePolicyAccess(ctx, policy); err != nil { + return api.SendError(ctx, err) + } + err = api.preheatCtl.UpdatePolicy(ctx, policy) if err != nil { return api.SendError(ctx, err) @@ -836,3 +847,19 @@ func (api *preheatAPI) ListProvidersUnderProject(ctx context.Context, params ope return operation.NewListProvidersUnderProjectOK().WithPayload(providers) } + +// requirePolicyAccess checks the project whether has the permission to the policy. +func (api *preheatAPI) requirePolicyAccess(ctx context.Context, policy *policy.Schema) error { + if policy != nil && policy.ID != 0 { + db, err := api.preheatCtl.GetPolicy(ctx, policy.ID) + if err != nil { + return err + } + // return err if project id does not match + if db.ProjectID != policy.ProjectID { + return errors.NotFoundError(errors.Errorf("project id %d does not match", policy.ProjectID)) + } + } + + return nil +} diff --git a/src/server/v2.0/handler/retention.go b/src/server/v2.0/handler/retention.go index 12d896bf5..30925f0fe 100644 --- a/src/server/v2.0/handler/retention.go +++ b/src/server/v2.0/handler/retention.go @@ -202,12 +202,16 @@ func (r *retentionAPI) UpdateRetention(ctx context.Context, params operation.Upd if err := r.checkRuleConflict(p); err != nil { return r.SendError(ctx, errors.ConflictError(err)) } - err := r.requireAccess(ctx, p, rbac.ActionUpdate) - if err != nil { + + if err := r.requireAccess(ctx, p, rbac.ActionUpdate); err != nil { return r.SendError(ctx, err) } - if err = r.retentionCtl.UpdateRetention(ctx, p); err != nil { + if err := r.requirePolicyAccess(ctx, p); err != nil { + return r.SendError(ctx, err) + } + + if err := r.retentionCtl.UpdateRetention(ctx, p); err != nil { return r.SendError(ctx, err) } return operation.NewUpdateRetentionOK() @@ -368,3 +372,22 @@ func (r *retentionAPI) requireAccess(ctx context.Context, p *policy.Metadata, ac } return r.RequireSystemAccess(ctx, action, rbac.ResourceTagRetention) } + +// requirePolicyAccess checks the scope reference whether has the permission to +// the retention policy. +func (r *retentionAPI) requirePolicyAccess(ctx context.Context, p *policy.Metadata) error { + // the id of policy should be consistent with project metadata + meta, err := r.proMetaMgr.Get(ctx, p.Scope.Reference, "retention_id") + if err != nil { + return err + } + // validate + if len(meta["retention_id"]) > 0 { + // return err if retention id does not match + if meta["retention_id"] != fmt.Sprintf("%d", p.ID) { + return errors.NotFoundError(errors.Errorf("the retention policy id %d does not match", p.ID)) + } + } + + return nil +}