Add a system administrator role, and gate account management behind it
CI / chart (pull_request) Successful in 2s
CI / security (pull_request) Successful in 16s
CI / test (pull_request) Successful in 1m37s

Until now every authenticated caller could create and delete users, set
anybody's password and mint anybody's API keys -- auth.go said so in a
comment. Defensible with one operator and a hand-made account; not once
people sign themselves up (#7), and not in a multi-tenant install (#4),
where the user list is no longer everybody who works here.

users.is_admin is the flag. AdminOnly gates creating and deleting users
and granting the flag itself. The endpoints that are self-service for
your own account and administration for somebody else's -- password,
ntfy topic, API keys -- go through requireSelfOrAdmin instead, because
which rule applies depends on the {id} in the path rather than on the
route.

Minting your own API key stays self-service. A key carries exactly the
rights of the user it belongs to, so issuing one is no more than signing
in again; requiring an admin for it would mean a responder cannot set up
the TUI without somebody else in the room.

/api/users stays readable by everybody. The queue's assignment control
and the on-call schedule both have to name people, and hiding the roster
from the people on it buys nothing.

THE MIGRATION MAKES EVERY EXISTING USER AN ADMINISTRATOR. They already
hold these powers, so nobody's access changes on upgrade: it names what
is already true and leaves demotion as a deliberate act. Promoting only
user 1 would silently strip the others, and could leave an install whose
only administrator is an account nobody has a password for.

Two guards keep an install administrable: the last administrator can be
neither deleted nor demoted, and nobody can delete or demote themselves
-- the likelier accident, where the only admin clears their own flag
while tidying up and locks the door behind them.

No UI changes: there are no account-management screens yet. models.User
carries is_admin (not omitempty, so a client can tell false from an old
server), which is what #5's admin page will render from.
This commit is contained in:
Niklas Ye
2026-09-20 13:20:04 +02:00
parent e3ad19c110
commit 1377d9005b
8 changed files with 491 additions and 19 deletions
+255
View File
@@ -0,0 +1,255 @@
package api_test
import (
"bytes"
"encoding/json"
"io"
"net/http"
"strconv"
"testing"
)
// The bootstrap user is an administrator; everybody it creates afterwards is
// not. These tests are about the line between them.
// id64 spells an id into a path segment.
func id64(n int64) string { return strconv.FormatInt(n, 10) }
// member creates an ordinary user and an API key for it, and returns a caller
// that authenticates as them. Minting the key goes through the admin's own
// credentials, which is how a real install hands one out.
func member(t *testing.T, s *ts, username string) (id int64, call func(method, path string, body any) *http.Response) {
t.Helper()
resp := s.req(t, http.MethodPost, "/api/users",
map[string]string{"username": username, "email": username + "@test.com"})
if resp.StatusCode != http.StatusCreated {
t.Fatalf("create %s: %d", username, resp.StatusCode)
}
var user struct {
ID int64 `json:"id"`
IsAdmin bool `json:"is_admin"`
}
decode(t, resp, &user)
if user.IsAdmin {
t.Fatalf("a created user must not be an administrator")
}
resp = s.req(t, http.MethodPost, "/api/users/"+id64(user.ID)+"/api-keys",
map[string]string{"name": "test"})
if resp.StatusCode != http.StatusCreated {
t.Fatalf("mint key for %s: %d", username, resp.StatusCode)
}
var key struct {
Key string `json:"key"`
}
decode(t, resp, &key)
return user.ID, func(method, path string, body any) *http.Response {
t.Helper()
var r io.Reader
if body != nil {
data, _ := json.Marshal(body)
r = bytes.NewReader(data)
}
req, _ := http.NewRequest(method, s.URL+path, r)
req.Header.Set("Authorization", "Bearer "+key.Key)
if body != nil {
req.Header.Set("Content-Type", "application/json")
}
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("%s %s: %v", method, path, err)
}
return resp
}
}
// The whole point of the release: a user who is not an administrator cannot
// manage other people's accounts. Every one of these was open to any
// authenticated caller before.
func TestAdmin_MemberIsRefusedAdministration(t *testing.T) {
s := newTS(t)
memberID, call := member(t, s, "member")
cases := []struct {
name string
method string
path string
body any
}{
{"create a user", http.MethodPost, "/api/users",
map[string]string{"username": "sneaky", "email": "sneaky@test.com"}},
{"delete the admin", http.MethodDelete, "/api/users/1", nil},
{"grant themselves admin", http.MethodPut, "/api/users/" + id64(memberID) + "/admin",
map[string]bool{"is_admin": true}},
{"set the admin's password", http.MethodPut, "/api/users/1/password",
map[string]string{"password": "hunter2-hunter2"}},
{"mint a key for the admin", http.MethodPost, "/api/users/1/api-keys",
map[string]string{"name": "borrowed"}},
{"retarget the admin's notifications", http.MethodPut, "/api/users/1/notify",
map[string]string{"ntfy_topic": "attacker-topic"}},
}
for _, c := range cases {
resp := call(c.method, c.path, c.body)
resp.Body.Close()
if resp.StatusCode != http.StatusForbidden {
t.Errorf("%s: expected 403, got %d", c.name, resp.StatusCode)
}
}
}
// Being refused other people's accounts must not cost a user their own.
func TestAdmin_MemberKeepsTheirOwnAccount(t *testing.T) {
s := newTS(t)
memberID, call := member(t, s, "member")
self := "/api/users/" + id64(memberID)
resp := call(http.MethodPut, self+"/notify", map[string]string{"ntfy_topic": "terdut-member"})
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Errorf("own notify target: %d", resp.StatusCode)
}
resp = call(http.MethodPut, self+"/password", map[string]string{"password": "correct-horse-battery"})
resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent {
t.Errorf("own password: %d", resp.StatusCode)
}
// An API key carries exactly the rights of its owner, so minting your own
// is no more than signing in again.
resp = call(http.MethodPost, self+"/api-keys", map[string]string{"name": "laptop"})
resp.Body.Close()
if resp.StatusCode != http.StatusCreated {
t.Errorf("own API key: %d", resp.StatusCode)
}
// And the queue still has to be able to name people.
resp = call(http.MethodGet, "/api/users", nil)
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Errorf("list users: %d", resp.StatusCode)
}
}
// Incident work is everybody's job; none of it is administration.
func TestAdmin_MemberCanWorkIncidents(t *testing.T) {
s := newTS(t)
_, call := member(t, s, "responder")
postWebhook(t, s, []map[string]any{
amAlert("fp-admin", "DiskFull", "firing", "2026-09-20T10:00:00Z", zeroTime, nil),
})
for _, c := range []struct {
name string
method string
path string
}{
{"list", http.MethodGet, "/api/incidents"},
{"acknowledge", http.MethodPost, "/api/incidents/1/acknowledge"},
{"resolve", http.MethodPost, "/api/incidents/1/resolve"},
} {
resp := call(c.method, c.path, nil)
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Errorf("%s: expected 200, got %d", c.name, resp.StatusCode)
}
}
}
// An install must never be left with nobody who can administer it.
func TestAdmin_LastAdministratorIsProtected(t *testing.T) {
s := newTS(t)
resp := s.req(t, http.MethodPut, "/api/users/1/admin", map[string]bool{"is_admin": false})
resp.Body.Close()
if resp.StatusCode != http.StatusConflict {
t.Errorf("self-demotion: expected 409, got %d", resp.StatusCode)
}
resp = s.req(t, http.MethodDelete, "/api/users/1", nil)
resp.Body.Close()
if resp.StatusCode != http.StatusConflict {
t.Errorf("deleting yourself: expected 409, got %d", resp.StatusCode)
}
// With a second administrator the first may stand down, but not while they
// are the only one — which is the same rule from the other side.
otherID, _ := member(t, s, "second")
resp = s.req(t, http.MethodPut, "/api/users/"+id64(otherID)+"/admin", map[string]bool{"is_admin": true})
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("granting admin: %d", resp.StatusCode)
}
resp = s.req(t, http.MethodDelete, "/api/users/"+id64(otherID), nil)
resp.Body.Close()
if resp.StatusCode != http.StatusNoContent {
t.Errorf("deleting the second admin: expected 204, got %d", resp.StatusCode)
}
}
// A promoted user gets the powers with the flag, and loses them with it.
func TestAdmin_GrantAndRevokeChangeWhatIsAllowed(t *testing.T) {
s := newTS(t)
memberID, call := member(t, s, "promotee")
admin := "/api/users/" + id64(memberID) + "/admin"
resp := call(http.MethodPost, "/api/users", map[string]string{"username": "a", "email": "a@test.com"})
resp.Body.Close()
if resp.StatusCode != http.StatusForbidden {
t.Fatalf("before the grant: %d", resp.StatusCode)
}
resp = s.req(t, http.MethodPut, admin, map[string]bool{"is_admin": true})
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("grant: %d", resp.StatusCode)
}
resp = call(http.MethodPost, "/api/users", map[string]string{"username": "b", "email": "b@test.com"})
resp.Body.Close()
if resp.StatusCode != http.StatusCreated {
t.Errorf("after the grant: expected 201, got %d", resp.StatusCode)
}
resp = s.req(t, http.MethodPut, admin, map[string]bool{"is_admin": false})
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("revoke: %d", resp.StatusCode)
}
resp = call(http.MethodPost, "/api/users", map[string]string{"username": "c", "email": "c@test.com"})
resp.Body.Close()
if resp.StatusCode != http.StatusForbidden {
t.Errorf("after the revoke: expected 403, got %d", resp.StatusCode)
}
}
// The flag has to reach the client, or the web UI cannot decide what to show.
func TestAdmin_MeReportsTheFlag(t *testing.T) {
s := newTS(t)
var me struct {
User struct {
IsAdmin bool `json:"is_admin"`
} `json:"user"`
}
decode(t, s.req(t, http.MethodGet, "/api/me", nil), &me)
if !me.User.IsAdmin {
t.Error("the bootstrap user should be an administrator")
}
_, call := member(t, s, "plain")
var theirs struct {
User struct {
IsAdmin bool `json:"is_admin"`
} `json:"user"`
}
decode(t, call(http.MethodGet, "/api/me", nil), &theirs)
if theirs.User.IsAdmin {
t.Error("a created user should not be an administrator")
}
}