From b3f3ff79767694118b4869585c17b9d050ab5e4d Mon Sep 17 00:00:00 2001 From: FrameAutomata Date: Wed, 26 Aug 2026 12:13:52 -0500 Subject: [PATCH 1/4] fix: let a user removed from their last organization recover The layout pins every zero-project account to /setup (+layout.svelte:159), and /setup's only branch for a user with no writable organization was a sentence with no action: You need an owner, admin, or user role in an organization to create projects. So removing someone from their last organization mid-session left them locked to a dead-end screen with no route forward, which is the stuck state in #292. Three parts: 1. POST /api/organizations. Organizations could only be created by register (auth.controller.go) and SSO finish-setup (oauth.controller.go), both account-creation paths, so an existing user had no way to make one. The new endpoint creates an organization with the caller as owner. Timezone defaults to UTC and is validated with time.LoadLocation, because on-call schedule resolution is tz-aware calendar math and a bad zone would surface much later as wrong shift boundaries. Cloud gating follows the established hook pattern (ProjectLimitHook, MemberLimitHook, CheckLimitHook); OrganizationLimitHook is keyed on the user rather than an org since it runs before the org exists. 2. /setup distinguishes the two cases it used to collapse. No organizations at all offers a create form; organizations that are all readonly keeps the existing message, which is correct advice there. 3. authState.organizations is hydrated from localStorage and only rewritten on login, so a membership removed mid-session stays cached. /setup now refreshes from /me/login-bundle on mount and renders a loading state until it resolves, so the recovery screen never acts on a stale list. Also adds routes_test.go, which registers the real route tree. Gin panics at registration on a wildcard conflict -- a boot-time crash no handler test would catch -- and POST /api/organizations is a static sibling of the /api/organizations/:organizationId/... subtree, the shape most likely to trip it. Nothing else in the suite covered this. Verified: go vet, gofmt, and the full backend suite pass; the four new endpoint tests (7 cases) pass; svelte-check reports 0 errors with no warnings in the changed file; routes_test.go passes under all three build-tag combinations. Closes #292 Co-Authored-By: Claude Opus 5 (1M context) --- backend/app/controllers/limits.go | 6 + .../controllers/organization.controller.go | 80 ++++++++ .../controllers/organization_create_test.go | 177 ++++++++++++++++++ backend/app/controllers/routes.go | 1 + backend/app/controllers/routes_test.go | 45 +++++ frontend/src/routes/setup/+page.svelte | 168 +++++++++++++---- 6 files changed, 436 insertions(+), 41 deletions(-) create mode 100644 backend/app/controllers/organization_create_test.go create mode 100644 backend/app/controllers/routes_test.go diff --git a/backend/app/controllers/limits.go b/backend/app/controllers/limits.go index e11f5785f..7d6fa7a25 100644 --- a/backend/app/controllers/limits.go +++ b/backend/app/controllers/limits.go @@ -15,3 +15,9 @@ var MemberLimitHook func(tx *sql.Tx, orgId int) error // CheckLimitHook caps synthetic checks per organization (cloud plans). Nil // means unlimited; a LimitExceededError surfaces as a 422 in the dialog. var CheckLimitHook func(tx *sql.Tx, orgId int) error + +// OrganizationLimitHook caps how many organizations a single user may own +// (cloud plans). Unlike the hooks above it is keyed on the user, not an +// organization, because it runs before the organization exists. Nil means +// unlimited; a LimitExceededError surfaces as a 422 in the dialog. +var OrganizationLimitHook func(tx *sql.Tx, userId int) error diff --git a/backend/app/controllers/organization.controller.go b/backend/app/controllers/organization.controller.go index c201adc91..c23cb278e 100644 --- a/backend/app/controllers/organization.controller.go +++ b/backend/app/controllers/organization.controller.go @@ -2,12 +2,15 @@ package controllers import ( "database/sql" + "errors" "github.com/tracewayapp/traceway/backend/app/db" "github.com/tracewayapp/traceway/backend/app/middleware" "github.com/tracewayapp/traceway/backend/app/models" "github.com/tracewayapp/traceway/backend/app/repositories/transactional" "net/http" + "strings" "time" + "unicode/utf8" "github.com/gin-gonic/gin" traceway "go.tracewayapp.com" @@ -111,3 +114,80 @@ func (c *organizationController) UpdateSettings(ctx *gin.Context) { } var OrganizationController = organizationController{} + +type CreateOrganizationRequest struct { + Name string `json:"name" binding:"required"` + Timezone string `json:"timezone"` +} + +func validateOrganizationName(name string) string { + nameLen := utf8.RuneCountInString(name) + if nameLen < 1 || nameLen > 100 { + return "Organization name must be between 1 and 100 characters" + } + return "" +} + +// Create lets an authenticated user start a new organization they own. Every +// other org-creating path (register, SSO finish-setup) is an account-creation +// path, so a user removed from their last organization otherwise has no way +// back into a usable account. +func (c *organizationController) Create(ctx *gin.Context) { + var request CreateOrganizationRequest + if err := ctx.ShouldBindJSON(&request); err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "Organization name is required"}) + return + } + + name := strings.TrimSpace(request.Name) + if message := validateOrganizationName(name); message != "" { + ctx.JSON(http.StatusUnprocessableEntity, gin.H{"error": message}) + return + } + + // Unlike register, which takes the timezone from a required form field, this + // endpoint is reached from a recovery screen with nothing to prefill from. + timezone := strings.TrimSpace(request.Timezone) + if timezone == "" { + timezone = "UTC" + } + // On-call schedule resolution is tz-aware calendar math, so an unparseable + // zone here would surface much later as wrong shift boundaries. + if _, err := time.LoadLocation(timezone); err != nil { + ctx.JSON(http.StatusUnprocessableEntity, gin.H{"error": "Unknown timezone"}) + return + } + + userId := middleware.GetUserId(ctx) + tx := db.GetTx(ctx) + + if OrganizationLimitHook != nil { + if err := OrganizationLimitHook(tx, userId); err != nil { + var limitErr *LimitExceededError + if errors.As(err, &limitErr) { + ctx.JSON(http.StatusUnprocessableEntity, gin.H{"error": limitErr.Message}) + return + } + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("organization limit hook failed: %w", err)) + return + } + } + + org, err := transactional.OrganizationRepository.Create(tx, name, timezone) + if err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to create organization: %w", err)) + return + } + + if _, err := transactional.OrganizationRepository.AddUser(tx, org.Id, userId, "owner"); err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to add creator to organization: %w", err)) + return + } + + ctx.JSON(http.StatusCreated, models.UserOrganizationResponse{ + Id: org.Id, + Name: org.Name, + Role: "owner", + Timezone: org.Timezone, + }) +} diff --git a/backend/app/controllers/organization_create_test.go b/backend/app/controllers/organization_create_test.go new file mode 100644 index 000000000..de29bdb2b --- /dev/null +++ b/backend/app/controllers/organization_create_test.go @@ -0,0 +1,177 @@ +//go:build !telemetry_ch && !transactional_pg && !telemetry_duckdb + +package controllers + +import ( + "database/sql" + "encoding/json" + "strings" + "testing" + + "github.com/tracewayapp/traceway/backend/app/db" + "github.com/tracewayapp/traceway/backend/app/repositories/transactional" +) + +// createOrganization runs the handler against a real transaction and returns +// the recorder, so each case asserts on the wire response the dialog sees. +func createOrganization(t *testing.T, tx *sql.Tx, userId int, body string) (int, map[string]any) { + t.Helper() + c, recorder := newControllerTestContext(t, tx, userId, "POST", "/organizations", body) + OrganizationController.Create(c) + + var payload map[string]any + if recorder.Body.Len() > 0 { + if err := json.Unmarshal(recorder.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode response %q: %v", recorder.Body.String(), err) + } + } + return recorder.Code, payload +} + +func TestCreateOrganizationMakesCallerOwner(t *testing.T) { + setupSetupControllerDB(t) + + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + defer tx.Rollback() + + user, err := transactional.UserRepository.Create(tx, "solo@example.com", "Solo User", "hashed-password") + if err != nil { + t.Fatalf("create user: %v", err) + } + + status, payload := createOrganization(t, tx, user.Id, `{"name":"Recovered Org","timezone":"Europe/Belgrade"}`) + if status != 201 { + t.Fatalf("expected 201, got %d (%v)", status, payload) + } + if payload["role"] != "owner" { + t.Errorf("expected creator to be owner, got %v", payload["role"]) + } + if payload["name"] != "Recovered Org" { + t.Errorf("unexpected name: %v", payload["name"]) + } + if payload["timezone"] != "Europe/Belgrade" { + t.Errorf("unexpected timezone: %v", payload["timezone"]) + } + + // The whole point of the endpoint: the user can now reach a usable account. + orgId := int(payload["id"].(float64)) + role, err := transactional.OrganizationRepository.GetUserRole(tx, orgId, user.Id) + if err != nil { + t.Fatalf("get role: %v", err) + } + if role != "owner" { + t.Errorf("expected persisted role owner, got %q", role) + } +} + +func TestCreateOrganizationDefaultsTimezoneToUTC(t *testing.T) { + setupSetupControllerDB(t) + + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + defer tx.Rollback() + + user, err := transactional.UserRepository.Create(tx, "notz@example.com", "No TZ", "hashed-password") + if err != nil { + t.Fatalf("create user: %v", err) + } + + status, payload := createOrganization(t, tx, user.Id, `{"name":"No Timezone"}`) + if status != 201 { + t.Fatalf("expected 201, got %d (%v)", status, payload) + } + if payload["timezone"] != "UTC" { + t.Errorf("expected UTC default, got %v", payload["timezone"]) + } +} + +func TestCreateOrganizationValidation(t *testing.T) { + setupSetupControllerDB(t) + + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + defer tx.Rollback() + + user, err := transactional.UserRepository.Create(tx, "invalid@example.com", "Invalid", "hashed-password") + if err != nil { + t.Fatalf("create user: %v", err) + } + + cases := []struct { + name string + body string + wantStatus int + }{ + {"missing name", `{}`, 400}, + {"blank name", `{"name":" "}`, 422}, + {"name too long", `{"name":"` + strings.Repeat("a", 101) + `"}`, 422}, + {"unknown timezone", `{"name":"Fine","timezone":"Mars/Olympus"}`, 422}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + status, payload := createOrganization(t, tx, user.Id, tc.body) + if status != tc.wantStatus { + t.Fatalf("expected %d, got %d (%v)", tc.wantStatus, status, payload) + } + if payload["error"] == nil || payload["error"] == "" { + t.Errorf("expected an error message for the dialog, got %v", payload) + } + }) + } + + // A 100-rune name is the boundary and must be accepted. + status, payload := createOrganization(t, tx, user.Id, `{"name":"`+strings.Repeat("a", 100)+`"}`) + if status != 201 { + t.Fatalf("expected 100-char name to be accepted, got %d (%v)", status, payload) + } +} + +func TestCreateOrganizationLimitHookBlocks(t *testing.T) { + setupSetupControllerDB(t) + + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + defer tx.Rollback() + + user, err := transactional.UserRepository.Create(tx, "capped@example.com", "Capped", "hashed-password") + if err != nil { + t.Fatalf("create user: %v", err) + } + + prevHook := OrganizationLimitHook + gotUserId := 0 + OrganizationLimitHook = func(_ *sql.Tx, userId int) error { + gotUserId = userId + return &LimitExceededError{Message: "Your plan allows 1 organization"} + } + t.Cleanup(func() { OrganizationLimitHook = prevHook }) + + status, payload := createOrganization(t, tx, user.Id, `{"name":"Second Org"}`) + if status != 422 { + t.Fatalf("expected 422 from the limit hook, got %d (%v)", status, payload) + } + if payload["error"] != "Your plan allows 1 organization" { + t.Errorf("expected the hook's message to reach the dialog, got %v", payload["error"]) + } + if gotUserId != user.Id { + t.Errorf("hook should be keyed on the creating user, got %d want %d", gotUserId, user.Id) + } + + orgs, err := transactional.OrganizationRepository.FindByUserId(tx, user.Id) + if err != nil { + t.Fatalf("find orgs: %v", err) + } + if len(orgs) != 0 { + t.Errorf("blocked request must not create an organization, found %d", len(orgs)) + } +} diff --git a/backend/app/controllers/routes.go b/backend/app/controllers/routes.go index 907166fe0..00530230e 100644 --- a/backend/app/controllers/routes.go +++ b/backend/app/controllers/routes.go @@ -227,6 +227,7 @@ func RegisterControllers(router *gin.RouterGroup) { router.GET("/password-reset/:token", middleware.RateLimitPerIP(30, time.Minute), PasswordResetController.ValidateToken) router.POST("/password-reset/:token", middleware.RateLimitPerIP(10, time.Minute), middleware.BufferAuthBody, middleware.Transactional, PasswordResetController.ResetPassword) + router.POST("/organizations", middleware.UseAppAuth, middleware.Transactional, OrganizationController.Create) router.GET("/organizations/:organizationId/settings", middleware.UseAppAuth, middleware.RequireAdminAccess, OrganizationController.GetSettings) router.PUT("/organizations/:organizationId/settings", middleware.UseAppAuth, middleware.RequireAdminAccess, middleware.Transactional, OrganizationController.UpdateSettings) router.GET("/organizations/:organizationId/members", middleware.UseAppAuth, middleware.RequireAdminAccess, OrganizationController.GetMembers) diff --git a/backend/app/controllers/routes_test.go b/backend/app/controllers/routes_test.go new file mode 100644 index 000000000..79282646a --- /dev/null +++ b/backend/app/controllers/routes_test.go @@ -0,0 +1,45 @@ +package controllers + +import ( + "testing" + + "github.com/gin-gonic/gin" + "github.com/tracewayapp/traceway/backend/app/config" +) + +// Gin panics at registration time when a new path conflicts with an existing +// wildcard, which would take the process down at boot rather than failing any +// handler test. Registering the real tree is the only way to catch it, and +// nothing else in the suite does. +func TestRouteTreeRegistersWithoutConflict(t *testing.T) { + gin.SetMode(gin.TestMode) + + prevConfig := config.Config + if config.Config == nil { + config.Init(&config.Cfg{}) + } + t.Cleanup(func() { config.Init(prevConfig) }) + + defer func() { + if r := recover(); r != nil { + t.Fatalf("route registration panicked: %v", r) + } + }() + + engine := gin.New() + RegisterControllers(engine.Group("/api")) + + // POST /api/organizations is a static sibling of the + // /api/organizations/:organizationId/... subtree, the shape most likely to + // trip the wildcard conflict above. + found := false + for _, route := range engine.Routes() { + if route.Method == "POST" && route.Path == "/api/organizations" { + found = true + break + } + } + if !found { + t.Error("POST /api/organizations was not registered") + } +} diff --git a/frontend/src/routes/setup/+page.svelte b/frontend/src/routes/setup/+page.svelte index 797ff99b1..c507d4887 100644 --- a/frontend/src/routes/setup/+page.svelte +++ b/frontend/src/routes/setup/+page.svelte @@ -1,8 +1,15 @@
-
-

