Add a regression test for the team/admin/self authorization pattern
Part of the same security-hardening pass as the last four commits.
terdut-server's authz is already solid -- centralized predicates
(requireTeamMember, requireTeamOwner, requireSelfOrAdmin, AdminOnly)
rather than ad hoc per-handler checks, confirmed by spot-checking several
handlers while writing this. But it is enforced by convention, not the
type system: a future handler that forgets its guard would compile and
read fine on review, exactly like one that remembers it.
authz_scope_test.go builds two teams and, for every team-scoped route
(members, OIDC groups, invites, escalation, dead man's switches,
integrations, schedule, plus every /api/incidents/{id}/... route, scoped
by the incident's own team_id through incidentIDParam's single
chokepoint), calls it as one team's owner against the other team's
resources and asserts 404 -- requireTeamMember and requireTeamOwner both
answer a non-member that way. Separate tests cover AdminOnly's routes
(403 for a non-admin) and requireSelfOrAdmin's (403 for a non-admin
acting on someone else's account).
Verified the test actually catches a regression, not just that it
passes today: temporarily removed handleListTeamMembers' requireTeamMember
call, confirmed exactly that one subtest failed and nothing else did,
then put it back.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Niklas Ye
@@ -0,0 +1,181 @@
|
||||
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.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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user