Let each team name its own OIDC group, not a global mapping

Team membership from single sign-on used to come from one env var,
TERDUT_OIDC_GROUP_MAPPINGS, matched against a team by name and creating
the team if none existed. That put the decision in the server's
environment rather than the team's own hands, needed a restart to
change, and let a typo in a team name silently create a stray team.

Each team now carries its own oidc_member_group and oidc_owner_group,
set by its owner (or an administrator) from the Members tab, or PUT
/api/teams/{teamID}/oidc-groups. The "highest role wins" rule
TERDUT_OIDC_GROUP_MAPPINGS used to apply across mappings now applies
across one team's own two fields: being in both makes somebody an
owner. The sync no longer creates a team by name; a group only ever
grants into a team that already exists.

This is a breaking change for anyone already using
TERDUT_OIDC_GROUP_MAPPINGS, deliberately not auto-migrated: an
OIDC-sourced membership is dropped at a user's next sign-in until its
team's owner re-sets the group. The README's OIDC section spells out
the migration and the risk of a visible access gap during it.

TERDUT_OIDC_ADMIN_GROUP and TERDUT_OIDC_ALLOWED_GROUPS are untouched --
only team membership moved. terdut-tui needs no change: it only reads
GET /api/teams and GET /api/teams/{id}/members, and neither response
shape moved.
This commit is contained in:
Niklas Ye
2026-09-27 11:43:57 +02:00
parent 97a4814c04
commit 5b4683febf
17 changed files with 554 additions and 129 deletions
+41 -15
View File
@@ -211,7 +211,15 @@ func handleOIDCCallback(db *sql.DB, prov *oidc.Provider, publicURL string) http.
return
}
userID, err := signInSSO(r.Context(), db, cfg, identity, grants)
teamGroups, err := loadTeamGroups(r.Context(), db)
if err != nil {
log.Printf("oidc: load team groups: %v", err)
ssoRedirect(w, r, ssoFailed)
return
}
teamGrants := oidc.ComputeTeamGrants(teamGroups, identity.Groups)
userID, err := signInSSO(r.Context(), db, cfg, identity, grants, teamGrants)
if err != nil {
var se ssoError
if errors.As(err, &se) {
@@ -253,7 +261,7 @@ func safeNext(next string) string {
// signInSSO resolves the identity to a user and applies its grants, in one
// transaction: a login that fails half way must not leave memberships changed.
func signInSSO(ctx context.Context, db *sql.DB, cfg config.OIDC, id *oidc.Identity, g oidc.Grants) (int64, error) {
func signInSSO(ctx context.Context, db *sql.DB, cfg config.OIDC, id *oidc.Identity, g oidc.Grants, teamRoles map[int64]string) (int64, error) {
tx, err := db.BeginTx(ctx, nil)
if err != nil {
return 0, err
@@ -272,12 +280,34 @@ func signInSSO(ctx context.Context, db *sql.DB, cfg config.OIDC, id *oidc.Identi
if disabled {
return 0, ssoDisabled
}
if err := syncGrants(ctx, tx, userID, g); err != nil {
if err := syncGrants(ctx, tx, userID, g, teamRoles); err != nil {
return 0, err
}
return userID, tx.Commit()
}
// loadTeamGroups reads every team's own OIDC group binding, for the sync to
// evaluate against one user's groups at a time. Teams are few, so this reads
// the whole table rather than filtering it.
func loadTeamGroups(ctx context.Context, db *sql.DB) ([]oidc.TeamGroup, error) {
rows, err := db.QueryContext(ctx,
"SELECT id, COALESCE(oidc_member_group, ''), COALESCE(oidc_owner_group, '') FROM teams")
if err != nil {
return nil, err
}
defer rows.Close()
var out []oidc.TeamGroup
for rows.Next() {
var tg oidc.TeamGroup
if err := rows.Scan(&tg.TeamID, &tg.MemberGroup, &tg.OwnerGroup); err != nil {
return nil, err
}
out = append(out, tg)
}
return out, rows.Err()
}
// resolveSSOUser finds the user an identity belongs to, linking or creating one
// when this is its first sign-in.
//
@@ -407,7 +437,11 @@ func refreshProfile(ctx context.Context, tx *sql.Tx, userID int64, id *oidc.Iden
// administrator is a manual one. Rows added by hand are 'manual', and the sync
// only ever raises them (turning them into 'oidc' rows), never lowers or removes
// them.
func syncGrants(ctx context.Context, tx *sql.Tx, userID int64, g oidc.Grants) error {
//
// teamRoles is keyed by team ID, not name: a team must already exist, with its
// own oidc_member_group/oidc_owner_group set by its owner, before a group can
// grant access to it. The sync never creates a team.
func syncGrants(ctx context.Context, tx *sql.Tx, userID int64, g oidc.Grants, teamRoles map[int64]string) error {
// Administrator. A manual administrator stays one whatever the groups say.
if g.Admin {
if _, err := tx.ExecContext(ctx,
@@ -422,21 +456,13 @@ func syncGrants(ctx context.Context, tx *sql.Tx, userID int64, g oidc.Grants) er
}
// Teams. The result of the loop is the set of teams the groups grant.
granted := make([]int64, 0, len(g.Teams))
for name, role := range g.Teams {
if _, err := tx.ExecContext(ctx,
"INSERT INTO teams (name) VALUES ($1) ON CONFLICT (name) DO NOTHING", name); err != nil {
return err
}
var teamID int64
if err := tx.QueryRowContext(ctx, "SELECT id FROM teams WHERE name = $1", name).Scan(&teamID); err != nil {
return err
}
granted := make([]int64, 0, len(teamRoles))
for teamID, role := range teamRoles {
granted = append(granted, teamID)
// A row the sync owns follows the groups in both directions. One added by
// hand is only raised: a member the owner made an owner by hand is not
// demoted because the mapping says member.
// demoted because the group says member.
if _, err := tx.ExecContext(ctx, `
INSERT INTO team_members (team_id, user_id, role, source)
VALUES ($1, $2, $3, 'oidc')
+74
View File
@@ -0,0 +1,74 @@
package api
import (
"database/sql"
"net/http"
)
// teamOIDCGroups is one team's own OIDC binding: which group, if any, grants
// member access and which grants owner access. The same shape answers GET and
// is accepted by PUT. An empty string means no group grants that role here.
type teamOIDCGroups struct {
MemberGroup string `json:"member_group"`
OwnerGroup string `json:"owner_group"`
}
// handleGetTeamOIDCGroups answers which groups control a team's membership.
// Member-gated like the member list itself: this is part of "who is in the
// team and why", not a setting only an owner should be able to see.
func handleGetTeamOIDCGroups(db *sql.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
teamID, ok := teamParam(w, r)
if !ok {
return
}
if !requireTeamMember(w, r, teamID) {
return
}
var g teamOIDCGroups
err := db.QueryRowContext(r.Context(),
"SELECT COALESCE(oidc_member_group, ''), COALESCE(oidc_owner_group, '') FROM teams WHERE id = $1",
teamID).Scan(&g.MemberGroup, &g.OwnerGroup)
if err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
}
respond(w, http.StatusOK, g)
}
}
// handleSetTeamOIDCGroups sets which groups control a team's membership.
//
// Owner-gated, the same as the schedule, the integrations and the escalation
// ladder: this decides who can end up in the team, which is exactly the kind
// of thing only the team's own owner (or an administrator repairing it) should
// be able to change. An empty string clears a binding.
func handleSetTeamOIDCGroups(db *sql.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
teamID, ok := teamParam(w, r)
if !ok {
return
}
if !requireTeamOwner(w, r, teamID) {
return
}
var req teamOIDCGroups
if err := decodeJSON(r, &req); err != nil {
respond(w, http.StatusBadRequest, errResp("invalid request body"))
return
}
if _, err := db.ExecContext(r.Context(), `
UPDATE teams
SET oidc_member_group = NULLIF($1, ''),
oidc_owner_group = NULLIF($2, '')
WHERE id = $3`,
req.MemberGroup, req.OwnerGroup, teamID); err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
}
w.WriteHeader(http.StatusNoContent)
}
}
+50 -10
View File
@@ -127,9 +127,10 @@ type idpUser struct {
badNonce bool
}
// ssoConfig is a terdut configuration wired to idp, with the mapping the tests
// share: terdut-users may sign in, terdut-admins administer, and the sre groups
// grant roles in the SRE team.
// ssoConfig is a terdut configuration wired to idp: terdut-users may sign in,
// terdut-admins administer. Which groups grant which team is not config
// anymore — it is each team's own oidc_member_group/oidc_owner_group, so a
// test that needs one seeds it with seedTeam.
func ssoConfig(idp *fakeIdP) config.Config {
c := testConfig()
c.OIDC = config.OIDC{
@@ -143,16 +144,28 @@ func ssoConfig(idp *fakeIdP) config.Config {
GroupsClaim: "groups",
AllowedGroups: []string{"terdut-users"},
AdminGroup: "terdut-admins",
GroupMappings: []config.GroupMapping{
{Group: "sre", Team: "SRE", Role: "member"},
{Group: "sre-leads", Team: "SRE", Role: "owner"},
{Group: "platform", Team: "Platform", Role: "member"},
},
SessionMaxAge: 12 * time.Hour,
}
return c
}
// seedTeam creates a team with an OIDC group binding, the way an owner would
// set one from the Members tab. Teams are no longer created by the sync
// itself, so a test whose groups should grant something needs the team to
// already exist. An empty group means that role is not granted by one.
func (s *ts) seedTeam(t *testing.T, name, memberGroup, ownerGroup string) int64 {
t.Helper()
var id int64
err := s.db.QueryRow(`
INSERT INTO teams (name, oidc_member_group, oidc_owner_group)
VALUES ($1, NULLIF($2, ''), NULLIF($3, '')) RETURNING id`,
name, memberGroup, ownerGroup).Scan(&id)
if err != nil {
t.Fatalf("seed team %q: %v", name, err)
}
return id
}
func newSSOTS(t *testing.T, idp *fakeIdP, tweak ...func(*config.Config)) *ts {
t.Helper()
c := ssoConfig(idp)
@@ -290,6 +303,8 @@ func sameMap(a, b map[string]string) bool {
func TestSSO_FirstSignInCreatesUserAndGrantsTeams(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
s.seedTeam(t, "SRE", "sre", "sre-leads")
s.seedTeam(t, "Platform", "platform", "")
b := ssoBrowser(t, s)
if loc := signInSSO(t, idp, b, withGroups(alice, "terdut-users", "sre", "platform")); loc != "/" {
@@ -305,6 +320,27 @@ func TestSSO_FirstSignInCreatesUserAndGrantsTeams(t *testing.T) {
}
}
// A group matching no team's own binding grants nothing and creates nothing:
// unlike the old global mapping, the sync never creates a team by name.
func TestSSO_NoAutoCreateTeam(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
var before int
s.db.QueryRow("SELECT COUNT(*) FROM teams").Scan(&before)
signInSSO(t, idp, ssoBrowser(t, s), alice) // groups include "sre"; no team names it
if got := s.memberships(t, "alice"); len(got) != 0 {
t.Errorf("memberships %v, want none: no team's oidc_member_group/oidc_owner_group is set", got)
}
var after int
s.db.QueryRow("SELECT COUNT(*) FROM teams").Scan(&after)
if after != before {
t.Errorf("team count %d -> %d, want no team created", before, after)
}
}
func TestSSO_RefusedOutsideAllowedGroups(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
@@ -362,6 +398,7 @@ func TestSSO_ManualAdminIsNeverRevoked(t *testing.T) {
func TestSSO_LosingAGroupRemovesOnlyManagedAccess(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
s.seedTeam(t, "SRE", "sre", "sre-leads")
signInSSO(t, idp, ssoBrowser(t, s), alice)
// Somebody adds alice to another team by hand.
@@ -379,6 +416,7 @@ func TestSSO_LosingAGroupRemovesOnlyManagedAccess(t *testing.T) {
func TestSSO_HighestRoleWinsAndRoleChangesFollow(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
s.seedTeam(t, "SRE", "sre", "sre-leads")
signInSSO(t, idp, ssoBrowser(t, s), withGroups(alice, "terdut-users", "sre", "sre-leads"))
if got := s.memberships(t, "alice"); !sameMap(got, map[string]string{"SRE": "owner/oidc"}) {
@@ -396,11 +434,11 @@ func TestSSO_ManualMemberIsRaisedNeverLowered(t *testing.T) {
// alice exists locally, is a manual owner of SRE, and is linked by email.
s.exec(t, "INSERT INTO users (username, email) VALUES ('alice', 'alice@example.com')")
s.exec(t, "INSERT INTO teams (name) VALUES ('SRE')")
s.seedTeam(t, "SRE", "sre", "")
s.exec(t, `INSERT INTO team_members (team_id, user_id, role)
VALUES ((SELECT id FROM teams WHERE name = 'SRE'), (SELECT id FROM users WHERE username = 'alice'), 'owner')`)
signInSSO(t, idp, ssoBrowser(t, s), alice) // the mapping only says member
signInSSO(t, idp, ssoBrowser(t, s), alice) // the group only grants member
if got := s.memberships(t, "alice"); !sameMap(got, map[string]string{"SRE": "owner/manual"}) {
t.Errorf("%v: a hand-made owner must not be lowered by a member mapping", got)
}
@@ -614,6 +652,7 @@ func TestSSO_ProviderErrorGoesBackToTheUI(t *testing.T) {
func TestSSO_ManagedAccessCannotBeEditedByHand(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
s.seedTeam(t, "SRE", "sre", "")
signInSSO(t, idp, ssoBrowser(t, s), withGroups(alice, "terdut-users", "sre", "terdut-admins"))
var aliceID, sreID int64
@@ -712,6 +751,7 @@ func TestSSO_UnreachableProviderRedirectsWithAnError(t *testing.T) {
func TestSSO_APIShowsWhereAccessCameFrom(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
s.seedTeam(t, "SRE", "sre", "")
b := ssoBrowser(t, s)
signInSSO(t, idp, b, withGroups(alice, "terdut-users", "sre", "terdut-admins"))
+5
View File
@@ -160,6 +160,11 @@ func NewRouter(db *sql.DB, notify NotifyConfig, cfg config.Config) http.Handler
r.Post("/api/teams/{teamID}/members", handleAddTeamMember(db))
r.Delete("/api/teams/{teamID}/members/{userID}", handleRemoveTeamMember(db))
// A team's own OIDC group binding: which provider groups grant member
// and owner access to it.
r.Get("/api/teams/{teamID}/oidc-groups", handleGetTeamOIDCGroups(db))
r.Put("/api/teams/{teamID}/oidc-groups", handleSetTeamOIDCGroups(db))
// Invite links into this team.
r.Get("/api/teams/{teamID}/invites", handleListInvites(db))
r.Post("/api/teams/{teamID}/invites", handleCreateInvite(db, notify.PublicURL))
+16 -4
View File
@@ -237,6 +237,14 @@ type adminTeam struct {
CreatedAt time.Time `json:"created_at"`
Members int64 `json:"members"`
OpenIncidents int64 `json:"open_incidents"`
// OIDCMemberGroup and OIDCOwnerGroup are the team's own group binding,
// read-only here: an administrator can see why a team's OIDC-sourced
// membership looks the way it does without being able to change it out
// from under the team's owner. Setting it is PUT
// /api/teams/{teamID}/oidc-groups, owner-only.
OIDCMemberGroup string `json:"oidc_member_group,omitempty"`
OIDCOwnerGroup string `json:"oidc_owner_group,omitempty"`
}
// handleAdminListTeams lists every team on the server, with its size. The
@@ -248,7 +256,8 @@ func handleAdminListTeams(db *sql.DB) http.HandlerFunc {
SELECT t.id, t.name, t.created_at,
(SELECT COUNT(*) FROM team_members m WHERE m.team_id = t.id),
(SELECT COUNT(*) FROM incidents i
WHERE i.team_id = t.id AND i.resolved_at IS NULL)
WHERE i.team_id = t.id AND i.resolved_at IS NULL),
COALESCE(t.oidc_member_group, ''), COALESCE(t.oidc_owner_group, '')
FROM teams t
ORDER BY t.name`)
if err != nil {
@@ -261,7 +270,8 @@ func handleAdminListTeams(db *sql.DB) http.HandlerFunc {
for rows.Next() {
var t adminTeam
var created int64
if err := rows.Scan(&t.ID, &t.Name, &created, &t.Members, &t.OpenIncidents); err != nil {
if err := rows.Scan(&t.ID, &t.Name, &created, &t.Members, &t.OpenIncidents,
&t.OIDCMemberGroup, &t.OIDCOwnerGroup); err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
}
@@ -298,10 +308,12 @@ func handleAdminGetTeam(db *sql.DB) http.HandlerFunc {
SELECT t.id, t.name, t.created_at,
(SELECT COUNT(*) FROM team_members m WHERE m.team_id = t.id),
(SELECT COUNT(*) FROM incidents i
WHERE i.team_id = t.id AND i.resolved_at IS NULL)
WHERE i.team_id = t.id AND i.resolved_at IS NULL),
COALESCE(t.oidc_member_group, ''), COALESCE(t.oidc_owner_group, '')
FROM teams t
WHERE t.id = $1`, teamID).
Scan(&t.ID, &t.Name, &created, &t.Members, &t.OpenIncidents)
Scan(&t.ID, &t.Name, &created, &t.Members, &t.OpenIncidents,
&t.OIDCMemberGroup, &t.OIDCOwnerGroup)
if errors.Is(err, sql.ErrNoRows) {
respond(w, http.StatusNotFound, errResp("not found"))
return
+106
View File
@@ -327,6 +327,112 @@ func TestTeams_MemberCannotConfigureTheTeam(t *testing.T) {
}
}
// A team's own OIDC group binding follows the same rule as its schedule and
// its integrations: an owner sets it, a member may only read it, an outsider
// learns nothing, and an administrator can still reach it to repair a team
// whose owner has left.
func TestTeamOIDCGroups_OwnerOnlyToEdit(t *testing.T) {
s := newTS(t)
team := newTeam(t, s, "sre") // team.call authenticates as its owner
// A plain member of the same team.
var plain struct {
ID int64 `json:"id"`
}
decode(t, s.req(t, http.MethodPost, "/api/users",
map[string]string{"username": "plain", "email": "plain@test.com"}), &plain)
resp := s.req(t, http.MethodPost, "/api/teams/"+id64(team.id)+"/members",
map[string]any{"user_id": plain.ID, "role": "member"})
resp.Body.Close()
var key struct {
Key string `json:"key"`
}
decode(t, s.req(t, http.MethodPost, "/api/users/"+id64(plain.ID)+"/api-keys",
map[string]string{"name": "test"}), &key)
memberCall := 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
}
// A member of a different team altogether.
_, outsiderCall := member(t, s, "outsider")
path := "/api/teams/" + id64(team.id) + "/oidc-groups"
resp = team.call(http.MethodPut, path, map[string]string{"member_group": "sre", "owner_group": "sre-leads"})
resp.Body.Close()
if resp.StatusCode != http.StatusNoContent {
t.Errorf("owner PUT: %d, want 204", resp.StatusCode)
}
var got struct {
MemberGroup string `json:"member_group"`
OwnerGroup string `json:"owner_group"`
}
decode(t, team.call(http.MethodGet, path, nil), &got)
if got.MemberGroup != "sre" || got.OwnerGroup != "sre-leads" {
t.Errorf("owner GET after PUT: %+v", got)
}
resp = memberCall(http.MethodGet, path, nil)
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Errorf("member GET: %d, want 200", resp.StatusCode)
}
resp = memberCall(http.MethodPut, path, map[string]string{"member_group": "anything"})
resp.Body.Close()
if resp.StatusCode != http.StatusForbidden {
t.Errorf("member PUT: %d, want 403", resp.StatusCode)
}
// 404, not 403: whether the team exists is itself something only its
// members should learn.
resp = outsiderCall(http.MethodGet, path, nil)
resp.Body.Close()
if resp.StatusCode != http.StatusNotFound {
t.Errorf("outsider GET: %d, want 404", resp.StatusCode)
}
resp = outsiderCall(http.MethodPut, path, map[string]string{"member_group": "anything"})
resp.Body.Close()
if resp.StatusCode != http.StatusNotFound {
t.Errorf("outsider PUT: %d, want 404", resp.StatusCode)
}
// An administrator who is not a member may still set it, the same bypass
// that lets one repair a team whose owner has left.
resp = s.req(t, http.MethodPut, path, map[string]string{"member_group": "sre2"})
resp.Body.Close()
if resp.StatusCode != http.StatusNoContent {
t.Errorf("admin PUT: %d, want 204", resp.StatusCode)
}
// An empty string clears a binding, stored as NULL rather than the literal
// empty string, so an empty group claim can never accidentally match it.
resp = team.call(http.MethodPut, path, map[string]string{"member_group": "", "owner_group": ""})
resp.Body.Close()
var cleared struct {
MemberGroup string `json:"member_group"`
OwnerGroup string `json:"owner_group"`
}
decode(t, team.call(http.MethodGet, path, nil), &cleared)
if cleared.MemberGroup != "" || cleared.OwnerGroup != "" {
t.Errorf("cleared: %+v", cleared)
}
}
// A team is not somewhere an outsider can look, whatever they know about it.
func TestTeams_OutsiderSeesNothing(t *testing.T) {
s := newTS(t)
+7 -35
View File
@@ -1,7 +1,6 @@
package config
import (
"encoding/json"
"errors"
"fmt"
"net/url"
@@ -112,25 +111,10 @@ type OIDC struct {
// AdminGroup grants the system administrator flag while the user is in it.
AdminGroup string
// GroupMappings grants team roles. A user in Group gets Role in Team.
GroupMappings []GroupMapping
// SessionMaxAge is the hard ceiling on a session made by an SSO login. The
// login is the only moment groups are re-read, so this is how long a change
// in the provider may take to reach terdut.
SessionMaxAge time.Duration
// parseErr is a malformed TERDUT_OIDC_GROUP_MAPPINGS, reported by Validate:
// Load cannot fail, and a mapping that was silently dropped would grant
// less access than the operator wrote down.
parseErr error
}
// GroupMapping grants Role in Team to members of Group.
type GroupMapping struct {
Group string `json:"group"`
Team string `json:"team"`
Role string `json:"role"`
}
// Enabled reports whether single sign-on is configured.
@@ -185,11 +169,6 @@ func loadOIDC() OIDC {
AdminGroup: os.Getenv("TERDUT_OIDC_ADMIN_GROUP"),
SessionMaxAge: duration("TERDUT_OIDC_SESSION_MAX_AGE", 12*time.Hour),
}
if raw := strings.TrimSpace(os.Getenv("TERDUT_OIDC_GROUP_MAPPINGS")); raw != "" {
if err := json.Unmarshal([]byte(raw), &o.GroupMappings); err != nil {
o.parseErr = fmt.Errorf("TERDUT_OIDC_GROUP_MAPPINGS: %w", err)
}
}
return o
}
@@ -199,14 +178,11 @@ func loadOIDC() OIDC {
// than not starting.
func (c Config) Validate() error {
o := c.OIDC
if o.parseErr != nil {
return o.parseErr
}
if !o.Enabled() {
if c.DisablePasswordLogin {
return errors.New("TERDUT_PASSWORD_LOGIN=false without TERDUT_OIDC_ISSUER leaves no way to sign in")
}
if len(o.GroupMappings) > 0 || o.AdminGroup != "" || len(o.AllowedGroups) > 0 {
if o.AdminGroup != "" || len(o.AllowedGroups) > 0 {
return errors.New("TERDUT_OIDC_* group settings are set but TERDUT_OIDC_ISSUER is not")
}
return nil
@@ -223,16 +199,12 @@ func (c Config) Validate() error {
if o.SessionMaxAge <= 0 {
return errors.New("TERDUT_OIDC_SESSION_MAX_AGE must be positive")
}
for i, m := range o.GroupMappings {
if m.Group == "" || m.Team == "" {
return fmt.Errorf("TERDUT_OIDC_GROUP_MAPPINGS[%d]: group and team are required", i)
}
if m.Role != "owner" && m.Role != "member" {
return fmt.Errorf("TERDUT_OIDC_GROUP_MAPPINGS[%d]: role must be owner or member, got %q", i, m.Role)
}
}
if c.DisablePasswordLogin && len(o.GroupMappings) == 0 && o.AdminGroup == "" {
return errors.New("TERDUT_PASSWORD_LOGIN=false with no OIDC group grants leaves nobody able to do anything")
// Team grants are no longer visible here: they live on each team's own
// oidc_member_group/oidc_owner_group columns, set by that team's owner, not
// in config Validate can see at startup. The one thing left to guard against
// is an install nobody can administer at all.
if c.DisablePasswordLogin && o.AdminGroup == "" {
return errors.New("TERDUT_PASSWORD_LOGIN=false with no TERDUT_OIDC_ADMIN_GROUP leaves nobody able to administer the install")
}
return nil
}
+1 -11
View File
@@ -28,16 +28,6 @@ func TestValidate(t *testing.T) {
{"missing secret", func(m map[string]string) { delete(m, "TERDUT_OIDC_CLIENT_SECRET") }, "CLIENT_SECRET"},
{"missing public url", func(m map[string]string) { delete(m, "TERDUT_PUBLIC_URL") }, "PUBLIC_URL"},
{"bad issuer", func(m map[string]string) { m["TERDUT_OIDC_ISSUER"] = "not a url" }, "not a URL"},
{"bad mapping json", func(m map[string]string) { m["TERDUT_OIDC_GROUP_MAPPINGS"] = "{nope" }, "GROUP_MAPPINGS"},
{"bad mapping role", func(m map[string]string) {
m["TERDUT_OIDC_GROUP_MAPPINGS"] = `[{"group":"g","team":"t","role":"admin"}]`
}, "role must be"},
{"mapping needs team", func(m map[string]string) {
m["TERDUT_OIDC_GROUP_MAPPINGS"] = `[{"group":"g","role":"member"}]`
}, "group and team"},
{"good mapping", func(m map[string]string) {
m["TERDUT_OIDC_GROUP_MAPPINGS"] = `[{"group":"g","team":"t","role":"owner"}]`
}, ""},
{"password off without sso", func(m map[string]string) {
clear(m)
m["TERDUT_PASSWORD_LOGIN"] = "false"
@@ -56,7 +46,7 @@ func TestValidate(t *testing.T) {
tt.env(env)
for _, k := range []string{
"TERDUT_PUBLIC_URL", "TERDUT_PASSWORD_LOGIN", "TERDUT_OIDC_ISSUER", "TERDUT_OIDC_CLIENT_ID",
"TERDUT_OIDC_CLIENT_SECRET", "TERDUT_OIDC_ADMIN_GROUP", "TERDUT_OIDC_GROUP_MAPPINGS",
"TERDUT_OIDC_CLIENT_SECRET", "TERDUT_OIDC_ADMIN_GROUP",
} {
t.Setenv(k, env[k])
}
@@ -0,0 +1,26 @@
-- Per-team OIDC group configuration, replacing the global
-- TERDUT_OIDC_GROUP_MAPPINGS env var.
--
-- Group -> team -> role used to be one global list an operator set for the
-- whole install, matched against a team by name, and the sync would create
-- the team if no team by that name existed yet. That put the decision of
-- which group controls a team in the server's environment rather than the
-- team's own hands, meant changing it needed an env var edit and a restart,
-- and let a typo in a team name silently create a stray team.
--
-- Each team now names, itself, which group grants membership and which
-- grants ownership. Nullable: most teams need neither. No uniqueness
-- constraint on either column — two teams may legitimately watch the same
-- provider group (a broad team and a narrower one both keyed off overlapping
-- groups is a choice for their owners to make, not one the schema should
-- refuse).
--
-- BREAKING CHANGE, deliberately not auto-migrated: TERDUT_OIDC_GROUP_MAPPINGS
-- stops being read as of this version, and the sync no longer creates a team
-- by name. Every team's group binding must be set again through
-- PUT /api/teams/{teamID}/oidc-groups. Until an owner does that, an
-- OIDC-sourced membership in that team is dropped at that user's next SSO
-- sign-in, the same way any other loss of group access is handled. See the
-- README's OIDC section.
ALTER TABLE teams ADD COLUMN oidc_member_group TEXT;
ALTER TABLE teams ADD COLUMN oidc_owner_group TEXT;
+40 -14
View File
@@ -18,7 +18,9 @@ const (
roleMember = "member"
)
// Grants is the access a set of groups confers.
// Grants is the account-wide access a set of groups confers. Team access is a
// separate question — see TeamGroup and ComputeTeamGrants — because it is
// configured per team in the database, not in this package's cfg.
type Grants struct {
// Admitted is false when AllowedGroups is set and the user is in none of
// them. Nothing else in the struct means anything then.
@@ -26,21 +28,16 @@ type Grants struct {
// Admin is whether the user is in the admin group.
Admin bool
// Teams maps team name to role. Where several groups grant the same team the
// highest role wins, so belonging to both a members group and an owners
// group makes somebody an owner rather than whichever mapping came last.
Teams map[string]string
}
// ComputeGrants evaluates the configured mappings against groups.
// ComputeGrants evaluates the account-wide configuration against groups.
func ComputeGrants(cfg config.OIDC, groups []string) Grants {
in := make(map[string]bool, len(groups))
for _, g := range groups {
in[g] = true
}
g := Grants{Teams: map[string]string{}}
var g Grants
g.Admitted = len(cfg.AllowedGroups) == 0
for _, allowed := range cfg.AllowedGroups {
@@ -54,16 +51,45 @@ func ComputeGrants(cfg config.OIDC, groups []string) Grants {
}
g.Admin = cfg.AdminGroup != "" && in[cfg.AdminGroup]
return g
}
for _, m := range cfg.GroupMappings {
if !in[m.Group] {
continue
// TeamGroup is one team's own OIDC binding: which group, if any, grants
// member access to it and which grants owner access, as read from
// teams.oidc_member_group / teams.oidc_owner_group.
type TeamGroup struct {
TeamID int64
MemberGroup string // "" means no group grants member access here.
OwnerGroup string // "" means no group grants owner access here.
}
// ComputeTeamGrants evaluates every team's own group binding against groups,
// and returns the role each team grants, keyed by team ID. A team absent from
// the result is not granted at all. Where a team's member and owner groups
// both match, the owner group wins — the same "highest role wins" rule that
// applied across the old global mapping list applies here across one team's
// two fields, so belonging to both groups makes somebody an owner rather than
// whichever field happened to be checked last.
func ComputeTeamGrants(teamGroups []TeamGroup, groups []string) map[int64]string {
in := make(map[string]bool, len(groups))
for _, g := range groups {
in[g] = true
}
out := map[int64]string{}
for _, tg := range teamGroups {
role := ""
if tg.MemberGroup != "" && in[tg.MemberGroup] {
role = roleMember
}
if rank(m.Role) > rank(g.Teams[m.Team]) {
g.Teams[m.Team] = m.Role
if tg.OwnerGroup != "" && in[tg.OwnerGroup] && rank(roleOwner) > rank(role) {
role = roleOwner
}
if role != "" {
out[tg.TeamID] = role
}
}
return g
return out
}
// rank orders roles; an unknown or absent role ranks lowest.
+76 -18
View File
@@ -11,11 +11,6 @@ func testCfg() config.OIDC {
return config.OIDC{
AllowedGroups: []string{"terdut-users"},
AdminGroup: "terdut-admins",
GroupMappings: []config.GroupMapping{
{Group: "sre", Team: "SRE", Role: "member"},
{Group: "sre-leads", Team: "SRE", Role: "owner"},
{Group: "platform", Team: "Platform", Role: "member"},
},
}
}
@@ -28,27 +23,17 @@ func TestComputeGrants(t *testing.T) {
{
name: "not in an allowed group is refused",
groups: []string{"sre", "terdut-admins"},
want: Grants{Admitted: false, Teams: map[string]string{}},
want: Grants{Admitted: false},
},
{
name: "allowed but no grants",
groups: []string{"terdut-users"},
want: Grants{Admitted: true, Teams: map[string]string{}},
want: Grants{Admitted: true},
},
{
name: "admin group grants admin",
groups: []string{"terdut-users", "terdut-admins"},
want: Grants{Admitted: true, Admin: true, Teams: map[string]string{}},
},
{
name: "team roles from several groups",
groups: []string{"terdut-users", "sre", "platform"},
want: Grants{Admitted: true, Teams: map[string]string{"SRE": "member", "Platform": "member"}},
},
{
name: "highest role wins whatever the order",
groups: []string{"sre-leads", "terdut-users", "sre"},
want: Grants{Admitted: true, Teams: map[string]string{"SRE": "owner"}},
want: Grants{Admitted: true, Admin: true},
},
}
for _, tt := range tests {
@@ -69,6 +54,79 @@ func TestComputeGrants_NoAllowedGroupsAdmitsEveryone(t *testing.T) {
}
}
// testTeamGroups is one SRE team keyed off two groups (a member group and a
// higher owner group) and one Platform team keyed off a member group only —
// the same shape the old global TERDUT_OIDC_GROUP_MAPPINGS example used.
func testTeamGroups() []TeamGroup {
return []TeamGroup{
{TeamID: 1, MemberGroup: "sre", OwnerGroup: "sre-leads"},
{TeamID: 2, MemberGroup: "platform"},
}
}
func TestComputeTeamGrants(t *testing.T) {
tests := []struct {
name string
teamGroups []TeamGroup
groups []string
want map[int64]string
}{
{
name: "no matching group grants nothing",
teamGroups: testTeamGroups(),
groups: []string{"terdut-users"},
want: map[int64]string{},
},
{
name: "member group grants member",
teamGroups: testTeamGroups(),
groups: []string{"sre"},
want: map[int64]string{1: roleMember},
},
{
name: "owner group grants owner",
teamGroups: testTeamGroups(),
groups: []string{"sre-leads"},
want: map[int64]string{1: roleOwner},
},
{
name: "in both of a team's groups, owner wins",
teamGroups: testTeamGroups(),
groups: []string{"sre", "sre-leads"},
want: map[int64]string{1: roleOwner},
},
{
name: "several teams from several groups",
teamGroups: testTeamGroups(),
groups: []string{"sre", "platform"},
want: map[int64]string{1: roleMember, 2: roleMember},
},
{
name: "two teams may share a group",
teamGroups: []TeamGroup{
{TeamID: 1, MemberGroup: "sre"},
{TeamID: 2, MemberGroup: "sre"},
},
groups: []string{"sre"},
want: map[int64]string{1: roleMember, 2: roleMember},
},
{
name: "a team with neither field set is never granted",
teamGroups: []TeamGroup{{TeamID: 1}},
groups: []string{"sre", "sre-leads", "platform"},
want: map[int64]string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := ComputeTeamGrants(tt.teamGroups, tt.groups)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("got %+v, want %+v", got, tt.want)
}
})
}
}
func TestStringList(t *testing.T) {
if got := stringList([]any{"a", "", 3, "b"}); !reflect.DeepEqual(got, []string{"a", "b"}) {
t.Errorf("list: %v", got)
+7
View File
@@ -146,6 +146,13 @@ function identityCard() {
fact('Created', when(t.created_at)),
fact('Members', String(t.members)),
fact('Open incidents', String(t.open_incidents)),
// Read-only here: an administrator can see why a team's OIDC-sourced
// membership looks the way it does, but setting it is the team's own
// owner's call, from the Team tab.
...(state.auth?.oidc?.enabled ? [
fact('OIDC member group', t.oidc_member_group || '—'),
fact('OIDC owner group', t.oidc_owner_group || '—'),
] : []),
),
form, err, ok,
);
+4
View File
@@ -135,6 +135,10 @@ export const addTeamMember = (id, userID, role) =>
call('POST', `/teams/${id}/members`, { body: { user_id: userID, role } });
export const removeTeamMember = (id, userID) => call('DELETE', `/teams/${id}/members/${userID}`);
// Which OIDC groups grant member and owner access to this team.
export const oidcGroups = (id) => call('GET', `/teams/${id}/oidc-groups`);
export const setOidcGroups = (id, body) => call('PUT', `/teams/${id}/oidc-groups`, { body });
export const integrations = (id) => call('GET', `/teams/${id}/integrations`);
export const createIntegration = (id, name) =>
call('POST', `/teams/${id}/integrations`, { body: { name } });
+75 -4
View File
@@ -101,8 +101,12 @@ async function load(id) {
return { members, schedule };
}
if (tab === 'members') {
const [members, users] = await Promise.all([api.teamMembers(id), allUsers()]);
return { members, users };
const [members, users, oidcGroups] = await Promise.all([
api.teamMembers(id),
allUsers(),
state.auth?.oidc?.enabled ? api.oidcGroups(id) : null,
]);
return { members, users, oidcGroups };
}
if (tab === 'escalation') {
const [members, escalation] = await Promise.all([api.teamMembers(id), api.escalation(id)]);
@@ -1046,6 +1050,73 @@ function shiftCell(m) {
: h('span', { text: 'not scheduled' });
}
// The team's own OIDC group binding, shown only on an SSO-enabled install:
// which group grants membership and which grants ownership. Read-only text
// for a member, an edit sheet for an owner — the server enforces the same
// split on the endpoint underneath.
function oidcGroupsCard() {
if (!state.auth?.oidc?.enabled) return null;
const g = data.oidcGroups || { member_group: '', owner_group: '' };
return h('div', { class: 'card' },
h('div', { class: 'card-head' },
h('h2', { text: 'Single sign-on' }),
isOwner() && h('button', {
class: 'btn', type: 'button', text: 'Edit', onclick: openOidcGroupsEditor,
})),
h('p', { class: 'muted small' },
'Members of the group below are added to this team automatically at ',
'sign-in; members of the owner group become owners. Leave a field ',
'blank to grant nothing this way.'),
h('dl', { class: 'user-facts' },
fact('Member group', g.member_group || '—'),
fact('Owner group', g.owner_group || '—'),
),
);
}
function fact(label, value) {
return [h('dt', { text: label }), h('dd', { text: value })];
}
function openOidcGroupsEditor() {
const g = data.oidcGroups || { member_group: '', owner_group: '' };
const memberGroup = h('input', {
type: 'text', value: g.member_group, placeholder: 'e.g. sre', autofocus: true,
});
const ownerGroup = h('input', { type: 'text', value: g.owner_group, placeholder: 'e.g. sre-leads' });
const problem = h('p', { class: 'load-error', hidden: true });
const form = h('form', { class: 'stacked-form' },
h('label', {}, 'Member group ', memberGroup),
h('label', {}, 'Owner group ', ownerGroup),
h('p', { class: 'muted small' },
'A person in both becomes an owner. Whoever the group lists is kept in ',
'sync at their next sign-in — a member added by hand can still be made ',
'an owner, but not the other way round.'),
problem,
h('div', { class: 'sheet-actions' },
h('button', { class: 'btn', type: 'button', text: 'Cancel', onclick: () => closeSheet(false) }),
h('button', { class: 'btn btn-primary', type: 'submit', text: 'Save' })));
form.addEventListener('submit', async (e) => {
e.preventDefault();
try {
await api.setOidcGroups(teamID, {
member_group: memberGroup.value.trim(),
owner_group: ownerGroup.value.trim(),
});
} catch (err) {
problem.textContent = err.message;
problem.hidden = false;
return;
}
closeSheet(true);
refresh();
});
openSheet(() => [h('h2', { class: 'sheet-title', text: 'Single sign-on groups' }), form]);
}
function membersCard() {
const members = data.members || [];
const owners = members.filter((m) => m.role === 'owner').length;
@@ -1090,7 +1161,7 @@ function membersCard() {
);
});
return h('div', { class: 'card' },
return [oidcGroupsCard(), h('div', { class: 'card' },
h('div', { class: 'card-head' },
h('h2', { text: 'Members' }),
isOwner() && h('button', {
@@ -1108,7 +1179,7 @@ function membersCard() {
h('th'))),
h('tbody', {}, rows)))
: h('p', { class: 'muted', text: 'Nobody is in this team.' }),
);
)];
}
// One sheet for both jobs a member's row has: who, and as what. Adding is