774fdfcaa8
ctxUser/ctxTeams (human) and ctxServiceAccount (+ a synthetic ctxTeams
entry, service account) used to be two parallel, un-unified context
representations -- every authz predicate had to remember which one(s) it
needed, and every place that forgot either wrongly 403'd a service account
(terdut-server#23, terdut-operator#3), crashed on an unchecked zero-value
user id, or silently no-op'd. New internal/api/caller.go collapses both
into one Caller, stored under one ctxCaller key by serveAs/serveAsServiceAccount;
every existing predicate (userFromContext, callerTeamIDs, callerRole,
callerIsAdmin, isInstanceServiceAccount, AdminOnly, requireSelfOrAdmin,
requireTeamOwner, OperatorModeBlock) now reads through it, with identical
behavior for every untouched call site (alerts.go, incidents.go,
schedule.go, stats.go, etc.) -- confirmed by the full existing suite
passing unchanged.
Four real fixes land alongside the refactor, not just the restructuring:
1. callerMayManageServiceAccount gains the one load-bearing branch this
exists for: an instance-scoped service account may now manage (mint or
revoke a key on) any team-scoped account, not only a human admin, that
team's human owner, or the account itself. handleCreateServiceAccount
already let an instance-scoped caller *create* a team-scoped account for
any team; adopting or rotating one it didn't just create in the same
call -- terdut-operator's own documented crash-window recovery -- had no
equivalent permission and 403'd forever. Closes terdut-operator#3.
2. handleCreateInvite wrote a service-account caller's zero-value user id
straight into invites.created_by (nullable, but never passed as nil),
which foreign-key-violates against users(id) -- a 500, not success, for
any team-scoped service account minting an invite. Fixed the same way
handleCreateServiceAccount already handles the analogous case. Found
live while verifying this change, not filed separately since it's fixed
in the same place it was found.
3. handleMe and handleTestNotification 500'd for a service-account caller
(fetchUser/the ntfy_topic lookup against a zero-value user id that
matches no row); handleDismissOnboarding silently no-op'd (UPDATE ...
WHERE id = 0). All three now call Caller.AsHuman() and return an
explicit 403 ("this endpoint is for human accounts only").
4. Ratifies, rather than further narrows, two capabilities a team-scoped
service account already had by construction and this document's own
text once called "a gap acknowledged rather than closed": owner-equivalent
reach over membership/invites, and minting another service account for
its own team. terdut-operator's new TerdutTeam invite-minting feature is
about to depend on the first one, so this makes it documented, tested,
intentional behavior instead of an accident nobody was supposed to rely
on.
AdminOnly/requireSelfOrAdmin are unchanged in effect: still human-only,
forever, for every scope of service account -- confirmed by
TestAdminOnly_RefusesEveryServiceAccountScope. terdut-server#23's named
routes (POST /api/users, PUT /api/admin/settings) were never the right
thing to widen; its real fix is the terdut-operator invite feature,
recorded in SERVICE-ACCOUNTS.md's "What this unblocks" and closing that
issue once it ships.
SERVICE-ACCOUNTS.md amended in place (not a new file, its own established
convention) to describe the as-built Caller model, correct its own
aspirational claim about AdminOnly that TEAM-LOOKUP.md had already flagged
as not matching shipped code, and record all of the above.
124 lines
4.7 KiB
Go
124 lines
4.7 KiB
Go
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
|
|
}
|