From 4882a8088f4481d9b89a68aea669cf802164534b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ju=CC=88rg=20Lehni?= Date: Fri, 16 Oct 2026 22:41:04 +0200 Subject: [PATCH] Server: Replace passport with dito's own login and session handling - Read login credentials from the request body in UserMixin.login() - Restore session users and define ctx.login(), ctx.logout() and ctx.isAuthenticated() in handleUser(), always installed - Keep the passport.user session key, so existing sessions stay valid - Deprecate app.passport, which has no effect anymore - Drop the koa-passport and passport-local dependencies --- CHANGELOG.md | 12 + docs/api/server/application.md | 8 +- docs/api/server/controllers.md | 4 +- docs/api/server/index.md | 2 +- docs/api/server/model.md | 39 +-- docs/guide/server/configuration.md | 3 +- docs/guide/server/controllers.md | 6 +- packages/server/package.json | 2 - packages/server/src/app/Application.js | 13 +- packages/server/src/app/Application.test.js | 4 +- .../server/src/controllers/UsersController.js | 8 +- packages/server/src/middleware/handleUser.js | 71 ++++- .../server/src/middleware/handleUser.test.js | 281 ++++++++++++++---- packages/server/src/mixins/UserMixin.js | 123 ++------ packages/server/src/mixins/UserMixin.test.js | 120 -------- packages/server/types/index.d.ts | 18 +- pnpm-lock.yaml | 49 --- tests/e2e/account/fixtures.ts | 3 +- tests/server/timestamped-user.test.ts | 67 +---- tests/server/users-controller.test.ts | 156 +++++++++- 20 files changed, 516 insertions(+), 473 deletions(-) delete mode 100644 packages/server/src/mixins/UserMixin.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index de09a109c..9769dc1c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ version, see the upgrade guides in `docs/`. - **utils:** `capitalize()` doesn't start new words after digits, e.g. `capitalize('the 1st place')` returns `'The 1st Place'`, where it returned `'The 1St Place'` (#356). - **utils:** `escapeHtml()` escapes single quotes as `'`, so its result is safe inside single-quoted attributes, where it left them unescaped (#359). - **utils:** `isAbsoluteUrl()` only accepts letters, digits, plus, period and hyphen in schemes, e.g. `isAbsoluteUrl('a,b:c')` returns `false`, where commas were accepted too (#360). +- **server:** `UserMixin.login()` reads the credentials only from the request body, where Passport's local strategy also read them from the query string. +- **server:** Dito.js logs users in and restores them from the session itself, instead of through koa-passport. Passport's `ctx.req.user` mirror of `ctx.state.user` and custom Passport strategies through Dito.js are gone. +- **server:** `UsersController` only treats users of its own model class as logged in, not users of its sub-classes, which have their own tables and ids. This affects apps that serve sub-classed user models through the parent's `UsersController`: their `session`, `self` and `logout` actions no longer mistake such users for the parent's user with the same id. +- **server:** `UserMixin.sessionQuery()` is internal: `sessionScope` sets the scopes of the users of logins and sessions. ### New @@ -29,6 +33,14 @@ version, see the upgrade guides in `docs/`. - **admin:** The account menu opens as a floating panel, right-aligned with the user name, and leaves the name unchanged while it is open (#344). - **utils:** `isTypedArray()` checks for typed arrays, e.g. `Uint8Array`, including Node.js buffers (#350). +### Deprecations + +- **server:** `app.passport` has no effect: users are always handled, with sessions when `app.session` is set. + +### Improvements + +- **server:** `@ditojs/server` no longer depends on `koa-passport` and `passport-local`. Existing sessions stay valid. + ### Fixes - **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/server/application.md b/docs/api/server/application.md index e02a3a599..81282af5b 100644 --- a/docs/api/server/application.md +++ b/docs/api/server/application.md @@ -42,7 +42,7 @@ controllers. Nothing is initialized until [`setup()`](#setup) or | `validator` | [`Validator`](validator.md) | A custom validator. Default: `new Validator()`. | | `router` | `object` | A custom router. The app calls its `add(method, path, route)` with the upper-case method and a route object, and `find(method, path)`, which returns `{ status, handler, params, allowed }` with the route object as `handler`. Default: a `@ditojs/router` instance. | | `events` | `Record` | Event listeners, keyed by event name. Several comma-separated names may share one key. See [Events](#events). | -| `middleware` | `Koa.Middleware` | Application-level middleware, installed after the route lookup and before the body parser, transactions, session and passport. Combine several with [koa-compose](https://github.com/koajs/compose). | +| `middleware` | `Koa.Middleware` | Application-level middleware, installed after the route lookup and before the body parser, transactions, session and user handling. Combine several with [koa-compose](https://github.com/koajs/compose). | | `services` | `Record>` | Services to add, see [`addServices()`](#addservices). | | `models` | `Record>` | Models to add, see [`addModels()`](#addmodels). | | `controllers` | `ApplicationControllers` | Controllers to add, see [`addControllers()`](#addcontrollers). | @@ -105,7 +105,7 @@ augmentation of `ApplicationConfig` for project-specific settings. | `etag` | `boolean` | `true` | Conditional GET and ETag headers. | | `bodyParser` | `object` | | Options of [koa-bodyparser](https://github.com/koajs/bodyparser#options). | | `session` | `boolean \| object` | `false` | Sessions through [koa-session](https://github.com/koajs/session). `modelClass` (a name or a model class) stores them in a model, e.g. one using [`SessionMixin`](model.md#sessionmixin). `autoCommit` (default `true`) commits the session after each request, on transacted routes only when the request succeeds. | -| `passport` | `boolean` | `false` | Authentication through koa-passport, with sessions when `session` is set. Needed for [`UserMixin`](model.md#usermixin). | +| `passport` | `boolean` | | Deprecated, without effect: users are always handled, see [`UserMixin`](model.md#usermixin). | ### config.admin @@ -859,8 +859,8 @@ app.setupMiddleware(middleware?) Installs the middleware: logger, response time, request logging, error handling, context extensions, helmet, CORS, compression, ETag, request storage, route -lookup, the passed `middleware`, body parser, transactions, session, passport -with the user handling, and the route handler. Middleware added later through +lookup, the passed `middleware`, body parser, transactions, session, the user +handling, and the route handler. Middleware added later through Koa's `app.use()` sees the requests that the route handler passes on, e.g. those without a route. *Hook.* diff --git a/docs/api/server/controllers.md b/docs/api/server/controllers.md index 1ecfb70f7..79bb58094 100644 --- a/docs/api/server/controllers.md +++ b/docs/api/server/controllers.md @@ -122,7 +122,7 @@ Requests of actions have these members on the Koa context: | `ctx.controller` | The controller. | | `ctx.params` | The route parameters, e.g. `{ id: '1' }`. | | `ctx.route` | The route, see [`addRoute()`](./application.md#addroute). | -| `ctx.logger` | The logger of the request, with its request id (and the user with `app.passport`). | +| `ctx.logger` | The logger of the request, with its request id and user. | | `ctx.memberId` | The validated member id, in member actions. | | `ctx.filteredQuery` | The query parameters not consumed by `parameters`. | | `ctx.transaction` | The transaction of transacted actions. | @@ -175,7 +175,7 @@ type ControllerAuthorize = Authorize | Record | function | Called with `(ctx, member)`; its result is checked like a setting. Returning `undefined` denies, returning `null` authorizes like unset. | Strings and arrays deny requests without an authenticated user -(`ctx.state.user`, set by the application's Passport middleware). +(`ctx.state.user`, see [`UserMixin`](./model.md#sessions)). `authorize` can be set in three places, from strongest to weakest: diff --git a/docs/api/server/index.md b/docs/api/server/index.md index fcbf362e0..cce06e6f4 100644 --- a/docs/api/server/index.md +++ b/docs/api/server/index.md @@ -51,7 +51,7 @@ explains the concepts; these pages list the members. | Export | Description | | --------------------------------------------------------------- | ---------------------------------------------- | | [`TimestampedMixin`, `TimestampedModel`](model.md#timestampedmixin) | `createdAt` and `updatedAt`. | -| [`UserMixin`, `UserModel`](model.md#usermixin) | Users with Passport authentication. | +| [`UserMixin`, `UserModel`](model.md#usermixin) | Users that log in with a password. | | [`SessionMixin`, `SessionModel`](model.md#sessionmixin) | Session storage. | | [`AssetMixin`, `AssetModel`](model.md#assetmixin) | Asset records. | diff --git a/docs/api/server/model.md b/docs/api/server/model.md index 749477eaf..1ab9e25e2 100644 --- a/docs/api/server/model.md +++ b/docs/api/server/model.md @@ -585,7 +585,7 @@ static setup() Called once by [`app.setupModels()`](application.md#setupmodels), before `initialize()`. Internal: dito's mixins override it, e.g. -[`UserMixin`](#usermixin) registers its Passport strategy here, and call +[`UserMixin`](#usermixin) defines its password property here, and call `super.setup()`. Apps override [`initialize()`](#static-initialize) instead, which needs no `super`. @@ -935,9 +935,7 @@ export class User extends UserMixin(Model) { } ``` -Users that log in with a username and password, through -[Passport](https://www.passportjs.org/)'s local strategy. Enable -`app.passport` (and usually `app.session`) in the +Users that log in with a username and password. Enable `app.session` in the [configuration](application.md#configapp), and serve the model through a [`UsersController`](controllers.md#userscontroller) for the `login`, `logout`, `session` and `self` actions. @@ -967,21 +965,11 @@ Set as `static options` on the model: const user = await User.login(ctx, options?) ``` -Authenticates the request's credentials with the model's Passport strategy and -logs the user in with `ctx.login(user, options)`. Resolves to the user. Rejects -with an [`AuthenticationError`](errors.md#authenticationerror): status 401 with -"Password or username is incorrect" for wrong credentials, or the status and -message of the strategy, e.g. 400 "Missing credentials". - -#### `static` sessionQuery() - -```js -const query = User.sessionQuery(trx?) -``` - -The query that loads users for logins and sessions, with the `sessionScope` -scopes applied. Override it to load users differently. *Hook.* Returns -[`QueryBuilder`](query-builder.md). +Authenticates the credentials in the request body, under `usernameProperty` and +`passwordProperty`, and logs the user in with `ctx.login(user, options)`. +Resolves to the user. Rejects with an +[`AuthenticationError`](errors.md#authenticationerror): status 400 with "Missing +credentials", or 401 with "Password or username is incorrect". #### `static` setup() @@ -989,8 +977,8 @@ scopes applied. Override it to load users differently. *Hook.* Returns static setup() ``` -Registers the Passport strategy of the model under its class name, and the -accessor of the password property. Call `super.setup()` when overriding it. +Defines the accessor of the password property. Call `super.setup()` when +overriding it. #### $verifyPassword() @@ -1043,8 +1031,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 `sessionQuery()`; users that no longer exist, or that the -session scope excludes, end the session. +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. +`ctx.login(user, { session: false })` logs a user in for the request only, and +`ctx.isAuthenticated()` tells whether the request has a user. #### Login events @@ -1076,8 +1066,7 @@ const app = new Application({ config: { app: { keys: ['…'], - session: { modelClass: 'Session' }, - passport: true + session: { modelClass: 'Session' } } }, models: { User, Session } diff --git a/docs/guide/server/configuration.md b/docs/guide/server/configuration.md index e8f8208b2..6683126c4 100644 --- a/docs/guide/server/configuration.md +++ b/docs/guide/server/configuration.md @@ -36,7 +36,6 @@ export default { normalizePaths: true, proxy: true, session: { modelClass: 'Session' }, - passport: true, keys: [env.APP_KEY] }, @@ -101,7 +100,7 @@ export default { - `keys`: the secret keys for signed cookies. They also sign uploaded files, see [Storages](storages.md#signatures). Dito.js removes them from `app.config`. - - `session` and `passport`: needed for users who log in, see + - `session`: needed for users who log in, see [Controllers: users and sessions](controllers.md#users-and-sessions). The middleware that is on by default (`helmet`, `cors`, `compress`, `etag`, diff --git a/docs/guide/server/controllers.md b/docs/guide/server/controllers.md index 7addf54df..d6194a5b9 100644 --- a/docs/guide/server/controllers.md +++ b/docs/guide/server/controllers.md @@ -520,17 +520,15 @@ every exported controller is registered and routed. ## Users and sessions -Logging in needs sessions, Passport, a user model and a users controller. +Logging in needs sessions, a user model and a users controller. -The configuration turns on sessions and Passport, and the keys that sign the -session cookie: +The configuration turns on sessions, and the keys that sign the session cookie: ```js // config/index.js export default { app: { session: { modelClass: 'Session' }, - passport: true, keys: [process.env.APP_KEY] } } diff --git a/packages/server/package.json b/packages/server/package.json index 216dc9d09..4cf2e3fdc 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -53,7 +53,6 @@ "koa-conditional-get": "^3.0.0", "koa-helmet": "^9.0.0", "koa-mount": "^4.2.0", - "koa-passport": "^6.0.0", "koa-response-time": "^2.1.0", "koa-session": "^7.0.2", "koa-static": "^5.0.0", @@ -63,7 +62,6 @@ "multer-s3": "github:ditojs/multer-s3#dito", "nanoid": "^6.0.2", "parse-duration": "^2.1.9", - "passport-local": "^1.0.0", "passthrough-counter": "^1.0.0", "picocolors": "^1.1.1", "picomatch": "^4.0.7", diff --git a/packages/server/src/app/Application.js b/packages/server/src/app/Application.js index 2af6acca4..9bd3d2007 100644 --- a/packages/server/src/app/Application.js +++ b/packages/server/src/app/Application.js @@ -13,7 +13,6 @@ import compose from 'koa-compose' import compress from 'koa-compress' import conditional from 'koa-conditional-get' import mount from 'koa-mount' -import passport from 'koa-passport' import helmet from 'koa-helmet' import responseTime from 'koa-response-time' import { Model, knexSnakeCaseMappers, ref } from 'objection' @@ -672,15 +671,9 @@ export class Application extends Koa { if (app.session) { this.use(handleSession(this, getOptions(app.session))) } - // 6. passport - if (app.passport) { - this.use(passport.initialize()) - if (app.session) { - this.use(passport.session()) - } - this.use(handleUser()) - } - // 6. finally handle the found route, or set status / allow accordingly. + // 6. users + this.use(handleUser()) + // 7. finally handle the found route, or set status / allow accordingly. this.use(handleRoute()) } diff --git a/packages/server/src/app/Application.test.js b/packages/server/src/app/Application.test.js index 497632a30..8f93d6b79 100644 --- a/packages/server/src/app/Application.test.js +++ b/packages/server/src/app/Application.test.js @@ -889,8 +889,8 @@ describe('Application', () => { }) }) - it('handles users with passport, also without sessions', async () => { - const app = createKitchenApp({ passport: true }) + it('handles users, also without sessions', async () => { + const app = createKitchenApp() await withServer(app, async url => { const response = await fetch(`${url}/kitchen/login`) expect(await response.json()).toEqual({ hasLogin: true }) diff --git a/packages/server/src/controllers/UsersController.js b/packages/server/src/controllers/UsersController.js index ac03ea315..bbf8de187 100644 --- a/packages/server/src/controllers/UsersController.js +++ b/packages/server/src/controllers/UsersController.js @@ -80,7 +80,11 @@ export class UsersController extends ModelController { } isAuthenticated(ctx) { - // Make sure the currently logged in user has the correct model-class: - return ctx.isAuthenticated() && ctx.state.user instanceof this.modelClass + // Make sure the currently logged in user has the exact model class, since + // sub-classes have their own tables and ids: + return ( + ctx.isAuthenticated() && + ctx.state.user.constructor === this.modelClass + ) } } diff --git a/packages/server/src/middleware/handleUser.js b/packages/server/src/middleware/handleUser.js index a749da352..a2985bf37 100644 --- a/packages/server/src/middleware/handleUser.js +++ b/packages/server/src/middleware/handleUser.js @@ -1,31 +1,74 @@ export function handleUser() { - return (ctx, next) => { + 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 + 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) { + 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 + } + } + // Attach a logger instance with the user to the context. ctx.logger = ctx.logger?.child({ user: ctx.state.user }) || null - // Override `ctx.login()` and `ctx.logout()` with versions that emit events - // on the user model: - const { login, logout } = ctx - - ctx.login = ctx.logIn = async function (user, options = {}) { - await user.$emit('before:login', options) - await login.call(this, user, options) - await user.$emit('after:login', options) + ctx.login = ctx.logIn = async (user, options = {}) => { + const { session } = ctx + const { name, sessionQuery } = user.constructor + const shouldUseSession = options.session !== false + // Check before emitting events, for logins that fail anyway: + if (shouldUseSession && !session) { + throw new Error(`Unable to log in without sessions, see 'app.session'.`) + } else if (shouldUseSession && !sessionQuery) { + throw new Error(`Unable to log in '${name}', which isn't a user.`) + } else { + await user.$emit('before:login', options) + if (shouldUseSession) { + session.passport = { user: getUserIdentifier(user) } + // Regenerate the session id against session fixation, which also + // saves the session. + await session.regenerate() + } + ctx.state.user = user + await user.$emit('after:login', options) + } } - ctx.logout = ctx.logOut = async function (options = {}) { + ctx.logout = ctx.logOut = async (options = {}) => { const { user } = ctx.state await user?.$emit('before:logout', options) - await logout.call(this, options) - // Clear the session after logout, apparently koa-passport doesn't take - // care of this itself: - // https://stackoverflow.com/questions/55818887/koa-passport-logout-is-not-clearing-session + ctx.state.user = null + // Clear the whole session after logout, not just the user: ctx.session = null await user?.$emit('after:logout', options) } + ctx.isAuthenticated = () => !!ctx.state.user + ctx.isUnauthenticated = () => !ctx.state.user + return next() // Don't set back `ctx.logger` because the context is still valid. It is // used in `logResponse()` of the `logRequests()` middleware, among others. } } + +// To support multiple user model classes, sessions identify users by both their +// model class name and id: +export function getUserIdentifier(user) { + return `${user.constructor.name}-${user.$id()}` +} + +export function parseUserIdentifier(identifier) { + // Only split at the first hyphen, since ids may contain hyphens (UUIDs): + const [, modelName, id] = identifier.match(/^([^-]+)-(.+)$/) ?? [] + return { modelName, id } +} diff --git a/packages/server/src/middleware/handleUser.test.js b/packages/server/src/middleware/handleUser.test.js index 4480e67fb..bbfdc66fd 100644 --- a/packages/server/src/middleware/handleUser.test.js +++ b/packages/server/src/middleware/handleUser.test.js @@ -1,80 +1,243 @@ -import { vi } from 'vitest' -import { handleUser } from './handleUser.js' +import { createTestKnex } from '../../../../tests/utils/test-knex.ts' +import { Application } from '../app/Application.js' +import { Controller } from '../controllers/index.js' +import { Model, UserModel } from '../models/index.js' -function createUser(events) { - return { - id: 7, - async $emit(event, options) { - events.push(`${event} ${JSON.stringify(options)}`) +const uuid = '4b0b6f1e-2c3d-4e5f-8a9b-0c1d2e3f4a5b' +const goneId = '00000000-0000-4000-8000-000000000000' +const events = [] + +class Reader extends UserModel { + static properties = { + id: { + type: 'string', + format: 'uuid', + primary: true + } + } + + static hooks = { + 'before:login'(reader, options) { + events.push(`before:login ${reader.id} ${JSON.stringify(options)}`) + }, + 'after:login'(reader) { + events.push(`after:login ${reader.id}`) + }, + 'before:logout'(reader) { + events.push(`before:logout ${reader.id}`) + }, + 'after:logout'(reader) { + events.push(`after:logout ${reader.id}`) } } } -function createContext(events, user = null) { - return { - logger: { - child: bindings => ({ bindings }) +class KeyedReader extends UserModel { + static properties = { + key: { + type: 'string', + primary: true + } + } +} + +class Shelf extends Model { + static hooks = { + 'before:login'() { + events.push('before:login shelf') + } + } +} + +class Readers extends Controller { + actions = { + async 'post login'(ctx) { + const { id, session } = ctx.request.body + await ctx.login(await Reader.query().findById(id), { session }) + return getState(ctx) + }, + + async 'post shelf-login'(ctx) { + await ctx.login(Shelf.fromJson({ id: 1 })) + return getState(ctx) + }, + + async 'post sessionless-login'(ctx) { + // Clear the session the way logouts do: + ctx.session = null + await ctx.login(await Reader.query().findById(ctx.request.body.id)) + return getState(ctx) }, - state: { user }, - session: { visits: 1 }, - async login(user, options) { - events.push(`login ${user.id} ${JSON.stringify(options)}`) + + async 'post logout'(ctx) { + await ctx.logout() + return getState(ctx) + }, + + 'get state'(ctx) { + return getState(ctx) }, - async logout(options) { - events.push(`logout ${JSON.stringify(options)}`) + + // Store identifiers in the session the way logins do: + 'post identifier'(ctx) { + ctx.session.passport = { user: ctx.request.body.identifier } + return null } } } -describe('handleUser()', () => { - it('binds the user to the logger of the context', async () => { - const user = createUser([]) - const ctx = createContext([], user) - const next = vi.fn(() => 'next') - expect(await handleUser()(ctx, next)).toBe('next') - expect(ctx.logger).toEqual({ bindings: { user } }) +function getState(ctx) { + return { + user: ctx.state.user?.$id() ?? null, + authenticated: ctx.isAuthenticated(), + loggedUser: ctx.logger.bindings().user?.id ?? null, + identifier: ctx.session?.passport?.user ?? null + } +} + +// Booting PGlite can exceed the default timeout under full-suite load. +describe('handleUser()', { timeout: 30_000 }, () => { + const app = new Application({ + config: { + app: { + normalizePaths: true, + keys: ['reader-secret'], + session: true + }, + knex: createTestKnex(), + log: { silent: true }, + logger: { level: 'silent' }, + server: { host: '127.0.0.1', port: 0 } + }, + models: { Reader, KeyedReader, Shelf }, + controllers: { Readers } }) - it('sets no logger without a logger on the context', async () => { - const ctx = createContext([]) - delete ctx.logger - await handleUser()(ctx, () => {}) - expect(ctx.logger).toBeNull() + let cookie + const request = async (path, body) => { + const { port } = app.server.address() + const response = await fetch(`http://127.0.0.1:${port}/readers/${path}`, { + method: body ? 'POST' : 'GET', + headers: { cookie, 'content-type': 'application/json' }, + body: body && JSON.stringify(body) + }) + const setCookie = response.headers.getSetCookie() + if (setCookie.length > 0) { + cookie = setCookie.map(entry => entry.split(';')[0]).join('; ') + } + return response.status === 204 ? null : response.json() + } + + beforeAll(async () => { + const { knex } = app + for (const [name, key] of [['Reader', 'id'], ['KeyedReader', 'key']]) { + await knex.schema.createTable(name, table => { + table.string(key).primary() + table.string('username') + table.string('hash') + table.timestamp('lastLogin') + }) + } + await Reader.query().insert({ id: uuid, username: 'someone' }) + await KeyedReader.query().insert({ key: 'k-1', username: 'someone' }) + await app.start() }) - it('emits login events on the user around `ctx.login()`', async () => { - const events = [] - const ctx = createContext(events) - await handleUser()(ctx, () => {}) - expect(ctx.logIn).toBe(ctx.login) - await ctx.login(createUser(events)) - expect(events).toEqual([ - 'before:login {}', - 'login 7 {}', - 'after:login {}' - ]) + afterAll(async () => { + await app.stop() + await app.knex.destroy() }) - it('emits logout events and clears the session on `ctx.logout()`', async () => { - const events = [] - const ctx = createContext(events, createUser(events)) - await handleUser()(ctx, () => {}) - expect(ctx.logOut).toBe(ctx.logout) - await ctx.logout({ keepSessionInfo: true }) - expect(events).toEqual([ - 'before:logout {"keepSessionInfo":true}', - 'logout {"keepSessionInfo":true}', - 'after:logout {"keepSessionInfo":true}' - ]) - expect(ctx.session).toBeNull() + beforeEach(() => { + cookie = '' + events.length = 0 + }) + + it('logs in users and restores them from the session', async () => { + expect((await request('state')).authenticated).toBe(false) + expect(await request('login', { id: uuid })).toEqual({ + user: uuid, + authenticated: true, + loggedUser: null, + identifier: `Reader-${uuid}` + }) + // Restored users are bound to the logger of the request too: + expect(await request('state')).toEqual({ + user: uuid, + authenticated: true, + loggedUser: uuid, + identifier: `Reader-${uuid}` + }) + }) + + it('restores users from identifiers of existing sessions', async () => { + await request('identifier', { identifier: `Reader-${uuid}` }) + expect((await request('state')).user).toBe(uuid) + await request('identifier', { identifier: 'KeyedReader-k-1' }) + expect((await request('state')).user).toBe('k-1') }) - it('logs out without a user', async () => { - const events = [] - const ctx = createContext(events) - await handleUser()(ctx, () => {}) - await ctx.logout() - expect(events).toEqual(['logout {}']) - expect(ctx.session).toBeNull() + it('drops identifiers that do not refer to users', async () => { + for (const identifier of [ + 'Reader', + `Reader-${goneId}`, + 'Shelf-1', + 'Unknown-1' + ]) { + await request('identifier', { identifier }) + expect(await request('state')).toMatchObject({ + user: null, + identifier: null + }) + } + }) + + it('refuses to log in models that are not users', async () => { + const response = await request('shelf-login', {}) + expect(response.message).toBe( + `Unable to log in 'Shelf', which isn't a user.` + ) + expect((await request('state')).authenticated).toBe(false) + expect(events).toEqual([]) + }) + + it('refuses to log in without sessions', async () => { + const response = await request('sessionless-login', { id: uuid }) + expect(response.message).toBe( + `Unable to log in without sessions, see 'app.session'.` + ) + expect(events).toEqual([]) + }) + + it('logs in without a session only for the request', async () => { + expect(await request('login', { id: uuid, session: false })).toEqual({ + user: uuid, + authenticated: true, + loggedUser: null, + identifier: null + }) + expect((await request('state')).authenticated).toBe(false) + }) + + it('logs out users and clears the session', async () => { + await request('login', { id: uuid }) + expect(await request('logout', {})).toEqual({ + user: null, + authenticated: false, + loggedUser: uuid, + identifier: null + }) + expect((await request('state')).authenticated).toBe(false) + }) + + it('emits login and logout events on the user', async () => { + await request('login', { id: uuid }) + await request('logout', {}) + expect(events).toEqual([ + `before:login ${uuid} {}`, + `after:login ${uuid}`, + `before:logout ${uuid}`, + `after:logout ${uuid}` + ]) }) }) diff --git a/packages/server/src/mixins/UserMixin.js b/packages/server/src/mixins/UserMixin.js index 5bcca8650..20aa530d2 100644 --- a/packages/server/src/mixins/UserMixin.js +++ b/packages/server/src/mixins/UserMixin.js @@ -1,7 +1,5 @@ import bcrypt from 'bcryptjs' -import passport from 'koa-passport' -import { Strategy as LocalStrategy } from 'passport-local' -import { mixin, asArray, isString } from '@ditojs/utils' +import { mixin, asArray } from '@ditojs/utils' import { AuthenticationError } from '../errors/index.js' export const UserMixin = mixin( @@ -64,11 +62,7 @@ export const UserMixin = mixin( static setup() { super.setup() - userClasses[this.name] = this - const { - usernameProperty, - passwordProperty - } = this.definition.options + const { passwordProperty } = this.definition.options Object.defineProperty(this.prototype, passwordProperty, { get() { // Nice try ;) @@ -81,66 +75,34 @@ export const UserMixin = mixin( configurable: true }) - passport.use( - this.name, - new LocalStrategy( - { - usernameField: usernameProperty, - passwordField: passwordProperty, - // Wee need the `req` object, so we can get the active database - // transaction through `req.ctx.transaction`: - passReqToCallback: true - }, - async (req, username, password, done) => { - try { - const user = await this.sessionQuery( - req.ctx.transaction - ).findOne(usernameProperty, username) - const res = - user && (await user.$verifyPassword(password)) - ? user - : null - done(null, res) - } catch (err) { - done(err) - } - } - ) - ) } static async login(ctx, options) { - // Unfortunately koa-passport isn't promisified yet so do some wrapping: - return new Promise((resolve, reject) => { - // Use a custom callback to handle authentication, see: - // http://www.passportjs.org/docs/downloads/html/#custom-callback - passport.authenticate( - this.name, - async (err, user, info, status) => { - if (err) { - reject(err) - } else if (user) { - try { - await ctx.login(user, options) - resolve(user) - } catch (err) { - reject(err) - } - } else { - // Passport passes `info` as a string or a `{ message }` - // object, and an optional status, e.g. 400 for missing - // credentials. Without one, `AuthenticationError` uses 401. - reject( - new AuthenticationError({ - message: 'Password or username is incorrect', - ...(isString(info) ? { message: info } : info), - ...(status != null && { status }) - }) - ) - } - } - )(ctx) - }) + const { + usernameProperty, + passwordProperty + } = this.definition.options + const { + [usernameProperty]: username, + [passwordProperty]: password + } = ctx.request.body + if (username && password) { + const user = await this.sessionQuery(ctx.transaction).findOne( + usernameProperty, + username + ) + if (user && (await user.$verifyPassword(password))) { + await ctx.login(user, options) + return user + } else { + throw new AuthenticationError('Password or username is incorrect') + } + } else { + throw new AuthenticationError({ + message: 'Missing credentials', + status: 400 + }) + } } static sessionQuery(trx) { @@ -150,34 +112,3 @@ export const UserMixin = mixin( } } ) - -const userClasses = {} - -// NOTE: We can't use toCallback() here since passport checks function arity to -// determine sequence of arguments received, and `req` would not be included: -passport.serializeUser((req, user, done) => { - // To support multiple user model classes, use both the model class name and - // id as identifier. - const modelName = user?.constructor.name - const identifier = - modelName && userClasses[modelName] - ? `${modelName}-${user.$id()}` - : null - done(null, identifier) -}) - -passport.deserializeUser(async (req, identifier, done) => { - // Only split at the first hyphen, since ids may contain hyphens (UUIDs). - const [, modelName, userId] = identifier.match(/^([^-]+)-(.+)$/) ?? [] - const userClass = userClasses[modelName] - try { - const user = userClass - ? await userClass.sessionQuery(req.ctx.transaction).findById(userId) - : null - // Passing `false` ends the session of users that no longer exist or are - // excluded by the session scope, while `undefined` means "not handled". - done(null, user ?? false) - } catch (err) { - done(err) - } -}) diff --git a/packages/server/src/mixins/UserMixin.test.js b/packages/server/src/mixins/UserMixin.test.js deleted file mode 100644 index f6b6c641c..000000000 --- a/packages/server/src/mixins/UserMixin.test.js +++ /dev/null @@ -1,120 +0,0 @@ -import passport from 'koa-passport' -import { Application } from '../app/Application.js' -import { AuthenticationError } from '../errors/index.js' -import { UserModel } from '../models/UserModel.js' - -class SessionUser extends UserModel { - static properties = { - id: { - type: 'string', - format: 'uuid', - primary: true - } - } - - static sessionQuery() { - return { - findById: id => SessionUser.fromJson({ id, username: 'someone' }) - } - } -} - -class KeyedSessionUser extends UserModel { - static properties = { - key: { - type: 'string', - primary: true - } - } - - static sessionQuery() { - return { - findById: key => KeyedSessionUser.fromJson({ key, username: 'someone' }) - } - } -} - -const app = new Application({ - config: { log: { silent: true } }, - models: { SessionUser, KeyedSessionUser } -}) - -beforeAll(() => app.setupModels()) - -function serializeUser(user) { - return new Promise((resolve, reject) => { - passport.serializeUser(user, {}, (err, identifier) => - err ? reject(err) : resolve(identifier) - ) - }) -} - -function deserializeUser(identifier) { - return new Promise((resolve, reject) => { - passport.deserializeUser(identifier, { ctx: {} }, (err, user) => - err ? reject(err) : resolve(user) - ) - }) -} - -describe('UserMixin session serialization', () => { - const uuid = '4b0b6f1e-2c3d-4e5f-8a9b-0c1d2e3f4a5b' - - it('round-trips users with hyphenated ids', async () => { - const user = SessionUser.fromJson({ id: uuid, username: 'someone' }) - const identifier = await serializeUser(user) - expect(identifier).toBe(`SessionUser-${uuid}`) - const deserializedUser = await deserializeUser(identifier) - expect(deserializedUser).toBeInstanceOf(SessionUser) - expect(deserializedUser.id).toBe(uuid) - }) - - it('round-trips users with a custom id column', async () => { - const user = KeyedSessionUser.fromJson({ key: 'k1', username: 'someone' }) - const identifier = await serializeUser(user) - expect(identifier).toBe('KeyedSessionUser-k1') - const deserializedUser = await deserializeUser(identifier) - expect(deserializedUser).toBeInstanceOf(KeyedSessionUser) - expect(deserializedUser.key).toBe('k1') - }) - - it('ignores identifiers without a separator', async () => { - expect(await deserializeUser('SessionUserX')).toBeFalsy() - }) -}) - -describe('UserMixin.login()', () => { - it('passes the strategy status on to the AuthenticationError', async () => { - // `passport-local` fails missing credentials with status 400. - const ctx = { - req: {}, - request: { body: {} }, - query: {}, - state: {}, - set() {} - } - const error = await SessionUser.login(ctx).catch(error => error) - expect(error).toBeInstanceOf(AuthenticationError) - expect(error.message).toBe('Missing credentials') - expect(error.status).toBe(400) - }) - - it('uses string infos of strategies as error messages', async () => { - // Custom strategies may fail with a message string and no status. - passport.use('SessionUser', { - authenticate() { - this.fail('Library card expired') - } - }) - try { - const ctx = { req: {}, request: { body: {} }, state: {}, set() {} } - const error = await SessionUser.login(ctx).catch(error => error) - expect(error).toBeInstanceOf(AuthenticationError) - expect(error.message).toBe('Library card expired') - expect(error.status).toBe(401) - } finally { - // Restore the local strategy of the model. - SessionUser.setup() - } - }) -}) diff --git a/packages/server/types/index.d.ts b/packages/server/types/index.d.ts index 2625a3ef4..0d0e9e8f0 100644 --- a/packages/server/types/index.d.ts +++ b/packages/server/types/index.d.ts @@ -193,9 +193,8 @@ export interface ApplicationConfig { modelClass?: string | Class }) /** - * Enable passport authentication middleware - * - * @defaultValue `false` + * @deprecated Users are always handled, with sessions when `session` is + * set. This setting has no effect. */ passport?: boolean /** @@ -592,7 +591,7 @@ export class Application<$Models extends Models = Models> { setupKnex(): void /** * Configures all Koa middleware (logger, error - * handling, CORS, compression, session, passport, + * handling, CORS, compression, session, users, * etc.). */ setupMiddleware(middleware?: Koa.Middleware): void @@ -4356,21 +4355,24 @@ export const UserMixin: >( usernameProperty?: string passwordProperty?: string /** - * This option can be used to specify (eager) scopes to be applied when - * the user is deserialized from the session. + * Scopes applied when loading the user of a login or a session. */ sessionScope?: OrArrayOf } - /** Registers the passport strategy for this user class. */ + /** Defines the password property that sets the hash. */ setup(): void - /** Authenticates a user via Passport. */ + /** + * Authenticates a user with the credentials of the request body, and + * logs them in. + */ login( ctx: KoaContext, options?: Record ): Promise> + /** @internal */ sessionQuery( trx?: Knex.Transaction ): QueryBuilder> diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1d75d0cab..09d0cf392 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -581,9 +581,6 @@ importers: koa-mount: specifier: ^4.2.0 version: 4.2.0(supports-color@7.2.0) - koa-passport: - specifier: ^6.0.0 - version: 6.0.0 koa-response-time: specifier: ^2.1.0 version: 2.1.0 @@ -611,9 +608,6 @@ importers: parse-duration: specifier: ^2.1.9 version: 2.1.9 - passport-local: - specifier: ^1.0.0 - version: 1.0.0 passthrough-counter: specifier: ^1.0.0 version: 1.0.0 @@ -4113,10 +4107,6 @@ packages: resolution: {integrity: sha512-2iHQc7vbA9qLeVq5gKAYh3m5DOMMlMfIKjW/REPAS18Mf63daCJHHVXY9nbu7ivrnYn5PiPC4CE523Tf5qvjeQ==} engines: {node: '>= 7.6.0'} - koa-passport@6.0.0: - resolution: {integrity: sha512-bgcrQN7Ylfgi1PVr5l6hHYkr38RHUzx+ty3m7e/xoTte8MR0zbDt6+pvP3/nuF/yXL6Ba7IzX1rSqmCy6OrrIw==} - engines: {node: '>= 4'} - koa-response-time@2.1.0: resolution: {integrity: sha512-W2YvnmqmmdEwjJh2rvUPqDD+yjnKvIQry6JhV9dHEwGpfcRzjmQBRVNg62Czbvut/Dpf+l7b9uFauet157UeUQ==} @@ -4744,18 +4734,6 @@ packages: resolution: {integrity: sha512-CiyeOxFT/JZyN5m0z9PfXw4SCBJ6Sygz1Dpl0wqjlhDEGGBP1GnsUVEL0p63hoG1fcj3fHynXi9NYO4nWOL+qQ==} engines: {node: '>= 0.8'} - passport-local@1.0.0: - resolution: {integrity: sha512-9wCE6qKznvf9mQYYbgJ3sVOHmCWoUNMVFoZzNoznmISbhnNNPhN9xfY3sLmScHMetEJeoY7CXwfhCe7argfQow==} - engines: {node: '>= 0.4.0'} - - passport-strategy@1.0.0: - resolution: {integrity: sha512-CB97UUvDKJde2V0KDWWB3lyf6PC3FaZP7YxZ2G8OAtn9p4HI9j9JLP9qjOGZFvyl8uwNT8qM+hGnz/n16NI7oA==} - engines: {node: '>= 0.4.0'} - - passport@0.6.0: - resolution: {integrity: sha512-0fe+p3ZnrWRW74fe8+SvCyf4a3Pb2/h7gFkQ8yTJpAO50gDzlfjZUZTO1k5Eg9kUct22OxHLqDZoKUWRHOh9ug==} - engines: {node: '>= 0.4.0'} - passthrough-counter@1.0.0: resolution: {integrity: sha512-Wy8PXTLqPAN0oEgBrlnsXPMww3SYJ44tQ8aVrGAI4h4JZYCS0oYqsPqtPR8OhJpv6qFbpbB7XAn0liKV7EXubA==} @@ -4781,9 +4759,6 @@ packages: pathe@2.0.3: resolution: {integrity: sha512-WUjGcAqP1gQacoQe+OBJsFA7Ld4DyXuUIjZ5cc75cLHvJ7dtNsTugphxIADwspS+AraAUePCKrSVtPLFj/F88w==} - pause@0.0.1: - resolution: {integrity: sha512-KG8UEiEVkR3wGEb4m5yZkVCzigAD+cVEJck2CzYZO37ZGJfctvVptVO192MwrtPhzONn6go8ylnOdMhKqi4nfg==} - pend@1.2.0: resolution: {integrity: sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==} @@ -5806,10 +5781,6 @@ packages: util-deprecate@1.0.2: resolution: {integrity: sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==} - utils-merge@1.0.1: - resolution: {integrity: sha512-pMZTvIkT1d+TFGvDOqodOclx0QWkkgi6Tdoa8gC8ffGAAqz9pzPTZWAybbsHHoED/ztMtkv/VoYTYyShUn81hA==} - engines: {node: '>= 0.4.0'} - uuid@14.0.2: resolution: {integrity: sha512-xZe/16rV4aa+HGSOCiY2YeLT1OybRLrrkL/Rqaq7p7GMVXjFh+6wN4oMYgjFmnSnhY8t6Xpdl2l9qmnHYuMHwQ==} hasBin: true @@ -9638,10 +9609,6 @@ snapshots: transitivePeerDependencies: - supports-color - koa-passport@6.0.0: - dependencies: - passport: 0.6.0 - koa-response-time@2.1.0: {} koa-send@5.0.1(supports-color@7.2.0): @@ -10521,18 +10488,6 @@ snapshots: parseurl@1.3.3: {} - passport-local@1.0.0: - dependencies: - passport-strategy: 1.0.0 - - passport-strategy@1.0.0: {} - - passport@0.6.0: - dependencies: - passport-strategy: 1.0.0 - pause: 0.0.1 - utils-merge: 1.0.1 - passthrough-counter@1.0.0: {} path-exists@4.0.0: {} @@ -10550,8 +10505,6 @@ snapshots: pathe@2.0.3: {} - pause@0.0.1: {} - pend@1.2.0: {} perfect-debounce@2.1.0: {} @@ -11684,8 +11637,6 @@ snapshots: util-deprecate@1.0.2: {} - utils-merge@1.0.1: {} - uuid@14.0.2: {} v8-to-istanbul@9.3.0: diff --git a/tests/e2e/account/fixtures.ts b/tests/e2e/account/fixtures.ts index 17a5997ef..1547cd91e 100644 --- a/tests/e2e/account/fixtures.ts +++ b/tests/e2e/account/fixtures.ts @@ -24,8 +24,7 @@ export const test = defineScenario({ normalizePaths: true, helmet: false, keys: ['account-secret'], - session: true, - passport: true + session: true } }, // The computed `password` property isn't stored in the database: diff --git a/tests/server/timestamped-user.test.ts b/tests/server/timestamped-user.test.ts index c2606a82c..aa06b3cd9 100644 --- a/tests/server/timestamped-user.test.ts +++ b/tests/server/timestamped-user.test.ts @@ -1,5 +1,3 @@ -// @ts-expect-error -- koa-passport comes without type declarations. -import passport from 'koa-passport' import type { ModelProperties } from '@ditojs/server' import { AuthenticationError, @@ -12,7 +10,6 @@ import { createTestDatabase, destroyTestApp } from './setup.js' -import { isSQLiteTest } from '../utils/test-knex.js' class Shelf extends TimestampedModel { declare id: number @@ -88,25 +85,11 @@ function createLibrarian(json: Record) { function createLoginContext(body: Record): any { return { - req: {}, request: { body }, - query: {}, - state: {} as Record, - set() {}, login: vi.fn(async () => {}) } } -function deserializeUser(identifier: string) { - return new Promise((resolve, reject) => { - ;(passport as any).deserializeUser( - identifier, - { ctx: {} }, - (err: Error | null, user: any) => (err ? reject(err) : resolve(user)) - ) - }) -} - let alice: Librarian let bob: Librarian @@ -301,55 +284,15 @@ describe('UserMixin', () => { email: 'alice@example.com', password: 'open sesame' }) - const sessionQuery = vi - .spyOn(Librarian, 'sessionQuery') - .mockImplementation(() => { - throw new Error('Database is down') - }) + // A failing database can't be reached otherwise: + const query = vi.spyOn(Librarian, 'query').mockImplementation(() => { + throw new Error('Database is down') + }) try { await expect(Librarian.login(ctx)).rejects.toThrow('Database is down') } finally { - sessionQuery.mockRestore() + query.mockRestore() } }) }) - - describe('session deserialization', () => { - it('loads users through the session scope', async () => { - const user: any = await deserializeUser(`Librarian-${alice.id}`) - expect(user).toBeInstanceOf(Librarian) - expect(user.id).toBe(alice.id) - }) - - // `findById()` resolves to `undefined` for users that are excluded by the - // session scope or were deleted, which passport treats as "not handled", - // failing with an error instead of ending the session. - test('logs out users excluded by the session scope', async () => { - expect(await deserializeUser(`Librarian-${bob.id}`)).toBe(false) - }) - - test('logs out users that no longer exist', async () => { - expect(await deserializeUser('Librarian-999999')).toBe(false) - }) - - it('ignores identifiers of unknown user classes', async () => { - expect(await deserializeUser(`Shelf-${alice.id}`)).toBe(false) - }) - - it('passes on query errors', async context => { - context.skip(isSQLiteTest, 'SQLite compares text with integer ids') - await expect(deserializeUser('Librarian-not-a-number')).rejects.toThrow() - }) - }) - - it('does not serialize users of unregistered classes', async () => { - const promise = new Promise((resolve, reject) => { - ;(passport as any).serializeUser( - Shelf.fromJson({ id: 1, label: 'Poetry' }), - {}, - (err: Error | null, id: any) => (err ? reject(err) : resolve(id)) - ) - }) - await expect(promise).rejects.toThrow('Failed to serialize user') - }) }) diff --git a/tests/server/users-controller.test.ts b/tests/server/users-controller.test.ts index 6ce81c117..e8da01e74 100644 --- a/tests/server/users-controller.test.ts +++ b/tests/server/users-controller.test.ts @@ -2,6 +2,7 @@ import type { ModelProperties } from '@ditojs/server' import { SessionModel, UserModel, UsersController } from '@ditojs/server' import { createTestApp, getAppUrl } from '../utils/app.js' import { createTestDatabase } from '../utils/database.js' +import { isSQLiteTest } from '../utils/test-knex.js' const loginEvents: string[] = [] const patchTransactions: boolean[] = [] @@ -13,12 +14,20 @@ class Member extends UserModel { declare username: string declare nickname: string | null + static override options = { + sessionScope: 'current' + } + static override properties: ModelProperties = { nickname: { type: 'string' } } + static override scopes = { + current: (query: any) => query.whereNot('username', 'retired') + } + static override hooks: any = { 'before:login'(member: Member) { if (member.username === 'locked') { @@ -38,6 +47,9 @@ class Member extends UserModel { } } +// Dito gives subclasses their own tables, so their ids overlap with members: +class Lead extends Member {} + class Members extends UsersController { override modelClass = Member @@ -48,7 +60,8 @@ class Members extends UsersController { 'post login', 'post logout', 'get session', - 'get self' + 'get self', + 'post broken-login' ], 'authorize': { get: true @@ -62,6 +75,15 @@ class Members extends UsersController { const response = await super['post login'](ctx) return { ...response, transacted: !!ctx.transaction } } + }, + + // Fail after logging in, to roll back the session with the transaction: + 'post broken-login': { + transacted: true, + async handler(ctx: any) { + await Member.login(ctx) + throw new Error('Login is broken') + } } } @@ -94,6 +116,10 @@ class Editors extends UsersController { } } +class Leads extends UsersController { + override modelClass = Lead +} + // Users that only admins may access, while anyone can log in and out: class Admins extends UsersController { override modelClass = Member @@ -109,12 +135,11 @@ describe('UsersController', () => { keys: ['test-secret'], session: { modelClass: 'Session' - }, - passport: true + } } }, - models: { Session, Member }, - controllers: { Members, Admins, Readers, Editors } + models: { Session, Member, Lead }, + controllers: { Members, Leads, Admins, Readers, Editors } }) let url: string @@ -155,16 +180,18 @@ describe('UsersController', () => { } beforeAll(async () => { - await createTestDatabase(app, Member) + await createTestDatabase(app, Member, Lead) // Session ids are strings, not auto-incrementing integers. await app.knex.schema.createTable('Session', table => { table.string('id').primary() table.jsonb('value') }) // The computed `password` property isn't stored in the database. - await app.knex.schema.alterTable('Member', table => { - table.dropColumn('password') - }) + for (const name of ['Member', 'Lead']) { + await app.knex.schema.alterTable(name, table => { + table.dropColumn('password') + }) + } await app.start() url = getAppUrl(app) member = await Member.query().insert({ @@ -312,9 +339,120 @@ describe('UsersController', () => { const logout = await request('/members/logout', { method: 'POST' }) expect(logout.data).toEqual({ success: true, authenticated: false }) expect(loginEvents).toEqual(['before logout ada', 'after logout ada']) + expect(await Session.query()).toEqual([]) expect((await request('/members/session')).data.authenticated).toBe(false) }) + it('regenerates the session id on login', async () => { + const request = createClient() + const credentials = { username: 'ada', password: 'secret' } + await request('/members/login', { method: 'POST', body: credentials }) + const [before] = await Session.query() + await request('/members/login', { method: 'POST', body: credentials }) + const sessions = await Session.query() + expect(sessions).toHaveLength(1) + expect(sessions[0].id).not.toBe(before.id) + }) + + it('does not read credentials from the query string', async () => { + const request = createClient() + const login = await request('/members/login?username=ada&password=secret', { + method: 'POST', + body: {} + }) + expect(login.status).toBe(400) + expect(login.data.error).toBe('Missing credentials') + }) + + it('leaves no session behind when a transacted login fails', async () => { + const request = createClient() + const login = await request('/members/broken-login', { + method: 'POST', + body: { username: 'ada', password: 'secret' } + }) + expect(login.status).toBe(500) + expect(await Session.query()).toEqual([]) + expect((await request('/members/session')).data.authenticated).toBe(false) + }) + + describe('session users', () => { + let other: Member + + beforeEach(async () => { + other = await Member.query().insert({ + username: 'bo', + password: 'secret' + } as any) + }) + + afterEach(async () => { + await Member.query().deleteById(other.id) + }) + + const loginOther = async () => { + const request = createClient() + await request('/members/login', { + method: 'POST', + body: { username: 'bo', password: 'secret' } + }) + expect((await request('/members/session')).data.authenticated).toBe(true) + return request + } + + const expectDroppedUser = async (request: any) => { + expect((await request('/members/session')).data.authenticated).toBe(false) + const [session] = await Session.query() + expect(session.value.passport).toEqual({}) + } + + it('drops users excluded by the session scope', async () => { + const request = await loginOther() + await Member.query().patchById(other.id, { username: 'retired' }) + await expectDroppedUser(request) + }) + + it('drops users that no longer exist', async () => { + const request = await loginOther() + await Member.query().deleteById(other.id) + await expectDroppedUser(request) + }) + + it('passes on errors of the session query', async context => { + context.skip(isSQLiteTest, 'SQLite compares text with integer ids') + const request = await loginOther() + await Session.query().patch({ + value: { passport: { user: 'Member-not-a-number' } } + } as any) + expect((await request('/members/session')).status).toBe(400) + }) + }) + + it('only authenticates users of its own model class', async () => { + const lead = await Lead.query().insert({ + id: member.id, + username: 'lea', + password: 'secret' + } as any) + try { + const request = createClient() + await request('/leads/login', { + method: 'POST', + body: { username: 'lea', password: 'secret' } + }) + expect((await request('/leads/session')).data).toMatchObject({ + authenticated: true, + user: { id: lead.id, username: 'lea' } + }) + expect((await request('/members/session')).data).toEqual({ + authenticated: false, + user: null + }) + expect((await request('/members/self')).status).toBe(204) + } finally { + await Lead.query().deleteById(lead.id) + } + }) + it('does not log out unauthenticated requests', async () => { const request = createClient() const logout = await request('/members/logout', { method: 'POST' })