bypass rbac for signed url upload (#2395)

* signed url change

* add handler for signed urls

* restore function call

Author: breardon2011

taco/cmd/statesman/main.go

@@ -201,12 +201,13 @@ func main() {
 
 	// Register routes with interface-based dependencies
 	api.RegisterRoutes(e, api.Dependencies{
-		Repository:  fullRepo,      // RBAC-wrapped repository (used by all routes)
-		BlobStore:   blobStore,     // Direct blob access (for legacy components)
-		QueryStore:  queryStore,    // Direct query access
-		RBACManager: rbacManager,   // RBAC management
-		Signer:      signer,        // JWT signing
-		AuthEnabled: !*authDisable, // Auth flag
+		Repository:          fullRepo,      // RBAC-wrapped repository (used by authenticated routes)
+		UnwrappedRepository: repo,          // Unwrapped repository (for pre-authorized operations like signed URLs)
+		BlobStore:           blobStore,     // Direct blob access (for legacy components)
+		QueryStore:          queryStore,    // Direct query access
+		RBACManager:         rbacManager,   // RBAC management
+		Signer:              signer,        // JWT signing
+		AuthEnabled:         !*authDisable, // Auth flag
 	})
 
 	// Start server

taco/internal/api/internal.go

@@ -149,7 +149,8 @@ func RegisterInternalRoutes(e *echo.Echo, deps Dependencies) {
 	}
 
 	// Create TFE handler with webhook auth context
-	tfeHandler := tfe.NewTFETokenHandler(authHandler, deps.Repository, deps.BlobStore, deps.RBACManager, tfeIdentifierResolver)
+	// Pass both wrapped (for authenticated calls) and unwrapped (for signed URLs) repositories
+	tfeHandler := tfe.NewTFETokenHandler(authHandler, deps.Repository, deps.UnwrappedRepository, deps.BlobStore, deps.RBACManager, tfeIdentifierResolver)
 
 	// TFE group with webhook auth (for UI pass-through)
 	tfeInternal := e.Group("/internal/tfe/api/v2")

taco/internal/api/routes.go

@@ -29,12 +29,13 @@ import (
 // Dependencies holds all the interface-based dependencies for routes.
 // This uses interface segregation - each handler gets ONLY what it needs.
 type Dependencies struct {
-	Repository  domain.UnitRepository  // RBAC-wrapped repository (used by all routes)
-	BlobStore   storage.UnitStore      // Direct blob access (for legacy components like API tokens)
-	QueryStore  query.Store            // Direct query access (analytics, RBAC)
-	RBACManager *rbac.RBACManager      // RBAC management (RBAC routes only)
-	Signer      *authpkg.Signer        // JWT signing (auth, middleware)
-	AuthEnabled bool                   // Whether auth is enabled
+	Repository          domain.UnitRepository  // RBAC-wrapped repository (used by all routes)
+	UnwrappedRepository domain.UnitRepository  // Unwrapped repository (for pre-authorized operations like signed URLs)
+	BlobStore           storage.UnitStore      // Direct blob access (for legacy components like API tokens)
+	QueryStore          query.Store            // Direct query access (analytics, RBAC)
+	RBACManager         *rbac.RBACManager      // RBAC management (RBAC routes only)
+	Signer              *authpkg.Signer        // JWT signing (auth, middleware)
+	AuthEnabled         bool                   // Whether auth is enabled
 }
 
 // RegisterRoutes registers all API routes with interface-scoped dependencies.
@@ -244,16 +245,17 @@ func RegisterRoutes(e *echo.Echo, deps Dependencies) {
 	v1.DELETE("/rbac/permissions/:id", rbacHandler.DeletePermission)
 	v1.POST("/rbac/test", rbacHandler.TestPermissions)
 
-	// TFE api - inject auth handler, full repository, blob store for tokens, and RBAC dependencies
+	// TFE api - inject auth handler, wrapped & unwrapped repositories, blob store for tokens, and RBAC dependencies
 	// TFE handler scopes to TFEOperations internally but needs blob store for API token storage
+	// Unwrapped repository is used for signed URL operations (pre-authorized, no RBAC checks needed)
 	// Create identifier resolver for org resolution
 	var tfeIdentifierResolver domain.IdentifierResolver
 	if deps.QueryStore != nil {
 		if db := repositories.GetDBFromQueryStore(deps.QueryStore); db != nil {
 			tfeIdentifierResolver = repositories.NewIdentifierResolver(db)
 		}
 	}
-	tfeHandler := tfe.NewTFETokenHandler(authHandler, deps.Repository, deps.BlobStore, deps.RBACManager, tfeIdentifierResolver)
+	tfeHandler := tfe.NewTFETokenHandler(authHandler, deps.Repository, deps.UnwrappedRepository, deps.BlobStore, deps.RBACManager, tfeIdentifierResolver)
 
 	// Create protected TFE group - opaque tokens only
 	tfeGroup := e.Group("/tfe/api/v2")

taco/internal/storage/s3store.go

@@ -293,55 +293,81 @@ func (s *s3Store) Download(ctx context.Context, id string) ([]byte, error) {
 }
 
 func (s *s3Store) Upload(ctx context.Context, id string, data []byte, lockID string) error {
+    fmt.Printf("[S3Store.Upload] START - id=%s, dataLen=%d, lockID=%s\n", id, len(data), lockID)
+    
     meta, err := s.Get(ctx, id)
     if err != nil {
+        fmt.Printf("[S3Store.Upload] Get failed: %v\n", err)
         return err
     }
+    fmt.Printf("[S3Store.Upload] Current meta - Size=%d, Locked=%t\n", meta.Size, meta.Locked)
+    
     // Lock checks
     if lockID != "" && meta.LockInfo != nil && meta.LockInfo.ID != lockID {
+        fmt.Printf("[S3Store.Upload] Lock conflict: provided lockID=%s, current lockID=%s\n", lockID, meta.LockInfo.ID)
         return ErrLockConflict
     }
     if lockID == "" && meta.Locked {
+        fmt.Printf("[S3Store.Upload] Lock conflict: no lockID provided but state is locked\n")
         return ErrLockConflict
     }
 
     // Archive current tfstate if it exists and has content
     if meta.Size > 0 {
+        fmt.Printf("[S3Store.Upload] ARCHIVING: Current state size=%d, creating version...\n", meta.Size)
+        
         // Download current tfstate to get its content for hashing
         currentData, err := s.Download(ctx, id)
         if err != nil {
+            fmt.Printf("[S3Store.Upload] ARCHIVING FAILED: Could not download current state: %v\n", err)
             return fmt.Errorf("failed to read current state for archiving: %w", err)
         }
+        fmt.Printf("[S3Store.Upload] ARCHIVING: Downloaded %d bytes\n", len(currentData))
 
         // Generate version key with hash of current content
         timestamp := time.Now().UTC()
         versionKey := s.versionKeyWithHash(id, timestamp, currentData)
+        fmt.Printf("[S3Store.Upload] ARCHIVING: Version key=%s\n", versionKey)
 
         // Copy current to versioned location
+        sourceKey := s.objKey(id)
+        copySource := fmt.Sprintf("%s/%s", s.bucket, sourceKey)
+        fmt.Printf("[S3Store.Upload] ARCHIVING: CopyObject from=%s to=%s\n", copySource, versionKey)
+        
         _, err = s.client.CopyObject(ctx, &s3.CopyObjectInput{
             Bucket:     &s.bucket,
             Key:        aws.String(versionKey),
-            CopySource: aws.String(fmt.Sprintf("%s/%s", s.bucket, s.objKey(id))),
+            CopySource: aws.String(copySource),
         })
         if err != nil {
+            fmt.Printf("[S3Store.Upload] ARCHIVING FAILED: CopyObject error: %v\n", err)
             return fmt.Errorf("failed to archive current version: %w", err)
         }
+        fmt.Printf("[S3Store.Upload] ARCHIVING SUCCESS: Version created at %s\n", versionKey)
 
         // Clean up old versions after successful archiving
         if err := s.cleanupOldVersions(ctx, id); err != nil {
-            fmt.Printf("Warning: failed to cleanup old versions for %s: %v\n", id, err)
+            fmt.Printf("[S3Store.Upload] Warning: failed to cleanup old versions for %s: %v\n", id, err)
         }
+    } else {
+        fmt.Printf("[S3Store.Upload] SKIPPING ARCHIVE: Current state size is 0 (first upload or empty state)\n")
     }
 
     // Upload new tfstate
+    newKey := s.objKey(id)
+    fmt.Printf("[S3Store.Upload] Uploading new state to key=%s, size=%d bytes\n", newKey, len(data))
+    
     if _, err := s.client.PutObject(ctx, &s3.PutObjectInput{
         Bucket: &s.bucket,
-        Key:    aws.String(s.objKey(id)),
+        Key:    aws.String(newKey),
         Body:   bytes.NewReader(data),
         ContentType: aws.String("application/json"),
     }); err != nil {
+        fmt.Printf("[S3Store.Upload] Upload failed: %v\n", err)
         return err
     }
+    
+    fmt.Printf("[S3Store.Upload] SUCCESS: New state uploaded\n")
     return nil
 }
 

taco/internal/tfe/tfe.go

@@ -10,24 +10,24 @@ import (
 // TfeHandler implements Terraform Cloud/Enterprise API.
 // Uses TFEOperations interface (6 methods) - cannot create, list, delete, or manage versions.
 type TfeHandler struct {
-	authHandler       *auth.Handler
-	stateStore        domain.TFEOperations  // Scoped to TFE operations 
-	rbacManager       *rbac.RBACManager
-    apiTokens         *auth.APITokenManager
+	authHandler        *auth.Handler
+	stateStore         domain.TFEOperations  // RBAC-wrapped for authenticated operations
+	directStateStore   domain.TFEOperations  // Direct access for pre-authorized operations (signed URLs)
+	rbacManager        *rbac.RBACManager
+	apiTokens          *auth.APITokenManager
 	identifierResolver domain.IdentifierResolver // For resolving org external IDs
 }
 
 // NewTFETokenHandler creates a new TFE handler.
-// Accepts full repository for state ops and blob store for API token storage.
-func NewTFETokenHandler(authHandler *auth.Handler, fullRepo domain.UnitRepository, blobStore storage.UnitStore, rbacManager *rbac.RBACManager, identifierResolver domain.IdentifierResolver) *TfeHandler {
-	// Scope to TFE operations for state management (handler only uses Get, Upload, Lock methods)
-	stateStore := domain.TFEOperations(fullRepo)
-	
+// Accepts wrapped (RBAC-enforced) and unwrapped (direct) repositories.
+// The unwrapped repository is used for signed URL operations which are pre-authorized.
+func NewTFETokenHandler(authHandler *auth.Handler, wrappedRepo domain.UnitRepository, unwrappedRepo domain.UnitRepository, blobStore storage.UnitStore, rbacManager *rbac.RBACManager, identifierResolver domain.IdentifierResolver) *TfeHandler {
 	return &TfeHandler{
-		authHandler:       authHandler,
-		stateStore:        stateStore,
-		rbacManager:       rbacManager,
-        apiTokens:         auth.NewAPITokenManagerFromStore(blobStore),  // Use blob store for token storage
+		authHandler:        authHandler,
+		stateStore:         domain.TFEOperations(wrappedRepo),    // Use RBAC wrapper for authenticated calls
+		directStateStore:   domain.TFEOperations(unwrappedRepo),  // Bypass RBAC for signed URLs
+		rbacManager:        rbacManager,
+		apiTokens:          auth.NewAPITokenManagerFromStore(blobStore),
 		identifierResolver: identifierResolver,
 	}
 }

taco/internal/tfe/workspaces.go

@@ -1178,21 +1178,8 @@ func (h *TfeHandler) DownloadStateVersion(c echo.Context) error {
 
 // UploadStateVersion handles PUT /tfe/api/v2/state-versions/:id/upload
 func (h *TfeHandler) UploadStateVersion(c echo.Context) error {
-	fmt.Printf("UploadStateVersion: START - Method=%s, URI=%s\n", c.Request().Method, c.Request().RequestURI)
-
-	// Debug: Check if Authorization header is present
-	authHeader := c.Request().Header.Get("Authorization")
-	fmt.Printf("UploadStateVersion: Authorization header present: %t\n", authHeader != "")
-	if authHeader != "" {
-		// Don't log the full token for security, just whether it looks like a Bearer token
-		fmt.Printf("UploadStateVersion: Authorization header format: %s\n",
-			strings.SplitN(authHeader, " ", 2)[0])
-	}
-
 	stateVersionID := c.Param("id")
-	fmt.Printf("UploadStateVersion: stateVersionID=%s\n", stateVersionID)
 	if stateVersionID == "" {
-		fmt.Printf("UploadStateVersion: ERROR - state_version_id required\n")
 		return c.JSON(400, map[string]string{"error": "state_version_id required"})
 	}
 
@@ -1209,47 +1196,37 @@ func (h *TfeHandler) UploadStateVersion(c echo.Context) error {
 	if err := h.checkWorkspacePermission(c, "unit.write", workspaceID); err != nil {
 		// Only enforce RBAC if we have a real auth error, not just missing headers
 		if !strings.Contains(err.Error(), "no authorization header") {
-			fmt.Printf("UploadStateVersion: RBAC permission denied: %v\n", err)
 			return c.JSON(http.StatusForbidden, map[string]string{
 				"error": "insufficient permissions to upload state",
 				"hint":  "contact your administrator to grant unit.write permission",
 			})
 		}
-		// If no auth header, allow but log for security monitoring
-		fmt.Printf("UploadStateVersion: No auth header - allowing upload based on lock validation\n")
 	}
 
 	// Read the state data from request body
 	stateData, err := io.ReadAll(c.Request().Body)
-	fmt.Printf("UploadStateVersion: Read %d bytes from body, err=%v\n", len(stateData), err)
 	if err != nil {
-		fmt.Printf("UploadStateVersion: ERROR - Failed to read state data: %v\n", err)
 		return c.JSON(400, map[string]string{"error": "Failed to read state data"})
 	}
-	if len(stateData) > 0 {
-		fmt.Printf("UploadStateVersion: Body preview: %s\n", string(stateData))
-	}
 
 	// Extract unit UUID from state ID - repository expects just the UUID
 	unitUUID := extractUnitUUID(stateID)
-	fmt.Printf("UploadStateVersion: Extracted unitUUID=%s from stateID=%s\n", unitUUID, stateID)
 
+	// Use directStateStore for signed URL operations (pre-authorized, no RBAC checks)
 	// Check if state exists (no auto-creation)
-	_, err = h.stateStore.Get(c.Request().Context(), unitUUID)
+	_, err = h.directStateStore.Get(c.Request().Context(), unitUUID)
 	if err == storage.ErrNotFound {
-		fmt.Printf("UploadStateVersion: Unit not found - no auto-creation\n")
 		return c.JSON(404, map[string]string{
 			"error": "Unit not found. Please create the unit first using 'taco unit create " + unitUUID + "' or the opentaco_unit Terraform resource.",
 		})
 	} else if err != nil {
-		fmt.Printf("UploadStateVersion: ERROR - Failed to check state existence: %v\n", err)
 		return c.JSON(500, map[string]string{
 			"error": "Failed to check state existence",
 		})
 	}
 
 	// Get the current lock to extract lock ID for state upload
-	currentLock, lockErr := h.stateStore.GetLock(c.Request().Context(), unitUUID)
+	currentLock, lockErr := h.directStateStore.GetLock(c.Request().Context(), unitUUID)
 	if lockErr != nil && lockErr != storage.ErrNotFound {
 		return c.JSON(500, map[string]string{"error": "Failed to get lock status"})
 	}
@@ -1261,23 +1238,18 @@ func (h *TfeHandler) UploadStateVersion(c echo.Context) error {
 	}
 
 	// Upload the state with proper lock ID
-	fmt.Printf("UploadStateVersion: Uploading to storage with lockID=%s\n", lockID)
-	err = h.stateStore.Upload(c.Request().Context(), unitUUID, stateData, lockID)
-	fmt.Printf("UploadStateVersion: Upload result, err=%v\n", err)
+	err = h.directStateStore.Upload(c.Request().Context(), unitUUID, stateData, lockID)
 	if err != nil {
 		if err == storage.ErrLockConflict {
-			fmt.Printf("UploadStateVersion: ERROR - Workspace is locked\n")
 			return c.JSON(423, map[string]string{
 				"error": "Workspace is locked",
 			})
 		}
-		fmt.Printf("UploadStateVersion: ERROR - Failed to upload state: %v\n", err)
 		return c.JSON(500, map[string]string{
 			"error": "Failed to upload state",
 		})
 	}
 
-	fmt.Printf("UploadStateVersion: SUCCESS - State uploaded successfully\n")
 	// Return 204 No Content as expected by Terraform
 	return c.NoContent(204)
 }