From 5b4683febf68c5e1438f6e8265e7a4a32d981b62 Mon Sep 17 00:00:00 2001 From: Niklas Ye Date: Sun, 27 Sep 2026 11:43:57 +0200 Subject: [PATCH] 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. --- README.md | 30 +++-- .../terdut-server/templates/deployment.yaml | 4 - charts/terdut-server/values.yaml | 10 +- internal/api/oidc.go | 56 ++++++--- internal/api/oidc_teams.go | 74 ++++++++++++ internal/api/oidc_test.go | 60 ++++++++-- internal/api/router.go | 5 + internal/api/settings.go | 20 +++- internal/api/teams_test.go | 106 ++++++++++++++++++ internal/config/config.go | 42 ++----- internal/config/config_test.go | 12 +- .../db/migrations/013_oidc_team_groups.sql | 26 +++++ internal/oidc/grants.go | 54 ++++++--- internal/oidc/grants_test.go | 94 +++++++++++++--- internal/web/static/js/adminteam.js | 7 ++ internal/web/static/js/api.js | 4 + internal/web/static/js/team.js | 79 ++++++++++++- 17 files changed, 554 insertions(+), 129 deletions(-) create mode 100644 internal/api/oidc_teams.go create mode 100644 internal/db/migrations/013_oidc_team_groups.sql diff --git a/README.md b/README.md index 7845f1e..2e6c504 100644 --- a/README.md +++ b/README.md @@ -152,9 +152,14 @@ TERDUT_OIDC_CLIENT_ID=terdut TERDUT_OIDC_CLIENT_SECRET=... TERDUT_OIDC_ALLOWED_GROUPS=terdut-users,terdut-admins TERDUT_OIDC_ADMIN_GROUP=terdut-admins -TERDUT_OIDC_GROUP_MAPPINGS='[{"group":"sre","team":"SRE","role":"member"},{"group":"sre-leads","team":"SRE","role":"owner"}]' ``` +Which team a group grants is not server-wide config: each team names its own +group(s), set by that team's own owner (or an administrator) from its Members +tab, or `PUT /api/teams/{teamID}/oidc-groups {"member_group":"sre","owner_group":"sre-leads"}`. +A team must already exist before a group can grant access to it — the sync +never creates one. + The web UI's sign-in page shows a "Sign in with " button (a plain link to `/api/oidc/login`) above the password form, or instead of it when `TERDUT_PASSWORD_LOGIN=false`; it asks `GET /api/auth/config` what the server offers @@ -175,8 +180,8 @@ Account page does not offer to set a password nobody could use. 2. *Whether.* With `TERDUT_OIDC_ALLOWED_GROUPS` set, somebody in none of them is refused and nothing is created. 3. *What.* The administrator flag follows `TERDUT_OIDC_ADMIN_GROUP`. Team roles - follow the mappings; where several groups grant the same team the highest role - wins. + follow each team's own `oidc_member_group`/`oidc_owner_group`; where both of a + team's groups match, the owner group wins. **Managed access.** What the sync grants is marked as managed by single sign-on, and only that is ever changed by it. It is added at sign-in, and removed at the @@ -184,10 +189,18 @@ next sign-in after the group is gone, even if that leaves a team without an owne (an administrator can always repair a team) — the provider is the source of truth for what it grants, so the last-owner and last-administrator guards do not apply. Memberships and administrators added by hand are left alone; the exception is a -hand-added member whose mapping grants a *higher* role, who is raised and from then -on managed. Editing managed access by hand (`POST` or `DELETE` on a team's -members, revoking an SSO-granted administrator) is refused with `409`, since the -next sign-in would undo it. +hand-added member whose team's own group grants a *higher* role, who is raised and +from then on managed. Editing managed access by hand (`POST` or `DELETE` on a +team's members, revoking an SSO-granted administrator) is refused with `409`, since +the next sign-in would undo it. + +> **Upgrading past migration 013: reconfigure every team's groups.** +> `TERDUT_OIDC_GROUP_MAPPINGS` is gone, and the sync no longer creates a team by +> name. Group-to-team-role mapping is now each team's own setting — an owner sets +> it from the Members tab, or `PUT /api/teams/{teamID}/oidc-groups`. Until a team's +> owner does that, an OIDC-sourced membership in it is dropped at that user's next +> SSO sign-in, the same as any other loss of group access. Set every team's groups +> before affected users next sign in, to avoid a visible gap in access. **How fast changes arrive.** Groups are read only at sign-in. A session made by an SSO sign-in has a hard ceiling (`TERDUT_OIDC_SESSION_MAX_AGE`, default 12h) that @@ -329,7 +342,6 @@ over an administrator's edit. | `TERDUT_OIDC_TRUST_EMAIL` | `false` | Link a first sign-in to an existing local user by email even if the provider does not mark the address verified | | `TERDUT_OIDC_ALLOWED_GROUPS` | — | Comma-separated. Only people in one of these may sign in. Empty admits everybody the provider authenticates | | `TERDUT_OIDC_ADMIN_GROUP` | — | Members are system administrators | -| `TERDUT_OIDC_GROUP_MAPPINGS` | — | JSON list of `{"group","team","role"}` (`role` is `owner` or `member`). Teams that do not exist are created | | `TERDUT_OIDC_SESSION_MAX_AGE` | `12h` | Hard ceiling on a session made by an SSO sign-in | Durations use Go syntax (`30m`, `12h`, `168h`). An unparseable value falls back to the default. @@ -829,6 +841,8 @@ administrator who is not in the team gets the same `404` as anybody else. | `GET` | `/api/teams/{teamID}/members` | member | Who is in the team, with `status` (`oncall` if the rota has them today, `unpageable` when a page to them would go nowhere — even if they are on call — else `reachable`), `on_call`, `next_shift` (first rota day after today), `pageable` and `problem` (`has no ntfy topic` / `account is disabled`; never the topic itself) and `last_active_at` (their newest session or API-key use). Every member sees the same list | | `POST` | `/api/teams/{teamID}/members` | **owner** | Add a member, or change their role `{"user_id","role"}`. `409` when it would demote the last owner, or the membership is managed by single sign-on | | `DELETE` | `/api/teams/{teamID}/members/{userID}` | **owner** | Remove a member. `409` for the last owner, or a membership managed by single sign-on | +| `GET` | `/api/teams/{teamID}/oidc-groups` | member | Which groups control this team's membership: `{"member_group","owner_group"}`. An empty string means no group grants that role here | +| `PUT` | `/api/teams/{teamID}/oidc-groups` | **owner** | Set them. An empty string clears a binding | | `GET` | `/api/teams/{teamID}/integrations` | member | List integrations. Never returns keys. Each carries `status` (`active` if its key posted within 24h, `quiet` if it has but not lately, `never`), `last_used_at` (last webhook, usable or not), `last_alert_at` (when an alert last arrived on it) and `alerts_24h` (distinct alerts it refreshed in the last day). Alerts delivered before the source was recorded (migration 010) have none, so the last two fill in as Alertmanager re-sends them | | `PATCH` | `/api/teams/{teamID}/integrations/{integrationID}` | **owner** | Rename `{"name"}`. The key does not change | | `POST` | `/api/teams/{teamID}/integrations` | **owner** | Mint an integration `{"name","kind"}` — key and URL shown once | diff --git a/charts/terdut-server/templates/deployment.yaml b/charts/terdut-server/templates/deployment.yaml index 03015bc..1d49ad6 100644 --- a/charts/terdut-server/templates/deployment.yaml +++ b/charts/terdut-server/templates/deployment.yaml @@ -107,10 +107,6 @@ spec: - name: TERDUT_OIDC_ADMIN_GROUP value: {{ .Values.oidc.adminGroup | quote }} {{- end }} - {{- if .Values.oidc.groupMappings }} - - name: TERDUT_OIDC_GROUP_MAPPINGS - value: {{ .Values.oidc.groupMappings | toJson | quote }} - {{- end }} {{- end }} livenessProbe: httpGet: diff --git a/charts/terdut-server/values.yaml b/charts/terdut-server/values.yaml index d2c1ddb..e9b0e68 100644 --- a/charts/terdut-server/values.yaml +++ b/charts/terdut-server/values.yaml @@ -148,12 +148,10 @@ oidc: allowedGroups: [] # Members of this group are system administrators. adminGroup: "" - # Team roles from groups. A team that does not exist is created. Where several - # groups grant the same team the highest role wins. - # - group: sre - # team: SRE - # role: member # member or owner - groupMappings: [] + # Which group grants a team's membership and ownership is each team's own + # setting now, not chart config: an owner sets it from the Members tab, or + # PUT /api/teams/{teamID}/oidc-groups. A team must already exist for a group + # to grant access to it. # Hard ceiling on a session made by a single sign-on login. sessionMaxAge: 12h diff --git a/internal/api/oidc.go b/internal/api/oidc.go index 45deb5a..0e5b306 100644 --- a/internal/api/oidc.go +++ b/internal/api/oidc.go @@ -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') diff --git a/internal/api/oidc_teams.go b/internal/api/oidc_teams.go new file mode 100644 index 0000000..7c8fb18 --- /dev/null +++ b/internal/api/oidc_teams.go @@ -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) + } +} diff --git a/internal/api/oidc_test.go b/internal/api/oidc_test.go index 243dc00..ef8343a 100644 --- a/internal/api/oidc_test.go +++ b/internal/api/oidc_test.go @@ -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")) diff --git a/internal/api/router.go b/internal/api/router.go index dbd1f49..4a482b6 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -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)) diff --git a/internal/api/settings.go b/internal/api/settings.go index b5bb401..f736c5f 100644 --- a/internal/api/settings.go +++ b/internal/api/settings.go @@ -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 diff --git a/internal/api/teams_test.go b/internal/api/teams_test.go index 1a49d37..d8d317a 100644 --- a/internal/api/teams_test.go +++ b/internal/api/teams_test.go @@ -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) diff --git a/internal/config/config.go b/internal/config/config.go index 51f7415..cfffdd7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -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 } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b642053..7919636 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -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]) } diff --git a/internal/db/migrations/013_oidc_team_groups.sql b/internal/db/migrations/013_oidc_team_groups.sql new file mode 100644 index 0000000..d269402 --- /dev/null +++ b/internal/db/migrations/013_oidc_team_groups.sql @@ -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; diff --git a/internal/oidc/grants.go b/internal/oidc/grants.go index 5ac5459..2a67a65 100644 --- a/internal/oidc/grants.go +++ b/internal/oidc/grants.go @@ -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. diff --git a/internal/oidc/grants_test.go b/internal/oidc/grants_test.go index c5dffa5..68f814f 100644 --- a/internal/oidc/grants_test.go +++ b/internal/oidc/grants_test.go @@ -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) diff --git a/internal/web/static/js/adminteam.js b/internal/web/static/js/adminteam.js index 6336a88..2261575 100644 --- a/internal/web/static/js/adminteam.js +++ b/internal/web/static/js/adminteam.js @@ -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, ); diff --git a/internal/web/static/js/api.js b/internal/web/static/js/api.js index ed5f349..1aa8fc6 100644 --- a/internal/web/static/js/api.js +++ b/internal/web/static/js/api.js @@ -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 } }); diff --git a/internal/web/static/js/team.js b/internal/web/static/js/team.js index d6e6edc..e2e70cb 100644 --- a/internal/web/static/js/team.js +++ b/internal/web/static/js/team.js @@ -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