Fix username squatting on failed webauthn

This commit is contained in:
Jonathon Leight committed 2026-07-05 13:22:12 -04:00
1 parent 00c8e98689
commit cb07b89410
8 files changed
+370 -63

No files matched your search

+1 -4
View File
@@ -138,7 +138,4 @@ internal/web routing, templates, static assets, confirm orchestration
## Known follow-ups
- An abandoned passkey signup leaves an orphan account holding that username (can't log in, blocks
re-signup). Add cleanup or upsert-on-begin.
- Passkey `navigator.credentials` create/get is only verifiable in a real browser / virtual
authenticator — not covered by automated tests.
- (none currently)
+77 -32
View File
@@ -45,17 +45,20 @@ func (s *Service) RequireSSO(next http.Handler) http.Handler {
// --- Passkey registration ---
// RegisterBegin starts a passkey registration ceremony. If a user is logged
// in, the passkey is added to that account; otherwise a new account is created
// from the posted email.
// RegisterBegin starts a passkey registration ceremony. If a user is logged in,
// the passkey is added to that account. Otherwise this begins a NEW account
// signup: the account is NOT persisted yet — we reserve its id (which becomes
// the stable WebAuthn user handle) and stash the pending username, and only
// write the row once a credential is verified at RegisterFinish. That keeps an
// abandoned ceremony from leaving an orphan account that squats the username.
func (s *Service) RegisterBegin(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
var u *store.User
var name string // optional label, only for adding a passkey to an existing account
var waUser *webauthnUser
var name string // passkey label — only when adding to an existing account
if uid := s.CurrentUserID(ctx); uid != 0 {
var err error
if u, err = s.store.GetUserByID(ctx, uid); err != nil {
u, err := s.store.GetUserByID(ctx, uid)
if err != nil {
httpError(w, http.StatusInternalServerError, "load user")
return
}
@@ -64,28 +67,38 @@ func (s *Service) RegisterBegin(w http.ResponseWriter, r *http.Request) {
}
_ = json.NewDecoder(r.Body).Decode(&body)
name = NormalizePasskeyName(body.Name)
// Existing credentials are loaded so the authenticator excludes them.
if waUser, err = s.loadWebAuthnUser(ctx, u); err != nil {
httpError(w, http.StatusInternalServerError, "load credentials")
return
}
} else {
username, displayName, ok := readCreds(r)
if !ok {
httpError(w, http.StatusBadRequest, "username must be 3–32 chars (letters, digits, _ . -)")
return
}
created, err := s.store.CreateUser(ctx, username, displayName)
if errors.Is(err, store.ErrDuplicate) {
// Fail fast on a taken name (authoritatively re-checked at finish's insert).
if _, err := s.store.GetUserByUsername(ctx, username); err == nil {
httpError(w, http.StatusConflict, "username already taken — choose another or sign in")
return
}
if err != nil {
httpError(w, http.StatusInternalServerError, "create user")
} else if !errors.Is(err, store.ErrNotFound) {
httpError(w, http.StatusInternalServerError, "check username")
return
}
u = created
}
waUser, err := s.loadWebAuthnUser(ctx, u)
if err != nil {
httpError(w, http.StatusInternalServerError, "load credentials")
return
reservedID, err := s.store.ReserveUserID(ctx)
if err != nil {
httpError(w, http.StatusInternalServerError, "reserve account")
return
}
// In-memory only — no row is written until finish verifies a credential.
u := &store.User{ID: reservedID, Username: username}
if displayName != "" {
u.DisplayName = &displayName
}
waUser = &webauthnUser{user: u}
s.Sessions.Put(ctx, sessKeyWANewName, username)
s.Sessions.Put(ctx, sessKeyWANewDN, displayName)
}
// Prefer discoverable (resident-key) credentials so the holder can sign in
@@ -98,7 +111,7 @@ func (s *Service) RegisterBegin(w http.ResponseWriter, r *http.Request) {
httpError(w, http.StatusInternalServerError, "begin registration")
return
}
if err := s.stashCeremony(ctx, u.ID, sessionData); err != nil {
if err := s.stashCeremony(ctx, waUser.user.ID, sessionData); err != nil {
httpError(w, http.StatusInternalServerError, "save ceremony")
return
}
@@ -106,7 +119,10 @@ func (s *Service) RegisterBegin(w http.ResponseWriter, r *http.Request) {
writeJSON(w, options)
}
// RegisterFinish completes a passkey registration and logs the user in.
// RegisterFinish completes a passkey registration and logs the user in. For a
// deferred new-account ceremony it verifies the credential FIRST, then writes
// the account row (with the reserved id) — so a failed or abandoned ceremony
// never persists an account.
func (s *Service) RegisterFinish(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
uid, sessionData, ok := s.popCeremony(ctx)
@@ -114,17 +130,32 @@ func (s *Service) RegisterFinish(w http.ResponseWriter, r *http.Request) {
httpError(w, http.StatusBadRequest, "no registration in progress")
return
}
u, err := s.store.GetUserByID(ctx, uid)
if err != nil {
httpError(w, http.StatusInternalServerError, "load user")
return
}
waUser, err := s.loadWebAuthnUser(ctx, u)
if err != nil {
httpError(w, http.StatusInternalServerError, "load credentials")
return
// A pending username marks a new-account ceremony (id reserved, row not yet
// written); its absence means we're adding a passkey to an existing account.
newName := s.Sessions.PopString(ctx, sessKeyWANewName)
newDisplay := s.Sessions.PopString(ctx, sessKeyWANewDN)
var u *store.User
var waUser *webauthnUser
if newName != "" {
u = &store.User{ID: uid, Username: newName}
if newDisplay != "" {
u.DisplayName = &newDisplay
}
waUser = &webauthnUser{user: u} // in-memory; no row yet
} else {
var err error
if u, err = s.store.GetUserByID(ctx, uid); err != nil {
httpError(w, http.StatusInternalServerError, "load user")
return
}
if waUser, err = s.loadWebAuthnUser(ctx, u); err != nil {
httpError(w, http.StatusInternalServerError, "load credentials")
return
}
}
// Verify the credential before writing anything durable.
cred, err := s.wa.FinishRegistration(waUser, *sessionData, r)
if err != nil {
httpError(w, http.StatusBadRequest, "registration failed: "+err.Error())
@@ -135,8 +166,22 @@ func (s *Service) RegisterFinish(w http.ResponseWriter, r *http.Request) {
httpError(w, http.StatusInternalServerError, "marshal credential")
return
}
name := s.Sessions.GetString(ctx, sessKeyWAName)
s.Sessions.Remove(ctx, sessKeyWAName)
if newName != "" {
// The credential is proven — now persist the account with its reserved id.
created, err := s.store.CreateUserWithID(ctx, uid, newName, newDisplay)
if errors.Is(err, store.ErrDuplicate) || errors.Is(err, store.ErrUsernameReserved) {
httpError(w, http.StatusConflict, "username already taken — choose another or sign in")
return
}
if err != nil {
httpError(w, http.StatusInternalServerError, "create account")
return
}
u = created
}
name := s.Sessions.PopString(ctx, sessKeyWAName)
if err := s.store.AddCredential(ctx, u.ID, cred.ID, blob, name); err != nil {
httpError(w, http.StatusInternalServerError, "store credential")
return
+7 -2
View File
@@ -18,10 +18,15 @@ import (
const (
sessKeyUserID = "user_id" // int64: the authenticated user
sessKeyLoginID = "login_id" // string: the logins-row id backing this session
sessKeyWAUID = "wa_uid" // int64: user mid-ceremony
sessKeyWAUID = "wa_uid" // int64: user (or reserved id) mid-ceremony
sessKeyWAData = "wa_data" // []byte: marshaled webauthn.SessionData
sessKeyWAName = "wa_name" // string: pending passkey name for the in-flight registration
sessKeyNext = "next" // string: post-auth redirect target
// Deferred passkey signup: a logged-out register ceremony carries the pending
// account's username/display name (present only for new-account ceremonies).
// The account row is written at finish, once a credential is verified.
sessKeyWANewName = "wa_new_username" // string: pending new-account username
sessKeyWANewDN = "wa_new_display" // string: pending new-account display name
sessKeyNext = "next" // string: post-auth redirect target
)
// Service wires WebAuthn, the data store, and session management.
+30
View File
@@ -1,10 +1,13 @@
package core
import (
"errors"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"github.com/jleight/meshtender/internal/store"
)
// Black-box coverage for the auth host's non-visual POST endpoints: password
@@ -94,6 +97,33 @@ func TestWebAuthnBeginRateLimited(t *testing.T) {
}
}
// TestPasskeyBeginDefersAccount: a logged-out register/begin issues a challenge
// but does NOT persist an account — the row is only written once a credential is
// verified at finish (which needs a real authenticator, covered by the e2e
// suite). Regression for the pre-release audit finding + README follow-up that an
// abandoned passkey signup left an orphan account squatting the username.
func TestPasskeyBeginDefersAccount(t *testing.T) {
t.Parallel()
st, ctx, ts, h := splitServer(t)
resp := jsonPost(t, ts.URL+"/api/register/begin", h.auth, `{"username":"ghost","displayName":"Ghost"}`)
resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("register/begin = %d, want 200 (challenge issued)", resp.StatusCode)
}
if _, err := st.GetUserByUsername(ctx, "ghost"); !errors.Is(err, store.ErrNotFound) {
t.Fatalf("begin persisted an account for an unfinished ceremony (err=%v)", err)
}
// A name already taken is still rejected up front.
authSSO(t, ts, h, "taken")
dup := jsonPost(t, ts.URL+"/api/register/begin", h.auth, `{"username":"taken"}`)
dup.Body.Close()
if dup.StatusCode != http.StatusConflict {
t.Fatalf("register/begin for taken username = %d, want 409", dup.StatusCode)
}
}
// assertAccountOK asserts a 303 back to /account carrying a success ("ok") flash.
func assertAccountOK(t *testing.T, resp *http.Response, label string) {
t.Helper()
+47 -18
View File
@@ -125,10 +125,26 @@ func rootReachableHosts() hostLayout {
return hostLayout{app: "app." + browserHost(), auth: "auth." + browserHost(), root: browserHost()}
}
// newE2EServer stands up the app on a 0.0.0.0 listener so the browser container
// can connect back to it, and returns both address forms. An optional hostLayout
// selects which surface the browser can reach (defaults to the app surface).
// newE2EServer stands up the app (plain HTTP) on a 0.0.0.0 listener so the
// browser container can connect back to it, and returns both address forms. An
// optional hostLayout selects which surface the browser can reach (defaults to
// the app surface).
func newE2EServer(t *testing.T, layout ...hostLayout) *e2eServer {
return buildE2EServer(t, false, layout...)
}
// newE2ETLSServer is like newE2EServer but serves HTTPS (a self-signed cert) and
// marks cookies Secure. WebAuthn ceremonies require a secure context, which a
// plain-HTTP non-localhost origin is not; the browser accepts the self-signed
// cert via Security.setIgnoreCertificateErrors (see the passkey test).
func newE2ETLSServer(t *testing.T, layout ...hostLayout) *e2eServer {
return buildE2EServer(t, true, layout...)
}
// buildE2EServer is the shared constructor. It listens first so the WebAuthn RP
// origins can carry the actual (dynamic) port, then builds the surfaces. secure
// selects HTTPS + Secure cookies.
func buildE2EServer(t *testing.T, secure bool, layout ...hostLayout) *e2eServer {
t.Helper()
hosts := defaultHosts()
if len(layout) > 0 {
@@ -145,45 +161,58 @@ func newE2EServer(t *testing.T, layout ...hostLayout) *e2eServer {
_, _ = rand.Read(masterKey[:])
idSvc, _ := identity.LoadOrCreate(ctx, st, masterKey)
// Listen up front so the RP origins can include the concrete port.
ln, err := net.Listen("tcp", "0.0.0.0:0")
if err != nil {
t.Fatalf("listen: %v", err)
}
port := ln.Addr().(*net.TCPAddr).Port
scheme := "http"
if secure {
scheme = "https"
}
origin := func(h string) string { return fmt.Sprintf("%s://%s:%d", scheme, h, port) }
// The server runs across three distinct hosts so the Dispatcher can tell the
// surfaces apart. The browser can navigate only the one named browserHost()
// (host.docker.internal) — the sole alias the container resolves back here; the
// layout decides which surface that is (app by default, auth for account tests).
appHost, authHost, rootHost := hosts.app, hosts.auth, hosts.root
// layout decides which surface that is. The RP ID is browserHost(): a WebAuthn
// ceremony's origin host is that name (or a subdomain of it), so it's a valid
// RP ID, and every surface origin is allowed.
authSvc, err := auth.New(st, st.Pool(), auth.Config{
RPID: "localhost", RPDisplayName: "test", RPOrigins: []string{"http://localhost"},
AppHost: appHost, AuthHost: authHost, RootHost: rootHost,
RPID: browserHost(), RPDisplayName: "test",
RPOrigins: []string{origin(hosts.app), origin(hosts.auth), origin(hosts.root)},
AppHost: hosts.app, AuthHost: hosts.auth, RootHost: hosts.root,
Secure: secure,
})
if err != nil {
t.Fatalf("auth: %v", err)
}
srv, err := core.NewServer(st, authSvc, idSvc, &config.Config{
PrimaryHost: appHost, AuthHost: authHost, RootHost: rootHost,
PrimaryHost: hosts.app, AuthHost: hosts.auth, RootHost: hosts.root, Secure: secure,
})
if err != nil {
t.Fatalf("server: %v", err)
}
ln, err := net.Listen("tcp", "0.0.0.0:0")
if err != nil {
t.Fatalf("listen: %v", err)
}
ts := httptest.NewUnstartedServer(srv.Handler())
ts.Listener.Close() // drop the default 127.0.0.1 listener
ts.Listener = ln
ts.Start()
if secure {
ts.StartTLS()
} else {
ts.Start()
}
t.Cleanup(ts.Close)
port := ln.Addr().(*net.TCPAddr).Port
// ts.URL reflects the 0.0.0.0 listener; rewrite it to loopback so the
// host-side Go client dials a concrete address.
ts.URL = fmt.Sprintf("http://127.0.0.1:%d", port)
ts.URL = fmt.Sprintf("%s://127.0.0.1:%d", scheme, port)
return &e2eServer{
store: st,
ctx: ctx,
ts: ts,
hostURL: ts.URL,
browserURL: fmt.Sprintf("http://%s:%d", browserHost(), port),
browserURL: fmt.Sprintf("%s://%s:%d", scheme, browserHost(), port),
}
}
+109
View File
@@ -0,0 +1,109 @@
//go:build browser
package e2e
import (
"context"
"errors"
"testing"
"time"
"github.com/chromedp/cdproto/security"
"github.com/chromedp/cdproto/webauthn"
"github.com/chromedp/chromedp"
"github.com/jleight/meshtender/internal/store"
)
// virtualAuthenticator enables the WebAuthn CDP domain and installs a virtual
// authenticator that auto-approves user presence/verification, so
// navigator.credentials.create/get succeed headlessly. It also tells the browser
// to accept the harness's self-signed cert (WebAuthn needs a secure context, and
// an HTTPS origin — even with an untrusted cert — qualifies, whereas plain HTTP
// on a non-localhost host does not).
func virtualAuthenticator(ctx context.Context) error {
return chromedp.Run(ctx,
security.SetIgnoreCertificateErrors(true),
webauthn.Enable(),
chromedp.ActionFunc(func(ctx context.Context) error {
_, err := webauthn.AddVirtualAuthenticator(&webauthn.VirtualAuthenticatorOptions{
Protocol: webauthn.AuthenticatorProtocolCtap2,
Transport: webauthn.AuthenticatorTransportInternal,
HasResidentKey: true,
HasUserVerification: true,
IsUserVerified: true,
AutomaticPresenceSimulation: true,
}).Do(ctx)
return err
}),
)
}
// waitForUser polls until the account exists (the ceremony's server side runs
// asynchronously after the click) or the deadline passes.
func waitForUser(t *testing.T, e *e2eServer, username string) *store.User {
t.Helper()
deadline := time.Now().Add(20 * time.Second)
for {
u, err := e.store.GetUserByUsername(e.ctx, username)
if err == nil {
return u
}
if !errors.Is(err, store.ErrNotFound) {
t.Fatalf("GetUserByUsername: %v", err)
}
if time.Now().After(deadline) {
return nil
}
time.Sleep(200 * time.Millisecond)
}
}
// TestPasskeySignupCeremony drives the real passkey signup end to end through a
// virtual authenticator: the browser runs navigator.credentials.create on the
// live signup page, and only when the credential is verified at finish does the
// account get written (the deferred-creation flow). This is the browser-level
// proof of item 3 that the Go tests can't give (they can't complete a ceremony).
func TestPasskeySignupCeremony(t *testing.T) {
e := newE2ETLSServer(t, authReachableHosts())
ctx, cancel, watch := startBrowser(t)
defer cancel()
if err := virtualAuthenticator(ctx); err != nil {
t.Fatalf("set up virtual authenticator: %v", err)
}
const username = "passkeysignup"
// Pre-condition: no such account yet.
if _, err := e.store.GetUserByUsername(e.ctx, username); !errors.Is(err, store.ErrNotFound) {
t.Fatalf("account should not exist before the ceremony (err=%v)", err)
}
if err := chromedp.Run(ctx,
chromedp.Navigate(e.browserURL+"/signup"),
chromedp.WaitVisible("#signup-passkey-btn", chromedp.ByID),
chromedp.SendKeys("#username", username, chromedp.ByID),
chromedp.Click("#signup-passkey-btn", chromedp.ByID),
); err != nil {
t.Fatalf("drive signup page: %v", err)
}
u := waitForUser(t, e, username)
if u == nil {
// Surface the on-page status for diagnostics.
var status string
_ = chromedp.Run(ctx, chromedp.Text("#passkey-status", &status, chromedp.ByID, chromedp.AtLeast(0)))
t.Fatalf("account was not created by the passkey ceremony; page status = %q", status)
}
// A verified credential must have been persisted for the new account.
creds, err := e.store.GetCredentials(e.ctx, u.ID)
if err != nil {
t.Fatalf("GetCredentials: %v", err)
}
if len(creds) != 1 {
t.Fatalf("new account has %d credentials, want exactly 1", len(creds))
}
watch.assertClean(t) // the signup page + ceremony must not trip the CSP
}
+39 -7
View File
@@ -53,11 +53,39 @@ func scanUser(row pgx.Row) (*User, error) {
return &u, nil
}
// CreateUser inserts a new user and returns it. displayName may be empty (then
// stored as NULL). Returns ErrDuplicate if the username is already taken, or
// ErrUsernameReserved if it was recently released by someone else and is still
// within its cooldown.
// CreateUser inserts a new user with a database-assigned id and returns it.
// displayName may be empty (then stored as NULL). Returns ErrDuplicate if the
// username is already taken, or ErrUsernameReserved if it was recently released
// by someone else and is still within its cooldown.
func (s *Store) CreateUser(ctx context.Context, username, displayName string) (*User, error) {
return s.createUser(ctx, 0, username, displayName)
}
// ReserveUserID consumes and returns the next users.id from the identity
// sequence without inserting a row. The deferred passkey signup uses it to fix
// the account's id — and therefore its WebAuthn user handle, which is the id
// encoded as 8 bytes — at BeginRegistration, then writes the row with
// CreateUserWithID only once a credential is verified at finish. An abandoned
// ceremony merely leaves a gap in the sequence; no orphan account is created.
func (s *Store) ReserveUserID(ctx context.Context) (int64, error) {
var id int64
if err := s.pool.QueryRow(ctx,
`SELECT nextval(pg_get_serial_sequence('users', 'id'))`).Scan(&id); err != nil {
return 0, fmt.Errorf("reserve user id: %w", err)
}
return id, nil
}
// CreateUserWithID inserts a new user with a caller-reserved id (from
// ReserveUserID). See CreateUser for the error contract.
func (s *Store) CreateUserWithID(ctx context.Context, id int64, username, displayName string) (*User, error) {
return s.createUser(ctx, id, username, displayName)
}
// createUser is the shared insert. id == 0 lets the identity column assign one;
// a nonzero id is forced via OVERRIDING SYSTEM VALUE (the column is GENERATED
// ALWAYS).
func (s *Store) createUser(ctx context.Context, id int64, username, displayName string) (*User, error) {
var dn *string
if displayName != "" {
dn = &displayName
@@ -71,9 +99,13 @@ func (s *Store) CreateUser(ctx context.Context, username, displayName string) (*
if reserved {
return nil, ErrUsernameReserved
}
u, err := scanUser(s.pool.QueryRow(ctx,
`INSERT INTO users (username, display_name) VALUES ($1, $2) RETURNING `+userCols,
username, dn))
query := `INSERT INTO users (username, display_name) VALUES ($1, $2) RETURNING ` + userCols
args := []any{username, dn}
if id != 0 {
query = `INSERT INTO users (id, username, display_name) OVERRIDING SYSTEM VALUE VALUES ($1, $2, $3) RETURNING ` + userCols
args = []any{id, username, dn}
}
u, err := scanUser(s.pool.QueryRow(ctx, query, args...))
if isUniqueViolation(err) {
return nil, ErrDuplicate
}
+60
View File
@@ -0,0 +1,60 @@
package store
import (
"errors"
"testing"
)
// TestReserveUserIDAndCreateWithID covers the deferred passkey-signup primitives:
// ReserveUserID hands out increasing ids without writing rows, and
// CreateUserWithID later persists the row at exactly that id (so the WebAuthn
// user handle, which is the id, still resolves).
func TestReserveUserIDAndCreateWithID(t *testing.T) {
t.Parallel()
st, ctx := orgTestStore(t)
id1, err := st.ReserveUserID(ctx)
if err != nil {
t.Fatalf("reserve 1: %v", err)
}
id2, err := st.ReserveUserID(ctx)
if err != nil {
t.Fatalf("reserve 2: %v", err)
}
if id2 <= id1 {
t.Fatalf("reserved ids not increasing: %d then %d", id1, id2)
}
// Reserving must not create any rows (that's the whole point — no orphans).
var n int
if err := st.pool.QueryRow(ctx, `SELECT count(*) FROM users`).Scan(&n); err != nil {
t.Fatalf("count users: %v", err)
}
if n != 0 {
t.Fatalf("ReserveUserID created %d user rows, want 0", n)
}
// Create at the reserved id; it must round-trip through GetUserByID — the path
// discoverable login uses to resolve a credential's handle back to an account.
u, err := st.CreateUserWithID(ctx, id2, "deferred", "Deferred User")
if err != nil {
t.Fatalf("CreateUserWithID: %v", err)
}
if u.ID != id2 {
t.Fatalf("created id = %d, want reserved %d", u.ID, id2)
}
got, err := st.GetUserByID(ctx, id2)
if err != nil || got.Username != "deferred" {
t.Fatalf("GetUserByID(%d) = %+v, %v", id2, got, err)
}
// The first real account still bootstraps instance caps.
if !u.CapManageUsers || !u.CapManageCatalog {
t.Fatalf("first account should bootstrap caps, got %+v", u)
}
// A duplicate username is rejected even via the explicit-id path.
id3, _ := st.ReserveUserID(ctx)
if _, err := st.CreateUserWithID(ctx, id3, "deferred", ""); !errors.Is(err, ErrDuplicate) {
t.Fatalf("duplicate username = %v, want ErrDuplicate", err)
}
}