Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 4358e84b24 | |||
| f45dc2f925 | |||
| 774fdfcaa8 | |||
| fd26fef1ba | |||
| a9d788cc83 |
+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
|
||||||
|
|
||||||
|
|||||||
@@ -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.1
|
version: 0.34.0
|
||||||
appVersion: "v0.33.1"
|
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
|
||||||
|
}
|
||||||
+29
-36
@@ -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
|
||||||
ctxSession contextKey = "session"
|
// predicate in this package reads from — a human and a service account
|
||||||
ctxTeams contextKey = "teams"
|
// used to be two parallel, un-unified context keys (ctxUser/ctxTeams vs.
|
||||||
ctxServiceAccount contextKey = "service_account"
|
// 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
|
// 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
|
||||||
|
|||||||
@@ -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()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user