From 5c90ab5b83edad270ef8dc835e9762010b379f4a Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 09:02:19 +0100 Subject: [PATCH 1/5] feat: add organization invitations Invite by email only, owner-only sending, 5-hour single-use tokens delivered through MailX's own outbound pipeline (same pattern as password reset). A single combined accept endpoint handles both an already-logged-in invitee and a no-account-yet invitee (signup folded into acceptance, always using the invitation's own email). Org logo/inviter avatar are plain nullable URL fields shown in the invite email when set. Dedicated per-human rate limit on sending. Migration 000030 adds org_invitations, tenants.logo_url, humans.avatar_url. --- .ilana/architecture.md | 3 +- .ilana/decisions.md | 1 + .ilana/ledger.md | 3 + .ilana/milestones.md | 2 + .ilana/state.json | 4 +- cmd/mailx/abuseconfig.go | 2 + internal/api/abuse.go | 41 +++ internal/api/humanauth_handler.go | 92 +++++++ internal/api/openapi.go | 52 ++++ internal/api/routes.go | 8 + internal/database/humans.go | 11 +- .../000030_org_invitations.down.sql | 3 + .../migrations/000030_org_invitations.up.sql | 30 +++ internal/database/org_invitations.go | 169 ++++++++++++ internal/database/tenants.go | 7 +- internal/humanauth/org_invitation_test.go | 247 ++++++++++++++++++ internal/humanauth/service.go | 182 +++++++++++++ internal/ratelimit/policy.go | 11 + 18 files changed, 859 insertions(+), 9 deletions(-) create mode 100644 internal/database/migrations/000030_org_invitations.down.sql create mode 100644 internal/database/migrations/000030_org_invitations.up.sql create mode 100644 internal/database/org_invitations.go create mode 100644 internal/humanauth/org_invitation_test.go diff --git a/.ilana/architecture.md b/.ilana/architecture.md index 082d6ec..1a3cc6f 100644 --- a/.ilana/architecture.md +++ b/.ilana/architecture.md @@ -491,4 +491,5 @@ Redis queue polling (200ms..2s), not blocking primitives (multi-condition wake-u - New routes, registered directly on the API server's top-level mux (NOT inside the API-key-authenticated `/v1/` chain — `net/http.ServeMux` prefers the more specific `/v1/auth/*`/`/v1/orgs` patterns over the `/v1/` catch-all): `POST /v1/auth/signup`, `POST /v1/auth/login`, `POST /v1/auth/refresh`, `POST /v1/auth/logout` (public), `POST /v1/orgs`, `GET /v1/orgs` (require a human JWT via the new `humanAuthMiddleware`, distinct from `authenticateMiddleware`/`requireScope`). `api.Config.HumanAuth` is optional (nil disables these routes; existing API-key routes are unaffected either way). - New env var: `MAILX_JWT_SECRET` (same convention as `MAILX_API_KEY_PEPPER`; unset falls back to an ephemeral per-process secret with a startup warning — sessions do not survive a restart in that mode). - Password reset (DEC-213): migration 000029 adds `password_reset_tokens` (human_id FK CASCADE, `token_hash` unique-indexed, `expires_at`, `used_at` nullable, partial index on unresolved tokens per human). `Service.ForgotPassword`/`ResetPassword`: 5-minute single-use tokens (`PasswordResetTokenTTL`), same anti-enumeration response shape as `Login`, one collapsed `ErrPasswordResetTokenInvalid` for any unknown/expired/used/race-lost token. `database.ResetPassword` is one transaction: consume-token (RowsAffected-checked), update `password_hash`, revoke every refresh token the account has. Email delivery reuses `api.SubmissionAcceptor` (the v0.42 accept-a-message primitive) via a new `humanauth.Mailer` interface, configured by `MAILX_SYSTEM_TENANT_ID`/`MAILX_SYSTEM_FROM_ADDRESS`/`MAILX_DASHBOARD_BASE_URL`; unset means no mailer (token still created, logged not sent) rather than a startup failure. New routes `POST /v1/auth/forgot-password`/`reset-password`, public, behind their own `passwordResetIPLimitMiddleware` bucket (`Policy.PasswordResetIPRate`/`PasswordResetIPBurst`, default 1/60 rps burst 3 — much tighter than login's, since forgot-password sends a real email per call). -- Known limitation / explicitly deferred (not built): Paystack/billing, a `plan` field, email verification, OAuth/social login, MFA, and any RBAC beyond owner/member. The frontend's `{name, slug}` org-create contract is accepted; `slug` is currently a no-op input (tenants has no slug column yet). +- Organization invitations (DEC-216): migration 000030 adds `org_invitations` (tenant_id FK CASCADE, invited_by FK CASCADE, `normalized_email`/`raw_email`, `token_hash` unique-indexed, `expires_at`, `accepted_at` nullable, partial index on `(tenant_id, normalized_email) WHERE accepted_at IS NULL`) plus plain nullable URL columns `tenants.logo_url` and `humans.avatar_url` (no upload pipeline — operator decision). `Service.InviteToOrganization(ctx, inviterHumanID, tenantID, email)`: owner-only (`database.IsTenantOwner` re-checked server-side, `ErrNotOrgOwner` otherwise — no broader RBAC), 5-hour single-use tokens (`OrgInvitationTTL`), same `humanauth.Mailer`/`WithDashboardBaseURL` delivery wiring as password reset, degrades the same way when no mailer is configured. `Service.AcceptOrgInvitation(ctx, rawToken, existingHumanID, signupName, signupPassword)` is a single combined accept path (operator decision, not separate signup-then-join calls): with an existing session, the invitation's email must match that account's own (`ErrOrgInvitationEmailMismatch` otherwise) and `database.AcceptOrgInvitationForExistingHuman` atomically consumes the token and inserts `tenant_members` (`ON CONFLICT DO NOTHING`); with no session, `database.AcceptOrgInvitationWithSignup` atomically consumes the token, creates the human account (ALWAYS using the invitation's own stored email, never client-supplied), and inserts membership, all in one transaction. Both failure paths collapse to `ErrOrgInvitationInvalid`. New routes: `POST /v1/orgs/{id}/invites` (owner-only, behind `humanAuthMiddleware` then a new per-human `orgInviteLimitMiddleware` — `Policy.OrgInviteRate`/`OrgInviteBurst`, default 1/30 rps burst 10, keyed by the inviting human's ID rather than IP since the caller is already authenticated) and `POST /v1/orgs/invites/accept` (deliberately NOT behind `humanAuthMiddleware` — the invitee may have no account yet; reads an optional bearer token directly). +- Known limitation / explicitly deferred (not built): Paystack/billing, a `plan` field, email verification, OAuth/social login, MFA, and any RBAC beyond owner/member. The frontend's `{name, slug}` org-create contract is accepted; `slug` is currently a no-op input (tenants has no slug column yet). Avatar/logo are URL-only fields with no upload/hosting pipeline. diff --git a/.ilana/decisions.md b/.ilana/decisions.md index 997fd8c..0aa233a 100644 --- a/.ilana/decisions.md +++ b/.ilana/decisions.md @@ -220,3 +220,4 @@ Process decisions above (DEC-001..DEC-007) belong to the v0.23 FLEET run and sta - DEC-213 [v0.47 phase 1 follow-up]: added password reset for human accounts, per the operator's explicit spec (email -> account lookup -> single-use token -> emailed link -> new+confirm password -> token expires 5 min if unused -> dedicated rate limiting). Migration 000029 adds `password_reset_tokens` (id, human_id FK CASCADE, token_hash UNIQUE-indexed, expires_at, used_at nullable, created_at), with a partial index on `human_id WHERE used_at IS NULL`. Raw tokens are never stored, only `sha256`-style `hashRawToken` output (same pattern as refresh tokens); `PasswordResetTokenTTL = 5 * time.Minute`. `ForgotPassword(ctx, email)` follows the established anti-enumeration posture (DEC-208-era Login precedent): identical response whether or not the account exists, and the mailer is only invoked for a real account. Email delivery goes through MailX's OWN outbound pipeline (`api.SubmissionAcceptor`, the reusable v0.42 accept-a-message primitive) via a new `humanauth.Mailer` interface and `WithMailer`/`WithDashboardBaseURL` service options, rather than a parallel ad hoc send path or a third-party provider — decided with the operator, who explicitly chose dogfooding MailX's own pipeline. Configured via `MAILX_SYSTEM_TENANT_ID`/`MAILX_SYSTEM_FROM_ADDRESS`/`MAILX_DASHBOARD_BASE_URL`; if the first two are unset, the mailer is nil and `ForgotPassword` still creates the token (for a future non-email delivery path) but logs a warning instead of sending — graceful degradation matching this codebase's `MAILX_JWT_SECRET`-ephemeral-fallback convention, not a startup failure, since system email is optional for a self-hoster. `ResetPassword(ctx, rawToken, newPassword)` is one atomic transaction (`database.ResetPassword`): consume the token via a `RowsAffected`-checked `UPDATE ... WHERE used_at IS NULL` (so concurrent double-use of the same token only lets one caller win, mirroring DEC-212's refresh-token race fix), update `password_hash`, and revoke ALL of the account's refresh tokens — a password reset ends every existing session, not just the one that requested it. Both failure paths (unknown/expired/used/lost-race token) collapse to one sentinel, `ErrPasswordResetTokenInvalid`, deliberately never distinguishing which, same anti-enumeration posture as `ErrInvalidCredentials`. New dedicated rate-limit bucket (`Policy.PasswordResetIPRate`/`PasswordResetIPBurst`, default 1/60 rps burst 3, env `MAILX_LIMIT_PASSWORD_RESET_IP_RPS`/`_BURST`) shared by `POST /v1/auth/forgot-password` and `/v1/auth/reset-password` via a new `passwordResetIPLimitMiddleware`, deliberately much tighter than `AuthIPRate` (1 rps burst 10) since forgot-password triggers a real outbound send per call (email-bombing risk) and reset-password is the account's most dangerous credential-change action. Both endpoints documented in `internal/api/openapi.go` (`ForgotPasswordRequest`/`ResetPasswordRequest` schemas). Verified: `gofmt`/`go vet`/`go build` clean; new tests `TestForgotPasswordOnlyEmailsKnownAccounts`, `TestResetPasswordFlow` (old password rejected, new password works, token single-use, unknown token same generic error, short password rejected), `TestResetPasswordExpiredToken` (new `humanauth.WithNow` option added purely for TTL-expiry testing without sleeping) all pass; full `go test ./...` and `go test -race ./...` clean; migration 000029 validated via a manual down/up round-trip against the dev Postgres container (then rolled back before committing, since the container's `schema_migrations` bookkeeping must stay in sync with what the migrator itself will apply on next boot); Docker rebuild + boot smoke clean, live-curl-verified both endpoints (200 generic response for forgot-password regardless of account existence, 429 with `password_reset_rate_limited` once the burst is exhausted, 422 `invalid_reset_token` for a bogus reset token). - DEC-214 [v0.47 phase 1 fourth Greptile pass]: fixed 2 P1 findings posted minutes after DEC-213's password reset commit landed, both real. (1) `Refresh` revoked the old token and inserted its replacement as two independent statements; a `ResetPassword` landing in the gap between them would run its "revoke every active token" update before the replacement existed, so that one session survived a reset that was supposed to end all of them. Fixed with `database.RotateRefreshToken`, a single transaction combining both, closing the gap the same way DEC-212's `TouchLoginAndCreateRefreshToken` closed an analogous one for login. Proven by `TestResetPasswordDuringConcurrentRefreshEndsAllSessions`, which fires `Refresh` and `ResetPassword` concurrently against the same session and asserts any token the refresh managed to mint is unusable afterward. (2) `ForgotPassword` would build a relative `/reset-password?token=...` link and actually send it if `MAILX_SYSTEM_TENANT_ID`/`_FROM_ADDRESS` were set but `MAILX_DASHBOARD_BASE_URL` was left unset — a real, unusable email. `ForgotPassword` now refuses to send (same generic-error/logged-not-surfaced path as "no mailer configured") when the dashboard base URL is empty, requiring all three system-mail env vars together rather than degrading partially. Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, Docker rebuild+boot smoke clean. - DEC-215 [v0.47 phase 1 fifth Greptile pass + CI fix]: fixed 3 more findings from Greptile's fourth review pass (posted alongside DEC-214's two, initially missed since they targeted older code from the third pass rather than the just-shipped password reset) plus a CI-only failure the fixes exposed. (1) "Erased mail remains on disk" (P1): `materializeOne`'s GDPR-erasure disk-cleanup branch logged a failed file delete and then `return nil` (success) regardless — since the driving `broadcast_recipients` row is already gone, nothing else would ever retry it, and the caller's generic error/metrics path never even saw the failure. Now returns the wrapped delete error instead of nil, so the caller's existing "transient error" handling logs/counts it AND — critically — no longer calls `finishMaterialized` against a row that no longer exists. (2) "Stale last-login response" (P2): `Login` echoed back its own `loginAt` regardless of whether `TouchLoginAndCreateRefreshToken`'s monotonic guard actually applied it — if a concurrent later login won the race, the response would report an older `last_login_at` than what was actually stored. `TouchLoginAndCreateRefreshToken` now also returns the value ACTUALLY stored (a follow-up `SELECT` inside the same transaction, unconditionally, not only on a lost race, since a plain conditional re-fetch reads more branches than it saves), and `Login` uses that instead of its own local timestamp. (3) "Cleanup metric loses its phase" (P2): `observability.broadcastPhases`'s allowlist (`snapshot`/`materialize`/`complete`) didn't include `erasure_cleanup`, so `BroadcastExpansionBatch("erasure_cleanup", ...)` silently collapsed to the `other` label bucket — added `erasure_cleanup` to the allowlist and also added an `"ok"` observation on the cleanup-succeeded path (previously only the failure path recorded anything for this phase at all). (4) CI-only failure (Go CI workflow on PR #22, unrelated to the above 3): `TestTouchHumanLoginIsMonotonic`/`TestTouchLoginAndCreateRefreshTokenIsAtomicAndMonotonic`/`TestLoginRecordsLastLoginButSignUpDoesNot` failed intermittently in GitHub Actions (not locally, where Docker Postgres happened not to trigger it) because Go's `time.Now()` carries nanosecond precision but PostgreSQL's `timestamptz` only stores microseconds — a round-tripped value's `.Equal()` against the original in-memory value fails on the truncated nanosecond tail. Fixed by truncating to microsecond precision at the source: `humanauth.NewService`'s default clock (`time.Now().UTC().Truncate(time.Microsecond)`) and the affected test helpers in `humans_test.go`, rather than loosening the equality checks — the truncation reflects the actual precision PostgreSQL guarantees, so this closes the whole class of tests, not just the 3 that happened to fail this run. All verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, Docker rebuild+boot smoke clean. +- DEC-216 [v0.47 phase 1 follow-up]: added organization invitations. Four scoped decisions asked via AskUserQuestion before coding (operator confirmed all recommended defaults, no assumptions): avatar/logo as plain nullable URL columns only (no upload pipeline this milestone); a single combined accept endpoint handling both an already-logged-in invitee and a no-account-yet invitee (signup folded into acceptance), rather than separate signup-then-join calls; owner-only sending (matches today's single-role `tenant_members` model — no broader RBAC added); a dedicated tighter rate-limit bucket, same pattern as password reset. Migration 000030 adds `org_invitations` (id, tenant_id FK CASCADE, invited_by FK CASCADE, normalized_email/raw_email, token_hash UNIQUE-indexed, expires_at, accepted_at nullable, created_at; partial index on `(tenant_id, normalized_email) WHERE accepted_at IS NULL`) plus `tenants.logo_url` and `humans.avatar_url` (both nullable TEXT). `humanauth.InviteToOrganization(ctx, inviterHumanID, tenantID, email)` re-checks `database.IsTenantOwner` itself (never trusts a client-supplied "am I the owner" claim) — returns `ErrNotOrgOwner` otherwise — then creates a 5-hour single-use token (`OrgInvitationTTL`, same hashed-token shape as `password_reset_tokens`) and emails it through the SAME `humanauth.Mailer`/`WithDashboardBaseURL` wiring password reset already established (DEC-213) — no new mailer plumbing. The email body (`orgInvitationHTML`) shows the org logo and inviter avatar only when their URL fields are set (never a broken-image placeholder), the org name, and the accept link; degrades the same way as `ForgotPassword` when no mailer is configured (token still created, durable, logged-not-surfaced). `AcceptOrgInvitation(ctx, rawToken, existingHumanID, signupName, signupPassword)` implements the combined flow: with `existingHumanID` set, the invitation's email must case-insensitively match that account's own email (`ErrOrgInvitationEmailMismatch` otherwise) and `database.AcceptOrgInvitationForExistingHuman` atomically consumes the token (`RowsAffected`-checked, same single-use race guard as `ResetPassword`) and inserts the `tenant_members` row (`ON CONFLICT DO NOTHING`, so re-clicking an already-joined invite is not an error); with it empty, `database.AcceptOrgInvitationWithSignup` atomically consumes the token, creates the human account, and inserts membership in ONE transaction — critically, the account's email is ALWAYS the invitation's own stored address, never anything client-supplied, so a token issued for one address can never mint an account under another. Both failure paths (unknown/expired/already-accepted/lost-race token) collapse to one sentinel `ErrOrgInvitationInvalid`, same anti-enumeration-adjacent posture as password reset's collapsed error. `POST /v1/orgs/{id}/invites` sits behind `humanAuthMiddleware` then a new `orgInviteLimitMiddleware` (`Policy.OrgInviteRate`/`OrgInviteBurst`, default 1/30 rps burst 10, env `MAILX_LIMIT_ORG_INVITE_RPS`/`_BURST`) keyed by the inviting human's ID rather than IP — unlike the unauthenticated auth-surface buckets, this caller already has a verified identity, which is a stronger key than IP and avoids punishing every user behind a shared NAT/proxy for one owner's invite volume. `POST /v1/orgs/invites/accept` is deliberately NOT behind `humanAuthMiddleware` (an invitee may have no account yet); it reads an optional bearer token directly instead. Documented in `internal/api/openapi.go` (`InviteRequest`/`AcceptInviteRequest`/`AcceptInviteResponse` schemas, both new paths) — `TestOpenAPIRoutesMatchRuntime`/`TestOpenAPISpecParses` still pass. 5 new tests in `internal/humanauth/org_invitation_test.go` (owner-only enforcement, email-mismatch-on-existing-account, combined signup+join for an unknown invitee including using the invitation's own email over any client-supplied one, TTL expiry, and a signup-conflict-rolls-back-the-whole-transaction case proving a failed accept never silently consumes the token) all passing against real Postgres. Verified: `gofmt`/`go vet`/`go build` clean; full `go test ./...` and `go test -race ./...` clean against real Postgres+Redis; migration 000030 validated via a down/up/re-up round-trip against the dev Postgres container; Docker rebuild+boot smoke clean; live-curl-verified against the running server — owner-only 403 for a non-owner, invitation row durably created despite "no mailer configured" (same known dev-environment gap DEC-213 hit, not a new one), 422 `invalid_invitation_token` for a bogus accept token, and the new rate limiter observed live (9 consecutive charged calls then 429 at burst 10). diff --git a/.ilana/ledger.md b/.ilana/ledger.md index 4c3e513..d03395f 100644 --- a/.ilana/ledger.md +++ b/.ilana/ledger.md @@ -302,3 +302,6 @@ User flagged new Greptile comments as urgent right after the password-reset comm ## 2026-09-25 | v0.47 phase 1 fifth Greptile pass + CI fix | GATE PASS User listed 5 named findings plus a failing Go CI check and asked to verify/fix all. Cross-referenced against DEC-214 (already fixed 2 of the 5 - the refresh/reset race and the missing-dashboard-URL guard) and found the other 3 were genuinely still open: erased-mail-stays-on-disk (materializeOne returned nil/success even when the disk cleanup itself failed, so the caller's error path and finishMaterialized both ran as if nothing was wrong - fixed by propagating the cleanup failure), a stale last-login API response (Login echoed its own timestamp instead of what the monotonic guard actually stored, so a client could be told an older time than what's in the database during a concurrent-login race - fixed by having the DB call return the true stored value and having Login use that instead), and a broadcast metric losing its phase label (erasure_cleanup wasn't in the metrics package's phase allowlist, so it silently fell into "other" and became useless for alerting - added it to the allowlist). Separately investigated the failing GitHub Actions "Go CI" run directly via gh run view --log-failed rather than guessing: 3 tests failed only in CI, never locally, because Postgres timestamptz truncates to microseconds while Go's time.Now() carries nanoseconds, so a round-tripped .Equal() comparison fails on environments where the timing happens to hit that boundary. Fixed at the source (truncate the production clock and the test helpers to microsecond precision) rather than patching the 3 symptomatic tests, since the same class of bug could resurface anywhere else a timestamp round-trips through Postgres. Verified: gofmt/go vet/go build clean, full go test ./... and go test -race ./... clean, Docker rebuild+boot smoke clean. Ilana updated: decisions.md (DEC-215), state.json (DEC counter). + +## 2026-09-25 | v0.47 phase 1 follow-up: organization invitations | GATE PASS +Continued from a fresh-chat handoff per CLAUDE.md's Ilana-first rule (read ledger.md/state.json/architecture.md before touching code). Asked 4 scoped AskUserQuestion decisions before coding per the handoff's explicit instruction not to assume: avatar/logo as plain URL fields (no upload pipeline), a single combined accept-invite endpoint (not separate signup-then-join), owner-only sending, and a dedicated tighter rate-limit bucket - user accepted all 4 recommended defaults. Read the existing password-reset code (DEC-213) first as the pattern to mirror (hashed single-use tokens, own-pipeline mailer, collapsed anti-enumeration-style error) rather than designing from scratch. Built: migration 000030 (`org_invitations` table, `tenants.logo_url`, `humans.avatar_url`), `database.IsTenantOwner`/`CreateOrgInvitation`/`GetOrgInvitationByHash`/`AcceptOrgInvitationForExistingHuman`/`AcceptOrgInvitationWithSignup` (the last two atomic, RowsAffected-guarded single-use consumption matching `ResetPassword`'s race-safety pattern), `humanauth.InviteToOrganization`/`AcceptOrgInvitation` (owner-only server-side re-check, 5-hour TTL, combined existing-session-or-signup accept, invitation's own email always wins over any client-supplied one), new `POST /v1/orgs/{id}/invites` (owner-only, per-human rate-limited) and `POST /v1/orgs/invites/accept` (deliberately outside `humanAuthMiddleware` since the invitee may have no account) routes and handlers, a new `orgInviteLimitMiddleware` (keyed by human ID, not IP, since the caller here is already authenticated - a deliberate departure from the IP-keyed auth-surface buckets), and OpenAPI documentation for both endpoints. 5 new tests in `internal/humanauth/org_invitation_test.go` (owner-only enforcement, email-mismatch on an existing account, combined signup+join using the invitation's own email, TTL expiry, and a signup-conflict-rolls-back-the-whole-transaction case). Verified: gofmt/go vet/go build clean; full `go test ./...` and `go test -race ./...` clean against real Postgres+Redis (existing Docker stack); migration 000030 validated via a down/up/re-up round-trip against the dev Postgres container; Docker rebuild+boot smoke clean; live-curl-verified against the running server - owner-only 403 for a non-owner, an invitation row durably created despite "no mailer configured" (the same known dev-environment gap DEC-213 already hit, not a regression), 422 for a bogus accept token, and the new rate limiter observed live (9 charged calls then 429 exactly at burst 10). Ilana updated: milestones.md (v0.47 phase 1 section extended), architecture.md ("Human accounts & organizations" section extended with the invitations subsection), decisions.md (DEC-216), state.json (DEC counter, mailx status). diff --git a/.ilana/milestones.md b/.ilana/milestones.md index d7f5f5c..87f21a2 100644 --- a/.ilana/milestones.md +++ b/.ilana/milestones.md @@ -97,3 +97,5 @@ explicitly deferred to a later phase — v0.47 as a whole is NOT complete. - `internal/api` gained `humanauth_handler.go` and `humanAuthMiddleware`, wired via `api.Config.HumanAuth`. - `internal/api/openapi.go` documents `/auth/signup`, `/auth/login`, `/auth/refresh`, `/auth/logout`, `/orgs` (GET/POST) and a `HumanAuth` security scheme; `TestOpenAPIRoutesMatchRuntime`/`TestOpenAPISpecParses` still pass. - Tests: `internal/database/humans_test.go` (7 cases, real Postgres) and `internal/humanauth/service_test.go` (7 cases, real Postgres), covering duplicate-email rejection, case-insensitive login, refresh rotation, reuse-of-revoked-token session-wide revocation, logout, atomic org creation (including the failure-leaves-no-orphan case), membership-scoped listing, and JWT round-trip/tamper/expiry. +- Follow-up: password reset (migration 000029, DEC-213/214/215) — 5-minute single-use tokens, delivered through MailX's own outbound pipeline, dedicated rate limit, all-refresh-tokens-revoked-on-reset. +- Follow-up: organization invitations (migration 000030, DEC-216) — invite by email only (no invite-code flow), owner-only sending, 5-hour single-use tokens, a single combined accept endpoint (existing-session or no-account-yet signup, both atomic), org logo/inviter avatar as plain nullable URL fields, own-pipeline email delivery, dedicated per-human rate limit. 5 new tests in `internal/humanauth/org_invitation_test.go`. diff --git a/.ilana/state.json b/.ilana/state.json index f9de72b..d20f33d 100644 --- a/.ilana/state.json +++ b/.ilana/state.json @@ -18,7 +18,7 @@ "DEF": 27, "CR": 24, "RSK": 43, - "DEC": 215, + "DEC": 216, "MET": 72, "ETH": 1 }, @@ -37,7 +37,7 @@ "last_completed_milestone": "v0.46", "ilana_current_through": "v0.47 (partial)", "current_milestone": "v0.47 Human Accounts & Organizations", - "current_milestone_status": "v0.47 phase 1 complete (human auth + org membership) plus password reset (5-min single-use tokens, own-pipeline delivery, dedicated rate limit); billing/plans, OAuth, MFA still deferred", + "current_milestone_status": "v0.47 phase 1 complete (human auth + org membership) plus password reset and organization invitations (both 5-min/5-hour single-use tokens, own-pipeline delivery, dedicated rate limits); billing/plans, OAuth, MFA still deferred", "next_milestone": "v0.47 phase 2 (billing/plans, dashboard backend) or v0.48 (TBD)" } } diff --git a/cmd/mailx/abuseconfig.go b/cmd/mailx/abuseconfig.go index 23138ab..3ed81e0 100644 --- a/cmd/mailx/abuseconfig.go +++ b/cmd/mailx/abuseconfig.go @@ -93,6 +93,8 @@ func loadAbusePolicy(get func(string) string) (policy ratelimit.Policy, enabled n("MAILX_LIMIT_AUTH_IP_BURST", &policy.AuthIPBurst) f("MAILX_LIMIT_PASSWORD_RESET_IP_RPS", &policy.PasswordResetIPRate) n("MAILX_LIMIT_PASSWORD_RESET_IP_BURST", &policy.PasswordResetIPBurst) + f("MAILX_LIMIT_ORG_INVITE_RPS", &policy.OrgInviteRate) + n("MAILX_LIMIT_ORG_INVITE_BURST", &policy.OrgInviteBurst) n("MAILX_RETRY_JITTER_PERCENT", &policy.RetryJitterPercent) if raw := strings.TrimSpace(get("MAILX_LIMIT_PERMIT_TTL")); raw != "" { d, e := time.ParseDuration(raw) diff --git a/internal/api/abuse.go b/internal/api/abuse.go index 1a69201..b5dbab9 100644 --- a/internal/api/abuse.go +++ b/internal/api/abuse.go @@ -203,6 +203,47 @@ func passwordResetIPLimitMiddleware(a *AbuseControls) func(http.Handler) http.Ha } } +// orgInviteLimitMiddleware protects POST /v1/orgs/{id}/invites with its own +// tighter bucket, keyed by the inviting human's ID rather than IP — unlike +// the auth-surface middlewares above, this route is already authenticated +// (behind humanAuthMiddleware, which must run first) so a stable, unspoofable +// identity is available. See Policy.OrgInviteRate's doc for why a shared +// bucket with ordinary API traffic would be too permissive here (this +// triggers a real outbound email per call). +func orgInviteLimitMiddleware(a *AbuseControls) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + if a == nil || a.Limiter == nil { + return next + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + humanID, _ := humanIDFromContext(r.Context()) + p := a.Policy + dec, err := a.Limiter.Allow(r.Context(), + ratelimit.Bucket{Key: "org:invite:human:" + humanID, Rate: p.OrgInviteRate, Burst: p.OrgInviteBurst, Cost: 1}, + ) + switch { + case err != nil: + a.Metrics.AbuseDecision("org_invite", "unavailable") + a.log().Error("rate_limiter_unavailable", "route_class", "org_invite") + e := newError(ErrTemporarilyUnavailable, "rate_limiter_unavailable", "request limiting is temporarily unavailable; retry later") + e.RetryAfter = unavailableRetryAfter + writeError(w, r, e) + case dec.Impossible: + a.Metrics.AbuseDecision("org_invite", "impossible") + writeError(w, r, newError(ErrInternal, "internal_error", "request limit is misconfigured")) + case !dec.Allowed: + a.Metrics.AbuseDecision("org_invite", "limited") + e := newError(ErrRateLimited, "org_invite_rate_limited", "too many invitations sent; retry after the interval in Retry-After") + e.RetryAfter = ratelimit.RetryAfterSeconds(dec.RetryAfter) + writeError(w, r, e) + default: + a.Metrics.AbuseDecision("org_invite", "allowed") + next.ServeHTTP(w, r) + } + }) + } +} + // authClientIP returns the IP to key the auth rate limiter on: the // connection peer (RemoteAddr), UNLESS that peer is a configured trusted // proxy, in which case the rightmost entry of X-Forwarded-For is used diff --git a/internal/api/humanauth_handler.go b/internal/api/humanauth_handler.go index 60b8719..769eb7d 100644 --- a/internal/api/humanauth_handler.go +++ b/internal/api/humanauth_handler.go @@ -260,6 +260,98 @@ func (h *humanAuthHandler) handleListOrgs(w http.ResponseWriter, r *http.Request writeJSON(w, http.StatusOK, map[string]any{"data": out}) } +type inviteRequest struct { + Email string `json:"email"` +} + +// handleCreateInvite sends an org invitation. Owner-only: the org ID comes +// from the path, the inviter's identity from the JWT (never trust a +// client-supplied "am I the owner" claim). humanauth.Service.InviteToOrganization +// itself re-checks tenant_members before doing anything, so this handler's +// job is only request parsing and error-shape translation. +func (h *humanAuthHandler) handleCreateInvite(w http.ResponseWriter, r *http.Request) { + humanID, ok := humanIDFromContext(r.Context()) + if !ok { + writeError(w, r, newError(ErrAuthentication, "invalid_access_token", "missing or invalid access token")) + return + } + tenantID := r.PathValue("id") + if !acceptsJSONContentType(r.Header.Get("Content-Type")) { + writeError(w, r, newError(ErrUnsupportedMediaType, "unsupported_media_type", "Content-Type must be application/json")) + return + } + var req inviteRequest + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxBodyBytes)).Decode(&req); err != nil || req.Email == "" { + writeError(w, r, newError(ErrInvalidRequest, "invalid_json", "email is required")) + return + } + if err := h.svc.InviteToOrganization(r.Context(), humanID, tenantID, req.Email); err != nil { + if errors.Is(err, humanauth.ErrNotOrgOwner) { + writeError(w, r, newError(ErrForbidden, "not_org_owner", "only an organization owner can send invitations")) + return + } + slog.Default().Error("org_invite_failed", "error", err.Error()) + writeError(w, r, newError(ErrValidation, "invalid_invitation", err.Error())) + return + } + writeJSON(w, http.StatusOK, map[string]string{"message": "invitation sent"}) +} + +type acceptInviteRequest struct { + Token string `json:"token"` + Name string `json:"name"` // only used when the caller has no existing session + Password string `json:"password"` // only used when the caller has no existing session +} + +// handleAcceptInvite is the single combined accept endpoint (operator +// decision): a caller with a valid human access token accepts under their +// existing account (email must match the invite); a caller with no +// Authorization header must supply name+password and is signed up and +// joined atomically in one call, keyed by the invite token. This route is +// intentionally NOT behind humanAuthMiddleware — that middleware would +// reject every unauthenticated (no-account-yet) request before this +// handler ever saw it — so the bearer token here is read directly and +// treated as optional. +func (h *humanAuthHandler) handleAcceptInvite(w http.ResponseWriter, r *http.Request) { + if !acceptsJSONContentType(r.Header.Get("Content-Type")) { + writeError(w, r, newError(ErrUnsupportedMediaType, "unsupported_media_type", "Content-Type must be application/json")) + return + } + var req acceptInviteRequest + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxBodyBytes)).Decode(&req); err != nil || req.Token == "" { + writeError(w, r, newError(ErrInvalidRequest, "invalid_json", "token is required")) + return + } + var existingHumanID string + if token, ok := extractBearerHumanToken(r); ok { + claims, err := h.svc.VerifyAccessToken(token) + if err != nil { + writeError(w, r, newError(ErrAuthentication, "invalid_access_token", "access token is invalid or expired")) + return + } + existingHumanID = claims.HumanID + } + result, err := h.svc.AcceptOrgInvitation(r.Context(), req.Token, existingHumanID, req.Name, req.Password) + if err != nil { + switch { + case errors.Is(err, humanauth.ErrOrgInvitationInvalid): + writeError(w, r, newError(ErrValidation, "invalid_invitation_token", "this invitation is invalid or has expired")) + case errors.Is(err, humanauth.ErrOrgInvitationEmailMismatch): + writeError(w, r, newError(ErrForbidden, "invitation_email_mismatch", "this invitation was sent to a different email address")) + case errors.Is(err, humanauth.ErrEmailTaken): + writeError(w, r, newError(ErrConflictType, "email_taken", "an account with this email already exists; log in and try again")) + default: + writeError(w, r, newError(ErrValidation, "invalid_invitation", err.Error())) + } + return + } + resp := map[string]any{"organization": orgResourceFrom(result.Tenant)} + if result.Session != nil { + resp["session"] = sessionResponseFrom(*result.Session) + } + writeJSON(w, http.StatusOK, resp) +} + // extractBearerHumanToken parses "Bearer " the same way // extractBearerToken does for API keys, kept separate so the two // credential formats never share validation code paths. diff --git a/internal/api/openapi.go b/internal/api/openapi.go index d76ae51..e1359cd 100644 --- a/internal/api/openapi.go +++ b/internal/api/openapi.go @@ -138,6 +138,35 @@ const openAPISpec = `{ } } }, + "/orgs/{id}/invites": { + "post": { + "summary": "Invite someone to join the organization by email", + "description": "Requires a human access token (HumanAuth) belonging to an OWNER of this organization (403 not_org_owner otherwise). No separate invite-code flow: only the invitee's email is needed. Sends an email (via MailX's own outbound pipeline) showing the org name, org logo and inviter avatar when set, and an accept link. The token is single-use and expires 5 hours after issuance. Rate-limited per inviting human, separately from and tighter than ordinary API traffic, since it triggers a real outbound email send.", + "security": [{"HumanAuth": []}], + "parameters": [ + {"name": "id", "in": "path", "required": true, "schema": {"type": "string"}, "description": "Organization (tenant) ID."} + ], + "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/InviteRequest"}}}}, + "responses": { + "200": {"description": "OK", "content": {"application/json": {"schema": {"type": "object", "properties": {"message": {"type": "string"}}}}}}, + "403": {"description": "Caller is not an owner of this organization", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, + "/orgs/invites/accept": { + "post": { + "summary": "Accept an organization invitation", + "description": "Accepts a pending invitation by token. If the request carries a valid human Authorization bearer token, the invite is accepted under that existing account (its email must match the invitation's, or 403 invitation_email_mismatch). Otherwise name and password are required and a new account is created (using the invitation's own email, never a client-supplied one) and joined to the organization in one atomic call. The token is single-use and expires 5 hours after issuance.", + "security": [], + "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/AcceptInviteRequest"}}}}, + "responses": { + "200": {"description": "OK. \"session\" is present only when this call created a new account.", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/AcceptInviteResponse"}}}}, + "403": {"description": "Invitation is for a different email than the authenticated account", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "409": {"description": "An account with the invitation's email already exists (log in and retry)", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "422": {"description": "Invalid request, or the invitation token is invalid/expired/accepted", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, "/emails": { "post": { "summary": "Send an email", @@ -2870,6 +2899,29 @@ const openAPISpec = `{ "data": {"type": "array", "items": {"$ref": "#/components/schemas/Organization"}} } }, + "InviteRequest": { + "type": "object", + "required": ["email"], + "properties": { + "email": {"type": "string", "format": "email"} + } + }, + "AcceptInviteRequest": { + "type": "object", + "required": ["token"], + "properties": { + "token": {"type": "string"}, + "name": {"type": "string", "description": "Required only when accepting with no existing session (creates the account)."}, + "password": {"type": "string", "format": "password", "minLength": 8, "description": "Required only when accepting with no existing session (creates the account)."} + } + }, + "AcceptInviteResponse": { + "type": "object", + "properties": { + "organization": {"$ref": "#/components/schemas/Organization"}, + "session": {"allOf": [{"$ref": "#/components/schemas/Session"}], "description": "Present only when accepting created a new account."} + } + }, "SendEmailRequest": { "type": "object", "required": [ diff --git a/internal/api/routes.go b/internal/api/routes.go index 1d46e74..e47af7c 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -184,6 +184,14 @@ func newMux(h *emailHandler, authSvc authService, readiness func() error, extras orgsAuthenticated := humanAuthMiddleware(extras[0].humanAuth) mux.Handle("POST /v1/orgs", orgsAuthenticated(http.HandlerFunc(ha.handleCreateOrg))) mux.Handle("GET /v1/orgs", orgsAuthenticated(http.HandlerFunc(ha.handleListOrgs))) + // Owner-only, JWT-authenticated, then its own tighter per-human + // bucket (see orgInviteLimitMiddleware's doc) — chained in that + // order so the limiter always has a real human ID to key on. + mux.Handle("POST /v1/orgs/{id}/invites", orgsAuthenticated(chain(http.HandlerFunc(ha.handleCreateInvite), orgInviteLimitMiddleware(abuse)))) + // Deliberately NOT behind orgsAuthenticated: the invitee may have no + // account yet, so a bearer token here is optional — see + // handleAcceptInvite's doc. + mux.HandleFunc("POST /v1/orgs/invites/accept", ha.handleAcceptInvite) } if len(extras) > 0 && extras[0].feedback != nil { // Deliberately NOT under /v1 and NOT authenticateMiddleware: this is the diff --git a/internal/database/humans.go b/internal/database/humans.go index 9847bd7..efd1891 100644 --- a/internal/database/humans.go +++ b/internal/database/humans.go @@ -19,6 +19,9 @@ type Human struct { CreatedAt time.Time UpdatedAt time.Time LastLoginAt *time.Time + // AvatarURL is nil until the human sets one; a plain URL, no upload + // pipeline (see migration 000030's doc). Used by org-invite emails. + AvatarURL *string } // RefreshToken is one issued refresh token row (see migration 000025). @@ -68,10 +71,10 @@ func (db *DB) CreateHuman(ctx context.Context, name, email, passwordHash string) func (db *DB) GetHumanByEmail(ctx context.Context, email string) (Human, error) { var h Human err := db.pool.QueryRow(ctx, ` - SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at + SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at, avatar_url FROM humans WHERE normalized_email = $1`, normalizeEmail(email), - ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt) + ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt, &h.AvatarURL) if err != nil { return Human{}, normalizeErr(err) } @@ -82,9 +85,9 @@ func (db *DB) GetHumanByEmail(ctx context.Context, email string) (Human, error) func (db *DB) GetHuman(ctx context.Context, id string) (Human, error) { var h Human err := db.pool.QueryRow(ctx, ` - SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at + SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at, avatar_url FROM humans WHERE id = $1`, id, - ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt) + ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt, &h.AvatarURL) if err != nil { return Human{}, normalizeErr(err) } diff --git a/internal/database/migrations/000030_org_invitations.down.sql b/internal/database/migrations/000030_org_invitations.down.sql new file mode 100644 index 0000000..de65952 --- /dev/null +++ b/internal/database/migrations/000030_org_invitations.down.sql @@ -0,0 +1,3 @@ +DROP TABLE org_invitations; +ALTER TABLE humans DROP COLUMN avatar_url; +ALTER TABLE tenants DROP COLUMN logo_url; diff --git a/internal/database/migrations/000030_org_invitations.up.sql b/internal/database/migrations/000030_org_invitations.up.sql new file mode 100644 index 0000000..f2625c9 --- /dev/null +++ b/internal/database/migrations/000030_org_invitations.up.sql @@ -0,0 +1,30 @@ +-- v0.47 (Phase 1) follow-up: organization invitations, plus the URL-only +-- avatar/logo fields the invite email needs. Per the operator's decisions: +-- invite by email only (no separate invite-code UX), owner-only sending, +-- 5-hour single-use tokens (same hashed-token shape as password_reset_tokens), +-- and images are plain nullable URL columns (no upload pipeline yet). + +ALTER TABLE tenants ADD COLUMN logo_url TEXT; +ALTER TABLE humans ADD COLUMN avatar_url TEXT; + +CREATE TABLE org_invitations ( + id TEXT PRIMARY KEY, + tenant_id TEXT NOT NULL REFERENCES tenants(id) ON DELETE CASCADE, + invited_by TEXT NOT NULL REFERENCES humans(id) ON DELETE CASCADE, + -- Case-insensitive matching follows humans.normalized_email's + -- convention; raw_email is kept for display in operator/API responses. + normalized_email TEXT NOT NULL CHECK (length(normalized_email) BETWEEN 3 AND 254 AND normalized_email !~ '\s'), + raw_email TEXT NOT NULL, + token_hash TEXT NOT NULL, + -- 5-hour TTL is enforced by the application (humanauth.OrgInvitationTTL), + -- same convention as password_reset_tokens.expires_at. + expires_at TIMESTAMPTZ NOT NULL, + accepted_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- Hot path: "verify by hash" on accept. +CREATE UNIQUE INDEX idx_org_invitations_hash ON org_invitations (token_hash); +-- "does this tenant already have a pending invite for this email" - +-- queried before sending a new one to avoid duplicate outstanding invites. +CREATE INDEX idx_org_invitations_tenant_email ON org_invitations (tenant_id, normalized_email) WHERE accepted_at IS NULL; diff --git a/internal/database/org_invitations.go b/internal/database/org_invitations.go new file mode 100644 index 0000000..c00c7fe --- /dev/null +++ b/internal/database/org_invitations.go @@ -0,0 +1,169 @@ +package database + +import ( + "context" + "errors" + "fmt" + "time" +) + +// OrgInvitation is one issued organization-invitation row (migration +// 000030). Deliberately its own table, not a repurposed password_reset_tokens +// row — an invite authorizes joining a specific tenant, not authenticating +// an existing account, and carries fields (tenant_id, invited_by, email) +// that have no analog there. +type OrgInvitation struct { + ID string + TenantID string + InvitedBy string + NormalizedEmail string + RawEmail string + TokenHash string + ExpiresAt time.Time + AcceptedAt *time.Time + CreatedAt time.Time +} + +// IsTenantOwner reports whether humanID is an 'owner' member of tenantID — +// the only role permitted to send an org invitation (operator decision: +// owner-only, no broader RBAC yet — see DEC-205's deferral of a full role +// matrix). +func (db *DB) IsTenantOwner(ctx context.Context, tenantID, humanID string) (bool, error) { + var role string + err := db.pool.QueryRow(ctx, + `SELECT role FROM tenant_members WHERE tenant_id = $1 AND human_id = $2`, tenantID, humanID, + ).Scan(&role) + if err != nil { + if errors.Is(normalizeErr(err), ErrNotFound) { + return false, nil + } + return false, normalizeErr(err) + } + return role == "owner", nil +} + +// CreateOrgInvitation inserts a new invitation row. +func (db *DB) CreateOrgInvitation(ctx context.Context, tenantID, invitedBy, email, tokenHash string, expiresAt time.Time) (OrgInvitation, error) { + id, err := newID() + if err != nil { + return OrgInvitation{}, err + } + var inv OrgInvitation + err = db.pool.QueryRow(ctx, ` + INSERT INTO org_invitations (id, tenant_id, invited_by, normalized_email, raw_email, token_hash, expires_at) + VALUES ($1, $2, $3, $4, $5, $6, $7) + RETURNING id, tenant_id, invited_by, normalized_email, raw_email, token_hash, expires_at, accepted_at, created_at`, + id, tenantID, invitedBy, normalizeEmail(email), email, tokenHash, expiresAt, + ).Scan(&inv.ID, &inv.TenantID, &inv.InvitedBy, &inv.NormalizedEmail, &inv.RawEmail, &inv.TokenHash, &inv.ExpiresAt, &inv.AcceptedAt, &inv.CreatedAt) + if err != nil { + return OrgInvitation{}, normalizeErr(err) + } + return inv, nil +} + +// GetOrgInvitationByHash loads an invitation row by its hash. Returns +// ErrNotFound if absent (regardless of accepted/expired — callers check +// those fields themselves, same convention as GetPasswordResetTokenByHash). +func (db *DB) GetOrgInvitationByHash(ctx context.Context, tokenHash string) (OrgInvitation, error) { + var inv OrgInvitation + err := db.pool.QueryRow(ctx, ` + SELECT id, tenant_id, invited_by, normalized_email, raw_email, token_hash, expires_at, accepted_at, created_at + FROM org_invitations WHERE token_hash = $1`, tokenHash, + ).Scan(&inv.ID, &inv.TenantID, &inv.InvitedBy, &inv.NormalizedEmail, &inv.RawEmail, &inv.TokenHash, &inv.ExpiresAt, &inv.AcceptedAt, &inv.CreatedAt) + if err != nil { + return OrgInvitation{}, normalizeErr(err) + } + return inv, nil +} + +// ErrOrgInvitationConsumed means this exact invitation row was already +// marked accepted by a concurrent call — same race-safety pattern as +// ErrPasswordResetTokenConsumed. +var ErrOrgInvitationConsumed = errors.New("database: org invitation already accepted") + +// AcceptOrgInvitationForExistingHuman atomically: (1) marks the invitation +// accepted, but ONLY if not already accepted (RowsAffected-checked, same +// pattern as ResetPassword), and (2) inserts the tenant_members row — +// ON CONFLICT DO NOTHING, since the invitee may already belong to the +// tenant (e.g. re-clicking an old link after already joining some other +// way must not error). Used when the invite is accepted by an already +// logged-in human whose account email matches the invitation. +func (db *DB) AcceptOrgInvitationForExistingHuman(ctx context.Context, invitationID, tenantID, humanID string, now time.Time) error { + tx, err := db.pool.Begin(ctx) + if err != nil { + return fmt.Errorf("database: begin accept invitation: %w", normalizeErr(err)) + } + defer func() { _ = tx.Rollback(ctx) }() + + tag, err := tx.Exec(ctx, `UPDATE org_invitations SET accepted_at = $2 WHERE id = $1 AND accepted_at IS NULL`, invitationID, now) + if err != nil { + return fmt.Errorf("database: consume org invitation: %w", normalizeErr(err)) + } + if tag.RowsAffected() == 0 { + return ErrOrgInvitationConsumed + } + + if _, err := tx.Exec(ctx, + `INSERT INTO tenant_members (tenant_id, human_id, role) VALUES ($1, $2, 'member') ON CONFLICT DO NOTHING`, + tenantID, humanID, + ); err != nil { + return fmt.Errorf("database: insert tenant member: %w", normalizeErr(err)) + } + + if err := tx.Commit(ctx); err != nil { + return fmt.Errorf("database: commit accept invitation: %w", normalizeErr(err)) + } + return nil +} + +// AcceptOrgInvitationWithSignup atomically: (1) marks the invitation +// accepted (same RowsAffected guard as AcceptOrgInvitationForExistingHuman), +// (2) creates the new human account, and (3) inserts the tenant_members +// row — all three or none, so a partial application (account created but +// never joined, or vice versa) is never observable. Used when the invitee +// has no MailX account yet: the invite link itself carries them through +// signup, so email is always the invitation's own (never client-supplied, +// to prevent an invite token for one address minting an account under +// another). +func (db *DB) AcceptOrgInvitationWithSignup(ctx context.Context, invitationID, tenantID, name, email, passwordHash string, now time.Time) (Human, error) { + tx, err := db.pool.Begin(ctx) + if err != nil { + return Human{}, fmt.Errorf("database: begin accept invitation with signup: %w", normalizeErr(err)) + } + defer func() { _ = tx.Rollback(ctx) }() + + tag, err := tx.Exec(ctx, `UPDATE org_invitations SET accepted_at = $2 WHERE id = $1 AND accepted_at IS NULL`, invitationID, now) + if err != nil { + return Human{}, fmt.Errorf("database: consume org invitation: %w", normalizeErr(err)) + } + if tag.RowsAffected() == 0 { + return Human{}, ErrOrgInvitationConsumed + } + + humanID, err := newID() + if err != nil { + return Human{}, err + } + var h Human + err = tx.QueryRow(ctx, ` + INSERT INTO humans (id, name, normalized_email, email, password_hash) + VALUES ($1, $2, $3, $4, $5) + RETURNING id, name, email, password_hash, role, created_at, updated_at`, + humanID, name, normalizeEmail(email), email, passwordHash, + ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt) + if err != nil { + return Human{}, normalizeErr(err) + } + + if _, err := tx.Exec(ctx, + `INSERT INTO tenant_members (tenant_id, human_id, role) VALUES ($1, $2, 'member')`, + tenantID, h.ID, + ); err != nil { + return Human{}, normalizeErr(err) + } + + if err := tx.Commit(ctx); err != nil { + return Human{}, fmt.Errorf("database: commit accept invitation with signup: %w", normalizeErr(err)) + } + return h, nil +} diff --git a/internal/database/tenants.go b/internal/database/tenants.go index 145d6fc..db7db6b 100644 --- a/internal/database/tenants.go +++ b/internal/database/tenants.go @@ -18,6 +18,9 @@ type Tenant struct { // window - see internal/database/retention.go's DefaultRetentionDays // for what applies in that case. RetentionDays *int + // LogoURL is nil until the org sets one; a plain URL, no upload + // pipeline (see migration 000030's doc). Used by org-invite emails. + LogoURL *string } // CreateTenant inserts a new tenant and returns its generated ID. @@ -44,8 +47,8 @@ func (db *DB) CreateTenant(ctx context.Context, name string) (Tenant, error) { func (db *DB) GetTenant(ctx context.Context, id string) (Tenant, error) { var t Tenant err := db.pool.QueryRow(ctx, - `SELECT id, name, created_at, retention_days FROM tenants WHERE id = $1`, id, - ).Scan(&t.ID, &t.Name, &t.CreatedAt, &t.RetentionDays) + `SELECT id, name, created_at, retention_days, logo_url FROM tenants WHERE id = $1`, id, + ).Scan(&t.ID, &t.Name, &t.CreatedAt, &t.RetentionDays, &t.LogoURL) if err != nil { return Tenant{}, normalizeErr(err) } diff --git a/internal/humanauth/org_invitation_test.go b/internal/humanauth/org_invitation_test.go new file mode 100644 index 0000000..62d9b43 --- /dev/null +++ b/internal/humanauth/org_invitation_test.go @@ -0,0 +1,247 @@ +package humanauth + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/Ferousco-dev/mailx/internal/database" +) + +func extractInviteToken(t *testing.T, text string) string { + t.Helper() + idx := strings.Index(text, "token=") + if idx == -1 { + t.Fatalf("no token= in invite email body: %q", text) + } + raw := text[idx+len("token="):] + if end := strings.IndexAny(raw, "\n "); end != -1 { + raw = raw[:end] + } + return raw +} + +func TestInviteOwnerOnly(t *testing.T) { + db := newTestDB(t) + mailer := &fakeMailer{} + svc, err := NewService(db, testSecret(), WithMailer(mailer), WithDashboardBaseURL("https://app.mailx.dev")) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + owner, err := svc.SignUp(ctx, "Ada", "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + tenant, err := svc.CreateOrganization(ctx, owner.Human.ID, "Acme Inc", "") + if err != nil { + t.Fatal(err) + } + other, err := svc.SignUp(ctx, "Grace", "grace@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + + if err := svc.InviteToOrganization(ctx, other.Human.ID, tenant.ID, "invitee@example.com"); !errors.Is(err, ErrNotOrgOwner) { + t.Fatalf("expected ErrNotOrgOwner for a non-member, got %v", err) + } + + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "invitee@example.com"); err != nil { + t.Fatalf("expected owner invite to succeed, got %v", err) + } + if len(mailer.calls) != 1 || mailer.calls[0].to != "invitee@example.com" { + t.Fatalf("expected exactly one invite email to invitee@example.com, got %+v", mailer.calls) + } + if !strings.Contains(mailer.calls[0].subject, "Acme Inc") { + t.Fatalf("expected org name in subject, got %q", mailer.calls[0].subject) + } +} + +func TestAcceptInviteExistingAccountMustMatchEmail(t *testing.T) { + db := newTestDB(t) + mailer := &fakeMailer{} + svc, err := NewService(db, testSecret(), WithMailer(mailer), WithDashboardBaseURL("https://app.mailx.dev")) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + owner, err := svc.SignUp(ctx, "Ada", "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + tenant, err := svc.CreateOrganization(ctx, owner.Human.ID, "Acme Inc", "") + if err != nil { + t.Fatal(err) + } + invitee, err := svc.SignUp(ctx, "Grace", "grace@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "invitee@example.com"); err != nil { + t.Fatal(err) + } + raw := extractInviteToken(t, mailer.calls[0].text) + + // Wrong account (Grace's email doesn't match the invitation's). + if _, err := svc.AcceptOrgInvitation(ctx, raw, invitee.Human.ID, "", ""); !errors.Is(err, ErrOrgInvitationEmailMismatch) { + t.Fatalf("expected ErrOrgInvitationEmailMismatch, got %v", err) + } + + // Right account: sign up as the actual invitee, then accept. + realInvitee, err := svc.SignUp(ctx, "Invitee", "invitee@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + result, err := svc.AcceptOrgInvitation(ctx, raw, realInvitee.Human.ID, "", "") + if err != nil { + t.Fatalf("expected accept to succeed for the matching account, got %v", err) + } + if result.Session != nil { + t.Fatal("expected no new session for an already-logged-in caller") + } + if result.Tenant.ID != tenant.ID { + t.Fatalf("unexpected tenant: %+v", result.Tenant) + } + orgs, err := svc.ListOrganizationsForHuman(ctx, realInvitee.Human.ID) + if err != nil { + t.Fatal(err) + } + if len(orgs) != 1 || orgs[0].ID != tenant.ID { + t.Fatalf("expected invitee to have joined the org, got %+v", orgs) + } + + // Re-accepting the same (now-consumed) token must fail, never double-join. + if _, err := svc.AcceptOrgInvitation(ctx, raw, realInvitee.Human.ID, "", ""); !errors.Is(err, ErrOrgInvitationInvalid) { + t.Fatalf("expected ErrOrgInvitationInvalid on reuse, got %v", err) + } +} + +func TestAcceptInviteSignsUpUnknownInvitee(t *testing.T) { + db := newTestDB(t) + mailer := &fakeMailer{} + svc, err := NewService(db, testSecret(), WithMailer(mailer), WithDashboardBaseURL("https://app.mailx.dev")) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + owner, err := svc.SignUp(ctx, "Ada", "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + tenant, err := svc.CreateOrganization(ctx, owner.Human.ID, "Acme Inc", "") + if err != nil { + t.Fatal(err) + } + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "newbie@example.com"); err != nil { + t.Fatal(err) + } + raw := extractInviteToken(t, mailer.calls[0].text) + + // No existing session (existingHumanID == ""): must sign up, using the + // INVITATION's email, not anything client-supplied. + result, err := svc.AcceptOrgInvitation(ctx, raw, "", "Newbie", "hunter22hunter") + if err != nil { + t.Fatalf("expected combined signup+accept to succeed, got %v", err) + } + if result.Session == nil { + t.Fatal("expected a fresh session for a newly created account") + } + if result.Session.Human.Email != "newbie@example.com" { + t.Fatalf("expected account email to be the invitation's own address, got %q", result.Session.Human.Email) + } + orgs, err := svc.ListOrganizationsForHuman(ctx, result.Session.Human.ID) + if err != nil { + t.Fatal(err) + } + if len(orgs) != 1 || orgs[0].ID != tenant.ID { + t.Fatalf("expected the new account to have joined the org, got %+v", orgs) + } + + // Short password must be rejected before any account is created. + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "second@example.com"); err != nil { + t.Fatal(err) + } + raw2 := extractInviteToken(t, mailer.calls[1].text) + if _, err := svc.AcceptOrgInvitation(ctx, raw2, "", "Someone", "short"); err == nil { + t.Fatal("expected a short password to be rejected") + } +} + +func TestAcceptInviteExpiredToken(t *testing.T) { + db := newTestDB(t) + mailer := &fakeMailer{} + start := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) + current := start + svc, err := NewService(db, testSecret(), WithMailer(mailer), WithDashboardBaseURL("https://app.mailx.dev"), + WithNow(func() time.Time { return current })) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + owner, err := svc.SignUp(ctx, "Ada", "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + tenant, err := svc.CreateOrganization(ctx, owner.Human.ID, "Acme Inc", "") + if err != nil { + t.Fatal(err) + } + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "invitee@example.com"); err != nil { + t.Fatal(err) + } + raw := extractInviteToken(t, mailer.calls[0].text) + + current = start.Add(OrgInvitationTTL + time.Second) + if _, err := svc.AcceptOrgInvitation(ctx, raw, "", "Invitee", "hunter22hunter"); !errors.Is(err, ErrOrgInvitationInvalid) { + t.Fatalf("expected ErrOrgInvitationInvalid for an expired invitation, got %v", err) + } +} + +func TestAcceptInviteEmailAlreadyTaken(t *testing.T) { + db := newTestDB(t) + mailer := &fakeMailer{} + svc, err := NewService(db, testSecret(), WithMailer(mailer), WithDashboardBaseURL("https://app.mailx.dev")) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + owner, err := svc.SignUp(ctx, "Ada", "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + tenant, err := svc.CreateOrganization(ctx, owner.Human.ID, "Acme Inc", "") + if err != nil { + t.Fatal(err) + } + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "taken@example.com"); err != nil { + t.Fatal(err) + } + raw := extractInviteToken(t, mailer.calls[0].text) + + // The invitation's own email gets registered by some other means + // before the invite is accepted (e.g. a separate signup). + if _, err := svc.SignUp(ctx, "Someone Else", "taken@example.com", "hunter22hunter"); err != nil { + t.Fatal(err) + } + + if _, err := svc.AcceptOrgInvitation(ctx, raw, "", "Taken", "hunter22hunter"); !errors.Is(err, ErrEmailTaken) { + t.Fatalf("expected ErrEmailTaken, got %v", err) + } + + // The invitation must NOT be consumed by the failed attempt above — + // database.AcceptOrgInvitationWithSignup runs the email-uniqueness + // conflict and the accepted_at update in the same transaction, so a + // conflict rolls back the whole thing, leaving the token still usable + // (e.g. by the account holder logging in and accepting normally). + var invited database.OrgInvitation + invited, err = db.GetOrgInvitationByHash(ctx, hashRawToken(raw)) + if err != nil { + t.Fatal(err) + } + if invited.AcceptedAt != nil { + t.Fatal("expected the invitation to remain unconsumed after a rolled-back signup conflict") + } +} diff --git a/internal/humanauth/service.go b/internal/humanauth/service.go index 54f6ab7..c4c785a 100644 --- a/internal/humanauth/service.go +++ b/internal/humanauth/service.go @@ -31,6 +31,21 @@ var ErrRefreshTokenInvalid = errors.New("humanauth: refresh token invalid or exp // anti-enumeration posture as ErrInvalidCredentials. var ErrPasswordResetTokenInvalid = errors.New("humanauth: password reset token invalid or expired") +// ErrOrgInvitationInvalid covers every way an org invitation token can fail +// to accept - unknown, expired, already accepted, or lost a concurrent +// consume-race - deliberately never distinguished to the caller, same +// posture as ErrPasswordResetTokenInvalid. +var ErrOrgInvitationInvalid = errors.New("humanauth: org invitation invalid or expired") + +// ErrOrgInvitationEmailMismatch is returned when an already-logged-in +// human tries to accept an invitation addressed to a different email. +var ErrOrgInvitationEmailMismatch = errors.New("humanauth: this invitation was sent to a different email address") + +// ErrNotOrgOwner is returned when a non-owner tries to send an org +// invitation — sending is owner-only (operator decision; see DEC-205's +// deferral of a full role matrix). +var ErrNotOrgOwner = errors.New("humanauth: only an organization owner can send invitations") + const ( // AccessTokenTTL is short-lived per the JWT model agreed with the // operator: ~15 minutes. @@ -40,6 +55,12 @@ const ( // PasswordResetTokenTTL is deliberately short - the operator's own // stated requirement: unused within 5 minutes, it must not work. PasswordResetTokenTTL = 5 * time.Minute + // OrgInvitationTTL is the operator's own stated requirement: unused + // within 5 hours, an invitation must not work (much longer than a + // password reset link since it's a lower-risk action shared over + // whatever channel the inviter chooses, not a same-session self-serve + // flow). + OrgInvitationTTL = 5 * time.Hour ) // Mailer sends a system-originated email to a human account holder @@ -413,3 +434,164 @@ func (s *Service) CreateOrganization(ctx context.Context, humanID, name, slug st func (s *Service) ListOrganizationsForHuman(ctx context.Context, humanID string) ([]database.Tenant, error) { return s.db.ListOrganizationsForHuman(ctx, humanID) } + +// InviteToOrganization issues a 5-hour org invitation and emails it. +// Sending is owner-only (returns ErrNotOrgOwner otherwise) — inviting +// someone needs only their email, no separate invite-code flow; the email +// itself carries the accept link. Mirrors ForgotPassword's shape but, +// unlike it, is NOT anti-enumeration: the caller is already an +// authenticated org owner deliberately inviting a specific address, so +// there is nothing to hide from them. +func (s *Service) InviteToOrganization(ctx context.Context, inviterHumanID, tenantID, email string) error { + email = strings.TrimSpace(email) + if email == "" { + return fmt.Errorf("humanauth: invitee email is required") + } + isOwner, err := s.db.IsTenantOwner(ctx, tenantID, inviterHumanID) + if err != nil { + return fmt.Errorf("humanauth: check org owner: %w", err) + } + if !isOwner { + return ErrNotOrgOwner + } + tenant, err := s.db.GetTenant(ctx, tenantID) + if err != nil { + return fmt.Errorf("humanauth: get tenant: %w", err) + } + inviter, err := s.db.GetHuman(ctx, inviterHumanID) + if err != nil { + return fmt.Errorf("humanauth: get inviter: %w", err) + } + raw, err := generateRawToken() + if err != nil { + return err + } + if _, err := s.db.CreateOrgInvitation(ctx, tenantID, inviterHumanID, email, hashRawToken(raw), s.now().Add(OrgInvitationTTL)); err != nil { + return fmt.Errorf("humanauth: create org invitation: %w", err) + } + if s.mailer == nil { + return fmt.Errorf("humanauth: org invitation created but no mailer is configured") + } + link := s.dashboardBaseURL + "/accept-invite?token=" + raw + subject := fmt.Sprintf("%s invites you to join the organization", tenant.Name) + text := fmt.Sprintf("%s (%s) has invited you to join %s on MailX.\n\n"+ + "Accept the invitation within 5 hours:\n%s\n\n"+ + "If you weren't expecting this, you can safely ignore this email.", + inviter.Name, inviter.Email, tenant.Name, link) + html := orgInvitationHTML(tenant, inviter, link) + if err := s.mailer.SendSystemEmail(ctx, email, subject, text, html); err != nil { + return fmt.Errorf("humanauth: send org invitation email: %w", err) + } + return nil +} + +// orgInvitationHTML renders the invite email body: org logo + inviter +// avatar when set (plain tags against the URL-only fields — see +// migration 000030's doc; no image processing/hosting here), org name, +// and the accept link. Either image is omitted entirely when its URL is +// unset, rather than showing a broken-image placeholder. +func orgInvitationHTML(tenant database.Tenant, inviter database.Human, link string) string { + var b strings.Builder + b.WriteString("
") + if tenant.LogoURL != nil && *tenant.LogoURL != "" { + fmt.Fprintf(&b, `%s logo`, *tenant.LogoURL, tenant.Name) + } + if inviter.AvatarURL != nil && *inviter.AvatarURL != "" { + fmt.Fprintf(&b, `%s`, *inviter.AvatarURL, inviter.Name) + } + fmt.Fprintf(&b, "

%s invites you to join the organization.

", tenant.Name) + fmt.Fprintf(&b, `

Accept the invitation within 5 hours.

`, link) + b.WriteString("

If you weren't expecting this, you can safely ignore this email.

") + b.WriteString("
") + return b.String() +} + +// AcceptOrgInvitationResult is what AcceptOrgInvitation hands back: the +// tenant joined, and a fresh session only when a new account was created +// (an already-logged-in caller keeps using their existing session — see +// AcceptOrgInvitation's doc). +type AcceptOrgInvitationResult struct { + Tenant database.Tenant + Session *Session // non-nil only when a new account was created by this call +} + +// AcceptOrgInvitation accepts a pending invitation identified by rawToken. +// Handles both cases the invitee can be in (operator decision: one +// combined endpoint, not separate signup-then-join calls): +// +// - existingHumanID != "": the caller already has a session. The +// invitation's email must match that account's own email +// (case-insensitive) — otherwise ErrOrgInvitationEmailMismatch, since +// accepting someone else's invitation under your own account would +// silently misattribute membership. +// - existingHumanID == "": the caller has no account yet. signupName and +// signupPassword create one, atomically joined to the org in the same +// transaction (database.AcceptOrgInvitationWithSignup) — the account's +// email is ALWAYS the invitation's own address, never client-supplied, +// so an invite token for one address can never mint an account under +// another. +func (s *Service) AcceptOrgInvitation(ctx context.Context, rawToken, existingHumanID, signupName, signupPassword string) (AcceptOrgInvitationResult, error) { + inv, err := s.db.GetOrgInvitationByHash(ctx, hashRawToken(rawToken)) + if err != nil { + if errors.Is(err, database.ErrNotFound) { + return AcceptOrgInvitationResult{}, ErrOrgInvitationInvalid + } + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: get org invitation: %w", err) + } + now := s.now() + if inv.AcceptedAt != nil || !inv.ExpiresAt.After(now) { + return AcceptOrgInvitationResult{}, ErrOrgInvitationInvalid + } + tenant, err := s.db.GetTenant(ctx, inv.TenantID) + if err != nil { + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: get tenant: %w", err) + } + + if existingHumanID != "" { + h, err := s.db.GetHuman(ctx, existingHumanID) + if err != nil { + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: get human: %w", err) + } + if normalizeEmailForCompare(h.Email) != inv.NormalizedEmail { + return AcceptOrgInvitationResult{}, ErrOrgInvitationEmailMismatch + } + if err := s.db.AcceptOrgInvitationForExistingHuman(ctx, inv.ID, inv.TenantID, existingHumanID, now); err != nil { + if errors.Is(err, database.ErrOrgInvitationConsumed) { + return AcceptOrgInvitationResult{}, ErrOrgInvitationInvalid + } + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: accept org invitation: %w", err) + } + return AcceptOrgInvitationResult{Tenant: tenant}, nil + } + + name := strings.TrimSpace(signupName) + if name == "" || len(signupPassword) < 8 { + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: name and a password of at least 8 characters are required to accept this invitation") + } + hash, err := bcrypt.GenerateFromPassword([]byte(signupPassword), bcrypt.DefaultCost) + if err != nil { + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: hash password: %w", err) + } + h, err := s.db.AcceptOrgInvitationWithSignup(ctx, inv.ID, inv.TenantID, name, inv.RawEmail, string(hash), now) + if err != nil { + if errors.Is(err, database.ErrOrgInvitationConsumed) { + return AcceptOrgInvitationResult{}, ErrOrgInvitationInvalid + } + if errors.Is(err, database.ErrConflict) { + return AcceptOrgInvitationResult{}, ErrEmailTaken + } + return AcceptOrgInvitationResult{}, fmt.Errorf("humanauth: accept org invitation with signup: %w", err) + } + session, err := s.mintSession(ctx, h) + if err != nil { + return AcceptOrgInvitationResult{}, err + } + return AcceptOrgInvitationResult{Tenant: tenant, Session: &session}, nil +} + +// normalizeEmailForCompare mirrors database's unexported normalizeEmail +// (lowercase + trim) — duplicated here rather than exported across the +// package boundary for one comparison. +func normalizeEmailForCompare(email string) string { + return strings.ToLower(strings.TrimSpace(email)) +} diff --git a/internal/ratelimit/policy.go b/internal/ratelimit/policy.go index 9ca536d..350e74b 100644 --- a/internal/ratelimit/policy.go +++ b/internal/ratelimit/policy.go @@ -66,6 +66,15 @@ type Policy struct { // dangerous credential-change action. PasswordResetIPRate float64 PasswordResetIPBurst int + // OrgInviteRate/OrgInviteBurst bound POST /v1/orgs/{id}/invites per + // inviting human (not IP — unlike the auth-surface buckets above, this + // caller is already authenticated, so their human ID is a stronger, + // unspoofable key than an IP would be). Deliberately its own tighter + // bucket, not the general requestLimitMiddleware tenant/key buckets: + // this endpoint sends a real outbound email per call, the same + // email-bombing concern PasswordResetIPRate exists for. + OrgInviteRate float64 + OrgInviteBurst int } const ( @@ -89,6 +98,7 @@ func DefaultPolicy() Policy { MaxRecipientsPerMessage: 50, AuthIPRate: 1, AuthIPBurst: 10, PasswordResetIPRate: 1.0 / 60, PasswordResetIPBurst: 3, + OrgInviteRate: 1.0 / 30, OrgInviteBurst: 10, } } @@ -117,6 +127,7 @@ func (p Policy) Validate() error { count("max recipients per message", p.MaxRecipientsPerMessage, 1000), rate("auth IP rate", p.AuthIPRate), count("auth IP burst", p.AuthIPBurst, maxBurst), rate("password reset IP rate", p.PasswordResetIPRate), count("password reset IP burst", p.PasswordResetIPBurst, maxBurst), + rate("org invite rate", p.OrgInviteRate), count("org invite burst", p.OrgInviteBurst, maxBurst), } { if e != nil { return e From 9de8825be55ee558b67425ef5d891cff82351513 Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 09:23:56 +0100 Subject: [PATCH 2/5] fix: address first Greptile review pass on organization invitations - escape owner/inviter-supplied names in the invitation HTML email - refuse to send an invite when the mailer is configured but the dashboard base URL is not, matching password reset's guard - rate-limit the public invite-accept endpoint (bcrypt per call) - recheck expiry atomically at token-consume time, not just when the request started - invalidate a prior pending invite when re-inviting the same address instead of stacking duplicate valid links - validate the invitee email before it reaches the database --- .ilana/decisions.md | 1 + .ilana/ledger.md | 3 + .ilana/state.json | 2 +- internal/api/abuse.go | 45 +++++++++++ internal/api/humanauth_handler.go | 9 +++ internal/api/routes.go | 6 +- internal/database/org_invitations.go | 45 +++++++++-- internal/humanauth/org_invitation_test.go | 95 +++++++++++++++++++++++ internal/humanauth/service.go | 24 +++++- 9 files changed, 215 insertions(+), 15 deletions(-) diff --git a/.ilana/decisions.md b/.ilana/decisions.md index 0aa233a..81e418e 100644 --- a/.ilana/decisions.md +++ b/.ilana/decisions.md @@ -221,3 +221,4 @@ Process decisions above (DEC-001..DEC-007) belong to the v0.23 FLEET run and sta - DEC-214 [v0.47 phase 1 fourth Greptile pass]: fixed 2 P1 findings posted minutes after DEC-213's password reset commit landed, both real. (1) `Refresh` revoked the old token and inserted its replacement as two independent statements; a `ResetPassword` landing in the gap between them would run its "revoke every active token" update before the replacement existed, so that one session survived a reset that was supposed to end all of them. Fixed with `database.RotateRefreshToken`, a single transaction combining both, closing the gap the same way DEC-212's `TouchLoginAndCreateRefreshToken` closed an analogous one for login. Proven by `TestResetPasswordDuringConcurrentRefreshEndsAllSessions`, which fires `Refresh` and `ResetPassword` concurrently against the same session and asserts any token the refresh managed to mint is unusable afterward. (2) `ForgotPassword` would build a relative `/reset-password?token=...` link and actually send it if `MAILX_SYSTEM_TENANT_ID`/`_FROM_ADDRESS` were set but `MAILX_DASHBOARD_BASE_URL` was left unset — a real, unusable email. `ForgotPassword` now refuses to send (same generic-error/logged-not-surfaced path as "no mailer configured") when the dashboard base URL is empty, requiring all three system-mail env vars together rather than degrading partially. Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, Docker rebuild+boot smoke clean. - DEC-215 [v0.47 phase 1 fifth Greptile pass + CI fix]: fixed 3 more findings from Greptile's fourth review pass (posted alongside DEC-214's two, initially missed since they targeted older code from the third pass rather than the just-shipped password reset) plus a CI-only failure the fixes exposed. (1) "Erased mail remains on disk" (P1): `materializeOne`'s GDPR-erasure disk-cleanup branch logged a failed file delete and then `return nil` (success) regardless — since the driving `broadcast_recipients` row is already gone, nothing else would ever retry it, and the caller's generic error/metrics path never even saw the failure. Now returns the wrapped delete error instead of nil, so the caller's existing "transient error" handling logs/counts it AND — critically — no longer calls `finishMaterialized` against a row that no longer exists. (2) "Stale last-login response" (P2): `Login` echoed back its own `loginAt` regardless of whether `TouchLoginAndCreateRefreshToken`'s monotonic guard actually applied it — if a concurrent later login won the race, the response would report an older `last_login_at` than what was actually stored. `TouchLoginAndCreateRefreshToken` now also returns the value ACTUALLY stored (a follow-up `SELECT` inside the same transaction, unconditionally, not only on a lost race, since a plain conditional re-fetch reads more branches than it saves), and `Login` uses that instead of its own local timestamp. (3) "Cleanup metric loses its phase" (P2): `observability.broadcastPhases`'s allowlist (`snapshot`/`materialize`/`complete`) didn't include `erasure_cleanup`, so `BroadcastExpansionBatch("erasure_cleanup", ...)` silently collapsed to the `other` label bucket — added `erasure_cleanup` to the allowlist and also added an `"ok"` observation on the cleanup-succeeded path (previously only the failure path recorded anything for this phase at all). (4) CI-only failure (Go CI workflow on PR #22, unrelated to the above 3): `TestTouchHumanLoginIsMonotonic`/`TestTouchLoginAndCreateRefreshTokenIsAtomicAndMonotonic`/`TestLoginRecordsLastLoginButSignUpDoesNot` failed intermittently in GitHub Actions (not locally, where Docker Postgres happened not to trigger it) because Go's `time.Now()` carries nanosecond precision but PostgreSQL's `timestamptz` only stores microseconds — a round-tripped value's `.Equal()` against the original in-memory value fails on the truncated nanosecond tail. Fixed by truncating to microsecond precision at the source: `humanauth.NewService`'s default clock (`time.Now().UTC().Truncate(time.Microsecond)`) and the affected test helpers in `humans_test.go`, rather than loosening the equality checks — the truncation reflects the actual precision PostgreSQL guarantees, so this closes the whole class of tests, not just the 3 that happened to fail this run. All verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, Docker rebuild+boot smoke clean. - DEC-216 [v0.47 phase 1 follow-up]: added organization invitations. Four scoped decisions asked via AskUserQuestion before coding (operator confirmed all recommended defaults, no assumptions): avatar/logo as plain nullable URL columns only (no upload pipeline this milestone); a single combined accept endpoint handling both an already-logged-in invitee and a no-account-yet invitee (signup folded into acceptance), rather than separate signup-then-join calls; owner-only sending (matches today's single-role `tenant_members` model — no broader RBAC added); a dedicated tighter rate-limit bucket, same pattern as password reset. Migration 000030 adds `org_invitations` (id, tenant_id FK CASCADE, invited_by FK CASCADE, normalized_email/raw_email, token_hash UNIQUE-indexed, expires_at, accepted_at nullable, created_at; partial index on `(tenant_id, normalized_email) WHERE accepted_at IS NULL`) plus `tenants.logo_url` and `humans.avatar_url` (both nullable TEXT). `humanauth.InviteToOrganization(ctx, inviterHumanID, tenantID, email)` re-checks `database.IsTenantOwner` itself (never trusts a client-supplied "am I the owner" claim) — returns `ErrNotOrgOwner` otherwise — then creates a 5-hour single-use token (`OrgInvitationTTL`, same hashed-token shape as `password_reset_tokens`) and emails it through the SAME `humanauth.Mailer`/`WithDashboardBaseURL` wiring password reset already established (DEC-213) — no new mailer plumbing. The email body (`orgInvitationHTML`) shows the org logo and inviter avatar only when their URL fields are set (never a broken-image placeholder), the org name, and the accept link; degrades the same way as `ForgotPassword` when no mailer is configured (token still created, durable, logged-not-surfaced). `AcceptOrgInvitation(ctx, rawToken, existingHumanID, signupName, signupPassword)` implements the combined flow: with `existingHumanID` set, the invitation's email must case-insensitively match that account's own email (`ErrOrgInvitationEmailMismatch` otherwise) and `database.AcceptOrgInvitationForExistingHuman` atomically consumes the token (`RowsAffected`-checked, same single-use race guard as `ResetPassword`) and inserts the `tenant_members` row (`ON CONFLICT DO NOTHING`, so re-clicking an already-joined invite is not an error); with it empty, `database.AcceptOrgInvitationWithSignup` atomically consumes the token, creates the human account, and inserts membership in ONE transaction — critically, the account's email is ALWAYS the invitation's own stored address, never anything client-supplied, so a token issued for one address can never mint an account under another. Both failure paths (unknown/expired/already-accepted/lost-race token) collapse to one sentinel `ErrOrgInvitationInvalid`, same anti-enumeration-adjacent posture as password reset's collapsed error. `POST /v1/orgs/{id}/invites` sits behind `humanAuthMiddleware` then a new `orgInviteLimitMiddleware` (`Policy.OrgInviteRate`/`OrgInviteBurst`, default 1/30 rps burst 10, env `MAILX_LIMIT_ORG_INVITE_RPS`/`_BURST`) keyed by the inviting human's ID rather than IP — unlike the unauthenticated auth-surface buckets, this caller already has a verified identity, which is a stronger key than IP and avoids punishing every user behind a shared NAT/proxy for one owner's invite volume. `POST /v1/orgs/invites/accept` is deliberately NOT behind `humanAuthMiddleware` (an invitee may have no account yet); it reads an optional bearer token directly instead. Documented in `internal/api/openapi.go` (`InviteRequest`/`AcceptInviteRequest`/`AcceptInviteResponse` schemas, both new paths) — `TestOpenAPIRoutesMatchRuntime`/`TestOpenAPISpecParses` still pass. 5 new tests in `internal/humanauth/org_invitation_test.go` (owner-only enforcement, email-mismatch-on-existing-account, combined signup+join for an unknown invitee including using the invitation's own email over any client-supplied one, TTL expiry, and a signup-conflict-rolls-back-the-whole-transaction case proving a failed accept never silently consumes the token) all passing against real Postgres. Verified: `gofmt`/`go vet`/`go build` clean; full `go test ./...` and `go test -race ./...` clean against real Postgres+Redis; migration 000030 validated via a down/up/re-up round-trip against the dev Postgres container; Docker rebuild+boot smoke clean; live-curl-verified against the running server — owner-only 403 for a non-owner, invitation row durably created despite "no mailer configured" (same known dev-environment gap DEC-213 hit, not a new one), 422 `invalid_invitation_token` for a bogus accept token, and the new rate limiter observed live (9 consecutive charged calls then 429 at burst 10). +- DEC-217 [org invitations, first Greptile pass]: fixed 6 findings on PR #23 (organization invitations, DEC-216), all posted on the initial review of that PR's diff. (1) "Invitation email HTML injection" (P1/security): `orgInvitationHTML` interpolated the owner-supplied organization name (and inviter name/avatar URL) directly into the email's HTML without escaping, letting an org owner inject deceptive content or links into mail MailX itself sends. Fixed with `html.EscapeString` on every interpolated value, for both text nodes and attribute values. (2) "Invitation link lacks a host" (P1): `InviteToOrganization` would send a relative `/accept-invite?token=...` link if the mailer was configured but `MAILX_DASHBOARD_BASE_URL` wasn't — same class of bug as DEC-215's password-reset fix, fixed the same way (refuse to send, same generic-error/logged-not-surfaced path as no mailer at all). (3) "Invite acceptance lacks throttling" (P1/security): the public `POST /v1/orgs/invites/accept` route ran a full bcrypt hash per no-account-yet acceptance with no rate limit at all, letting concurrent requests against one invitation exhaust server CPU before the single-use token was consumed. Added `orgInviteAcceptIPLimitMiddleware`, reusing `AuthIPRate`/`AuthIPBurst` (same bcrypt-per-call cost class as Login, no extra email-sending risk that would justify a tighter budget). (4) "Expired invitation can be accepted" (P1): both `AcceptOrgInvitationForExistingHuman` and `AcceptOrgInvitationWithSignup` only checked `accepted_at IS NULL` in their consume UPDATE — a request that passed the service layer's expiry check just before the 5-hour deadline could still be granted membership if DB processing/lock-wait carried it past that deadline. Fixed by adding `AND expires_at > $2` to both UPDATE statements, so expiry is atomically rechecked at the moment of consumption, not just when the request started; `ErrOrgInvitationConsumed` now covers both "already accepted" and "expired at consume time" (deliberately not distinguished, same anti-enumeration posture as the rest of this token family). (5) "Pending invites are not deduplicated" (P2): `idx_org_invitations_tenant_email`'s own comment claimed it existed to support checking for an existing pending invite before sending, but `CreateOrgInvitation` never performed that check and the index isn't unique, so repeated invites to the same address created independently valid links. Enforced the documented intent instead of removing it: `CreateOrgInvitation` now invalidates (expires) any other still-pending invitation for the same (tenant, email) inside the same transaction as the new insert — re-inviting resends, it doesn't stack a duplicate. (6) "Invalid addresses expose database errors" (P2): `handleCreateInvite` accepted any non-empty string as an email and let a malformed one (e.g. internal whitespace) trip `org_invitations`' own CHECK constraint, surfacing a raw PostgreSQL error as `invalid_invitation`. Fixed by validating with `net/mail.ParseAddress` before ever reaching the database, returning a clean `invalid_email` response instead. All verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean (4 new regression tests: HTML-escaping, missing-dashboard-URL refusal, re-invite invalidates the prior link, plus the existing suite unchanged), Docker rebuild+boot smoke clean. diff --git a/.ilana/ledger.md b/.ilana/ledger.md index d03395f..ece901e 100644 --- a/.ilana/ledger.md +++ b/.ilana/ledger.md @@ -305,3 +305,6 @@ User listed 5 named findings plus a failing Go CI check and asked to verify/fix ## 2026-09-25 | v0.47 phase 1 follow-up: organization invitations | GATE PASS Continued from a fresh-chat handoff per CLAUDE.md's Ilana-first rule (read ledger.md/state.json/architecture.md before touching code). Asked 4 scoped AskUserQuestion decisions before coding per the handoff's explicit instruction not to assume: avatar/logo as plain URL fields (no upload pipeline), a single combined accept-invite endpoint (not separate signup-then-join), owner-only sending, and a dedicated tighter rate-limit bucket - user accepted all 4 recommended defaults. Read the existing password-reset code (DEC-213) first as the pattern to mirror (hashed single-use tokens, own-pipeline mailer, collapsed anti-enumeration-style error) rather than designing from scratch. Built: migration 000030 (`org_invitations` table, `tenants.logo_url`, `humans.avatar_url`), `database.IsTenantOwner`/`CreateOrgInvitation`/`GetOrgInvitationByHash`/`AcceptOrgInvitationForExistingHuman`/`AcceptOrgInvitationWithSignup` (the last two atomic, RowsAffected-guarded single-use consumption matching `ResetPassword`'s race-safety pattern), `humanauth.InviteToOrganization`/`AcceptOrgInvitation` (owner-only server-side re-check, 5-hour TTL, combined existing-session-or-signup accept, invitation's own email always wins over any client-supplied one), new `POST /v1/orgs/{id}/invites` (owner-only, per-human rate-limited) and `POST /v1/orgs/invites/accept` (deliberately outside `humanAuthMiddleware` since the invitee may have no account) routes and handlers, a new `orgInviteLimitMiddleware` (keyed by human ID, not IP, since the caller here is already authenticated - a deliberate departure from the IP-keyed auth-surface buckets), and OpenAPI documentation for both endpoints. 5 new tests in `internal/humanauth/org_invitation_test.go` (owner-only enforcement, email-mismatch on an existing account, combined signup+join using the invitation's own email, TTL expiry, and a signup-conflict-rolls-back-the-whole-transaction case). Verified: gofmt/go vet/go build clean; full `go test ./...` and `go test -race ./...` clean against real Postgres+Redis (existing Docker stack); migration 000030 validated via a down/up/re-up round-trip against the dev Postgres container; Docker rebuild+boot smoke clean; live-curl-verified against the running server - owner-only 403 for a non-owner, an invitation row durably created despite "no mailer configured" (the same known dev-environment gap DEC-213 already hit, not a regression), 422 for a bogus accept token, and the new rate limiter observed live (9 charged calls then 429 exactly at burst 10). Ilana updated: milestones.md (v0.47 phase 1 section extended), architecture.md ("Human accounts & organizations" section extended with the invitations subsection), decisions.md (DEC-216), state.json (DEC counter, mailx status). + +## 2026-09-25 | org invitations first Greptile pass | GATE PASS +User asked to check Greptile's comments on the new org-invitations PR (#23). Fetched via gh api and found 6 findings, all genuinely new (first review of that diff), 4 of them P1. Fixed all 6: HTML injection in the invite email (owner-supplied org/inviter names were interpolated unescaped, letting an org owner inject content into mail MailX itself sends - fixed with html.EscapeString), a missing-dashboard-URL guard identical to the one already fixed for password reset (DEC-215), no rate limiting at all on the public accept-invite endpoint despite it running bcrypt per call (added an IP-keyed limiter reusing the login-cost-class bucket), an expiry check that only ran at the service layer and not atomically at DB-consume time (a request could slip past its 5-hour deadline during processing and still be granted membership - fixed by rechecking expires_at inside the same UPDATE that consumes the token), a misleading index comment claiming deduplication support that was never actually enforced (fixed by invalidating any prior pending invite to the same address when a new one is sent, rather than just removing the comment), and unvalidated email input reaching a database CHECK constraint and leaking a raw Postgres error to the caller (added net/mail.ParseAddress validation before the DB call). Verified: gofmt/go vet/go build clean, full go test ./... and go test -race ./... clean including 3 new regression tests, Docker rebuild+boot smoke clean. Also discovered and corrected a process gap this session: PR #22 (human accounts/password reset) had already been merged to main by the time the org-invitations commit was made, so that commit was sitting unpushed on Feranmi_works with no PR - opened PR #23 to bring it into main properly rather than assuming a push to the feature branch alone was sufficient. Ilana updated: decisions.md (DEC-217), state.json (DEC counter). diff --git a/.ilana/state.json b/.ilana/state.json index d20f33d..bf89cf8 100644 --- a/.ilana/state.json +++ b/.ilana/state.json @@ -18,7 +18,7 @@ "DEF": 27, "CR": 24, "RSK": 43, - "DEC": 216, + "DEC": 217, "MET": 72, "ETH": 1 }, diff --git a/internal/api/abuse.go b/internal/api/abuse.go index b5dbab9..fe70935 100644 --- a/internal/api/abuse.go +++ b/internal/api/abuse.go @@ -244,6 +244,51 @@ func orgInviteLimitMiddleware(a *AbuseControls) func(http.Handler) http.Handler } } +// orgInviteAcceptIPLimitMiddleware protects the public POST +// /v1/orgs/invites/accept route, which is unauthenticated (the invitee has +// no session or API key yet - the invite token itself is the credential) +// and, on the no-account-yet path, runs a full bcrypt hash before the +// single-use token is atomically consumed. Without a limit, concurrent +// requests against one valid invitation could exhaust server CPU on +// repeated expensive hashing before any of them wins the race (Greptile +// P1, PR #23). Reuses AuthIPRate/AuthIPBurst rather than a new bucket: +// this route is the same cost class as Login (one bcrypt op per call) and +// carries no extra email-sending risk that would justify a tighter budget +// like PasswordResetIPRate's. +func orgInviteAcceptIPLimitMiddleware(a *AbuseControls) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + if a == nil || a.Limiter == nil { + return next + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := authClientIP(r, a.TrustedProxyCIDRs) + p := a.Policy + dec, err := a.Limiter.Allow(r.Context(), + ratelimit.Bucket{Key: "org:invite:accept:ip:" + ip, Rate: p.AuthIPRate, Burst: p.AuthIPBurst, Cost: 1}, + ) + switch { + case err != nil: + a.Metrics.AbuseDecision("org_invite_accept", "unavailable") + a.log().Error("rate_limiter_unavailable", "route_class", "org_invite_accept") + e := newError(ErrTemporarilyUnavailable, "rate_limiter_unavailable", "request limiting is temporarily unavailable; retry later") + e.RetryAfter = unavailableRetryAfter + writeError(w, r, e) + case dec.Impossible: + a.Metrics.AbuseDecision("org_invite_accept", "impossible") + writeError(w, r, newError(ErrInternal, "internal_error", "request limit is misconfigured")) + case !dec.Allowed: + a.Metrics.AbuseDecision("org_invite_accept", "limited") + e := newError(ErrRateLimited, "org_invite_accept_rate_limited", "too many invitation-acceptance attempts from this address; retry after the interval in Retry-After") + e.RetryAfter = ratelimit.RetryAfterSeconds(dec.RetryAfter) + writeError(w, r, e) + default: + a.Metrics.AbuseDecision("org_invite_accept", "allowed") + next.ServeHTTP(w, r) + } + }) + } +} + // authClientIP returns the IP to key the auth rate limiter on: the // connection peer (RemoteAddr), UNLESS that peer is a configured trusted // proxy, in which case the rightmost entry of X-Forwarded-For is used diff --git a/internal/api/humanauth_handler.go b/internal/api/humanauth_handler.go index 769eb7d..be198bd 100644 --- a/internal/api/humanauth_handler.go +++ b/internal/api/humanauth_handler.go @@ -5,6 +5,7 @@ import ( "errors" "log/slog" "net/http" + stdmail "net/mail" "strings" "time" @@ -285,6 +286,14 @@ func (h *humanAuthHandler) handleCreateInvite(w http.ResponseWriter, r *http.Req writeError(w, r, newError(ErrInvalidRequest, "invalid_json", "email is required")) return } + // Validate BEFORE it ever reaches the database: an address with, say, + // internal whitespace would otherwise trip the invitation table's own + // constraint and surface a raw PostgreSQL error to the caller instead + // of a useful validation response (Greptile P2, PR #23). + if _, err := stdmail.ParseAddress(req.Email); err != nil { + writeError(w, r, newError(ErrValidation, "invalid_email", "email is not a valid address")) + return + } if err := h.svc.InviteToOrganization(r.Context(), humanID, tenantID, req.Email); err != nil { if errors.Is(err, humanauth.ErrNotOrgOwner) { writeError(w, r, newError(ErrForbidden, "not_org_owner", "only an organization owner can send invitations")) diff --git a/internal/api/routes.go b/internal/api/routes.go index e47af7c..c747773 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -190,8 +190,10 @@ func newMux(h *emailHandler, authSvc authService, readiness func() error, extras mux.Handle("POST /v1/orgs/{id}/invites", orgsAuthenticated(chain(http.HandlerFunc(ha.handleCreateInvite), orgInviteLimitMiddleware(abuse)))) // Deliberately NOT behind orgsAuthenticated: the invitee may have no // account yet, so a bearer token here is optional — see - // handleAcceptInvite's doc. - mux.HandleFunc("POST /v1/orgs/invites/accept", ha.handleAcceptInvite) + // handleAcceptInvite's doc. Still rate-limited by IP + // (orgInviteAcceptIPLimitMiddleware) since this route is public and + // the no-account-yet path runs a full bcrypt hash per call. + mux.Handle("POST /v1/orgs/invites/accept", chain(http.HandlerFunc(ha.handleAcceptInvite), orgInviteAcceptIPLimitMiddleware(abuse))) } if len(extras) > 0 && extras[0].feedback != nil { // Deliberately NOT under /v1 and NOT authenticateMiddleware: this is the diff --git a/internal/database/org_invitations.go b/internal/database/org_invitations.go index c00c7fe..a36269e 100644 --- a/internal/database/org_invitations.go +++ b/internal/database/org_invitations.go @@ -42,22 +42,44 @@ func (db *DB) IsTenantOwner(ctx context.Context, tenantID, humanID string) (bool return role == "owner", nil } -// CreateOrgInvitation inserts a new invitation row. +// CreateOrgInvitation inserts a new invitation row. Any other still-pending +// (unaccepted, unexpired) invitation for the same (tenant, email) is +// invalidated first - re-inviting an address resends, it doesn't stack a +// second independently valid link (idx_org_invitations_tenant_email exists +// for exactly this check; enforcing it here, not just indexing for it, was +// a real gap - Greptile P2, PR #23). func (db *DB) CreateOrgInvitation(ctx context.Context, tenantID, invitedBy, email, tokenHash string, expiresAt time.Time) (OrgInvitation, error) { id, err := newID() if err != nil { return OrgInvitation{}, err } + tx, err := db.pool.Begin(ctx) + if err != nil { + return OrgInvitation{}, fmt.Errorf("database: begin create org invitation: %w", normalizeErr(err)) + } + defer func() { _ = tx.Rollback(ctx) }() + + normalized := normalizeEmail(email) + if _, err := tx.Exec(ctx, + `UPDATE org_invitations SET expires_at = now() WHERE tenant_id = $1 AND normalized_email = $2 AND accepted_at IS NULL AND expires_at > now()`, + tenantID, normalized, + ); err != nil { + return OrgInvitation{}, fmt.Errorf("database: invalidate prior pending invitation: %w", normalizeErr(err)) + } + var inv OrgInvitation - err = db.pool.QueryRow(ctx, ` + err = tx.QueryRow(ctx, ` INSERT INTO org_invitations (id, tenant_id, invited_by, normalized_email, raw_email, token_hash, expires_at) VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING id, tenant_id, invited_by, normalized_email, raw_email, token_hash, expires_at, accepted_at, created_at`, - id, tenantID, invitedBy, normalizeEmail(email), email, tokenHash, expiresAt, + id, tenantID, invitedBy, normalized, email, tokenHash, expiresAt, ).Scan(&inv.ID, &inv.TenantID, &inv.InvitedBy, &inv.NormalizedEmail, &inv.RawEmail, &inv.TokenHash, &inv.ExpiresAt, &inv.AcceptedAt, &inv.CreatedAt) if err != nil { return OrgInvitation{}, normalizeErr(err) } + if err := tx.Commit(ctx); err != nil { + return OrgInvitation{}, fmt.Errorf("database: commit create org invitation: %w", normalizeErr(err)) + } return inv, nil } @@ -76,9 +98,16 @@ func (db *DB) GetOrgInvitationByHash(ctx context.Context, tokenHash string) (Org return inv, nil } -// ErrOrgInvitationConsumed means this exact invitation row was already -// marked accepted by a concurrent call — same race-safety pattern as -// ErrPasswordResetTokenConsumed. +// ErrOrgInvitationConsumed covers both ways the accept UPDATE can affect +// zero rows: the invitation was already accepted by a concurrent call, OR +// it passed its expires_at between the service layer's own expiry check +// and this statement actually running (a slow request, GC pause, or lock +// wait can carry a borderline-valid request past the 5-hour deadline) - +// the WHERE clause rechecks expiry here for exactly that reason, so +// nothing can be granted membership after the window closes (Greptile P1, +// PR #23). Same race-safety pattern as ErrPasswordResetTokenConsumed; +// deliberately not distinguished from the "already accepted" case, since +// the caller collapses both into the same generic ErrOrgInvitationInvalid. var ErrOrgInvitationConsumed = errors.New("database: org invitation already accepted") // AcceptOrgInvitationForExistingHuman atomically: (1) marks the invitation @@ -95,7 +124,7 @@ func (db *DB) AcceptOrgInvitationForExistingHuman(ctx context.Context, invitatio } defer func() { _ = tx.Rollback(ctx) }() - tag, err := tx.Exec(ctx, `UPDATE org_invitations SET accepted_at = $2 WHERE id = $1 AND accepted_at IS NULL`, invitationID, now) + tag, err := tx.Exec(ctx, `UPDATE org_invitations SET accepted_at = $2 WHERE id = $1 AND accepted_at IS NULL AND expires_at > $2`, invitationID, now) if err != nil { return fmt.Errorf("database: consume org invitation: %w", normalizeErr(err)) } @@ -132,7 +161,7 @@ func (db *DB) AcceptOrgInvitationWithSignup(ctx context.Context, invitationID, t } defer func() { _ = tx.Rollback(ctx) }() - tag, err := tx.Exec(ctx, `UPDATE org_invitations SET accepted_at = $2 WHERE id = $1 AND accepted_at IS NULL`, invitationID, now) + tag, err := tx.Exec(ctx, `UPDATE org_invitations SET accepted_at = $2 WHERE id = $1 AND accepted_at IS NULL AND expires_at > $2`, invitationID, now) if err != nil { return Human{}, fmt.Errorf("database: consume org invitation: %w", normalizeErr(err)) } diff --git a/internal/humanauth/org_invitation_test.go b/internal/humanauth/org_invitation_test.go index 62d9b43..75126b2 100644 --- a/internal/humanauth/org_invitation_test.go +++ b/internal/humanauth/org_invitation_test.go @@ -245,3 +245,98 @@ func TestAcceptInviteEmailAlreadyTaken(t *testing.T) { t.Fatal("expected the invitation to remain unconsumed after a rolled-back signup conflict") } } + +// TestInviteEmailHTMLIsEscaped is a regression test for Greptile's P1 +// finding (PR #23): an owner-supplied organization name (or inviter name) +// containing HTML must not reach the invitation email unescaped, since an +// org owner could otherwise inject deceptive content/links into mail MailX +// sends on their behalf. +func TestInviteEmailHTMLIsEscaped(t *testing.T) { + db := newTestDB(t) + mailer := &fakeMailer{} + svc, err := NewService(db, testSecret(), WithMailer(mailer), WithDashboardBaseURL("https://app.mailx.dev")) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + owner, err := svc.SignUp(ctx, ``, "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + tenant, err := svc.CreateOrganization(ctx, owner.Human.ID, `Acme `, "") + if err != nil { + t.Fatal(err) + } + if err := svc.InviteToOrganization(ctx, owner.Human.ID, tenant.ID, "invitee@example.com"); err != nil { + t.Fatal(err) + } + htmlBody := mailer.calls[0].html + if strings.Contains(htmlBody, "