Merge pull request #48 from Dewberry/error-discard-placeholder-obj

Discard 0 byte placeholder object when listing
Dewberry · Jul 15, 2024 · 7ad3820 · 7ad3820
2 parents 2726603 + b22b07f
commit 7ad3820
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 10 deletions.
diff --git 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/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()

diff --git 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")

diff --git 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 {

diff --git 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") {
@@ -190,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)
@@ -205,6 +227,7 @@ func (bh *BlobHandler) HandleDeleteObjectsByList(c echo.Context) error {
 			return c.JSON(http.StatusNotFound, errMsg.Error())
 		}
 
+		key := aws.String(s3Path)
 		keys = append(keys, *key)
 	}
 

diff --git 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,

diff --git 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...))
@@ -100,11 +100,11 @@ 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
-	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...))