Compare commits

..

4 Commits

Author SHA1 Message Date
Niklas Ye 4b15079ac2 Set the chart's placeholder version to 0.33.1
CI / chart (push) Successful in 1s
CI / security (push) Successful in 19s
CI / test (push) Successful in 4m42s
Release / test (push) Successful in 3s
Release / chart (push) Successful in 2s
Release / image (push) Successful in 2m11s
Release / scan-image (push) Successful in 3s
Release / binaries (push) Successful in 2m47s
Cosmetic: `make helm-package` passes --version/--app-version from the
tag, so this field decides nothing about what gets published. Still done
so the tree doesn't say 0.33.0 while heading for a v0.33.1 release.
Cites fc9f47c, the fix this version actually is.
2026-10-02 09:18:28 +02:00
Niklas Ye fc9f47cc8d Refuse an OIDC sign-in from creating the very first user
Closes the race terdut-operator#1 found: /api/bootstrap and OIDC
auto-provisioning both key off the same signal (SELECT COUNT(*) FROM
users) with no coordination between them, so an otherwise-ordinary OIDC
sign-in against a freshly-created, not-yet-bootstrapped install could
create user #1 itself and take the one slot /api/bootstrap expects to win
uncontested (terdut-operator's own design, DESIGN.md §1/§6, assumes it is
the only caller). The operator has no way to recover from losing that
race -- it never gets a credential, and nothing it owns can clear the
occupying user row.

resolveSSOUser now checks the same gate handleBootstrap already does,
right where it's about to create a brand-new user (an identity nobody has
linked yet, that also matches no existing local account by email) -- not
anywhere else, since every other sign-in on an already-bootstrapped
install is unaffected. New sso_error code `not_bootstrapped`: the person
sees "this install is still setting up, try again in a moment" and a
second attempt once something has actually bootstrapped succeeds
normally, same as any other first sign-in.

Does not fix the other half of that issue (BootstrapStateLost's own
"delete and recreate" instructions still don't work once something has
occupied the slot some other way) -- this closes the specific race, not
every path to that state.
2026-10-02 09:12:32 +02:00
Niklas Ye bc9f793f1f Add GET /api/teams?name= (TEAM-LOOKUP.md)
CI / chart (push) Successful in 1s
CI / security (push) Successful in 1m15s
CI / test (push) Successful in 5m48s
Resolves the gap TEAM-LOOKUP.md raised: an instance-scoped service
account had no way to recover a team's id after a 409 on POST
/api/teams, unlike the already-solved equivalent for service accounts
themselves (GET /api/service-accounts?name=).

Same route, extended the same way handleListServiceAccounts already
branches on ?name=: unset behaves exactly as before (the caller's own
teams via team_members); set looks up one team by exact name, open to
any authenticated caller -- not gated by isInstanceServiceAccount or
AdminOnly, since what it discloses (a name is taken, nothing about who's
in it) is the same low sensitivity that lookup already accepts for
service-account names.

