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/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/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 |
|
| `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 |
|
| `POST` | `/api/logout` | Ends the session and clears the cookie |
|
||||||
| `GET` | `/api/me` | The caller: `{user, has_password}` |
|
| `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
|
### Auth middleware
|
||||||
|
|
||||||
`internal/api/middleware.go`'s existing dual resolution (`Authorization: Bearer`
|
**Revised** (this section originally described an aspiration that didn't
|
||||||
→ `apiKeyUser()`, or session cookie → `sessionUser()`, both landing on the same
|
match what shipped — `TEAM-LOOKUP.md` already caught one instance of that,
|
||||||
`models.User` + team-membership context) gains a third path: a bearer token that
|
and a fuller audit found three more; this is the corrected, as-built
|
||||||
hashes to a `service_account_keys.key_hash` resolves to a distinct principal
|
description, not the original proposal).
|
||||||
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.
|
|
||||||
|
|
||||||
**Team scope, as implemented, is owner-equivalent for every `requireTeamOwner`
|
`internal/api/middleware.go`'s dual resolution (`Authorization: Bearer` →
|
||||||
endpoint, membership and invites included — nothing server-side carves those
|
`apiKeyUser()`, or session cookie → `sessionUser()`) and the service-account
|
||||||
two out.** That's broader than what `terdut-operator`'s CRDs actually need
|
path (`serviceAccountFor()`) both resolve into one `Caller` type
|
||||||
(escalation/deadman/integrations/OIDC-bindings only; membership is explicitly
|
(`internal/api/caller.go`), not two parallel, un-unified context
|
||||||
never gitops-managed, see its DESIGN.md §4.2), a gap acknowledged rather than
|
representations the way an earlier version of this server kept them. Every
|
||||||
closed here: narrowing this to exclude
|
authorization predicate reads `Caller`'s methods:
|
||||||
`POST/DELETE /api/teams/{teamID}/members*` and
|
|
||||||
`.../invites*` specifically for a service-account caller is a small, isolated
|
- `Caller.IsAdmin()` — true **only** for a human system administrator, never
|
||||||
follow-up (special-case those handlers rather than `requireTeamOwner` itself,
|
for a service account of either scope, under any circumstance. `AdminOnly`
|
||||||
which every other owner-gated endpoint still wants shared). Until then, what
|
and `requireSelfOrAdmin` key on this alone — user management
|
||||||
actually keeps membership out of automation's hands is that no operator built
|
(`POST /api/users`, `PUT /api/users/{id}/admin`, etc.) and
|
||||||
against this scope should ever call those two endpoints — not a server-side
|
`GET/PUT /api/admin/settings` stay human-only, forever. The original text
|
||||||
refusal.
|
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
|
## 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
|
holding many credentials in one place (the operator's namespace) an
|
||||||
acceptable trade rather than reintroducing the mirrored design's
|
acceptable trade rather than reintroducing the mirrored design's
|
||||||
server-admin-equivalent-everywhere problem.
|
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
|
## 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:
|
# 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
|
# image.tag stays "latest", which is what a local install actually pulls. appVersion is
|
||||||
# metadata and drives nothing.
|
# metadata and drives nothing.
|
||||||
version: 0.33.0
|
version: 0.34.0
|
||||||
appVersion: "v0.33.0"
|
appVersion: "v0.34.0"
|
||||||
|
|||||||
@@ -21,6 +21,23 @@ spec:
|
|||||||
{{- include "terdut-server.selectorLabels" . | nindent 8 }}
|
{{- include "terdut-server.selectorLabels" . | nindent 8 }}
|
||||||
spec:
|
spec:
|
||||||
enableServiceLinks: false
|
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:
|
containers:
|
||||||
- name: terdut-server
|
- name: terdut-server
|
||||||
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
|
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
|
||||||
|
|||||||
@@ -33,6 +33,26 @@ database:
|
|||||||
passwordSecret:
|
passwordSecret:
|
||||||
name: ""
|
name: ""
|
||||||
key: password
|
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:
|
service:
|
||||||
type: ClusterIP
|
type: ClusterIP
|
||||||
|
|||||||
@@ -279,7 +279,11 @@ type meResponse struct {
|
|||||||
// between the login form and the app, since it cannot read its own cookie.
|
// between the login form and the app, since it cannot read its own cookie.
|
||||||
func handleMe(db *sql.DB) http.HandlerFunc {
|
func handleMe(db *sql.DB) http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
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)
|
user, err := fetchUser(r.Context(), db, caller.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
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
|
||||||
|
}
|
||||||
+28
-35
@@ -16,10 +16,13 @@ import (
|
|||||||
type contextKey string
|
type contextKey string
|
||||||
|
|
||||||
const (
|
const (
|
||||||
ctxUser contextKey = "user"
|
// 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"
|
ctxSession contextKey = "session"
|
||||||
ctxTeams contextKey = "teams"
|
|
||||||
ctxServiceAccount contextKey = "service_account"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// AuthMiddleware accepts either of the two credentials the server issues: an
|
// 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
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
ctx := context.WithValue(r.Context(), ctxTeams, teams)
|
ctx := context.WithValue(r.Context(), ctxCaller, Caller{user: &u, memberships: teams})
|
||||||
ctx = context.WithValue(ctx, ctxUser, u)
|
|
||||||
if sessionID != 0 {
|
if sessionID != 0 {
|
||||||
ctx = context.WithValue(ctx, ctxSession, sessionID)
|
ctx = context.WithValue(ctx, ctxSession, sessionID)
|
||||||
}
|
}
|
||||||
@@ -192,9 +194,13 @@ func hashToken(token string) string {
|
|||||||
return hex.EncodeToString(h[:])
|
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) {
|
func userFromContext(ctx context.Context) (models.User, bool) {
|
||||||
u, ok := ctx.Value(ctxUser).(models.User)
|
c, _ := callerFromContext(ctx)
|
||||||
return u, ok
|
return c.AsHuman()
|
||||||
}
|
}
|
||||||
|
|
||||||
// serviceAccountPrincipal is a service account as resolved from its key:
|
// 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
|
// key is only ever set by the client that holds it, never attached by a
|
||||||
// browser to a request another site makes.
|
// browser to a request another site makes.
|
||||||
func serveAsServiceAccount(w http.ResponseWriter, r *http.Request, next http.Handler, sa serviceAccountPrincipal) {
|
func serveAsServiceAccount(w http.ResponseWriter, r *http.Request, next http.Handler, sa serviceAccountPrincipal) {
|
||||||
ctx := r.Context()
|
var memberships []membership
|
||||||
if sa.scope == models.ServiceAccountScopeTeam {
|
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))
|
next.ServeHTTP(w, r.WithContext(ctx))
|
||||||
}
|
}
|
||||||
|
|
||||||
func serviceAccountFromContext(ctx context.Context) (serviceAccountPrincipal, bool) {
|
// isInstanceServiceAccount is a thin compatibility wrapper over
|
||||||
sa, ok := ctx.Value(ctxServiceAccount).(serviceAccountPrincipal)
|
// Caller.IsInstanceServiceAccount(), for call sites outside this package's
|
||||||
return sa, ok
|
// core predicates (handleCreateTeam, handleCreateServiceAccount) that
|
||||||
}
|
// needed this exact, narrow check before the Caller abstraction existed.
|
||||||
|
|
||||||
// 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.
|
|
||||||
func isInstanceServiceAccount(ctx context.Context) bool {
|
func isInstanceServiceAccount(ctx context.Context) bool {
|
||||||
sa, ok := serviceAccountFromContext(ctx)
|
c, _ := callerFromContext(ctx)
|
||||||
return ok && sa.scope == models.ServiceAccountScopeInstance
|
return c.IsInstanceServiceAccount()
|
||||||
}
|
}
|
||||||
|
|
||||||
// operatorReason marks a write that operator mode refused as such, distinct
|
// 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 next
|
||||||
}
|
}
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
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)
|
next.ServeHTTP(w, r)
|
||||||
return
|
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,
|
// 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.
|
// and an admin who needs to see a team's queue can add themselves to it.
|
||||||
func callerTeamIDs(ctx context.Context) []int64 {
|
func callerTeamIDs(ctx context.Context) []int64 {
|
||||||
ms, _ := ctx.Value(ctxTeams).([]membership)
|
c, _ := callerFromContext(ctx)
|
||||||
ids := make([]int64, 0, len(ms))
|
return c.TeamIDs()
|
||||||
for _, m := range ms {
|
|
||||||
ids = append(ids, m.teamID)
|
|
||||||
}
|
|
||||||
return ids
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// callerRole reports the caller's role in one team, and whether they are in it
|
// callerRole reports the caller's role in one team, and whether they are in it
|
||||||
// at all.
|
// at all.
|
||||||
func callerRole(ctx context.Context, teamID int64) (string, bool) {
|
func callerRole(ctx context.Context, teamID int64) (string, bool) {
|
||||||
ms, _ := ctx.Value(ctxTeams).([]membership)
|
c, _ := callerFromContext(ctx)
|
||||||
for _, m := range ms {
|
return c.Role(teamID)
|
||||||
if m.teamID == teamID {
|
|
||||||
return m.role, true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return "", false
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// requireTeamMember answers the request and reports false unless the caller
|
// 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
|
ssoNoEmail ssoError = "no_email" // the provider sent no email address
|
||||||
ssoEmailConflict ssoError = "email_conflict" // a local account has this email and cannot be linked
|
ssoEmailConflict ssoError = "email_conflict" // a local account has this email and cannot be linked
|
||||||
ssoDisabled ssoError = "disabled" // the linked account is disabled
|
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
|
// 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
|
return 0, ssoEmailConflict
|
||||||
}
|
}
|
||||||
case errors.Is(err, sql.ErrNoRows):
|
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)
|
userID, err = createSSOUser(ctx, tx, id)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, err
|
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) {
|
func TestSSO_NoEmailIsRefused(t *testing.T) {
|
||||||
idp := newFakeIdP(t)
|
idp := newFakeIdP(t)
|
||||||
s := newSSOTS(t, idp)
|
s := newSSOTS(t, idp)
|
||||||
|
|||||||
@@ -41,10 +41,14 @@ func callerIsAdmin(ctx context.Context) bool {
|
|||||||
return ok && u.IsAdmin
|
return ok && u.IsAdmin
|
||||||
}
|
}
|
||||||
|
|
||||||
// callerOwnsTeam reports whether the caller is a human owner of teamID. Built
|
// callerOwnsTeam reports whether the caller is owner-equivalent for teamID:
|
||||||
// on callerRole/ctxTeams like requireTeamOwner, but without writing a
|
// a human owner, or that team's own team-scoped service account (its single
|
||||||
// response: callers here need to combine it with other ways of being
|
// synthetic membership, serveAsServiceAccount — ratified in
|
||||||
// allowed, not stop at the first no.
|
// 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 {
|
func callerOwnsTeam(ctx context.Context, teamID int64) bool {
|
||||||
role, ok := callerRole(ctx, teamID)
|
role, ok := callerRole(ctx, teamID)
|
||||||
return ok && role == models.RoleOwner
|
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
|
// callerMayManageServiceAccount reports whether the caller may mint or revoke
|
||||||
// a key on sa: a system administrator, that team-scoped account's own human
|
// 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
|
// owner, the account rotating its own credential (not a privilege
|
||||||
// privilege escalation, the same reasoning requireSelfOrAdmin already rests
|
// escalation, the same reasoning requireSelfOrAdmin already rests on for a
|
||||||
// on for a user's own API keys.
|
// 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 {
|
func callerMayManageServiceAccount(ctx context.Context, sa models.ServiceAccount) bool {
|
||||||
if callerIsAdmin(ctx) {
|
if callerIsAdmin(ctx) {
|
||||||
return true
|
return true
|
||||||
@@ -183,7 +202,11 @@ func callerMayManageServiceAccount(ctx context.Context, sa models.ServiceAccount
|
|||||||
if sa.TeamID != nil && callerOwnsTeam(ctx, *sa.TeamID) {
|
if sa.TeamID != nil && callerOwnsTeam(ctx, *sa.TeamID) {
|
||||||
return true
|
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 true
|
||||||
}
|
}
|
||||||
return false
|
return false
|
||||||
|
|||||||
@@ -181,10 +181,136 @@ func TestServiceAccount_TeamScopeManagesItsResources(t *testing.T) {
|
|||||||
resp2.Body.Close()
|
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
|
// 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) {
|
func TestServiceAccount_SelfRotatesItsOwnKey(t *testing.T) {
|
||||||
s := newTS(t)
|
s := newTS(t)
|
||||||
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
|
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"))
|
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||||
return
|
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)
|
expires := time.Now().Add(inviteTTL)
|
||||||
|
|
||||||
var out inviteJSON
|
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)
|
INSERT INTO invites (token_hash, team_id, role, created_by, expires_at, max_uses)
|
||||||
VALUES ($1, $2, $3, $4, $5, $6)
|
VALUES ($1, $2, $3, $4, $5, $6)
|
||||||
RETURNING id, team_id, role, created_at, expires_at, max_uses, uses`,
|
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 {
|
Scan(&out.ID, &out.TeamID, &out.Role, &created, &expiresAt, &out.MaxUses, &out.Uses); err != nil {
|
||||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||||
return
|
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"))
|
errResp("this server has no ntfy configured, so it can send nothing"))
|
||||||
return
|
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
|
var topic *string
|
||||||
if err := db.QueryRowContext(r.Context(),
|
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"))
|
respond(w, http.StatusBadRequest, errResp("dismissed is required"))
|
||||||
return
|
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
|
var err error
|
||||||
if *req.Dismissed {
|
if *req.Dismissed {
|
||||||
|
|||||||
@@ -2,10 +2,14 @@ package api_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
|
"database/sql"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/cookiejar"
|
"net/http/cookiejar"
|
||||||
"testing"
|
"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,
|
// 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)
|
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"
|
"github.com/go-chi/chi/v5"
|
||||||
)
|
)
|
||||||
|
|
||||||
// handleListTeams lists the caller's own teams, each with their role in it. An
|
// handleListTeams lists the caller's own teams, each with their role in it,
|
||||||
// administrator listing every team goes through the admin endpoint instead:
|
// or — with ?name= — looks up one team by exact name regardless of caller
|
||||||
// this one answers "what am I part of", which is what the UI's team filter and
|
// identity (TEAM-LOOKUP.md). An administrator listing every team goes
|
||||||
// the combined queue are built from.
|
// 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 {
|
func handleListTeams(db *sql.DB) http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
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())
|
caller, _ := userFromContext(r.Context())
|
||||||
rows, err := db.QueryContext(r.Context(), `
|
rows, err := db.QueryContext(r.Context(), `
|
||||||
SELECT t.id, t.name, t.created_at, m.role, m.source
|
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:
|
// 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
|
// "what is this person in", which /api/teams cannot answer because it is always
|
||||||
// about the caller.
|
// about the caller.
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"testing"
|
"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
|
// 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)
|
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.`,
|
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.',
|
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.',
|
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.`;
|
}[code] || `Signing in with ${sso} failed.`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user