(cherry-pick)fix: revise the process of policy update (#17058)

fix: revise the process of policy update

Signed-off-by: chlins <chenyuzh@vmware.com>
This commit is contained in:
Chenyu Zhang 2022-06-22 23:05:04 +08:00 committed by GitHub
parent 87f58abba3
commit ce18afaf8b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 108 additions and 5 deletions

View File

@ -2,18 +2,19 @@ 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/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 {
@ -59,6 +60,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)
}
@ -81,6 +91,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)
}
@ -141,3 +155,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
}

View File

@ -2,10 +2,14 @@ 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/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 +20,7 @@ func newNotificationJobAPI() *notificationJobAPI {
return &notificationJobAPI{
webhookjobMgr: job.Mgr,
webhookPolicyMgr: policy.Mgr,
projectMgr: project.Mgr,
}
}
@ -23,6 +28,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 +42,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 +75,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
}

View File

@ -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
}

View File

@ -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
}