From f9b5af45ef8863d50ad01d724de42101c418e85c Mon Sep 17 00:00:00 2001 From: Brian Reardon Date: Thu, 30 Oct 2025 14:46:06 -0700 Subject: [PATCH 1/6] exempt org from some middleware, readable cloudblock, unique unit name --- taco/internal/api/routes.go | 9 +- taco/internal/middleware/org_context.go | 26 +- taco/internal/query/common/sql_store.go | 114 ++++++-- taco/internal/repositories/org_repository.go | 13 +- taco/internal/tfe/tfe.go | 20 +- taco/internal/tfe/workspaces.go | 265 ++++++++++++++---- ...031000000_add_unique_unit_name_per_org.sql | 6 + ...031000000_add_unique_unit_name_per_org.sql | 6 + ...031000000_add_unique_unit_name_per_org.sql | 6 + .../_dashboard/dashboard/units.$unitId.tsx | 15 +- 10 files changed, 380 insertions(+), 100 deletions(-) create mode 100644 taco/migrations/mysql/20251031000000_add_unique_unit_name_per_org.sql create mode 100644 taco/migrations/postgres/20251031000000_add_unique_unit_name_per_org.sql create mode 100644 taco/migrations/sqlite/20251031000000_add_unique_unit_name_per_org.sql diff --git a/taco/internal/api/routes.go b/taco/internal/api/routes.go index 8d069aa92..d3c9e7d5d 100644 --- a/taco/internal/api/routes.go +++ b/taco/internal/api/routes.go @@ -246,7 +246,14 @@ func RegisterRoutes(e *echo.Echo, deps Dependencies) { // TFE api - inject auth handler, full repository, blob store for tokens, and RBAC dependencies // TFE handler scopes to TFEOperations internally but needs blob store for API token storage - tfeHandler := tfe.NewTFETokenHandler(authHandler, deps.Repository, deps.BlobStore, deps.RBACManager) + // 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) // Create protected TFE group - opaque tokens only tfeGroup := e.Group("/tfe/api/v2") diff --git a/taco/internal/middleware/org_context.go b/taco/internal/middleware/org_context.go index 85c9d9fbe..408f9fa37 100644 --- a/taco/internal/middleware/org_context.go +++ b/taco/internal/middleware/org_context.go @@ -44,7 +44,22 @@ func JWTOrgResolverMiddleware(resolver domain.IdentifierResolver) echo.Middlewar func ResolveOrgContextMiddleware(resolver domain.IdentifierResolver) echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { - log.Printf("[WebhookOrgResolver] MIDDLEWARE INVOKED for path: %s", c.Path()) + path := c.Request().URL.Path + method := c.Request().Method + + log.Printf("[WebhookOrgResolver] MIDDLEWARE INVOKED for path: %s, method: %s", path, method) + + // Skip org resolution for endpoints that create/list orgs + // These endpoints don't require an existing org context + skipOrgResolution := (method == "POST" && path == "/internal/api/orgs") || + (method == "POST" && path == "/internal/api/orgs/sync") || + (method == "GET" && path == "/internal/api/orgs") || + (method == "GET" && path == "/internal/api/orgs/user") + + if skipOrgResolution { + log.Printf("[WebhookOrgResolver] Skipping org resolution for endpoint: %s %s", method, path) + return next(c) + } // Get org name from echo context (set by WebhookAuth) orgName, ok := c.Get("organization_id").(string) @@ -64,7 +79,14 @@ func ResolveOrgContextMiddleware(resolver domain.IdentifierResolver) echo.Middle orgUUID, err := resolver.ResolveOrganization(c.Request().Context(), orgName) if err != nil { log.Printf("[WebhookOrgResolver] ERROR: Failed to resolve organization '%s': %v", orgName, err) - return echo.NewHTTPError(500, "Failed to resolve organization") + log.Printf("[WebhookOrgResolver] ERROR: This likely means the organization doesn't exist in the database yet or the external_org_id doesn't match") + log.Printf("[WebhookOrgResolver] ERROR: Check if the organization was created successfully with external_org_id='%s'", orgName) + return echo.NewHTTPError(500, map[string]interface{}{ + "error": "Failed to resolve organization", + "detail": err.Error(), + "org_identifier": orgName, + "hint": "The organization may not exist in the database or the external_org_id doesn't match", + }) } log.Printf("[WebhookOrgResolver] SUCCESS: Resolved '%s' to UUID: %s", orgName, orgUUID) diff --git a/taco/internal/query/common/sql_store.go b/taco/internal/query/common/sql_store.go index ee906a587..45df47cb4 100644 --- a/taco/internal/query/common/sql_store.go +++ b/taco/internal/query/common/sql_store.go @@ -143,44 +143,104 @@ func (s *SQLStore) GetUnit(ctx context.Context, id string) (*types.Unit, error) return &unit, nil } -// parseBlobPath parses a blob path into org and unit name -// Supports: "org/name" or "name" (defaults to "default" org) -func (s *SQLStore) parseBlobPath(ctx context.Context, blobPath string) (orgUUID, name string, err error) { +// parseBlobPath parses a blob path into org UUID and unit name +// Supports multiple formats for backward compatibility and new UUID-based paths: +// - "org-uuid/unit-uuid" (new format: both UUIDs) +// - "org-uuid/unit-name" (hybrid: org UUID, unit by name) +// - "org-name/unit-name" (legacy: both by name) +// - "unit-name" (legacy: defaults to "default" org, unit by name) +func (s *SQLStore) parseBlobPath(ctx context.Context, blobPath string) (orgUUID, unitName string, err error) { parts := strings.SplitN(strings.Trim(blobPath, "/"), "/", 2) - var orgName string + var orgIdentifier, unitIdentifier string if len(parts) == 2 { - // Format: "org/name" - orgName = parts[0] - name = parts[1] + // Format: "org-identifier/unit-identifier" + orgIdentifier = parts[0] + unitIdentifier = parts[1] } else { - // Format: "name" - use default org - orgName = "default" - name = parts[0] + // Format: "unit-identifier" - use default org + orgIdentifier = "default" + unitIdentifier = parts[0] } - // Ensure org exists - var org types.Organization - err = s.db.WithContext(ctx).Where("name = ?", orgName).First(&org).Error - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { - // Create org if it doesn't exist (for migration) - org = types.Organization{ - Name: orgName, - DisplayName: fmt.Sprintf("Auto-created: %s", orgName), - CreatedBy: "system-sync", - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + // Step 1: Resolve org identifier to UUID + // Try as UUID first, then fall back to name lookup + if isUUID(orgIdentifier) { + // Direct UUID - verify it exists + var org types.Organization + err = s.db.WithContext(ctx).Where("id = ?", orgIdentifier).First(&org).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return "", "", fmt.Errorf("organization UUID not found: %s", orgIdentifier) } - if err := s.db.WithContext(ctx).Create(&org).Error; err != nil { - return "", "", fmt.Errorf("failed to create org: %w", err) + return "", "", fmt.Errorf("failed to lookup org by UUID: %w", err) + } + orgUUID = org.ID + } else { + // Lookup by name + var org types.Organization + err = s.db.WithContext(ctx).Where("name = ?", orgIdentifier).First(&org).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + // Create org if it doesn't exist (for migration/sync) + org = types.Organization{ + Name: orgIdentifier, + DisplayName: fmt.Sprintf("Auto-created: %s", orgIdentifier), + CreatedBy: "system-sync", + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + if err := s.db.WithContext(ctx).Create(&org).Error; err != nil { + return "", "", fmt.Errorf("failed to create org: %w", err) + } + } else { + return "", "", fmt.Errorf("failed to lookup org by name: %w", err) } - } else { - return "", "", fmt.Errorf("failed to lookup org: %w", err) } + orgUUID = org.ID } - return org.ID, name, nil + // Step 2: Resolve unit identifier to name + // If unit identifier is a UUID, look up its name; otherwise use as-is + if isUUID(unitIdentifier) { + // Look up unit by UUID to get its name + var unit types.Unit + err = s.db.WithContext(ctx).Where("id = ? AND org_id = ?", unitIdentifier, orgUUID).First(&unit).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return "", "", fmt.Errorf("unit UUID not found: %s in org %s", unitIdentifier, orgUUID) + } + return "", "", fmt.Errorf("failed to lookup unit by UUID: %w", err) + } + unitName = unit.Name + } else { + // Already a name, use directly + unitName = unitIdentifier + } + + return orgUUID, unitName, nil +} + +// isUUID checks if a string looks like a UUID (simple heuristic) +// Format: 8-4-4-4-12 hex digits with hyphens +func isUUID(s string) bool { + if len(s) != 36 { + return false + } + // Quick check: positions of hyphens + if s[8] != '-' || s[13] != '-' || s[18] != '-' || s[23] != '-' { + return false + } + // Check that other characters are hex digits + for i, c := range s { + if i == 8 || i == 13 || i == 18 || i == 23 { + continue + } + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) { + return false + } + } + return true } // SyncEnsureUnit creates or updates a unit from blob storage diff --git a/taco/internal/repositories/org_repository.go b/taco/internal/repositories/org_repository.go index fb9f069d4..a2f8d4ee8 100644 --- a/taco/internal/repositories/org_repository.go +++ b/taco/internal/repositories/org_repository.go @@ -79,6 +79,15 @@ func (r *orgRepository) Create(ctx context.Context, orgID, name, displayName, ex externalOrgIDPtr = &externalOrgID } + slog.Info("Creating organization entity", + "orgID", orgID, + "name", name, + "displayName", displayName, + "externalOrgID", externalOrgID, + "externalOrgIDPtr", externalOrgIDPtr, + "createdBy", createdBy, + ) + entity := &types.Organization{ Name: orgID, DisplayName: displayName, @@ -92,11 +101,13 @@ func (r *orgRepository) Create(ctx context.Context, orgID, name, displayName, ex return nil, fmt.Errorf("failed to create organization: %w", err) } - slog.Info("Organization created successfully", + slog.Info("Organization created successfully in database", + "dbID", entity.ID, "orgID", orgID, "name", name, "displayName", displayName, "externalOrgID", externalOrgID, + "storedExternalOrgID", getStringValue(entity.ExternalOrgID), "createdBy", createdBy, ) diff --git a/taco/internal/tfe/tfe.go b/taco/internal/tfe/tfe.go index ea268cd33..cf131c1cd 100644 --- a/taco/internal/tfe/tfe.go +++ b/taco/internal/tfe/tfe.go @@ -10,22 +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 // Scoped to TFE operations + 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) *TfeHandler { +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) return &TfeHandler{ - authHandler: authHandler, - stateStore: stateStore, - rbacManager: rbacManager, - apiTokens: auth.NewAPITokenManagerFromStore(blobStore), // Use blob store for token storage + authHandler: authHandler, + stateStore: stateStore, + rbacManager: rbacManager, + apiTokens: auth.NewAPITokenManagerFromStore(blobStore), // Use blob store for token storage + identifierResolver: identifierResolver, } } diff --git a/taco/internal/tfe/workspaces.go b/taco/internal/tfe/workspaces.go index 16d7537d7..c363a4a98 100644 --- a/taco/internal/tfe/workspaces.go +++ b/taco/internal/tfe/workspaces.go @@ -82,15 +82,51 @@ func generateStateVersionID(stateID string, timestamp int64) string { return fmt.Sprintf("sv-%s-%d", encodedStateID, timestamp) } -// convertWorkspaceToStateID converts a TFE workspace ID to a state ID -// e.g., "ws-myworkspace" -> "myworkspace" (strip the ws- prefix to match CLI-created states) -func convertWorkspaceToStateID(workspaceID string) string { +// convertWorkspaceToStateIDWithOrg converts a workspace name to an org-scoped unit ID +// Workspace name is a human-readable unit name (e.g., "my-app-prod") +// Returns: "/" for S3 storage (both UUIDs for immutable paths) +func (h *TfeHandler) convertWorkspaceToStateIDWithOrg(ctx context.Context, orgIdentifier, workspaceName string) (string, error) { // Validate input + if strings.TrimSpace(workspaceName) == "" { + return "", fmt.Errorf("workspace name cannot be empty") + } + + // Strip "ws-" prefix if present (TFE compatibility for legacy workspace IDs) + if strings.HasPrefix(workspaceName, "ws-") { + workspaceName = strings.TrimPrefix(workspaceName, "ws-") + if workspaceName == "" { + return "", fmt.Errorf("invalid workspace name: empty after stripping ws- prefix") + } + } + + // If no org identifier provided or no resolver, return workspace name as-is (backwards compat) + if orgIdentifier == "" || h.identifierResolver == nil { + return workspaceName, nil + } + + // Step 1: Resolve organization identifier (external_org_id, UUID, or name) to UUID + orgUUID, err := h.identifierResolver.ResolveOrganization(ctx, orgIdentifier) + if err != nil { + return "", fmt.Errorf("failed to resolve organization '%s': %w", orgIdentifier, err) + } + + // Step 2: Resolve unit name to UUID within the organization + unitUUID, err := h.identifierResolver.ResolveUnit(ctx, workspaceName, orgUUID) + if err != nil { + return "", fmt.Errorf("failed to resolve unit '%s' in org '%s': %w", workspaceName, orgIdentifier, err) + } + + // Return org-scoped unit path for S3 storage + // Format: / (both UUIDs for immutable, rename-safe paths) + // Example: "123e4567-e89b-12d3-a456-426614174000/987f6543-e21a-43d2-b789-123456789abc" + return fmt.Sprintf("%s/%s", orgUUID, unitUUID), nil +} + +// Legacy function for backwards compatibility - no org scoping +func convertWorkspaceToStateID(workspaceID string) string { if strings.TrimSpace(workspaceID) == "" { return "" } - - // If it's a TFE workspace ID (ws-something), extract just the workspace name if strings.HasPrefix(workspaceID, "ws-") { result := strings.TrimPrefix(workspaceID, "ws-") if result == "" { @@ -98,10 +134,55 @@ func convertWorkspaceToStateID(workspaceID string) string { } return result } - // Otherwise, return as-is (for direct workspace names) return workspaceID } +// getOrgFromContext extracts organization identifier from the echo context +// The org is set by authentication middleware (JWT contains org claim) +// Falls back to "default" if no org is found +func getOrgFromContext(c echo.Context) string { + // Try jwt_org first (set by RequireAuth middleware from JWT claims) + if jwtOrg := c.Get("jwt_org"); jwtOrg != nil { + if orgStr, ok := jwtOrg.(string); ok && orgStr != "" { + return orgStr + } + } + + // Try organization_id (set by WebhookAuth middleware) + if orgID := c.Get("organization_id"); orgID != nil { + if orgStr, ok := orgID.(string); ok && orgStr != "" { + return orgStr + } + } + + // Fall back to default org + return "default" +} + +// parseOrgParam parses organization parameter in format "display:identifier" or just "identifier" +// Returns the identifier part that can be resolved (external_org_id, UUID, or name) +// +// Supported formats: +// - "Personal:org_01K8..." -> returns "org_01K8..." (display name for convenience) +// - "org_01K8..." -> returns "org_01K8..." (identifier only, works fine!) +// - "Acme:123e4567-..." -> returns "123e4567-..." (UUID identifier) +// - "123e4567-..." -> returns "123e4567-..." (UUID only) +// +// The display name (before colon) is purely cosmetic for user convenience. +// The identifier (after colon, or the whole string if no colon) is what gets resolved. +func parseOrgParam(orgParam string) (displayName, identifier string) { + // Format: "DisplayName:identifier" or just "identifier" + if strings.Contains(orgParam, ":") { + parts := strings.SplitN(orgParam, ":", 2) + if len(parts) == 2 { + return strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) + } + } + // No colon - identifier only (display name omitted for convenience) + identifier = strings.TrimSpace(orgParam) + return "", identifier +} + // extractWorkspaceIDFromParam extracts workspace ID from URL parameter func extractWorkspaceIDFromParam(c echo.Context) string { workspaceID := c.Param("workspace_id") @@ -231,10 +312,45 @@ func (h *TfeHandler) GetWorkspace(c echo.Context) error { c.Response().Header().Set("Tfp-Api-Version", "2.5") c.Response().Header().Set("X-Terraform-Enterprise-App", "Terraform Enterprise") + orgParam := c.Param("org_name") workspaceName := c.Param("workspace_name") + if workspaceName == "" { return c.JSON(400, map[string]string{"error": "workspace_name invalid"}) } + + // Parse org param - supports both "Display:identifier" and just "identifier" + displayName, orgIdentifier := parseOrgParam(orgParam) + + if displayName != "" { + fmt.Printf("GetWorkspace: orgParam=%s (display=%s, identifier=%s), workspaceName=%s\n", + orgParam, displayName, orgIdentifier, workspaceName) + } else { + fmt.Printf("GetWorkspace: orgParam=%s (identifier only, no display name), workspaceName=%s\n", + orgIdentifier, workspaceName) + } + + // Convert workspace name to unit ID (org-scoped if org provided) + // workspaceName is now the human-readable unit name, not a UUID + stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) + if err != nil { + fmt.Printf("GetWorkspace: failed to resolve workspace: %v\n", err) + return c.JSON(500, map[string]string{ + "error": "failed to resolve workspace", + "detail": err.Error(), + }) + } + + fmt.Printf("GetWorkspace: resolved stateID=%s\n", stateID) + + // Check if unit exists (optional - may auto-create later) + _, err = h.stateStore.Get(c.Request().Context(), stateID) + locked := false + if err == nil { + // Check if locked + lockInfo, _ := h.stateStore.GetLock(c.Request().Context(), stateID) + locked = (lockInfo != nil) + } workspace := &tfe.TFEWorkspace{ ID: tfe.NewTfeResourceIdentifier(tfe.WorkspaceType, workspaceName).String(), @@ -250,7 +366,7 @@ func (h *TfeHandler) GetWorkspace(c echo.Context) error { ExecutionMode: "local", FileTriggersEnabled: false, GlobalRemoteState: false, - Locked: false, + Locked: locked, MigrationEnvironment: "", Name: workspaceName, Operations: false, @@ -274,7 +390,7 @@ func (h *TfeHandler) GetWorkspace(c echo.Context) error { TagNames: nil, CurrentRun: nil, Organization: &tfe.TFEOrganization{ - Name: "opentaco", + Name: orgParam, // Return the full org param (includes display name if provided) }, Outputs: nil, } @@ -291,17 +407,33 @@ func (h *TfeHandler) LockWorkspace(c echo.Context) error { c.Response().Header().Set("Tfp-Api-Version", "2.5") c.Response().Header().Set("X-Terraform-Enterprise-App", "Terraform Enterprise") - // Extract workspace ID and convert to state ID + // Extract workspace ID (format: ws-{workspace-name}) workspaceID := extractWorkspaceIDFromParam(c) if workspaceID == "" { return c.JSON(400, map[string]string{"error": "workspace_id required"}) } - // Debug logging - fmt.Printf("LockWorkspace: workspaceID=%s\n", workspaceID) + // Strip ws- prefix to get workspace name + workspaceName := convertWorkspaceToStateID(workspaceID) + fmt.Printf("LockWorkspace: workspaceID=%s, workspaceName=%s\n", workspaceID, workspaceName) + + // Get org from authentication context (JWT claim or webhook header) + orgIdentifier := getOrgFromContext(c) + fmt.Printf("LockWorkspace: orgIdentifier=%s (from auth context)\n", orgIdentifier) + + // Resolve to UUID/UUID path + stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) + if err != nil { + fmt.Printf("LockWorkspace: failed to resolve workspace: %v\n", err) + return c.JSON(500, map[string]string{ + "error": "failed to resolve workspace", + "detail": err.Error(), + }) + } + fmt.Printf("LockWorkspace: resolved stateID=%s\n", stateID) // Check RBAC permission for locking workspace - if err := h.checkWorkspacePermission(c, "unit:write", workspaceID); err != nil { + if err := h.checkWorkspacePermission(c, "unit:write", stateID); err != nil { return c.JSON(http.StatusForbidden, map[string]string{ "error": "insufficient permissions to lock workspace", "hint": "contact your administrator to grant unit:write permission", @@ -313,9 +445,6 @@ func (h *TfeHandler) LockWorkspace(c echo.Context) error { return c.JSON(500, map[string]string{"error": "State store not initialized"}) } - stateID := convertWorkspaceToStateID(workspaceID) - fmt.Printf("LockWorkspace: stateID=%s\n", stateID) - // Check if state exists, enot _, err := h.stateStore.Get(c.Request().Context(), stateID) fmt.Printf("LockWorkspace: Get result, err=%v\n", err) @@ -391,14 +520,28 @@ func (h *TfeHandler) UnlockWorkspace(c echo.Context) error { c.Response().Header().Set("Tfp-Api-Version", "2.5") c.Response().Header().Set("X-Terraform-Enterprise-App", "Terraform Enterprise") - // Extract workspace ID and convert to state ID + // Extract workspace ID (format: ws-{workspace-name}) workspaceID := extractWorkspaceIDFromParam(c) if workspaceID == "" { return c.JSON(400, map[string]string{"error": "workspace_id required"}) } - stateID := convertWorkspaceToStateID(workspaceID) - fmt.Printf("UnlockWorkspace: workspaceID=%s, stateID=%s\n", workspaceID, stateID) + // Strip ws- prefix to get workspace name + workspaceName := convertWorkspaceToStateID(workspaceID) + + // Get org from authentication context (JWT claim or webhook header) + orgIdentifier := getOrgFromContext(c) + + // Resolve to UUID/UUID path + stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) + if err != nil { + fmt.Printf("UnlockWorkspace: failed to resolve workspace: %v\n", err) + return c.JSON(500, map[string]string{ + "error": "failed to resolve workspace", + "detail": err.Error(), + }) + } + fmt.Printf("UnlockWorkspace: workspaceID=%s, resolved stateID=%s\n", workspaceID, stateID) // Get current lock to find lock ID currentLock, err := h.stateStore.GetLock(c.Request().Context(), stateID) @@ -459,14 +602,28 @@ func (h *TfeHandler) ForceUnlockWorkspace(c echo.Context) error { c.Response().Header().Set("Tfp-Api-Version", "2.5") c.Response().Header().Set("X-Terraform-Enterprise-App", "Terraform Enterprise") - // Extract workspace ID and convert to state ID + // Extract workspace ID (format: ws-{workspace-name}) workspaceID := extractWorkspaceIDFromParam(c) if workspaceID == "" { return c.JSON(400, map[string]string{"error": "workspace_id required"}) } - stateID := convertWorkspaceToStateID(workspaceID) - fmt.Printf("ForceUnlockWorkspace: workspaceID=%s, stateID=%s\n", workspaceID, stateID) + // Strip ws- prefix to get workspace name + workspaceName := convertWorkspaceToStateID(workspaceID) + + // Get org from authentication context (JWT claim or webhook header) + orgIdentifier := getOrgFromContext(c) + + // Resolve to UUID/UUID path + stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) + if err != nil { + fmt.Printf("ForceUnlockWorkspace: failed to resolve workspace: %v\n", err) + return c.JSON(500, map[string]string{ + "error": "failed to resolve workspace", + "detail": err.Error(), + }) + } + fmt.Printf("ForceUnlockWorkspace: workspaceID=%s, resolved stateID=%s\n", workspaceID, stateID) // Get current lock to find lock ID currentLock, err := h.stateStore.GetLock(c.Request().Context(), stateID) @@ -520,6 +677,7 @@ func (h *TfeHandler) GetCurrentStateVersion(c echo.Context) error { c.Response().Header().Set("Tfp-Api-Version", "2.5") c.Response().Header().Set("X-Terraform-Enterprise-App", "Terraform Enterprise") + // Extract workspace ID (format: ws-{workspace-name}) workspaceID := extractWorkspaceIDFromParam(c) fmt.Printf("GetCurrentStateVersion: workspaceID=%s\n", workspaceID) if workspaceID == "" { @@ -527,15 +685,25 @@ func (h *TfeHandler) GetCurrentStateVersion(c echo.Context) error { return c.JSON(400, map[string]string{"error": "workspace_id required"}) } - stateID := convertWorkspaceToStateID(workspaceID) - fmt.Printf("GetCurrentStateVersion: stateID=%s\n", stateID) - if stateID == "" { - fmt.Printf("GetCurrentStateVersion: ERROR - invalid state ID from workspace ID: %s\n", workspaceID) - return c.JSON(400, map[string]string{"error": "invalid workspace ID"}) + // Strip ws- prefix to get workspace name + workspaceName := convertWorkspaceToStateID(workspaceID) + + // Get org from authentication context (JWT claim or webhook header) + orgIdentifier := getOrgFromContext(c) + + // Resolve to UUID/UUID path + stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) + if err != nil { + fmt.Printf("GetCurrentStateVersion: failed to resolve workspace: %v\n", err) + return c.JSON(500, map[string]string{ + "error": "failed to resolve workspace", + "detail": err.Error(), + }) } + fmt.Printf("GetCurrentStateVersion: resolved stateID=%s\n", stateID) // Check RBAC permission with correct three-scenario logic - if err := h.checkWorkspacePermission(c, "unit:read", workspaceID); err != nil { + if err := h.checkWorkspacePermission(c, "unit:read", stateID); err != nil { return c.JSON(http.StatusForbidden, map[string]string{ "error": "insufficient permissions to access workspace", "hint": "contact your administrator to grant unit:read permission", @@ -593,6 +761,7 @@ func (h *TfeHandler) CreateStateVersion(c echo.Context) error { c.Response().Header().Set("Tfp-Api-Version", "2.5") c.Response().Header().Set("X-Terraform-Enterprise-App", "Terraform Enterprise") + // Extract workspace ID (format: ws-{workspace-name}) workspaceID := extractWorkspaceIDFromParam(c) fmt.Printf("CreateStateVersion: workspaceID=%s\n", workspaceID) if workspaceID == "" { @@ -600,15 +769,25 @@ func (h *TfeHandler) CreateStateVersion(c echo.Context) error { return c.JSON(400, map[string]string{"error": "workspace_id required"}) } - stateID := convertWorkspaceToStateID(workspaceID) - fmt.Printf("CreateStateVersion: stateID=%s\n", stateID) - if stateID == "" { - fmt.Printf("CreateStateVersion: ERROR - invalid state ID from workspace ID: %s\n", workspaceID) - return c.JSON(400, map[string]string{"error": "invalid workspace ID"}) + // Strip ws- prefix to get workspace name + workspaceName := convertWorkspaceToStateID(workspaceID) + + // Get org from authentication context (JWT claim or webhook header) + orgIdentifier := getOrgFromContext(c) + + // Resolve to UUID/UUID path + stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) + if err != nil { + fmt.Printf("CreateStateVersion: failed to resolve workspace: %v\n", err) + return c.JSON(500, map[string]string{ + "error": "failed to resolve workspace", + "detail": err.Error(), + }) } + fmt.Printf("CreateStateVersion: resolved stateID=%s\n", stateID) // Check RBAC permission for creating/writing state versions - if err := h.checkWorkspacePermission(c, "unit:write", workspaceID); err != nil { + if err := h.checkWorkspacePermission(c, "unit:write", stateID); err != nil { return c.JSON(http.StatusForbidden, map[string]string{ "error": "insufficient permissions to create state version", "hint": "contact your administrator to grant unit:write permission", @@ -1051,23 +1230,3 @@ func (h *TfeHandler) ShowStateVersion(c echo.Context) error { } return c.JSON(http.StatusOK, resp) } - -// getOrgFromContext extracts org ID from Echo context or defaults to "default" -func getOrgFromContext(c echo.Context) string { - // Try jwt_org (from JWT auth) - if jwtOrg := c.Get("jwt_org"); jwtOrg != nil { - if orgStr, ok := jwtOrg.(string); ok && orgStr != "" { - return orgStr - } - } - - // Try organization_id (from webhook auth) - if orgID := c.Get("organization_id"); orgID != nil { - if orgStr, ok := orgID.(string); ok && orgStr != "" { - return orgStr - } - } - - // Default for self-hosted - return "default" -} diff --git a/taco/migrations/mysql/20251031000000_add_unique_unit_name_per_org.sql b/taco/migrations/mysql/20251031000000_add_unique_unit_name_per_org.sql new file mode 100644 index 000000000..23da20a63 --- /dev/null +++ b/taco/migrations/mysql/20251031000000_add_unique_unit_name_per_org.sql @@ -0,0 +1,6 @@ +-- Add unique constraint on (org_id, name) for units table +-- This ensures unit names are unique within each organization + +-- Create unique index on org_id + name +CREATE UNIQUE INDEX `idx_units_org_name` ON `units` (`org_id`, `name`); + diff --git a/taco/migrations/postgres/20251031000000_add_unique_unit_name_per_org.sql b/taco/migrations/postgres/20251031000000_add_unique_unit_name_per_org.sql new file mode 100644 index 000000000..07ea2f080 --- /dev/null +++ b/taco/migrations/postgres/20251031000000_add_unique_unit_name_per_org.sql @@ -0,0 +1,6 @@ +-- Add unique constraint on (org_id, name) for units table +-- This ensures unit names are unique within each organization + +-- Create unique index on org_id + name +CREATE UNIQUE INDEX "idx_units_org_name" ON "public"."units" ("org_id", "name"); + diff --git a/taco/migrations/sqlite/20251031000000_add_unique_unit_name_per_org.sql b/taco/migrations/sqlite/20251031000000_add_unique_unit_name_per_org.sql new file mode 100644 index 000000000..23da20a63 --- /dev/null +++ b/taco/migrations/sqlite/20251031000000_add_unique_unit_name_per_org.sql @@ -0,0 +1,6 @@ +-- Add unique constraint on (org_id, name) for units table +-- This ensures unit names are unique within each organization + +-- Create unique index on org_id + name +CREATE UNIQUE INDEX `idx_units_org_name` ON `units` (`org_id`, `name`); + diff --git a/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx b/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx index ee533d6a0..32d522b80 100644 --- a/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx +++ b/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx @@ -75,7 +75,7 @@ export const Route = createFileRoute( )({ component: RouteComponent, loader: async ({ context, params: {unitId} }) => { - const { user, organisationId } = context; + const { user, organisationId, organisationName } = context; const unitData = await getUnitFn({data: {organisationId: organisationId || '', userId: user?.id || '', email: user?.email || '', unitId: unitId}}) const unitVersionsData = await getUnitVersionsFn({data: {organisationId: organisationId || '', userId: user?.id || '', email: user?.email || '', unitId: unitId}}) const unitStatusData = await getUnitStatusFn({data: {organisationId: organisationId || '', userId: user?.id || '', email: user?.email || '', unitId: unitId}}) @@ -89,7 +89,8 @@ export const Route = createFileRoute( unitStatus: unitStatusData, unitVersions: unitVersionsData.versions, user, - organisationId, + organisationId, + organisationName, publicHostname, } @@ -120,7 +121,7 @@ function formatDate(input: Date | string | number) { function RouteComponent() { const data = Route.useLoaderData() - const { unitData, unitVersions, unitStatus, organisationId, publicHostname, user } = data + const { unitData, unitVersions, unitStatus, organisationId, organisationName, publicHostname, user } = data const unit = unitData const router = useRouter() @@ -344,9 +345,9 @@ function RouteComponent() { {`terraform { cloud { hostname = "${publicHostname}" - organization = "${organisationId}" + organization = "${organisationName}:${organisationId}" workspaces { - name = "${unit.id}" + name = "${unit.name}" } } }`} @@ -355,9 +356,9 @@ function RouteComponent() { content={`terraform { cloud { hostname = "${publicHostname}" - organization = "${organisationId}" + organization = "${organisationName}:${organisationId}" workspaces { - name = "${unit.id}" + name = "${unit.name}" } } }`} From 5572ff93076e45a374acf48b4621e016956a8be3 Mon Sep 17 00:00:00 2001 From: Brian Reardon Date: Thu, 30 Oct 2025 14:50:20 -0700 Subject: [PATCH 2/6] message in org unique --- taco/internal/repositories/unit_repository.go | 10 ++++++++++ taco/internal/unit/handler.go | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/taco/internal/repositories/unit_repository.go b/taco/internal/repositories/unit_repository.go index fbf63b05d..ab9618ae9 100644 --- a/taco/internal/repositories/unit_repository.go +++ b/taco/internal/repositories/unit_repository.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log" + "strings" "time" "github.com/diggerhq/digger/opentaco/internal/domain" @@ -71,6 +72,15 @@ func (r *UnitRepository) Create(ctx context.Context, orgID, name string) (*stora } if err := r.db.WithContext(ctx).Create(unit).Error; err != nil { + // Check if this is a unique constraint violation + // GORM doesn't have a specific error type, so we check the error string + errMsg := err.Error() + if strings.Contains(errMsg, "duplicate") || + strings.Contains(errMsg, "unique constraint") || + strings.Contains(errMsg, "UNIQUE constraint") || + strings.Contains(errMsg, "idx_units_org_name") { + return nil, storage.ErrAlreadyExists + } return nil, fmt.Errorf("failed to create unit in database: %w", err) } diff --git a/taco/internal/unit/handler.go b/taco/internal/unit/handler.go index bd31895e8..327cf53a0 100644 --- a/taco/internal/unit/handler.go +++ b/taco/internal/unit/handler.go @@ -111,7 +111,10 @@ func (h *Handler) CreateUnit(c echo.Context) error { if err != nil { if err == storage.ErrAlreadyExists { analytics.SendEssential("unit_create_failed_already_exists") - return c.JSON(http.StatusConflict, map[string]string{"error": "Unit already exists"}) + return c.JSON(http.StatusConflict, map[string]string{ + "error": "Unit already exists", + "detail": fmt.Sprintf("A unit with name '%s' already exists in this organization", name), + }) } // Log the actual error for debugging c.Logger().Errorf("Failed to create unit '%s' in org '%s': %v", name, orgCtx.OrgID, err) From a26b0f4a2ac5ca8497d0d1ee398185843a4221f5 Mon Sep 17 00:00:00 2001 From: Brian Reardon Date: Thu, 30 Oct 2025 15:07:10 -0700 Subject: [PATCH 3/6] correct comment - we dont resolve names --- taco/internal/tfe/workspaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taco/internal/tfe/workspaces.go b/taco/internal/tfe/workspaces.go index c363a4a98..6eac17bde 100644 --- a/taco/internal/tfe/workspaces.go +++ b/taco/internal/tfe/workspaces.go @@ -104,7 +104,7 @@ func (h *TfeHandler) convertWorkspaceToStateIDWithOrg(ctx context.Context, orgId return workspaceName, nil } - // Step 1: Resolve organization identifier (external_org_id, UUID, or name) to UUID + // Step 1: Resolve organization identifier (external_org_id or UUID) to UUID orgUUID, err := h.identifierResolver.ResolveOrganization(ctx, orgIdentifier) if err != nil { return "", fmt.Errorf("failed to resolve organization '%s': %w", orgIdentifier, err) From cbb3ea240d3284637f3f58879c91ea3c0a79bc3b Mon Sep 17 00:00:00 2001 From: Brian Reardon Date: Thu, 30 Oct 2025 15:31:56 -0700 Subject: [PATCH 4/6] blob paths uuid/uuid throughout, prevent default --- taco/internal/query/common/sql_store.go | 167 +++++++----------- taco/internal/repositories/unit_repository.go | 44 ++--- taco/internal/tfe/workspaces.go | 57 ++++-- .../_dashboard/dashboard/units.$unitId.tsx | 4 +- 4 files changed, 130 insertions(+), 142 deletions(-) diff --git a/taco/internal/query/common/sql_store.go b/taco/internal/query/common/sql_store.go index 45df47cb4..a72b333f3 100644 --- a/taco/internal/query/common/sql_store.go +++ b/taco/internal/query/common/sql_store.go @@ -10,6 +10,7 @@ import ( "github.com/diggerhq/digger/opentaco/internal/query/types" "github.com/diggerhq/digger/opentaco/internal/rbac" + "github.com/google/uuid" "gorm.io/gorm" ) @@ -143,111 +144,64 @@ func (s *SQLStore) GetUnit(ctx context.Context, id string) (*types.Unit, error) return &unit, nil } -// parseBlobPath parses a blob path into org UUID and unit name -// Supports multiple formats for backward compatibility and new UUID-based paths: -// - "org-uuid/unit-uuid" (new format: both UUIDs) -// - "org-uuid/unit-name" (hybrid: org UUID, unit by name) -// - "org-name/unit-name" (legacy: both by name) -// - "unit-name" (legacy: defaults to "default" org, unit by name) -func (s *SQLStore) parseBlobPath(ctx context.Context, blobPath string) (orgUUID, unitName string, err error) { +// parseBlobPath parses a UUID-based blob path into org UUID and unit UUID +// Expected format: "org-uuid/unit-uuid" +// This is the only format used - all blob paths are UUID-based for immutability +func (s *SQLStore) parseBlobPath(ctx context.Context, blobPath string) (orgUUID, unitUUID string, err error) { parts := strings.SplitN(strings.Trim(blobPath, "/"), "/", 2) - var orgIdentifier, unitIdentifier string - if len(parts) == 2 { - // Format: "org-identifier/unit-identifier" - orgIdentifier = parts[0] - unitIdentifier = parts[1] - } else { - // Format: "unit-identifier" - use default org - orgIdentifier = "default" - unitIdentifier = parts[0] + if len(parts) != 2 { + return "", "", fmt.Errorf("invalid blob path format: expected 'org-uuid/unit-uuid', got '%s'", blobPath) } - // Step 1: Resolve org identifier to UUID - // Try as UUID first, then fall back to name lookup - if isUUID(orgIdentifier) { - // Direct UUID - verify it exists - var org types.Organization - err = s.db.WithContext(ctx).Where("id = ?", orgIdentifier).First(&org).Error - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { - return "", "", fmt.Errorf("organization UUID not found: %s", orgIdentifier) - } - return "", "", fmt.Errorf("failed to lookup org by UUID: %w", err) - } - orgUUID = org.ID - } else { - // Lookup by name - var org types.Organization - err = s.db.WithContext(ctx).Where("name = ?", orgIdentifier).First(&org).Error - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { - // Create org if it doesn't exist (for migration/sync) - org = types.Organization{ - Name: orgIdentifier, - DisplayName: fmt.Sprintf("Auto-created: %s", orgIdentifier), - CreatedBy: "system-sync", - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - if err := s.db.WithContext(ctx).Create(&org).Error; err != nil { - return "", "", fmt.Errorf("failed to create org: %w", err) - } - } else { - return "", "", fmt.Errorf("failed to lookup org by name: %w", err) - } + orgUUID = parts[0] + unitUUID = parts[1] + + // Validate both are UUIDs + if !isUUID(orgUUID) { + return "", "", fmt.Errorf("invalid org UUID in blob path: %s", orgUUID) + } + if !isUUID(unitUUID) { + return "", "", fmt.Errorf("invalid unit UUID in blob path: %s", unitUUID) + } + + // Verify org exists + var org types.Organization + err = s.db.WithContext(ctx).Where("id = ?", orgUUID).First(&org).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return "", "", fmt.Errorf("organization not found: %s", orgUUID) } - orgUUID = org.ID + return "", "", fmt.Errorf("failed to lookup organization: %w", err) } - // Step 2: Resolve unit identifier to name - // If unit identifier is a UUID, look up its name; otherwise use as-is - if isUUID(unitIdentifier) { - // Look up unit by UUID to get its name - var unit types.Unit - err = s.db.WithContext(ctx).Where("id = ? AND org_id = ?", unitIdentifier, orgUUID).First(&unit).Error - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { - return "", "", fmt.Errorf("unit UUID not found: %s in org %s", unitIdentifier, orgUUID) - } - return "", "", fmt.Errorf("failed to lookup unit by UUID: %w", err) + // Verify unit exists + var unit types.Unit + err = s.db.WithContext(ctx).Where("id = ? AND org_id = ?", unitUUID, orgUUID).First(&unit).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return "", "", fmt.Errorf("unit not found: %s in org %s", unitUUID, orgUUID) } - unitName = unit.Name - } else { - // Already a name, use directly - unitName = unitIdentifier + return "", "", fmt.Errorf("failed to lookup unit: %w", err) } - return orgUUID, unitName, nil + return orgUUID, unitUUID, nil } -// isUUID checks if a string looks like a UUID (simple heuristic) -// Format: 8-4-4-4-12 hex digits with hyphens +// isUUID checks if a string is a valid UUID +// Uses proper UUID parsing to validate format and structure +// This is critical for distinguishing UUID-based paths from name-based paths: +// - UUID: "123e4567-89ab-12d3-a456-426614174000" → lookup by ID +// - Name: "my-app-prod" → lookup by name func isUUID(s string) bool { - if len(s) != 36 { - return false - } - // Quick check: positions of hyphens - if s[8] != '-' || s[13] != '-' || s[18] != '-' || s[23] != '-' { - return false - } - // Check that other characters are hex digits - for i, c := range s { - if i == 8 || i == 13 || i == 18 || i == 23 { - continue - } - if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) { - return false - } - } - return true + _, err := uuid.Parse(s) + return err == nil } // SyncEnsureUnit creates or updates a unit from blob storage -// Supports blob paths: "org/name" or "name" (defaults to default org) -// UUIDs are auto-generated via BeforeCreate hook -func (s *SQLStore) SyncEnsureUnit(ctx context.Context, unitName string) error { - orgUUID, name, err := s.parseBlobPath(ctx, unitName) +// Expects UUID-based blob path: "org-uuid/unit-uuid" +func (s *SQLStore) SyncEnsureUnit(ctx context.Context, blobPath string) error { + orgUUID, unitUUID, err := s.parseBlobPath(ctx, blobPath) if err != nil { return err } @@ -255,7 +209,7 @@ func (s *SQLStore) SyncEnsureUnit(ctx context.Context, unitName string) error { // Check if unit exists var existing types.Unit err = s.db.WithContext(ctx). - Where(queryOrgAndName, orgUUID, name). + Where("id = ? AND org_id = ?", unitUUID, orgUUID). First(&existing).Error if err == nil { @@ -267,47 +221,44 @@ func (s *SQLStore) SyncEnsureUnit(ctx context.Context, unitName string) error { return err } - // Create new unit (UUID auto-generated by BeforeCreate) - unit := types.Unit{ - OrgID: orgUUID, - Name: name, - } - return s.db.WithContext(ctx).Create(&unit).Error + // Unit doesn't exist - this shouldn't happen with UUID-based paths + // as units should be created via UnitRepository first + return fmt.Errorf("unit %s not found in database (UUID-based paths require unit to exist)", unitUUID) } -func (s *SQLStore) SyncUnitMetadata(ctx context.Context, unitName string, size int64, updated time.Time) error { - orgUUID, name, err := s.parseBlobPath(ctx, unitName) +func (s *SQLStore) SyncUnitMetadata(ctx context.Context, blobPath string, size int64, updated time.Time) error { + orgUUID, unitUUID, err := s.parseBlobPath(ctx, blobPath) if err != nil { return err } return s.db.WithContext(ctx).Model(&types.Unit{}). - Where(queryOrgAndName, orgUUID, name). + Where("id = ? AND org_id = ?", unitUUID, orgUUID). Updates(map[string]interface{}{ "size": size, "updated_at": updated, }).Error } -func (s *SQLStore) SyncDeleteUnit(ctx context.Context, unitName string) error { - orgUUID, name, err := s.parseBlobPath(ctx, unitName) +func (s *SQLStore) SyncDeleteUnit(ctx context.Context, blobPath string) error { + orgUUID, unitUUID, err := s.parseBlobPath(ctx, blobPath) if err != nil { return err } return s.db.WithContext(ctx). - Where(queryOrgAndName, orgUUID, name). + Where("id = ? AND org_id = ?", unitUUID, orgUUID). Delete(&types.Unit{}).Error } -func (s *SQLStore) SyncUnitLock(ctx context.Context, unitName string, lockID, lockWho string, lockCreated time.Time) error { - orgUUID, name, err := s.parseBlobPath(ctx, unitName) +func (s *SQLStore) SyncUnitLock(ctx context.Context, blobPath string, lockID, lockWho string, lockCreated time.Time) error { + orgUUID, unitUUID, err := s.parseBlobPath(ctx, blobPath) if err != nil { return err } return s.db.WithContext(ctx).Model(&types.Unit{}). - Where(queryOrgAndName, orgUUID, name). + Where("id = ? AND org_id = ?", unitUUID, orgUUID). Updates(map[string]interface{}{ "locked": true, "lock_id": lockID, @@ -316,14 +267,14 @@ func (s *SQLStore) SyncUnitLock(ctx context.Context, unitName string, lockID, lo }).Error } -func (s *SQLStore) SyncUnitUnlock(ctx context.Context, unitName string) error { - orgUUID, name, err := s.parseBlobPath(ctx, unitName) +func (s *SQLStore) SyncUnitUnlock(ctx context.Context, blobPath string) error { + orgUUID, unitUUID, err := s.parseBlobPath(ctx, blobPath) if err != nil { return err } return s.db.WithContext(ctx).Model(&types.Unit{}). - Where(queryOrgAndName, orgUUID, name). + Where("id = ? AND org_id = ?", unitUUID, orgUUID). Updates(map[string]interface{}{ "locked": false, "lock_id": "", diff --git a/taco/internal/repositories/unit_repository.go b/taco/internal/repositories/unit_repository.go index ab9618ae9..36d9a09f5 100644 --- a/taco/internal/repositories/unit_repository.go +++ b/taco/internal/repositories/unit_repository.go @@ -84,8 +84,9 @@ func (r *UnitRepository) Create(ctx context.Context, orgID, name string) (*stora return nil, fmt.Errorf("failed to create unit in database: %w", err) } - // Construct org-scoped blob path: {orgId}/{name} - blobPath := fmt.Sprintf("%s/%s", org.Name, name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + // This is immutable - unit renames won't affect S3 paths + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Create blob in storage _, err = r.blobStore.Create(ctx, blobPath) @@ -95,8 +96,8 @@ func (r *UnitRepository) Create(ctx context.Context, orgID, name string) (*stora return nil, fmt.Errorf("failed to create blob storage: %w", err) } - log.Printf("Created unit: UUID=%s, Org=%s, Name=%s, BlobPath=%s", - unit.ID, org.Name, name, blobPath) + log.Printf("Created unit: UUID=%s, Org=%s (%s), Name=%s, BlobPath=%s", + unit.ID, org.Name, org.ID, name, blobPath) return &storage.UnitMetadata{ ID: unit.ID, // UUID @@ -126,8 +127,8 @@ func (r *UnitRepository) Get(ctx context.Context, uuid string) (*storage.UnitMet return nil, fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Get blob metadata (size, lock info) blobMeta, err := r.blobStore.Get(ctx, blobPath) @@ -207,8 +208,8 @@ func (r *UnitRepository) Delete(ctx context.Context, uuid string) error { return fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Delete from blob storage first if err := r.blobStore.Delete(ctx, blobPath); err != nil && err != storage.ErrNotFound { @@ -220,7 +221,8 @@ func (r *UnitRepository) Delete(ctx context.Context, uuid string) error { return fmt.Errorf("failed to delete unit from database: %w", err) } - log.Printf("Deleted unit: UUID=%s, Org=%s, Name=%s", uuid, org.Name, unit.Name) + log.Printf("Deleted unit: UUID=%s, Org=%s (%s), Name=%s, BlobPath=%s", + uuid, org.Name, org.ID, unit.Name, blobPath) return nil } @@ -241,8 +243,8 @@ func (r *UnitRepository) Download(ctx context.Context, uuid string) ([]byte, err return nil, fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Download from blob storage return r.blobStore.Download(ctx, blobPath) @@ -291,8 +293,8 @@ func (r *UnitRepository) Upload(ctx context.Context, uuid string, data []byte, l return fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Upload to blob storage if err := r.blobStore.Upload(ctx, blobPath, data, lockID); err != nil { @@ -327,8 +329,8 @@ func (r *UnitRepository) Lock(ctx context.Context, uuid string, lockInfo *storag return fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Lock in blob storage if err := r.blobStore.Lock(ctx, blobPath, lockInfo); err != nil { @@ -365,8 +367,8 @@ func (r *UnitRepository) Unlock(ctx context.Context, uuid string, lockID string) return fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) // Unlock in blob storage if err := r.blobStore.Unlock(ctx, blobPath, lockID); err != nil { @@ -403,8 +405,8 @@ func (r *UnitRepository) ListVersions(ctx context.Context, uuid string) ([]*stor return nil, fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) return r.blobStore.ListVersions(ctx, blobPath) } @@ -426,8 +428,8 @@ func (r *UnitRepository) RestoreVersion(ctx context.Context, uuid string, versio return fmt.Errorf(errMsgOrgNotFound, err) } - // Construct blob path - blobPath := fmt.Sprintf("%s/%s", org.Name, unit.Name) + // Construct UUID-based blob path: {org-uuid}/{unit-uuid} + blobPath := fmt.Sprintf("%s/%s", org.ID, unit.ID) return r.blobStore.RestoreVersion(ctx, blobPath, versionTimestamp, lockID) } diff --git a/taco/internal/tfe/workspaces.go b/taco/internal/tfe/workspaces.go index 6eac17bde..8b9cb3cce 100644 --- a/taco/internal/tfe/workspaces.go +++ b/taco/internal/tfe/workspaces.go @@ -139,24 +139,24 @@ func convertWorkspaceToStateID(workspaceID string) string { // getOrgFromContext extracts organization identifier from the echo context // The org is set by authentication middleware (JWT contains org claim) -// Falls back to "default" if no org is found -func getOrgFromContext(c echo.Context) string { +// Returns error if no organization context is found +func getOrgFromContext(c echo.Context) (string, error) { // Try jwt_org first (set by RequireAuth middleware from JWT claims) if jwtOrg := c.Get("jwt_org"); jwtOrg != nil { if orgStr, ok := jwtOrg.(string); ok && orgStr != "" { - return orgStr + return orgStr, nil } } // Try organization_id (set by WebhookAuth middleware) if orgID := c.Get("organization_id"); orgID != nil { if orgStr, ok := orgID.(string); ok && orgStr != "" { - return orgStr + return orgStr, nil } } - // Fall back to default org - return "default" + // No organization context found - this is an error condition + return "", fmt.Errorf("no organization context found in request") } // parseOrgParam parses organization parameter in format "display:identifier" or just "identifier" @@ -418,7 +418,14 @@ func (h *TfeHandler) LockWorkspace(c echo.Context) error { fmt.Printf("LockWorkspace: workspaceID=%s, workspaceName=%s\n", workspaceID, workspaceName) // Get org from authentication context (JWT claim or webhook header) - orgIdentifier := getOrgFromContext(c) + orgIdentifier, err := getOrgFromContext(c) + if err != nil { + fmt.Printf("LockWorkspace: %v\n", err) + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "Organization context required", + "detail": err.Error(), + }) + } fmt.Printf("LockWorkspace: orgIdentifier=%s (from auth context)\n", orgIdentifier) // Resolve to UUID/UUID path @@ -530,7 +537,14 @@ func (h *TfeHandler) UnlockWorkspace(c echo.Context) error { workspaceName := convertWorkspaceToStateID(workspaceID) // Get org from authentication context (JWT claim or webhook header) - orgIdentifier := getOrgFromContext(c) + orgIdentifier, err := getOrgFromContext(c) + if err != nil { + fmt.Printf("UnlockWorkspace: %v\n", err) + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "Organization context required", + "detail": err.Error(), + }) + } // Resolve to UUID/UUID path stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) @@ -612,7 +626,14 @@ func (h *TfeHandler) ForceUnlockWorkspace(c echo.Context) error { workspaceName := convertWorkspaceToStateID(workspaceID) // Get org from authentication context (JWT claim or webhook header) - orgIdentifier := getOrgFromContext(c) + orgIdentifier, err := getOrgFromContext(c) + if err != nil { + fmt.Printf("ForceUnlockWorkspace: %v\n", err) + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "Organization context required", + "detail": err.Error(), + }) + } // Resolve to UUID/UUID path stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) @@ -689,7 +710,14 @@ func (h *TfeHandler) GetCurrentStateVersion(c echo.Context) error { workspaceName := convertWorkspaceToStateID(workspaceID) // Get org from authentication context (JWT claim or webhook header) - orgIdentifier := getOrgFromContext(c) + orgIdentifier, err := getOrgFromContext(c) + if err != nil { + fmt.Printf("GetCurrentStateVersion: %v\n", err) + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "Organization context required", + "detail": err.Error(), + }) + } // Resolve to UUID/UUID path stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) @@ -773,7 +801,14 @@ func (h *TfeHandler) CreateStateVersion(c echo.Context) error { workspaceName := convertWorkspaceToStateID(workspaceID) // Get org from authentication context (JWT claim or webhook header) - orgIdentifier := getOrgFromContext(c) + orgIdentifier, err := getOrgFromContext(c) + if err != nil { + fmt.Printf("CreateStateVersion: %v\n", err) + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "Organization context required", + "detail": err.Error(), + }) + } // Resolve to UUID/UUID path stateID, err := h.convertWorkspaceToStateIDWithOrg(c.Request().Context(), orgIdentifier, workspaceName) diff --git a/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx b/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx index 32d522b80..271379576 100644 --- a/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx +++ b/ui/src/routes/_authenticated/_dashboard/dashboard/units.$unitId.tsx @@ -345,7 +345,7 @@ function RouteComponent() { {`terraform { cloud { hostname = "${publicHostname}" - organization = "${organisationName}:${organisationId}" + organization = "${organisationName ? `${organisationName}:` : ''}${organisationId}" workspaces { name = "${unit.name}" } @@ -356,7 +356,7 @@ function RouteComponent() { content={`terraform { cloud { hostname = "${publicHostname}" - organization = "${organisationName}:${organisationId}" + organization = "${organisationName ? `${organisationName}:` : ''}${organisationId}" workspaces { name = "${unit.name}" } From 36eec45b7dc0a05f30be350c7e4e8917c48089c6 Mon Sep 17 00:00:00 2001 From: Brian Reardon Date: Thu, 30 Oct 2025 15:58:13 -0700 Subject: [PATCH 5/6] fix build errors --- taco/internal/tfe/workspaces.go | 10 +++++++--- taco/internal/unit/handler.go | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/taco/internal/tfe/workspaces.go b/taco/internal/tfe/workspaces.go index 8b9cb3cce..89f06585a 100644 --- a/taco/internal/tfe/workspaces.go +++ b/taco/internal/tfe/workspaces.go @@ -1,6 +1,7 @@ package tfe import ( + "context" "crypto/md5" "encoding/base64" "encoding/json" @@ -266,8 +267,11 @@ func (h *TfeHandler) checkWorkspacePermission(c echo.Context, action string, wor // TFE endpoints: verify opaque token only (for clear API boundaries) var principal rbac.Principal if h.apiTokens != nil { - // Extract org from context or default to "default" - orgID := getOrgFromContext(c) + // Extract org from context + orgID, err := getOrgFromContext(c) + if err != nil { + return fmt.Errorf("failed to get organization context: %v", err) + } if tokenRecord, err := h.apiTokens.Verify(c.Request().Context(), orgID, token); err == nil { principal = rbac.Principal{ Subject: tokenRecord.Subject, @@ -453,7 +457,7 @@ func (h *TfeHandler) LockWorkspace(c echo.Context) error { } // Check if state exists, enot - _, err := h.stateStore.Get(c.Request().Context(), stateID) + _, err = h.stateStore.Get(c.Request().Context(), stateID) fmt.Printf("LockWorkspace: Get result, err=%v\n", err) if err == storage.ErrNotFound { fmt.Printf("LockWorkspace: Unit not found - no auto-creation\n") diff --git a/taco/internal/unit/handler.go b/taco/internal/unit/handler.go index 327cf53a0..8e78b415d 100644 --- a/taco/internal/unit/handler.go +++ b/taco/internal/unit/handler.go @@ -2,6 +2,7 @@ package unit import ( "context" + "fmt" "io" "net/http" "strings" From e9210b809c71de67ea0bfdcf11f30ff7c7584218 Mon Sep 17 00:00:00 2001 From: Brian Reardon Date: Thu, 30 Oct 2025 17:24:07 -0700 Subject: [PATCH 6/6] adjust models file to match index --- taco/internal/query/types/models.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taco/internal/query/types/models.go b/taco/internal/query/types/models.go index 7ba221322..1a5b0d28f 100644 --- a/taco/internal/query/types/models.go +++ b/taco/internal/query/types/models.go @@ -141,8 +141,8 @@ func (u *User) BeforeCreate(tx *gorm.DB) error { type Unit struct { ID string `gorm:"type:varchar(36);primaryKey"` - OrgID string `gorm:"type:varchar(36);index"` // Foreign key to organizations.id (UUID) - Name string `gorm:"type:varchar(255);not null;index"` + OrgID string `gorm:"type:varchar(36);index;uniqueIndex:idx_units_org_name"` // Foreign key to organizations.id (UUID) + Name string `gorm:"type:varchar(255);not null;index;uniqueIndex:idx_units_org_name"` Size int64 `gorm:"default:0"` UpdatedAt time.Time `gorm:"autoUpdateTime"` Locked bool `gorm:"default:false"`