From 342db7450f7a478b151efbe8eb0f61e31722c15c Mon Sep 17 00:00:00 2001 From: Tamar Kalir Date: Wed, 20 Aug 2025 09:44:12 +0300 Subject: [PATCH] migrating branch operation endpoints to use PermissionDescriptor --- pkg/api/controller.go | 79 ++++++----------------------------- pkg/permissions/permission.go | 31 ++++++++++++++ 2 files changed, 43 insertions(+), 67 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index ce2ecc879be..4e69d6603b0 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -2866,12 +2866,7 @@ func (c *Controller) ListBranches(w http.ResponseWriter, r *http.Request, reposi } func (c *Controller) CreateBranch(w http.ResponseWriter, r *http.Request, body apigen.CreateBranchJSONRequestBody, repository string) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.CreateBranchAction, - Resource: permissions.BranchArn(repository, body.Name), - }, - }) { + if !c.authorizeReq(w, r, "CreateBranch", permissions.PermissionParams{Repository: &repository, Branch: &body.Name}, nil) { return } ctx := r.Context() @@ -2893,12 +2888,7 @@ func (c *Controller) CreateBranch(w http.ResponseWriter, r *http.Request, body a } func (c *Controller) DeleteBranch(w http.ResponseWriter, r *http.Request, repository, branch string, body apigen.DeleteBranchParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.DeleteBranchAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "DeleteBranch", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -2912,12 +2902,7 @@ func (c *Controller) DeleteBranch(w http.ResponseWriter, r *http.Request, reposi } func (c *Controller) GetBranch(w http.ResponseWriter, r *http.Request, repository, branch string) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadBranchAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "GetBranch", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -3047,12 +3032,7 @@ func (c *Controller) handleAPIError(ctx context.Context, w http.ResponseWriter, } func (c *Controller) ResetBranch(w http.ResponseWriter, r *http.Request, body apigen.ResetBranchJSONRequestBody, repository, branch string) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.RevertBranchAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "ResetBranch", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -3080,13 +3060,8 @@ func (c *Controller) ResetBranch(w http.ResponseWriter, r *http.Request, body ap } func (c *Controller) HardResetBranch(w http.ResponseWriter, r *http.Request, repository, branch string, params apigen.HardResetBranchParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - // TODO(ozkatz): Can we have another action here? - Action: permissions.RevertBranchAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + // TODO(ozkatz): Can we have another action here? + if !c.authorizeReq(w, r, "HardResetBranch", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -3211,12 +3186,7 @@ func importStatusToResponse(status *graveler.ImportStatus) apigen.ImportStatus { } func (c *Controller) ImportStatus(w http.ResponseWriter, r *http.Request, repository, branch string, params apigen.ImportStatusParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadBranchAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "ImportStatus", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -3230,12 +3200,7 @@ func (c *Controller) ImportStatus(w http.ResponseWriter, r *http.Request, reposi } func (c *Controller) ImportCancel(w http.ResponseWriter, r *http.Request, repository, branch string, params apigen.ImportCancelParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ImportCancelAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "ImportCancel", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -3249,12 +3214,7 @@ func (c *Controller) ImportCancel(w http.ResponseWriter, r *http.Request, reposi } func (c *Controller) Commit(w http.ResponseWriter, r *http.Request, body apigen.CommitJSONRequestBody, repository, branch string, params apigen.CommitParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.CreateCommitAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "Commit", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -3708,12 +3668,7 @@ func (c *Controller) CopyObject(w http.ResponseWriter, r *http.Request, body api } func (c *Controller) RevertBranch(w http.ResponseWriter, r *http.Request, body apigen.RevertBranchJSONRequestBody, repository, branch string) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.RevertBranchAction, - Resource: permissions.BranchArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "RevertBranch", permissions.PermissionParams{Repository: &repository, Branch: &branch}, nil) { return } ctx := r.Context() @@ -4516,12 +4471,7 @@ func (c *Controller) DiffRefs(w http.ResponseWriter, r *http.Request, repository } func (c *Controller) LogCommits(w http.ResponseWriter, r *http.Request, repository, ref string, params apigen.LogCommitsParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadBranchAction, - Resource: permissions.BranchArn(repository, ref), - }, - }) { + if !c.authorizeReq(w, r, "LogCommits", permissions.PermissionParams{Repository: &repository, Branch: &ref}, nil) { return } ctx := r.Context() @@ -5038,12 +4988,7 @@ func (c *Controller) GetUnderlyingProperties(w http.ResponseWriter, r *http.Requ } func (c *Controller) MergeIntoBranch(w http.ResponseWriter, r *http.Request, body apigen.MergeIntoBranchJSONRequestBody, repository, sourceRef, destinationBranch string) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.CreateCommitAction, - Resource: permissions.BranchArn(repository, destinationBranch), - }, - }) { + if !c.authorizeReq(w, r, "MergeIntoBranch", permissions.PermissionParams{Repository: &repository, Branch: &destinationBranch}, nil) { return } ctx := r.Context() diff --git a/pkg/permissions/permission.go b/pkg/permissions/permission.go index c87e0718567..14e9217cab8 100644 --- a/pkg/permissions/permission.go +++ b/pkg/permissions/permission.go @@ -65,6 +65,7 @@ func ExternalPrincipalArn(principalID string) string { type PermissionParams struct { Repository *string Path *string + Branch *string } type PermissionDescriptor interface { @@ -84,8 +85,27 @@ func (o *ObjectPermission) Permission(params PermissionParams) Node { } } +type BranchPermission struct { + Action string +} + +func (o *BranchPermission) Permission(params PermissionParams) Node { + return Node{ + Permission: Permission{ + Action: o.Action, + Resource: ObjectArn(*params.Repository, *params.Branch), + }, + } +} + var readObjectPermission = ObjectPermission{Action: ReadObjectAction} var writeObjectPermission = ObjectPermission{Action: WriteObjectAction} +var createBranchPermission = BranchPermission{Action: CreateBranchAction} +var deleteBranchPermission = BranchPermission{Action: DeleteBranchAction} +var readBranchPermission = BranchPermission{Action: ReadBranchAction} +var revertBranchPermission = BranchPermission{Action: RevertBranchAction} +var createCommitPermission = BranchPermission{Action: CreateCommitAction} +var importCancelPermission = BranchPermission{Action: ImportCancelAction} var permissionByOp = map[string]PermissionDescriptor{ "HeadObject": &readObjectPermission, @@ -97,6 +117,17 @@ var permissionByOp = map[string]PermissionDescriptor{ "UpdateObjectUserMetadata": &writeObjectPermission, "UploadObject": &writeObjectPermission, "UploadObjectPreflight": &writeObjectPermission, + "CreateBranch": &createBranchPermission, + "DeleteBranch": &deleteBranchPermission, + "GetBranch": &readBranchPermission, + "RevertBranch": &revertBranchPermission, + "LogCommits": &readBranchPermission, + "ResetBranch": &revertBranchPermission, + "MergeIntoBranch": &createCommitPermission, + "HardResetBranch": &revertBranchPermission, + "ImportStatus": &readBranchPermission, + "Commit": &createCommitPermission, + "ImportCancel": &importCancelPermission, } func GetPermissionDescriptor(operationId string) PermissionDescriptor {