Compare commits

...

9 Commits

Author SHA1 Message Date
Niklas Ye 4358e84b24 Set the chart's placeholder version to 0.34.0
CI / test (push) Successful in 4s
CI / chart (push) Successful in 1s
CI / security (push) Successful in 12s
Release / test (push) Successful in 4s
Release / chart (push) Successful in 7s
Release / image (push) Successful in 2m27s
Release / binaries (push) Successful in 2m35s
Release / scan-image (push) Successful in 3s
2026-10-02 22:08:35 +02:00
niklas f45dc2f925 Merge pull request 'internal/api: unify human/service-account authz into one Caller type' (#24) from unify-caller-authz into main
CI / chart (push) Successful in 1s
CI / security (push) Successful in 22s
CI / test (push) Successful in 27s
2026-10-02 20:06:37 +00:00
Niklas Ye 774fdfcaa8 internal/api: unify human/service-account authz into one Caller type
CI / chart (pull_request) Successful in 1s
CI / security (pull_request) Successful in 15s
CI / test (pull_request) Successful in 5m21s
ctxUser/ctxTeams (human) and ctxServiceAccount (+ a synthetic ctxTeams
entry, service account) used to be two parallel, un-unified context
representations -- every authz predicate had to remember which one(s) it
needed, and every place that forgot either wrongly 403'd a service account
(terdut-server#23, terdut-operator#3), crashed on an unchecked zero-value
user id, or silently no-op'd. New internal/api/caller.go collapses both
into one Caller, stored under one ctxCaller key by serveAs/serveAsServiceAccount;
every existing predicate (userFromContext, callerTeamIDs, callerRole,
callerIsAdmin, isInstanceServiceAccount, AdminOnly, requireSelfOrAdmin,
requireTeamOwner, OperatorModeBlock) now reads through it, with identical
behavior for every untouched call site (alerts.go, incidents.go,
schedule.go, stats.go, etc.) -- confirmed by the full existing suite
passing unchanged.

Four real fixes land alongside the refactor, not just the restructuring:

1. callerMayManageServiceAccount gains the one load-bearing branch this
   exists for: an instance-scoped service account may now manage (mint or
   revoke a key on) any team-scoped account, not only a human admin, that
   team's human owner, or the account itself. handleCreateServiceAccount
   already let an instance-scoped caller *create* a team-scoped account for
   any team; adopting or rotating one it didn't just create in the same
   call -- terdut-operator's own documented crash-window recovery -- had no
   equivalent permission and 403'd forever. Closes terdut-operator#3.

2. handleCreateInvite wrote a service-account caller's zero-value user id
   straight into invites.created_by (nullable, but never passed as nil),
   which foreign-key-violates against users(id) -- a 500, not success, for
   any team-scoped service account minting an invite. Fixed the same way
   handleCreateServiceAccount already handles the analogous case. Found
   live while verifying this change, not filed separately since it's fixed
   in the same place it was found.

3. handleMe and handleTestNotification 500'd for a service-account caller
   (fetchUser/the ntfy_topic lookup against a zero-value user id that
   matches no row); handleDismissOnboarding silently no-op'd (UPDATE ...
   WHERE id = 0). All three now call Caller.AsHuman() and return an
   explicit 403 ("this endpoint is for human accounts only").

4. Ratifies, rather than further narrows, two capabilities a team-scoped
   service account already had by construction and this document's own
   text once called "a gap acknowledged rather than closed": owner-equivalent
   reach over membership/invites, and minting another service account for
   its own team. terdut-operator's new TerdutTeam invite-minting feature is
   about to depend on the first one, so this makes it documented, tested,
   intentional behavior instead of an accident nobody was supposed to rely
   on.

AdminOnly/requireSelfOrAdmin are unchanged in effect: still human-only,
forever, for every scope of service account -- confirmed by
TestAdminOnly_RefusesEveryServiceAccountScope. terdut-server#23's named
routes (POST /api/users, PUT /api/admin/settings) were never the right
thing to widen; its real fix is the terdut-operator invite feature,
recorded in SERVICE-ACCOUNTS.md's "What this unblocks" and closing that
issue once it ships.

SERVICE-ACCOUNTS.md amended in place (not a new file, its own established
convention) to describe the as-built Caller model, correct its own
aspirational claim about AdminOnly that TEAM-LOOKUP.md had already flagged
as not matching shipped code, and record all of the above.
2026-10-02 21:52:43 +02:00
Niklas Ye fd26fef1ba Set the chart's placeholder version to 0.33.2
CI / test (push) Successful in 4s
CI / chart (push) Successful in 1s
CI / security (push) Successful in 12s
Release / test (push) Successful in 3s
Release / chart (push) Successful in 3s
Release / binaries (push) Successful in 16s
Release / image (push) Successful in 1m8s
Release / scan-image (push) Successful in 1s
make helm-package passes --version and --app-version from the tag, so
these fields decide nothing about what is published -- but a tree heading
for v0.33.2 that still says 0.33.1 tells its reader something false.
Same as 4b15079 and 6f8499f before it.
2026-10-02 09:42:50 +02:00
Niklas Ye a9d788cc83 Wait for Postgres to accept connections before the main container starts
A Deployment created before Postgres has finished its very first boot --
initdb plus Patroni leader election, on a from-scratch postgres-operator
cluster -- crash-looped a few times. db.Open()'s own ping-retry budget
(pingAttempts/pingRetryDelay, internal/db/db.go) is sized for a much
shorter, different race -- NetworkPolicy propagation, a few seconds -- not
for genuine first-time cluster creation, which routinely takes longer, so
it exhausted and the process exited before ever binding its HTTP port. A
startupProbe cannot fix that: the crash happens before there is anything
to probe.

Added a wait-for-postgres init container instead: it loops pg_isready
against database.dsn until Postgres actually answers, before the main
container's own, unchanged retry budget gets a chance to run out.
pg_isready needs no credentials -- it reports PQPING_OK on anything that
amounts to a Postgres backend answering, including an auth challenge --
so no PGPASSWORD is wired into it.

Chart-only; no Go code changed. database.waitForPostgres.enabled defaults
to true and can be turned off if something else already guarantees
Postgres is reachable before this Deployment is created.
2026-10-02 09:42:42 +02:00
Niklas Ye 4b15079ac2 Set the chart's placeholder version to 0.33.1
CI / chart (push) Successful in 1s
CI / security (push) Successful in 19s
CI / test (push) Successful in 4m42s
Release / test (push) Successful in 3s
Release / chart (push) Successful in 2s
Release / image (push) Successful in 2m11s
Release / scan-image (push) Successful in 3s
Release / binaries (push) Successful in 2m47s
Cosmetic: `make helm-package` passes --version/--app-version from the
tag, so this field decides nothing about what gets published. Still done
so the tree doesn't say 0.33.0 while heading for a v0.33.1 release.
Cites fc9f47c, the fix this version actually is.
2026-10-02 09:18:28 +02:00
Niklas Ye fc9f47cc8d Refuse an OIDC sign-in from creating the very first user
Closes the race terdut-operator#1 found: /api/bootstrap and OIDC
auto-provisioning both key off the same signal (SELECT COUNT(*) FROM
users) with no coordination between them, so an otherwise-ordinary OIDC
sign-in against a freshly-created, not-yet-bootstrapped install could
create user #1 itself and take the one slot /api/bootstrap expects to win
uncontested (terdut-operator's own design, DESIGN.md §1/§6, assumes it is
the only caller). The operator has no way to recover from losing that
race -- it never gets a credential, and nothing it owns can clear the
occupying user row.

resolveSSOUser now checks the same gate handleBootstrap already does,
right where it's about to create a brand-new user (an identity nobody has
linked yet, that also matches no existing local account by email) -- not
anywhere else, since every other sign-in on an already-bootstrapped
install is unaffected. New sso_error code `not_bootstrapped`: the person
sees "this install is still setting up, try again in a moment" and a
second attempt once something has actually bootstrapped succeeds
normally, same as any other first sign-in.

