diff --git a/CHANGELOG.md b/CHANGELOG.md index 507ec26c3..079cc9712 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ version, see the upgrade guides in `docs/`. - **server:** `Controller.setup()` is internal, like `Model.setup()`, and may be asynchronous: apps override `initialize()` instead. - **server:** `UserMixin.sessionQuery()` is internal: `sessionScope` sets the scopes of the users of logins and sessions. - **server:** With `app.auth.totp`, `UserMixin.login()` doesn't log in users with TOTP and resolves to `null`, and `'post login'` answers `{ success: false, codeRequired: true }` for them, with status 200: they log in with their code through `'post login-code'`. +- **server:** Changing a user's password ends their sessions: sessions store a hash of the user's `hash` at login, and end once it changes. The session of the request stays logged in after `'post password'`, `'post password-reset-confirm'` and the member `patch` and `update` of `UsersController`, e.g. `PATCH /users/self`. Elsewhere, e.g. in other controllers, custom actions or collection patches, it ends too, unless the action calls `ctx.login(user)` afterwards. Sessions from before this release, and logins of users loaded without `hash`, stay valid until their next login. - **admin:** Dialogs validate their data when submitted, like forms, and only resolve once it's valid, where they resolved with invalid data, e.g. with empty `required` fields. - **admin:** `@simplewebauthn/browser` and `lean-qr` are required peer dependencies of `@ditojs/admin`: admin apps install them, also without `login.passkeys` or `login.totp`. @@ -37,6 +38,8 @@ version, see the upgrade guides in `docs/`. - **server:** `UsersController` lets users change their password through `'post password'`, with the current password. Subclasses that set `collection.allow` list it to route it. - **server:** `CredentialMixin` and `CredentialModel` store the credentials of all user models in one table, set in `app.auth.credentials`. Deleting users deletes their credentials. - **server:** Two-factor authentication with TOTP, with `app.auth.totp`, `app.auth.credentials.secret`, sessions stored on the server and the optional peer dependency `otpauth`. Users with TOTP enter a code from their authenticator app or one of their backup codes after the password, through `'post login-code'`, and set up and turn off TOTP through `'get totp'`, `'post totp'`, `'post totp-confirm'` and `'delete totp'`, see [Two-factor authentication](docs/guide/server/controllers.md#two-factor-authentication). Ten wrong codes in a row lock the account for 15 minutes. Passkey logins don't ask for codes. +- **server:** Password resets by email, with `app.auth.passwordReset`, `app.auth.credentials.secret` and sessions stored on the server. `UsersController` adds `'post password-reset'`, which emails a link through the app's `sendPasswordReset()` and answers the same for unknown usernames, and `'post password-reset-confirm'`, which sets the new password with the link's token. Links work once, until they expire after `expires`, `'1h'` by default, and changing the password invalidates them. One `confirmUrl` serves the whole app, see [Password reset](docs/guide/server/controllers.md#password-reset). +- **admin:** `login: { passwordReset: true }` adds a Forgot password? button to the login dialog, which requests the email, and opening the admin with `#password-reset=` and the token of the link asks for the new password, again after validation errors. - **admin:** `login: { passkeys: true }` adds a passkey login to the login dialog, and a Security dialog to the account menu that changes the user's password, and lists, adds and removes the user's passkeys. Requires the new peer dependency `@simplewebauthn/browser`. - **admin:** The login dialog asks users with TOTP for the code of their authenticator app, or a backup code, after the password. `login: { totp: true }` adds a Security dialog to the account menu that changes the user's password, sets up TOTP with a QR code and shows the backup codes, and turns it off again. Requires the new peer dependency `lean-qr`. - **admin:** Dialogs take `settings.minDuration`, the time in milliseconds that they stay visible when they're submitted right after they opened. The code dialog of the login stays for a second, so users see their password manager fill in the code. @@ -59,6 +62,8 @@ version, see the upgrade guides in `docs/`. ### Fixes +- **admin:** Dialogs keep buttons that are wider than their components off their edges. +- **admin:** Failed login requests notify the `message` of their response, e.g. of a passkey login without a challenge, instead of `'Request failed with status code: 400'`. - **admin:** Errors of `context.request()` carry the `status` of their response, as documented. - **admin:** Lists outside of routes, e.g. in dialogs, keep their scope, page, order and filters to themselves, instead of loading the query of the current route and navigating to their changes. Lists in routes still change them through the route. - **router:** A trailing slash of the `prefix` option is removed unless in strict mode, so `new Router({ prefix: '/api/' })` routes `/users` as `/api/users` instead of `/api//users` (#336). diff --git a/docs/api/admin/admin.md b/docs/api/admin/admin.md index 5e56599c3..26bebf557 100644 --- a/docs/api/admin/admin.md +++ b/docs/api/admin/admin.md @@ -47,6 +47,7 @@ new DitoAdmin(element, options) | `options.login.redirectAfterLogin` | `string` | A URL to load after a successful login, instead of resolving the views in place. | | `options.login.passkeys` | `boolean` | Adds a Sign In with Passkey button to the login dialog, and a Security item to the account menu, whose dialog changes the user's password through [`api.users.password`](#users), and lists, adds and removes the user's passkeys, named after their providers. Needs passkeys on the server, see [Passkeys](../../guide/server/controllers.md#passkeys), and the peer dependency `@simplewebauthn/browser`. | | `options.login.totp` | `boolean` | Adds a Security item to the account menu, whose dialog changes the user's password, sets up two-factor authentication with TOTP, with a QR code for the authenticator app and the backup codes, and turns it off again. Needs TOTP on the server, see [Two-factor authentication](../../guide/server/controllers.md#two-factor-authentication), and the peer dependency `lean-qr`. The login dialog asks for the code whenever the server requires one. | +| `options.login.passwordReset` | `boolean` | Adds a Forgot password? button to the login dialog, which asks for the username and posts it to [`api.users.passwordReset`](#users), for the server to email a link. The link opens the admin with `#password-reset=` and the token, which asks for the new password, posts it to `api.users.passwordResetConfirm`, asking again with the server's validation errors, and removes the token from the URL, before asking to log in. Needs password resets on the server, see [Password reset](../../guide/server/controllers.md#password-reset). | | `options.spinner` | `{ size?: string, color?: string } \| null` | Size and color of the loading spinner in the header. `null` hides it. | | `options.account.settings` | `Form \| Promise \| () => …` | The form of the account settings, which the account menu opens at `/account/settings`, to edit the logged-in user through [`api.users.self`](#users). Also a promise or a function returning it, e.g. a dynamic `import()`. Its `label` is the view's label, default `'Settings'`. See [Account settings](../../guide/admin/views.md#account-settings). | @@ -151,6 +152,8 @@ api: { session: { path: 'session', method: 'get' }, self: { path: 'self' }, password: { path: 'password', method: 'post' }, + passwordReset: { path: 'password-reset', method: 'post' }, + passwordResetConfirm: { path: 'password-reset-confirm', method: 'post' }, passkeyLoginOptions: { path: 'passkey-login-options', method: 'post' }, passkeyLogin: { path: 'passkey-login', method: 'post' }, passkeyOptions: { path: 'passkey-options', method: 'post' }, @@ -164,8 +167,9 @@ api: { `users.path` is the collection of the user model; `users: 'users'` is short for `users: { path: 'users' }`. `login`, `logout`, `session`, `self`, -`password`, `loginCode`, the passkey resources of `login.passkeys` and the -TOTP resources of `login.totp` are resources nested under it, shown here with +`password`, `loginCode`, the password reset resources of `login.passwordReset`, +the passkey resources of `login.passkeys` and the TOTP resources of +`login.totp` are resources nested under it, shown here with their defaults, so the admin logs in through `POST users/login`. The server's `UsersController` provides them. The admin reads `user` from the responses of `login` and `session`, and @@ -370,8 +374,9 @@ each failure, until it succeeds or the user cancels. When the response has `codeRequired`, for users with TOTP, it asks for the code of their authenticator app or a backup code, and posts it to `api.users.loginCode`, asking again after wrong codes, and for the password when the user cancels or -the login expires. With -`login.redirectAfterLogin`, a successful login loads that URL instead. +the login expires. With `login.passwordReset`, it requests a password reset +instead when the user chooses Forgot password?, and then asks to log in again. +With `login.redirectAfterLogin`, a successful login loads that URL instead. ##### Return value diff --git a/docs/api/server/application.md b/docs/api/server/application.md index a3fcd82be..77f589eed 100644 --- a/docs/api/server/application.md +++ b/docs/api/server/application.md @@ -123,7 +123,10 @@ app: { id: 'example.com', origins: ['https://example.com', 'https://admin.example.com'] }, - totp: { issuer: 'Example' } + totp: { issuer: 'Example' }, + passwordReset: { + confirmUrl: 'https://example.com/admin/#password-reset={token}' + } } } ``` @@ -131,13 +134,16 @@ app: { | Key | Type | Description | | ------------------------ | -------------------------- | ------------------------------------------------------------------------------------------------ | | `credentials.modelClass` | `string \| Class` | The model that stores the credentials of all user models, using [`CredentialMixin`](model.md#credentialmixin). | -| `credentials.secret` | `string` | A long random string, required by `totp`, that encrypts the TOTP secrets and hashes the backup codes. It is separate from `keys`, which can rotate: changing it makes the stored TOTP secrets and backup codes unusable. | +| `credentials.secret` | `string` | A long random string, required by `totp` and `passwordReset`, that encrypts the TOTP secrets and hashes the backup codes and password reset tokens. It is separate from `keys`, which can rotate: changing it makes the stored TOTP secrets, backup codes and password reset links unusable. | | `passkeys` | `object` | Enables the passkeys of [`UsersController`](controllers.md#userscontroller). Needs `credentials` and a `session` stored on the server, through `modelClass` or `store`, else the setup fails. | | `passkeys.name` | `string` | The name of the app that authenticators show. | | `passkeys.id` | `string` | The domain that the passkeys belong to, the WebAuthn relying party id. Frontends on this domain or its subdomains can use them. | | `passkeys.origins` | `string[]` | The origins of the frontends that log in with passkeys, which may differ from the API's. | | `totp` | `object` | Enables two-factor authentication with TOTP in [`UsersController`](controllers.md#userscontroller). Needs `credentials`, `credentials.secret` and a `session` stored on the server, else the setup fails. | | `totp.issuer` | `string` | The name of the app that authenticator apps show. | +| `passwordReset` | `object` | Enables password resets by email in [`UsersController`](controllers.md#userscontroller), which sends the emails through [`sendPasswordReset()`](controllers.md#sendpasswordreset). Needs `credentials`, `credentials.secret` and a `session` stored on the server, else the setup fails. | +| `passwordReset.confirmUrl` | `string` | The URL of the page that the emails link to, with a `{token}` placeholder for the token, best in the fragment, e.g. `'https://example.com/admin/#password-reset={token}'` for the admin's [`login.passwordReset`](../admin/admin.md). One URL for the whole app: all users controllers send links to it. | +| `passwordReset.expires` | `number \| string` | How long the links are valid, in milliseconds or as a duration string, e.g. `'30m'`. Default `'1h'`. | Passkeys need the optional peer dependency `@simplewebauthn/server`, TOTP needs `otpauth`. diff --git a/docs/api/server/controllers.md b/docs/api/server/controllers.md index a7227817c..5d5bdfedb 100644 --- a/docs/api/server/controllers.md +++ b/docs/api/server/controllers.md @@ -9,7 +9,7 @@ exported by `@ditojs/server`: | [`CollectionController`](#collectioncontroller) | Base class of model and relation controllers. Not extended directly. | | [`ModelController`](#modelcontroller) | CRUD routes for a model's collection and members, plus its relations. | | [`RelationController`](#relationcontroller) | Routes for a relation of a model controller. Created from `relations`, never extended. | -| [`UsersController`](#userscontroller) | A model controller with login, logout, session, passkey and TOTP actions. | +| [`UsersController`](#userscontroller) | A model controller with login, logout, session, passkey, TOTP and password reset actions. | | [`AdminController`](#admincontroller) | Serves Dito.js Admin. | Controllers are registered through the `controllers` option of the @@ -848,6 +848,35 @@ and after the current one for clocks that drift. Each backup code is deleted once used. Passkey logins don't ask for codes. A subclass that sets `collection.allow` has to list the TOTP actions too, else they aren't routed. +With [`app.auth.passwordReset`](./application.md#configappauth), users who +forgot their password set a new one through a link that the app emails them, +with these collection actions, which aren't routed without it: + +| Action | Response | +| -------------------------------- | ------------------------------------------------------- | +| `'post password-reset'` | Emails a link to the user of the username in the body, under the model's `usernameProperty`, through [`sendPasswordReset()`](#sendpasswordreset): `{ success: true }`, also for unknown usernames, or 400 without a username or with one that isn't a string. | +| `'post password-reset-confirm'` | Sets the password of the link's user to `{ token, password }`, validated by the user model: `{ success }`, or 400 with `'Invalid or expired token'`. Doesn't log in. | + +`'post password-reset'` answers with the same response and status whether the +user exists or not, and doesn't wait for the email, but the time it takes can +still differ, as it stores a token for existing users. Users get at most one +email a minute. Limit the rate of requests per client in the proxy in front of +the app. Each link works once, until it expires, and setting the password, also +through `'post password'` or a member `patch` or `update`, invalidates the +user's other links. Only users of the controller's `modelClass` are found, +through their `sessionScope`. A subclass that sets `collection.allow` has to +list the password reset actions too, else they aren't routed, and only +controllers that route `'post password-reset'` need to override +`sendPasswordReset()`. All users controllers of the app share the one +`confirmUrl` of `app.auth.passwordReset`. + +Any change of a user's `hash` ends the sessions of the user. The session of the +request that changes the password stays logged in after `'post password'`, +`'post password-reset-confirm'` and the member `patch` and `update` of +`UsersController`, e.g. `PATCH /users/self`. Elsewhere, e.g. in other +controllers, custom actions or collection patches, it ends too, unless the +action logs the user in again with `ctx.login(user)` afterwards. + TOTP is only asked for by [`UserMixin.login()`](./model.md#static-login), and thus by `'post login'`. `ctx.login(user)` logs in without it, by design, e.g. for impersonation or custom login flows. The credentials of TOTP are stored @@ -856,12 +885,13 @@ user model classes on one table each have their own, so TOTP set up through one isn't asked for by logins through the other. The controller's [`authorize`](#authorize) doesn't apply to `login`, `logout`, -`session`, `password`, the passkey and the TOTP actions, which authenticate -users, or are only allowed for the authenticated user; `self` follows it. Its -member actions default to `authorize: ['$self']`, and are routed when listed -in `allow` like on other model controllers. In member routes, `self` stands -for the authenticated user, see [`getMemberId()`](#getmemberid-1), e.g. -`PATCH /users/self` runs `member.patch` on them. +`session`, `password`, the passkey, TOTP and password reset actions, which +authenticate users, or are only allowed for the authenticated user; `self` +follows it. Its member actions default to `authorize: ['$self']`, and are +routed when listed in `allow` like on other model controllers. In member +routes, `self` stands for the authenticated user, see +[`getMemberId()`](#getmemberid-1), e.g. `PATCH /users/self` runs +`member.patch` on them. ```js import { UsersController } from '@ditojs/server' @@ -880,6 +910,31 @@ const isAuthenticated = controller.isAuthenticated(ctx) Returns whether the request is authenticated with a user of `modelClass`. +### sendPasswordReset() + +```js +export class Users extends UsersController { + modelClass = User + + // @override + async sendPasswordReset(ctx, user, url) { + await this.app.services.mailer.send({ + to: user.email, + subject: 'Reset your password', + text: `Set a new password within the next hour: ${url}` + }) + } +} +``` + +Sends `user` the email with `url`, the `confirmUrl` of +[`app.auth.passwordReset`](./application.md#configappauth) with the token, for +`'post password-reset'`. Controllers that route it override it, else the setup +fails. It may be asynchronous. It's called once the transaction of the request +is committed, if the action is `transacted`, without `ctx.transaction`. The +answer doesn't wait for it, so its errors, also synchronous ones, are only +emitted as `'error'` on the app. + ### getMemberId() ```js diff --git a/docs/api/server/model.md b/docs/api/server/model.md index 03bba3097..0d505b798 100644 --- a/docs/api/server/model.md +++ b/docs/api/server/model.md @@ -1038,7 +1038,10 @@ Returns `boolean`: whether this user is the logged in user of the request, Logged in users are stored in the session as `-`, so several user model classes can be used side by side. When a session is loaded, the user is fetched with the `sessionScope` and set as `ctx.state.user`; users that -no longer exist, or that the session scope excludes, end the session. +no longer exist, that the session scope excludes, or whose `hash` changed since +the login, end the session. The session stores a hash of `hash` for this, and +`UsersController` updates it when users change their own password, so their +own session stays logged in. `ctx.login(user, { session: false })` logs a user in for the request only, and `ctx.isAuthenticated()` tells whether the request has a user. @@ -1091,16 +1094,16 @@ export class Credential extends CredentialModel {} ``` A model to store the credentials of users beyond their password: their -[passkeys and TOTP](controllers.md#userscontroller). One table holds the +[passkeys, TOTP and password reset tokens](controllers.md#userscontroller). One table holds the credentials of all user models. Use it with `app.auth.credentials.modelClass`, the model's name or class, see [`config.app`](application.md#configapp). | Property | Description | | ------------ | --------------------------------------------------------------------------- | -| `type` | `string`, required: `'passkey'`, `'totp'`, one per user, or `'backup-code'`, ten per user with TOTP. | +| `type` | `string`, required: `'passkey'`, `'totp'`, one per user, `'backup-code'`, ten per user with TOTP, or `'password-reset'`. | | `userModel` | `string`, required: the name of the user's model class. | | `userId` | `string`, required: the id of the user, also of integer ids. | -| `key` | `string`, nullable, unique per `userModel`: the credential id of passkeys. | +| `key` | `string`, nullable, unique per `userModel`: the credential id of passkeys, the hash of password reset tokens. | | `name` | `string`, nullable: the name of the provider of passkeys, e.g. `'1Password'`. | | `data` | `object`, hidden: the public key, signature counter and transports of passkeys, the encrypted secret and the time step of the last accepted code of TOTP, the hash of backup codes. | | `createdAt` | `timestamp`, set on insert. | diff --git a/docs/guide/server/controllers.md b/docs/guide/server/controllers.md index 73a0ab588..3a19362da 100644 --- a/docs/guide/server/controllers.md +++ b/docs/guide/server/controllers.md @@ -717,6 +717,67 @@ dialog asks for the code, and its `login.totp` option sets up and turns off TOTP in the account menu, see [`DitoAdmin`](../../api/admin/admin.md#parameters). +### Password reset + +Users who forgot their password can set a new one through a link that the app +emails them. Add the model for the credentials as for passkeys, with a secret +as for TOTP, set the URL of the page that the link opens, and send the emails +in the users controller: + +```js +// config/index.js +export default { + app: { + session: { modelClass: 'Session' }, + auth: { + credentials: { + modelClass: 'Credential', + secret: process.env.CREDENTIALS_SECRET + }, + passwordReset: { + confirmUrl: 'https://example.com/reset-password#token={token}', + expires: '1h' + } + } + } +} +``` + +```js +export class Users extends UsersController { + modelClass = User + + async sendPasswordReset(ctx, user, url) { + await sendEmail({ + to: user.email, + subject: 'Reset your password', + text: `Set a new password within the next hour: ${url}` + }) + } +} +``` + +The token goes in the fragment of the URL, which browsers don't send to +servers, also not in the `Referer` header. The page of the link reads it and +posts it with the new password. The request answers with the same response and +status whether the user exists or not: + +```js +await post('password-reset', { username }) +// On the page of the link: +const token = new URLSearchParams(location.hash.slice(1)).get('token') +await post('password-reset-confirm', { token, password }) +``` + +Each link works once, until it expires, and doesn't log in: the user logs in +with the new password afterwards. Users get at most one email a minute, and +changing the password ends their other sessions. The time of the answer can +still differ for existing users, so limit the rate of requests per client in +the proxy in front of the app. The admin's `login.passwordReset` option adds +the request to its login dialog, with `confirmUrl` pointing to the admin, e.g. +`'https://example.com/admin/#password-reset={token}'`, see +[`DitoAdmin`](../../api/admin/admin.md#parameters). + ## Admin The `AdminController` serves the admin interface at its path. In development diff --git a/packages/admin/src/DitoAdmin.js b/packages/admin/src/DitoAdmin.js index 6e0ee3c76..247595367 100644 --- a/packages/admin/src/DitoAdmin.js +++ b/packages/admin/src/DitoAdmin.js @@ -60,6 +60,8 @@ export default class DitoAdmin { logout: 'post', session: 'get', password: 'post', + passwordReset: 'post', + passwordResetConfirm: 'post', passkeyLoginOptions: 'post', passkeyLogin: 'post', passkeyOptions: 'post', diff --git a/packages/admin/src/components/DitoDialog.vue b/packages/admin/src/components/DitoDialog.vue index fafa93ff0..4bff7937c 100644 --- a/packages/admin/src/components/DitoDialog.vue +++ b/packages/admin/src/components/DitoDialog.vue @@ -25,7 +25,7 @@ generateLabels ) template(#buttons) - DitoButtons.dito-buttons--large( + DitoButtons.dito-dialog__buttons.dito-buttons--large( ref="dialogButtons" :buttons="buttonSchemas" :data="dialogData" @@ -206,6 +206,11 @@ export default { order: -1; } + // Keep buttons that are wider than the components off the edges: + &__buttons { + padding-inline: $content-padding; + } + &__form { position: relative; display: flex; diff --git a/packages/admin/src/components/DitoRoot.test.js b/packages/admin/src/components/DitoRoot.test.js index 3589d1200..7d045b05b 100644 --- a/packages/admin/src/components/DitoRoot.test.js +++ b/packages/admin/src/components/DitoRoot.test.js @@ -182,7 +182,8 @@ describe('DitoRoot', () => { : { user: { id: 1, username: data?.username } }, '/login-code': { user: { id: 3, username: 'totp-user' } }, '/passkey-login-options': { challenge: 'challenge' }, - '/passkey-login': { user: { id: 2, username: 'passkey-user' } } + '/passkey-login': { user: { id: 2, username: 'passkey-user' } }, + '/password-reset': { success: true } } return method === 'post' && url in responses ? { data: responses[url] } @@ -305,6 +306,40 @@ describe('DitoRoot', () => { const { dialog } = await mountLoggedOut({}) expect(dialog.text()).not.toContain('Sign In with Passkey') }) + + it('requests a password reset, and then asks to log in', async () => { + const { admin, request, dialog } = await mountLoggedOut({ + login: { passwordReset: true } + }) + const button = dialog + .findAll('button') + .find(button => button.text() === 'Forgot password?') + await button.trigger('click') + await flushPromises() + // The login dialog may still be leaving: + const reset = admin.wrapper.findAll('.dito-dialog').at(-1) + expect(reset.find('input[name="password"]').exists()).toBe(false) + await enterValue(reset.find('input[name="username"]'), 'reader') + await reset.find('button[type="submit"]').trigger('click') + await flushPromises() + expect(request).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'post', + url: '/password-reset', + data: { username: 'reader' } + }) + ) + expect(admin.wrapper.find('.dito-notification').text()).toContain( + 'If the account exists, we sent you an email.' + ) + const login = admin.wrapper.findAll('.dito-dialog').at(-1) + expect(login.find('input[name="password"]').exists()).toBe(true) + }) + + it('offers password resets only with the option', async () => { + const { dialog } = await mountLoggedOut({}) + expect(dialog.text()).not.toContain('Forgot password?') + }) }) describe('showDialog()', () => { diff --git a/packages/admin/src/components/DitoRoot.vue b/packages/admin/src/components/DitoRoot.vue index 5bb16def4..2d543bd18 100644 --- a/packages/admin/src/components/DitoRoot.vue +++ b/packages/admin/src/components/DitoRoot.vue @@ -97,7 +97,10 @@ import { import { RouteTree } from '../utils/RouteTree.ts' import { FileDropTargets } from '../utils/FileDropTargets.js' import { adminContextKey, fileDropTargetsKey } from '../utils/keys.ts' -import { createCodeComponent } from '../utils/security.js' +import { + createCodeComponent, + createNewPasswordComponents +} from '../utils/security.js' export default { components: { DitoDialog }, @@ -168,9 +171,14 @@ export default { { notify, // Return the entered credentials, `'passkey'` to log in with a passkey, - // or `null` if the user cancels: + // `'passwordReset'` to reset the password, or `null` if the user + // cancels: requestLoginData() { - const { additionalComponents, passkeys } = props.options.login || {} + const { + additionalComponents, + passkeys, + passwordReset + } = props.options.login || {} // Browsers with WebAuthn have `PublicKeyCredential`: const hasPasskeys = passkeys && !!window.PublicKeyCredential return showDialog({ @@ -200,6 +208,14 @@ export default { } }), + ...(passwordReset && { + passwordReset: { + type: 'button', + text: 'Forgot password?', + onClick: ({ closeDialog }) => closeDialog('passwordReset') + } + }), + // `DitoDialog` adds the click event of the cancel button: cancel: { type: 'button', @@ -229,6 +245,53 @@ export default { // Show password managers filling in the code: settings: { minDuration: 1000 } }) + }, + + // Return the username to reset the password of, or `null` if the user + // cancels: + requestPasswordReset() { + return showDialog({ + components: { + username: { + type: 'text', + autocomplete: 'username', + autofocus: true, + required: true + } + }, + buttons: { + send: { + type: 'submit', + text: 'Send Email' + }, + + cancel: { + type: 'button', + text: 'Cancel' + } + }, + settings: { label: 'Reset Password' } + }) + }, + + // Return the new password of a password reset, or `null` if the user + // cancels: + requestNewPassword() { + return showDialog({ + components: createNewPasswordComponents({ autofocus: true }), + buttons: { + change: { + type: 'submit', + text: 'Set Password' + }, + + cancel: { + type: 'button', + text: 'Cancel' + } + }, + settings: { label: 'Set New Password' } + }) } } ) diff --git a/packages/admin/src/utils/Session.test.js b/packages/admin/src/utils/Session.test.js index 66309488d..f6e9a08a1 100644 --- a/packages/admin/src/utils/Session.test.js +++ b/packages/admin/src/utils/Session.test.js @@ -19,6 +19,8 @@ function createSession({ responses = {}, loginData = [{ username: 'tester', password: 'secret' }], loginCodes = [], + passwordResets = [], + newPasswords = [], resolvedViews = [true], redirectAfterLogin = null } = {}) { @@ -29,7 +31,9 @@ function createSession({ session: { method: 'get', path: 'session' }, passkeyLoginOptions: { method: 'post', path: 'passkey-login-options' }, passkeyLogin: { method: 'post', path: 'passkey-login' }, - loginCode: { method: 'post', path: 'login-code' } + loginCode: { method: 'post', path: 'login-code' }, + passwordReset: { method: 'post', path: 'password-reset' }, + passwordResetConfirm: { method: 'post', path: 'password-reset-confirm' } }, resources: { any: resource => `/${resource.path}` }, getApiUrl: ({ url }) => url, @@ -53,6 +57,8 @@ function createSession({ const userInterface = { requestLoginData: vi.fn(async () => loginData.shift() ?? null), requestLoginCode: vi.fn(async () => loginCodes.shift() ?? null), + requestPasswordReset: vi.fn(async () => passwordResets.shift() ?? null), + requestNewPassword: vi.fn(async () => newPasswords.shift() ?? null), notify: vi.fn() } const detach = session.attachUserInterface(userInterface) @@ -108,7 +114,162 @@ describe('Session', () => { }) }) + describe('start() with a password reset link', () => { + const resetUrl = '/admin/books?page=2#password-reset=token' + const getPath = () => location.pathname + location.search + location.hash + + beforeEach(() => { + history.replaceState(null, '', resetUrl) + }) + + afterEach(() => { + history.replaceState(null, '', '/') + }) + + it('sets the new password, and then asks to log in', async () => { + const { session, api, userInterface } = createSession({ + responses: { + 'post /password-reset-confirm': { success: true }, + 'get /session': { user: null }, + 'post /login': { user: { ...user } } + }, + newPasswords: [{ password: 'changed', confirmation: 'changed' }] + }) + await session.start() + expect(api.request).toHaveBeenCalledWith({ + method: 'post', + url: '/password-reset-confirm', + data: { token: 'token', password: 'changed' } + }) + expect(userInterface.notify).toHaveBeenCalledWith({ + type: 'success', + text: 'Your password was changed. Please log in.' + }) + expect(userInterface.requestLoginData).toHaveBeenCalledOnce() + expect(getPath()).toBe('/admin/books?page=2') + }) + + it('notifies invalid tokens, and then asks to log in', async () => { + const error = 'Invalid or expired token' + const { session, userInterface } = createSession({ + responses: { + 'post /password-reset-confirm': Object.assign(new Error(error), { + response: { status: 400, data: { message: error } } + }), + 'get /session': { user: null }, + 'post /login': { user: { ...user } } + }, + newPasswords: [{ password: 'changed', confirmation: 'changed' }] + }) + await session.start() + expect(userInterface.notify).toHaveBeenCalledWith( + expect.objectContaining({ type: 'error', text: error }) + ) + expect(userInterface.requestLoginData).toHaveBeenCalledOnce() + expect(getPath()).toBe('/admin/books?page=2') + }) + + it('asks again with the token for invalid passwords', async () => { + let attempt = 0 + const { session, api, userInterface } = createSession({ + responses: { + 'post /password-reset-confirm': () => { + if (attempt++ === 0) { + throw Object.assign(new Error('Validation error'), { + response: { + status: 400, + data: { + type: 'ModelValidation', + message: 'The provided data is not valid', + errors: { + password: [ + { message: 'must NOT have fewer than 6 characters' } + ] + } + } + } + }) + } + return { success: true } + }, + 'get /session': { user: null }, + 'post /login': { user: { ...user } } + }, + newPasswords: [ + { password: 'short', confirmation: 'short' }, + { password: 'changed', confirmation: 'changed' } + ] + }) + await session.start() + expect(userInterface.notify).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'error', + text: ['password must NOT have fewer than 6 characters'] + }) + ) + expect(userInterface.requestNewPassword).toHaveBeenCalledTimes(2) + expect(api.request).toHaveBeenLastCalledWith( + expect.objectContaining({ url: '/login' }) + ) + expect(api.request).toHaveBeenCalledWith({ + method: 'post', + url: '/password-reset-confirm', + data: { token: 'token', password: 'changed' } + }) + expect(userInterface.notify).toHaveBeenCalledWith({ + type: 'success', + text: 'Your password was changed. Please log in.' + }) + }) + + it('removes the token when the dialog is cancelled', async () => { + const { session, api } = createSession({ + responses: { + 'get /session': { user: null }, + 'post /login': { user: { ...user } } + } + }) + await session.start() + expect(api.request).not.toHaveBeenCalledWith( + expect.objectContaining({ url: '/password-reset-confirm' }) + ) + expect(getPath()).toBe('/admin/books?page=2') + }) + }) + describe('login()', () => { + it('requests a password reset, and then asks to log in', async () => { + const { session, api, userInterface } = createSession({ + responses: { + 'post /password-reset': { success: true }, + 'post /login': { user: { ...user } } + }, + loginData: ['passwordReset', { username: 'tester', password: 'new' }], + passwordResets: [{ username: 'tester' }] + }) + expect(await session.login()).toEqual(user) + expect(api.request).toHaveBeenCalledWith({ + method: 'post', + url: '/password-reset', + data: { username: 'tester' } + }) + expect(userInterface.notify).toHaveBeenCalledWith({ + type: 'info', + text: 'If the account exists, we sent you an email.' + }) + expect(userInterface.requestLoginData).toHaveBeenCalledTimes(2) + }) + + it('asks to log in when the password reset is cancelled', async () => { + const { session, api, userInterface } = createSession({ + responses: { 'post /login': { user: { ...user } } }, + loginData: ['passwordReset', { username: 'tester', password: 'x' }] + }) + expect(await session.login()).toEqual(user) + expect(api.request).toHaveBeenCalledOnce() + expect(userInterface.notify).not.toHaveBeenCalled() + }) + it('asks again after failures, until it succeeds', async () => { let attempt = 0 const { session, userInterface } = createSession({ diff --git a/packages/admin/src/utils/Session.ts b/packages/admin/src/utils/Session.ts index 3db31b2e8..92d760117 100644 --- a/packages/admin/src/utils/Session.ts +++ b/packages/admin/src/utils/Session.ts @@ -8,8 +8,12 @@ import type { Api } from './types.ts' export type User = Record interface UserInterface { - requestLoginData(): Promise | 'passkey' | null> + requestLoginData(): Promise< + Record | 'passkey' | 'passwordReset' | null + > requestLoginCode(): Promise<{ code: string } | null> + requestPasswordReset(): Promise | null> + requestNewPassword(): Promise<{ password: string } | null> notify(options: NotifyOptions): void } @@ -44,14 +48,24 @@ export class Session { } // `requestLoginData()` returns the credentials, `'passkey'` to log in with a - // passkey, or `null` when cancelled. `requestLoginCode()` returns the TOTP or - // backup code of users with TOTP, or `null` when cancelled. + // passkey, `'passwordReset'` to reset the password, or `null` when cancelled. + // `requestLoginCode()` returns the TOTP or backup code of users with TOTP, + // `requestPasswordReset()` the username to reset the password of, and + // `requestNewPassword()` the new password, or `null` when cancelled. attachUserInterface({ requestLoginData, requestLoginCode, + requestPasswordReset, + requestNewPassword, notify }: UserInterface) { - const userInterface = { requestLoginData, requestLoginCode, notify } + const userInterface = { + requestLoginData, + requestLoginCode, + requestPasswordReset, + requestNewPassword, + notify + } this.#userInterface = userInterface return () => { if (this.#userInterface === userInterface) { @@ -61,6 +75,15 @@ export class Session { } async start() { + // The link of password reset emails opens the admin with the token in the + // fragment, which browsers don't send to servers: + const url = new URL(location.href) + const token = new URLSearchParams(url.hash.slice(1)).get('password-reset') + if (token) { + await this.#confirmPasswordReset(token) + url.hash = '' + history.replaceState(history.state, '', url) + } if (await this.fetchUser()) { await this.#resolveViews() } else { @@ -73,6 +96,9 @@ export class Session { const loginData = await this.#userInterface?.requestLoginData() if (!loginData) { return null + } else if (loginData === 'passwordReset') { + await this.#requestPasswordReset() + return this.login() } let user try { @@ -131,6 +157,54 @@ export class Session { } } + async #requestPasswordReset() { + const data = await this.#userInterface?.requestPasswordReset() + if (data) { + try { + await this.#request(this.#api.users.passwordReset, data) + // The server doesn't tell whether the account exists: + this.#userInterface?.notify({ + type: 'info', + text: 'If the account exists, we sent you an email.' + }) + } catch (error) { + this.#notifyError(error) + } + } + } + + async #confirmPasswordReset(token: string) { + const data = await this.#userInterface?.requestNewPassword() + if (data) { + try { + await this.#request(this.#api.users.passwordResetConfirm, { + token, + password: data.password + }) + this.#userInterface?.notify({ + type: 'success', + text: 'Your password was changed. Please log in.' + }) + } catch (error: any) { + // Ask again with the same token for invalid passwords: + const errors: Record | undefined = + error.response?.data?.errors + if (errors) { + this.#userInterface?.notify({ + type: 'error', + title: 'Invalid Password', + text: Object.entries(errors).flatMap(([path, errs]) => + errs.map(({ message }) => `${path} ${message}`) + ) + }) + await this.#confirmPasswordReset(token) + } else { + this.#notifyError(error) + } + } + } + } + // Resolve to whether the user was logged out: async logout(): Promise { try { @@ -185,7 +259,9 @@ export class Session { } #notifyError(error: any) { - const responseError = error.response?.data?.error || error + // Logins answer their errors in `error`, other actions in `message`: + const { data } = error.response ?? {} + const responseError = data?.error || data?.message || error if (this.#userInterface) { this.#userInterface.notify({ type: 'error', diff --git a/packages/admin/src/utils/security.js b/packages/admin/src/utils/security.js index 8232bfada..f41b9f705 100644 --- a/packages/admin/src/utils/security.js +++ b/packages/admin/src/utils/security.js @@ -55,21 +55,7 @@ export function showSecurityDialog(adminContext, { passkeys, totp }) { required: true, autofocus: true }, - password: { - type: 'password', - label: 'New Password', - autocomplete: 'new-password', - required: true - }, - confirmation: { - type: 'password', - label: 'Confirm New Password', - autocomplete: 'new-password', - required: true, - validate: ({ value, item }) => ( - value === item.password || 'must match the new password' - ) - } + ...createNewPasswordComponents() }, buttons: { cancel: {}, @@ -271,6 +257,29 @@ export function createCodeComponent() { } } +// The new password and its confirmation, of the Security dialog and of password +// resets: +export function createNewPasswordComponents({ autofocus = false } = {}) { + return { + password: { + type: 'password', + label: 'New Password', + autocomplete: 'new-password', + autofocus, + required: true + }, + confirmation: { + type: 'password', + label: 'Confirm New Password', + autocomplete: 'new-password', + required: true, + validate: ({ value, item }) => ( + value === item.password || 'must match the new password' + ) + } + } +} + // Two codes per line: function formatBackupCodes(codes) { return codes diff --git a/packages/admin/types/index.d.ts b/packages/admin/types/index.d.ts index e7010b5b3..b0b0f6b87 100644 --- a/packages/admin/types/index.d.ts +++ b/packages/admin/types/index.d.ts @@ -297,6 +297,13 @@ export interface ApiConfig { * with the default path `'password'`. */ password?: { path?: string; method?: HttpMethod } + /** + * The password reset endpoints of `login.passwordReset`, posted to with + * the username and with the token and the new password, with the default + * paths `'password-reset'` and `'password-reset-confirm'`. + */ + passwordReset?: { path?: string; method?: HttpMethod } + passwordResetConfirm?: { path?: string; method?: HttpMethod } /** * The passkey endpoints of `login.passkeys`, posted to, with the default * paths `'passkey-login-options'`, `'passkey-login'` and @@ -3222,6 +3229,12 @@ interface AdminOptions { * the user's two-factor authentication with TOTP. */ totp?: boolean + /** + * Adds a "Forgot password?" button to the login dialog, which asks the + * server to email a link that opens the admin with `#password-reset=` + * and the token, to set a new password. + */ + passwordReset?: boolean } /** * Controls the loading spinner displayed in the diff --git a/packages/server/src/controllers/UsersController.js b/packages/server/src/controllers/UsersController.js index b2e6d27b6..29de49954 100644 --- a/packages/server/src/controllers/UsersController.js +++ b/packages/server/src/controllers/UsersController.js @@ -6,17 +6,21 @@ import { randomBytes } from 'node:crypto' import aaguids from 'aaguids' +import { isString } from '@ditojs/utils' import { ModelController } from './ModelController.js' import { getUserIdentifier, - parseUserIdentifier + parseUserIdentifier, + updatePasswordFingerprint } from '../middleware/handleUser.js' import { authTimeout, backupCodeCount, codeLockTime, - maxCodeFailures + maxCodeFailures, + passwordResetInterval } from '../utils/auth.js' +import { getDuration } from '../utils/duration.js' import { AuthenticationError, AuthorizationError, @@ -36,6 +40,8 @@ export class UsersController extends ModelController { 'get session': true, 'post passkey-login-options': true, 'post passkey-login': true, + 'post password-reset': true, + 'post password-reset-confirm': true, // Users manage their own password and passkeys: 'post password': ctx => this.isAuthenticated(ctx), 'get passkeys': ctx => this.isAuthenticated(ctx), @@ -118,6 +124,7 @@ export class UsersController extends ModelController { await user .$query(ctx.transaction) .patch({ [passwordProperty]: password }) + await handlePasswordChange(this, ctx, user) return { success: true } } else { throw new AuthenticationError({ @@ -128,6 +135,116 @@ export class UsersController extends ModelController { } }, + // Answer the same whether the user exists or not, so that the answer + // doesn't tell which usernames exist: + async 'post password-reset'(ctx) { + const { modelClass } = this + const { usernameProperty } = modelClass.definition.options + const username = ctx.request.body[usernameProperty] + if (!username || !isString(username)) { + throw new ResponseError({ + message: `${username ? 'Invalid' : 'Missing'} ${usernameProperty}`, + status: 400 + }) + } + const user = await modelClass + .sessionQuery(ctx.transaction) + .findOne(usernameProperty, username) + if (user) { + const { confirmUrl, expires } = this.passwordReset + // Lock the user, so that concurrent requests send at most one email: + const token = await modelClass.credentialClass.transaction( + ctx.transaction, + async trx => { + await user.$query(trx).ignoreScope().forUpdate() + const query = getPasswordResetQuery(modelClass, user, trx) + const isThrottled = !!(await query + .clone() + .findOne('createdAt', '>', getTimeAgo(passwordResetInterval))) + if (isThrottled) { + return null + } else { + // Clean up the expired tokens of the user: + await query + .clone() + .where('createdAt', '<=', getTimeAgo(expires)) + .delete() + const token = randomBytes(32).toString('base64url') + await modelClass.credentialClass.query(trx).insert({ + type: 'password-reset', + userModel: modelClass.name, + userId: `${user.$id()}`, + key: hashPasswordResetToken(this, token) + }) + return token + } + } + ) + if (token) { + const url = confirmUrl.replace('{token}', token) + // Don't wait for the email, so that its failures don't change the + // answer, also when `sendPasswordReset()` throws right away: + const send = () => { + Promise.resolve() + .then(() => this.sendPasswordReset(ctx, user, url)) + .catch(err => this.app.emit('error', err, ctx)) + } + // Only send links whose token is committed: + if (ctx.transaction) { + ctx.transaction.on('commit', send) + } else { + send() + } + } + } + return { success: true } + }, + + 'post password-reset-confirm': { + parameters: { + token: { type: 'string', required: true }, + password: { type: 'string', required: true } + }, + + async handler(ctx, { token, password }) { + const { modelClass } = this + // Lock the token, so that concurrent requests can't use it twice: + const user = await modelClass.credentialClass.transaction( + ctx.transaction, + async trx => { + const credential = await modelClass + .credentialQuery(trx) + .findOne({ + type: 'password-reset', + key: hashPasswordResetToken(this, token) + }) + .where('createdAt', '>', getTimeAgo(this.passwordReset.expires)) + .forUpdate() + const user = credential + ? await modelClass.sessionQuery(trx).findById(credential.userId) + : null + if (user) { + const { passwordProperty } = modelClass.definition.options + // The password property validates the password and sets its + // hash, which ends the other sessions of the user: + await user.$query(trx).patch({ [passwordProperty]: password }) + await getPasswordResetQuery(modelClass, user, trx).delete() + } + return user + } + ) + if (user) { + updatePasswordFingerprint(ctx, user) + return { success: true } + } else { + throw new AuthenticationError({ + message: 'Invalid or expired token', + status: 400 + }) + } + } + }, + 'get self'(ctx) { return this.isAuthenticated(ctx) ? this.member.get.call( @@ -398,11 +515,17 @@ export class UsersController extends ModelController { const { session, auth } = this.app.config.app this.passkeys = auth?.passkeys ?? null this.totp = auth?.totp ?? null + this.passwordReset = auth?.passwordReset + ? { + ...auth.passwordReset, + expires: getDuration(auth.passwordReset.expires ?? '1h') + } + : null // Passkey challenges and pending logins are kept in the session, stored on - // the server so that old cookies can't replay them, and both need + // the server so that old cookies can't replay them, and all need // credentials: - const checkAuth = name => { + const checkAuth = (name, needsSecret = false) => { if (!(session?.modelClass || session?.store || session?.ContextStore)) { throw new ControllerError( this, @@ -415,6 +538,13 @@ export class UsersController extends ModelController { `Unable to use ${name} without credentials, see ` + `'app.auth.credentials'.` ) + } else if (needsSecret && !auth.credentials.secret) { + // Credentials are encrypted and hashed with a secret of their own, not + // `app.keys`, since rotating those would make them unusable: + throw new ControllerError( + this, + `Unable to use ${name} without 'app.auth.credentials.secret'.` + ) } } @@ -437,20 +567,34 @@ export class UsersController extends ModelController { `Invalid TOTP, missing 'app.auth.totp.issuer'.` ) } - checkAuth('TOTP') - // TOTP secrets are encrypted with a secret of their own, not `app.keys`, - // since rotating those would make them unreadable: - if (!auth.credentials.secret) { + checkAuth('TOTP', true) + } + if (this.passwordReset) { + if (!this.passwordReset.confirmUrl?.includes('{token}')) { throw new ControllerError( this, - `Unable to use TOTP without 'app.auth.credentials.secret'.` + `Invalid password reset, 'app.auth.passwordReset.confirmUrl' ` + + `needs a '{token}' placeholder.` ) } + checkAuth('password reset', true) } // Set up the routes before importing, so that sub-classes that don't await // `super.setup()` still get them before `initialize()`, and configuration // errors are thrown right away: super.setup() + // Only controllers that route the password reset send its emails, after + // `allow` and `processValues()`: + if ( + Object.hasOwn(this.collection, 'post password-reset') && + this.sendPasswordReset === UsersController.prototype.sendPasswordReset + ) { + throw new ControllerError( + this, + `Unable to use password reset without overriding ` + + `'sendPasswordReset()'.` + ) + } // The libraries are optional peer dependencies, so we only import them // when they are used: const importModule = (name, module) => @@ -474,12 +618,13 @@ export class UsersController extends ModelController { // @override processValues(values) { const processed = super.processValues(values) - // Without passkeys or TOTP, we don't route their actions, which are all in - // `collection`: + // Without passkeys, TOTP or password reset, we don't route their actions, + // which are all in `collection`: if (values === this.collection) { for (const name of [ ...(this.passkeys ? [] : passkeyActions), - ...(this.totp ? [] : totpActions) + ...(this.totp ? [] : totpActions), + ...(this.passwordReset ? [] : passwordResetActions) ]) { delete processed.values[name] } @@ -500,6 +645,18 @@ export class UsersController extends ModelController { } } + // @override + async executeAndFetchById(action, ctx, modify, body = ctx.request.body) { + const result = await super.executeAndFetchById(action, ctx, modify, body) + const { passwordProperty } = this.modelClass.definition.options + if (body[passwordProperty] !== undefined) { + // Users change their password through patches too, e.g. in + // `PATCH /users/self`: + await handlePasswordChange(this, ctx, result) + } + return result + } + isAuthenticated(ctx) { // Make sure the currently logged in user has the exact model class, since // sub-classes have their own tables and ids: @@ -537,6 +694,17 @@ export class UsersController extends ModelController { } } + // Send the user the email with the `url` that resets their password, with + // `app.auth.passwordReset`: + // @overridable + async sendPasswordReset(/* ctx, user, url */) { + throw new ControllerError( + this, + `Unable to send password resets without overriding ` + + `'sendPasswordReset()'.` + ) + } + // Verify a TOTP code, or consume a backup code of the user. Too many wrong // codes in a row lock the account for a while. The credentials stay locked // meanwhile, so that concurrent requests can't use the same code or miss a @@ -616,6 +784,12 @@ const totpActions = [ 'delete totp' ] +// Only routed with `app.auth.passwordReset`, as apps send the emails: +const passwordResetActions = [ + 'post password-reset', + 'post password-reset-confirm' +] + // The lengths of the IV and the authentication tag of AES-256-GCM: const ivLength = 12 const tagLength = 16 @@ -721,6 +895,38 @@ function decrypt(secret, value, user) { ]).toString('utf8') } +// Keep the session of users that change their own password, while their other +// sessions end, and invalidate the links of their password resets: +async function handlePasswordChange(controller, ctx, user) { + if (controller.passwordReset) { + await getPasswordResetQuery( + controller.modelClass, + user, + ctx.transaction + ).delete() + } + updatePasswordFingerprint(ctx, user) +} + +function getPasswordResetQuery(modelClass, user, trx) { + return modelClass.credentialQuery(trx).where({ + type: 'password-reset', + userId: `${user.$id()}` + }) +} + +// Tokens are random, so a fast hash suffices, as for backup codes: +function hashPasswordResetToken(controller, token) { + const { secret } = controller.app.config.app.auth.credentials + return createHmac('sha256', deriveKey(secret, 'password-reset')) + .update(token) + .digest('hex') +} + +function getTimeAgo(duration) { + return new Date(Date.now() - duration) +} + // Backup codes are random, so a fast hash suffices. Keyed with the secret, a // leaked database alone doesn't allow guessing them: function hashBackupCode(secret, code) { diff --git a/packages/server/src/middleware/handleUser.js b/packages/server/src/middleware/handleUser.js index a2985bf37..9e910230a 100644 --- a/packages/server/src/middleware/handleUser.js +++ b/packages/server/src/middleware/handleUser.js @@ -1,20 +1,30 @@ +import { createHash } from 'node:crypto' + export function handleUser() { return async (ctx, next) => { // The session key `passport` stems from koa-passport. We keep it so that // existing sessions remain valid. - const identifier = ctx.session?.passport?.user + const { user: identifier, passwordFingerprint } = ( + ctx.session?.passport ?? {} + ) if (identifier) { const { modelName, id } = parseUserIdentifier(identifier) const userClass = ctx.app.models[modelName] const user = userClass?.sessionQuery ? await userClass.sessionQuery(ctx.transaction).findById(id) : null - if (user) { + if ( + user && ( + // Sessions from before fingerprints were stored stay valid: + passwordFingerprint === undefined || + passwordFingerprint === getPasswordFingerprint(user) + ) + ) { ctx.state.user = user } else { - // End the session of users that no longer exist or are excluded by the - // session scope. - delete ctx.session.passport.user + // End the session of users that no longer exist, are excluded by the + // session scope, or changed their password since. + delete ctx.session.passport } } @@ -33,7 +43,14 @@ export function handleUser() { } else { await user.$emit('before:login', options) if (shouldUseSession) { - session.passport = { user: getUserIdentifier(user) } + session.passport = { + user: getUserIdentifier(user), + // Users loaded without `hash` get sessions without fingerprints, + // like sessions from before them: + ...(user.hash !== undefined && { + passwordFingerprint: getPasswordFingerprint(user) + }) + } // Regenerate the session id against session fixation, which also // saves the session. await session.regenerate() @@ -72,3 +89,20 @@ export function parseUserIdentifier(identifier) { const [, modelName, id] = identifier.match(/^([^-]+)-(.+)$/) ?? [] return { modelName, id } } + +// Keep the session of a user that changed their password, while their other +// sessions end: +export function updatePasswordFingerprint(ctx, user) { + const { passport } = ctx.session ?? {} + if (passport?.user === getUserIdentifier(user)) { + passport.passwordFingerprint = getPasswordFingerprint(user) + } +} + +// Sessions can be stored in cookies, so they store a hash of the password hash, +// not the hash itself: +function getPasswordFingerprint(user) { + return user.hash + ? createHash('sha256').update(user.hash).digest('base64url') + : null +} diff --git a/packages/server/src/middleware/handleUser.test.js b/packages/server/src/middleware/handleUser.test.js index bbfdc66fd..b0d40ba80 100644 --- a/packages/server/src/middleware/handleUser.test.js +++ b/packages/server/src/middleware/handleUser.test.js @@ -5,6 +5,7 @@ import { Model, UserModel } from '../models/index.js' const uuid = '4b0b6f1e-2c3d-4e5f-8a9b-0c1d2e3f4a5b' const goneId = '00000000-0000-4000-8000-000000000000' +const passwordId = '9c8b7a6f-5e4d-4c3b-8a2f-1e0d9c8b7a6f' const events = [] class Reader extends UserModel { @@ -57,6 +58,13 @@ class Readers extends Controller { return getState(ctx) }, + // Log in users loaded without their `hash`: + async 'post id-login'(ctx) { + const { id } = ctx.request.body + await ctx.login(await Reader.query().findById(id).select('id')) + return getState(ctx) + }, + async 'post shelf-login'(ctx) { await ctx.login(Shelf.fromJson({ id: 1 })) return getState(ctx) @@ -140,6 +148,8 @@ describe('handleUser()', { timeout: 30_000 }, () => { } await Reader.query().insert({ id: uuid, username: 'someone' }) await KeyedReader.query().insert({ key: 'k-1', username: 'someone' }) + // `Reader` replaces the properties of `UserMixin`, so we set `hash` here: + await knex('Reader').insert({ id: passwordId, hash: 'hash' }) await app.start() }) @@ -192,6 +202,13 @@ describe('handleUser()', { timeout: 30_000 }, () => { } }) + it('keeps the session of users logged in without their hash', async () => { + expect((await request('id-login', { id: passwordId })).user).toBe( + passwordId + ) + expect((await request('state')).user).toBe(passwordId) + }) + it('refuses to log in models that are not users', async () => { const response = await request('shelf-login', {}) expect(response.message).toBe( diff --git a/packages/server/src/utils/auth.js b/packages/server/src/utils/auth.js index 76ffe6ab1..416f7cbd3 100644 --- a/packages/server/src/utils/auth.js +++ b/packages/server/src/utils/auth.js @@ -7,3 +7,6 @@ export const maxCodeFailures = 10 export const codeLockTime = 15 * 60 * 1000 // 15 minutes export const backupCodeCount = 10 + +// Users get at most one password reset email in this time: +export const passwordResetInterval = 60 * 1000 // 1 minute diff --git a/packages/server/types/index.d.ts b/packages/server/types/index.d.ts index 47ae2e840..acbbd2940 100644 --- a/packages/server/types/index.d.ts +++ b/packages/server/types/index.d.ts @@ -210,7 +210,8 @@ export interface ApplicationConfig { modelClass: string | Class /** * A long random string that encrypts the TOTP secrets and hashes the - * backup codes, required by `totp`. Changing it invalidates them. + * backup codes and password reset tokens, required by `totp` and + * `passwordReset`. Changing it invalidates them. */ secret?: string } @@ -233,6 +234,24 @@ export interface ApplicationConfig { /** The name of the app shown by authenticator apps. */ issuer: string } + /** + * Enables password resets by email, which need `session`, `credentials` + * and `credentials.secret`, and `UsersController.sendPasswordReset()`. + */ + passwordReset?: { + /** + * The URL of the page that the email links to, with a `{token}` + * placeholder, e.g. + * `'https://example.com/admin/#password-reset={token}'`. + */ + confirmUrl: string + /** + * How long the link is valid, in milliseconds or as a duration string. + * + * @defaultValue `'1h'` + */ + expires?: number | string + } } /** * Set signed cookie keys. @@ -2848,6 +2867,11 @@ export class UsersController< * and the user is an instance of the controller's model. */ isAuthenticated(ctx: KoaContext): boolean + /** + * Sends the user the email with the `url` that resets their password, with + * `app.auth.passwordReset`. Apps override it. + */ + sendPasswordReset(ctx: KoaContext, user: M, url: string): void | Promise } export class AdminController extends Controller { @@ -4374,12 +4398,12 @@ export const CredentialMixin: >( target: T ) => T & Constructor<{ - /** `'passkey'`, `'totp'` or `'backup-code'` */ + /** `'passkey'`, `'totp'`, `'backup-code'` or `'password-reset'` */ type: string /** The name of the user's model class. */ userModel: string userId: string - /** The credential id of passkeys. */ + /** The credential id of passkeys, the hash of password reset tokens. */ key: string | null /** The label that the user gave the credential. */ name: string | null diff --git a/tests/e2e/passkeys/admin/__screenshots__/login-passkeys.png b/tests/e2e/passkeys/admin/__screenshots__/login-passkeys.png index 89229ff45..321a96a62 100644 Binary files a/tests/e2e/passkeys/admin/__screenshots__/login-passkeys.png and b/tests/e2e/passkeys/admin/__screenshots__/login-passkeys.png differ diff --git a/tests/e2e/passkeys/admin/__screenshots__/password-reset-confirm.png b/tests/e2e/passkeys/admin/__screenshots__/password-reset-confirm.png new file mode 100644 index 000000000..a7af44d6a Binary files /dev/null and b/tests/e2e/passkeys/admin/__screenshots__/password-reset-confirm.png differ diff --git a/tests/e2e/passkeys/admin/__screenshots__/password-reset.png b/tests/e2e/passkeys/admin/__screenshots__/password-reset.png new file mode 100644 index 000000000..b56b2873d Binary files /dev/null and b/tests/e2e/passkeys/admin/__screenshots__/password-reset.png differ diff --git a/tests/e2e/passkeys/admin/password-reset.spec.ts b/tests/e2e/passkeys/admin/password-reset.spec.ts new file mode 100644 index 000000000..a24c6a1f1 --- /dev/null +++ b/tests/e2e/passkeys/admin/password-reset.spec.ts @@ -0,0 +1,75 @@ +import type { Page } from '@playwright/test' +import { test, expect, sentPasswordResets } from '../fixtures.js' + +async function login(page: Page, password: string) { + const dialog = page.getByRole('dialog') + await dialog.getByLabel('Username').fill('ada') + await dialog.getByLabel('Password').fill(password) + await dialog.getByRole('button', { name: 'Sign In', exact: true }).click() +} + +async function setNewPassword(page: Page, url: string, password: string) { + // Links that only change the fragment of the open page don't load it again, + // so start from a blank page, like a new tab of the email's link: + await page.goto('about:blank') + await page.goto(url) + const dialog = page.getByRole('dialog', { name: 'Set New Password' }) + await dialog.getByLabel('New Password', { exact: true }).fill(password) + await dialog.getByLabel('Confirm New Password').fill(password) + await dialog.getByRole('button', { name: 'Set Password' }).click() + // Wait for the dialog to leave, as the login dialog asks next: + await expect(dialog).toHaveCount(0) +} + +test.describe('password reset', () => { + test('resets the password through the emailed link', async ({ + page, + url + }) => { + // Request the email from the login dialog: + await page.goto(`${url}/admin/`) + await page.getByRole('button', { name: 'Forgot password?' }).click() + const request = page.getByRole('dialog', { name: 'Reset Password' }) + await request.getByLabel('Username').fill('ada') + await request.getByRole('button', { name: 'Send Email' }).click() + await expect(page.locator('.dito-notification')).toContainText( + 'If the account exists, we sent you an email.' + ) + await expect(page.getByRole('dialog').getByLabel('Password')).toBeVisible() + expect(sentPasswordResets).toHaveLength(1) + + // Open the link on the admin's URL, and set a new password: + const { hash } = new URL(sentPasswordResets[0]) + const link = `${url}/admin/${hash}` + await setNewPassword(page, link, 'changed') + await expect(page.locator('.dito-notification').last()).toContainText( + 'Your password was changed. Please log in.' + ) + expect(page.url()).toBe(`${url}/admin/`) + + // The old password fails, the new one logs in: + await login(page, 'secret') + await expect(page.locator('.dito-notification').last()).toContainText( + 'Password or username is incorrect' + ) + await login(page, 'changed') + await expect(page.locator('.dito-account')).toHaveText('ada') + + // The link only works once: + await page.context().clearCookies() + await setNewPassword(page, link, 'another') + await expect(page.locator('.dito-notification').last()).toContainText( + 'Invalid or expired token' + ) + await expect(page.getByRole('dialog').getByLabel('Username')).toBeVisible() + }) + + test('asks for a matching confirmation', async ({ page, url }) => { + await page.goto(`${url}/admin/#password-reset=token`) + const dialog = page.getByRole('dialog', { name: 'Set New Password' }) + await dialog.getByLabel('New Password', { exact: true }).fill('changed') + await dialog.getByLabel('Confirm New Password').fill('different') + await dialog.getByRole('button', { name: 'Set Password' }).click() + await expect(dialog).toContainText('must match the new password') + }) +}) diff --git a/tests/e2e/passkeys/admin/security.screenshot.ts b/tests/e2e/passkeys/admin/security.screenshot.ts index 9a57ab3fd..aec19d629 100644 --- a/tests/e2e/passkeys/admin/security.screenshot.ts +++ b/tests/e2e/passkeys/admin/security.screenshot.ts @@ -110,6 +110,23 @@ test.describe('screenshots', () => { await expectScreenshot(page, 'login-passkeys.png') }) + test('password reset dialog', async ({ page, url }) => { + await page.goto(`${url}/admin/`) + await page.getByRole('button', { name: 'Forgot password?' }).click() + const dialog = page.getByRole('dialog', { name: 'Reset Password' }) + await expect(dialog.getByLabel('Username')).toBeFocused() + await expectScreenshot(page, 'password-reset.png') + }) + + test('set new password dialog', async ({ page, url }) => { + await page.goto(`${url}/admin/#password-reset=token`) + const dialog = page.getByRole('dialog', { name: 'Set New Password' }) + await expect( + dialog.getByLabel('New Password', { exact: true }) + ).toBeFocused() + await expectScreenshot(page, 'password-reset-confirm.png') + }) + test('security dialog without passkeys', async ({ page, url }) => { await login(page, url) const dialog = await openSecurity(page) diff --git a/tests/e2e/passkeys/fixtures.ts b/tests/e2e/passkeys/fixtures.ts index c47abf1b1..feac2942d 100644 --- a/tests/e2e/passkeys/fixtures.ts +++ b/tests/e2e/passkeys/fixtures.ts @@ -7,8 +7,15 @@ import { User } from './models/User.js' export { expect } from '@playwright/test' +// The links of the password reset emails, which the tests open: +export const sentPasswordResets: string[] = [] + class Users extends UsersController { override modelClass = User + + override async sendPasswordReset(ctx: unknown, user: User, url: string) { + sentPasswordResets.push(url) + } } // Browsers only allow passkeys in secure contexts, so the admin runs on @@ -32,12 +39,16 @@ export const test = defineScenario({ normalizePaths: true, helmet: false, keys: ['passkeys-secret'], - // Passkeys and TOTP need sessions stored on the server: + // Passkeys, TOTP and password resets need sessions stored on the server: session: { modelClass: 'Session' }, auth: { credentials: { modelClass: 'Credential', secret: 'credentials-secret' }, passkeys, - totp: { issuer: 'Example' } + totp: { issuer: 'Example' }, + // The tests open the token of the links on the admin's own URL: + passwordReset: { + confirmUrl: 'http://localhost/admin/#password-reset={token}' + } } } }, @@ -52,6 +63,7 @@ export const test = defineScenario({ }) }, beforeEach: async () => { + sentPasswordResets.length = 0 await User.query().insert({ username: 'ada', password: 'secret' } as any) } }) diff --git a/tests/e2e/passkeys/views.ts b/tests/e2e/passkeys/views.ts index 36fd1bb35..a4722eaa2 100644 --- a/tests/e2e/passkeys/views.ts +++ b/tests/e2e/passkeys/views.ts @@ -6,5 +6,6 @@ export const dashboard = { export const login = { passkeys: true, - totp: true + totp: true, + passwordReset: true } as const diff --git a/tests/server/password-reset.test.ts b/tests/server/password-reset.test.ts new file mode 100644 index 000000000..7594c4aff --- /dev/null +++ b/tests/server/password-reset.test.ts @@ -0,0 +1,544 @@ +import type { ModelProperties } from '@ditojs/server' +import { + CredentialModel, + SessionModel, + UserModel, + UsersController +} from '@ditojs/server' +import { createTestApp, getAppUrl } from '../utils/app.js' +import { createClient } from '../utils/client.js' +import { createTestDatabase } from '../utils/database.js' + +const sentResets: { user: string; url: string }[] = [] +// Whether the request still had its transaction when sending: +const sendTransactions: boolean[] = [] + +const getLinkToken = (link: string) => + new URLSearchParams(new URL(link).hash.slice(1)).get('password-reset')! + +class Session extends SessionModel { + declare value: Record +} + +class Credential extends CredentialModel { + declare id: number + declare type: string + declare userModel: string + declare userId: string + declare key: string | null +} + +class Member extends UserModel { + declare id: number + declare username: string + + static override options = { + sessionScope: 'current' + } + + static override properties: ModelProperties = { + password: { + type: 'string', + computed: true, + minLength: 6 + } + } + + static override scopes = { + current: (query: any) => query.whereNot('username', 'retired') + } +} + +// Another user model, whose ids overlap with those of members: +class Guest extends UserModel { + declare id: number +} + +class Members extends UsersController { + override modelClass = Member + + override member: any = { + allow: ['patch'] + } + + // Not async, so that its errors are thrown right away: + override sendPasswordReset(ctx: any, user: Member, url: string) { + if (user.username === 'broken') { + throw new Error('Unable to send') + } + sentResets.push({ user: user.username, url }) + sendTransactions.push(!!ctx.transaction) + } +} + +// Members whose password reset runs in a transaction, which fails on request: +class TransactedMembers extends Members { + override collection: any = { + 'post password-reset': { + transacted: true, + async handler(ctx: any) { + const result = await super['post password-reset'](ctx) + if (ctx.request.body.fail) { + throw new Error('Failed') + } + return result + } + } + } +} + +class Guests extends UsersController { + override modelClass = Guest + + override async sendPasswordReset(ctx: any, user: Guest, url: string) { + sentResets.push({ user: 'guest', url }) + } +} + +const auth = { + credentials: { modelClass: 'Credential', secret: 'credentials-secret' }, + passwordReset: { + confirmUrl: 'https://example.com/admin/#password-reset={token}', + expires: '30m' + } +} + +describe('Password reset', () => { + const app = createTestApp({ + config: { + app: { + normalizePaths: true, + helmet: false, + keys: ['test-secret'], + session: { modelClass: 'Session' }, + auth + } + }, + models: { Session, Credential, Member, Guest }, + controllers: { Members, Guests, TransactedMembers } + }) + + let url: string + let member: Member + + const requestReset = async ( + username: string | Record = 'ada', + path = '/members' + ) => + createClient(url)(`${path}/password-reset`, { + method: 'POST', + body: typeof username === 'string' ? { username } : username + }) + + // Request a reset for ada, and return the token of the sent link: + const getToken = async () => { + await requestReset() + const [{ url }] = sentResets.splice(0) + return getLinkToken(url) + } + + const confirmReset = async ( + token: string, + password = 'changed', + request = createClient(url) + ) => + request('/members/password-reset-confirm', { + method: 'POST', + body: { token, password } + }) + + const login = async (password: string) => { + const request = createClient(url) + const response = await request('/members/login', { + method: 'POST', + body: { username: 'ada', password } + }) + return { request, success: response.data.success } + } + + const isAuthenticated = async (request: ReturnType) => + (await request('/members/session')).data.authenticated + + // Move the creation of ada's tokens back in time: + const ageTokens = (milliseconds: number) => + Credential.query() + .where('type', 'password-reset') + .patch({ createdAt: new Date(Date.now() - milliseconds) } as any) + + beforeAll(async () => { + await createTestDatabase(app, Session, Member, Guest, Credential) + for (const table of ['Member', 'Guest']) { + await app.knex.schema.alterTable(table, table => { + table.dropColumn('password') + }) + } + await app.start() + url = getAppUrl(app) + }) + + beforeEach(async () => { + member = await Member.query().insert({ + username: 'ada', + password: 'secret' + } as any) + }) + + afterAll(async () => { + await app.stop() + await app.knex.destroy() + }) + + afterEach(async () => { + sentResets.length = 0 + sendTransactions.length = 0 + await app.knex('Session').del() + await app.knex('Credential').del() + await app.knex('Member').del() + await app.knex('Guest').del() + }) + + it('sends a link with a token, stored hashed', async () => { + const response = await requestReset() + expect(response.status).toBe(200) + expect(response.data).toEqual({ success: true }) + expect(sentResets).toEqual([ + { + user: 'ada', + url: expect.stringMatching( + /^https:\/\/example\.com\/admin\/#password-reset=[\w-]{43}$/ + ) + } + ]) + const token = getLinkToken(sentResets[0].url) + const credentials = await Credential.query() + expect(credentials).toEqual([ + expect.objectContaining({ + type: 'password-reset', + userModel: 'Member', + userId: `${member.id}` + }) + ]) + expect(credentials[0].key).toMatch(/^[0-9a-f]{64}$/) + expect(JSON.stringify(credentials)).not.toContain(token) + }) + + it('answers the same for unknown usernames', async () => { + const known = await requestReset() + const unknown = await requestReset('nobody') + expect(unknown.status).toBe(known.status) + expect(unknown.data).toEqual(known.data) + expect(sentResets.map(({ user }) => user)).toEqual(['ada']) + }) + + it('answers the same for users excluded by the session scope', async () => { + await Member.query().patchById(member.id, { username: 'retired' }) + const response = await requestReset('retired') + expect(response.status).toBe(200) + expect(response.data).toEqual({ success: true }) + expect(sentResets).toEqual([]) + }) + + it('rejects requests without a valid username', async () => { + for (const [body, message] of [ + [{}, 'Missing username'], + [{ username: ['ada'] }, 'Invalid username'] + ] as const) { + const response = await createClient(url)('/members/password-reset', { + method: 'POST', + body + }) + expect(response.status).toBe(400) + expect(response.data.message).toBe(message) + } + expect(sentResets).toEqual([]) + }) + + it('answers the same when sending fails right away', async () => { + await Member.query().insert({ + username: 'broken', + password: 'secret' + } as any) + const errors: Error[] = [] + const onError = (error: Error) => errors.push(error) + app.on('error', onError) + try { + const response = await requestReset('broken') + expect(response.status).toBe(200) + expect(response.data).toEqual({ success: true }) + await vi.waitFor(() => expect(errors).toHaveLength(1)) + expect(errors[0].message).toBe('Unable to send') + } finally { + app.off('error', onError) + } + }) + + it('sends one link for concurrent requests', async () => { + const responses = await Promise.all( + Array.from({ length: 5 }, () => requestReset()) + ) + expect(responses.map(({ status }) => status)).toEqual(Array(5).fill(200)) + expect(sentResets).toHaveLength(1) + expect(await Credential.query()).toHaveLength(1) + }) + + it('sends links once the transaction of the request commits', async () => { + const response = await requestReset('ada', '/transacted-members') + expect(response.status).toBe(200) + expect(sentResets).toHaveLength(1) + expect(sendTransactions).toEqual([false]) + // Requests that roll back don't send their links: + await ageTokens(61 * 1000) + const failed = await requestReset( + { username: 'ada', fail: true }, + '/transacted-members' + ) + expect(failed.status).toBe(500) + expect(sentResets).toHaveLength(1) + }) + + it('sends at most one link a minute', async () => { + await requestReset() + const response = await requestReset() + expect(response.data).toEqual({ success: true }) + expect(sentResets).toHaveLength(1) + await ageTokens(61 * 1000) + await requestReset() + expect(sentResets).toHaveLength(2) + }) + + it('cleans up expired tokens with new ones', async () => { + await requestReset() + await ageTokens(31 * 60 * 1000) + await requestReset() + expect(await Credential.query()).toHaveLength(1) + }) + + it('sets the new password, without logging in', async () => { + const token = await getToken() + const request = createClient(url) + const response = await confirmReset(token, 'changed', request) + expect(response.status).toBe(200) + expect(response.data).toEqual({ success: true }) + expect(await isAuthenticated(request)).toBe(false) + expect((await login('secret')).success).toBe(false) + expect((await login('changed')).success).toBe(true) + }) + + it('accepts each token only once', async () => { + const token = await getToken() + expect((await confirmReset(token)).status).toBe(200) + const response = await confirmReset(token, 'another') + expect(response.status).toBe(400) + expect(response.data.message).toBe('Invalid or expired token') + expect((await login('changed')).success).toBe(true) + }) + + it('deletes all tokens of the user with the reset', async () => { + const first = await getToken() + await ageTokens(61 * 1000) + const second = await getToken() + expect((await confirmReset(second)).status).toBe(200) + expect((await confirmReset(first, 'another')).status).toBe(400) + expect(await Credential.query()).toEqual([]) + }) + + it('deletes the tokens of the user with password changes', async () => { + for (const [path, method, body] of [ + ['password', 'POST', { currentPassword: 'secret', password: 'changed' }], + ['self', 'PATCH', { password: 'changed' }] + ] as const) { + await Member.query().patchById(member.id, { password: 'secret' } as any) + const token = await getToken() + const { request } = await login('secret') + const response = await request(`/members/${path}`, { method, body }) + expect(response.status).toBe(200) + expect(await Credential.query()).toEqual([]) + expect((await confirmReset(token, 'another')).status).toBe(400) + } + }) + + it('rejects expired tokens', async () => { + const token = await getToken() + await ageTokens(31 * 60 * 1000) + const response = await confirmReset(token) + expect(response.status).toBe(400) + expect(response.data.message).toBe('Invalid or expired token') + expect((await login('secret')).success).toBe(true) + }) + + it('rejects unknown tokens', async () => { + const response = await confirmReset('unknown') + expect(response.status).toBe(400) + expect(response.data.message).toBe('Invalid or expired token') + }) + + it('only accepts tokens for their user model', async () => { + // With the same id as the member: + const guest = await Guest.query().insert({ + id: member.id, + username: 'ada', + password: 'secret' + } as any) + await requestReset('ada', '/guests') + const [{ url: link }] = sentResets + const token = getLinkToken(link) + const response = await confirmReset(token) + expect(response.status).toBe(400) + expect((await login('secret')).success).toBe(true) + const stored = await Guest.query().findById(guest.id) + expect(await stored!.$verifyPassword('secret')).toBe(true) + }) + + it('rejects tokens of users excluded by the session scope', async () => { + const token = await getToken() + await Member.query().patchById(member.id, { username: 'retired' }) + const response = await confirmReset(token) + expect(response.status).toBe(400) + expect(response.data.message).toBe('Invalid or expired token') + }) + + it('validates the new password and keeps the token', async () => { + const token = await getToken() + const response = await confirmReset(token, 'short') + expect(response.status).toBe(400) + expect(response.data.type).toBe('ModelValidation') + expect((await confirmReset(token)).status).toBe(200) + }) + + it('ends the sessions of the user', async () => { + const { request } = await login('secret') + expect(await isAuthenticated(request)).toBe(true) + await confirmReset(await getToken()) + expect(await isAuthenticated(request)).toBe(false) + }) + + it('keeps the session that resets the password', async () => { + const { request } = await login('secret') + const other = (await login('secret')).request + await confirmReset(await getToken(), 'changed', request) + expect(await isAuthenticated(request)).toBe(true) + expect(await isAuthenticated(other)).toBe(false) + }) +}) + +describe('Password reset turned off', () => { + // Models are only set up once, so the app needs its own user model: + class User extends UserModel {} + class Users extends UsersController { + override modelClass = User + } + + const app = createTestApp({ + config: { + app: { normalizePaths: true, keys: ['test-secret'], session: true } + }, + models: { User }, + controllers: { Users } + }) + + afterAll(() => app.knex.destroy()) + + it(`doesn't route the password reset actions`, async () => { + await app.setup() + for (const path of ['password-reset', 'password-reset-confirm']) { + expect(app.router.find('POST', `/users/${path}`).handler).toBeUndefined() + } + }) +}) + +describe('Password reset setup', () => { + const setup = async ( + app: Record, + { isOverridden = true } = {} + ) => { + // Models are only set up once, so each app needs its own user model: + class User extends UserModel {} + class Users extends UsersController { + override modelClass = User + } + class SendingUsers extends Users { + override async sendPasswordReset() {} + } + const testApp = createTestApp({ + config: { app: { keys: ['test-secret'], ...app } }, + models: { Session, Credential, User }, + controllers: { Users: isOverridden ? SendingUsers : Users } + }) + try { + await testApp.setup() + } finally { + await testApp.knex.destroy() + } + } + + const session = { modelClass: 'Session' } + const { passwordReset } = auth + + it('requires sessions stored on the server', async () => { + await expect(setup({ session: true, auth })).rejects.toThrow( + `Unable to use password reset without sessions stored on the server, ` + + `see 'app.session'.` + ) + }) + + it('requires the secret of the credentials', async () => { + await expect( + setup({ + session, + auth: { credentials: { modelClass: 'Credential' }, passwordReset } + }) + ).rejects.toThrow( + `Unable to use password reset without 'app.auth.credentials.secret'.` + ) + }) + + it('requires a confirm URL with a token', async () => { + for (const confirmUrl of [undefined, 'https://example.com/admin/']) { + await expect( + setup({ session, auth: { ...auth, passwordReset: { confirmUrl } } }) + ).rejects.toThrow( + `Invalid password reset, 'app.auth.passwordReset.confirmUrl' needs ` + + `a '{token}' placeholder.` + ) + } + }) + + it('requires sendPasswordReset() only where it is routed', async () => { + class User extends UserModel {} + class Users extends UsersController { + override modelClass = User + override async sendPasswordReset() {} + } + // Users that only log in, as `allow` doesn't route the password reset: + class Admins extends UsersController { + override modelClass = User + override collection: any = { + allow: ['post login', 'post logout', 'get session'] + } + } + const testApp = createTestApp({ + config: { + app: { normalizePaths: true, keys: ['test-secret'], session, auth } + }, + models: { Session, Credential, User }, + controllers: { Users, Admins } + }) + try { + await testApp.setup() + const findHandler = (path: string) => + testApp.router.find('POST', path).handler + expect(findHandler('/users/password-reset')).toBeDefined() + expect(findHandler('/admins/password-reset')).toBeUndefined() + } finally { + await testApp.knex.destroy() + } + }) + + it('requires sendPasswordReset() to be overridden', async () => { + await expect( + setup({ session, auth }, { isOverridden: false }) + ).rejects.toThrow( + `Unable to use password reset without overriding 'sendPasswordReset()'.` + ) + }) +}) diff --git a/tests/server/users-controller.test.ts b/tests/server/users-controller.test.ts index 3c0271af9..e402bf710 100644 --- a/tests/server/users-controller.test.ts +++ b/tests/server/users-controller.test.ts @@ -32,7 +32,8 @@ class Member extends UserModel { } static override scopes = { - current: (query: any) => query.whereNot('username', 'retired') + current: (query: any) => query.whereNot('username', 'retired'), + profile: (query: any) => query.select('id', 'username', 'nickname') } static override hooks: any = { @@ -123,6 +124,24 @@ class Editors extends UsersController { } } +// Members whose patches answer without `hash`, as their scope omits it: +class Profiles extends UsersController { + override modelClass = Member + override scope = 'profile' + override member: any = { + allow: ['patch'] + } +} + +// Members that patch any member, also their password: +class Moderators extends UsersController { + override modelClass = Member + override member: any = { + allow: ['patch'], + authorize: { patch: true } + } +} + class Leads extends UsersController { override modelClass = Lead } @@ -146,7 +165,15 @@ describe('UsersController', () => { } }, models: { Session, Member, Lead }, - controllers: { Members, Leads, Admins, Readers, Editors } + controllers: { + Members, + Leads, + Admins, + Readers, + Editors, + Profiles, + Moderators + } }) let url: string @@ -375,7 +402,7 @@ describe('UsersController', () => { const expectDroppedUser = async (request: any) => { expect((await request('/members/session')).data.authenticated).toBe(false) const [session] = await Session.query() - expect(session.value.passport).toEqual({}) + expect(session.value.passport).toBeUndefined() } it('drops users excluded by the session scope', async () => { @@ -390,6 +417,49 @@ describe('UsersController', () => { await expectDroppedUser(request) }) + it('drops users whose password changed', async () => { + const request = await loginOther() + await Member.query().patchById(other.id, { password: 'changed' } as any) + await expectDroppedUser(request) + }) + + it('keeps sessions from before password fingerprints', async () => { + const request = await loginOther() + const [session] = await Session.query() + await session.$query().patch({ + value: { passport: { user: `Member-${other.id}` } } + } as any) + await Member.query().patchById(other.id, { password: 'changed' } as any) + expect((await request('/members/session')).data.authenticated).toBe(true) + }) + + it('keeps users patched without their hash', async () => { + const request = await loginOther() + const response = await request('/profiles/self', { + method: 'PATCH', + body: { nickname: 'Bobo' } + }) + expect(response.status).toBe(200) + expect((await request('/members/session')).data.authenticated).toBe(true) + }) + + it(`keeps the session that changes others' passwords`, async () => { + const request = createClient(url) + await request('/members/login', { + method: 'POST', + body: { username: 'ada', password: 'secret' } + }) + const response = await request(`/moderators/${other.id}`, { + method: 'PATCH', + body: { password: 'changed' } + }) + expect(response.status).toBe(200) + expect((await request('/members/session')).data).toMatchObject({ + authenticated: true, + user: { id: member.id } + }) + }) + it('passes on errors of the session query', async context => { context.skip(isSQLiteTest, 'SQLite compares text with integer ids') const request = await loginOther() @@ -482,6 +552,8 @@ describe('UsersController', () => { expect(self.data).toMatchObject({ id: other.id, nickname: 'Bobo' }) expect(self.data).not.toHaveProperty('hash') expect((await Member.query().findById(member.id))!.nickname).toBeNull() + // Changing the own password keeps the session: + expect((await request('/members/session')).data.authenticated).toBe(true) // The password is hashed like in other patches: const login = await createClient(url)('/members/login', { method: 'POST', @@ -567,6 +639,27 @@ describe('UsersController', () => { expect(await isPassword('changed')).toBe(true) }) + it('ends the other sessions of the user, but not its own', async () => { + const other = createClient(url) + await other('/readers/login', { + method: 'POST', + body: { username: 'ada', password: 'secret' } + }) + expect((await other('/readers/session')).data.authenticated).toBe(true) + const request = createClient(url) + await request('/readers/login', { + method: 'POST', + body: { username: 'ada', password: 'secret' } + }) + const response = await request('/readers/password', { + method: 'POST', + body: { currentPassword: 'secret', password: 'changed' } + }) + expect(response.status).toBe(200) + expect((await request('/readers/session')).data.authenticated).toBe(true) + expect((await other('/readers/session')).data.authenticated).toBe(false) + }) + it('rejects wrong current passwords', async () => { const response = await changePassword({ currentPassword: 'wrong', diff --git a/tests/types/admin/admin.test-d.ts b/tests/types/admin/admin.test-d.ts index fd56d8b3b..3c2dda4d0 100644 --- a/tests/types/admin/admin.test-d.ts +++ b/tests/types/admin/admin.test-d.ts @@ -66,7 +66,12 @@ describe('DitoAdmin', () => { assertType>([ '#app', { - login: { redirectAfterLogin: '/books', passkeys: true, totp: true }, + login: { + redirectAfterLogin: '/books', + passkeys: true, + totp: true, + passwordReset: true + }, spinner: null } ]) diff --git a/tests/types/server/application.test-d.ts b/tests/types/server/application.test-d.ts index 4ec8ae9f8..8ac6242bb 100644 --- a/tests/types/server/application.test-d.ts +++ b/tests/types/server/application.test-d.ts @@ -72,4 +72,19 @@ describe('Application', () => { } }) }) + + it('accepts a password reset in app.auth', () => { + assertType({ + app: { + session: { modelClass: 'Session' }, + auth: { + credentials: { modelClass: 'Credential', secret: 'secret' }, + passwordReset: { + confirmUrl: 'https://example.com/admin/#password-reset={token}', + expires: '30m' + } + } + } + }) + }) })