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..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,51 +144,64 @@ 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 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 orgName string - if len(parts) == 2 { - // Format: "org/name" - orgName = parts[0] - name = parts[1] - } else { - // Format: "name" - use default org - orgName = "default" - name = parts[0] + if len(parts) != 2 { + return "", "", fmt.Errorf("invalid blob path format: expected 'org-uuid/unit-uuid', got '%s'", blobPath) + } + + 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) } - // Ensure org exists + // Verify org exists var org types.Organization - err = s.db.WithContext(ctx).Where("name = ?", orgName).First(&org).Error + err = s.db.WithContext(ctx).Where("id = ?", orgUUID).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(), - } - 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: %w", err) + return "", "", fmt.Errorf("organization not found: %s", orgUUID) + } + return "", "", fmt.Errorf("failed to lookup organization: %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) } + return "", "", fmt.Errorf("failed to lookup unit: %w", err) } - return org.ID, name, nil + return orgUUID, unitUUID, nil +} + +// 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 { + _, 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 } @@ -195,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 { @@ -207,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, @@ -256,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/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"` 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/repositories/unit_repository.go b/taco/internal/repositories/unit_repository.go index fbf63b05d..36d9a09f5 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,11 +72,21 @@ 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) } - // 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) @@ -85,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 @@ -116,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) @@ -197,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 { @@ -210,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 } @@ -231,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) @@ -281,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 { @@ -317,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 { @@ -355,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 { @@ -393,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) } @@ -416,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/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..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" @@ -82,15 +83,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 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) + } + + // 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 +135,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) +// 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, 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, nil + } + } + + // 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" +// 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") @@ -185,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, @@ -231,10 +316,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 +370,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 +394,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 +411,40 @@ 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, 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 + 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,11 +456,8 @@ 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) + _, 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") @@ -391,14 +531,35 @@ 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, 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) + 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 +620,35 @@ 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, 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) + 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 +702,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 +710,32 @@ 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, 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) + 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 +793,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 +801,32 @@ 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, 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) + 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 +1269,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/internal/unit/handler.go b/taco/internal/unit/handler.go index bd31895e8..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" @@ -111,7 +112,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) 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..271379576 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 ? `${organisationName}:` : ''}${organisationId}" workspaces { - name = "${unit.id}" + name = "${unit.name}" } } }`} @@ -355,9 +356,9 @@ function RouteComponent() { content={`terraform { cloud { hostname = "${publicHostname}" - organization = "${organisationId}" + organization = "${organisationName ? `${organisationName}:` : ''}${organisationId}" workspaces { - name = "${unit.id}" + name = "${unit.name}" } } }`}