Set Up Projects

-

- Let your coding agent propose the project setup for your approval, or create projects - manually. -

-
+ {#if refreshing} + + {:else if hasNoOrganizations} +
+

Create an Organization

+

+ You're not a member of any organization right now — if you were removed from one, an admin + can invite you back. You can also start your own. +

+
- {#if writableOrgs.length === 0} -

- You need an owner, admin, or user role in an organization to create projects. -

- {:else} - {#if writableOrgs.length > 1} -
- - { - if (val) selectedOrgId = Number(val); - }} - > - - {selectedOrgName || 'Select organization'} - - - {#each writableOrgs as org (org.id)} - {org.name} - {/each} - - +
+
+ +
- {/if} - {#if selectedOrgId !== null} - {#key selectedOrgId} - - {/key} + {#if createError} +

{createError}

+ {/if} + + + + {:else} +
+

Set Up Projects

+

+ Let your coding agent propose the project setup for your approval, or create projects + manually. +

+
+ + {#if writableOrgs.length === 0} +

+ You need an owner, admin, or user role in an organization to create projects. +

+ {:else} + {#if writableOrgs.length > 1} +
+ + { + if (val) selectedOrgId = Number(val); + }} + > + + {selectedOrgName || 'Select organization'} + + + {#each writableOrgs as org (org.id)} + {org.name} + {/each} + + +
+ {/if} + + {#if selectedOrgId !== null} + {#key selectedOrgId} + + {/key} + {/if} {/if} {/if}
From b7c96caa3f550345c860d30a785ee4a075efec2c Mon Sep 17 00:00:00 2001 From: FrameAutomata Date: Wed, 26 Aug 2026 12:42:26 -0500 Subject: [PATCH 2/4] fix: gate org creation, run registration hooks, drop the render gate Follow-up to the review of this branch. Three substantive fixes plus cleanups, all found after actually running the app. Security/policy: POST /api/organizations had no self-hosted single-org gate. Register enforces one (auth.controller.go:112) but the new handler skipped it, and the route carries no role guard and OrganizationLimitHook is nil outside cloud -- so any authenticated user of any role, readonly included, could mint an organization and own it. Now mirrors Register's rule. It answers 422 rather than Register's 409 because the message has to reach the recovery form, and api.ts only extracts response bodies from 401/403/422 -- a 409 surfaces to the user as "API Error: Conflict". Verified in a browser: the message renders in the form. Correctness: the handler skipped PostRegistrationHooks, which both other org-creating paths run for every new org+owner pair. That is the cloud build's provisioning seam, so organizations created here were silently missing whatever cloud wires there. Now runs them, which is why the handler loads the full user rather than just its id. Frontend: the `refreshing` render gate blocked the whole page on a /me/login-bundle round trip for every user, to avoid a branch flip in one rare case. A screenshot caught it still spinning at 3.5s. The cached org list is correct for every case except the mid-session removal, so the page now renders immediately and lets the response correct the branch. Reuse/simplification: - oncall.LoadTimezone instead of an inline time.LoadLocation plus a third bespoke "unknown timezone" message - ErrorAlert instead of a raw

, matching every other inline form error in the app - routes_test.go: dropped the defer/recover, which discarded the panic stack naming the conflicting path; slices.ContainsFunc; cleanup only in the branch that mutates config - organization_create_test.go: newOrgTestUser helper collapses a 10-line preamble repeated four times - dropped a redundant selectedOrgId write the $effect already owns New tests: self-hosted allows only one org, cloud allows more, PostRegistrationHooks run. Co-Authored-By: Claude Opus 5 (1M context) --- .../controllers/organization.controller.go | 49 ++++- .../controllers/organization_create_test.go | 168 ++++++++++++------ backend/app/controllers/routes_test.go | 29 +-- frontend/src/routes/setup/+page.svelte | 19 +- 4 files changed, 170 insertions(+), 95 deletions(-) diff --git a/backend/app/controllers/organization.controller.go b/backend/app/controllers/organization.controller.go index c23cb278e..d2596850c 100644 --- a/backend/app/controllers/organization.controller.go +++ b/backend/app/controllers/organization.controller.go @@ -3,9 +3,11 @@ package controllers import ( "database/sql" "errors" + "github.com/tracewayapp/traceway/backend/app/config" "github.com/tracewayapp/traceway/backend/app/db" "github.com/tracewayapp/traceway/backend/app/middleware" "github.com/tracewayapp/traceway/backend/app/models" + "github.com/tracewayapp/traceway/backend/app/oncall" "github.com/tracewayapp/traceway/backend/app/repositories/transactional" "net/http" "strings" @@ -152,15 +154,34 @@ func (c *organizationController) Create(ctx *gin.Context) { timezone = "UTC" } // On-call schedule resolution is tz-aware calendar math, so an unparseable - // zone here would surface much later as wrong shift boundaries. - if _, err := time.LoadLocation(timezone); err != nil { - ctx.JSON(http.StatusUnprocessableEntity, gin.H{"error": "Unknown timezone"}) + // zone would surface much later as wrong shift boundaries. + if _, err := oncall.LoadTimezone(timezone); err != nil { + ctx.JSON(http.StatusUnprocessableEntity, gin.H{"error": err.Error()}) return } userId := middleware.GetUserId(ctx) tx := db.GetTx(ctx) + // Self-hosted instances allow exactly one organization; Register enforces the + // same rule (as a 409). This answers 422 instead because the message has to + // reach the recovery form, and api.ts only extracts bodies from 401/403/422 -- + // a 409 would surface to the user as "API Error: Conflict". + // Without this an authenticated user of any role -- readonly + // included -- could mint an organization here and own it, since the route + // carries no role guard and OrganizationLimitHook is nil outside cloud. + if config.Config.CloudMode != "true" { + hasOrganizations, err := transactional.OrganizationRepository.HasOrganizations(tx) + if err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to check for existing organizations: %w", err)) + return + } + if hasOrganizations { + ctx.JSON(http.StatusUnprocessableEntity, gin.H{"error": "This instance already has an organization. Ask an administrator to invite you to it."}) + return + } + } + if OrganizationLimitHook != nil { if err := OrganizationLimitHook(tx, userId); err != nil { var limitErr *LimitExceededError @@ -173,17 +194,37 @@ func (c *organizationController) Create(ctx *gin.Context) { } } + user, err := transactional.UserRepository.FindById(tx, userId) + if err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to load creating user: %w", err)) + return + } + if user == nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("authenticated user %d not found", userId)) + return + } + org, err := transactional.OrganizationRepository.Create(tx, name, timezone) if err != nil { ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to create organization: %w", err)) return } - if _, err := transactional.OrganizationRepository.AddUser(tx, org.Id, userId, "owner"); err != nil { + if _, err := transactional.OrganizationRepository.AddUser(tx, org.Id, user.Id, "owner"); err != nil { ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("failed to add creator to organization: %w", err)) return } + // Register and FinishSetup both run these for every new org+owner pair; it is + // the cloud build's provisioning seam, so an organization created here must + // not skip it. + for _, hook := range PostRegistrationHooks { + if err := hook(tx, org, user); err != nil { + ctx.AbortWithError(http.StatusInternalServerError, traceway.NewStackTraceErrorf("post-registration hook failed: %w", err)) + return + } + } + ctx.JSON(http.StatusCreated, models.UserOrganizationResponse{ Id: org.Id, Name: org.Name, diff --git a/backend/app/controllers/organization_create_test.go b/backend/app/controllers/organization_create_test.go index de29bdb2b..2c21a7451 100644 --- a/backend/app/controllers/organization_create_test.go +++ b/backend/app/controllers/organization_create_test.go @@ -8,12 +8,34 @@ import ( "strings" "testing" + "github.com/tracewayapp/traceway/backend/app/config" "github.com/tracewayapp/traceway/backend/app/db" + "github.com/tracewayapp/traceway/backend/app/models" "github.com/tracewayapp/traceway/backend/app/repositories/transactional" ) +// newOrgTestUser gives each case a fresh database holding exactly one user and +// no organization. createSetupTestAccount is not reusable here: it creates an +// organization, which is the thing these tests need to be absent. +func newOrgTestUser(t *testing.T, email string) (*sql.Tx, int) { + t.Helper() + setupSetupControllerDB(t) + + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + t.Cleanup(func() { tx.Rollback() }) + + user, err := transactional.UserRepository.Create(tx, email, "Test User", "hashed-password") + if err != nil { + t.Fatalf("create user: %v", err) + } + return tx, user.Id +} + // createOrganization runs the handler against a real transaction and returns -// the recorder, so each case asserts on the wire response the dialog sees. +// the recorder, so each case asserts on the wire response the form sees. func createOrganization(t *testing.T, tx *sql.Tx, userId int, body string) (int, map[string]any) { t.Helper() c, recorder := newControllerTestContext(t, tx, userId, "POST", "/organizations", body) @@ -28,21 +50,21 @@ func createOrganization(t *testing.T, tx *sql.Tx, userId int, body string) (int, return recorder.Code, payload } -func TestCreateOrganizationMakesCallerOwner(t *testing.T) { - setupSetupControllerDB(t) - - tx, err := db.DB.Begin() - if err != nil { - t.Fatalf("begin: %v", err) +func setCloudMode(t *testing.T, cloud bool) { + t.Helper() + prev := config.Config + mode := "" + if cloud { + mode = "true" } - defer tx.Rollback() + config.Init(&config.Cfg{CloudMode: mode}) + t.Cleanup(func() { config.Init(prev) }) +} - user, err := transactional.UserRepository.Create(tx, "solo@example.com", "Solo User", "hashed-password") - if err != nil { - t.Fatalf("create user: %v", err) - } +func TestCreateOrganizationMakesCallerOwner(t *testing.T) { + tx, userId := newOrgTestUser(t, "solo@example.com") - status, payload := createOrganization(t, tx, user.Id, `{"name":"Recovered Org","timezone":"Europe/Belgrade"}`) + status, payload := createOrganization(t, tx, userId, `{"name":"Recovered Org","timezone":"Europe/Belgrade"}`) if status != 201 { t.Fatalf("expected 201, got %d (%v)", status, payload) } @@ -58,7 +80,7 @@ func TestCreateOrganizationMakesCallerOwner(t *testing.T) { // The whole point of the endpoint: the user can now reach a usable account. orgId := int(payload["id"].(float64)) - role, err := transactional.OrganizationRepository.GetUserRole(tx, orgId, user.Id) + role, err := transactional.OrganizationRepository.GetUserRole(tx, orgId, userId) if err != nil { t.Fatalf("get role: %v", err) } @@ -68,20 +90,9 @@ func TestCreateOrganizationMakesCallerOwner(t *testing.T) { } func TestCreateOrganizationDefaultsTimezoneToUTC(t *testing.T) { - setupSetupControllerDB(t) - - tx, err := db.DB.Begin() - if err != nil { - t.Fatalf("begin: %v", err) - } - defer tx.Rollback() + tx, userId := newOrgTestUser(t, "notz@example.com") - user, err := transactional.UserRepository.Create(tx, "notz@example.com", "No TZ", "hashed-password") - if err != nil { - t.Fatalf("create user: %v", err) - } - - status, payload := createOrganization(t, tx, user.Id, `{"name":"No Timezone"}`) + status, payload := createOrganization(t, tx, userId, `{"name":"No Timezone"}`) if status != 201 { t.Fatalf("expected 201, got %d (%v)", status, payload) } @@ -91,18 +102,7 @@ func TestCreateOrganizationDefaultsTimezoneToUTC(t *testing.T) { } func TestCreateOrganizationValidation(t *testing.T) { - setupSetupControllerDB(t) - - tx, err := db.DB.Begin() - if err != nil { - t.Fatalf("begin: %v", err) - } - defer tx.Rollback() - - user, err := transactional.UserRepository.Create(tx, "invalid@example.com", "Invalid", "hashed-password") - if err != nil { - t.Fatalf("create user: %v", err) - } + tx, userId := newOrgTestUser(t, "invalid@example.com") cases := []struct { name string @@ -117,57 +117,109 @@ func TestCreateOrganizationValidation(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - status, payload := createOrganization(t, tx, user.Id, tc.body) + status, payload := createOrganization(t, tx, userId, tc.body) if status != tc.wantStatus { t.Fatalf("expected %d, got %d (%v)", tc.wantStatus, status, payload) } if payload["error"] == nil || payload["error"] == "" { - t.Errorf("expected an error message for the dialog, got %v", payload) + t.Errorf("expected an error message for the form, got %v", payload) } }) } // A 100-rune name is the boundary and must be accepted. - status, payload := createOrganization(t, tx, user.Id, `{"name":"`+strings.Repeat("a", 100)+`"}`) + status, payload := createOrganization(t, tx, userId, `{"name":"`+strings.Repeat("a", 100)+`"}`) if status != 201 { t.Fatalf("expected 100-char name to be accepted, got %d (%v)", status, payload) } } -func TestCreateOrganizationLimitHookBlocks(t *testing.T) { - setupSetupControllerDB(t) +// Self-hosted instances allow exactly one organization. Without this the route, +// which carries no role guard, would let any authenticated user own a new one. +func TestCreateOrganizationSelfHostedAllowsOnlyOne(t *testing.T) { + tx, userId := newOrgTestUser(t, "selfhosted@example.com") + setCloudMode(t, false) - tx, err := db.DB.Begin() - if err != nil { - t.Fatalf("begin: %v", err) + status, payload := createOrganization(t, tx, userId, `{"name":"First Org"}`) + if status != 201 { + t.Fatalf("first organization should be allowed, got %d (%v)", status, payload) } - defer tx.Rollback() - user, err := transactional.UserRepository.Create(tx, "capped@example.com", "Capped", "hashed-password") - if err != nil { - t.Fatalf("create user: %v", err) + status, payload = createOrganization(t, tx, userId, `{"name":"Second Org"}`) + if status != 422 { + t.Fatalf("expected 422 for a second self-hosted organization, got %d (%v)", status, payload) } + if payload["error"] == nil { + t.Error("expected an actionable message pointing at an administrator") + } +} + +func TestCreateOrganizationCloudAllowsMoreThanOne(t *testing.T) { + tx, userId := newOrgTestUser(t, "cloud@example.com") + setCloudMode(t, true) + + if status, payload := createOrganization(t, tx, userId, `{"name":"First Org"}`); status != 201 { + t.Fatalf("first organization: expected 201, got %d (%v)", status, payload) + } + if status, payload := createOrganization(t, tx, userId, `{"name":"Second Org"}`); status != 201 { + t.Fatalf("cloud must allow a second organization, got %d (%v)", status, payload) + } +} + +// Register and FinishSetup run these for every new org+owner pair; cloud wires +// provisioning there, so an organization created here must not skip them. +func TestCreateOrganizationRunsPostRegistrationHooks(t *testing.T) { + tx, userId := newOrgTestUser(t, "hooks@example.com") + + var gotOrg *models.Organization + var gotUser *models.User + prev := PostRegistrationHooks + PostRegistrationHooks = []func(*sql.Tx, *models.Organization, *models.User) error{ + func(_ *sql.Tx, org *models.Organization, user *models.User) error { + gotOrg, gotUser = org, user + return nil + }, + } + t.Cleanup(func() { PostRegistrationHooks = prev }) + + status, payload := createOrganization(t, tx, userId, `{"name":"Hooked Org"}`) + if status != 201 { + t.Fatalf("expected 201, got %d (%v)", status, payload) + } + if gotOrg == nil || gotUser == nil { + t.Fatal("PostRegistrationHooks did not run for an organization created here") + } + if gotOrg.Name != "Hooked Org" { + t.Errorf("hook got org %q", gotOrg.Name) + } + if gotUser.Id != userId { + t.Errorf("hook got user %d, want %d", gotUser.Id, userId) + } +} + +func TestCreateOrganizationLimitHookBlocks(t *testing.T) { + tx, userId := newOrgTestUser(t, "capped@example.com") prevHook := OrganizationLimitHook gotUserId := 0 - OrganizationLimitHook = func(_ *sql.Tx, userId int) error { - gotUserId = userId + OrganizationLimitHook = func(_ *sql.Tx, id int) error { + gotUserId = id return &LimitExceededError{Message: "Your plan allows 1 organization"} } t.Cleanup(func() { OrganizationLimitHook = prevHook }) - status, payload := createOrganization(t, tx, user.Id, `{"name":"Second Org"}`) + status, payload := createOrganization(t, tx, userId, `{"name":"Second Org"}`) if status != 422 { t.Fatalf("expected 422 from the limit hook, got %d (%v)", status, payload) } if payload["error"] != "Your plan allows 1 organization" { - t.Errorf("expected the hook's message to reach the dialog, got %v", payload["error"]) + t.Errorf("expected the hook's message to reach the form, got %v", payload["error"]) } - if gotUserId != user.Id { - t.Errorf("hook should be keyed on the creating user, got %d want %d", gotUserId, user.Id) + if gotUserId != userId { + t.Errorf("hook should be keyed on the creating user, got %d want %d", gotUserId, userId) } - orgs, err := transactional.OrganizationRepository.FindByUserId(tx, user.Id) + orgs, err := transactional.OrganizationRepository.FindByUserId(tx, userId) if err != nil { t.Fatalf("find orgs: %v", err) } diff --git a/backend/app/controllers/routes_test.go b/backend/app/controllers/routes_test.go index 79282646a..3fa55613e 100644 --- a/backend/app/controllers/routes_test.go +++ b/backend/app/controllers/routes_test.go @@ -1,45 +1,32 @@ package controllers import ( + "slices" "testing" "github.com/gin-gonic/gin" "github.com/tracewayapp/traceway/backend/app/config" ) -// Gin panics at registration time when a new path conflicts with an existing +// Gin panics at registration when a new path conflicts with an existing // wildcard, which would take the process down at boot rather than failing any // handler test. Registering the real tree is the only way to catch it, and -// nothing else in the suite does. +// nothing else in the suite does. The panic is deliberately left uncaught: its +// stack names the conflicting path, which a t.Fatalf would discard. func TestRouteTreeRegistersWithoutConflict(t *testing.T) { gin.SetMode(gin.TestMode) - prevConfig := config.Config if config.Config == nil { config.Init(&config.Cfg{}) + t.Cleanup(func() { config.Init(nil) }) } - t.Cleanup(func() { config.Init(prevConfig) }) - - defer func() { - if r := recover(); r != nil { - t.Fatalf("route registration panicked: %v", r) - } - }() engine := gin.New() RegisterControllers(engine.Group("/api")) - // POST /api/organizations is a static sibling of the - // /api/organizations/:organizationId/... subtree, the shape most likely to - // trip the wildcard conflict above. - found := false - for _, route := range engine.Routes() { - if route.Method == "POST" && route.Path == "/api/organizations" { - found = true - break - } - } - if !found { + if !slices.ContainsFunc(engine.Routes(), func(r gin.RouteInfo) bool { + return r.Method == "POST" && r.Path == "/api/organizations" + }) { t.Error("POST /api/organizations was not registered") } } diff --git a/frontend/src/routes/setup/+page.svelte b/frontend/src/routes/setup/+page.svelte index c507d4887..ee268753d 100644 --- a/frontend/src/routes/setup/+page.svelte +++ b/frontend/src/routes/setup/+page.svelte @@ -1,6 +1,6 @@

- {#if refreshing} - - {:else if hasNoOrganizations} + {#if hasNoOrganizations}

Create an Organization

@@ -98,9 +95,7 @@ />

- {#if createError} -

{createError}

- {/if} + + +
+ {:else} +
+

Create an Organization

+

+ Name it and pick the timezone its on-call schedules should follow. You'll own it, and can + invite the rest of your team afterwards. +

- + +
+ + +
+ +
+ + + + {timezone} + + + {#each timezones as tz (tz)} + + {#snippet children({ selected })} + {tz} + {#if selected} + + {/if} + {/snippet} + + {/each} + + +
- - + + +
+ + +
+ + {/if} {:else}

Set Up Projects