Tests cover the exact motivating scenario (create, 409 on a retry,
recover the id via ?name=), the empty-array-not-an-error case, that no
role/source is reported for a non-member match, and that a caller who
isn't a member of the matched team still gets it.
2026-10-01 10:49:11 +02:00
Niklas Ye 871274a3a0 Request: team lookup for service accounts (TEAM-LOOKUP.md)
Raised by terdut-operator's TerdutTeam controller (ROADMAP.md Stage 2):
an instance-scoped service account has no way to recover a team's id
after a 409 on POST /api/teams, unlike the equivalent, already-solved
case for service accounts themselves (GET /api/service-accounts?name=).
Also corrects a claim in SERVICE-ACCOUNTS.md's own text that doesn't
match AdminOnly's actual code -- confirmed against source, not assumed.
2026-10-01 10:43:09 +02:00
8 changed files with 251 additions and 7 deletions
+1 -1
View File
@@ -793,7 +793,7 @@ attempted.
| `POST` | `/api/oidc/device/token` | `{"device_code"}` → `202 {"status":"pending"}`, then `200` with the session cookie once approved (once only). `410` with `{"error":"expired"}` or `{"error":"denied"}`; `429 {"error":"slow_down"}` if polled faster than `interval` |
| `POST` | `/api/oidc/device/approve` | **session** — `{"user_code"}`. Approves a pending device login as the caller. `403` for an API key; `404` for an unknown, expired or already decided code |
| `POST` | `/api/oidc/device/deny` | **session** — `{"user_code"}`. Refuses it |
| `GET` | `/api/oidc/callback` | Where the provider sends the browser back. Sets the session cookie and redirects to `/`, or to `/?sso_error=<code>` — one of `denied`, `expired`, `failed`, `unavailable`, `not_allowed`, `no_email`, `email_conflict`, `disabled` |
| `GET` | `/api/oidc/callback` | Where the provider sends the browser back. Sets the session cookie and redirects to `/`, or to `/?sso_error=<code>` — one of `denied`, `expired`, `failed`, `unavailable`, `not_allowed`, `no_email`, `email_conflict`, `disabled`, `not_bootstrapped` (no user exists on this install yet — sign in again once something has called `/api/bootstrap`) |
| `POST` | `/api/logout` | Ends the session and clears the cookie |
| `GET` | `/api/me` | The caller: `{user, has_password}` |
+73
View File
@@ -0,0 +1,73 @@
# Team lookup for service accounts: closing terdut-operator's create-path crash window
This is a design note for a feature, not an implementation plan — same posture as
`SERVICE-ACCOUNTS.md`, and raised for the same reason: `terdut-operator`'s `TerdutTeam`
controller (ROADMAP.md Stage 2, a separate repo, no shared code) hit a gap this server has
no answer for yet.
## The problem
`POST /api/teams` (`handleCreateTeam`, confirmed against `internal/api/teams.go`) lets an
instance-scoped service account create a team — it has its own explicit
`isInstanceServiceAccount(...)` branch alongside the human-user path, not gated by
`AdminOnly`. If that call succeeds server-side but the caller (`TerdutTeam`'s controller)
crashes before persisting the resulting team ID locally, a retry's `POST` 409s on the name's
unique constraint (confirmed: the `isUniqueViolation` branch in the same handler).
Recovering from that 409 means looking the team up by name, and nothing today permits that
for a service account:
- `GET /api/teams` (`handleListTeams`) answers "what teams does the *caller* belong to", via
a `team_members` join keyed on `userFromContext`'s `caller.ID` — confirmed against source.
A service account is never a member of anything, so this always returns empty for one,
regardless of what exists.
- `GET /api/admin/teams` (`handleAdminListTeams`) is gated by `AdminOnly`, and `AdminOnly`'s
actual code (`internal/api/middleware.go`) checks only `userFromContext(...).IsAdmin` — no
branch for a service account at all, confirmed against source. This contradicts
`SERVICE-ACCOUNTS.md`'s own text, which claims "an instance-scoped [service account
satisfies] `AdminOnly` for team-creation/listing purposes" — that claim doesn't match this
endpoint's actual, shipped code. (Team *creation* is fine: `handleCreateTeam` isn't behind
`AdminOnly` at all, it has its own check. Only the listing half of that sentence is wrong.)
This is exactly the shape of gap `SERVICE-ACCOUNTS.md`'s own `GET /api/service-accounts?name=`
closed for service accounts themselves (confirmed: that endpoint's own comment —
"the name lookup is open to any authenticated caller... what lets a service account find its
own account on the 403 that follows a second POST"). Teams never got the equivalent, because
nothing needed it until an operator started creating them unattended.
## Goals
- A service-account-accessible way to look up one team by exact name, mirroring
`GET /api/service-accounts?name=` as closely as possible — same shape, same reasoning,
same low sensitivity of what it discloses.
- No change to today's behavior for an empty/no-name request.
## Proposed shape
Extend `GET /api/teams` itself, the same way `handleListServiceAccounts` already branches on
a `?name=` query param, rather than adding a new route:
- `name` unset (today's behavior, unchanged): the caller's own teams, via `team_members`.
- `name=<value>` set: look up that one team by exact name — a one-or-zero-length array, not
an error on no match, mirroring `GET /api/service-accounts?name=`'s own response shape and
status codes exactly. Deliberately **not** gated by `isInstanceServiceAccount` or
`AdminOnly`: a human caller who's already a member sees this same information in their own
team list regardless, and a non-member learning only that a name is taken — not who's in
the team, not any of its data — is the same low-sensitivity disclosure
`GET /api/service-accounts?name=` already accepts for service-account names.
## What this unblocks
Directly resolves the crash-window gap in `terdut-operator`'s `TerdutTeam` controller: on a
409 from `POST /api/teams`, `GET /api/teams?name=<the same name>` — authenticated with the
same instance-scoped credential that just got the 409 — finds the id, and the controller
proceeds as if its own create had returned it directly. The same adopt-on-409 pattern already
proven for service accounts (that repo's `DESIGN.md` §6 point 1, §5's general rule), not a
new one.
## Suggested sequencing
Land this before `TerdutTeam`'s create path is implemented — the same reasoning
`SERVICE-ACCOUNTS.md` gave for its own sequencing: writing that code against today's gap as a
"known-temporary workaround" is wasted effort when the fix is this small and this
well-precedented.
+2 -2
View File
@@ -15,5 +15,5 @@ type: application
# appVersion and image.tag in values.yaml no longer agree, and that is not an oversight:
# image.tag stays "latest", which is what a local install actually pulls. appVersion is
# metadata and drives nothing.
version: 0.33.0
appVersion: "v0.33.0"
version: 0.33.1
appVersion: "v0.33.1"
+25
View File
@@ -48,6 +48,17 @@ const (
ssoNoEmail ssoError = "no_email" // the provider sent no email address
ssoEmailConflict ssoError = "email_conflict" // a local account has this email and cannot be linked
ssoDisabled ssoError = "disabled" // the linked account is disabled
// ssoNotBootstrapped: this identity has no existing account, and no user
// exists on this install yet either -- creating one here would race
// POST /api/bootstrap for the one gitops-managed installs expect to win
// it (terdut-operator's own DESIGN.md §1, §6), which has no way to
// recover if it loses. The person sees this for at most as long as it
// takes whatever is bootstrapping this install to finish; signing in
// again afterward hits the ordinary first-sign-in path. Found by
// terdut-operator#1: nothing stopped an otherwise-ordinary OIDC sign-in
// from quietly winning this race against an operator that assumed it
// was the only caller.
ssoNotBootstrapped ssoError = "not_bootstrapped"
)
// handleAuthConfig says how this server can be signed in to, so the login form
@@ -366,6 +377,20 @@ func resolveSSOUser(ctx context.Context, tx *sql.Tx, cfg config.OIDC, id *oidc.I
return 0, ssoEmailConflict
}
case errors.Is(err, sql.ErrNoRows):
// Creating the very first user is /api/bootstrap's own job (same
// gate, same table: SELECT COUNT(*) FROM users in handleBootstrap).
// An identity nobody has linked yet, on an install with no users at
// all, is exactly the race terdut-operator#1 found: whoever gets
// here first wins a slot the other side has no way to recover from
// losing. Refusing it here costs an otherwise-ordinary sign-in
// nothing but a retry once bootstrap has actually run.
var userCount int
if err := tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM users").Scan(&userCount); err != nil {
return 0, err
}
if userCount == 0 {
return 0, ssoNotBootstrapped
}
userID, err = createSSOUser(ctx, tx, id)
if err != nil {
return 0, err
+39
View File
@@ -543,6 +543,45 @@ func TestSSO_DisabledUserIsRefused(t *testing.T) {
}
}
// TestSSO_FirstUserIsRefusedUntilBootstrap is terdut-operator#1: an
// otherwise-ordinary OIDC sign-in against a brand-new, not-yet-bootstrapped
// install must not be allowed to create the first user and win the race
// POST /api/bootstrap expects to win uncontested. Built directly over
// api.NewRouter rather than newSSOTS/newTS, both of which bootstrap before
// a test body ever runs -- exactly the state this test needs to not have yet.
func TestSSO_FirstUserIsRefusedUntilBootstrap(t *testing.T) {
idp := newFakeIdP(t)
database := newTestDB(t)
srv := httptest.NewServer(api.NewRouter(database, api.NotifyConfig{PublicURL: "http://terdut.test"}, ssoConfig(idp), "test"))
t.Cleanup(srv.Close)
first := newBrowser(t, srv.URL)
first.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }
if loc := signInSSO(t, idp, first, alice); loc != "/?sso_error=not_bootstrapped" {
t.Fatalf("sent to %q, want not_bootstrapped", loc)
}
// Bootstrap the install for real, the way terdut-operator's own
// reconcileBootstrap does.
resp, err := http.Post(srv.URL+"/api/bootstrap", "application/json",
strings.NewReader(`{"username":"admin","email":"admin@test.com"}`))
if err != nil {
t.Fatal(err)
}
resp.Body.Close()
if resp.StatusCode != http.StatusCreated {
t.Fatalf("bootstrap: %d", resp.StatusCode)
}
// The same identity, signing in again, is this install's ordinary first
// SSO user now -- no longer refused.
second := newBrowser(t, srv.URL)
second.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }
if loc := signInSSO(t, idp, second, alice); loc != "/" {
t.Errorf("sent to %q after bootstrap, want success", loc)
}
}
func TestSSO_NoEmailIsRefused(t *testing.T) {
idp := newFakeIdP(t)
s := newSSOTS(t, idp)
+36 -4
View File
@@ -13,12 +13,19 @@ import (
"github.com/go-chi/chi/v5"
)
// handleListTeams lists the caller's own teams, each with their role in it. An
// administrator listing every team goes through the admin endpoint instead:
// this one answers "what am I part of", which is what the UI's team filter and
// the combined queue are built from.
// handleListTeams lists the caller's own teams, each with their role in it,
// or — with ?name= — looks up one team by exact name regardless of caller
// identity (TEAM-LOOKUP.md). An administrator listing every team goes
// through the admin endpoint instead: the no-name case here answers "what am
// I part of", which is what the UI's team filter and the combined queue are
// built from.
func handleListTeams(db *sql.DB) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
if name := strings.TrimSpace(r.URL.Query().Get("name")); name != "" {
handleListTeamsByName(db, w, r, name)
return
}
caller, _ := userFromContext(r.Context())
rows, err := db.QueryContext(r.Context(), `
SELECT t.id, t.name, t.created_at, m.role, m.source
@@ -51,6 +58,31 @@ func handleListTeams(db *sql.DB) http.HandlerFunc {
}
}
// handleListTeamsByName answers "is there a team named exactly this", open to
// any authenticated caller including a service account (TEAM-LOOKUP.md) —
// mirrors handleListServiceAccounts' own ?name= lookup: a one-or-zero-length
// array, never an error on no match, and no caller-identity filtering at
// all, since what it discloses (a name is taken, nothing about who's in it
// or any of its data) is the same low sensitivity that lookup already
// accepts for service-account names.
func handleListTeamsByName(db *sql.DB, w http.ResponseWriter, r *http.Request, name string) {
var t models.Team
var created int64
err := db.QueryRowContext(r.Context(),
"SELECT id, name, created_at FROM teams WHERE name = $1", name,
).Scan(&t.ID, &t.Name, &created)
if errors.Is(err, sql.ErrNoRows) {
respond(w, http.StatusOK, []models.Team{})
return
}
if err != nil {
respond(w, http.StatusInternalServerError, errResp("internal error"))
return
}
t.CreatedAt = time.Unix(created, 0).UTC()
respond(w, http.StatusOK, []models.Team{t})
}
// handleUserTeams lists one user's teams, for the admin page's per-user view:
// "what is this person in", which /api/teams cannot answer because it is always
// about the caller.
+74
View File
@@ -6,6 +6,8 @@ import (
"io"
"net/http"
"testing"
"git.ryuvia.com/niklas/terdut-server/internal/models"
)
// The whole point of #4: two teams sharing one server must not see each other's
@@ -454,3 +456,75 @@ func TestTeams_OutsiderSeesNothing(t *testing.T) {
t.Errorf("blue's team list: %v", teams)
}
}
// ---------------------------------------------------------------------------
// GET /api/teams?name= (TEAM-LOOKUP.md)
// ---------------------------------------------------------------------------
func TestListTeamsByName_FindsExactMatch(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
teamID := createTeamAs(t, s, instanceKey, "platform")
teams := list(t, s.reqAs(t, instanceKey, http.MethodGet, "/api/teams?name=platform", nil))
if len(teams) != 1 {
t.Fatalf("expected exactly one match for ?name=platform, got %d: %v", len(teams), teams)
}
if int64(teams[0]["id"].(float64)) != teamID {
t.Errorf("id = %v, want %d", teams[0]["id"], teamID)
}
// No membership, so no role to report (models.Team's own doc comment:
// "empty when nobody in particular is asking").
if _, has := teams[0]["role"]; has {
t.Errorf("expected no role on a name-lookup match, got %v", teams[0]["role"])
}
}
func TestListTeamsByName_NoMatchIsAnEmptyArrayNotAnError(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
resp := s.reqAs(t, instanceKey, http.MethodGet, "/api/teams?name=does-not-exist", nil)
if resp.StatusCode != http.StatusOK {
t.Fatalf("expected 200 on no match, got %d", resp.StatusCode)
}
teams := list(t, resp)
if len(teams) != 0 {
t.Errorf("expected an empty array, got %v", teams)
}
}
// The actual motivating scenario (TEAM-LOOKUP.md): a service account that
// already created a team, interrupted before it could remember the id,
// recovers it via ?name= on the same name its own POST 409s on.
func TestListTeamsByName_RecoversAfterCreateConflict(t *testing.T) {
s := newTS(t)
instanceKey := createServiceAccount(t, s, s.key, "terdut-operator", models.ServiceAccountScopeInstance, 0)
original := createTeamAs(t, s, instanceKey, "recovered")
conflict := s.reqAs(t, instanceKey, http.MethodPost, "/api/teams", map[string]string{"name": "recovered"})
if conflict.StatusCode != http.StatusConflict {
t.Fatalf("expected 409 recreating the same name, got %d", conflict.StatusCode)
}
conflict.Body.Close()
teams := list(t, s.reqAs(t, instanceKey, http.MethodGet, "/api/teams?name=recovered", nil))
if len(teams) != 1 || int64(teams[0]["id"].(float64)) != original {
t.Fatalf("expected to recover the original team %d via ?name=, got %v", original, teams)
}
}
// Not gated by isInstanceServiceAccount or AdminOnly (TEAM-LOOKUP.md): any
// authenticated caller may ask whether a name is taken, the same low
// sensitivity GET /api/service-accounts?name= already accepts.
func TestListTeamsByName_OpenToAnyAuthenticatedCaller(t *testing.T) {
s := newTS(t)
red := newTeam(t, s, "red")
_ = createTeamAs(t, s, s.key, "blue-target")
// red's own member, not a member of "blue-target", still gets a match.
teams := list(t, red.call(http.MethodGet, "/api/teams?name=blue-target", nil))
if len(teams) != 1 || teams[0]["name"] != "blue-target" {
t.Errorf("expected a non-member caller to still find the team by name, got %v", teams)
}
}
+1
View File
@@ -355,6 +355,7 @@ function ssoErrorText(code, name) {
no_email: `${sso} did not send an email address for you, which terdut needs.`,
email_conflict: 'An account with your email address already exists and could not be linked to this sign-in. Ask an administrator.',
disabled: 'Your account is disabled. Ask an administrator.',
not_bootstrapped: 'This install is still setting up. Try again in a moment.',
}[code] || `Signing in with ${sso} failed.`;
}