Compare commits
9 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 4358e84b24 | |||
| f45dc2f925 | |||
| 774fdfcaa8 | |||
| fd26fef1ba | |||
| a9d788cc83 | |||
| 4b15079ac2 | |||
| fc9f47cc8d | |||
| bc9f793f1f | |||
| 871274a3a0 |
@@ -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
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
@@ -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 }}"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
@@ -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 {
|
||||
|
||||
@@ -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
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.`;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user