Does not fix the other half of that issue (BootstrapStateLost's own
"delete and recreate" instructions still don't work once something has
occupied the slot some other way) -- this closes the specific race, not
every path to that state.
2026-10-02 09:12:32 +02:00
Niklas Ye bc9f793f1f Add GET /api/teams?name= (TEAM-LOOKUP.md)
CI / chart (push) Successful in 1s
CI / security (push) Successful in 1m15s
CI / test (push) Successful in 5m48s
Resolves the gap TEAM-LOOKUP.md raised: an instance-scoped service
account had no way to recover a team's id after a 409 on POST
/api/teams, unlike the already-solved equivalent for service accounts
themselves (GET /api/service-accounts?name=).

Same route, extended the same way handleListServiceAccounts already
branches on ?name=: unset behaves exactly as before (the caller's own
teams via team_members); set looks up one team by exact name, open to
any authenticated caller -- not gated by isInstanceServiceAccount or
AdminOnly, since what it discloses (a name is taken, nothing about who's
in it) is the same low sensitivity that lookup already accepts for
service-account names.

Tests cover the exact motivating scenario (create, 409 on a retry,
recover the id via ?name=), the empty-array-not-an-error case, that no
role/source is reported for a non-member match, and that a caller who
isn't a member of the matched team still gets it.
2026-10-01 10:49:11 +02:00
Niklas Ye 871274a3a0 Request: team lookup for service accounts (TEAM-LOOKUP.md)
Raised by terdut-operator's TerdutTeam controller (ROADMAP.md Stage 2):
an instance-scoped service account has no way to recover a team's id
after a 409 on POST /api/teams, unlike the equivalent, already-solved
case for service accounts themselves (GET /api/service-accounts?name=).
Also corrects a claim in SERVICE-ACCOUNTS.md's own text that doesn't
match AdminOnly's actual code -- confirmed against source, not assumed.
2026-10-01 10:43:09 +02:00
18 changed files with 790 additions and 87 deletions
+1 -1
View File
@@ -793,7 +793,7 @@ attempted.
| `POST` | `/api/oidc/device/token` | `{"device_code"}` → `202 {"status":"pending"}`, then `200` with the session cookie once approved (once only). `410` with `{"error":"expired"}` or `{"error":"denied"}`; `429 {"error":"slow_down"}` if polled faster than `interval` |
| `POST` | `/api/oidc/device/approve` | **session** — `{"user_code"}`. Approves a pending device login as the caller. `403` for an API key; `404` for an unknown, expired or already decided code |
| `POST` | `/api/oidc/device/deny` | **session** — `{"user_code"}`. Refuses it |
| `GET` | `/api/oidc/callback` | Where the provider sends the browser back. Sets the session cookie and redirects to `/`, or to `/?sso_error=<code>` — one of `denied`, `expired`, `failed`, `unavailable`, `not_allowed`, `no_email`, `email_conflict`, `disabled` |
| `GET` | `/api/oidc/callback` | Where the provider sends the browser back. Sets the session cookie and redirects to `/`, or to `/?sso_error=<code>` — one of `denied`, `expired`, `failed`, `unavailable`, `not_allowed`, `no_email`, `email_conflict`, `disabled`, `not_bootstrapped` (no user exists on this install yet — sign in again once something has called `/api/bootstrap`) |
| `POST` | `/api/logout` | Ends the session and clears the cookie |
| `GET` | `/api/me` | The caller: `{user, has_password}` |
+108 -31
View File
@@ -119,38 +119,100 @@ new key, revoke the old one," not "recreate the account."
### Auth middleware
`internal/api/middleware.go`'s existing dual resolution (`Authorization: Bearer`
→ `apiKeyUser()`, or session cookie → `sessionUser()`, both landing on the same
`models.User` + team-membership context) gains a third path: a bearer token that
hashes to a `service_account_keys.key_hash` resolves to a distinct principal
type, not a synthesized `models.User`. `requireTeamMember`/`requireTeamOwner`
treat a matching team-scoped service account as owner-equivalent for that one
team (satisfies the same checks a real team owner would), and an instance-scoped
one as satisfying `AdminOnly` for team-creation/listing purposes **and** for
minting a `team`-scoped service account against any team (`POST
/api/service-accounts {"scope":"team","teamID":...}`) — this second permission
is what lets an operator-style caller create a team, then immediately mint that
team its own narrower credential, without a human in the loop for every team.
Neither permission extends to user-management endpoints (`POST /api/users`,
`PUT /api/users/{id}/admin`, etc.), which stay human-admin-only. Anywhere
identity is recorded for a human (incident timeline
`acknowledged_by`/`assigned_to`, audit-relevant fields), a service-account
principal is stored and displayed distinctly, e.g. `service-account:terdut-operator`,
never coerced into a `user_id` FK.
**Revised** (this section originally described an aspiration that didn't
match what shipped — `TEAM-LOOKUP.md` already caught one instance of that,
and a fuller audit found three more; this is the corrected, as-built
description, not the original proposal).
**Team scope, as implemented, is owner-equivalent for every `requireTeamOwner`
endpoint, membership and invites included — nothing server-side carves those
two out.** That's broader than what `terdut-operator`'s CRDs actually need
(escalation/deadman/integrations/OIDC-bindings only; membership is explicitly
never gitops-managed, see its DESIGN.md §4.2), a gap acknowledged rather than
closed here: narrowing this to exclude
`POST/DELETE /api/teams/{teamID}/members*` and
`.../invites*` specifically for a service-account caller is a small, isolated
follow-up (special-case those handlers rather than `requireTeamOwner` itself,
which every other owner-gated endpoint still wants shared). Until then, what
actually keeps membership out of automation's hands is that no operator built
against this scope should ever call those two endpoints — not a server-side
refusal.
`internal/api/middleware.go`'s dual resolution (`Authorization: Bearer` →
`apiKeyUser()`, or session cookie → `sessionUser()`) and the service-account
path (`serviceAccountFor()`) both resolve into one `Caller` type
(`internal/api/caller.go`), not two parallel, un-unified context
representations the way an earlier version of this server kept them. Every
authorization predicate reads `Caller`'s methods:
- `Caller.IsAdmin()` — true **only** for a human system administrator, never
for a service account of either scope, under any circumstance. `AdminOnly`
and `requireSelfOrAdmin` key on this alone — user management
(`POST /api/users`, `PUT /api/users/{id}/admin`, etc.) and
`GET/PUT /api/admin/settings` stay human-only, forever. The original text
here claimed an instance-scoped service account satisfies `AdminOnly` "for
team-creation/listing purposes" — that was never true of the shipped code
(`TEAM-LOOKUP.md` caught the listing half; the creation half was always a
separate, bespoke check in `handleCreateTeam`, not `AdminOnly` itself) and
is not being made true now. Don't widen `AdminOnly`: every time this has
come up, the fix has been a narrower, purpose-built capability instead
(`?name=` lookups for teams and service accounts; now
`terdut-operator`'s own invite-minting feature for the one real gap this
boundary left — how a human ever gets a first login on a no-OIDC,
operator-managed install. See the bottom of "What this unblocks.")
- `Caller.IsInstanceServiceAccount()` — true only for an instance-scoped
service account, never for a human (including a human admin).
`handleCreateTeam` uses exactly this: a human creates a team by being a
human (and becomes its owner); an instance-scoped service account creates
one with no human owner at all. The two paths are not interchangeable, so
this predicate deliberately does not also admit a human admin.
- `Caller.Role(teamID)`/`TeamIDs()` — a human's real `team_members` rows, or
a team-scoped service account's single synthetic owner membership
(`serveAsServiceAccount`). This is what makes `requireTeamMember`/
`requireTeamOwner` treat a team-scoped service account as owner-equivalent
for that one team, with no separate branch needed in either function.
- `Caller.ServiceAccountID()` — used by `OperatorModeBlock` ("any service
account passes") and by `callerMayManageServiceAccount`'s self-rotation
check.
- `Caller.AsHuman()` — the accessor every handler that needs a real
`user_id` to act on behalf of must call and check, instead of reading a
user off context unconditionally. Before the `Caller` type existed, four
handlers did the latter and silently misbehaved for a service-account
caller: `handleMe` and `handleTestNotification` 500'd (a zero-value user id
that matches no row), `handleDismissOnboarding` silently no-op'd (`UPDATE
... WHERE id = 0` affects nothing, still returns 204), and
`handleCreateInvite` wrote that same zero value into `invites.created_by`
— a real foreign-key violation, not just a wrong answer, since that column
is nullable but was never passed as `nil`. All four now call `AsHuman()`
and return an explicit 403 ("this endpoint is for human accounts only")
or, for the invite case, leave `created_by` `NULL` the same way
`handleCreateServiceAccount` already did for the analogous situation.
**Team scope is owner-equivalent for every `requireTeamOwner` endpoint,
membership and invites included — by design, not by an unclosed gap.** An
earlier version of this document flagged this as "acknowledged rather than
closed," kept in check only by the social convention that nobody *builds*
automation against those two routes. That convention is retired:
`terdut-operator`'s `TerdutTeam` controller now mints and revokes its own
team's invite link through exactly this capability (its existing
team-scoped credential, `POST`/`DELETE /api/teams/{teamID}/invites`), which
is the real fix for the human-onboarding gap below — not a narrower
carve-out of this capability. `service_accounts_test.go`'s
`TestServiceAccount_TeamScopeManagesItsOwnInvites` pins it.
**A team-scoped account can also mint another service account scoped to its
own team** (`handleCreateServiceAccount`'s `callerOwnsTeam` branch, which a
team-scoped caller already satisfies for its own team via the synthetic
membership above). Kept, not restricted, for the same reason: a team-scoped
credential is that team's owner's reach, full stop — carving this one
capability out while leaving membership/invites alone would be an arbitrary
asymmetry. Pinned by
`TestServiceAccount_TeamScopeCanMintAnotherAccountForItsOwnTeam`.
**`callerMayManageServiceAccount` gained the one load-bearing fix this
redesign exists for:** an instance-scoped service account may manage
(mint/revoke a key on) *any* team-scoped account, not only one admin, that
team's human owner, or the account itself. `handleCreateServiceAccount`
already let an instance-scoped caller *create* a team-scoped account for
any team; this closes the gap where adopting or rotating one it didn't just
create in the same call — exactly `terdut-operator`'s documented
adopt-on-409 crash-window recovery (its own `DESIGN.md` §5) — 403'd forever
instead of succeeding (`terdut-operator#3`). Pinned by
`TestServiceAccount_InstanceScopeAdoptsAnExistingTeamScopedAccountsKey`.
Anywhere identity is recorded for a human (incident timeline
`acknowledged_by`/`assigned_to`, audit-relevant fields), a service-account
caller is still coerced into a bare `user_id` of `0` today — `Caller`'s new
`Identity()` accessor exists for exactly this follow-up, but wiring it in
needs a schema migration (an actor-attribution column distinct from
`user_id`) and is deliberately out of scope here. Tracked separately, not by
this document.
## What this unblocks
@@ -177,6 +239,21 @@ Directly resolves `terdut-operator` DESIGN.md §6's two broken assumptions:
holding many credentials in one place (the operator's namespace) an
acceptable trade rather than reintroducing the mirrored design's
server-admin-equivalent-everywhere problem.
4. **A human can get a first login on a no-OIDC, operator-managed install —
without ever touching `AdminOnly` or `/api/admin/settings`.** This was
filed as `terdut-server#23` ("no API path to create a human login after
bootstrap") and diagnosed, at the time, as this server needing to let a
service account through `AdminOnly`. It doesn't: the fix lives entirely
in `terdut-operator`, because a team-scoped credential was *already*
owner-equivalent for `POST /api/teams/{teamID}/invites`, and invite
redemption (`POST /api/signup` with an `invite` token) bypasses
`signup_mode` entirely — `terdut-operator` just never grew a feature to
use either fact. Its `TerdutTeam` controller now mints and surfaces one
via its own existing team-scoped credential (`spec.invite`,
`status.inviteSecretRef`, see that repo's own docs), so a human joins a
CRD-managed team by a real invite link, the same way anyone else would.
`terdut-server#23` is closed with this note once that feature ships — its
named routes stay human-only, correctly, not a gap.
## Suggested sequencing
+73
View File
@@ -0,0 +1,73 @@
# Team lookup for service accounts: closing terdut-operator's create-path crash window
This is a design note for a feature, not an implementation plan — same posture as
`SERVICE-ACCOUNTS.md`, and raised for the same reason: `terdut-operator`'s `TerdutTeam`
controller (ROADMAP.md Stage 2, a separate repo, no shared code) hit a gap this server has
no answer for yet.
## The problem
`POST /api/teams` (`handleCreateTeam`, confirmed against `internal/api/teams.go`) lets an
instance-scoped service account create a team — it has its own explicit
`isInstanceServiceAccount(...)` branch alongside the human-user path, not gated by
`AdminOnly`. If that call succeeds server-side but the caller (`TerdutTeam`'s controller)
crashes before persisting the resulting team ID locally, a retry's `POST` 409s on the name's
unique constraint (confirmed: the `isUniqueViolation` branch in the same handler).
Recovering from that 409 means looking the team up by name, and nothing today permits that
for a service account:
- `GET /api/teams` (`handleListTeams`) answers "what teams does the *caller* belong to", via
a `team_members` join keyed on `userFromContext`'s `caller.ID` — confirmed against source.
A service account is never a member of anything, so this always returns empty for one,
regardless of what exists.
- `GET /api/admin/teams` (`handleAdminListTeams`) is gated by `AdminOnly`, and `AdminOnly`'s
actual code (`internal/api/middleware.go`) checks only `userFromContext(...).IsAdmin` — no
branch for a service account at all, confirmed against source. This contradicts
`SERVICE-ACCOUNTS.md`'s own text, which claims "an instance-scoped [service account
satisfies] `AdminOnly` for team-creation/listing purposes" — that claim doesn't match this
endpoint's actual, shipped code. (Team *creation* is fine: `handleCreateTeam` isn't behind
`AdminOnly` at all, it has its own check. Only the listing half of that sentence is wrong.)
This is exactly the shape of gap `SERVICE-ACCOUNTS.md`'s own `GET /api/service-accounts?name=`
closed for service accounts themselves (confirmed: that endpoint's own comment —
"the name lookup is open to any authenticated caller... what lets a service account find its
own account on the 403 that follows a second POST"). Teams never got the equivalent, because
nothing needed it until an operator started creating them unattended.
## Goals
- A service-account-accessible way to look up one team by exact name, mirroring
`GET /api/service-accounts?name=` as closely as possible — same shape, same reasoning,
same low sensitivity of what it discloses.
- No change to today's behavior for an empty/no-name request.
## Proposed shape
Extend `GET /api/teams` itself, the same way `handleListServiceAccounts` already branches on
a `?name=` query param, rather than adding a new route:
- `name` unset (today's behavior, unchanged): the caller's own teams, via `team_members`.
- `name=<value>` set: look up that one team by exact name — a one-or-zero-length array, not
an error on no match, mirroring `GET /api/service-accounts?name=`'s own response shape and
status codes exactly. Deliberately **not** gated by `isInstanceServiceAccount` or
`AdminOnly`: a human caller who's already a member sees this same information in their own
team list regardless, and a non-member learning only that a name is taken — not who's in
the team, not any of its data — is the same low-sensitivity disclosure
`GET /api/service-accounts?name=` already accepts for service-account names.
## What this unblocks
Directly resolves the crash-window gap in `terdut-operator`'s `TerdutTeam` controller: on a
409 from `POST /api/teams`, `GET /api/teams?name=<the same name>` — authenticated with the
same instance-scoped credential that just got the 409 — finds the id, and the controller
proceeds as if its own create had returned it directly. The same adopt-on-409 pattern already
proven for service accounts (that repo's `DESIGN.md` §6 point 1, §5's general rule), not a
new one.
## Suggested sequencing
Land this before `TerdutTeam`'s create path is implemented — the same reasoning
`SERVICE-ACCOUNTS.md` gave for its own sequencing: writing that code against today's gap as a
"known-temporary workaround" is wasted effort when the fix is this small and this
well-precedented.
+2 -2
View File
@@ -15,5 +15,5 @@ type: application
# appVersion and image.tag in values.yaml no longer agree, and that is not an oversight:
# image.tag stays "latest", which is what a local install actually pulls. appVersion is
# metadata and drives nothing.
version: 0.33.0
appVersion: "v0.33.0"
version: 0.34.0
appVersion: "v0.34.0"
@@ -21,6 +21,23 @@ spec:
{{- include "terdut-server.selectorLabels" . | nindent 8 }}
spec:
enableServiceLinks: false
{{- if .Values.database.waitForPostgres.enabled }}
initContainers:
- name: wait-for-postgres
image: "{{ .Values.database.waitForPostgres.image.repository }}:{{ .Values.database.waitForPostgres.image.tag }}"
imagePullPolicy: {{ .Values.database.waitForPostgres.image.pullPolicy }}
env:
- name: TERDUT_DB_DSN
value: {{ required "database.dsn is required" .Values.database.dsn | quote }}
command:
- sh
- -c
- |
until pg_isready -d "$TERDUT_DB_DSN"; do
echo "wait-for-postgres: not ready yet, retrying in 2s"
sleep 2
done
{{- end }}
containers:
- name: terdut-server
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
+20
View File
@@ -33,6 +33,26 @@ database:
passwordSecret:
name: ""
key: password
# Blocks the main container from starting until Postgres accepts
# connections. Without this, a Deployment created before Postgres has
# finished its very first boot -- initdb plus Patroni leader election, on a
# from-scratch postgres-operator cluster -- crash-loops a few times: the
# app's own ping-retry budget on startup (pingAttempts/pingRetryDelay in
# internal/db/db.go) is sized for a much shorter, different race --
# NetworkPolicy propagation, a few seconds -- not for genuine first-time
# cluster creation, which routinely takes longer, so it exhausts and the
# process exits before ever binding its HTTP port. A startupProbe cannot
# help here: the crash happens before there is anything to probe.
#
# pg_isready needs no credentials -- it reports PQPING_OK on anything that
# amounts to "a Postgres backend answered", including an auth challenge --
# so no PGPASSWORD is wired into this container.
waitForPostgres:
enabled: true
image:
repository: postgres
tag: "17-alpine"
pullPolicy: IfNotPresent
service:
type: ClusterIP
+5 -1
View File
@@ -279,7 +279,11 @@ type meResponse struct {
// between the login form and the app, since it cannot read its own cookie.
func handleMe(db *sql.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
caller, _ := userFromContext(r.Context())
caller, ok := userFromContext(r.Context())
if !ok {
respond(w, http.StatusForbidden, errResp("this endpoint is for human accounts only"))
return
}
user, err := fetchUser(r.Context(), db, caller.ID)
if err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
+123
View File
@@ -0,0 +1,123 @@
package api
import (
"context"
"fmt"
"git.ryuvia.com/niklas/terdut-server/internal/models"
)
// Caller is the one principal type every authorization predicate in this
// package reads from. Before this, a human (ctxUser + ctxTeams) and a
// service account (ctxServiceAccount + a synthetic ctxTeams entry) were two
// parallel, un-unified context representations — every predicate had to
// remember which one(s) it needed to check, and the ones that forgot either
// 403'd a service account that should have been let through (terdut-server#23,
// terdut-operator#3), crashed on an unchecked zero-value user ID (handleMe,
// handleTestNotification), or silently no-op'd (handleDismissOnboarding).
// serveAs and serveAsServiceAccount now both build exactly one Caller and
// store it under one context key; everything else in this file is a read
// of one of its methods.
type Caller struct {
// user is set for a human caller (session cookie or a user's own API
// key), nil for a service account of either scope.
user *models.User
// sa is set for a service-account caller, nil for a human.
sa *serviceAccountPrincipal
// memberships is the caller's real team_members rows for a human, or —
// for a team-scoped service account — the single synthetic owner
// membership serveAsServiceAccount injects (see its own comment for
// why). Always nil for an instance-scoped service account: it acts on
// teams by id, not by belonging to one.
memberships []membership
}
// AsHuman returns the real user behind this caller, or false for a service
// account of either scope. Every handler that needs a real user_id to act
// on behalf of — not just "is this caller sufficiently privileged" — calls
// this and handles the false case explicitly, replacing the unchecked
// userFromContext(ctx) zero-value reads that used to silently misbehave for
// a service-account caller.
func (c Caller) AsHuman() (models.User, bool) {
if c.user == nil {
return models.User{}, false
}
return *c.user, true
}
// IsAdmin is true only for a human system administrator — never for a
// service account, of either scope, under any circumstance. AdminOnly and
// requireSelfOrAdmin key on this and nothing else: user management and
// /api/admin/settings stay human-only forever, by design (SERVICE-ACCOUNTS.md).
func (c Caller) IsAdmin() bool {
return c.user != nil && c.user.IsAdmin
}
// IsInstanceServiceAccount reports whether this caller is specifically an
// instance-scoped service account — never true for a human, including a
// human admin. handleCreateTeam needs exactly this: a human creates a team
// by being a human (and becomes its owner as a side effect), an
// instance-scoped service account creates one with no human owner at all;
// the two paths are not interchangeable, so this predicate must not also
// admit a human admin the way MayActAsInstanceAdmin deliberately does.
func (c Caller) IsInstanceServiceAccount() bool {
return c.sa != nil && c.sa.scope == models.ServiceAccountScopeInstance
}
// Role reports the caller's role in teamID, and whether they belong to it
// at all.
func (c Caller) Role(teamID int64) (string, bool) {
for _, m := range c.memberships {
if m.teamID == teamID {
return m.role, true
}
}
return "", false
}
// TeamIDs lists every team this caller belongs to: a human's real
// memberships, or a team-scoped service account's own single team. Always
// empty for an instance-scoped service account.
func (c Caller) TeamIDs() []int64 {
ids := make([]int64, 0, len(c.memberships))
for _, m := range c.memberships {
ids = append(ids, m.teamID)
}
return ids
}
// ServiceAccountID reports this caller's own service-account id, for the
// "may manage/rotate its own credential" self-check in
// callerMayManageServiceAccount, and for OperatorModeBlock's "any service
// account passes" rule.
func (c Caller) ServiceAccountID() (int64, bool) {
if c.sa == nil {
return 0, false
}
return c.sa.id, true
}
// Identity is a stable, log/audit-facing string distinguishing a human
// caller from a service account — "user:42" or "service-account:7". Not
// wired into any database column today (incidents.go's acknowledged_by/
// assigned_to/user_id are explicitly out of scope for this change — that
// needs its own schema migration, tracked separately), but this is the one
// place in the request path that already knows which kind of caller this
// is, and that follow-up will want exactly this accessor.
func (c Caller) Identity() string {
switch {
case c.user != nil:
return fmt.Sprintf("user:%d", c.user.ID)
case c.sa != nil:
return fmt.Sprintf("service-account:%d", c.sa.id)
default:
return "unknown"
}
}
func callerFromContext(ctx context.Context) (Caller, bool) {
c, ok := ctx.Value(ctxCaller).(Caller)
return c, ok
}
+29 -36
View File
@@ -16,10 +16,13 @@ import (
type contextKey string
const (
ctxUser contextKey = "user"
ctxSession contextKey = "session"
ctxTeams contextKey = "teams"
ctxServiceAccount contextKey = "service_account"
// ctxCaller holds the one Caller (see caller.go) every authorization
// predicate in this package reads from — a human and a service account
// used to be two parallel, un-unified context keys (ctxUser/ctxTeams vs.
// ctxServiceAccount); this is why that was a mistake, not a smaller
// version of the same idea.
ctxCaller contextKey = "caller"
ctxSession contextKey = "session"
)
// AuthMiddleware accepts either of the two credentials the server issues: an
@@ -179,8 +182,7 @@ func serveAs(w http.ResponseWriter, r *http.Request, next http.Handler, db *sql.
return
}
ctx := context.WithValue(r.Context(), ctxTeams, teams)
ctx = context.WithValue(ctx, ctxUser, u)
ctx := context.WithValue(r.Context(), ctxCaller, Caller{user: &u, memberships: teams})
if sessionID != 0 {
ctx = context.WithValue(ctx, ctxSession, sessionID)
}
@@ -192,9 +194,13 @@ func hashToken(token string) string {
return hex.EncodeToString(h[:])
}
// userFromContext is a thin compatibility wrapper over Caller.AsHuman(), so
// every call site written before the Caller abstraction (alerts.go,
// incidents.go, schedule.go, stats.go, and more) needs no change and keeps
// its exact existing behavior.
func userFromContext(ctx context.Context) (models.User, bool) {
u, ok := ctx.Value(ctxUser).(models.User)
return u, ok
c, _ := callerFromContext(ctx)
return c.AsHuman()
}
// serviceAccountPrincipal is a service account as resolved from its key:
@@ -244,26 +250,21 @@ func serviceAccountFor(ctx context.Context, db *sql.DB, token string) (serviceAc
// key is only ever set by the client that holds it, never attached by a
// browser to a request another site makes.
func serveAsServiceAccount(w http.ResponseWriter, r *http.Request, next http.Handler, sa serviceAccountPrincipal) {
ctx := r.Context()
var memberships []membership
if sa.scope == models.ServiceAccountScopeTeam {
ctx = context.WithValue(ctx, ctxTeams, []membership{{teamID: sa.teamID, role: models.RoleOwner}})
memberships = []membership{{teamID: sa.teamID, role: models.RoleOwner}}
}
ctx = context.WithValue(ctx, ctxServiceAccount, sa)
ctx := context.WithValue(r.Context(), ctxCaller, Caller{sa: &sa, memberships: memberships})
next.ServeHTTP(w, r.WithContext(ctx))
}
func serviceAccountFromContext(ctx context.Context) (serviceAccountPrincipal, bool) {
sa, ok := ctx.Value(ctxServiceAccount).(serviceAccountPrincipal)
return sa, ok
}
// isInstanceServiceAccount reports whether the caller is an instance-scoped
// service account — the one identity allowed to create a team and mint a
// team-scoped account against any of them, the two things system
// administration can already do that this extends to automation.
// isInstanceServiceAccount is a thin compatibility wrapper over
// Caller.IsInstanceServiceAccount(), for call sites outside this package's
// core predicates (handleCreateTeam, handleCreateServiceAccount) that
// needed this exact, narrow check before the Caller abstraction existed.
func isInstanceServiceAccount(ctx context.Context) bool {
sa, ok := serviceAccountFromContext(ctx)
return ok && sa.scope == models.ServiceAccountScopeInstance
c, _ := callerFromContext(ctx)
return c.IsInstanceServiceAccount()
}
// operatorReason marks a write that operator mode refused as such, distinct
@@ -291,7 +292,8 @@ func OperatorModeBlock(cfg config.Config) func(http.Handler) http.Handler {
return next
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if _, ok := serviceAccountFromContext(r.Context()); ok {
caller, _ := callerFromContext(r.Context())
if _, ok := caller.ServiceAccountID(); ok {
next.ServeHTTP(w, r)
return
}
@@ -333,24 +335,15 @@ func callerMemberships(ctx context.Context, db *sql.DB, userID int64) ([]members
// administration is about accounts, not about reading other people's incidents,
// and an admin who needs to see a team's queue can add themselves to it.
func callerTeamIDs(ctx context.Context) []int64 {
ms, _ := ctx.Value(ctxTeams).([]membership)
ids := make([]int64, 0, len(ms))
for _, m := range ms {
ids = append(ids, m.teamID)
}
return ids
c, _ := callerFromContext(ctx)
return c.TeamIDs()
}
// callerRole reports the caller's role in one team, and whether they are in it
// at all.
func callerRole(ctx context.Context, teamID int64) (string, bool) {
ms, _ := ctx.Value(ctxTeams).([]membership)
for _, m := range ms {
if m.teamID == teamID {
return m.role, true
}
}
return "", false
c, _ := callerFromContext(ctx)
return c.Role(teamID)
}
// requireTeamMember answers the request and reports false unless the caller
+25
View File
@@ -48,6 +48,17 @@ const (
ssoNoEmail ssoError = "no_email" // the provider sent no email address
ssoEmailConflict ssoError = "email_conflict" // a local account has this email and cannot be linked
ssoDisabled ssoError = "disabled" // the linked account is disabled
// ssoNotBootstrapped: this identity has no existing account, and no user
// exists on this install yet either -- creating one here would race
// POST /api/bootstrap for the one gitops-managed installs expect to win
// it (terdut-operator's own DESIGN.md §1, §6), which has no way to
// recover if it loses. The person sees this for at most as long as it
// takes whatever is bootstrapping this install to finish; signing in
// again afterward hits the ordinary first-sign-in path. Found by
// terdut-operator#1: nothing stopped an otherwise-ordinary OIDC sign-in
// from quietly winning this race against an operator that assumed it
// was the only caller.
ssoNotBootstrapped ssoError = "not_bootstrapped"
)
// handleAuthConfig says how this server can be signed in to, so the login form
@@ -366,6 +377,20 @@ func resolveSSOUser(ctx context.Context, tx *sql.Tx, cfg config.OIDC, id *oidc.I
return 0, ssoEmailConflict
}
case errors.Is(err, sql.ErrNoRows):
// Creating the very first user is /api/bootstrap's own job (same
// gate, same table: SELECT COUNT(*) FROM users in handleBootstrap).
// An identity nobody has linked yet, on an install with no users at
// all, is exactly the race terdut-operator#1 found: whoever gets
// here first wins a slot the other side has no way to recover from
// losing. Refusing it here costs an otherwise-ordinary sign-in
// nothing but a retry once bootstrap has actually run.
var userCount int
if err := tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM users").Scan(&userCount); err != nil {
return 0, err
}
if userCount == 0 {
return 0, ssoNotBootstrapped
}
userID, err = createSSOUser(ctx, tx, id)
if err != nil {
return 0, err
+39
View File
@@ -543,6 +543,45 @@ func TestSSO_DisabledUserIsRefused(t *testing.T) {
}
}
// TestSSO_FirstUserIsRefusedUntilBootstrap is terdut-operator#1: an
// otherwise-ordinary OIDC sign-in against a brand-new, not-yet-bootstrapped
// install must not be allowed to create the first user and win the race
// POST /api/bootstrap expects to win uncontested. Built directly over
// api.NewRouter rather than newSSOTS/newTS, both of which bootstrap before
// a test body ever runs -- exactly the state this test needs to not have yet.
func TestSSO_FirstUserIsRefusedUntilBootstrap(t *testing.T) {
idp := newFakeIdP(t)
database := newTestDB(t)
srv := httptest.NewServer(api.NewRouter(database, api.NotifyConfig{PublicURL: "http://terdut.test"}, ssoConfig(idp), "test"))
t.Cleanup(srv.Close)
first := newBrowser(t, srv.URL)
first.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }
if loc := signInSSO(t, idp, first, alice); loc != "/?sso_error=not_bootstrapped" {
t.Fatalf("sent to %q, want not_bootstrapped", loc)
}
// Bootstrap the install for real, the way terdut-operator's own
// reconcileBootstrap does.
resp, err := http.Post(srv.URL+"/api/bootstrap", "application/json",
strings.NewReader(`{"username":"admin","email":"admin@test.com"}`))
if err != nil {
t.Fatal(err)
}
resp.Body.Close()
if resp.StatusCode != http.StatusCreated {
t.Fatalf("bootstrap: %d", resp.StatusCode)
}
// The same identity, signing in again, is this install's ordinary first
// SSO user now -- no longer refused.
second := newBrowser(t, srv.URL)
second.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }
if loc := signInSSO(t, idp, second, alice); loc != "/" {
t.Errorf("sent to %q after bootstrap, want success", loc)
}
}
func TestSSO_NoEmailIsRefused(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
+31 -8
View File
@@ -41,10 +41,14 @@ func callerIsAdmin(ctx context.Context) bool {
return ok && u.IsAdmin
}
// callerOwnsTeam reports whether the caller is a human owner of teamID. Built
// on callerRole/ctxTeams like requireTeamOwner, but without writing a
// response: callers here need to combine it with other ways of being
// allowed, not stop at the first no.
// callerOwnsTeam reports whether the caller is owner-equivalent for teamID:
// a human owner, or that team's own team-scoped service account (its single
// synthetic membership, serveAsServiceAccount — ratified in
// SERVICE-ACCOUNTS.md as intentional, not an accident: a team-scoped
// credential is that team's owner's reach, full stop, membership and
// invites included). Built on callerRole like requireTeamOwner, but without
// writing a response: callers here need to combine it with other ways of
// being allowed, not stop at the first no.
func callerOwnsTeam(ctx context.Context, teamID int64) bool {
role, ok := callerRole(ctx, teamID)
return ok && role == models.RoleOwner
@@ -173,9 +177,24 @@ func fetchServiceAccount(ctx context.Context, db *sql.DB, id int64) (models.Serv
// callerMayManageServiceAccount reports whether the caller may mint or revoke
// a key on sa: a system administrator, that team-scoped account's own human
// owner, or the account rotating its own credential — which is not a
// privilege escalation, the same reasoning requireSelfOrAdmin already rests
// on for a user's own API keys.
// owner, the account rotating its own credential (not a privilege
// escalation, the same reasoning requireSelfOrAdmin already rests on for a
// user's own API keys) — or, new, an instance-scoped service account
// managing any team-scoped account.
//
// That last branch closes terdut-operator#3: handleCreateServiceAccount
// already lets an instance-scoped caller *create* a team-scoped account for
// any team (the branch below it, isInstanceServiceAccount(ctx)) — this
// account didn't have an equivalent reach to *adopt or rotate* one it
// didn't just create in the same call, which is exactly the recovery path
// terdut-operator's own documented crash-window handling depends on
// (DESIGN.md §5's general adopt-on-conflict rule): a reconcile that creates
// the account successfully but crashes before persisting its credential
// locally retries into a 409, and without this branch the only available
// recovery — minting a fresh key on the now-existing account — 403'd
// forever, with no way out. Granting it here is not a new power: it
// mirrors the create-time reach this scope already has, just extended to
// the retry path DESIGN.md's own crash-window reasoning requires.
func callerMayManageServiceAccount(ctx context.Context, sa models.ServiceAccount) bool {
if callerIsAdmin(ctx) {
return true
@@ -183,7 +202,11 @@ func callerMayManageServiceAccount(ctx context.Context, sa models.ServiceAccount
if sa.TeamID != nil && callerOwnsTeam(ctx, *sa.TeamID) {
return true
}
if self, ok := serviceAccountFromContext(ctx); ok && self.id == sa.ID {
caller, _ := callerFromContext(ctx)
if id, ok := caller.ServiceAccountID(); ok && id == sa.ID {
return true
}
if sa.TeamID != nil && caller.IsInstanceServiceAccount() {
return true
}
return false
+126
View File
@@ -181,10 +181,136 @@ func TestServiceAccount_TeamScopeManagesItsResources(t *testing.T) {
resp2.Body.Close()
}
// Documents the capability already granted at create time (handleCreateServiceAccount's
// own callerOwnsTeam branch) also applies here: a team-scoped account is that
// team's owner's reach, membership and further accounts included, not just
// the handful of endpoints exercised above. Kept, not restricted, for
// symmetry with the now-ratified membership/invite capability below.
func TestServiceAccount_TeamScopeCanMintAnotherAccountForItsOwnTeam(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamA := createTeamAs(t, s, instanceKey, "team-a")
keyA := createServiceAccount(t, s, instanceKey, "team-a-sa", models.ServiceAccountScopeTeam, teamA)
resp := s.reqAs(t, keyA, http.MethodPost, "/api/service-accounts",
map[string]any{"name": "team-a-sa-2", "scope": models.ServiceAccountScopeTeam, "team_id": teamA})
if resp.StatusCode != http.StatusCreated {
t.Errorf("team-scoped account minting another account for its own team: %d", resp.StatusCode)
}
resp.Body.Close()
}
// SERVICE-ACCOUNTS.md ratifies this explicitly: a team-scoped account is
// owner-equivalent for every requireTeamOwner endpoint, membership and
// invites included — terdut-operator's own invite-minting feature depends on
// exactly this. No test exercised handleCreateInvite from a service account
// before this change, and it would have 500'd (created_by written as a bare
// zero value against a NOT-validated-but-FK'd column) rather than succeeded;
// see the signup_test.go addition for that half.
func TestServiceAccount_TeamScopeManagesItsOwnInvites(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamA := createTeamAs(t, s, instanceKey, "team-a")
keyA := createServiceAccount(t, s, instanceKey, "team-a-sa", models.ServiceAccountScopeTeam, teamA)
var invite struct {
ID int64 `json:"id"`
}
resp := s.reqAs(t, keyA, http.MethodPost, "/api/teams/"+id64(teamA)+"/invites", map[string]any{})
if resp.StatusCode != http.StatusCreated {
t.Fatalf("team-scoped account creating an invite: %d", resp.StatusCode)
}
decode(t, resp, &invite)
var list []map[string]any
decode(t, s.reqAs(t, keyA, http.MethodGet, "/api/teams/"+id64(teamA)+"/invites", nil), &list)
if len(list) != 1 {
t.Errorf("expected the invite to list back, got %d", len(list))
}
if resp := s.reqAs(t, keyA, http.MethodDelete,
"/api/teams/"+id64(teamA)+"/invites/"+id64(invite.ID), nil); resp.StatusCode != http.StatusNoContent {
t.Errorf("team-scoped account revoking its own invite: %d", resp.StatusCode)
} else {
resp.Body.Close()
}
}
// ---------------------------------------------------------------------------
// Key rotation
// ---------------------------------------------------------------------------
// terdut-operator#3: an instance-scoped account is already trusted to CREATE
// a team-scoped account for any team (handleCreateServiceAccount's own
// isInstanceServiceAccount branch) — this pins that it is equally trusted to
// manage/rotate a key on one that already exists and that it did not just
// create in this call, which is the exact shape of terdut-operator's own
// crash-window recovery (mint succeeds, a later step is interrupted before
// persisting the credential locally, and the next reconcile retries into a
// 409 then needs to mint a fresh key on the now-existing account). Before
// this fix, the second POST .../keys below 403'd forever.
func TestServiceAccount_InstanceScopeAdoptsAnExistingTeamScopedAccountsKey(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamA := createTeamAs(t, s, instanceKey, "team-a")
resp := s.reqAs(t, instanceKey, http.MethodPost, "/api/service-accounts",
map[string]any{"name": "team-a-sa", "scope": models.ServiceAccountScopeTeam, "team_id": teamA})
if resp.StatusCode != http.StatusCreated {
t.Fatalf("create team-scoped account: %d", resp.StatusCode)
}
var created struct {
ServiceAccount struct {
ID int64 `json:"id"`
} `json:"service_account"`
}
decode(t, resp, &created)
// Simulates the adopt-on-409 recovery path: this instance-scoped caller
// did not just create this account in this call (a fresh *tdclient.Client
// request, same as a second, independent reconcile would issue), yet
// still needs to mint it a fresh key.
rotateResp := s.reqAs(t, instanceKey, http.MethodPost,
"/api/service-accounts/"+id64(created.ServiceAccount.ID)+"/keys", map[string]string{"name": "adopted"})
if rotateResp.StatusCode != http.StatusCreated {
t.Errorf("instance-scoped account adopting a team-scoped account's key: %d", rotateResp.StatusCode)
}
rotateResp.Body.Close()
}
// ---------------------------------------------------------------------------
// AdminOnly / requireSelfOrAdmin — unchanged after the Caller refactor
// ---------------------------------------------------------------------------
// The Caller abstraction must not have widened AdminOnly/requireSelfOrAdmin:
// user management and /api/admin/settings stay human-only, for every scope
// of service account, exactly as before.
func TestAdminOnly_RefusesEveryServiceAccountScope(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamA := createTeamAs(t, s, instanceKey, "team-a")
teamKey := createServiceAccount(t, s, instanceKey, "team-a-sa", models.ServiceAccountScopeTeam, teamA)
for _, key := range []string{instanceKey, teamKey} {
if resp := s.reqAs(t, key, http.MethodGet, "/api/admin/settings", nil); resp.StatusCode != http.StatusForbidden {
t.Errorf("expected 403 for a service account reading /api/admin/settings, got %d", resp.StatusCode)
} else {
resp.Body.Close()
}
if resp := s.reqAs(t, key, http.MethodPost, "/api/users",
map[string]string{"username": "nope", "email": "nope@example.com"}); resp.StatusCode != http.StatusForbidden {
t.Errorf("expected 403 for a service account creating a user, got %d", resp.StatusCode)
} else {
resp.Body.Close()
}
if resp := s.reqAs(t, key, http.MethodGet, "/api/me", nil); resp.StatusCode != http.StatusForbidden {
t.Errorf("expected 403 for a service account calling /api/me, got %d", resp.StatusCode)
} else {
resp.Body.Close()
}
}
}
func TestServiceAccount_SelfRotatesItsOwnKey(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
+24 -4
View File
@@ -359,7 +359,19 @@ func handleCreateInvite(db *sql.DB, publicURL string) http.HandlerFunc {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
}
caller, _ := userFromContext(r.Context())
// created_by is nullable (ON DELETE SET NULL) for exactly this
// reason: the caller minting an invite is not always a human with a
// real users row. A team-scoped service account is owner-equivalent
// here (requireTeamOwner above already let it through), and this
// must leave created_by NULL for one the same way
// handleCreateServiceAccount already does for the analogous case —
// an unchecked zero value would violate the users(id) foreign key
// instead of recording "nobody" cleanly.
var createdBy *int64
if u, ok := userFromContext(r.Context()); ok {
id := u.ID
createdBy = &id
}
expires := time.Now().Add(inviteTTL)
var out inviteJSON
@@ -368,7 +380,7 @@ func handleCreateInvite(db *sql.DB, publicURL string) http.HandlerFunc {
INSERT INTO invites (token_hash, team_id, role, created_by, expires_at, max_uses)
VALUES ($1, $2, $3, $4, $5, $6)
RETURNING id, team_id, role, created_at, expires_at, max_uses, uses`,
hash, teamID, req.Role, caller.ID, expires.Unix(), req.MaxUses).
hash, teamID, req.Role, createdBy, expires.Unix(), req.MaxUses).
Scan(&out.ID, &out.TeamID, &out.Role, &created, &expiresAt, &out.MaxUses, &out.Uses); err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
@@ -429,7 +441,11 @@ func handleTestNotification(cfg NotifyConfig, db *sql.DB) http.HandlerFunc {
errResp("this server has no ntfy configured, so it can send nothing"))
return
}
caller, _ := userFromContext(r.Context())
caller, ok := userFromContext(r.Context())
if !ok {
respond(w, http.StatusForbidden, errResp("this endpoint is for human accounts only"))
return
}
var topic *string
if err := db.QueryRowContext(r.Context(),
@@ -470,7 +486,11 @@ func handleDismissOnboarding(db *sql.DB) http.HandlerFunc {
respond(w, http.StatusBadRequest, errResp("dismissed is required"))
return
}
caller, _ := userFromContext(r.Context())
caller, ok := userFromContext(r.Context())
if !ok {
respond(w, http.StatusForbidden, errResp("this endpoint is for human accounts only"))
return
}
var err error
if *req.Dismissed {
+56
View File
@@ -2,10 +2,14 @@ package api_test
import (
"bytes"
"database/sql"
"encoding/json"
"net/http"
"net/http/cookiejar"
"testing"
"git.ryuvia.com/niklas/terdut-server/internal/api"
"git.ryuvia.com/niklas/terdut-server/internal/models"
)
// signup posts to the unauthenticated sign-up endpoint, the way the form does,
@@ -268,3 +272,55 @@ func TestSignup_ValidatesLikeTheRestOfTheServer(t *testing.T) {
t.Errorf("an existing username: expected 409, got %d", taken.StatusCode)
}
}
// A team-scoped service account has no users row to attribute created_by to.
// Before this fix, handleCreateInvite wrote its zero-value caller.ID straight
// into that (nullable, ON DELETE SET NULL) foreign key instead of leaving it
// NULL the way handleCreateServiceAccount already does for the same
// situation — a 500, not the 201 TestServiceAccount_TeamScopeManagesItsOwnInvites
// now confirms. This pins the column itself ends up NULL, not just "some
// response came back".
func TestSignup_InviteCreatedByAServiceAccountLeavesCreatedByNull(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamA := createTeamAs(t, s, instanceKey, "team-a")
keyA := createServiceAccount(t, s, instanceKey, "team-a-sa", models.ServiceAccountScopeTeam, teamA)
var created struct {
ID int64 `json:"id"`
}
decode(t, s.reqAs(t, keyA, http.MethodPost, "/api/teams/"+id64(teamA)+"/invites", map[string]any{}), &created)
var createdBy sql.NullInt64
if err := s.db.QueryRow("SELECT created_by FROM invites WHERE id = $1", created.ID).Scan(&createdBy); err != nil {
t.Fatalf("read back invites.created_by: %v", err)
}
if createdBy.Valid {
t.Errorf("expected created_by to be NULL for a service-account-minted invite, got %d", createdBy.Int64)
}
}
// Neither of these has a real user_id to act on behalf of; both must 403 a
// service account explicitly rather than 500 (handleTestNotification, which
// used to query ntfy_topic for user id 0) or silently no-op (handleDismissOnboarding,
// which used to UPDATE ... WHERE id = 0, affecting nothing and still
// returning 204).
func TestServiceAccount_HumanOnlyEndpointsRefuseExplicitly(t *testing.T) {
// BaseURL set (even to a fake, unreachable address) so handleTestNotification
// reaches its AsHuman() check instead of short-circuiting on "ntfy not
// configured" first — this test is about the human-only check, not ntfy.
s := newTS(t, api.NotifyConfig{BaseURL: "http://ntfy.invalid"})
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
if resp := s.reqAs(t, instanceKey, http.MethodPost, "/api/me/notify/test", nil); resp.StatusCode != http.StatusForbidden {
t.Errorf("expected 403 for a service account testing notifications, got %d", resp.StatusCode)
} else {
resp.Body.Close()
}
if resp := s.reqAs(t, instanceKey, http.MethodPut, "/api/me/onboarding",
map[string]bool{"dismissed": true}); resp.StatusCode != http.StatusForbidden {
t.Errorf("expected 403 for a service account dismissing onboarding, got %d", resp.StatusCode)
} else {
resp.Body.Close()
}
}
+36 -4
View File
@@ -13,12 +13,19 @@ import (
"github.com/go-chi/chi/v5"
)
// handleListTeams lists the caller's own teams, each with their role in it. An
// administrator listing every team goes through the admin endpoint instead:
// this one answers "what am I part of", which is what the UI's team filter and
// the combined queue are built from.
// handleListTeams lists the caller's own teams, each with their role in it,
// or — with ?name= — looks up one team by exact name regardless of caller
// identity (TEAM-LOOKUP.md). An administrator listing every team goes
// through the admin endpoint instead: the no-name case here answers "what am
// I part of", which is what the UI's team filter and the combined queue are
// built from.
func handleListTeams(db *sql.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
if name := strings.TrimSpace(r.URL.Query().Get("name")); name != "" {
handleListTeamsByName(db, w, r, name)
return
}
caller, _ := userFromContext(r.Context())
rows, err := db.QueryContext(r.Context(), `
SELECT t.id, t.name, t.created_at, m.role, m.source
@@ -51,6 +58,31 @@ func handleListTeams(db *sql.DB) http.HandlerFunc {
}
}
// handleListTeamsByName answers "is there a team named exactly this", open to
// any authenticated caller including a service account (TEAM-LOOKUP.md) —
// mirrors handleListServiceAccounts' own ?name= lookup: a one-or-zero-length
// array, never an error on no match, and no caller-identity filtering at
// all, since what it discloses (a name is taken, nothing about who's in it
// or any of its data) is the same low sensitivity that lookup already
// accepts for service-account names.
func handleListTeamsByName(db *sql.DB, w http.ResponseWriter, r *http.Request, name string) {
var t models.Team
var created int64
err := db.QueryRowContext(r.Context(),
"SELECT id, name, created_at FROM teams WHERE name = $1", name,
).Scan(&t.ID, &t.Name, &created)
if errors.Is(err, sql.ErrNoRows) {
respond(w, http.StatusOK, []models.Team{})
return
}
if err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
}
t.CreatedAt = time.Unix(created, 0).UTC()
respond(w, http.StatusOK, []models.Team{t})
}
// handleUserTeams lists one user's teams, for the admin page's per-user view:
// "what is this person in", which /api/teams cannot answer because it is always
// about the caller.
+74
View File
@@ -6,6 +6,8 @@ import (
"io"
"net/http"
"testing"
"git.ryuvia.com/niklas/terdut-server/internal/models"
)
// The whole point of #4: two teams sharing one server must not see each other's
@@ -454,3 +456,75 @@ func TestTeams_OutsiderSeesNothing(t *testing.T) {
t.Errorf("blue's team list: %v", teams)
}
}
// ---------------------------------------------------------------------------
// GET /api/teams?name= (TEAM-LOOKUP.md)
// ---------------------------------------------------------------------------
func TestListTeamsByName_FindsExactMatch(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamID := createTeamAs(t, s, instanceKey, "platform")
teams := list(t, s.reqAs(t, instanceKey, http.MethodGet, "/api/teams?name=platform", nil))
if len(teams) != 1 {
t.Fatalf("expected exactly one match for ?name=platform, got %d: %v", len(teams), teams)
}
if int64(teams[0]["id"].(float64)) != teamID {
t.Errorf("id = %v, want %d", teams[0]["id"], teamID)
}
// No membership, so no role to report (models.Team's own doc comment:
// "empty when nobody in particular is asking").
if _, has := teams[0]["role"]; has {
t.Errorf("expected no role on a name-lookup match, got %v", teams[0]["role"])
}
}
func TestListTeamsByName_NoMatchIsAnEmptyArrayNotAnError(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
resp := s.reqAs(t, instanceKey, http.MethodGet, "/api/teams?name=does-not-exist", nil)
if resp.StatusCode != http.StatusOK {
t.Fatalf("expected 200 on no match, got %d", resp.StatusCode)
}
teams := list(t, resp)
if len(teams) != 0 {
t.Errorf("expected an empty array, got %v", teams)
}
}
// The actual motivating scenario (TEAM-LOOKUP.md): a service account that
// already created a team, interrupted before it could remember the id,
// recovers it via ?name= on the same name its own POST 409s on.
func TestListTeamsByName_RecoversAfterCreateConflict(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
original := createTeamAs(t, s, instanceKey, "recovered")
conflict := s.reqAs(t, instanceKey, http.MethodPost, "/api/teams", map[string]string{"name": "recovered"})
if conflict.StatusCode != http.StatusConflict {
t.Fatalf("expected 409 recreating the same name, got %d", conflict.StatusCode)
}
conflict.Body.Close()
teams := list(t, s.reqAs(t, instanceKey, http.MethodGet, "/api/teams?name=recovered", nil))
if len(teams) != 1 || int64(teams[0]["id"].(float64)) != original {
t.Fatalf("expected to recover the original team %d via ?name=, got %v", original, teams)
}
}
// Not gated by isInstanceServiceAccount or AdminOnly (TEAM-LOOKUP.md): any
// authenticated caller may ask whether a name is taken, the same low
// sensitivity GET /api/service-accounts?name= already accepts.
func TestListTeamsByName_OpenToAnyAuthenticatedCaller(t *testing.T) {
s := newTS(t)
red := newTeam(t, s, "red")
_ = createTeamAs(t, s, s.key, "blue-target")
// red's own member, not a member of "blue-target", still gets a match.
teams := list(t, red.call(http.MethodGet, "/api/teams?name=blue-target", nil))
if len(teams) != 1 || teams[0]["name"] != "blue-target" {
t.Errorf("expected a non-member caller to still find the team by name, got %v", teams)
}
}
+1
View File
@@ -355,6 +355,7 @@ function ssoErrorText(code, name) {
no_email: `${sso} did not send an email address for you, which terdut needs.`,
email_conflict: 'An account with your email address already exists and could not be linked to this sign-in. Ask an administrator.',
disabled: 'Your account is disabled. Ask an administrator.',
not_bootstrapped: 'This install is still setting up. Try again in a moment.',
}[code] || `Signing in with ${sso} failed.`;
}