From b87c9ea6e22c4f08a2f5755ffebed72a71924e42 Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Thu, 20 Aug 2026 15:59:34 +0200 Subject: [PATCH] fix(auth): en afvist passkey sagde "forkert e-mail eller adgangskode" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Der er hverken en e-mail eller en adgangskode i et passkey-login. Loginet er discoverable — enheden ved selv, hvilken konto nøglen hører til, og der bliver ikke tastet noget. Alligevel fik en afvisning fladens generiske 401-tekst, og den nævner begge dele. Det sendte en person ud at lede efter en tastefejl i sin konto, der aldrig havde været der. Det, der faktisk var sket, var at instansen var flyttet fra http til https: en passkey hører til præcis den adresse, den blev lavet på — `RPOrigins` bygges af `VERDANDE_BASE_URL` med skema og alt — så et login på det ene skema mod en server, der venter det andet, kan ikke verificeres. Beskeden siger nu det, og siger også hvor man skal kigge. Serveren har hele tiden svaret "that key was not accepted"; det var oversættelsen i fladen, der gjorde den til noget andet. Aflysning bliver behandlet som før — den er ikke en fejl, og en fejl for den ville læse som om nøglen blev afvist. Co-Authored-By: Claude Opus 5 --- internal/store/secrets_test.go | 91 ++++++++++++++++++++++++++++ web/src/lib/components/SignIn.svelte | 12 +++- web/src/lib/locales/da.js | 2 + web/src/lib/locales/en.js | 2 + 4 files changed, 106 insertions(+), 1 deletion(-) diff --git a/internal/store/secrets_test.go b/internal/store/secrets_test.go index bda231f..504b936 100644 --- a/internal/store/secrets_test.go +++ b/internal/store/secrets_test.go @@ -186,3 +186,94 @@ func TestOnePersonCanHaveSettingsForMoreThanOneThing(t *testing.T) { t.Errorf("the second write did not land: %v", back["provider"]) } } + +// The upgrade path, which is the one nobody runs: an existing database with a row +// in the old shape, opened by a binary that carries 0025. +// +// Every other test here starts empty and runs all the migrations in order, which +// exercises the migration's SQL but not its *situation*. The situation is a table +// that already holds somebody's settings, and the question is whether they come +// through it and whether a second scope can be written afterwards. +func TestSettingsSurviveTheUpgradeToPerScopeRows(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.db") + + db, err := Open(path) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + user := &User{Email: "kristian@example.dk", Name: "Kristian"} + if err := db.CreateUser(ctx, user, "Indbakke"); err != nil { + t.Fatal(err) + } + + // Wind the database back to how v0.33.3 left it: user_id as the whole primary + // key, and 0025 not yet applied. + for _, stmt := range []string{ + `PRAGMA foreign_keys = OFF`, + `CREATE TABLE us_old ( + user_id TEXT PRIMARY KEY REFERENCES users (id) ON DELETE CASCADE, + scope TEXT NOT NULL DEFAULT 'general', + values_json TEXT NOT NULL DEFAULT '{}', + updated_at INTEGER NOT NULL + )`, + `INSERT INTO us_old (user_id, scope, values_json, updated_at) + SELECT user_id, scope, values_json, updated_at FROM user_settings`, + `DROP TABLE user_settings`, + `ALTER TABLE us_old RENAME TO user_settings`, + `DELETE FROM schema_migrations WHERE name = '0025_user_settings_per_scope.sql'`, + `PRAGMA foreign_keys = ON`, + } { + if _, err := db.ExecContext(ctx, stmt); err != nil { + t.Fatalf("rewind (%s): %v", stmt, err) + } + } + + // A Gmail connection, written the way the old upsert wrote one. + if _, err := db.ExecContext(ctx, ` + INSERT INTO user_settings (user_id, scope, values_json, updated_at) + VALUES (?, 'gmail', '{"label":"Verdande"}', 0)`, user.ID); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + + // And now the new binary opens it. + db, err = Open(path) + if err != nil { + t.Fatalf("the upgrade refused to open the database: %v", err) + } + defer db.Close() + + gmail, err := db.UserSettings(ctx, user.ID, "gmail") + if err != nil { + t.Fatal(err) + } + if gmail["label"] != "Verdande" { + t.Errorf("the Gmail settings did not survive the migration: %v", gmail) + } + + // The thing that was reported: set up the AI provider, and have it still be + // there afterwards. + if err := db.SetUserSettings(ctx, user.ID, "ai", map[string]any{ + "provider": "anthropic", "model": "claude-sonnet-5", + }); err != nil { + t.Fatalf("saving the AI provider after the upgrade: %v", err) + } + ai, err := db.UserSettings(ctx, user.ID, "ai") + if err != nil { + t.Fatal(err) + } + if ai["provider"] != "anthropic" { + t.Errorf("the AI provider is %v after the upgrade, want anthropic", ai["provider"]) + } + again, err := db.UserSettings(ctx, user.ID, "gmail") + if err != nil { + t.Fatal(err) + } + if again["label"] != "Verdande" { + t.Errorf("saving the AI provider took the Gmail settings: %v", again) + } +} diff --git a/web/src/lib/components/SignIn.svelte b/web/src/lib/components/SignIn.svelte index 6062f9d..0b9c783 100644 --- a/web/src/lib/components/SignIn.svelte +++ b/web/src/lib/components/SignIn.svelte @@ -59,7 +59,17 @@ } catch (e) { // A cancelled prompt is not a failure. Somebody changed their mind, and // an error for that reads as though the key was rejected. - if (e?.name !== 'NotAllowedError' && e?.message !== 'cancelled') { + if (e?.name === 'NotAllowedError' || e?.message === 'cancelled') { + // nothing to say + } else if (e?.code === 'unauthorized') { + // Not "wrong email or password". There is no email here and no + // password — the device signed, and the signature was refused. The + // generic 401 text sent somebody looking through their account for a + // typo that was never there; what had actually happened was that the + // site had moved from http to https, and a passkey is bound to the + // origin it was made on, scheme included. + error = t('passkey.notAccepted'); + } else { error = humanMessage(e); } } finally { diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index 5cc1f87..8bffba1 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -814,6 +814,8 @@ export const da = { 'passkey.hint': 'En nøgle på en enhed i stedet for en hemmelighed i et hoved. Den private halvdel forlader aldrig enheden, og signaturen er bundet til adressen — så en overbevisende kopi af login-siden et andet sted får ingenting.', 'passkey.unsupported': 'Denne browser understøtter ikke passkeys.', + 'passkey.notAccepted': + 'Nøglen blev ikke godtaget. Sker det for en nøgle, der virkede før, er adressen typisk skiftet — en passkey hører til præcis den adresse, den blev lavet på, skema og alt.', 'passkey.none': 'Ingen nøgler endnu.', 'passkey.name': 'Navn på nøglen', 'passkey.namePlaceholder': 'fx min bærbare', diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index 0de5ae7..52e6d6f 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -807,6 +807,8 @@ export const en = { 'passkey.hint': 'A key on a device instead of a secret in a head. The private half never leaves the device, and the signature is bound to the address that asked for it — so a convincing copy of the sign-in page somewhere else gets nothing.', 'passkey.unsupported': 'This browser does not support passkeys.', + 'passkey.notAccepted': + 'That key was not accepted. If it worked before, the address has usually changed — a passkey belongs to the exact address it was made on, scheme and all.', 'passkey.none': 'No keys yet.', 'passkey.name': 'Name for the key', 'passkey.namePlaceholder': 'e.g. my laptop',