926aa2d3ec
Part of the same security-hardening pass as the last five commits, and
the last item in its backlog. User API keys had no expiry at all --
unlike service-account keys, visibly distinct only by their "tdsa_"
prefix -- and, it turns out while implementing this, no way to list
them either: only create (returns the raw key once) and delete-by-id
existed, so a key's owner had no way to even discover what keys they
had short of remembering IDs from creation time.
handleCreateAPIKey takes an optional expires_in_days (0, the default,
keeps today's behavior: never expires, so no existing integration is
affected). apiKeyUser's lookup now carries `expires_at IS NULL OR
expires_at > now` as part of the query itself, the same way serveAs's
disabled_at check already works -- an expired key simply fails to
resolve, like a wrong one, rather than resolving and being caught
after the fact. New GET /api/users/{id}/api-keys (requireSelfOrAdmin,
same as create/delete) lists id/name/created_at/last_used_at/expires_at,
never the raw key.
Scoped down from the original plan on request: no web UI change, since
there turned out to be no existing API-keys UI at all to extend --
building one from scratch would have been a real feature addition, not
a hardening tweak.
Mirrored the additive expires_at field in terdut-tui's APIKey struct
(separate commit, separate repo) per this workspace's version-coupling
rule; the TUI does not create or list expiring keys itself yet.
New tests (api_keys_test.go): default never-expires, expires_in_days
sets expires_at, out-of-range values rejected, an expired key fails
auth after a fresh one worked, the listing never includes the raw key.
Also added the new GET route to authz_scope_test.go's self-or-admin
table from the previous commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
183 lines
6.5 KiB
Go
183 lines
6.5 KiB
Go
package api_test
|
|
|
|
import (
|
|
"net/http"
|
|
"testing"
|
|
)
|
|
|
|
// This file is the regression test for the pattern documented throughout
|
|
// middleware.go: every team-scoped handler calls requireTeamMember or
|
|
// requireTeamOwner before touching data, every self-or-admin handler calls
|
|
// requireSelfOrAdmin, and every admin-only route sits behind AdminOnly. That
|
|
// pattern is enforced by convention, not by the type system — a new handler
|
|
// that forgets the call would compile and pass review on a quick read just
|
|
// as easily as one that remembers it. These tests exercise every route that
|
|
// carries one of those guards as a caller who should be refused, so a future
|
|
// handler missing its guard fails CI instead of becoming a silent IDOR.
|
|
|
|
// TestAuthzScope_TeamScopedRoutesRefuseANonMember builds two teams and, for
|
|
// every team-scoped route, calls it as team A's owner against team B's
|
|
// resources. requireTeamMember and requireTeamOwner both answer a non-member
|
|
// with 404 (team.go's own reasoning: whether a team exists is itself
|
|
// something only its members should learn), so every one of these must come
|
|
// back 404 regardless of which of the two guards its handler uses.
|
|
func TestAuthzScope_TeamScopedRoutesRefuseANonMember(t *testing.T) {
|
|
s := newTS(t)
|
|
a := newTeam(t, s, "authz-a")
|
|
b := newTeam(t, s, "authz-b")
|
|
|
|
// An incident in B, to cover the ID-based routes under /api/incidents —
|
|
// scoped by the incident's own team_id rather than a {teamID} path
|
|
// segment, but through the same single chokepoint (incidentIDParam).
|
|
postToIntegration(t, s, b.key, "fp-authz-scope", "AuthzScopeAlert")
|
|
var incidents []struct {
|
|
ID int64 `json:"id"`
|
|
}
|
|
decode(t, b.call(http.MethodGet, "/api/incidents", nil), &incidents)
|
|
if len(incidents) == 0 {
|
|
t.Fatal("setup: no incident in team B to test against")
|
|
}
|
|
incidentPath := "/api/incidents/" + id64(incidents[0].ID)
|
|
|
|
bPath := "/api/teams/" + id64(b.id)
|
|
tests := []struct {
|
|
method, path string
|
|
}{
|
|
// Team membership/ownership itself.
|
|
{http.MethodPut, bPath},
|
|
{http.MethodDelete, bPath},
|
|
{http.MethodGet, bPath + "/members"},
|
|
{http.MethodPost, bPath + "/members"},
|
|
{http.MethodDelete, bPath + "/members/1"},
|
|
|
|
// OIDC group binding.
|
|
{http.MethodGet, bPath + "/oidc-groups"},
|
|
{http.MethodPut, bPath + "/oidc-groups"},
|
|
|
|
// Invites.
|
|
{http.MethodGet, bPath + "/invites"},
|
|
{http.MethodPost, bPath + "/invites"},
|
|
{http.MethodDelete, bPath + "/invites/1"},
|
|
|
|
// Escalation.
|
|
{http.MethodGet, bPath + "/escalation"},
|
|
{http.MethodPut, bPath + "/escalation"},
|
|
|
|
// Dead man's switches.
|
|
{http.MethodGet, bPath + "/deadman/switches"},
|
|
{http.MethodPost, bPath + "/deadman/switches"},
|
|
{http.MethodPut, bPath + "/deadman/switches/1"},
|
|
{http.MethodDelete, bPath + "/deadman/switches/1"},
|
|
|
|
// Integrations.
|
|
{http.MethodGet, bPath + "/integrations"},
|
|
{http.MethodPost, bPath + "/integrations"},
|
|
{http.MethodPatch, bPath + "/integrations/1"},
|
|
{http.MethodDelete, bPath + "/integrations/1"},
|
|
|
|
// Schedule.
|
|
{http.MethodGet, bPath + "/schedule"},
|
|
{http.MethodPost, bPath + "/schedule"},
|
|
{http.MethodDelete, bPath + "/schedule/1"},
|
|
|
|
// Incidents, scoped by the incident's own team rather than a
|
|
// {teamID} segment.
|
|
{http.MethodGet, incidentPath},
|
|
{http.MethodGet, incidentPath + "/alerts"},
|
|
{http.MethodGet, incidentPath + "/timeline"},
|
|
{http.MethodGet, incidentPath + "/similar"},
|
|
{http.MethodPost, incidentPath + "/acknowledge"},
|
|
{http.MethodDelete, incidentPath + "/acknowledge"},
|
|
{http.MethodPost, incidentPath + "/resolve"},
|
|
{http.MethodPost, incidentPath + "/assign"},
|
|
{http.MethodPost, incidentPath + "/snooze"},
|
|
{http.MethodDelete, incidentPath + "/snooze"},
|
|
{http.MethodPost, incidentPath + "/archive"},
|
|
{http.MethodDelete, incidentPath + "/archive"},
|
|
{http.MethodPost, incidentPath + "/notes"},
|
|
{http.MethodDelete, incidentPath + "/notes/1"},
|
|
}
|
|
|
|
for _, tc := range tests {
|
|
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
|
|
resp := a.call(tc.method, tc.path, nil)
|
|
defer resp.Body.Close()
|
|
if resp.StatusCode != http.StatusNotFound {
|
|
t.Errorf("status = %d, want %d (A is not a member of B)", resp.StatusCode, http.StatusNotFound)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestAuthzScope_AdminOnlyRoutesRefuseANonAdmin exercises AdminOnly's group
|
|
// in router.go directly: a signed-in, non-admin caller gets 403 from every
|
|
// route in it, before any handler body runs.
|
|
func TestAuthzScope_AdminOnlyRoutesRefuseANonAdmin(t *testing.T) {
|
|
s := newTS(t)
|
|
a := newTeam(t, s, "authz-admin")
|
|
|
|
tests := []struct {
|
|
method, path string
|
|
}{
|
|
{http.MethodPost, "/api/users"},
|
|
{http.MethodDelete, "/api/users/1"},
|
|
{http.MethodPut, "/api/users/1/admin"},
|
|
{http.MethodPut, "/api/users/1/disabled"},
|
|
{http.MethodGet, "/api/admin/teams"},
|
|
{http.MethodGet, "/api/admin/teams/" + id64(a.id)},
|
|
{http.MethodGet, "/api/admin/settings"},
|
|
{http.MethodPut, "/api/admin/settings"},
|
|
}
|
|
|
|
for _, tc := range tests {
|
|
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
|
|
resp := a.call(tc.method, tc.path, nil)
|
|
defer resp.Body.Close()
|
|
if resp.StatusCode != http.StatusForbidden {
|
|
t.Errorf("status = %d, want %d (not an admin)", resp.StatusCode, http.StatusForbidden)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestAuthzScope_SelfOrAdminRoutesRefuseAnotherNonAdminUser exercises
|
|
// requireSelfOrAdmin's call sites: a non-admin caller acting on a *different*
|
|
// user's account must be refused, the same as AdminOnly's routes, even
|
|
// though these sit in the general authenticated group rather than behind
|
|
// AdminOnly itself.
|
|
func TestAuthzScope_SelfOrAdminRoutesRefuseAnotherNonAdminUser(t *testing.T) {
|
|
s := newTS(t)
|
|
a := newTeam(t, s, "authz-self-a")
|
|
b := newTeam(t, s, "authz-self-b")
|
|
|
|
var members []struct {
|
|
UserID int64 `json:"user_id"`
|
|
}
|
|
decode(t, s.req(t, http.MethodGet, "/api/teams/"+id64(b.id)+"/members", nil), &members)
|
|
if len(members) == 0 {
|
|
t.Fatal("setup: team B has no members")
|
|
}
|
|
bUserID := id64(members[0].UserID)
|
|
|
|
tests := []struct {
|
|
method, path string
|
|
}{
|
|
{http.MethodGet, "/api/users/" + bUserID + "/teams"},
|
|
{http.MethodPut, "/api/users/" + bUserID + "/notify"},
|
|
{http.MethodPut, "/api/users/" + bUserID + "/password"},
|
|
{http.MethodGet, "/api/users/" + bUserID + "/api-keys"},
|
|
{http.MethodPost, "/api/users/" + bUserID + "/api-keys"},
|
|
{http.MethodDelete, "/api/users/" + bUserID + "/api-keys/1"},
|
|
}
|
|
|
|
for _, tc := range tests {
|
|
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
|
|
resp := a.call(tc.method, tc.path, nil)
|
|
defer resp.Body.Close()
|
|
if resp.StatusCode != http.StatusForbidden {
|
|
t.Errorf("status = %d, want %d (not self, not an admin)", resp.StatusCode, http.StatusForbidden)
|
|
}
|
|
})
|
|
}
|
|
}
|