From e4606cfc70214b7aa167f25207460f7164cf0326 Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Wed, 19 Aug 2026 11:04:18 +0000 Subject: [PATCH] feat(auth): enable password login on OAuth-only accounts, with disable control Admins can now reset the password of an OAuth-only user, which creates their credentials and enables password login; the new 'Disable password login' row action removes those credentials again. Resetting a password revokes all existing sessions. - add users.password_login_enabled column (migration 0005) - admin reset: set password, enable login, revoke sessions - disable action: clear password, disable login (only on OAuth-linked rows) - account endpoint: refuse password updates on OAuth-linked users - e2e: wipe the test DB before the webServer starts (Playwright starts the server before globalSetup, so wiping there orphaned the DB file) - row-actions strip test: five square actions, wrap on phone widths --- docs/ARCHITECTURE.md | 2 +- docs/AUTH.md | 14 +- docs/FEATURES.md | 5 +- drizzle/0005_green_raider.sql | 2 + drizzle/meta/0005_snapshot.json | 1241 ++++++++++++++++++ drizzle/meta/_journal.json | 7 + playwright.config.ts | 9 +- src/lib/server/auth.test.ts | 119 ++ src/lib/server/auth.ts | 60 +- src/lib/server/db/index.ts | 20 +- src/lib/server/db/schema.ts | 3 + src/lib/server/db/test-helpers.ts | 2 + src/lib/types/index.ts | 1 + src/lib/utils/auth-methods.test.ts | 150 +-- src/lib/utils/auth-methods.ts | 69 +- src/routes/(app)/settings/users/+page.svelte | 45 +- src/routes/api/admin/users/[id]/+server.ts | 29 +- src/routes/api/auth/account/+server.ts | 6 +- static/api/openapi.json | 2 +- tests/e2e/admin-users.spec.ts | 160 ++- tests/e2e/global-setup.ts | 15 +- 21 files changed, 1756 insertions(+), 205 deletions(-) create mode 100644 drizzle/0005_green_raider.sql create mode 100644 drizzle/meta/0005_snapshot.json diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 10faa87..244d53a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -91,7 +91,7 @@ For a single-user app with multiple devices: See `src/lib/server/db/schema.ts` for the complete Drizzle schema. Key tables: -- `users` - User accounts (password hash, OAuth provider link, role) +- `users` - User accounts (password hash, `password_login_enabled`, OAuth provider link, role) - `sessions` - Auth sessions (30-day TTL) - `notes` - Core note data with version tracking - `tags` - Unique tag names diff --git a/docs/AUTH.md b/docs/AUTH.md index 4522059..9ad0de9 100644 --- a/docs/AUTH.md +++ b/docs/AUTH.md @@ -14,20 +14,20 @@ On first launch, Crumbs presents a setup screen to create the initial admin acco ### Admin password resets -An admin can reset the password of any user other than themselves, provided that account signs in with a password. Crumbs generates the new password and shows it once in that user's row — copy it and pass it to the user out of band, because it is not stored anywhere and cannot be shown again once dismissed. If SMTP is configured Crumbs also tries to notify the user that their password was reset, but the attempt is best-effort: a delivery failure is logged on the server and never surfaced in the UI, so a successful reset is not evidence the user was told. The notification never contains the password. +An admin can reset the password of any user other than themselves. Crumbs generates the new password and shows it once in that user's row — copy it and pass it to the user out of band, because it is not stored anywhere and cannot be shown again once dismissed. If SMTP is configured Crumbs also tries to notify the user that their password was reset, but the attempt is best-effort: a delivery failure is logged on the server and never surfaced in the UI, so a successful reset is not evidence the user was told. The notification never contains the password. -The control appears on every row, but is unavailable in two cases, each stating its reason on the row: +The control appears on every row, but is unavailable in two cases: - **Your own row** — change your own password in **Settings > Profile**, which verifies your current password first. -- **An account that signs in via OAuth/SSO** — it has no password the login form would accept, so setting one would hand the user a credential that cannot work. +- **While a generated password is still on screen** — dismiss it first if you need to generate another. -It is also unavailable while a generated password is still on screen, and that case gives no reason — dismiss the password first if you need to generate another. +A reset enables password login for that account if it was not already enabled — including OAuth-only accounts — without removing their SSO link. Both sign-in methods can coexist. -A reset does not sign the user out. **Revoke all sessions** is a separate action on the same row and, like the reset, is not offered for your own account. +A reset **revokes all of the user's sessions** — they must sign in again with the new password (or via OAuth if linked). **Disable password login** is a separate action on rows where password login is enabled; it clears the stored hash, turns off password login, and revokes all sessions. It is not offered on your own row. A displayed password is only good until that account's password changes again: any later reset — by another admin, or by the user themselves from **Settings > Profile** — silently invalidates one still on screen, and nothing marks it as stale. Separately, if Crumbs cannot confirm a reset it says so and shows the password anyway; keep that one, since it may or may not have been applied, and reset again if the user cannot sign in. -> **No password recovery.** Crumbs has no forgot-password flow. An admin who forgets their own password cannot reset it themselves: the admin control excludes their own row, and the profile flow requires the current password. Another admin can reset you, so keep a second admin account as cheap insurance. Failing that, if an OAuth/SSO provider is configured and your account's email matches your identity there, signing in with that provider gets you back in without a password — at the cost of handing the account over to the provider permanently, after which your password no longer works. With no other admin and no provider configured, there is no way back in through the app. +> **No password recovery.** Crumbs has no forgot-password flow. An admin who forgets their own password cannot reset it themselves: the admin control excludes their own row, and the profile flow requires the current password. Another admin can reset you, so keep a second admin account as cheap insurance. Failing that, if an OAuth/SSO provider is configured and your account's email matches your identity there, signing in with that provider gets you back in without a password. An admin can also enable password login on an OAuth-linked account via **Settings > Users**. With no other admin and no provider configured, there is no way back in through the app. ## OAuth / SSO @@ -52,7 +52,7 @@ OAuth does **not** auto-create accounts. Users must be pre-created by an admin: After initial linking, the user is identified by their provider-specific ID (`sub` claim), so email changes on the provider side won't break login. -Tick **OAuth-only (no password)** when creating these accounts — the provider is how they will sign in anyway. If you set a password instead, treat it as temporary: the first OAuth sign-in hands the account over to the provider, after which the password no longer works at the login form and the admin password reset is unavailable for that account. +Tick **OAuth-only (no password)** when creating these accounts — the provider is how they will sign in. An admin can later enable password login via **Settings > Users** if the IdP becomes unavailable. If you set a password at invite time, it keeps working after the user's first OAuth sign-in — both methods coexist independently. ### Environment variables diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 793cdf0..90eb342 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -143,9 +143,10 @@ - Admin dashboard to create, list, and delete users - Assign roles (admin / user) - Reset another user's password — Crumbs generates a 20-character password and shows it once in that user's row, with a copy button; it cannot be shown again - - Only for accounts that sign in with a password — OAuth/SSO accounts show why the reset is unavailable + - Works for any account other than your own, including OAuth-only — the reset deliberately enables password login and revokes all sessions + - **Disable password login** turns off password auth, clears the hash, and revokes sessions (not offered on your own row) - Your own password is changed from Settings > Profile, not here - - A reset does not sign the user out — revoke their sessions separately to force a logout + - Use **Revoke all sessions** separately if you need to force a logout without changing credentials - Revoke all sessions (force logout) for any user other than yourself ### PWA / Offline-First diff --git a/drizzle/0005_green_raider.sql b/drizzle/0005_green_raider.sql new file mode 100644 index 0000000..a8c68aa --- /dev/null +++ b/drizzle/0005_green_raider.sql @@ -0,0 +1,2 @@ +ALTER TABLE `users` ADD `password_login_enabled` integer DEFAULT false NOT NULL;--> statement-breakpoint +UPDATE `users` SET `password_login_enabled` = 1 WHERE `auth_provider` = 'password'; \ No newline at end of file diff --git a/drizzle/meta/0005_snapshot.json b/drizzle/meta/0005_snapshot.json new file mode 100644 index 0000000..84d9b9b --- /dev/null +++ b/drizzle/meta/0005_snapshot.json @@ -0,0 +1,1241 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "a9370796-3530-46f7-a701-d2910836c639", + "prevId": "785b6b61-24b1-4350-b0ca-18946b41e2ad", + "tables": { + "api_keys": { + "name": "api_keys", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "key_hash": { + "name": "key_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "key_prefix": { + "name": "key_prefix", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_used_at": { + "name": "last_used_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "api_keys_user_id_users_id_fk": { + "name": "api_keys_user_id_users_id_fk", + "tableFrom": "api_keys", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "attachments": { + "name": "attachments", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filename": { + "name": "filename", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "mime_type": { + "name": "mime_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "size": { + "name": "size", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "path": { + "name": "path", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "thumbnail_path": { + "name": "thumbnail_path", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "featured": { + "name": "featured", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "attachments_user_id_users_id_fk": { + "name": "attachments_user_id_users_id_fk", + "tableFrom": "attachments", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "attachments_note_id_notes_id_fk": { + "name": "attachments_note_id_notes_id_fk", + "tableFrom": "attachments", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "login_attempts": { + "name": "login_attempts", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "ip": { + "name": "ip", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "success": { + "name": "success", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "timestamp": { + "name": "timestamp", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "login_attempts_ip_timestamp_idx": { + "name": "login_attempts_ip_timestamp_idx", + "columns": [ + "ip", + "timestamp" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "note_collaborators": { + "name": "note_collaborators", + "columns": { + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "added_by": { + "name": "added_by", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "added_at": { + "name": "added_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "note_collaborators_unique": { + "name": "note_collaborators_unique", + "columns": [ + "note_id", + "user_id" + ], + "isUnique": true + }, + "note_collaborators_user_id_idx": { + "name": "note_collaborators_user_id_idx", + "columns": [ + "user_id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "note_collaborators_note_id_notes_id_fk": { + "name": "note_collaborators_note_id_notes_id_fk", + "tableFrom": "note_collaborators", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "note_collaborators_user_id_users_id_fk": { + "name": "note_collaborators_user_id_users_id_fk", + "tableFrom": "note_collaborators", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "note_collaborators_added_by_users_id_fk": { + "name": "note_collaborators_added_by_users_id_fk", + "tableFrom": "note_collaborators", + "tableTo": "users", + "columnsFrom": [ + "added_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "note_links": { + "name": "note_links", + "columns": { + "source_note_id": { + "name": "source_note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "target_note_id": { + "name": "target_note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "note_links_source_idx": { + "name": "note_links_source_idx", + "columns": [ + "source_note_id" + ], + "isUnique": false + }, + "note_links_target_idx": { + "name": "note_links_target_idx", + "columns": [ + "target_note_id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "note_links_source_note_id_notes_id_fk": { + "name": "note_links_source_note_id_notes_id_fk", + "tableFrom": "note_links", + "tableTo": "notes", + "columnsFrom": [ + "source_note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "note_links_target_note_id_notes_id_fk": { + "name": "note_links_target_note_id_notes_id_fk", + "tableFrom": "note_links", + "tableTo": "notes", + "columnsFrom": [ + "target_note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "note_tags": { + "name": "note_tags", + "columns": { + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "tag_id": { + "name": "tag_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "note_tags_note_id_idx": { + "name": "note_tags_note_id_idx", + "columns": [ + "note_id" + ], + "isUnique": false + }, + "note_tags_tag_id_idx": { + "name": "note_tags_tag_id_idx", + "columns": [ + "tag_id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "note_tags_note_id_notes_id_fk": { + "name": "note_tags_note_id_notes_id_fk", + "tableFrom": "note_tags", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "note_tags_tag_id_tags_id_fk": { + "name": "note_tags_tag_id_tags_id_fk", + "tableFrom": "note_tags", + "tableTo": "tags", + "columnsFrom": [ + "tag_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "note_user_state": { + "name": "note_user_state", + "columns": { + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pinned": { + "name": "pinned", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "archived": { + "name": "archived", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "sort_order": { + "name": "sort_order", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": { + "note_user_state_unique": { + "name": "note_user_state_unique", + "columns": [ + "note_id", + "user_id" + ], + "isUnique": true + } + }, + "foreignKeys": { + "note_user_state_note_id_notes_id_fk": { + "name": "note_user_state_note_id_notes_id_fk", + "tableFrom": "note_user_state", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "note_user_state_user_id_users_id_fk": { + "name": "note_user_state_user_id_users_id_fk", + "tableFrom": "note_user_state", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "note_versions": { + "name": "note_versions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "checklist_mode": { + "name": "checklist_mode", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "color": { + "name": "color", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "note_versions_note_id_idx": { + "name": "note_versions_note_id_idx", + "columns": [ + "note_id" + ], + "isUnique": false + }, + "note_versions_note_id_version_unique": { + "name": "note_versions_note_id_version_unique", + "columns": [ + "note_id", + "version" + ], + "isUnique": true + } + }, + "foreignKeys": { + "note_versions_note_id_notes_id_fk": { + "name": "note_versions_note_id_notes_id_fk", + "tableFrom": "note_versions", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "notes": { + "name": "notes", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "color": { + "name": "color", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "pinned": { + "name": "pinned", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "archived": { + "name": "archived", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "trashed": { + "name": "trashed", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "trashed_at": { + "name": "trashed_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "checklist_mode": { + "name": "checklist_mode", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "sort_order": { + "name": "sort_order", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + } + }, + "indexes": { + "notes_user_id_idx": { + "name": "notes_user_id_idx", + "columns": [ + "user_id" + ], + "isUnique": false + }, + "notes_trashed_archived_idx": { + "name": "notes_trashed_archived_idx", + "columns": [ + "trashed", + "archived" + ], + "isUnique": false + }, + "notes_updated_at_idx": { + "name": "notes_updated_at_idx", + "columns": [ + "updated_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "notes_user_id_users_id_fk": { + "name": "notes_user_id_users_id_fk", + "tableFrom": "notes", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "sessions": { + "name": "sessions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "user_agent": { + "name": "user_agent", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ip": { + "name": "ip", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_used_at": { + "name": "last_used_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "sessions_user_id_users_id_fk": { + "name": "sessions_user_id_users_id_fk", + "tableFrom": "sessions", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "shared_notes": { + "name": "shared_notes", + "columns": { + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "token": { + "name": "token", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "shared_notes_token_unique": { + "name": "shared_notes_token_unique", + "columns": [ + "token" + ], + "isUnique": true + } + }, + "foreignKeys": { + "shared_notes_note_id_notes_id_fk": { + "name": "shared_notes_note_id_notes_id_fk", + "tableFrom": "shared_notes", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "sync_log": { + "name": "sync_log", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "note_id": { + "name": "note_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "operation": { + "name": "operation", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "timestamp": { + "name": "timestamp", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "client_id": { + "name": "client_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "sync_log_timestamp_idx": { + "name": "sync_log_timestamp_idx", + "columns": [ + "timestamp" + ], + "isUnique": false + } + }, + "foreignKeys": { + "sync_log_user_id_users_id_fk": { + "name": "sync_log_user_id_users_id_fk", + "tableFrom": "sync_log", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "sync_log_note_id_notes_id_fk": { + "name": "sync_log_note_id_notes_id_fk", + "tableFrom": "sync_log", + "tableTo": "notes", + "columnsFrom": [ + "note_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "tags": { + "name": "tags", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "tags_name_user_unique": { + "name": "tags_name_user_unique", + "columns": [ + "name", + "user_id" + ], + "isUnique": true + } + }, + "foreignKeys": { + "tags_user_id_users_id_fk": { + "name": "tags_user_id_users_id_fk", + "tableFrom": "tags", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "user_preferences": { + "name": "user_preferences", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "key": { + "name": "key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "value": { + "name": "value", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "user_preferences_user_key_unique": { + "name": "user_preferences_user_key_unique", + "columns": [ + "user_id", + "key" + ], + "isUnique": true + } + }, + "foreignKeys": { + "user_preferences_user_id_users_id_fk": { + "name": "user_preferences_user_id_users_id_fk", + "tableFrom": "user_preferences", + "tableTo": "users", + "columnsFrom": [ + "user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "role": { + "name": "role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'user'" + }, + "password_hash": { + "name": "password_hash", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "password_login_enabled": { + "name": "password_login_enabled", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "auth_provider": { + "name": "auth_provider", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'password'" + }, + "provider_id": { + "name": "provider_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index fe15b55..7e7af90 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -36,6 +36,13 @@ "when": 1785824271578, "tag": "0004_violet_nicolaos", "breakpoints": true + }, + { + "idx": 5, + "version": "6", + "when": 1787064677945, + "tag": "0005_green_raider", + "breakpoints": true } ] } \ No newline at end of file diff --git a/playwright.config.ts b/playwright.config.ts index d642b01..7d942cc 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -29,7 +29,14 @@ export default defineConfig({ } ], webServer: { - command: 'pnpm preview --port 4173', + // The wipe runs here (before the server opens the database), not in + // globalSetup: Playwright starts the webServer before globalSetup, so + // deleting the SQLite files later would orphan the server's open + // database onto a deleted inode. Tests that read the database file + // directly (see linkUserToOAuth in admin-users.spec.ts) must see the + // server's real database file. + command: + 'rm -f data/test-crumbs.db data/test-crumbs.db-wal data/test-crumbs.db-shm data/test-crumbs.db-journal && pnpm preview --port 4173', port: 4173, reuseExistingServer: !process.env.CI, timeout: 120_000, diff --git a/src/lib/server/auth.test.ts b/src/lib/server/auth.test.ts index 40755ee..865c39b 100644 --- a/src/lib/server/auth.test.ts +++ b/src/lib/server/auth.test.ts @@ -6,6 +6,13 @@ import * as argon2 from 'argon2'; import { randomBytes, randomUUID } from 'crypto'; import type { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3'; import type * as schema from './db/schema.js'; +import { + verifyPassword, + resetPassword, + findOrLinkOAuthUser, + createUser, + disablePasswordLogin +} from './auth.js'; let db: BetterSQLite3Database; @@ -125,3 +132,115 @@ describe('Auth - Sessions', () => { expect(session).toBeUndefined(); }); }); + +describe('Auth - password login enabled', () => { + it('verifyPassword succeeds when password login is enabled', async () => { + const password = randomUUID(); + const hash = await argon2.hash(password); + await db.insert(users).values({ + email: 'pw@test.com', + displayName: 'PW User', + passwordHash: hash, + passwordLoginEnabled: true, + authProvider: 'password', + createdAt: new Date() + }); + + const user = await verifyPassword('pw@test.com', password, db); + expect(user?.email).toBe('pw@test.com'); + }); + + it('verifyPassword rejects when password login is disabled even with a hash', async () => { + const password = randomUUID(); + const hash = await argon2.hash(password); + await db.insert(users).values({ + email: 'oauth@test.com', + displayName: 'OAuth User', + passwordHash: hash, + passwordLoginEnabled: false, + authProvider: 'google', + providerId: 'gid-1', + createdAt: new Date() + }); + + expect(await verifyPassword('oauth@test.com', password, db)).toBeNull(); + }); + + it('resetPassword enables password login on an OAuth-only account', async () => { + const [row] = await db + .insert(users) + .values({ + email: 'enable@test.com', + displayName: 'Enable', + passwordLoginEnabled: false, + authProvider: 'none', + createdAt: new Date() + }) + .returning(); + + const newPassword = randomUUID(); + await resetPassword(row.id, newPassword, db); + + const updated = await db.select().from(users).where(eq(users.id, row.id)).get(); + expect(updated?.passwordLoginEnabled).toBe(true); + expect(await argon2.verify(updated!.passwordHash!, newPassword)).toBe(true); + expect(await verifyPassword('enable@test.com', newPassword, db)).not.toBeNull(); + }); + + it('findOrLinkOAuthUser preserves password login when linking SSO', async () => { + const password = randomUUID(); + const hash = await argon2.hash(password); + await db.insert(users).values({ + email: 'both@test.com', + displayName: 'Both', + passwordHash: hash, + passwordLoginEnabled: true, + authProvider: 'password', + createdAt: new Date() + }); + + const linked = findOrLinkOAuthUser('google', 'google-id-1', 'both@test.com', 'Both', db); + expect(linked?.authProvider).toBe('google'); + expect(linked?.passwordLoginEnabled).toBe(true); + expect(await verifyPassword('both@test.com', password, db)).not.toBeNull(); + }); + + it('createUser sets passwordLoginEnabled from whether a password was given', async () => { + const withPassword = await createUser('a@test.com', 'A', 'password123', 'user', db); + expect(withPassword.passwordLoginEnabled).toBe(true); + + const oauthOnly = await createUser('b@test.com', 'B', null, 'user', db); + expect(oauthOnly.passwordLoginEnabled).toBe(false); + }); + + it('disablePasswordLogin clears the hash, disables login, and removes sessions', async () => { + const password = randomUUID(); + const hash = await argon2.hash(password); + const [row] = await db + .insert(users) + .values({ + email: 'disable@test.com', + displayName: 'Disable', + passwordHash: hash, + passwordLoginEnabled: true, + authProvider: 'password', + createdAt: new Date() + }) + .returning(); + + const token = randomBytes(32).toString('hex'); + await db.insert(sessions).values({ + id: token, + userId: row.id, + expiresAt: new Date(Date.now() + 86400000) + }); + + disablePasswordLogin(row.id, db); + + const updated = await db.select().from(users).where(eq(users.id, row.id)).get(); + expect(updated?.passwordLoginEnabled).toBe(false); + expect(updated?.passwordHash).toBeNull(); + expect(await verifyPassword('disable@test.com', password, db)).toBeNull(); + expect(await db.select().from(sessions).where(eq(sessions.userId, row.id)).all()).toHaveLength(0); + }); +}); diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index 097db2d..ee1525f 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -1,4 +1,4 @@ -import { db, sqlite } from './db/index.js'; +import { db, sqlite, type Db } from './db/index.js'; import { users, sessions } from './db/schema.js'; import { eq, lt, and } from 'drizzle-orm'; import * as argon2 from 'argon2'; @@ -16,6 +16,7 @@ function toUser(row: typeof users.$inferSelect): User { displayName: row.displayName, role: row.role, authProvider: row.authProvider, + passwordLoginEnabled: row.passwordLoginEnabled, providerId: row.providerId, createdAt: row.createdAt }; @@ -29,19 +30,21 @@ export async function isSetupComplete(): Promise { export async function setupUser( email: string, password: string, - displayName?: string + displayName?: string, + dbInstance: Db = db ): Promise { - const existing = db.select().from(users).get(); + const existing = dbInstance.select().from(users).get(); if (existing) return null; // Already set up const hash = await argon2.hash(password); - const result = db + const result = dbInstance .insert(users) .values({ email, displayName: displayName || email.split('@')[0] || 'Admin', role: 'admin', passwordHash: hash, + passwordLoginEnabled: true, authProvider: 'password', createdAt: new Date() }) @@ -52,12 +55,13 @@ export async function setupUser( export async function verifyPassword( email: string, - password: string + password: string, + dbInstance: Db = db ): Promise { - const user = db + const user = dbInstance .select() .from(users) - .where(and(eq(users.email, email), eq(users.authProvider, 'password'))) + .where(and(eq(users.email, email), eq(users.passwordLoginEnabled, true))) .get(); if (!user || !user.passwordHash) return null; @@ -195,16 +199,18 @@ export async function createUser( email: string, displayName: string, password: string | null, - role: 'admin' | 'user' = 'user' + role: 'admin' | 'user' = 'user', + dbInstance: Db = db ): Promise { const hash = password ? await argon2.hash(password) : null; - const result = db + const result = dbInstance .insert(users) .values({ email, displayName, role, passwordHash: hash, + passwordLoginEnabled: !!password, authProvider: password ? 'password' : 'none', createdAt: new Date() }) @@ -282,7 +288,7 @@ export async function changePassword( newPassword: string ): Promise { const user = db.select().from(users).where(eq(users.id, userId)).get(); - if (!user || !user.passwordHash) return false; + if (!user || !user.passwordHash || !user.passwordLoginEnabled) return false; const valid = await argon2.verify(user.passwordHash, currentPassword); if (!valid) return false; @@ -292,9 +298,26 @@ export async function changePassword( return true; } -export async function resetPassword(userId: number, newPassword: string): Promise { +export async function resetPassword( + userId: number, + newPassword: string, + dbInstance: Db = db +): Promise { const hash = await argon2.hash(newPassword); - db.update(users).set({ passwordHash: hash }).where(eq(users.id, userId)).run(); + dbInstance + .update(users) + .set({ passwordHash: hash, passwordLoginEnabled: true }) + .where(eq(users.id, userId)) + .run(); +} + +export function disablePasswordLogin(userId: number, dbInstance: Db = db): void { + dbInstance + .update(users) + .set({ passwordHash: null, passwordLoginEnabled: false }) + .where(eq(users.id, userId)) + .run(); + dbInstance.delete(sessions).where(eq(sessions.userId, userId)).run(); } export function updateUserRole(userId: number, role: 'admin' | 'user'): void { @@ -303,15 +326,17 @@ export function updateUserRole(userId: number, role: 'admin' | 'user'): void { /** * Find a user by OAuth provider, or by email. Invite-only: rejects if no matching user. + * Linking OAuth updates the provider fields only — password login stays as configured. */ export function findOrLinkOAuthUser( provider: string, providerId: string, email: string, - name: string + name: string, + dbInstance: Db = db ): User | null { // First try exact provider+providerId match - const byProvider = db + const byProvider = dbInstance .select() .from(users) .where(and(eq(users.authProvider, provider), eq(users.providerId, providerId))) @@ -319,11 +344,12 @@ export function findOrLinkOAuthUser( if (byProvider) return toUser(byProvider); // Try matching by email (invite-only: user must already exist) - const byEmail = db.select().from(users).where(eq(users.email, email)).get(); + const byEmail = dbInstance.select().from(users).where(eq(users.email, email)).get(); if (!byEmail) return null; // No matching user — not invited - // Link this OAuth provider to the existing user - db.update(users) + // Link this OAuth provider to the existing user without touching password login. + dbInstance + .update(users) .set({ authProvider: provider, providerId, displayName: byEmail.displayName || name }) .where(eq(users.id, byEmail.id)) .run(); diff --git a/src/lib/server/db/index.ts b/src/lib/server/db/index.ts index b5fba2b..e0640ae 100644 --- a/src/lib/server/db/index.ts +++ b/src/lib/server/db/index.ts @@ -28,6 +28,7 @@ sqlite.exec(` display_name TEXT NOT NULL DEFAULT '', role TEXT NOT NULL DEFAULT 'user', password_hash TEXT, + password_login_enabled INTEGER NOT NULL DEFAULT 0, auth_provider TEXT NOT NULL DEFAULT 'password', provider_id TEXT, created_at INTEGER NOT NULL @@ -269,17 +270,32 @@ if (pwCol && pwCol.notnull) { display_name TEXT NOT NULL DEFAULT '', role TEXT NOT NULL DEFAULT 'user', password_hash TEXT, + password_login_enabled INTEGER NOT NULL DEFAULT 0, auth_provider TEXT NOT NULL DEFAULT 'password', provider_id TEXT, created_at INTEGER NOT NULL ); - INSERT INTO __new_users(id, email, display_name, role, password_hash, auth_provider, provider_id, created_at) - SELECT id, email, display_name, role, password_hash, auth_provider, provider_id, created_at FROM users; + INSERT INTO __new_users(id, email, display_name, role, password_hash, password_login_enabled, auth_provider, provider_id, created_at) + SELECT id, email, display_name, role, password_hash, + CASE WHEN auth_provider = 'password' THEN 1 ELSE 0 END, + auth_provider, provider_id, created_at FROM users; DROP TABLE users; ALTER TABLE __new_users RENAME TO users; `); } +// Migration: add password_login_enabled (independent of OAuth provider) +const userColumnsAfterPw = sqlite + .prepare("PRAGMA table_info('users')") + .all() as { name: string }[]; +if (!userColumnsAfterPw.some((col) => col.name === 'password_login_enabled')) { + sqlite.exec(`ALTER TABLE users ADD COLUMN password_login_enabled INTEGER NOT NULL DEFAULT 0;`); + // Backfill: only accounts whose sign-in method was password keep it enabled. + // OAuth-linked rows may carry a legacy hash; leaving them disabled avoids + // silently widening access — an admin reset is the deliberate enable act. + sqlite.exec(`UPDATE users SET password_login_enabled = 1 WHERE auth_provider = 'password';`); +} + // Re-enable foreign keys sqlite.pragma('foreign_keys = ON'); diff --git a/src/lib/server/db/schema.ts b/src/lib/server/db/schema.ts index 678b8cc..91e103a 100644 --- a/src/lib/server/db/schema.ts +++ b/src/lib/server/db/schema.ts @@ -6,6 +6,9 @@ export const users = sqliteTable('users', { displayName: text('display_name').notNull().default(''), role: text('role', { enum: ['admin', 'user'] }).notNull().default('user'), passwordHash: text('password_hash'), + passwordLoginEnabled: integer('password_login_enabled', { mode: 'boolean' }) + .notNull() + .default(false), authProvider: text('auth_provider').notNull().default('password'), providerId: text('provider_id'), createdAt: integer('created_at', { mode: 'timestamp' }).notNull() diff --git a/src/lib/server/db/test-helpers.ts b/src/lib/server/db/test-helpers.ts index 0c5d7ae..522a80f 100644 --- a/src/lib/server/db/test-helpers.ts +++ b/src/lib/server/db/test-helpers.ts @@ -19,6 +19,7 @@ export function createTestDb(options?: { seedUser?: boolean }) { display_name TEXT NOT NULL DEFAULT '', role TEXT NOT NULL DEFAULT 'user', password_hash TEXT, + password_login_enabled INTEGER NOT NULL DEFAULT 0, auth_provider TEXT NOT NULL DEFAULT 'password', provider_id TEXT, created_at INTEGER NOT NULL @@ -173,6 +174,7 @@ export function createTestDb(options?: { seedUser?: boolean }) { displayName: 'Test User', role: 'admin', authProvider: 'password', + passwordLoginEnabled: true, createdAt: new Date() }).run(); } diff --git a/src/lib/types/index.ts b/src/lib/types/index.ts index a00bb66..d63306b 100644 --- a/src/lib/types/index.ts +++ b/src/lib/types/index.ts @@ -75,6 +75,7 @@ export interface User { displayName: string; role: 'admin' | 'user'; authProvider: string; + passwordLoginEnabled: boolean; providerId: string | null; createdAt: Date; } diff --git a/src/lib/utils/auth-methods.test.ts b/src/lib/utils/auth-methods.test.ts index 007ff57..85ed887 100644 --- a/src/lib/utils/auth-methods.test.ts +++ b/src/lib/utils/auth-methods.test.ts @@ -1,91 +1,95 @@ import { describe, it, expect } from 'vitest'; -import { readFileSync } from 'node:fs'; -import { canResetPassword, passwordResetBlockedReason } from './auth-methods.js'; - -const NON_PASSWORD_PROVIDERS = ['none', 'google', 'github', 'oidc']; +import { + canResetPassword, + passwordResetBlockedReason, + requiresPasswordForAccountDeletion, + canDisablePasswordLogin +} from './auth-methods.js'; describe('canResetPassword', () => { it('allows a reset for a password account', () => { - expect(canResetPassword({ authProvider: 'password' })).toBe(true); + expect(canResetPassword({ authProvider: 'password', passwordLoginEnabled: true })).toBe(true); }); - it('blocks a reset for every non-password provider', () => { - // verifyPassword() filters on authProvider === 'password', so a hash - // written for any other provider could never be used to sign in. - for (const authProvider of NON_PASSWORD_PROVIDERS) { - expect(canResetPassword({ authProvider }), authProvider).toBe(false); - } + it('allows a reset for an OAuth-only account', () => { + expect(canResetPassword({ authProvider: 'none', passwordLoginEnabled: false })).toBe(true); }); - it('blocks a reset for an unenumerated provider', () => { - expect(canResetPassword({ authProvider: 'saml' })).toBe(false); - expect(canResetPassword({ authProvider: '' })).toBe(false); + it('allows a reset for an OAuth-linked account', () => { + expect(canResetPassword({ authProvider: 'google', passwordLoginEnabled: true })).toBe(true); }); }); describe('passwordResetBlockedReason', () => { - it('returns null for a password account', () => { - expect(passwordResetBlockedReason({ authProvider: 'password' })).toBeNull(); - }); - - it('names the provider in the reason', () => { - expect(passwordResetBlockedReason({ authProvider: 'none' })).toBe( - 'This account has no password — it signs in via OAuth.' - ); - expect(passwordResetBlockedReason({ authProvider: 'google' })).toBe( - 'Managed by Google — password login is disabled for this account.' - ); - expect(passwordResetBlockedReason({ authProvider: 'github' })).toBe( - 'Managed by GitHub — password login is disabled for this account.' - ); - expect(passwordResetBlockedReason({ authProvider: 'oidc' })).toBe( - 'Managed by SSO — password login is disabled for this account.' - ); - }); - - it('falls back to a generic reason for an unenumerated provider', () => { - expect(passwordResetBlockedReason({ authProvider: 'saml' })).toBe( - 'Password login is not available for this account.' - ); - }); - - it('always explains a blocked reset', () => { - // authProvider is typed `string`, so no missing branch can be caught by - // the compiler. A null reason here would render a disabled control with - // no explanation — the confusion issue #77 reported. - // - // The Object.prototype keys are the interesting cases: with a plain object - // literal as the lookup table they resolve to inherited members, so a - // nullish fallback never fires and the function returns a function or an - // object in place of its declared string. Asserting typeof rather than - // truthiness is what catches that. - const providers = [ - ...NON_PASSWORD_PROVIDERS, - 'saml', - '', - 'PASSWORD', - 'ldap', - 'unknown', - '__proto__', - 'toString', - 'constructor', - 'hasOwnProperty', - 'valueOf' - ]; - for (const authProvider of providers) { - const user = { authProvider }; - expect(canResetPassword(user), authProvider).toBe(false); - const reason = passwordResetBlockedReason(user); - expect(typeof reason, authProvider).toBe('string'); - expect(reason, authProvider).not.toBe(''); + it('returns null for every account type', () => { + for (const user of [ + { authProvider: 'password', passwordLoginEnabled: true }, + { authProvider: 'none', passwordLoginEnabled: false }, + { authProvider: 'google', passwordLoginEnabled: true }, + { authProvider: 'oidc', passwordLoginEnabled: false } + ]) { + expect(passwordResetBlockedReason(user)).toBeNull(); } }); }); -describe('auth-methods module', () => { - it('does not use Math.random', () => { - // The reset-eligibility rule must be pure and deterministic. - const source = readFileSync(new URL('./auth-methods.ts', import.meta.url), 'utf8'); - expect(source).not.toContain('Math.random'); +describe('requiresPasswordForAccountDeletion', () => { + it('requires password for a pure password account', () => { + expect( + requiresPasswordForAccountDeletion({ authProvider: 'password', passwordLoginEnabled: true }) + ).toBe(true); + }); + + it('requires password for OAuth-only with admin-enabled password login', () => { + expect( + requiresPasswordForAccountDeletion({ authProvider: 'none', passwordLoginEnabled: true }) + ).toBe(true); + }); + + it('does not require password for OAuth-linked accounts even when password login is enabled', () => { + for (const authProvider of ['google', 'github', 'oidc']) { + expect( + requiresPasswordForAccountDeletion({ authProvider, passwordLoginEnabled: true }), + authProvider + ).toBe(false); + } + }); + + it('does not require password for OAuth-only without password login', () => { + expect( + requiresPasswordForAccountDeletion({ authProvider: 'none', passwordLoginEnabled: false }) + ).toBe(false); + }); +}); + +describe('canDisablePasswordLogin', () => { + it('allows disabling for an OAuth-linked user with password login enabled', () => { + expect( + canDisablePasswordLogin({ id: 2, passwordLoginEnabled: true, authProvider: 'google' }, 1) + ).toBe(true); + }); + + it('blocks disabling on the admin own row', () => { + expect( + canDisablePasswordLogin({ id: 1, passwordLoginEnabled: true, authProvider: 'google' }, 1) + ).toBe(false); + }); + + it('blocks disabling when password login is already off', () => { + expect( + canDisablePasswordLogin({ id: 2, passwordLoginEnabled: false, authProvider: 'google' }, 1) + ).toBe(false); + }); + + it('blocks disabling on a password-only account', () => { + expect( + canDisablePasswordLogin({ id: 2, passwordLoginEnabled: true, authProvider: 'password' }, 1) + ).toBe(false); + }); + + it('blocks disabling on OAuth-only with admin-enabled password but no provider link', () => { + expect( + canDisablePasswordLogin({ id: 2, passwordLoginEnabled: true, authProvider: 'none' }, 1) + ).toBe(false); }); }); diff --git a/src/lib/utils/auth-methods.ts b/src/lib/utils/auth-methods.ts index f314c07..71367ed 100644 --- a/src/lib/utils/auth-methods.ts +++ b/src/lib/utils/auth-methods.ts @@ -1,44 +1,51 @@ import type { User } from '$lib/types/index.js'; -/** - * Human-readable reason a password reset is unavailable, keyed by auth provider. - * Anything absent from this map falls back to BLOCKED_REASON_FALLBACK, so the - * blocked path can never be left unexplained. - * - * A Map, not an object literal: an object inherits Object.prototype, so a lookup - * for 'toString' or '__proto__' would resolve to an inherited member instead of - * missing, and the fallback below would never fire. A Map has no such keys, so - * totality holds by construction rather than by a guard that could be dropped. - */ -const BLOCKED_REASONS = new Map([ - ['none', 'This account has no password — it signs in via OAuth.'], - ['google', 'Managed by Google — password login is disabled for this account.'], - ['github', 'Managed by GitHub — password login is disabled for this account.'], - ['oidc', 'Managed by SSO — password login is disabled for this account.'] -]); - -const BLOCKED_REASON_FALLBACK = 'Password login is not available for this account.'; - /** * Whether an admin may set a new password for this account. * - * Password login is gated on `authProvider` — verifyPassword() only matches rows - * where it equals 'password' — so writing a hash for any other provider stores a - * credential that can never be used to sign in. + * Password login is gated on `passwordLoginEnabled`, which resetPassword() sets + * deliberately — so an admin can enable password auth on any account, including + * OAuth-only ones, without clearing their SSO link. */ -export function canResetPassword(user: Pick): boolean { - return user.authProvider === 'password'; +export function canResetPassword(_user: Pick): boolean { + return true; } /** * Why a password reset is blocked, or null when it is allowed. * - * `authProvider` is typed `string`, so the compiler cannot enforce exhaustiveness - * here. The fallback guarantees the invariant this module exists for: whenever - * `canResetPassword` is false, this returns a non-empty explanation, so the UI - * never shows a disabled control without saying why. + * Eligibility is checked in the UI for the signed-in admin's own row; every + * other account may receive a reset that enables password login. */ -export function passwordResetBlockedReason(user: Pick): string | null { - if (canResetPassword(user)) return null; - return BLOCKED_REASONS.get(user.authProvider) ?? BLOCKED_REASON_FALLBACK; +export function passwordResetBlockedReason( + _user: Pick +): string | null { + return null; +} + +/** + * Whether self-service account deletion must verify the user's password. + * + * OAuth-linked accounts may delete with their active SSO session alone, even + * when an admin has enabled password login. Pure password accounts, and + * OAuth-only rows with password login enabled but no provider link, still + * require the password. + */ +export function requiresPasswordForAccountDeletion( + user: Pick +): boolean { + if (user.authProvider === 'password') return true; + if (user.authProvider === 'none' && user.passwordLoginEnabled) return true; + return false; +} + +/** Whether an admin may turn off password login for this account. */ +export function canDisablePasswordLogin( + user: Pick, + adminId: number +): boolean { + if (user.id === adminId || !user.passwordLoginEnabled) return false; + // Only when another sign-in method exists — disabling on a password-only row + // would lock the user out with no OAuth fallback. + return user.authProvider !== 'password' && user.authProvider !== 'none'; } diff --git a/src/routes/(app)/settings/users/+page.svelte b/src/routes/(app)/settings/users/+page.svelte index e2df474..b7eb2cf 100644 --- a/src/routes/(app)/settings/users/+page.svelte +++ b/src/routes/(app)/settings/users/+page.svelte @@ -1,12 +1,13 @@ @@ -468,6 +500,17 @@ {/if} + {#if canDisablePasswordLogin(user, data.user?.id ?? -1)} + + {/if} {#if user.id !== data.user?.id}