From da530cf8fde725b63863d80ddbcdb1aa4c0a6061 Mon Sep 17 00:00:00 2001 From: Tamar Kalir Date: Tue, 12 Aug 2025 19:56:07 +0300 Subject: [PATCH 1/2] refactoring object-operation endpoints to use permission descriptors --- pkg/api/controller.go | 85 ++++++++++++----------------------- pkg/permissions/permission.go | 41 +++++++++++++++++ 2 files changed, 70 insertions(+), 56 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 3fa90d6bd81..a73db832c8a 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -111,6 +111,10 @@ type Controller struct { licenseManager license.Manager } +type AuthOpts struct { + callback func(w http.ResponseWriter, r *http.Request, code int, v interface{}) +} + var usageCounter = stats.NewUsageCounter() func NewController(cfg config.Config, catalog *catalog.Catalog, authenticator auth.Authenticator, authService auth.Service, authenticationService authentication.Service, blockAdapter block.Adapter, metadataManager auth.MetadataManager, migrator Migrator, collector stats.Collector, actions actionsHandler, auditChecker AuditChecker, logger logging.Logger, sessionStore sessions.Store, pathProvider upload.PathProvider, usageReporter stats.UsageReporterOperations, licenseManager license.Manager) *Controller { @@ -3377,12 +3381,7 @@ func (c *Controller) DeleteObject(w http.ResponseWriter, r *http.Request, reposi } func (c *Controller) UploadObjectPreflight(w http.ResponseWriter, r *http.Request, repository, branch string, params apigen.UploadObjectPreflightParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.WriteObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "UploadObjectPreflight", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } @@ -3393,12 +3392,7 @@ func (c *Controller) UploadObjectPreflight(w http.ResponseWriter, r *http.Reques } func (c *Controller) UploadObject(w http.ResponseWriter, r *http.Request, repository, branch string, params apigen.UploadObjectParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.WriteObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "UploadObject", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } ctx := r.Context() @@ -3563,12 +3557,7 @@ func (c *Controller) UploadObject(w http.ResponseWriter, r *http.Request, reposi } func (c *Controller) StageObject(w http.ResponseWriter, r *http.Request, body apigen.StageObjectJSONRequestBody, repository, branch string, params apigen.StageObjectParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.WriteObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "StageObject", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } ctx := r.Context() @@ -4383,12 +4372,7 @@ func (c *Controller) RestoreStatus(w http.ResponseWriter, r *http.Request, repos } func (c *Controller) CreateSymlinkFile(w http.ResponseWriter, r *http.Request, repository, branch string, params apigen.CreateSymlinkFileParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.WriteObjectAction, - Resource: permissions.ObjectArn(repository, branch), - }, - }) { + if !c.authorizeReq(w, r, "CreateSymlinkFile", permissions.PermissionParams{Repository: &repository, Path: &branch}, nil) { return } ctx := r.Context() @@ -4582,14 +4566,10 @@ func (c *Controller) LogCommits(w http.ResponseWriter, r *http.Request, reposito } func (c *Controller) HeadObject(w http.ResponseWriter, r *http.Request, repository, ref string, params apigen.HeadObjectParams) { - if !c.authorizeCallback(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }, func(w http.ResponseWriter, r *http.Request, code int, v interface{}) { - writeResponse(w, r, code, nil) - }) { + if !c.authorizeReq(w, r, "HeadObject", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, + &AuthOpts{callback: func(w http.ResponseWriter, r *http.Request, code int, v interface{}) { + writeResponse(w, r, code, nil) + }}) { return } ctx := r.Context() @@ -4724,12 +4704,7 @@ func (c *Controller) GetMetadataObject(w http.ResponseWriter, r *http.Request, r } func (c *Controller) GetObject(w http.ResponseWriter, r *http.Request, repository, ref string, params apigen.GetObjectParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "GetObject", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } ctx := r.Context() @@ -4947,12 +4922,7 @@ func (c *Controller) ListObjects(w http.ResponseWriter, r *http.Request, reposit } func (c *Controller) StatObject(w http.ResponseWriter, r *http.Request, repository, ref string, params apigen.StatObjectParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "StatObject", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } ctx := r.Context() @@ -5015,12 +4985,7 @@ func (c *Controller) StatObject(w http.ResponseWriter, r *http.Request, reposito } func (c *Controller) UpdateObjectUserMetadata(w http.ResponseWriter, r *http.Request, body apigen.UpdateObjectUserMetadataJSONRequestBody, repository, branch string, params apigen.UpdateObjectUserMetadataParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.WriteObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "UpdateObjectUserMetadata", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } ctx := r.Context() @@ -5036,12 +5001,7 @@ func (c *Controller) UpdateObjectUserMetadata(w http.ResponseWriter, r *http.Req } func (c *Controller) GetUnderlyingProperties(w http.ResponseWriter, r *http.Request, repository, ref string, params apigen.GetUnderlyingPropertiesParams) { - if !c.authorize(w, r, permissions.Node{ - Permission: permissions.Permission{ - Action: permissions.ReadObjectAction, - Resource: permissions.ObjectArn(repository, params.Path), - }, - }) { + if !c.authorizeReq(w, r, "GetUnderlyingProperties", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, nil) { return } ctx := r.Context() @@ -5944,6 +5904,19 @@ func (c *Controller) authorize(w http.ResponseWriter, r *http.Request, perms per return c.authorizeCallback(w, r, perms, writeError) } +func (c *Controller) authorizeReq(w http.ResponseWriter, r *http.Request, operationId string, params permissions.PermissionParams, opts *AuthOpts) bool { + desc := permissions.GetPermissionDescriptor(operationId) + if desc == nil { + c.Logger.Error(fmt.Sprintf("missing permission descriptor for %s", operationId)) + return false + } + callback := writeError + if opts != nil && opts.callback != nil { + callback = opts.callback + } + return c.authorizeCallback(w, r, desc.Permission(params), callback) +} + func (c *Controller) isNameValid(name, nameType string) (bool, string) { // URLs are % encoded. Allowing % signs in entity names would // limit the ability to use these entity names in the URL for both diff --git a/pkg/permissions/permission.go b/pkg/permissions/permission.go index 5035db76309..c87e0718567 100644 --- a/pkg/permissions/permission.go +++ b/pkg/permissions/permission.go @@ -61,3 +61,44 @@ func PolicyArn(policyID string) string { func ExternalPrincipalArn(principalID string) string { return authArnPrefix + "externalPrincipal/" + principalID } + +type PermissionParams struct { + Repository *string + Path *string +} + +type PermissionDescriptor interface { + Permission(PermissionParams) Node +} + +type ObjectPermission struct { + Action string +} + +func (o *ObjectPermission) Permission(params PermissionParams) Node { + return Node{ + Permission: Permission{ + Action: o.Action, + Resource: ObjectArn(*params.Repository, *params.Path), + }, + } +} + +var readObjectPermission = ObjectPermission{Action: ReadObjectAction} +var writeObjectPermission = ObjectPermission{Action: WriteObjectAction} + +var permissionByOp = map[string]PermissionDescriptor{ + "HeadObject": &readObjectPermission, + "GetObject": &readObjectPermission, + "StatObject": &readObjectPermission, + "GetUnderlyingProperties": &readObjectPermission, + "StageObject": &writeObjectPermission, + "CreateSymlinkFile": &writeObjectPermission, + "UpdateObjectUserMetadata": &writeObjectPermission, + "UploadObject": &writeObjectPermission, + "UploadObjectPreflight": &writeObjectPermission, +} + +func GetPermissionDescriptor(operationId string) PermissionDescriptor { + return permissionByOp[operationId] +} From db86f41fcec9e7f5ae6c56794e9fa443d61aa1d8 Mon Sep 17 00:00:00 2001 From: Tamar Kalir Date: Wed, 13 Aug 2025 18:37:40 +0300 Subject: [PATCH 2/2] changing callback -> onFailure in AuthOpts and authorizeCallback --- pkg/api/controller.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index a73db832c8a..ce2ecc879be 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -112,7 +112,8 @@ type Controller struct { } type AuthOpts struct { - callback func(w http.ResponseWriter, r *http.Request, code int, v interface{}) + // this function is called in authorizeCallback when authorization fails + onFailure func(w http.ResponseWriter, r *http.Request, httpStatusCode int, error interface{}) } var usageCounter = stats.NewUsageCounter() @@ -4567,8 +4568,8 @@ func (c *Controller) LogCommits(w http.ResponseWriter, r *http.Request, reposito func (c *Controller) HeadObject(w http.ResponseWriter, r *http.Request, repository, ref string, params apigen.HeadObjectParams) { if !c.authorizeReq(w, r, "HeadObject", permissions.PermissionParams{Repository: &repository, Path: ¶ms.Path}, - &AuthOpts{callback: func(w http.ResponseWriter, r *http.Request, code int, v interface{}) { - writeResponse(w, r, code, nil) + &AuthOpts{onFailure: func(w http.ResponseWriter, r *http.Request, httpStatusCode int, error interface{}) { + writeResponse(w, r, httpStatusCode, nil) }}) { return } @@ -5874,11 +5875,11 @@ func paginationFor(hasMore bool, results interface{}, fieldName string) apigen.P return pagination } -func (c *Controller) authorizeCallback(w http.ResponseWriter, r *http.Request, perms permissions.Node, cb func(w http.ResponseWriter, r *http.Request, code int, v interface{})) bool { +func (c *Controller) authorizeCallback(w http.ResponseWriter, r *http.Request, perms permissions.Node, onFailure func(w http.ResponseWriter, r *http.Request, httpStatusCode int, error interface{})) bool { ctx := r.Context() user, err := auth.GetUser(ctx) if err != nil { - cb(w, r, http.StatusUnauthorized, ErrAuthenticatingRequest) + onFailure(w, r, http.StatusUnauthorized, ErrAuthenticatingRequest) return false } resp, err := c.Auth.Authorize(ctx, &auth.AuthorizationRequest{ @@ -5886,15 +5887,15 @@ func (c *Controller) authorizeCallback(w http.ResponseWriter, r *http.Request, p RequiredPermissions: perms, }) if err != nil { - cb(w, r, http.StatusInternalServerError, err) + onFailure(w, r, http.StatusInternalServerError, err) return false } if resp.Error != nil { - cb(w, r, http.StatusUnauthorized, resp.Error) + onFailure(w, r, http.StatusUnauthorized, resp.Error) return false } if !resp.Allowed { - cb(w, r, http.StatusInternalServerError, "User does not have the required permissions") + onFailure(w, r, http.StatusInternalServerError, "User does not have the required permissions") return false } return true @@ -5910,11 +5911,11 @@ func (c *Controller) authorizeReq(w http.ResponseWriter, r *http.Request, operat c.Logger.Error(fmt.Sprintf("missing permission descriptor for %s", operationId)) return false } - callback := writeError - if opts != nil && opts.callback != nil { - callback = opts.callback + onFailure := writeError + if opts != nil && opts.onFailure != nil { + onFailure = opts.onFailure } - return c.authorizeCallback(w, r, desc.Permission(params), callback) + return c.authorizeCallback(w, r, desc.Permission(params), onFailure) } func (c *Controller) isNameValid(name, nameType string) (bool, string) {