refactor: update handler methods to use interfaces for admin and S3 services

Signed-off-by: Noooste <83548733+Noooste@users.noreply.github.com>

backend/internal/config/config.go

@@ -244,6 +244,13 @@ func (c *Config) Validate() error {
 		if len(c.Auth.OIDC.Scopes) == 0 {
 			return fmt.Errorf("oidc scopes are required when oidc is enabled")
 		}
+		// Every authenticated route on this service grants full admin
+		// access — there is no separate authorization layer. An empty
+		// admin_role would therefore promote every user in the IdP realm
+		// to cluster admin. Require operators to opt in explicitly.
+		if c.Auth.OIDC.AdminRole == "" {
+			return fmt.Errorf("oidc admin_role is required when oidc is enabled: leaving it empty would grant cluster-admin access to any authenticated IdP user")
+		}
 	}
 
 	return nil

backend/internal/handlers/buckets.go

@@ -7,14 +7,14 @@ import (
 	"github.com/gofiber/fiber/v3"
 )
 
-// BucketHandler handles bucket-related operations
+// BucketHandler handles bucket-related HTTP requests.
 type BucketHandler struct {
-	adminService *services.GarageAdminService
-	s3Service    *services.S3Service
+	adminService services.AdminService
+	s3Service    services.S3Storage
 }
 
