refactor(api): drop dead login concurrency limiter and reconcile passwordless comments
The passwordless migration (b330d77) removed the password-login route, leaving concurrencyLimiter — its bcrypt concurrency cap — with no caller, and scattered stale "local-password" / "change-password" references through the surviving auth code's comments. - Remove the dead concurrencyLimiter (type + newConcurrencyLimiter + acquire): no caller, no struct field, no test. Reword the one streamLimiter doc that contrasted against it. - Realign comments in repo.go, pgrepo.go, session.go, util.go to the passwordless reality: staff lookups feed email-OTP / passkey / setup redeem, not a password compare; RevokeUserSessionsExcept and DeleteAllPasskeyCredentialsForUser are retained (uncalled) for the P5 account-remediation path (#78); "local sessions" no longer implies a password. Comments and dead code only; no behavior change. Full WSL test tree green.
This commit is contained in:
5 files changed
+44
-78
No files matched your search
+2
-42
@@ -622,54 +622,14 @@ func (c *cooldownLimiter) release(name string, reservedAt time.Time) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---- login concurrency cap ----
|
|
||||||
|
|
||||||
// concurrencyLimiter bounds how many holders may run a guarded section at once. It
|
|
||||||
// backs the public login route's bcrypt cap (handleLogin): a buffered channel of n
|
|
||||||
// tokens; acquire takes one WITHOUT blocking (returning ok=false when the section
|
|
||||||
// is already full), release returns it. Unlike cooldownLimiter — a per-key time
|
|
||||||
// window — this bounds simultaneity, not frequency, which is the right shape for a
|
|
||||||
// CPU-costly section a flood would otherwise pin every core running. A non-positive
|
|
||||||
// cap disables it (acquire always admits, release is a no-op), mirroring the "zero
|
|
||||||
// disables" idiom of WakeCooldown and MaxRunningServers.
|
|
||||||
type concurrencyLimiter struct {
|
|
||||||
slots chan struct{}
|
|
||||||
}
|
|
||||||
|
|
||||||
// newConcurrencyLimiter builds a limiter admitting at most n concurrent holders. A
|
|
||||||
// non-positive n yields a disabled limiter (nil slots) that admits everyone.
|
|
||||||
func newConcurrencyLimiter(n int) *concurrencyLimiter {
|
|
||||||
if n <= 0 {
|
|
||||||
return &concurrencyLimiter{}
|
|
||||||
}
|
|
||||||
return &concurrencyLimiter{slots: make(chan struct{}, n)}
|
|
||||||
}
|
|
||||||
|
|
||||||
// acquire tries to take a slot without blocking. It returns a release func and true
|
|
||||||
// on success, or nil and false when the section is already at capacity. The disabled
|
|
||||||
// limiter (nil slots) always admits and returns a no-op release. release MUST be
|
|
||||||
// called exactly once on the success path, so it reads naturally as `release, ok :=
|
|
||||||
// l.acquire(); if !ok { shed }; defer/inline release()`.
|
|
||||||
func (l *concurrencyLimiter) acquire() (release func(), ok bool) {
|
|
||||||
if l.slots == nil {
|
|
||||||
return func() {}, true
|
|
||||||
}
|
|
||||||
select {
|
|
||||||
case l.slots <- struct{}{}:
|
|
||||||
return func() { <-l.slots }, true
|
|
||||||
default:
|
|
||||||
return nil, false
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// ---- per-principal stream cap ----
|
// ---- per-principal stream cap ----
|
||||||
|
|
||||||
// streamLimiter bounds how many concurrent guarded sections a single KEY may hold at
|
// streamLimiter bounds how many concurrent guarded sections a single KEY may hold at
|
||||||
// once. It backs the per-principal SSE stream cap (console + build-log relays): each
|
// once. It backs the per-principal SSE stream cap (console + build-log relays): each
|
||||||
// relay blocks for the life of a client's attachment and, under a stalled reader,
|
// relay blocks for the life of a client's attachment and, under a stalled reader,
|
||||||
// pins a goroutine plus a kube-apiserver follow connection, so an unbounded number of
|
// pins a goroutine plus a kube-apiserver follow connection, so an unbounded number of
|
||||||
// them from one principal is a control-plane connection-exhaustion vector. Unlike the
|
// them from one principal is a control-plane connection-exhaustion vector. Unlike a
|
||||||
// login concurrencyLimiter (a single global semaphore), this counts per key. A
|
// single global semaphore, this counts per key. A
|
||||||
// non-positive max disables it (acquire always admits, release is a no-op), the same
|
// non-positive max disables it (acquire always admits, release is a no-op), the same
|
||||||
// "zero disables" idiom as the other levers.
|
// "zero disables" idiom as the other levers.
|
||||||
type streamLimiter struct {
|
type streamLimiter struct {
|
||||||
|
|||||||
@@ -722,7 +722,7 @@ func (p *PGRepo) IsProtectedAdminLink(ctx context.Context, mcUUID string) (bool,
|
|||||||
return ok, err
|
return ok, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---- local-password auth (spec §B) ----
|
// ---- staff account lookups (spec §B, passwordless) ----
|
||||||
|
|
||||||
// UserByUsername loads a staff login projection by username, or ErrNotFound.
|
// UserByUsername loads a staff login projection by username, or ErrNotFound.
|
||||||
// The account is passwordless — staff authenticate via email-OTP / passkey, so
|
// The account is passwordless — staff authenticate via email-OTP / passkey, so
|
||||||
@@ -836,9 +836,10 @@ func (p *PGRepo) RevokeSession(ctx context.Context, tokenHash string) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
// RevokeUserSessionsExcept revokes every live session of a user except
|
// RevokeUserSessionsExcept revokes every live session of a user except keepTokenHash
|
||||||
// keepTokenHash — the change-password flow logs out the account's other devices
|
// — logs out an account's other devices while keeping the current one. Its original
|
||||||
// while keeping the current one.
|
// caller (the change-password flow) was removed in the passwordless migration; it is
|
||||||
|
// retained for the account-remediation path (P5, #78) and currently has no caller.
|
||||||
func (p *PGRepo) RevokeUserSessionsExcept(ctx context.Context, userID, keepTokenHash string) error {
|
func (p *PGRepo) RevokeUserSessionsExcept(ctx context.Context, userID, keepTokenHash string) error {
|
||||||
_, err := p.db.ExecContext(ctx,
|
_, err := p.db.ExecContext(ctx,
|
||||||
`UPDATE sessions SET revoked_at = now()
|
`UPDATE sessions SET revoked_at = now()
|
||||||
@@ -1030,8 +1031,10 @@ func (p *PGRepo) DeletePasskeyCredential(ctx context.Context, userID, id string)
|
|||||||
// DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds. Unlike the
|
// DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds. Unlike the
|
||||||
// single-credential delete this does NOT report ErrNotFound on zero rows: removing all of
|
// single-credential delete this does NOT report ErrNotFound on zero rows: removing all of
|
||||||
// a user's passkeys when they have none is a successful no-op, since "the user holds no
|
// a user's passkeys when they have none is a successful no-op, since "the user holds no
|
||||||
// passkeys" is exactly the intended post-condition. The change-password flow calls it so a
|
// passkeys" is exactly the intended post-condition. It is the remediation that stops a
|
||||||
// passkey planted through a transiently-hijacked session cannot survive the remediation.
|
// passkey planted through a transiently-hijacked session from surviving; its original
|
||||||
|
// caller (the change-password flow) was removed in the passwordless migration, so it is
|
||||||
|
// currently uncalled, retained for the account-remediation/reset path (P5, #78).
|
||||||
func (p *PGRepo) DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error {
|
func (p *PGRepo) DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error {
|
||||||
_, err := p.db.ExecContext(ctx,
|
_, err := p.db.ExecContext(ctx,
|
||||||
`DELETE FROM webauthn_credentials WHERE user_id = $1`, userID)
|
`DELETE FROM webauthn_credentials WHERE user_id = $1`, userID)
|
||||||
|
|||||||
+22
-20
@@ -92,9 +92,9 @@ type StaffUser struct {
|
|||||||
// be public (unlike a session token), so it is safe at rest. CredentialID is the
|
// be public (unlike a session token), so it is safe at rest. CredentialID is the
|
||||||
// authenticator's globally-unique handle (base64url) and PublicKey the COSE key
|
// authenticator's globally-unique handle (base64url) and PublicKey the COSE key
|
||||||
// (base64); SignCount is the uint32 signature counter captured at registration.
|
// (base64); SignCount is the uint32 signature counter captured at registration.
|
||||||
// LastUsedAt is nil until an assertion is verified — the login/step-up path that
|
// LastUsedAt is nil until an assertion stamps it. The passkey login door now exists
|
||||||
// would stamp it is out of scope for this enrollment-only slice (deferred), so it
|
// (Public /auth/passkey/login/{begin,finish}, #72), but no path yet writes
|
||||||
// stays nil through the flow this type backs.
|
// last_used_at, so in practice it stays nil; wiring the stamp is a follow-up there.
|
||||||
type PasskeyCredential struct {
|
type PasskeyCredential struct {
|
||||||
ID string
|
ID string
|
||||||
UserID string
|
UserID string
|
||||||
@@ -352,11 +352,13 @@ type Repo interface {
|
|||||||
// can only unbind their OWN credential. No matching (user, id) row → ErrNotFound,
|
// can only unbind their OWN credential. No matching (user, id) row → ErrNotFound,
|
||||||
// so a stale or cross-user id cannot silently no-op as success.
|
// so a stale or cross-user id cannot silently no-op as success.
|
||||||
DeletePasskeyCredential(ctx context.Context, userID, id string) error
|
DeletePasskeyCredential(ctx context.Context, userID, id string) error
|
||||||
// DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds. The
|
// DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds — the
|
||||||
// change-password flow calls it so a passkey planted via a transiently-hijacked
|
// remediation that stops a passkey planted via a transiently-hijacked session from
|
||||||
// session does not survive the remediation (password reset + session revoke) as a
|
// surviving as a standing login foothold. Its original caller, the change-password
|
||||||
// standing login foothold. Removing zero rows is success, not an error — an account
|
// flow, was removed in the passwordless migration, so it currently has no production
|
||||||
// with no passkeys is the intended post-condition either way.
|
// caller; it is retained for the account-remediation/reset path (P5, #78). Removing
|
||||||
|
// zero rows is success, not an error — an account with no passkeys is the intended
|
||||||
|
// post-condition either way.
|
||||||
DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error
|
DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error
|
||||||
|
|
||||||
// ---- player game-login: username-collision reclaim (spec §B3) ----
|
// ---- player game-login: username-collision reclaim (spec §B3) ----
|
||||||
@@ -398,18 +400,18 @@ type Repo interface {
|
|||||||
// name to protect). Keyed by UUID — the only identity velocity knows.
|
// name to protect). Keyed by UUID — the only identity velocity knows.
|
||||||
IsProtectedAdminLink(ctx context.Context, mcUUID string) (bool, error)
|
IsProtectedAdminLink(ctx context.Context, mcUUID string) (bool, error)
|
||||||
|
|
||||||
// ---- local-password auth (spec §B) ----
|
// ---- staff account lookups (spec §B, passwordless) ----
|
||||||
|
|
||||||
// UserByUsername loads the login projection of a staff account by its unique
|
// UserByUsername loads the login projection of a staff account by its unique
|
||||||
// username, or ErrNotFound. The caller compares PasswordHash itself so the
|
// username, or ErrNotFound. Its caller is the `felis breakGlass` recovery TUI,
|
||||||
// anti-enumeration dummy-hash compare runs even on a miss; a player row (NULL
|
// which resolves an Owner/Operator username before sending an email-OTP — there is
|
||||||
// password_hash → empty PasswordHash) is returned too and is rejected by the
|
// no password compare (the account is passwordless). A non-staff (role='user') row
|
||||||
// caller's hash compare, never by leaking "no such user".
|
// resolves too; callers that require staff enforce the role themselves.
|
||||||
UserByUsername(ctx context.Context, username string) (*StaffUser, error)
|
UserByUsername(ctx context.Context, username string) (*StaffUser, error)
|
||||||
// UserByID loads the same staff projection by user id, or ErrNotFound. The
|
// UserByID loads the same staff projection by user id, or ErrNotFound. Callers hold
|
||||||
// change-password flow uses it to re-verify the caller's current password: the
|
// a session (which yields a user id, not a username) and need the account behind it
|
||||||
// session yields a user id, not a username, so this is the id-keyed counterpart
|
// — e.g. setup redeem/status resolving the lockdown session's owner. It is the
|
||||||
// of UserByUsername.
|
// id-keyed counterpart of UserByUsername.
|
||||||
UserByID(ctx context.Context, id string) (*StaffUser, error)
|
UserByID(ctx context.Context, id string) (*StaffUser, error)
|
||||||
// UserByEmail resolves a VERIFIED email address to its login projection,
|
// UserByEmail resolves a VERIFIED email address to its login projection,
|
||||||
// case-insensitively, or ErrNotFound (spec §B email-first login). It is the
|
// case-insensitively, or ErrNotFound (spec §B email-first login). It is the
|
||||||
@@ -419,9 +421,9 @@ type Repo interface {
|
|||||||
// else's login by typing their email. Matching is on lower(email) to align with
|
// else's login by typing their email. Matching is on lower(email) to align with
|
||||||
// the users_verified_email_unique partial index (migration 0010), which
|
// the users_verified_email_unique partial index (migration 0010), which
|
||||||
// guarantees at most one verified row per normalized address, so the result is
|
// guarantees at most one verified row per normalized address, so the result is
|
||||||
// unambiguous. A player row (empty PasswordHash) resolves too — email-first
|
// unambiguous. A player (role='user') row resolves too — email-first login is
|
||||||
// login is passwordless and does not consult the hash — unlike the password
|
// passwordless and role-agnostic here; the door that consumes this result decides
|
||||||
// path, which this deliberately does not gate on.
|
// what each role may do.
|
||||||
UserByEmail(ctx context.Context, email string) (*StaffUser, error)
|
UserByEmail(ctx context.Context, email string) (*StaffUser, error)
|
||||||
// UpsertOwner creates or resets the single Owner account direct-to-Postgres
|
// UpsertOwner creates or resets the single Owner account direct-to-Postgres
|
||||||
// (the `felis setup` / `felis breakGlass` recovery path). role is forced to
|
// (the `felis setup` / `felis breakGlass` recovery path). role is forced to
|
||||||
|
|||||||
@@ -15,23 +15,23 @@ import (
|
|||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Local-password sessions (spec §B). The remote face authenticates statelessly
|
// Local sessions (spec §B, passwordless). The remote face authenticates statelessly
|
||||||
// with a Cloudflare-Access JWT and sets no cookie; local-password auth, used on
|
// with a Cloudflare-Access JWT and sets no cookie; the passwordless console login
|
||||||
// op.console when Zero Trust is not configured (and as the demo's primary web
|
// (email-OTP / passkey / setup redeem), used on op.console when Zero Trust is not
|
||||||
// login), needs a server-minted session. We store only the sha-256 of the opaque
|
// configured (and as the demo's primary web login), needs a server-minted session.
|
||||||
// cookie value, mirroring how service tokens are stored, so a database read never
|
// We store only the sha-256 of the opaque cookie value, mirroring how service tokens
|
||||||
// yields a usable cookie.
|
// are stored, so a database read never yields a usable cookie.
|
||||||
|
|
||||||
const (
|
const (
|
||||||
// sessionCookieName is the host-only session cookie. It carries no Domain
|
// sessionCookieName is the host-only session cookie. It carries no Domain
|
||||||
// attribute, so an op.console session is never sent to the player console.
|
// attribute, so an op.console session is never sent to the player console.
|
||||||
sessionCookieName = "felis_session"
|
sessionCookieName = "felis_session"
|
||||||
// sessionTTL bounds a local-password session. Staff re-authenticate after it.
|
// sessionTTL bounds a local session. Staff re-authenticate after it.
|
||||||
sessionTTL = 12 * time.Hour
|
sessionTTL = 12 * time.Hour
|
||||||
)
|
)
|
||||||
|
|
||||||
// LocalAuthEnabledKey is the platform_settings key that gates whether
|
// LocalAuthEnabledKey is the platform_settings key that gates whether
|
||||||
// local-password sessions are honored. It is flipped on by `felis breakGlass`
|
// local sessions are honored. It is flipped on by `felis breakGlass`
|
||||||
// direct-to-Postgres at first-run and read live per-request, so enabling local
|
// direct-to-Postgres at first-run and read live per-request, so enabling local
|
||||||
// auth needs no pod roll. Exported so the break-glass writer and this
|
// auth needs no pod roll. Exported so the break-glass writer and this
|
||||||
// per-request reader share one source of truth instead of drifting copies.
|
// per-request reader share one source of truth instead of drifting copies.
|
||||||
@@ -109,7 +109,7 @@ func hostIsAdminConsole(r *http.Request, rootDomain, adminHostname string) bool
|
|||||||
}
|
}
|
||||||
|
|
||||||
// SessionAuth is the composite ExternalAuth for the web face. It prefers a
|
// SessionAuth is the composite ExternalAuth for the web face. It prefers a
|
||||||
// local-password session cookie and otherwise delegates to the remote JWT
|
// local session cookie and otherwise delegates to the remote JWT
|
||||||
// verifier, so both auth models coexist on one face:
|
// verifier, so both auth models coexist on one face:
|
||||||
//
|
//
|
||||||
// - No cookie → delegate to Delegate (the Cloudflare-Access JWT path).
|
// - No cookie → delegate to Delegate (the Cloudflare-Access JWT path).
|
||||||
|
|||||||
@@ -12,7 +12,8 @@ const maxBodyBytes = 1 << 20 // 1 MiB
|
|||||||
|
|
||||||
// requireJSONContentType rejects a request whose body is not declared
|
// requireJSONContentType rejects a request whose body is not declared
|
||||||
// application/json, returning 415 before any decode. It guards the credential-bearing
|
// application/json, returning 415 before any decode. It guards the credential-bearing
|
||||||
// auth writes (login, change-password) against a cross-site forgery: an HTML form can
|
// auth writes (email-OTP, passkey, op-login, setup redeem) against a cross-site
|
||||||
|
// forgery: an HTML form can
|
||||||
// only POST as application/x-www-form-urlencoded, multipart/form-data, or text/plain
|
// only POST as application/x-www-form-urlencoded, multipart/form-data, or text/plain
|
||||||
// — never JSON — and a cross-site fetch that forces application/json triggers a CORS
|
// — never JSON — and a cross-site fetch that forces application/json triggers a CORS
|
||||||
// preflight this API never answers, so neither form can be forged off-origin. The
|
// preflight this API never answers, so neither form can be forged off-origin. The
|
||||||
|
|||||||
Reference in new issue
Block a user