From 36c6dcdac9ac8b10c5d4f29d9b5f40b3f2fb7932 Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Tue, 2 Jul 2024 12:43:05 -0400 Subject: [PATCH 1/8] discard 0 byte placeholder object when listing --- blobstore/list.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/blobstore/list.go b/blobstore/list.go index 5f6906b..1f9002b 100644 --- a/blobstore/list.go +++ b/blobstore/list.go @@ -104,6 +104,10 @@ func (bh *BlobHandler) HandleListByPrefix(c echo.Context) error { } for _, object := range page.Contents { // Handle files + // Skip zero-byte objects that match a common prefix with a trailing slash + if *object.Size == 0 && strings.HasSuffix(*object.Key, "/") { + continue + } if fullAccess || IsPermittedPrefix(bucket, *object.Key, permissions) { result = append(result, aws.StringValue(object.Key)) } @@ -185,6 +189,10 @@ func (bh *BlobHandler) HandleListByPrefixWithDetail(c echo.Context) error { for _, object := range page.Contents { // Handle files + // Skip zero-byte objects that match a common prefix with a trailing slash + if *object.Size == 0 && strings.HasSuffix(*object.Key, "/") { + continue + } if fullAccess || IsPermittedPrefix(bucket, *object.Key, permissions) { file := ListResult{ ID: count, From f041ec738670618794fabadabb365f090f07eeee Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Tue, 2 Jul 2024 15:52:47 -0400 Subject: [PATCH 2/8] always return true if the user is s3_admin or writer --- auth/auth.go | 4 ++-- blobstore/blobhandler.go | 11 ++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 314e866..3cee05a 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -123,7 +123,7 @@ func validateToken(tokenString string) (*Claims, error) { return claims, nil } -func overlap(s1 []string, s2 []string) bool { +func Overlap(s1 []string, s2 []string) bool { for _, x := range s1 { for _, y := range s2 { if x == y { @@ -164,7 +164,7 @@ func Authorize(handler echo.HandlerFunc, allowedRoles ...string) echo.HandlerFun // Store the claims in the echo.Context c.Set("claims", claims) - ok := overlap(claims.RealmAccess["roles"], allowedRoles) + ok := Overlap(claims.RealmAccess["roles"], allowedRoles) if !ok { return c.JSON(http.StatusUnauthorized, "user is not authorized") } diff --git a/blobstore/blobhandler.go b/blobstore/blobhandler.go index 15b2c99..c39c029 100644 --- a/blobstore/blobhandler.go +++ b/blobstore/blobhandler.go @@ -335,15 +335,20 @@ func (bh *BlobHandler) HandleCheckS3UserPermission(c echo.Context) error { log.Error(errMsg.Error()) return c.JSON(http.StatusForbidden, errMsg.Error()) } - prefix := c.QueryParam("prefix") - bucket := c.QueryParam("bucket") - operation := c.QueryParam("operation") claims, ok := c.Get("claims").(*auth.Claims) if !ok { errMsg := fmt.Errorf("could not get claims from request context") log.Error(errMsg.Error()) return c.JSON(http.StatusInternalServerError, errMsg.Error()) } + isAdminOrWriter := auth.Overlap(claims.RealmAccess["roles"], []string{"s3_admin", "s3_writer"}) + if isAdminOrWriter { + return c.JSON(http.StatusOK, true) + } + prefix := c.QueryParam("prefix") + bucket := c.QueryParam("bucket") + operation := c.QueryParam("operation") + userEmail := claims.Email if operation == "" || prefix == "" || bucket == "" { errMsg := fmt.Errorf("`prefix`, `operation` and 'bucket are required params") From bf79803c94a0b6622916c2b1c6f39773d915dafd Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Wed, 3 Jul 2024 23:52:48 -0400 Subject: [PATCH 3/8] Allow writers to delete objects --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index b2b3c94..f206c2f 100644 --- a/main.go +++ b/main.go @@ -88,7 +88,7 @@ func main() { e.PUT("/object/move", auth.Authorize(bh.HandleMoveObject, admin...)) e.GET("/object/download", auth.Authorize(bh.HandleGetPresignedDownloadURL, allUsers...)) e.POST("/object/upload", auth.Authorize(bh.HandleMultipartUpload, writers...)) //deprecated by presigned upload URL - e.DELETE("/object/delete", auth.Authorize(bh.HandleDeleteObject, admin...)) + e.DELETE("/object/delete", auth.Authorize(bh.HandleDeleteObject, writers...)) e.GET("/object/exists", auth.Authorize(bh.HandleGetObjExist, allUsers...)) e.GET("/object/presigned_upload", auth.Authorize(bh.HandleGetPresignedUploadURL, writers...)) e.GET("/object/multipart_upload_id", auth.Authorize(bh.HandleGetMultipartUploadID, writers...)) @@ -104,7 +104,7 @@ func main() { e.GET("/prefix/size", auth.Authorize(bh.HandleGetSize, allUsers...)) // universal - e.DELETE("/delete_keys", auth.Authorize(bh.HandleDeleteObjectsByList, admin...)) + e.DELETE("/delete_keys", auth.Authorize(bh.HandleDeleteObjectsByList, writers...)) // multi-bucket e.GET("/list_buckets", auth.Authorize(bh.HandleListBuckets, allUsers...)) From 4a6edf45459e11a2da6bd0815ec6f94c6431c0a7 Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Fri, 5 Jul 2024 11:53:02 -0400 Subject: [PATCH 4/8] allow writers to delete prefixes --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index f206c2f..2423299 100644 --- a/main.go +++ b/main.go @@ -100,7 +100,7 @@ func main() { // e.GET("/prefix/download", auth.Authorize(bh.HandleGetPresignedURLMultiObj, allUsers...)) e.GET("/prefix/download/script", auth.Authorize(bh.HandleGenerateDownloadScript, allUsers...)) e.PUT("/prefix/move", auth.Authorize(bh.HandleMovePrefix, admin...)) - e.DELETE("/prefix/delete", auth.Authorize(bh.HandleDeletePrefix, admin...)) + e.DELETE("/prefix/delete", auth.Authorize(bh.HandleDeletePrefix, writers...)) e.GET("/prefix/size", auth.Authorize(bh.HandleGetSize, allUsers...)) // universal From 9759eebbf076dc98bfcd5f6d0a4b39072c9b440c Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Fri, 5 Jul 2024 14:41:41 -0400 Subject: [PATCH 5/8] point to controller so that it updates with new bucket lists --- blobstore/buckets.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/blobstore/buckets.go b/blobstore/buckets.go index e813656..9ddff6f 100644 --- a/blobstore/buckets.go +++ b/blobstore/buckets.go @@ -93,7 +93,8 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error { return c.JSON(http.StatusInternalServerError, fmt.Errorf("error fetching user permissions: %s", err.Error())) } - for _, controller := range bh.S3Controllers { + for i := range bh.S3Controllers { + controller := &bh.S3Controllers[i] if bh.AllowAllBuckets { result, err := controller.ListBuckets() if err != nil { From 0242afbfd0b23bf976fdf7083efd7b8b2586bfbf Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Thu, 11 Jul 2024 16:27:44 -0400 Subject: [PATCH 6/8] add ablity to add rows to permission table --- auth/database.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/auth/database.go b/auth/database.go index bd1d929..770a655 100644 --- a/auth/database.go +++ b/auth/database.go @@ -15,6 +15,7 @@ type Database interface { CheckUserPermission(userEmail, bucket, prefix string, operations []string) bool Close() error GetUserAccessiblePrefixes(userEmail, bucket string, operations []string) ([]string, error) + AddBucketPermissions(userEmail, bucket string, prefixes []string, operation string) error } type PostgresDB struct { @@ -123,6 +124,26 @@ func (db *PostgresDB) CheckUserPermission(userEmail, bucket, prefix string, oper return hasPermission } +// AddBucketPermissions adds permissions for a user to access specific prefixes in a bucket for a given operation. +func (db *PostgresDB) AddBucketPermissions(userEmail, bucket string, prefixes []string, operation string) error { + allowedPrefixes := make([]string, len(prefixes)) + for i, prefix := range prefixes { + allowedPrefixes[i] = fmt.Sprintf("/%s/%s", bucket, prefix) + } + + query := ` + INSERT INTO permissions (user_email, operation, allowed_s3_prefixes) + VALUES ($1, $2, $3); + ` + + _, err := db.Handle.Exec(query, userEmail, operation, pq.Array(allowedPrefixes)) + if err != nil { + return fmt.Errorf("error adding bucket permissions: %v", err) + } + + return nil +} + // Close closes the database connection. func (db *PostgresDB) Close() error { return db.Handle.Close() From fcd09bd801fcfbc573eaa4160a697f65c08c5f72 Mon Sep 17 00:00:00 2001 From: Anton Kopti Date: Sat, 13 Jul 2024 18:04:47 -0400 Subject: [PATCH 7/8] add user permission check --- blobstore/delete.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/blobstore/delete.go b/blobstore/delete.go index 47fafe1..fb85d5f 100644 --- a/blobstore/delete.go +++ b/blobstore/delete.go @@ -73,6 +73,13 @@ func (bh *BlobHandler) HandleDeleteObject(c echo.Context) error { return c.JSON(http.StatusUnprocessableEntity, errMsg.Error()) } + httpCode, err := bh.CheckUserS3Permission(c, bucket, key, []string{"write"}) + if err != nil { + errMsg := fmt.Errorf("error while checking for user permission: %s", err) + log.Error(errMsg.Error()) + return c.JSON(httpCode, errMsg.Error()) + } + // If the key is not a folder, proceed with deleting a single object keyExist, err := s3Ctrl.KeyExists(bucket, key) if err != nil { @@ -103,6 +110,7 @@ func (bh *BlobHandler) HandleDeleteObject(c echo.Context) error { } func (bh *BlobHandler) HandleDeletePrefix(c echo.Context) error { + bucket := c.QueryParam("bucket") s3Ctrl, err := bh.GetController(bucket) if err != nil { @@ -119,6 +127,14 @@ func (bh *BlobHandler) HandleDeletePrefix(c echo.Context) error { if !strings.HasSuffix(prefix, "/") { prefix = prefix + "/" } + + httpCode, err := bh.CheckUserS3Permission(c, bucket, prefix, []string{"write"}) + if err != nil { + errMsg := fmt.Errorf("error while checking for user permission: %s", err) + log.Error(errMsg.Error()) + return c.JSON(httpCode, errMsg.Error()) + } + err = s3Ctrl.RecursivelyDeleteObjects(bucket, prefix) if err != nil { if strings.Contains(err.Error(), "prefix not found") { @@ -205,6 +221,13 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error { return c.JSON(http.StatusNotFound, errMsg.Error()) } + httpCode, err := bh.CheckUserS3Permission(c, bucket, s3Path, []string{"write"}) + if err != nil { + errMsg := fmt.Errorf("error while checking for user permission: %s", err) + log.Error(errMsg.Error()) + return c.JSON(httpCode, errMsg.Error()) + } + keys = append(keys, *key) } From b22b07f5d791881c4d74bf4a50180fb7ae059f1d Mon Sep 17 00:00:00 2001 From: ShaneMPutnam Date: Mon, 15 Jul 2024 12:24:07 +0000 Subject: [PATCH 8/8] Check permissions before checking if key exists for consistency --- blobstore/delete.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/blobstore/delete.go b/blobstore/delete.go index fb85d5f..f74f34d 100644 --- a/blobstore/delete.go +++ b/blobstore/delete.go @@ -206,7 +206,13 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error { keys := make([]string, 0, len(deleteRequest.Keys)) for _, p := range deleteRequest.Keys { s3Path := strings.TrimPrefix(p, "/") - key := aws.String(s3Path) + + httpCode, err := bh.CheckUserS3Permission(c, bucket, s3Path, []string{"write"}) + if err != nil { + errMsg := fmt.Errorf("error while checking for user permission: %s", err) + log.Error(errMsg.Error()) + return c.JSON(httpCode, errMsg.Error()) + } // Check if the key exists before appending it to the keys list keyExists, err := s3Ctrl.KeyExists(bucket, s3Path) @@ -221,13 +227,7 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error { return c.JSON(http.StatusNotFound, errMsg.Error()) } - httpCode, err := bh.CheckUserS3Permission(c, bucket, s3Path, []string{"write"}) - if err != nil { - errMsg := fmt.Errorf("error while checking for user permission: %s", err) - log.Error(errMsg.Error()) - return c.JSON(httpCode, errMsg.Error()) - } - + key := aws.String(s3Path) keys = append(keys, *key) }