diff --git a/backend/app/controllers/limits.go b/backend/app/controllers/limits.go index e11f5785..7d6fa7a2 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 c201adc9..34f3550b 100644 --- a/backend/app/controllers/organization.controller.go +++ b/backend/app/controllers/organization.controller.go @@ -2,12 +2,17 @@ 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" "time" + "unicode/utf8" "github.com/gin-gonic/gin" traceway "go.tracewayapp.com" @@ -111,3 +116,121 @@ 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 + } + + // Register binds the timezone as required. Here it is optional: the recovery + // form sends one, but this is a plain authenticated endpoint a CLI or script + // can also reach, and an org whose on-call schedules resolve against UTC is a + // better outcome than refusing to create it at all. + timezone := strings.TrimSpace(request.Timezone) + if timezone == "" { + timezone = "UTC" + } + // On-call schedule resolution is tz-aware calendar math, so an unparseable + // 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 + 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 + } + } + + 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, 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, + 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 00000000..2c21a745 --- /dev/null +++ b/backend/app/controllers/organization_create_test.go @@ -0,0 +1,229 @@ +//go:build !telemetry_ch && !transactional_pg && !telemetry_duckdb + +package controllers + +import ( + "database/sql" + "encoding/json" + "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 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) + 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 setCloudMode(t *testing.T, cloud bool) { + t.Helper() + prev := config.Config + mode := "" + if cloud { + mode = "true" + } + config.Init(&config.Cfg{CloudMode: mode}) + t.Cleanup(func() { config.Init(prev) }) +} + +func TestCreateOrganizationMakesCallerOwner(t *testing.T) { + tx, userId := newOrgTestUser(t, "solo@example.com") + + status, payload := createOrganization(t, tx, userId, `{"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, userId) + 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) { + tx, userId := newOrgTestUser(t, "notz@example.com") + + status, payload := createOrganization(t, tx, userId, `{"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) { + tx, userId := newOrgTestUser(t, "invalid@example.com") + + 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, 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 form, got %v", payload) + } + }) + } + + // A 100-rune name is the boundary and must be accepted. + 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) + } +} + +// 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) + + status, payload := createOrganization(t, tx, userId, `{"name":"First Org"}`) + if status != 201 { + t.Fatalf("first organization should be allowed, got %d (%v)", status, payload) + } + + 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, id int) error { + gotUserId = id + return &LimitExceededError{Message: "Your plan allows 1 organization"} + } + t.Cleanup(func() { OrganizationLimitHook = prevHook }) + + 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 form, got %v", payload["error"]) + } + 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, userId) + 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 907166fe..00530230 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 00000000..3fa55613 --- /dev/null +++ b/backend/app/controllers/routes_test.go @@ -0,0 +1,32 @@ +package controllers + +import ( + "slices" + "testing" + + "github.com/gin-gonic/gin" + "github.com/tracewayapp/traceway/backend/app/config" +) + +// 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. 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) + + if config.Config == nil { + config.Init(&config.Cfg{}) + t.Cleanup(func() { config.Init(nil) }) + } + + engine := gin.New() + RegisterControllers(engine.Group("/api")) + + 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 797ff99b..84bec362 100644 --- a/frontend/src/routes/setup/+page.svelte +++ b/frontend/src/routes/setup/+page.svelte @@ -1,8 +1,17 @@
-
-

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 hasNoOrganizations} + {#if step === 'choice'} +
+

You're not in an organization

+

+ Looks like you are not part of any organizations. Would you like to register for one? If + you were removed from one, an admin can also invite you back — logging out and returning + once the invitation arrives keeps you in that organization rather than starting a second. +

+ +
+ + +
+ {: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

+

+ 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 selectedOrgId !== null} + {#key selectedOrgId} + + {/key} + {/if} {/if} {/if}