-// NewBucketHandler creates a new bucket handler
-func NewBucketHandler(adminService *services.GarageAdminService, s3Service *services.S3Service) *BucketHandler {
+// NewBucketHandler creates a new bucket handler.
+func NewBucketHandler(adminService services.AdminService, s3Service services.S3Storage) *BucketHandler {
 	return &BucketHandler{
 		adminService: adminService,
 		s3Service:    s3Service,

backend/internal/handlers/cluster.go

@@ -7,13 +7,13 @@ import (
 	"github.com/gofiber/fiber/v3"
 )
 
-// ClusterHandler handles cluster management operations
+// ClusterHandler handles cluster-status HTTP requests.
 type ClusterHandler struct {
-	adminService *services.GarageAdminService
+	adminService services.AdminService
 }
 
-// NewClusterHandler creates a new cluster handler
-func NewClusterHandler(adminService *services.GarageAdminService) *ClusterHandler {
+// NewClusterHandler creates a new cluster handler.
+func NewClusterHandler(adminService services.AdminService) *ClusterHandler {
 	return &ClusterHandler{
 		adminService: adminService,
 	}

backend/internal/handlers/monitoring.go

@@ -7,14 +7,14 @@ import (
 	"github.com/gofiber/fiber/v3"
 )
 
-// MonitoringHandler handles monitoring operations
+// MonitoringHandler handles metrics and dashboard HTTP requests.
 type MonitoringHandler struct {
-	adminService *services.GarageAdminService
-	s3Service    *services.S3Service
+	adminService services.AdminService
+	s3Service    services.S3Storage
 }
 
-// NewMonitoringHandler creates a new monitoring handler
-func NewMonitoringHandler(adminService *services.GarageAdminService, s3Service *services.S3Service) *MonitoringHandler {
+// NewMonitoringHandler creates a new monitoring handler.
+func NewMonitoringHandler(adminService services.AdminService, s3Service services.S3Storage) *MonitoringHandler {
 	return &MonitoringHandler{
 		adminService: adminService,
 		s3Service:    s3Service,

backend/internal/handlers/objects.go

@@ -3,7 +3,10 @@ package handlers
 import (
 	"bufio"
 	"io"
+	"net/url"
+	"path"
 	"strconv"
+	"strings"
 	"time"
 
 	"Noooste/garage-ui/internal/models"
@@ -12,13 +15,66 @@ import (
 	"github.com/gofiber/fiber/v3"
 )
 
-// ObjectHandler handles object-related operations
-type ObjectHandler struct {
-	s3Service *services.S3Service
+// unsafeInlineContentTypes are MIME types that a browser can execute as
+// JavaScript in the response's origin when rendered inline. Since the SPA is
+// served from the same origin as the API, any uploader could otherwise plant
+// stored XSS by uploading a file with one of these Content-Types.
+var unsafeInlineContentTypes = map[string]struct{}{
+	"text/html":             {},
+	"application/xhtml+xml": {},
+	"image/svg+xml":         {},
+	"application/xml":       {},
+	"text/xml":              {},
+	"application/javascript": {},
+	"text/javascript":       {},
 }
 
-// NewObjectHandler creates a new object handler
-func NewObjectHandler(s3Service *services.S3Service) *ObjectHandler {
+// safeContentType rewrites Content-Types that the browser would treat as
+// executable to application/octet-stream.
+func safeContentType(ct string) string {
+	base := strings.TrimSpace(strings.ToLower(ct))
+	if i := strings.IndexByte(base, ';'); i >= 0 {
+		base = strings.TrimSpace(base[:i])
+	}
+	if _, bad := unsafeInlineContentTypes[base]; bad {
+		return "application/octet-stream"
+	}
+	return ct
+}
+
+// contentDispositionHeader builds an RFC 6266 / RFC 5987 Content-Disposition
+// header value with the user-controlled object key safely encoded. Strips
+// path components and control characters before emitting the ASCII fallback,
+// then appends the percent-encoded UTF-8 filename*= for full fidelity.
+func contentDispositionHeader(disposition, key string) string {
+	name := path.Base(key)
+	if name == "." || name == "/" || name == "" {
+		name = "download"
+	}
+	// ASCII-safe fallback: drop anything that could break the quoted value.
+	var asciiFallback strings.Builder
+	for _, r := range name {
+		if r < 0x20 || r == 0x7f || r == '"' || r == '\\' || r > 0x7e {
+			asciiFallback.WriteByte('_')
+			continue
+		}
+		asciiFallback.WriteRune(r)
+	}
+	fallback := asciiFallback.String()
+	if fallback == "" {
+		fallback = "download"
+	}
+	encoded := url.PathEscape(name)
+	return disposition + "; filename=\"" + fallback + "\"; filename*=UTF-8''" + encoded
+}
+
+// ObjectHandler handles object-related HTTP requests.
+type ObjectHandler struct {
+	s3Service services.S3Storage
+}
+
+// NewObjectHandler creates a new object handler.
+func NewObjectHandler(s3Service services.S3Storage) *ObjectHandler {
 	return &ObjectHandler{
 		s3Service: s3Service,
 	}
@@ -178,16 +234,22 @@ func (h *ObjectHandler) GetObject(c fiber.Ctx) error {
 		)
 	}
 
-	// Set response headers
-	c.Set("Content-Type", objectInfo.ContentType)
+	// The uploader controls Content-Type. Rewrite executable MIME types to
+	// application/octet-stream and always disable sniffing so stored HTML/SVG
+	// cannot run as XSS in the SPA origin when fetched inline.
+	c.Set("Content-Type", safeContentType(objectInfo.ContentType))
+	c.Set("X-Content-Type-Options", "nosniff")
 	c.Set("Content-Length", strconv.FormatInt(objectInfo.Size, 10))
 	c.Set("ETag", objectInfo.ETag)
 	c.Set("Last-Modified", objectInfo.LastModified.Format(time.RFC1123))
 
-	// Check if client wants to download or view inline
+	// The object key is attacker-controlled — build the header via the safe
+	// RFC 6266 helper to avoid quote/semicolon injection into filename=.
+	disposition := "inline"
 	if c.Query("download") == "true" {
-		c.Set("Content-Disposition", "attachment; filename=\""+key+"\"")
+		disposition = "attachment"
 	}
+	c.Set("Content-Disposition", contentDispositionHeader(disposition, key))
 
 	// Stream the object body to the client without buffering the entire file
 	return c.SendStreamWriter(func(w *bufio.Writer) {

backend/internal/handlers/users.go

@@ -9,13 +9,13 @@ import (
 	"github.com/gofiber/fiber/v3"
 )
 
-// UserHandler handles user/key management operations using Garage Admin API
+// UserHandler handles user and access key HTTP requests.
 type UserHandler struct {
-	adminService *services.GarageAdminService
+	adminService services.AdminService
 }
 
-// NewUserHandler creates a new user handler
-func NewUserHandler(adminService *services.GarageAdminService) *UserHandler {
+// NewUserHandler creates a new user handler.
+func NewUserHandler(adminService services.AdminService) *UserHandler {
 	return &UserHandler{
 		adminService: adminService,
 	}

backend/internal/middleware/cors.go

@@ -1,6 +1,7 @@
 package middleware
 
 import (
+	"strconv"
 	"strings"
 
 	"Noooste/garage-ui/internal/config"
@@ -20,10 +21,14 @@ func CORSMiddleware(cfg *config.CORSConfig) fiber.Handler {
 	return func(c fiber.Ctx) error {
 		origin := c.Get("Origin")
 
-		// Check if origin is allowed
-		if origin != "" && isAllowedOrigin(origin, cfg.AllowedOrigins) {
+		// Check if origin is allowed. When credentials are allowed we refuse
+		// to treat "*" as a match: reflecting an arbitrary Origin alongside
+		// Access-Control-Allow-Credentials: true lets any site read responses
+		// cross-origin with the user's session cookie.
+		if origin != "" && isAllowedOrigin(origin, cfg.AllowedOrigins, cfg.AllowCredentials) {
 			// Set CORS headers
 			c.Set("Access-Control-Allow-Origin", origin)
+			c.Set("Vary", "Origin")
 
 			if cfg.AllowCredentials {
 				c.Set("Access-Control-Allow-Credentials", "true")
@@ -41,7 +46,7 @@ func CORSMiddleware(cfg *config.CORSConfig) fiber.Handler {
 
 			// Set max age for preflight cache
 			if cfg.MaxAge > 0 {
-				c.Set("Access-Control-Max-Age", string(rune(cfg.MaxAge)))
+				c.Set("Access-Control-Max-Age", strconv.Itoa(cfg.MaxAge))
 			}
 		}
 
@@ -54,10 +59,14 @@ func CORSMiddleware(cfg *config.CORSConfig) fiber.Handler {
 	}
 }
 
-// isAllowedOrigin checks if an origin is in the allowed list
-func isAllowedOrigin(origin string, allowedOrigins []string) bool {
+// isAllowedOrigin checks if an origin is in the allowed list.
+// When allowCredentials is true, "*" is NOT honored — exact match is required.
+func isAllowedOrigin(origin string, allowedOrigins []string, allowCredentials bool) bool {
 	for _, allowed := range allowedOrigins {
-		if allowed == "*" || allowed == origin {
+		if allowed == origin {
+			return true
+		}
+		if allowed == "*" && !allowCredentials {
 			return true
 		}
 	}

backend/internal/services/interfaces.go

@@ -0,0 +1,68 @@
+package services
+
+import (
+	"context"
+	"io"
+	"time"
+
+	"Noooste/garage-ui/internal/models"
+)
+
+// AdminService is the set of Garage Admin API operations used by HTTP handlers.
+// It is implemented by *GarageAdminService in admin.go. Kept narrow so that
+// hand-rolled mocks in tests don't need to cover admin methods the handlers
+// never call.
+type AdminService interface {
+	// Access keys
+	ListKeys(ctx context.Context) ([]models.ListKeysResponseItem, error)
+	CreateKey(ctx context.Context, req models.CreateKeyRequest) (*models.GarageKeyInfo, error)
+	GetKeyInfo(ctx context.Context, keyID string, showSecret bool) (*models.GarageKeyInfo, error)
+	UpdateKey(ctx context.Context, keyID string, req models.UpdateKeyRequest) (*models.GarageKeyInfo, error)
+	DeleteKey(ctx context.Context, keyID string) error
+
+	// Buckets
+	ListBuckets(ctx context.Context) ([]models.ListBucketsResponseItem, error)
+	GetBucketInfo(ctx context.Context, bucketID string) (*models.GarageBucketInfo, error)
+	GetBucketInfoByAlias(ctx context.Context, globalAlias string) (*models.GarageBucketInfo, error)
+	CreateBucket(ctx context.Context, req models.CreateBucketAdminRequest) (*models.GarageBucketInfo, error)
+	UpdateBucket(ctx context.Context, bucketID string, req models.UpdateBucketRequest) (*models.GarageBucketInfo, error)
+	DeleteBucket(ctx context.Context, bucketID string) error
+	AllowBucketKey(ctx context.Context, req models.BucketKeyPermRequest) (*models.GarageBucketInfo, error)
+
+	// Cluster
+	GetClusterHealth(ctx context.Context) (*models.ClusterHealth, error)
+	GetClusterStatus(ctx context.Context) (*models.ClusterStatus, error)
+	GetClusterStatistics(ctx context.Context) (*models.ClusterStatistics, error)
+	GetNodeInfo(ctx context.Context, nodeID string) (*models.MultiNodeResponse, error)
+	GetNodeStatistics(ctx context.Context, nodeID string) (*models.MultiNodeResponse, error)
+
+	// Monitoring
+	HealthCheck(ctx context.Context) error
+	GetMetrics(ctx context.Context) (string, error)
+}
+
+// S3Storage is the set of S3 operations used by HTTP handlers. It is
+// implemented by *S3Service in s3.go. Methods on *S3Service that are not
+// called by handlers (ListBuckets, CreateBucket, DeleteBucket,
+// GetBucketStatistics) are intentionally excluded.
+type S3Storage interface {
+	ListObjects(ctx context.Context, bucketName, prefix string, maxKeys int, continuationToken string) (*models.ObjectListResponse, error)
+	UploadObject(ctx context.Context, bucketName, key string, body io.Reader, contentType string) (*models.ObjectUploadResponse, error)
+	GetObject(ctx context.Context, bucketName, key string) (io.ReadCloser, *models.ObjectInfo, error)
+	ObjectExists(ctx context.Context, bucketName, key string) (bool, error)
+	DeleteObject(ctx context.Context, bucketName, key string) error
+	GetObjectMetadata(ctx context.Context, bucketName, key string) (*models.ObjectInfo, error)
+	GetPresignedURL(ctx context.Context, bucketName, key string, expiresIn time.Duration) (string, error)
+	DeleteMultipleObjects(ctx context.Context, bucketName string, keys []string) error
+	UploadMultipleObjects(ctx context.Context, bucketName string, files []struct {
+		Key         string
+		Body        io.Reader
+		ContentType string
+	}) []UploadResult
+}
+
+// Compile-time guarantees that the concrete services implement the interfaces.
+var (
+	_ AdminService = (*GarageAdminService)(nil)
+	_ S3Storage    = (*S3Service)(nil)
+)