diff --git a/server/src/db/dialect/sql-functions.ts b/server/src/db/dialect/sql-functions.ts index 7a71cee90..4ee2cf65b 100644 --- a/server/src/db/dialect/sql-functions.ts +++ b/server/src/db/dialect/sql-functions.ts @@ -1,5 +1,6 @@ import { raw, type Platform, type RawQueryFragment } from '@mikro-orm/core'; import { SqlitePlatform } from '@mikro-orm/sql'; +import { sql, type Expression } from 'kysely'; /** * The only place a repository may spell a database function. @@ -99,3 +100,26 @@ export function lower(platform: Platform, ref: string): RawQueryFragment & symbo if (platform instanceof SqlitePlatform) return raw(`LOWER(${column(ref)})`); return unsupported(platform); } + +/** + * `CURRENT_TIMESTAMP`, for a query built through `em.getKysely()` — the + * escape hatch for a recursive CTE (`OauthTokensRepository.collectChainIds`) + * or a join the QueryBuilder's relation metadata cannot express correctly + * (`OauthTokensRepository.listActiveByUser`/`listAllActiveWithClientAndUser` + * — `oauth_tokens.client_id` references `oauth_clients.client_id`, not its + * primary key, and the generated entity's `client` relation has no + * `referencedColumnNames` override, so a QueryBuilder `.innerJoin('ot.client', + * …)` silently joins on the wrong column; see those methods' docstrings). + * Kysely has its own expression system, entirely separate from MikroORM's + * `raw()` — `currentTimestamp` above returns a value this file's OTHER + * consumers (the QueryBuilder, `nativeUpdate`) accept, but a Kysely query + * cannot use a `RawQueryFragment` as an operand. This is the one place a + * repository may reach for the Kysely spelling of the same literal, kept + * here rather than in a repository file so `no-restricted-syntax`'s ban on + * spelling raw SQL directly in `src/db/repositories/**` still holds — this + * file is exempt from that rule by design (it IS the one approved place). + */ +export function currentTimestampKysely(platform: Platform): Expression { + if (platform instanceof SqlitePlatform) return sql`CURRENT_TIMESTAMP`; + return unsupported(platform); +} diff --git a/server/src/db/repositories/OauthClients.repository.ts b/server/src/db/repositories/OauthClients.repository.ts index 8dee1e0b1..9772556e6 100644 --- a/server/src/db/repositories/OauthClients.repository.ts +++ b/server/src/db/repositories/OauthClients.repository.ts @@ -1,4 +1,322 @@ -import type { OauthClients } from '../entities/OauthClients.entity'; +import { OauthClients } from '../entities/OauthClients.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { EntityRepository } from '@mikro-orm/sql'; -export class OauthClientsRepository extends EntityRepository {} +/** + * A full `oauth_clients` row (`SELECT * FROM oauth_clients WHERE ...`), as + * the API emits it. `user_id` is the `persist(false)` mirror of the `user` + * relation (same pattern as `InviteTokens.created_by`/`createdByRef`) — a + * scalar column for `AssertRowKeys` purposes even though it is never written + * directly. + */ +export interface OauthClientRow { + id: string; + user_id: number | null; + name: string; + client_id: string; + client_secret_hash: string; + redirect_uris: string; // JSON array + allowed_scopes: string; // JSON array + created_at: string | null; + is_public: number; // 0 | 1 + created_via: string; + allows_client_credentials: number; // 0 | 1 +} + +const _oauthClientRowKeys: AssertRowKeys = true; + +/** + * OA1 (`listOAuthClients`) / OA5 (`createOAuthClient`'s re-select) — every + * column except `client_secret_hash`, which the settings panel and the + * create-response never carry. + */ +export interface OauthClientPublicRow { + id: string; + user_id: number | null; + name: string; + client_id: string; + redirect_uris: string; + allowed_scopes: string; + created_at: string | null; + is_public: number; + created_via: string; + allows_client_credentials: number; +} + +/** `OauthService.createOAuthClient`'s write (OA4) — every column the legacy INSERT names explicitly. */ +export interface NewOauthClientRow { + id: string; + user_id: number | null; + name: string; + client_id: string; + client_secret_hash: string; + redirect_uris: string; + allowed_scopes: string; + is_public: number; + created_via: string; + allows_client_credentials: number; +} + +/** + * OA6 (`rotateOAuthClientSecret`'s ownership+404 check, 3 columns) / OA9 + * (`deleteOAuthClient`'s ownership+404 check, 2 columns) — one method, the + * superset projection (same economy as `McpTokensRepository.findBasic` + * merging TK4/TK9): OA9's caller reads only `id`/`client_id` and ignores + * `is_public`. + */ +export interface OauthClientOwnedRow { + id: string; + client_id: string; + is_public: number; +} + +/** + * OA15 (`getSdkClient`) — the exact row the MCP SDK's `clientsStore` adapter + * (`oauth-sdk.provider.ts`'s `rowToInfo`) maps to `OAuthClientInformationFull`. + * Never carries `client_secret_hash` — a separate method from + * `findByClientIdFull`, not a service-side projection of it (Task 4 brief + * ruling: decide from how the result is used and record the decision) — + * the SDK adapter's scope should never see the hash column pass through it + * even transiently. + */ +export interface OauthClientSdkRow { + client_id: string; + name: string; + redirect_uris: string; + allowed_scopes: string; + is_public: number; + created_via: string; +} + +/** OA21 (`refreshTokens`'s client-auth read) — client auth needs only these three columns. */ +export interface OauthClientAuthRow { + client_id: string; + client_secret_hash: string; + is_public: number; +} + +/** + * OAuth 2.1 client registrations (self-service + DCR + machine clients), + * Plan 3b Task 4, inventory §3 OA1-OA10/OA15/OA21/OA29/OA30. Every row-out + * read passes `disableIdentityMap: true` (program RULING, Task 1 review B1 + * — see `Users.repository.ts`'s class-level docstring): a partial-field + * `find`/`findOne` merged into the request's identity map can go stale and + * get flushed back over an unrelated `nativeUpdate` in the same transaction; + * a `disableIdentityMap: true` read is always isolated and discarded. + * `client_secret_hash` is never hashed or compared here — that stays in + * `OauthService` (`hashToken`/`timingSafeEqualHex`). + */ +export class OauthClientsRepository extends EntityRepository { + // --------------------------------------------------------------------- + // OA1 — self-service client list + // --------------------------------------------------------------------- + + /** + * `SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, + * created_at, is_public, created_via, allows_client_credentials FROM + * oauth_clients WHERE user_id = ? ORDER BY created_at DESC` + * + * `fields` names the RELATION property `'user'`, not the `persist(false)` + * mirror `'user_id'` — verified empirically (not from the `.d.ts`, per + * `McpTokensRepository.findBasic`'s docstring): naming the mirror directly + * leaves `row.user_id` `undefined` (it has no column of its own to + * select); the relation's `Ref` loads instead, and its `.id` reads + * synchronously without a second query. + */ + async listByUser(userId: number): Promise { + const rows = await this.find( + { user: userId }, + { + fields: ['id', 'user', 'name', 'client_id', 'redirect_uris', 'allowed_scopes', 'created_at', 'is_public', 'created_via', 'allows_client_credentials'], + orderBy: { created_at: 'desc' }, + disableIdentityMap: true, + }, + ); + return rows.map((row) => ({ + id: row.id as string, + user_id: row.user?.id ?? null, + name: row.name, + client_id: row.client_id, + redirect_uris: row.redirect_uris, + allowed_scopes: row.allowed_scopes, + created_at: row.created_at ?? null, + is_public: row.is_public, + created_via: row.created_via, + allows_client_credentials: row.allows_client_credentials, + })); + } + + // --------------------------------------------------------------------- + // OA2 / OA3 — registration caps + // --------------------------------------------------------------------- + + /** `SELECT COUNT(*) as count FROM oauth_clients WHERE user_id = ?` (the 10-per-user cap). */ + async countByUser(userId: number): Promise { + return this.count({ user: userId }); + } + + /** `SELECT COUNT(*) as count FROM oauth_clients WHERE user_id IS NULL` (the 500 anonymous-DCR cap). */ + async countAnonymous(): Promise { + return this.count({ user: null }); + } + + // --------------------------------------------------------------------- + // OA4 / OA5 — client registration + // --------------------------------------------------------------------- + + /** + * `INSERT INTO oauth_clients (id, user_id, name, client_id, + * client_secret_hash, redirect_uris, allowed_scopes, is_public, + * created_via, allows_client_credentials) VALUES (...)` (OA4, + * **security-sensitive**: client registration), followed by `findPublicById` + * (OA5's re-select — same column list `listByUser` selects, `WHERE id = ?`). + * + * `em.insert` (program RULING), never `create()` + `persist().flush()`: + * `flush()` commits the request EntityManager's WHOLE unit of work, not + * just this row (`Users.repository.ts::insertUser`'s docstring). `id` is + * application-generated (`randomUUID()`, the service's job, never this + * repository's) — the legacy INSERT names it explicitly, so `em.insert` + * does too, never relying on a DB-generated default. + */ + async insertClient(row: NewOauthClientRow): Promise { + await this.getEntityManager().insert(OauthClients, { + id: row.id, + user: row.user_id, + name: row.name, + client_id: row.client_id, + client_secret_hash: row.client_secret_hash, + redirect_uris: row.redirect_uris, + allowed_scopes: row.allowed_scopes, + is_public: row.is_public, + created_via: row.created_via, + allows_client_credentials: row.allows_client_credentials, + }); + const inserted = await this.findPublicById(row.id); + if (!inserted) { + throw new Error(`OauthClientsRepository.insertClient: row ${row.id} not found immediately after insert`); + } + return inserted; + } + + /** OA1's exact column set, `WHERE id = ?` — `insertClient`'s re-select (OA5). */ + async findPublicById(id: string): Promise { + const row = await this.findOne( + { id }, + { + fields: ['id', 'user', 'name', 'client_id', 'redirect_uris', 'allowed_scopes', 'created_at', 'is_public', 'created_via', 'allows_client_credentials'], + disableIdentityMap: true, + }, + ); + return row + ? { + id: row.id as string, + user_id: row.user?.id ?? null, + name: row.name, + client_id: row.client_id, + redirect_uris: row.redirect_uris, + allowed_scopes: row.allowed_scopes, + created_at: row.created_at ?? null, + is_public: row.is_public, + created_via: row.created_via, + allows_client_credentials: row.allows_client_credentials, + } + : null; + } + + // --------------------------------------------------------------------- + // OA6 / OA9 — ownership + 404 checks (secret rotation / delete) + // --------------------------------------------------------------------- + + /** + * `SELECT id, client_id, is_public FROM oauth_clients WHERE id = ? AND + * user_id = ?` (OA6, `rotateOAuthClientSecret`) and `SELECT id, client_id + * FROM oauth_clients WHERE id = ? AND user_id = ?` (OA9, + * `deleteOAuthClient`) — ONE method, the superset projection (OA9's two + * columns are a strict subset of OA6's three, same economy as + * `McpTokensRepository.findBasic`'s TK4/TK9 merge). 404-never-403: scoping + * by `user` in the WHERE, not a separate ownership check after an + * unscoped read. + */ + async findOwned(id: string, userId: number): Promise { + const row = await this.findOne( + { id, user: userId }, + { fields: ['id', 'client_id', 'is_public'], disableIdentityMap: true }, + ); + return row ? { id: row.id as string, client_id: row.client_id, is_public: row.is_public } : null; + } + + // --------------------------------------------------------------------- + // OA7 — secret rotation write + // --------------------------------------------------------------------- + + /** `UPDATE oauth_clients SET client_secret_hash = ? WHERE id = ?` — **security-sensitive**. */ + async updateSecretHash(id: string, hash: string): Promise { + await this.nativeUpdate({ id }, { client_secret_hash: hash }); + } + + // --------------------------------------------------------------------- + // OA10 — delete + // --------------------------------------------------------------------- + + /** `DELETE FROM oauth_clients WHERE id = ?` */ + async remove(id: string): Promise { + await this.nativeDelete({ id }); + } + + // --------------------------------------------------------------------- + // OA15 — MCP SDK clientsStore adapter read + // --------------------------------------------------------------------- + + /** + * `SELECT client_id, name, redirect_uris, allowed_scopes, is_public, + * created_via FROM oauth_clients WHERE client_id = ?` — see + * `OauthClientSdkRow`'s docstring for why this stays its own method + * rather than a service-side projection of `findByClientIdFull`. + */ + async findSdkProjection(clientId: string): Promise { + const row = await this.findOne( + { client_id: clientId }, + { fields: ['client_id', 'name', 'redirect_uris', 'allowed_scopes', 'is_public', 'created_via'], disableIdentityMap: true }, + ); + return row + ? { client_id: row.client_id, name: row.name, redirect_uris: row.redirect_uris, allowed_scopes: row.allowed_scopes, is_public: row.is_public, created_via: row.created_via } + : null; + } + + // --------------------------------------------------------------------- + // OA21 — refresh-token-grant client authentication + // --------------------------------------------------------------------- + + /** + * `SELECT client_id, client_secret_hash, is_public FROM oauth_clients + * WHERE client_id = ?` (`refreshTokens`'s client-auth read — + * **security-sensitive**). A distinct projection from OA15's — not merged + * with it (D4: mirror each statement's exact column set; a prior draft of + * this repository conflated the two, corrected per the Task 4 controller + * addendum). + */ + async findAuthRow(clientId: string): Promise { + const row = await this.findOne( + { client_id: clientId }, + { fields: ['client_id', 'client_secret_hash', 'is_public'], disableIdentityMap: true }, + ); + return row ? { client_id: row.client_id, client_secret_hash: row.client_secret_hash, is_public: row.is_public } : null; + } + + // --------------------------------------------------------------------- + // OA29 / OA30 — authorize-flow + token-endpoint client lookups + // --------------------------------------------------------------------- + + /** + * `SELECT * FROM oauth_clients WHERE client_id = ?` — OA29 + * (`validateAuthorizeRequest`) and OA30 (`authenticateClient`, + * **security-sensitive**: token-endpoint client auth), identical + * statement text, two call sites — ONE method, the full row (D4: both + * legacy statements are genuinely `SELECT *`, unlike OA15/OA21's narrow + * projections). + */ + async findByClientIdFull(clientId: string): Promise { + const row = await this.findOne({ client_id: clientId }, { disableIdentityMap: true }); + return row ? (toRow(row) as OauthClientRow) : null; + } +} diff --git a/server/src/db/repositories/OauthConsents.repository.ts b/server/src/db/repositories/OauthConsents.repository.ts index 1a5b4a8a7..b634c3919 100644 --- a/server/src/db/repositories/OauthConsents.repository.ts +++ b/server/src/db/repositories/OauthConsents.repository.ts @@ -1,4 +1,84 @@ -import type { OauthConsents } from '../entities/OauthConsents.entity'; +import { OauthConsents } from '../entities/OauthConsents.entity'; +import { currentTimestamp } from '../dialect/sql-functions'; import { EntityRepository } from '@mikro-orm/sql'; -export class OauthConsentsRepository extends EntityRepository {} +/** + * Per-(client, user) OAuth consent grants, Plan 3b Task 4, inventory §3 + * OA11/OA12. `oauth_consents` has a genuine composite unique index on + * `(client_id, user_id)` (`Migration20200101012500_oauth_2.ts`: + * `UNIQUE(client_id, user_id)`, mirrored on the entity as `uniques: [{ + * properties: ['client', 'user'] }]`), so OA12's upsert decision (task + * brief ruling) is `em.upsert`, not a `nativeDelete` + `insert` pair — the + * natural key IS unique, no fallback needed. Scope union-not-narrow (M5: + * never narrow stored consent) stays in the SERVICE — this repository + * writes exactly the scopes JSON it is given. + */ +export class OauthConsentsRepository extends EntityRepository { + // --------------------------------------------------------------------- + // OA11 — consent-sufficiency read + // --------------------------------------------------------------------- + + /** + * `SELECT scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?` + * `null` when no consent has been granted yet — the JSON parse of a + * present-but-empty `scopes` value ("consent granted, zero scopes", + * structurally distinct from "never consented") stays in the service, + * unchanged. + * + * Named `findScopes`, not `find` — `EntityRepository#find` already exists + * with an incompatible signature (`(where, options) => Promise`); + * shadowing it would either fail to compile or silently hide the base + * method (same rule `upsertGrant` below follows for `#upsert`). + */ + async findScopes(clientId: string, userId: number): Promise<{ scopes: string } | null> { + const row = await this.findOne( + { client: clientId, user: userId }, + { fields: ['scopes'], disableIdentityMap: true }, + ); + return row ? { scopes: row.scopes } : null; + } + + // --------------------------------------------------------------------- + // OA12 — consent grant (upsert) + // --------------------------------------------------------------------- + + /** + * `INSERT OR REPLACE INTO oauth_consents (client_id, user_id, scopes, + * updated_at) VALUES (?, ?, ?, CURRENT_TIMESTAMP)` — + * **security-sensitive** (authorization grant). `em.upsert` with + * `onConflictFields: ['client', 'user']` (the RELATION properties, not + * their `persist(false)` mirrors — `Settings.repository.ts::upsertForUser`'s + * precedent) against the entity's own `uniques: [{ properties: ['client', + * 'user'] }]` — the natural key the migration's `UNIQUE(client_id, + * user_id)` declares, verified against the migration before choosing this + * over a delete+insert fallback (task brief ruling). + * + * `updated_at` is explicit in the payload, not left to the column's + * `defaultRaw('CURRENT_TIMESTAMP')`: a default only fires on INSERT, and + * `em.upsert`'s `'merge'` action updates ONLY the fields present in the + * payload on a conflict (verified against the installed 7.2.1 behaviour + * and the MikroORM docs: "values generated by property `onCreate` hooks… + * are automatically excluded from the merge clause") — the legacy + * statement's `INSERT OR REPLACE` always re-writes `updated_at` to + * `CURRENT_TIMESTAMP` regardless of whether the row existed (SQLite's + * `OR REPLACE` deletes-then-inserts, so there is no "update" branch to + * distinguish), so this repository stamps it explicitly on every call + * through `currentTimestamp(platform)`, matching that unconditional + * behaviour rather than relying on a default that would only cover the + * insert half. + * + * Named `upsertGrant`, not `upsert` — `EntityRepository#upsert` already + * exists with an incompatible signature (`(entityOrData, options) => + * Promise`); shadowing it would either fail to compile or silently + * hide the base method, the same collision `Users.repository.ts::insertUser` + * avoided with `create`. + */ + async upsertGrant(clientId: string, userId: number, scopes: string): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.getEntityManager().upsert( + OauthConsents, + { client: clientId, user: userId, scopes, updated_at: currentTimestamp(platform) }, + { onConflictFields: ['client', 'user'], onConflictAction: 'merge' }, + ); + } +} diff --git a/server/src/db/repositories/OauthTokens.repository.ts b/server/src/db/repositories/OauthTokens.repository.ts index a86354057..70cffed8a 100644 --- a/server/src/db/repositories/OauthTokens.repository.ts +++ b/server/src/db/repositories/OauthTokens.repository.ts @@ -1,4 +1,606 @@ -import type { OauthTokens } from '../entities/OauthTokens.entity'; +import { OauthTokens } from '../entities/OauthTokens.entity'; +import { type AssertRowKeys } from './_shared/rows'; +import { columnRef, currentTimestamp, currentTimestampKysely } from '../dialect/sql-functions'; import { EntityRepository } from '@mikro-orm/sql'; -export class OauthTokensRepository extends EntityRepository {} +/** + * The Kysely-side table shapes `em.getKysely()` needs for the two escape + * hatches in this repository (the recursive CTE and the `oauth_clients` + * join — see `collectChainIds`/`listActiveByUser`'s docstrings for why the + * QueryBuilder can't express either). Column-shaped, not entity-shaped: this + * is the raw driver's view of the tables, independent of MikroORM metadata. + */ +interface OauthTokensKyselyDB { + oauth_tokens: { + id: number; + client_id: string; + user_id: number; + scopes: string; + access_token_expires_at: string; + refresh_token_expires_at: string; + revoked_at: string | null; + created_at: string | null; + parent_token_id: number | null; + }; + oauth_clients: { + client_id: string; + name: string; + }; + users: { + id: number; + username: string; + }; +} + +/** `OauthService.issueTokens`/`issueClientCredentialsToken`'s write (OA13/OA14) — same column set, one method. */ +export interface NewOauthTokenRow { + client_id: string; + user_id: number; + access_token_hash: string; + refresh_token_hash: string; + scopes: string; + audience: string | null; + access_token_expires_at: string; + refresh_token_expires_at: string; + parent_token_id: number | null; +} + +/** OA16 (`getUserByAccessToken`) — every MCP request's bearer check, **security-sensitive**. */ +export interface OauthTokenWithUserRow { + scopes: string; + audience: string | null; + revoked_at: string | null; + access_token_expires_at: string; + user_id: number; + client_id: string; + username: string; + email: string; + role: string; +} + +/** OA17 (`findChainRoot`'s upward-walk read). */ +export interface OauthTokenParentRow { + id: number; + parent_token_id: number | null; +} + +/** OA22 (`refreshTokens`'s refresh-token-hash read). */ +export interface OauthTokenRefreshRow { + id: number; + client_id: string; + user_id: number; + scopes: string; + audience: string | null; + refresh_token_expires_at: string; + revoked_at: string | null; + parent_token_id: number | null; +} + +/** OA24 (`revokeToken`'s user-id-for-session-revoke lookup). */ +export interface OauthTokenUserIdRow { + user_id: number; +} + +/** OA27 (`revokeSession`'s ownership+404 check). */ +export interface OauthTokenOwnedRow { + id: number; + client_id: string; +} + +/** OA32 (`adminRevokeOAuthSession`'s 404 check — unscoped, needs `user_id` for the session-revoke call). */ +export interface OauthTokenAdminRow { + id: number; + user_id: number; + client_id: string; +} + +/** OA26 (`listOAuthSessions`, the user's own "active sessions" panel). */ +export interface OauthSessionRow { + id: number; + client_id: string; + client_name: string; + scopes: string; + access_token_expires_at: string; + refresh_token_expires_at: string; + created_at: string | null; +} + +/** OA31 (`listAllOAuthSessions`, the admin panel) — OA26's row plus the token owner. */ +export interface OauthSessionWithUserRow extends OauthSessionRow { + user_id: number; + username: string; +} + +/** + * No site in this domain does `SELECT * FROM oauth_tokens` (unlike + * `oauth_clients`'s OA29/OA30) — every method below mirrors a narrower + * legacy projection. `OauthTokenScalarColumns` is not a return type of any + * method here; it exists only so `AssertRowKeys` still pins that every + * scalar column of the entity is accounted for SOMEWHERE across this file's + * narrower row interfaces, the same compile-time guarantee the other two + * repositories in this domain get from a genuine full-row type. + */ +interface OauthTokenScalarColumns { + id: number; + client_id: string; + user_id: number; + access_token_hash: string; + refresh_token_hash: string; + scopes: string; + access_token_expires_at: string; + refresh_token_expires_at: string; + revoked_at: string | null; + created_at: string | null; + parent_token_id: number | null; + audience: string | null; +} + +const _oauthTokenScalarProbe: AssertRowKeys = true; + +/** + * OAuth 2.1 bearer/refresh tokens, Plan 3b Task 4, inventory §3 + * OA8/OA13/OA14/OA16-OA20/OA22-OA28/OA31-OA33 — the busiest table in this + * domain. Every row-out read passes `disableIdentityMap: true` (program + * RULING, Task 1 review B1 — see `Users.repository.ts`'s class-level + * docstring). No method here hashes, compares or generates a token — + * `OauthService` does (`hashToken`/`generateAccessToken`/…), this + * repository only ever sees an already-computed hash string. + * + * **The `client` relation targets the wrong column.** The generator built + * `client: p.manyToOne(OauthClients).ref().name('client_id')` with no + * `referencedColumnNames` override, so MikroORM's QueryBuilder joins it as + * `ot.client_id = oc.id` (`OauthClients`'s primary key) — verified + * empirically (`qb.getFormattedQuery()`). The actual schema + * (`Migration20200101012500_oauth_2.ts`) has `oauth_tokens.client_id + * REFERENCES oauth_clients(client_id)`, the UNIQUE natural key, not `oc.id` + * — the two are independently generated UUIDs + * (`OauthService.createOAuthClient`'s `id`/`clientId` locals). A plain + * equality filter/write on the `client` property (`{ client: clientId }` in + * a `find`/`nativeUpdate`) is unaffected — it binds the given scalar + * straight into the `client_id` column, no join involved, verified + * empirically — but an ORM-level JOIN through this relation + * (`.innerJoin('ot.client', 'oc')`) silently returns the WRONG rows. This + * repository never joins that relation: `listActiveByUser`/ + * `listAllActiveWithClientAndUser` (OA26/OA31, the two legacy statements + * that actually join `oauth_clients`) go through `em.getKysely()` instead, + * joining on the real `oc.client_id` column directly, entity metadata + * bypassed entirely. This is a pre-existing entity-generation defect + * (outside this task's scope to fix — `src/db/entities/**` is off limits + * here); flagged for Task 7 / the entity generator's own follow-up. + */ +export class OauthTokensRepository extends EntityRepository { + // --------------------------------------------------------------------- + // OA13 / OA14 — token issuance + // --------------------------------------------------------------------- + + /** + * `INSERT INTO oauth_tokens (client_id, user_id, access_token_hash, + * refresh_token_hash, scopes, audience, access_token_expires_at, + * refresh_token_expires_at, parent_token_id) VALUES (...)` — OA13 + * (`issueTokens`) and OA14 (`issueClientCredentialsToken`, whose + * `refresh_token_hash` is an unusable random placeholder and whose + * `refresh_token_expires_at` is already-expired — both decided by the + * caller, not this repository), same column list — ONE method. + * **Security-sensitive** (token mint). Neither legacy call site + * re-selects after the INSERT (the response is built from the already- + * computed raw values), so this returns nothing, matching exactly. + * + * Named `insertToken`, not `create` — `EntityRepository#create` already + * exists with an incompatible signature (a SYNCHRONOUS, non-persisting + * factory); shadowing it would fail to compile, the same collision + * `Users.repository.ts::insertUser` and `McpTokens.repository.ts + * ::insertToken` avoid. + */ + async insertToken(row: NewOauthTokenRow): Promise { + await this.getEntityManager().insert(OauthTokens, { + client: row.client_id, + user: row.user_id, + access_token_hash: row.access_token_hash, + refresh_token_hash: row.refresh_token_hash, + scopes: row.scopes, + audience: row.audience, + access_token_expires_at: row.access_token_expires_at, + refresh_token_expires_at: row.refresh_token_expires_at, + parentToken: row.parent_token_id, + }); + } + + // --------------------------------------------------------------------- + // AU18 / AU46 (AuthService, cross-domain — Task 4 brief ruling: not one of + // the 33 numbered OA sites, but built NOW so it exists for Task 5) + // --------------------------------------------------------------------- + + /** + * `UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE user_id = + * ? AND revoked_at IS NULL` — `AuthService.changePassword` (AU18) and + * `resetPassword` (AU46), both inside the same transaction as the + * password write itself (**security-sensitive**: every OAuth session the + * user held stops working the moment their password changes). Built here + * because this table belongs to `nest/oauth`, per the inventory's §6 + * cross-domain ruling (mirrors `McpTokensRepository.deleteAllForUser`'s + * precedent for the same AuthService sites on `mcp_tokens`) — Task 5 is + * the only caller. The legacy call sites wrap this statement in a + * `try/catch` ("oauth_tokens table may not exist in very old installs") + * — that tolerance is `AuthService`'s own concern, not reproduced here; + * this repository always assumes the table exists, the same as every + * other method in this file. + */ + async revokeAllForUser(userId: number): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate({ user: userId, revoked_at: null }, { revoked_at: currentTimestamp(platform) }); + } + + // --------------------------------------------------------------------- + // OA8 — mass revoke on secret rotation + // --------------------------------------------------------------------- + + /** + * `UPDATE oauth_tokens SET revoked_at = datetime('now') WHERE client_id = + * ? AND revoked_at IS NULL` — **security-sensitive** (mass revoke on + * secret rotation). The legacy statement spells `datetime('now')` rather + * than `CURRENT_TIMESTAMP` (every other revoke in this file uses the + * latter) — both route through `currentTimestamp(platform)` (Task 0's + * `SQLF-008` proves the two spellings are byte-identical on SQLite), one + * dialect path, not two. + */ + async revokeAllForClient(clientId: string): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate({ client: clientId, revoked_at: null }, { revoked_at: currentTimestamp(platform) }); + } + + // --------------------------------------------------------------------- + // OA16 — MCP bearer-token verification + // --------------------------------------------------------------------- + + /** + * `SELECT ot.scopes, ot.audience, ot.revoked_at, ot.access_token_expires_at, + * ot.user_id, ot.client_id, u.username, u.email, u.role FROM oauth_tokens + * ot JOIN users u ON ot.user_id = u.id WHERE ot.access_token_hash = ?` — + * **security-sensitive**, every MCP request's bearer check. `ot.user` + * (unlike `ot.client`) is a NORMAL relation — its column + * (`user_id`, default naming, no override) genuinely references + * `users.id`, verified empirically — so `.innerJoin('ot.user', 'u')` joins + * correctly and this stays on the QueryBuilder (D4's T5 tier), unlike + * OA26/OA31 below. `columnRef` for both FK scalars in the select list: + * `ot.user_id` because the same relation is also joined (the + * `McpTokensRepository.listAllWithUsername` finding — selecting a + * relation property while also joining it collapses into the join rather + * than emitting the FK column); `ot.client_id` for symmetry and because + * it is itself a `persist(false)` mirror with no column of its own to + * `.select()` by property name. + */ + async findByAccessTokenHashWithUser(hash: string): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('ot') + .innerJoin('ot.user', 'u') + .select([ + 'ot.scopes', 'ot.audience', 'ot.revoked_at', 'ot.access_token_expires_at', + columnRef(platform, 'ot.user_id'), columnRef(platform, 'ot.client_id'), + 'u.username', 'u.email', 'u.role', + ]) + .where({ 'ot.access_token_hash': hash }) + .execute('get', false); + return row ?? null; + } + + // --------------------------------------------------------------------- + // OA17 — rotation-chain upward walk (findChainRoot, looped in the service) + // --------------------------------------------------------------------- + + /** + * `SELECT id, parent_token_id FROM oauth_tokens WHERE id = ?` — + * **security-sensitive** (replay-detection chain walk). `findChainRoot`'s + * upward loop (≤ 100 iterations) stays in `OauthService`, calling this + * once per hop — NOT a second recursive CTE (Task 4 brief ruling): the + * loop already terminates on `parent_token_id === null`, and a CTE would + * need to walk UP the parent chain from an arbitrary starting id, which + * requires knowing the root in advance (the opposite direction from + * `collectChainIds`'s downward walk from a known root). + * + * `fields` names the RELATION property `'parentToken'`, not the + * `persist(false)` mirror `'parent_token_id'` (same rule as + * `McpTokensRepository.findBasic`'s `'user'`/`user_id'` finding): naming + * the mirror directly leaves it `undefined`. + */ + async findParent(id: number): Promise { + const row = await this.findOne({ id }, { fields: ['id', 'parentToken'], disableIdentityMap: true }); + return row ? { id: row.id, parent_token_id: row.parentToken?.id ?? null } : null; + } + + // --------------------------------------------------------------------- + // OA18 — recursive CTE (revokeChain) + // --------------------------------------------------------------------- + + /** + * ```sql + * WITH RECURSIVE chain(id) AS ( + * SELECT id FROM oauth_tokens WHERE id = ? + * UNION ALL + * SELECT t.id FROM oauth_tokens t JOIN chain c ON t.parent_token_id = c.id + * ) + * SELECT id FROM chain + * ``` + * + * **Security-sensitive** — finds every token in a rotation chain to mass- + * revoke. The MikroORM QueryBuilder has no recursive-CTE support (D4's T6 + * tier, the program's canonical `em.getKysely()` case, per the task + * brief); `getKysely()` reuses the ambient transaction connection inside + * `em.transactional()`/`uow.transactional()` (confirmed by its own doc + * comment and proven directly — see the repository test that revokes + * inside a transaction which then rolls back and asserts nothing was + * revoked), so D6's single-connection serialisation still holds. + * + * Proven equal to the legacy statement, run raw on the same seeded rows, + * on a 4-level chain plus a sibling branch that must NOT be collected + * (`OAUTHTOKREPO` CTE test) — see the task report for the exact generated + * SQL and both result sets side by side. + */ + async collectChainIds(rootId: number): Promise { + const rows = await this.getEntityManager() + .getKysely() + .withRecursive('chain', (db) => + db.selectFrom('oauth_tokens') + .select('id') + .where('id', '=', rootId) + .unionAll((eb) => + eb.selectFrom('oauth_tokens as ot2') + .innerJoin('chain as c', 'c.id', 'ot2.parent_token_id') + .select('ot2.id'), + ), + ) + .selectFrom('chain') + .select('id') + .execute(); + return rows.map((r) => r.id); + } + + // --------------------------------------------------------------------- + // OA19 — chain-wide revoke + // --------------------------------------------------------------------- + + /** + * `` UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id IN + * (${ids.map(() => '?').join(',')}) AND revoked_at IS NULL `` — + * **security-sensitive**. The legacy hand-built a dynamic-length + * placeholder list; `nativeUpdate({ id: { $in: ids } })` replaces it + * outright (D4/D10's base case for exactly this shape — the task brief's + * "one statement, assert the count"). The legacy also guards `ids.length + * > 0` before running the statement at all (an empty `IN ()` is either a + * syntax error or a vacuous no-op depending on driver, never attempted) — + * reproduced here rather than trusting `nativeUpdate` to no-op safely on + * an empty `$in` array. + */ + async revokeByIds(ids: number[]): Promise { + if (ids.length === 0) return; + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate({ id: { $in: ids }, revoked_at: null }, { revoked_at: currentTimestamp(platform) }); + } + + // --------------------------------------------------------------------- + // OA20 — concurrent-rotation detection + // --------------------------------------------------------------------- + + /** + * `SELECT id FROM oauth_tokens WHERE parent_token_id = ? AND revoked_at + * IS NULL LIMIT 1` — **security-sensitive**, distinguishes concurrent- + * refresh from theft (`isConcurrentRotation`). `parentToken` is a NORMAL + * self-referential relation (references `oauth_tokens.id`, its own + * table's primary key — no `.name()` override, unlike `client`), so a + * plain equality filter is unaffected by this file's class-level finding. + */ + async findSuccessorAlive(parentId: number): Promise<{ id: number } | null> { + const row = await this.findOne({ parentToken: parentId, revoked_at: null }, { fields: ['id'], disableIdentityMap: true }); + return row ? { id: row.id } : null; + } + + // --------------------------------------------------------------------- + // OA22 — refresh-token-hash lookup + // --------------------------------------------------------------------- + + /** + * `SELECT id, client_id, user_id, scopes, audience, + * refresh_token_expires_at, revoked_at, parent_token_id FROM oauth_tokens + * WHERE refresh_token_hash = ?` — **security-sensitive**. `fields` names + * every FK's RELATION property (`'client'`, `'user'`, `'parentToken'`), + * never the `persist(false)` mirrors — verified empirically that + * `row.client.id` reads back the RAW `client_id` COLUMN VALUE (not a + * dereference through the relation's — wrong — declared target): a `Ref`'s + * `.id` is simply the value stored in this entity's own FK column, + * synchronous, no second query, unaffected by what column the relation + * nominally references. Confirmed with a throwaway probe before writing + * this method (see the task report). + */ + async findByRefreshTokenHash(hash: string): Promise { + const row = await this.findOne( + { refresh_token_hash: hash }, + { fields: ['id', 'client', 'user', 'scopes', 'audience', 'refresh_token_expires_at', 'revoked_at', 'parentToken'], disableIdentityMap: true }, + ); + return row + ? { + id: row.id, + client_id: row.client.id, + user_id: row.user.id, + scopes: row.scopes, + audience: row.audience ?? null, + refresh_token_expires_at: row.refresh_token_expires_at, + revoked_at: row.revoked_at ?? null, + parent_token_id: row.parentToken?.id ?? null, + } + : null; + } + + // --------------------------------------------------------------------- + // OA23 / OA28 / OA33 — revoke by id + // --------------------------------------------------------------------- + + /** + * `UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?` — + * **security-sensitive**. THREE call sites share this exact statement: + * OA23 (`refreshTokens`'s rotation revoke-old-pair — stays NON- + * transactional with `issueTokens`, parity, see the deferred list in the + * task report), OA28 (`revokeSession`), OA33 + * (`adminRevokeOAuthSession`). **Not** OA25 (`revokeToken`'s UPDATE half) + * — that statement has a genuinely different WHERE + * (`(access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ?`, + * not `id = ?`); the inventory's summary prose conflated the two, the + * detailed per-site table (verified against `oauth.service.ts:536-540`) + * does not — see `revokeByAccessOrRefreshHashAndClient` below, and the + * task report's inventory-correction note. + */ + async revokeById(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate({ id }, { revoked_at: currentTimestamp(platform) }); + } + + // --------------------------------------------------------------------- + // OA24 / OA25 — revoke by presented token + // --------------------------------------------------------------------- + + /** + * `SELECT user_id FROM oauth_tokens WHERE (access_token_hash = ? OR + * refresh_token_hash = ?) AND client_id = ?` (`revokeToken`'s user-id + * lookup, so the caller can also revoke the user's live MCP sessions) — + * **security-sensitive**. + */ + async findByAccessOrRefreshHashAndClient(hash: string, clientId: string): Promise { + const row = await this.findOne( + { $or: [{ access_token_hash: hash }, { refresh_token_hash: hash }], client: clientId }, + { fields: ['user'], disableIdentityMap: true }, + ); + return row ? { user_id: row.user.id } : null; + } + + /** + * `UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE + * (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ?` — + * **security-sensitive**, the UPDATE half of `revokeToken`. Stays NON- + * transactional with `findByAccessOrRefreshHashAndClient` above (parity — + * see the task report's deferred list); a concurrent revoke/lookup race + * on the same token is the same legacy window this repository preserves, + * not widens or narrows. + */ + async revokeByAccessOrRefreshHashAndClient(hash: string, clientId: string): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate( + { $or: [{ access_token_hash: hash }, { refresh_token_hash: hash }], client: clientId }, + { revoked_at: currentTimestamp(platform) }, + ); + } + + // --------------------------------------------------------------------- + // OA26 — the user's own "active sessions" panel + // --------------------------------------------------------------------- + + /** + * ```sql + * SELECT ot.id, ot.client_id, oc.name AS client_name, ot.scopes, + * ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at + * FROM oauth_tokens ot + * JOIN oauth_clients oc ON ot.client_id = oc.client_id + * WHERE ot.user_id = ? AND ot.revoked_at IS NULL + * AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP + * ORDER BY ot.created_at DESC + * ``` + * + * `em.getKysely()`, NOT the QueryBuilder (this class's docstring): the + * join is `ot.client_id = oc.client_id`, the real FK target per the + * migration, which `.innerJoin('ot.client', 'oc')` cannot express + * correctly (it would join `ot.client_id = oc.id` instead — silently + * wrong, not an error). `currentTimestampKysely` for the same + * `CURRENT_TIMESTAMP` literal `currentTimestamp` spells for the + * QueryBuilder/`nativeUpdate` — Kysely has its own expression system and + * cannot consume a MikroORM `RawQueryFragment` directly (see + * `sql-functions.ts`'s docstring on that helper). Verified empirically + * against a real seeded row before committing to this shape (see the + * task report). + */ + async listActiveByUser(userId: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const rows = await this.getEntityManager() + .getKysely() + .selectFrom('oauth_tokens as ot') + .innerJoin('oauth_clients as oc', 'oc.client_id', 'ot.client_id') + .select(['ot.id', 'ot.client_id', 'oc.name as client_name', 'ot.scopes', 'ot.access_token_expires_at', 'ot.refresh_token_expires_at', 'ot.created_at']) + .where('ot.user_id', '=', userId) + .where('ot.revoked_at', 'is', null) + .where('ot.refresh_token_expires_at', '>', currentTimestampKysely(platform)) + .orderBy('ot.created_at', 'desc') + .execute(); + return rows.map((row) => ({ + id: row.id, + client_id: row.client_id, + client_name: row.client_name, + scopes: row.scopes, + access_token_expires_at: row.access_token_expires_at, + refresh_token_expires_at: row.refresh_token_expires_at, + created_at: row.created_at ?? null, + })); + } + + // --------------------------------------------------------------------- + // OA27 — session-revoke ownership + 404 check + // --------------------------------------------------------------------- + + /** `SELECT id, client_id FROM oauth_tokens WHERE id = ? AND user_id = ?` — 404-never-403, ownership scoped in the WHERE. */ + async findOwnedById(id: number, userId: number): Promise { + const row = await this.findOne({ id, user: userId }, { fields: ['id', 'client'], disableIdentityMap: true }); + return row ? { id: row.id, client_id: row.client.id } : null; + } + + // --------------------------------------------------------------------- + // OA31 — admin "live sessions" panel + // --------------------------------------------------------------------- + + /** + * ```sql + * SELECT ot.id, ot.client_id, oc.name AS client_name, ot.user_id, u.username, + * ot.scopes, ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at + * FROM oauth_tokens ot + * JOIN oauth_clients oc ON ot.client_id = oc.client_id + * JOIN users u ON u.id = ot.user_id + * WHERE ot.revoked_at IS NULL AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP + * ORDER BY ot.created_at DESC + * ``` + * + * `em.getKysely()`, same reason as `listActiveByUser` (OA26) — the + * `oauth_clients` join. `users` joins correctly either way (its relation + * is normally mapped), joined here through Kysely too for one consistent + * query shape rather than mixing a QueryBuilder join into a Kysely query. + * Malformed `scopes` JSON degrading to `null` per-row (not a 500) stays + * in the SERVICE, unchanged — this repository returns the raw column + * text. + */ + async listAllActiveWithClientAndUser(): Promise { + const rows = await this.getEntityManager() + .getKysely() + .selectFrom('oauth_tokens as ot') + .innerJoin('oauth_clients as oc', 'oc.client_id', 'ot.client_id') + .innerJoin('users as u', 'u.id', 'ot.user_id') + .select(['ot.id', 'ot.client_id', 'oc.name as client_name', 'ot.user_id', 'u.username', 'ot.scopes', 'ot.access_token_expires_at', 'ot.refresh_token_expires_at', 'ot.created_at']) + .where('ot.revoked_at', 'is', null) + .where('ot.refresh_token_expires_at', '>', currentTimestampKysely(this.getEntityManager().getPlatform())) + .orderBy('ot.created_at', 'desc') + .execute(); + return rows.map((row) => ({ + id: row.id, + client_id: row.client_id, + client_name: row.client_name, + user_id: row.user_id, + username: row.username, + scopes: row.scopes, + access_token_expires_at: row.access_token_expires_at, + refresh_token_expires_at: row.refresh_token_expires_at, + created_at: row.created_at ?? null, + })); + } + + // --------------------------------------------------------------------- + // OA32 — admin revoke's 404 check + // --------------------------------------------------------------------- + + /** `SELECT id, user_id, client_id FROM oauth_tokens WHERE id = ?` — unscoped (admin), distinct from `findOwnedById` (OA27). */ + async findById(id: number): Promise { + const row = await this.findOne({ id }, { fields: ['id', 'user', 'client'], disableIdentityMap: true }); + return row ? { id: row.id, user_id: row.user.id, client_id: row.client.id } : null; + } +} diff --git a/server/src/nest/oauth/oauth.module.ts b/server/src/nest/oauth/oauth.module.ts index 9dc0e1299..319f805fb 100644 --- a/server/src/nest/oauth/oauth.module.ts +++ b/server/src/nest/oauth/oauth.module.ts @@ -1,6 +1,7 @@ import { RateLimitModule } from '../common/rate-limit.module'; import { Module } from '@nestjs/common'; import type { MiddlewareConsumer, NestModule } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { authorizationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/authorize'; import { clientRegistrationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/register'; import { OauthPublicController } from './oauth-public.controller'; @@ -11,6 +12,10 @@ import { AuditModule } from '../audit/audit.module'; import { AddonsModule } from '../addons/addons.module'; import { AddonsService } from '../addons/addons.service'; import { createMcpAddonGate } from '../addons/mcp-addon-gate'; +import { OauthClients } from '../../db/entities/OauthClients.entity'; +import { OauthTokens } from '../../db/entities/OauthTokens.entity'; +import { OauthConsents } from '../../db/entities/OauthConsents.entity'; +import { Users } from '../../db/entities/Users.entity'; /** * OAuth 2.1 server (MCP). Public token/userinfo/revoke endpoints + the SPA's @@ -35,9 +40,19 @@ import { createMcpAddonGate } from '../addons/mcp-addon-gate'; * * Exports OauthService for AdminController (admin OAuth-session panel) and the * MCP transport's token verification. + * + * `MikroOrmModule.forFeature([OauthClients, OauthTokens, OauthConsents, + * Users])` (Plan 3b Task 4 deliverable): `Users` is registered for cross- + * module availability (inventory §6 — `OauthTokensRepository` needs to be + * importable from `AuthModule` too, Task 5's `revokeAllForUser`, the same + * shape `McpTokensRepository`/`InviteTokensRepository` already use across + * `nest/tokens`/`nest/auth`/`nest/oidc`) even though `OauthService` itself + * never injects `UsersRepository` directly — OA16/OA31 read `users` columns + * THROUGH `OauthTokensRepository`'s own join methods, not a separate + * `Users` call (Task 1's ruling: "do not add users methods" for this join). */ @Module({ - imports: [RateLimitModule, AuditModule, AddonsModule], + imports: [RateLimitModule, AuditModule, AddonsModule, MikroOrmModule.forFeature([OauthClients, OauthTokens, OauthConsents, Users])], controllers: [OauthPublicController, OauthApiController], providers: [OauthService, TrekClientsStore, TrekOAuthProvider], exports: [OauthService], diff --git a/server/src/nest/oauth/oauth.service.ts b/server/src/nest/oauth/oauth.service.ts index a8fe81c73..5d67de26f 100644 --- a/server/src/nest/oauth/oauth.service.ts +++ b/server/src/nest/oauth/oauth.service.ts @@ -1,5 +1,6 @@ import crypto, { randomBytes, randomUUID } from 'crypto'; import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { ADDON_IDS } from '../../addons'; import { getMcpSafeUrl } from '../../app-config'; // Import from scopes/sessionManager directly, NOT the ../../mcp barrel: the @@ -13,7 +14,13 @@ import { User } from '../../types'; import { AddonsService } from '../addons/addons.service'; import { AuditService } from '../audit/audit.service'; import { logWarn } from '../audit/audit-log.logger'; -import { DatabaseService } from '../database/database.service'; +import { toRowId } from '../common/row-id'; +import { OauthClients } from '../../db/entities/OauthClients.entity'; +import type { OauthClientRow, OauthClientsRepository } from '../../db/repositories/OauthClients.repository'; +import { OauthTokens } from '../../db/entities/OauthTokens.entity'; +import type { OauthTokenRefreshRow, OauthTokensRepository } from '../../db/repositories/OauthTokens.repository'; +import { OauthConsents } from '../../db/entities/OauthConsents.entity'; +import type { OauthConsentsRepository } from '../../db/repositories/OauthConsents.repository'; import { ACCESS_TOKEN_TTL_S, CODE_CHALLENGE_RE, @@ -27,12 +34,9 @@ import { parseSqliteUtc, redirectUriMatches, timingSafeEqualHex, - type OAuthClientRow, - type OAuthTokenRow, } from './oauth.helpers'; import { AUTH_CODE_TTL_MS, putPendingCode, takePendingCode, type PendingCode } from './oauth.pending-codes'; -export type { OAuthClientRow, OAuthTokenRow } from './oauth.helpers'; export type { PendingCode } from './oauth.pending-codes'; export interface OAuthTokenInfo { @@ -80,7 +84,9 @@ export interface ValidateAuthorizeResult { @Injectable() export class OauthService { constructor( - private readonly db: DatabaseService, + @InjectRepository(OauthClients) private readonly clients: OauthClientsRepository, + @InjectRepository(OauthTokens) private readonly tokens: OauthTokensRepository, + @InjectRepository(OauthConsents) private readonly consents: OauthConsentsRepository, private readonly addons: AddonsService, private readonly audit: AuditService, ) {} @@ -93,10 +99,7 @@ export class OauthService { // ------------------------------------------------------------------------- async listOAuthClients(userId: number): Promise[]> { - const rows = this.db.all( - 'SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, created_at, is_public, created_via, allows_client_credentials FROM oauth_clients WHERE user_id = ? ORDER BY created_at DESC', - userId, - ); + const rows = await this.clients.listByUser(userId); return rows.map(r => ({ ...r, is_public: Boolean(r.is_public), @@ -136,11 +139,11 @@ export class OauthService { if (!valid) return { error: `Invalid scopes: ${invalid.join(', ')}`, status: 400 }; if (userId !== null) { - const count = this.db.get<{ count: number }>('SELECT COUNT(*) as count FROM oauth_clients WHERE user_id = ?', userId)!.count; + const count = await this.clients.countByUser(userId); if (count >= 10) return { error: 'Maximum of 10 OAuth clients per user', status: 400 }; } else { // Anonymous DCR clients: enforce a global cap to prevent unbounded registration abuse - const count = this.db.get<{ count: number }>('SELECT COUNT(*) as count FROM oauth_clients WHERE user_id IS NULL')!.count; + const count = await this.clients.countAnonymous(); if (count >= 500) return { error: 'server_error', status: 503 }; } @@ -153,15 +156,18 @@ export class OauthService { const rawSecret = isPublic ? null : 'trekcs_' + randomBytes(24).toString('hex'); const secretHash = rawSecret ? hashToken(rawSecret) : randomBytes(32).toString('hex'); - this.db.run( - 'INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes, is_public, created_via, allows_client_credentials) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', - id, userId, name.trim(), clientId, secretHash, JSON.stringify(redirectUris), JSON.stringify(allowedScopes), isPublic ? 1 : 0, createdVia, isMachineClient ? 1 : 0, - ); - - const row = this.db.get( - 'SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, created_at, is_public, created_via, allows_client_credentials FROM oauth_clients WHERE id = ?', + const row = await this.clients.insertClient({ id, - )!; + user_id: userId, + name: name.trim(), + client_id: clientId, + client_secret_hash: secretHash, + redirect_uris: JSON.stringify(redirectUris), + allowed_scopes: JSON.stringify(allowedScopes), + is_public: isPublic ? 1 : 0, + created_via: createdVia, + allows_client_credentials: isMachineClient ? 1 : 0, + }); await this.audit.writeAudit({ userId, action: 'oauth.client.create', details: { client_id: clientId, name: name.trim(), is_public: isPublic, allows_client_credentials: isMachineClient }, ip }); @@ -188,17 +194,17 @@ export class OauthService { clientRowId: string, ip?: string | null, ): Promise<{ error?: string; status?: number; client_secret?: string }> { - const row = this.db.get('SELECT id, client_id, is_public FROM oauth_clients WHERE id = ? AND user_id = ?', clientRowId, userId); + const row = await this.clients.findOwned(clientRowId, userId); if (!row) return { error: 'Client not found', status: 404 }; if (row.is_public) return { error: 'Public clients do not use a client secret', status: 400 }; const rawSecret = 'trekcs_' + randomBytes(24).toString('hex'); const secretHash = hashToken(rawSecret); - this.db.run('UPDATE oauth_clients SET client_secret_hash = ? WHERE id = ?', secretHash, clientRowId); + await this.clients.updateSecretHash(clientRowId, secretHash); // Revoke all existing tokens for this client so old sessions are invalidated - this.db.run("UPDATE oauth_tokens SET revoked_at = datetime('now') WHERE client_id = ? AND revoked_at IS NULL", row.client_id); + await this.tokens.revokeAllForClient(row.client_id); // Terminate active MCP sessions for this (user, client) pair revokeUserSessionsForClient(userId, row.client_id); @@ -213,9 +219,9 @@ export class OauthService { clientRowId: string, ip?: string | null, ): Promise<{ error?: string; status?: number; success?: boolean }> { - const row = this.db.get('SELECT id, client_id FROM oauth_clients WHERE id = ? AND user_id = ?', clientRowId, userId); + const row = await this.clients.findOwned(clientRowId, userId); if (!row) return { error: 'Client not found', status: 404 }; - this.db.run('DELETE FROM oauth_clients WHERE id = ?', clientRowId); + await this.clients.remove(clientRowId); await this.audit.writeAudit({ userId, action: 'oauth.client.delete', details: { client_id: row.client_id }, ip }); return { success: true }; } @@ -247,9 +253,7 @@ export class OauthService { // ------------------------------------------------------------------------- async getConsent(clientId: string, userId: number): Promise { - const row = this.db.get<{ scopes: string }>( - 'SELECT scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?', clientId, userId, - ); + const row = await this.consents.findScopes(clientId, userId); return row ? JSON.parse(row.scopes) : null; } @@ -257,10 +261,7 @@ export class OauthService { // Union existing consent with newly approved scopes (M5: never narrow stored consent) const existing = (await this.getConsent(clientId, userId)) ?? []; const merged = Array.from(new Set([...existing, ...scopes])); - this.db.run( - 'INSERT OR REPLACE INTO oauth_consents (client_id, user_id, scopes, updated_at) VALUES (?, ?, ?, CURRENT_TIMESTAMP)', - clientId, userId, JSON.stringify(merged), - ); + await this.consents.upsertGrant(clientId, userId, JSON.stringify(merged)); await this.audit.writeAudit({ userId, action: 'oauth.consent.grant', details: { client_id: clientId, scopes: merged }, ip }); } @@ -294,11 +295,17 @@ export class OauthService { const accessExpiry = new Date(now.getTime() + ACCESS_TOKEN_TTL_S * 1000); const refreshExpiry = new Date(now.getTime() + REFRESH_TOKEN_TTL_MS); - this.db.run(` - INSERT INTO oauth_tokens - (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, parent_token_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - `, clientId, userId, accessHash, refreshHash, JSON.stringify(scopes), audience, accessExpiry.toISOString(), refreshExpiry.toISOString(), parentTokenId); + await this.tokens.insertToken({ + client_id: clientId, + user_id: userId, + access_token_hash: accessHash, + refresh_token_hash: refreshHash, + scopes: JSON.stringify(scopes), + audience, + access_token_expires_at: accessExpiry.toISOString(), + refresh_token_expires_at: refreshExpiry.toISOString(), + parent_token_id: parentTokenId, + }); return { access_token: rawAccess, @@ -334,11 +341,17 @@ export class OauthService { const now = new Date(); const accessExpiry = new Date(now.getTime() + ACCESS_TOKEN_TTL_S * 1000); - this.db.run(` - INSERT INTO oauth_tokens - (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, parent_token_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - `, clientId, userId, accessHash, placeholderHash, JSON.stringify(scopes), audience, accessExpiry.toISOString(), now.toISOString(), null); + await this.tokens.insertToken({ + client_id: clientId, + user_id: userId, + access_token_hash: accessHash, + refresh_token_hash: placeholderHash, + scopes: JSON.stringify(scopes), + audience, + access_token_expires_at: accessExpiry.toISOString(), + refresh_token_expires_at: now.toISOString(), + parent_token_id: null, + }); return { access_token: rawAccess, @@ -361,22 +374,13 @@ export class OauthService { allowed_scopes: string; is_public: number; created_via: string; - } | undefined> { - return this.db.get( - 'SELECT client_id, name, redirect_uris, allowed_scopes, is_public, created_via FROM oauth_clients WHERE client_id = ?', - clientId, - ); + } | null> { + return this.clients.findSdkProjection(clientId); } async getUserByAccessToken(rawToken: string): Promise { const hash = hashToken(rawToken); - const row = this.db.get(` - SELECT ot.scopes, ot.audience, ot.revoked_at, ot.access_token_expires_at, - ot.user_id, ot.client_id, u.username, u.email, u.role - FROM oauth_tokens ot - JOIN users u ON ot.user_id = u.id - WHERE ot.access_token_hash = ? - `, hash); + const row = await this.tokens.findByAccessTokenHashWithUser(hash); if (!row) return null; if (row.revoked_at) return null; @@ -398,7 +402,7 @@ export class OauthService { private async findChainRoot(tokenId: number): Promise { let current = tokenId; for (let i = 0; i < 100; i++) { - const row = this.db.get<{ id: number; parent_token_id: number | null }>('SELECT id, parent_token_id FROM oauth_tokens WHERE id = ?', current); + const row = await this.tokens.findParent(current); if (!row || row.parent_token_id === null) return current; current = row.parent_token_id; } @@ -407,21 +411,8 @@ export class OauthService { /** Revoke all tokens in the rotation chain rooted at rootId. Returns affected ids. */ private async revokeChain(rootId: number): Promise { - const rows = this.db.all<{ id: number }>(` - WITH RECURSIVE chain(id) AS ( - SELECT id FROM oauth_tokens WHERE id = ? - UNION ALL - SELECT t.id FROM oauth_tokens t JOIN chain c ON t.parent_token_id = c.id - ) - SELECT id FROM chain - `, rootId); - const ids = rows.map(r => r.id); - if (ids.length > 0) { - this.db.run( - `UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id IN (${ids.map(() => '?').join(',')}) AND revoked_at IS NULL`, - ...ids, - ); - } + const ids = await this.tokens.collectChainIds(rootId); + await this.tokens.revokeByIds(ids); return ids; } @@ -435,14 +426,11 @@ export class OauthService { * explicit revoke leaves no live child, and a chain revoked after a real replay * has every child revoked with it, so neither can slip through here. */ - private async isConcurrentRotation(row: OAuthTokenRow): Promise { + private async isConcurrentRotation(row: OauthTokenRefreshRow): Promise { const revokedAt = parseSqliteUtc(row.revoked_at); if (!revokedAt) return false; if (Date.now() - revokedAt.getTime() > REFRESH_ROTATION_GRACE_MS) return false; - const successor = this.db.get<{ id: number }>( - 'SELECT id FROM oauth_tokens WHERE parent_token_id = ? AND revoked_at IS NULL LIMIT 1', - row.id, - ); + const successor = await this.tokens.findSuccessorAlive(row.id); return !!successor; } @@ -452,7 +440,7 @@ export class OauthService { clientSecret: string | undefined, ip?: string | null, ): Promise<{ error?: string; status?: number; tokens?: Awaited> }> { - const client = this.db.get('SELECT client_id, client_secret_hash, is_public FROM oauth_clients WHERE client_id = ?', clientId); + const client = await this.clients.findAuthRow(clientId); if (!client) return { error: 'invalid_client', status: 401 }; if (!client.is_public) { if (!clientSecret || !timingSafeEqualHex(hashToken(clientSecret), client.client_secret_hash)) { @@ -461,10 +449,7 @@ export class OauthService { } const hash = hashToken(rawRefreshToken); - const row = this.db.get(` - SELECT id, client_id, user_id, scopes, audience, refresh_token_expires_at, revoked_at, parent_token_id - FROM oauth_tokens WHERE refresh_token_hash = ? - `, hash); + const row = await this.tokens.findByRefreshTokenHash(hash); if (!row) return { error: 'invalid_grant', status: 400 }; if (row.client_id !== clientId) return { error: 'invalid_grant', status: 400 }; @@ -512,7 +497,13 @@ export class OauthService { // already re-validates session.userId/clientId against the new token on every // request. Killing the session on every routine hourly refresh broke long-lived // MCP connections (#1475). - this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', row.id); + // + // Stays NON-transactional with the issueTokens() call below — legacy + // parity (Task 4 brief ruling, §9.6 of the inventory): revoke-old then + // issue-new are two separate statements, not one transaction. A crash + // between the two would leave the old token revoked with no successor — + // the same window the legacy code always had. + await this.tokens.revokeById(row.id); const tokens = await this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null); await this.audit.writeAudit({ userId: row.user_id, action: 'oauth.token.refresh', details: { client_id: clientId }, ip }); @@ -527,17 +518,14 @@ export class OauthService { async revokeToken(rawToken: string, clientId: string, userId?: number, ip?: string | null): Promise { const hash = hashToken(rawToken); - // Get the user_id for the token so we can revoke its MCP sessions - const row = this.db.get<{ user_id: number }>( - 'SELECT user_id FROM oauth_tokens WHERE (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ?', - hash, hash, clientId, - ); + // Get the user_id for the token so we can revoke its MCP sessions. + // + // Stays NON-transactional with the UPDATE below — legacy parity (Task 4 + // brief ruling, §9.6): a SELECT-then-UPDATE with no transaction between + // them, same window the legacy code always had. + const row = await this.tokens.findByAccessOrRefreshHashAndClient(hash, clientId); - this.db.run(` - UPDATE oauth_tokens - SET revoked_at = CURRENT_TIMESTAMP - WHERE (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ? - `, hash, hash, clientId); + await this.tokens.revokeByAccessOrRefreshHashAndClient(hash, clientId); const affectedUserId = row?.user_id ?? userId; if (affectedUserId) { @@ -551,17 +539,8 @@ export class OauthService { // ------------------------------------------------------------------------- async listOAuthSessions(userId: number): Promise[]> { - const rows = this.db.all>(` - SELECT ot.id, ot.client_id, oc.name AS client_name, ot.scopes, - ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at - FROM oauth_tokens ot - JOIN oauth_clients oc ON ot.client_id = oc.client_id - WHERE ot.user_id = ? - AND ot.revoked_at IS NULL - AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP - ORDER BY ot.created_at DESC - `, userId); - return rows.map(r => ({ ...r, scopes: JSON.parse(r.scopes as string) })); + const rows = await this.tokens.listActiveByUser(userId); + return rows.map(r => ({ ...r, scopes: JSON.parse(r.scopes) })); } async revokeSession( @@ -569,10 +548,13 @@ export class OauthService { sessionId: number, ip?: string | null, ): Promise<{ error?: string; status?: number; success?: boolean }> { - const row = this.db.get<{ id: number; client_id: string }>('SELECT id, client_id FROM oauth_tokens WHERE id = ? AND user_id = ?', sessionId, userId); + const id = toRowId(sessionId); + if (id === null) return { error: 'Session not found', status: 404 }; + + const row = await this.tokens.findOwnedById(id, userId); if (!row) return { error: 'Session not found', status: 404 }; - this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', sessionId); + await this.tokens.revokeById(id); revokeUserSessionsForClient(userId, row.client_id); @@ -610,7 +592,7 @@ export class OauthService { return { valid: false, error: 'invalid_request', error_description: 'client_id is required' }; } - const client = this.db.get('SELECT * FROM oauth_clients WHERE client_id = ?', params.client_id); + const client = await this.clients.findByClientIdFull(params.client_id); if (!client) { return { valid: false, error: 'invalid_client', error_description: 'Unknown client_id' }; } @@ -692,8 +674,8 @@ export class OauthService { // Client authentication (for token endpoint) // ------------------------------------------------------------------------- - async authenticateClient(clientId: string, clientSecret: string | undefined): Promise { - const client = this.db.get('SELECT * FROM oauth_clients WHERE client_id = ?', clientId); + async authenticateClient(clientId: string, clientSecret: string | undefined): Promise { + const client = await this.clients.findByClientIdFull(clientId); if (!client) return null; if (client.is_public) { // Public clients are identified by client_id alone — PKCE provides the security guarantee. @@ -712,16 +694,7 @@ export class OauthService { // --------------------------------------------------------------------------- async listAllOAuthSessions() { - const rows = this.db.all & { scopes: string }>(` - SELECT ot.id, ot.client_id, oc.name AS client_name, ot.user_id, u.username, - ot.scopes, ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at - FROM oauth_tokens ot - JOIN oauth_clients oc ON ot.client_id = oc.client_id - JOIN users u ON u.id = ot.user_id - WHERE ot.revoked_at IS NULL - AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP - ORDER BY ot.created_at DESC - `); + const rows = await this.tokens.listAllActiveWithClientAndUser(); // One malformed row must not 500 the whole admin OAuth-sessions panel. return rows.map((r) => { let scopes: unknown; @@ -735,11 +708,12 @@ export class OauthService { } async adminRevokeOAuthSession(id: string) { - const row = this.db.get<{ id: number; user_id: number; client_id: string }>( - 'SELECT id, user_id, client_id FROM oauth_tokens WHERE id = ?', id, - ); + const tokenId = toRowId(id); + if (tokenId === null) return { error: 'Session not found', status: 404 }; + + const row = await this.tokens.findById(tokenId); if (!row) return { error: 'Session not found', status: 404 }; - this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', id); + await this.tokens.revokeById(tokenId); revokeUserSessionsForClient(row.user_id, row.client_id); return {}; } diff --git a/server/tests/integration/admin.test.ts b/server/tests/integration/admin.test.ts index 67e6d2a5c..763c9a340 100644 --- a/server/tests/integration/admin.test.ts +++ b/server/tests/integration/admin.test.ts @@ -813,6 +813,16 @@ describe('OAuth sessions', () => { .set('Cookie', authCookie(admin.id)); expect(res.status).toBe(404); }); + + it('ADMIN-026B — DELETE /admin/oauth-sessions/abc (non-numeric id) returns the legacy 404, not a 500 (Plan 3b Task 4 controller addendum, per Task 2 review F1)', async () => { + const { user: admin } = createAdmin(testDb); + + const res = await request(app) + .delete('/api/admin/oauth-sessions/abc') + .set('Cookie', authCookie(admin.id)); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'Session not found' }); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/server/tests/integration/oauth-authorize.test.ts b/server/tests/integration/oauth-authorize.test.ts index 0d1c7fe71..702d3af54 100644 --- a/server/tests/integration/oauth-authorize.test.ts +++ b/server/tests/integration/oauth-authorize.test.ts @@ -41,18 +41,19 @@ import { buildApp } from '../../src/bootstrap'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser } from '../helpers/factories'; import { OauthService } from '../../src/nest/oauth/oauth.service'; -import { DatabaseService } from '../../src/nest/database/database.service'; import { createTestAddonsService } from '../helpers/test-addons'; import { AuditService } from '../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; +import { OauthClients } from '../../src/db/entities/OauthClients.entity'; +import { OauthTokens } from '../../src/db/entities/OauthTokens.entity'; +import { OauthConsents } from '../../src/db/entities/OauthConsents.entity'; // The consent controller writes pending codes through the container instance; // the SDK-mounted authorize path reads them back. The map is module-scoped in // oauth.pending-codes.ts, so a hand-built service instance shares it — the // same single-instance property the full auth-code loop below proves end-to-end. -const oauthDbs = new DatabaseService(testDb); let containerSideOauth: OauthService; let nestApp: INestApplication; @@ -85,7 +86,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); t = await createTestOrm(testDb); - containerSideOauth = new OauthService(oauthDbs, await createTestAddonsService(testDb, oauthDbs), new AuditService(t.repo(AuditLog), t.repo(Users))); + containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users))); }); beforeEach(() => { diff --git a/server/tests/integration/oauth.test.ts b/server/tests/integration/oauth.test.ts index fbe238959..66be9a618 100644 --- a/server/tests/integration/oauth.test.ts +++ b/server/tests/integration/oauth.test.ts @@ -38,19 +38,20 @@ import { createUser } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; import { ALL_SCOPES } from '../../src/mcp/scopes'; import { OauthService } from '../../src/nest/oauth/oauth.service'; -import { DatabaseService } from '../../src/nest/database/database.service'; import { createTestAddonsService } from '../helpers/test-addons'; import { AuditService } from '../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; +import { OauthClients } from '../../src/db/entities/OauthClients.entity'; +import { OauthTokens } from '../../src/db/entities/OauthTokens.entity'; +import { OauthConsents } from '../../src/db/entities/OauthConsents.entity'; // In production the consent controller writes pending codes through the // container instance and the SDK routes read them back through the same // injected singleton. These tests write codes the way the consent controller // does: through a service instance. The pending-code map is module-scoped in // oauth.pending-codes.ts, so the routes under test see every code written here. -const oauthDbs = new DatabaseService(testDb); let containerSideOauth: OauthService; // Arrow wrappers rather than `.bind`: with `strictBindCallApply: false` a bound // alias is typed `any`, which hides a missing `await` on the now-async methods. @@ -83,7 +84,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); t = await createTestOrm(testDb); - containerSideOauth = new OauthService(oauthDbs, await createTestAddonsService(testDb, oauthDbs), new AuditService(t.repo(AuditLog), t.repo(Users))); + containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users))); }); beforeEach(() => { @@ -1136,6 +1137,15 @@ describe('Sessions — /api/oauth/sessions', () => { expect(res.status).toBe(404); }); + it('OAUTH-043B — DELETE /sessions/abc (non-numeric id) returns the legacy 404, not a 500 (Plan 3b Task 4 controller addendum, per Task 2 review F1)', async () => { + const { user } = createUser(testDb); + const res = await request(app) + .delete('/api/oauth/sessions/abc') + .set('Cookie', authCookie(user.id)); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'Session not found' }); + }); + it('OAUTH-044 — DELETE /sessions/:id returns 403 when addon disabled', async () => { setMcpEnabled(false); const { user } = createUser(testDb); diff --git a/server/tests/unit/db/repositories/OauthClients.repository.test.ts b/server/tests/unit/db/repositories/OauthClients.repository.test.ts new file mode 100644 index 000000000..1f0db586f --- /dev/null +++ b/server/tests/unit/db/repositories/OauthClients.repository.test.ts @@ -0,0 +1,233 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser } from '../../../helpers/factories'; +import { OauthClients } from '../../../../src/db/entities/OauthClients.entity'; +import type { OauthClientsRepository } from '../../../../src/db/repositories/OauthClients.repository'; +import { UnitOfWork } from '../../../../src/nest/database/unit-of-work'; +import { withRequestContext } from '../../../../src/nest/database/request-context'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let clients: OauthClientsRepository; +let uow: UnitOfWork; + +beforeAll(async () => { + t = await createTestOrm(testDb); + clients = t.repo(OauthClients); + uow = new UnitOfWork(t.em); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); testDb.exec('DELETE FROM oauth_clients'); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +function seedClient( + overrides: Partial<{ id: string; userId: number | null; name: string; clientId: string; secretHash: string; redirectUris: string; allowedScopes: string; isPublic: number; createdVia: string; allowsClientCredentials: number }> = {}, +): { id: string; userId: number | null; clientId: string } { + const id = overrides.id ?? `row-${Math.random().toString(36).slice(2)}`; + const clientId = overrides.clientId ?? `proto-${Math.random().toString(36).slice(2)}`; + testDb.prepare( + `INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes, is_public, created_via, allows_client_credentials) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ).run( + id, + overrides.userId ?? null, + overrides.name ?? 'Test Client', + clientId, + overrides.secretHash ?? 'hash-value', + overrides.redirectUris ?? '["https://example.com/cb"]', + overrides.allowedScopes ?? '["trips:read"]', + overrides.isPublic ?? 0, + overrides.createdVia ?? 'settings_ui', + overrides.allowsClientCredentials ?? 0, + ); + return { id, userId: overrides.userId ?? null, clientId }; +} + +describe('OauthClientsRepository', () => { + describe('listByUser (OA1)', () => { + it('OAUTHCLIENTREPO-001: returns the caller\'s clients only, newest first, without the secret hash', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + seedClient({ userId: user.id, name: 'first', createdVia: 'settings_ui' }); + testDb.exec("UPDATE oauth_clients SET created_at = '2020-01-01 00:00:00' WHERE name = 'first'"); + seedClient({ userId: user.id, name: 'second' }); + testDb.exec("UPDATE oauth_clients SET created_at = '2021-01-01 00:00:00' WHERE name = 'second'"); + seedClient({ userId: other.id, name: 'not-mine' }); + + const rows = await clients.listByUser(user.id); + expect(rows.map((r) => r.name)).toEqual(['second', 'first']); + expect(rows[0]).not.toHaveProperty('client_secret_hash'); + expect(rows[0].user_id).toBe(user.id); + }); + + it('OAUTHCLIENTREPO-002: empty for a user with no clients', async () => { + const { user } = createUser(testDb); + expect(await clients.listByUser(user.id)).toEqual([]); + }); + }); + + describe('countByUser / countAnonymous (OA2/OA3)', () => { + it('OAUTHCLIENTREPO-003: countByUser only counts the given user\'s rows', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + seedClient({ userId: user.id }); + seedClient({ userId: user.id }); + seedClient({ userId: other.id }); + expect(await clients.countByUser(user.id)).toBe(2); + expect(await clients.countByUser(other.id)).toBe(1); + }); + + it('OAUTHCLIENTREPO-004: countAnonymous counts only NULL-user rows', async () => { + const { user } = createUser(testDb); + seedClient({ userId: user.id }); + seedClient({ userId: null }); + seedClient({ userId: null }); + expect(await clients.countAnonymous()).toBe(2); + }); + }); + + describe('insertClient / findPublicById (OA4/OA5)', () => { + it('OAUTHCLIENTREPO-005: writes exactly the given columns and the re-select matches', async () => { + const { user } = createUser(testDb); + const row = await clients.insertClient({ + id: 'client-row-1', + user_id: user.id, + name: 'My Client', + client_id: 'proto-abc', + client_secret_hash: 'secrethash', + redirect_uris: '["https://a.example.com/cb"]', + allowed_scopes: '["trips:read","trips:write"]', + is_public: 0, + created_via: 'settings_ui', + allows_client_credentials: 0, + }); + expect(row).toMatchObject({ + id: 'client-row-1', + user_id: user.id, + name: 'My Client', + client_id: 'proto-abc', + redirect_uris: '["https://a.example.com/cb"]', + allowed_scopes: '["trips:read","trips:write"]', + is_public: 0, + created_via: 'settings_ui', + allows_client_credentials: 0, + }); + const raw = testDb.prepare('SELECT * FROM oauth_clients WHERE id = ?').get('client-row-1') as Record; + expect(raw.client_secret_hash).toBe('secrethash'); + }); + + it('OAUTHCLIENTREPO-006: insertClient writes a NULL user_id for anonymous DCR clients', async () => { + const row = await clients.insertClient({ + id: 'client-row-2', + user_id: null, + name: 'DCR Client', + client_id: 'proto-dcr', + client_secret_hash: 'x', + redirect_uris: '[]', + allowed_scopes: '[]', + is_public: 1, + created_via: 'dcr', + allows_client_credentials: 0, + }); + expect(row.user_id).toBeNull(); + }); + }); + + describe('findOwned (OA6/OA9)', () => { + it('OAUTHCLIENTREPO-007: matches id+user, returns the superset projection', async () => { + const { user } = createUser(testDb); + const { id, clientId } = seedClient({ userId: user.id, isPublic: 1 }); + const row = await clients.findOwned(id, user.id); + expect(row).toEqual({ id, client_id: clientId, is_public: 1 }); + }); + + it('OAUTHCLIENTREPO-008: 404-shape null for another user\'s client (never 403)', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + const { id } = seedClient({ userId: other.id }); + expect(await clients.findOwned(id, user.id)).toBeNull(); + }); + }); + + describe('updateSecretHash (OA7)', () => { + it('OAUTHCLIENTREPO-009: writes the new hash, nothing else', async () => { + const { id } = seedClient({ secretHash: 'old' }); + await clients.updateSecretHash(id, 'new-hash'); + const row = testDb.prepare('SELECT client_secret_hash FROM oauth_clients WHERE id = ?').get(id) as { client_secret_hash: string }; + expect(row.client_secret_hash).toBe('new-hash'); + }); + }); + + describe('remove (OA10)', () => { + it('OAUTHCLIENTREPO-010: deletes the row', async () => { + const { id } = seedClient(); + await clients.remove(id); + expect(testDb.prepare('SELECT id FROM oauth_clients WHERE id = ?').get(id)).toBeUndefined(); + }); + }); + + describe('findSdkProjection (OA15)', () => { + it('OAUTHCLIENTREPO-011: never carries client_secret_hash', async () => { + const { clientId } = seedClient({ secretHash: 'super-secret', name: 'SDK Client' }); + const row = await clients.findSdkProjection(clientId); + expect(row).toEqual({ + client_id: clientId, + name: 'SDK Client', + redirect_uris: '["https://example.com/cb"]', + allowed_scopes: '["trips:read"]', + is_public: 0, + created_via: 'settings_ui', + }); + expect(row).not.toHaveProperty('client_secret_hash'); + }); + + it('OAUTHCLIENTREPO-012: null for unknown client_id', async () => { + expect(await clients.findSdkProjection('nonexistent')).toBeNull(); + }); + }); + + describe('findAuthRow (OA21)', () => { + it('OAUTHCLIENTREPO-013: exactly the three auth columns, a distinct projection from OA15', async () => { + const { clientId } = seedClient({ secretHash: 'auth-secret', isPublic: 0 }); + const row = await clients.findAuthRow(clientId); + expect(row).toEqual({ client_id: clientId, client_secret_hash: 'auth-secret', is_public: 0 }); + }); + }); + + describe('findByClientIdFull (OA29/OA30)', () => { + it('OAUTHCLIENTREPO-014: the full row, including the secret hash', async () => { + const { user } = createUser(testDb); + const { clientId } = seedClient({ userId: user.id, secretHash: 'full-row-secret' }); + const row = await clients.findByClientIdFull(clientId); + expect(row?.client_secret_hash).toBe('full-row-secret'); + expect(row?.user_id).toBe(user.id); + }); + + it('OAUTHCLIENTREPO-015: null for unknown client_id', async () => { + expect(await clients.findByClientIdFull('nope')).toBeNull(); + }); + }); + + describe('identity-map write-back regression (program RULING, D-shape)', () => { + it('OAUTHCLIENTREPO-016: listByUser then findAuthRow then updateSecretHash inside uow.transactional — the write survives', async () => { + const { user } = createUser(testDb); + const { id, clientId } = seedClient({ userId: user.id, secretHash: 'original', name: 'kept-name' }); + + await withRequestContext(t.orm, async () => { + // Projection A + await clients.listByUser(user.id); + // Projection B — a different field set on the same row + await clients.findAuthRow(clientId); + // The intended write, inside the request's own transaction. + await uow.transactional(async () => { + await clients.updateSecretHash(id, 'rotated'); + }); + }); + + const row = testDb.prepare('SELECT client_secret_hash, name FROM oauth_clients WHERE id = ?').get(id) as { client_secret_hash: string; name: string }; + expect(row.client_secret_hash).toBe('rotated'); // the nativeUpdate must stick + expect(row.name).toBe('kept-name'); // untouched by either read + }); + }); +}); diff --git a/server/tests/unit/db/repositories/OauthConsents.repository.test.ts b/server/tests/unit/db/repositories/OauthConsents.repository.test.ts new file mode 100644 index 000000000..bc0dfd1fd --- /dev/null +++ b/server/tests/unit/db/repositories/OauthConsents.repository.test.ts @@ -0,0 +1,112 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser } from '../../../helpers/factories'; +import { OauthConsents } from '../../../../src/db/entities/OauthConsents.entity'; +import type { OauthConsentsRepository } from '../../../../src/db/repositories/OauthConsents.repository'; +import { UnitOfWork } from '../../../../src/nest/database/unit-of-work'; +import { withRequestContext } from '../../../../src/nest/database/request-context'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let consents: OauthConsentsRepository; +let uow: UnitOfWork; + +beforeAll(async () => { + t = await createTestOrm(testDb); + consents = t.repo(OauthConsents); + uow = new UnitOfWork(t.em); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); testDb.exec('DELETE FROM oauth_consents'); testDb.exec('DELETE FROM oauth_clients'); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +function seedClient(userId: number, clientId: string): void { + testDb.prepare( + `INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes) VALUES (?, ?, ?, ?, ?, '[]', '[]')`, + ).run(`row-${clientId}`, userId, 'C', clientId, 'hash'); +} + +describe('OauthConsentsRepository', () => { + describe('findScopes (OA11)', () => { + it('OAUTHCONSENTREPO-001: null when no consent has been granted', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-1'); + expect(await consents.findScopes('proto-1', user.id)).toBeNull(); + }); + + it('OAUTHCONSENTREPO-002: returns the stored scopes JSON text', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-2'); + testDb.prepare(`INSERT INTO oauth_consents (client_id, user_id, scopes) VALUES (?, ?, ?)`).run('proto-2', user.id, '["trips:read"]'); + expect(await consents.findScopes('proto-2', user.id)).toEqual({ scopes: '["trips:read"]' }); + }); + }); + + describe('upsertGrant (OA12)', () => { + it('OAUTHCONSENTREPO-003: inserts a new row exactly as the legacy INSERT would', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-3'); + await consents.upsertGrant('proto-3', user.id, '["a"]'); + const row = testDb.prepare('SELECT client_id, user_id, scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?').get('proto-3', user.id); + expect(row).toEqual({ client_id: 'proto-3', user_id: user.id, scopes: '["a"]' }); + }); + + it('OAUTHCONSENTREPO-004: on an existing (client, user) it replaces the row in place — no duplicate, matching ON CONFLICT(client_id, user_id) DO UPDATE', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-4'); + await consents.upsertGrant('proto-4', user.id, '["a"]'); + await consents.upsertGrant('proto-4', user.id, '["a","b"]'); + const rows = testDb.prepare('SELECT scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?').all('proto-4', user.id); + expect(rows).toHaveLength(1); + expect((rows[0] as { scopes: string }).scopes).toBe('["a","b"]'); + }); + + it('OAUTHCONSENTREPO-005: updated_at is bumped on every call, insert or merge alike (unconditional, matching INSERT OR REPLACE)', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-5'); + await consents.upsertGrant('proto-5', user.id, '["a"]'); + const first = testDb.prepare('SELECT updated_at FROM oauth_consents WHERE client_id = ? AND user_id = ?').get('proto-5', user.id) as { updated_at: string }; + testDb.exec(`UPDATE oauth_consents SET updated_at = '2000-01-01 00:00:00' WHERE client_id = 'proto-5'`); + await consents.upsertGrant('proto-5', user.id, '["a","b"]'); + const second = testDb.prepare('SELECT updated_at FROM oauth_consents WHERE client_id = ? AND user_id = ?').get('proto-5', user.id) as { updated_at: string }; + expect(second.updated_at).not.toBe('2000-01-01 00:00:00'); + expect(first.updated_at).toBeTruthy(); + }); + + it('OAUTHCONSENTREPO-006: the same client scoped by different users stores two separate rows, not a collision', async () => { + const { user: a } = createUser(testDb); + const { user: b } = createUser(testDb); + seedClient(a.id, 'proto-6'); + await consents.upsertGrant('proto-6', a.id, '["a"]'); + await consents.upsertGrant('proto-6', b.id, '["b"]'); + expect(await consents.findScopes('proto-6', a.id)).toEqual({ scopes: '["a"]' }); + expect(await consents.findScopes('proto-6', b.id)).toEqual({ scopes: '["b"]' }); + }); + }); + + describe('identity-map isolation (program RULING, D-shape)', () => { + it('OAUTHCONSENTREPO-007: findScopes leaves no managed entity behind — a raw write to the same row on the same transaction connection survives the transaction\'s closing flush', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-7'); + testDb.prepare(`INSERT INTO oauth_consents (client_id, user_id, scopes) VALUES (?, ?, ?)`).run('proto-7', user.id, '["a"]'); + + await withRequestContext(t.orm, async () => { + await uow.transactional(async () => { + // Projection: the only read shape this repository has. + await consents.findScopes('proto-7', user.id); + // A write bypassing the ORM entirely, on the SAME single SQLite + // connection the open transaction holds (D3/D6) — the shape any + // nativeUpdate write from elsewhere in the same request takes. If + // findScopes had left a managed entity in the identity map (the + // B1 mechanism), the transaction's closing flush would try to + // write the READ snapshot back over this change. + testDb.prepare('UPDATE oauth_consents SET scopes = ? WHERE client_id = ? AND user_id = ?').run('["a","b"]', 'proto-7', user.id); + }); + }); + + const row = testDb.prepare('SELECT scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?').get('proto-7', user.id) as { scopes: string }; + expect(row.scopes).toBe('["a","b"]'); + }); + }); +}); diff --git a/server/tests/unit/db/repositories/OauthTokens.repository.test.ts b/server/tests/unit/db/repositories/OauthTokens.repository.test.ts new file mode 100644 index 000000000..48bea5281 --- /dev/null +++ b/server/tests/unit/db/repositories/OauthTokens.repository.test.ts @@ -0,0 +1,459 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser } from '../../../helpers/factories'; +import { OauthTokens } from '../../../../src/db/entities/OauthTokens.entity'; +import type { OauthTokensRepository } from '../../../../src/db/repositories/OauthTokens.repository'; +import { UnitOfWork } from '../../../../src/nest/database/unit-of-work'; +import { withRequestContext } from '../../../../src/nest/database/request-context'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let tokens: OauthTokensRepository; +let uow: UnitOfWork; + +beforeAll(async () => { + t = await createTestOrm(testDb); + tokens = t.repo(OauthTokens); + uow = new UnitOfWork(t.em); +}); +beforeEach(() => { + resetTestDb(testDb); + t.clear(); + testDb.exec('DELETE FROM oauth_tokens'); + testDb.exec('DELETE FROM oauth_consents'); + testDb.exec('DELETE FROM oauth_clients'); +}); +afterAll(async () => { await t.close(); testDb.close(); }); + +function seedClient(userId: number, clientId: string, name = 'Client'): void { + testDb.prepare( + `INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes) VALUES (?, ?, ?, ?, ?, '[]', '[]')`, + ).run(`row-${clientId}`, userId, name, clientId, 'hash'); +} + +function seedToken( + overrides: Partial<{ + id: number; clientId: string; userId: number; accessHash: string; refreshHash: string; scopes: string; + audience: string | null; accessExpiresAt: string; refreshExpiresAt: string; revokedAt: string | null; parentId: number | null; + }> = {}, +): number { + const info = testDb.prepare( + `INSERT INTO oauth_tokens (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, revoked_at, parent_token_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ).run( + overrides.clientId, + overrides.userId, + overrides.accessHash ?? `acc-${Math.random().toString(36).slice(2)}`, + overrides.refreshHash ?? `ref-${Math.random().toString(36).slice(2)}`, + overrides.scopes ?? '["trips:read"]', + overrides.audience ?? null, + overrides.accessExpiresAt ?? new Date(Date.now() + 3600_000).toISOString(), + overrides.refreshExpiresAt ?? new Date(Date.now() + 30 * 24 * 3600_000).toISOString(), + overrides.revokedAt ?? null, + overrides.parentId ?? null, + ); + const id = Number(info.lastInsertRowid); + if (overrides.id !== undefined) { + testDb.prepare('UPDATE oauth_tokens SET id = ? WHERE id = ?').run(overrides.id, id); + return overrides.id; + } + return id; +} + +describe('OauthTokensRepository', () => { + describe('insertToken (OA13/OA14)', () => { + it('OAUTHTOKREPO-001: writes exactly the given columns', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-1'); + const accessExpiry = new Date(Date.now() + 3600_000).toISOString(); + const refreshExpiry = new Date(Date.now() + 30 * 24 * 3600_000).toISOString(); + await tokens.insertToken({ + client_id: 'proto-1', + user_id: user.id, + access_token_hash: 'acchash', + refresh_token_hash: 'refhash', + scopes: '["trips:read"]', + audience: 'https://mcp.example.com', + access_token_expires_at: accessExpiry, + refresh_token_expires_at: refreshExpiry, + parent_token_id: null, + }); + const row = testDb.prepare('SELECT * FROM oauth_tokens WHERE access_token_hash = ?').get('acchash') as Record; + expect(row).toMatchObject({ + client_id: 'proto-1', + user_id: user.id, + access_token_hash: 'acchash', + refresh_token_hash: 'refhash', + scopes: '["trips:read"]', + audience: 'https://mcp.example.com', + revoked_at: null, + parent_token_id: null, + }); + }); + + it('OAUTHTOKREPO-002: parent_token_id is written when given (rotation)', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-2'); + const parentId = seedToken({ clientId: 'proto-2', userId: user.id }); + await tokens.insertToken({ + client_id: 'proto-2', user_id: user.id, access_token_hash: 'a2', refresh_token_hash: 'r2', scopes: '[]', audience: null, + access_token_expires_at: new Date().toISOString(), refresh_token_expires_at: new Date().toISOString(), parent_token_id: parentId, + }); + const row = testDb.prepare('SELECT parent_token_id FROM oauth_tokens WHERE access_token_hash = ?').get('a2') as { parent_token_id: number }; + expect(row.parent_token_id).toBe(parentId); + }); + }); + + describe('revokeAllForClient (OA8) — the two spellings of "now"', () => { + it("OAUTHTOKREPO-003: revokeAllForClient (legacy datetime('now')) and revokeById (legacy CURRENT_TIMESTAMP) store byte-identical text", async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-3'); + const idA = seedToken({ clientId: 'proto-3', userId: user.id }); + const idB = seedToken({ clientId: 'proto-3', userId: user.id }); + + await tokens.revokeAllForClient('proto-3'); // legacy: datetime('now') + await tokens.revokeById(idB); // legacy: CURRENT_TIMESTAMP + + const rowA = testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idA) as { revoked_at: string }; + const rowB = testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idB) as { revoked_at: string }; + expect(rowA.revoked_at).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/); + expect(rowB.revoked_at).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/); + }); + + it('OAUTHTOKREPO-004: revokeAllForClient only touches un-revoked rows of that client', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-4'); + seedClient(user.id, 'proto-4b'); + const idOther = seedToken({ clientId: 'proto-4b', userId: user.id }); + const idAlready = seedToken({ clientId: 'proto-4', userId: user.id, revokedAt: '2020-01-01 00:00:00' }); + const idLive = seedToken({ clientId: 'proto-4', userId: user.id }); + + await tokens.revokeAllForClient('proto-4'); + + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idOther) as { revoked_at: string | null }).revoked_at).toBeNull(); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idAlready) as { revoked_at: string }).revoked_at).toBe('2020-01-01 00:00:00'); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idLive) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + }); + }); + + describe('findByAccessTokenHashWithUser (OA16)', () => { + it('OAUTHTOKREPO-005: joins the token owner\'s username/email/role', async () => { + const { user } = createUser(testDb, { username: 'bearer-user', email: 'bearer@example.test' }); + seedClient(user.id, 'proto-5'); + seedToken({ clientId: 'proto-5', userId: user.id, accessHash: 'bearer-hash', scopes: '["trips:read"]', audience: 'aud' }); + + const row = await tokens.findByAccessTokenHashWithUser('bearer-hash'); + expect(row).toMatchObject({ + scopes: '["trips:read"]', audience: 'aud', revoked_at: null, user_id: user.id, client_id: 'proto-5', + username: 'bearer-user', email: 'bearer@example.test', role: 'user', + }); + }); + + it('OAUTHTOKREPO-006: null for an unknown hash', async () => { + expect(await tokens.findByAccessTokenHashWithUser('nope')).toBeNull(); + }); + }); + + describe('findParent (OA17)', () => { + it('OAUTHTOKREPO-007: returns id + parent_token_id', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-7'); + const rootId = seedToken({ clientId: 'proto-7', userId: user.id }); + const childId = seedToken({ clientId: 'proto-7', userId: user.id, parentId: rootId }); + expect(await tokens.findParent(childId)).toEqual({ id: childId, parent_token_id: rootId }); + expect(await tokens.findParent(rootId)).toEqual({ id: rootId, parent_token_id: null }); + }); + }); + + describe('collectChainIds (OA18) — recursive CTE, proven against the legacy raw statement', () => { + it('OAUTHTOKREPO-008: a 4-level chain is fully collected, a sibling branch is excluded — matches the legacy WITH RECURSIVE statement exactly', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-8'); + const id1 = seedToken({ clientId: 'proto-8', userId: user.id }); + const id2 = seedToken({ clientId: 'proto-8', userId: user.id, parentId: id1 }); + const id3 = seedToken({ clientId: 'proto-8', userId: user.id, parentId: id2 }); + const id4 = seedToken({ clientId: 'proto-8', userId: user.id, parentId: id3 }); + // A sibling branch off a DIFFERENT root — must not be collected. + const sibRoot = seedToken({ clientId: 'proto-8', userId: user.id }); + const sibChild = seedToken({ clientId: 'proto-8', userId: user.id, parentId: sibRoot }); + + const legacyRows = testDb.prepare(` + WITH RECURSIVE chain(id) AS ( + SELECT id FROM oauth_tokens WHERE id = ? + UNION ALL + SELECT t.id FROM oauth_tokens t JOIN chain c ON t.parent_token_id = c.id + ) + SELECT id FROM chain + `).all(id1) as Array<{ id: number }>; + const legacyIds = legacyRows.map((r) => r.id).sort((a, b) => a - b); + + const ids = (await tokens.collectChainIds(id1)).sort((a, b) => a - b); + + expect(ids).toEqual(legacyIds); + expect(ids).toEqual([id1, id2, id3, id4].sort((a, b) => a - b)); + expect(ids).not.toContain(sibRoot); + expect(ids).not.toContain(sibChild); + }); + + it('OAUTHTOKREPO-009: a token with no children collects only itself', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-9'); + const id = seedToken({ clientId: 'proto-9', userId: user.id }); + expect(await tokens.collectChainIds(id)).toEqual([id]); + }); + }); + + describe('revokeByIds (OA19) — the $in revoke', () => { + it('OAUTHTOKREPO-010: revokes exactly the given ids, only if still live', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-10'); + const idLive1 = seedToken({ clientId: 'proto-10', userId: user.id }); + const idLive2 = seedToken({ clientId: 'proto-10', userId: user.id }); + const idAlready = seedToken({ clientId: 'proto-10', userId: user.id, revokedAt: '2020-01-01 00:00:00' }); + const idUntouched = seedToken({ clientId: 'proto-10', userId: user.id }); + + await tokens.revokeByIds([idLive1, idLive2, idAlready]); + + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idLive1) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idLive2) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idAlready) as { revoked_at: string }).revoked_at).toBe('2020-01-01 00:00:00'); // untouched, not re-stamped + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idUntouched) as { revoked_at: string | null }).revoked_at).toBeNull(); + }); + + it('OAUTHTOKREPO-011: an empty array is a no-op (matches the legacy `if (ids.length > 0)` guard)', async () => { + await expect(tokens.revokeByIds([])).resolves.toBeUndefined(); + }); + }); + + describe('collectChainIds + revokeByIds inside a transaction — proves it joins the OPEN transaction', () => { + it('OAUTHTOKREPO-012: revoking a chain inside a uow.transactional that then ROLLS BACK revokes nothing', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-12'); + const rootId = seedToken({ clientId: 'proto-12', userId: user.id }); + const childId = seedToken({ clientId: 'proto-12', userId: user.id, parentId: rootId }); + + let caught: unknown; + try { + await withRequestContext(t.orm, async () => { + await uow.transactional(async () => { + const ids = await tokens.collectChainIds(rootId); + expect(ids.sort((a, b) => a - b)).toEqual([rootId, childId].sort((a, b) => a - b)); + await tokens.revokeByIds(ids); + throw new Error('force rollback'); + }); + }); + } catch (e) { caught = e; } + expect((caught as Error).message).toBe('force rollback'); + + const rows = testDb.prepare('SELECT id, revoked_at FROM oauth_tokens WHERE id IN (?, ?)').all(rootId, childId) as Array<{ id: number; revoked_at: string | null }>; + expect(rows.every((r) => r.revoked_at === null)).toBe(true); + }); + + it('OAUTHTOKREPO-013: the same sequence, committed, actually revokes the chain', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-13'); + const rootId = seedToken({ clientId: 'proto-13', userId: user.id }); + const childId = seedToken({ clientId: 'proto-13', userId: user.id, parentId: rootId }); + + await withRequestContext(t.orm, async () => { + await uow.transactional(async () => { + const ids = await tokens.collectChainIds(rootId); + await tokens.revokeByIds(ids); + }); + }); + + const rows = testDb.prepare('SELECT id, revoked_at FROM oauth_tokens WHERE id IN (?, ?)').all(rootId, childId) as Array<{ id: number; revoked_at: string | null }>; + expect(rows.every((r) => r.revoked_at !== null)).toBe(true); + }); + }); + + describe('findSuccessorAlive (OA20)', () => { + it('OAUTHTOKREPO-014: a revoked parent with a live child returns the child', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-14'); + const parentId = seedToken({ clientId: 'proto-14', userId: user.id, revokedAt: '2020-01-01 00:00:00' }); + const childId = seedToken({ clientId: 'proto-14', userId: user.id, parentId }); + expect(await tokens.findSuccessorAlive(parentId)).toEqual({ id: childId }); + }); + + it('OAUTHTOKREPO-015: no live child returns null', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-15'); + const parentId = seedToken({ clientId: 'proto-15', userId: user.id, revokedAt: '2020-01-01 00:00:00' }); + seedToken({ clientId: 'proto-15', userId: user.id, parentId, revokedAt: '2020-01-01 00:00:01' }); + expect(await tokens.findSuccessorAlive(parentId)).toBeNull(); + }); + }); + + describe('findByRefreshTokenHash (OA22)', () => { + it('OAUTHTOKREPO-016: returns every column the legacy statement selects, FK scalars read from the raw column value', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-16'); + const parentId = seedToken({ clientId: 'proto-16', userId: user.id }); + const id = seedToken({ clientId: 'proto-16', userId: user.id, refreshHash: 'refresh-16', parentId, audience: 'aud-16' }); + + const row = await tokens.findByRefreshTokenHash('refresh-16'); + expect(row).toEqual({ + id, client_id: 'proto-16', user_id: user.id, scopes: '["trips:read"]', audience: 'aud-16', + refresh_token_expires_at: expect.any(String), revoked_at: null, parent_token_id: parentId, + }); + }); + + it('OAUTHTOKREPO-017: null for an unknown hash', async () => { + expect(await tokens.findByRefreshTokenHash('nope')).toBeNull(); + }); + }); + + describe('revokeById (OA23/OA28/OA33)', () => { + it('OAUTHTOKREPO-018: revokes the given id only', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-18'); + const idA = seedToken({ clientId: 'proto-18', userId: user.id }); + const idB = seedToken({ clientId: 'proto-18', userId: user.id }); + await tokens.revokeById(idA); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idA) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idB) as { revoked_at: string | null }).revoked_at).toBeNull(); + }); + }); + + describe('findByAccessOrRefreshHashAndClient / revokeByAccessOrRefreshHashAndClient (OA24/OA25)', () => { + it('OAUTHTOKREPO-019: finds by access hash OR refresh hash, scoped to the client', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-19'); + seedToken({ clientId: 'proto-19', userId: user.id, accessHash: 'acc-19', refreshHash: 'ref-19' }); + expect(await tokens.findByAccessOrRefreshHashAndClient('acc-19', 'proto-19')).toEqual({ user_id: user.id }); + expect(await tokens.findByAccessOrRefreshHashAndClient('ref-19', 'proto-19')).toEqual({ user_id: user.id }); + }); + + it('OAUTHTOKREPO-020: does not match a token belonging to a different client', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-20a'); + seedClient(user.id, 'proto-20b'); + seedToken({ clientId: 'proto-20a', userId: user.id, accessHash: 'acc-20' }); + expect(await tokens.findByAccessOrRefreshHashAndClient('acc-20', 'proto-20b')).toBeNull(); + }); + + it('OAUTHTOKREPO-021: revokeByAccessOrRefreshHashAndClient revokes by either hash, scoped to the client', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-21'); + const id = seedToken({ clientId: 'proto-21', userId: user.id, accessHash: 'acc-21', refreshHash: 'ref-21' }); + await tokens.revokeByAccessOrRefreshHashAndClient('acc-21', 'proto-21'); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(id) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + }); + }); + + describe('listActiveByUser (OA26) — Kysely join on the real oauth_clients.client_id column', () => { + it('OAUTHTOKREPO-022: lists only this user\'s non-revoked, non-expired-refresh sessions, newest first, with the client name joined in', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + seedClient(user.id, 'proto-22a', 'Client A'); + seedClient(user.id, 'proto-22b', 'Client B'); + seedToken({ clientId: 'proto-22a', userId: user.id, scopes: '["trips:read"]' }); + testDb.exec("UPDATE oauth_tokens SET created_at = '2020-01-01 00:00:00' WHERE client_id = 'proto-22a'"); + seedToken({ clientId: 'proto-22b', userId: user.id }); + testDb.exec("UPDATE oauth_tokens SET created_at = '2021-01-01 00:00:00' WHERE client_id = 'proto-22b'"); + // revoked — excluded + seedToken({ clientId: 'proto-22a', userId: user.id, revokedAt: '2020-01-01 00:00:00' }); + // expired refresh — excluded + seedToken({ clientId: 'proto-22a', userId: user.id, refreshExpiresAt: '2000-01-01 00:00:00' }); + // another user's session — excluded + seedClient(other.id, 'proto-22c'); + seedToken({ clientId: 'proto-22c', userId: other.id }); + + const rows = await tokens.listActiveByUser(user.id); + expect(rows.map((r) => r.client_name)).toEqual(['Client B', 'Client A']); + expect(rows[0].scopes).toBeDefined(); + }); + }); + + describe('findOwnedById (OA27)', () => { + it('OAUTHTOKREPO-023: matches id+user', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-23'); + const id = seedToken({ clientId: 'proto-23', userId: user.id }); + expect(await tokens.findOwnedById(id, user.id)).toEqual({ id, client_id: 'proto-23' }); + }); + + it('OAUTHTOKREPO-024: 404-shape null for another user\'s session', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + seedClient(other.id, 'proto-24'); + const id = seedToken({ clientId: 'proto-24', userId: other.id }); + expect(await tokens.findOwnedById(id, user.id)).toBeNull(); + }); + }); + + describe('listAllActiveWithClientAndUser (OA31) — the admin panel', () => { + it('OAUTHTOKREPO-025: lists every active session across all users, with client name and username joined in', async () => { + const { user: a } = createUser(testDb, { username: 'admin-panel-a' }); + const { user: b } = createUser(testDb, { username: 'admin-panel-b' }); + seedClient(a.id, 'proto-25a', 'Client A'); + seedClient(b.id, 'proto-25b', 'Client B'); + seedToken({ clientId: 'proto-25a', userId: a.id }); + seedToken({ clientId: 'proto-25b', userId: b.id, revokedAt: '2020-01-01 00:00:00' }); // excluded + + const rows = await tokens.listAllActiveWithClientAndUser(); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ client_name: 'Client A', username: 'admin-panel-a', user_id: a.id }); + }); + }); + + describe('findById (OA32)', () => { + it('OAUTHTOKREPO-026: unscoped by owner — for the admin 404 check', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-26'); + const id = seedToken({ clientId: 'proto-26', userId: user.id }); + expect(await tokens.findById(id)).toEqual({ id, user_id: user.id, client_id: 'proto-26' }); + }); + + it('OAUTHTOKREPO-027: null for an unknown id', async () => { + expect(await tokens.findById(999999)).toBeNull(); + }); + }); + + describe('revokeAllForUser (AU18/AU46 — built now for Task 5)', () => { + it('OAUTHTOKREPO-028: revokes every live session for the user, across every client, leaves other users untouched', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + seedClient(user.id, 'proto-28a'); + seedClient(user.id, 'proto-28b'); + seedClient(other.id, 'proto-28c'); + const idA = seedToken({ clientId: 'proto-28a', userId: user.id }); + const idB = seedToken({ clientId: 'proto-28b', userId: user.id }); + const idAlready = seedToken({ clientId: 'proto-28a', userId: user.id, revokedAt: '2020-01-01 00:00:00' }); + const idOther = seedToken({ clientId: 'proto-28c', userId: other.id }); + + await tokens.revokeAllForUser(user.id); + + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idA) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idB) as { revoked_at: string | null }).revoked_at).not.toBeNull(); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idAlready) as { revoked_at: string }).revoked_at).toBe('2020-01-01 00:00:00'); + expect((testDb.prepare('SELECT revoked_at FROM oauth_tokens WHERE id = ?').get(idOther) as { revoked_at: string | null }).revoked_at).toBeNull(); + }); + }); + + describe('identity-map write-back regression (program RULING, D-shape)', () => { + it('OAUTHTOKREPO-029: findByRefreshTokenHash (projection A) then findParent (projection B) then revokeById inside uow.transactional — the write survives', async () => { + const { user } = createUser(testDb); + seedClient(user.id, 'proto-29'); + const id = seedToken({ clientId: 'proto-29', userId: user.id, refreshHash: 'refresh-29', scopes: '["kept-scope"]' }); + + await withRequestContext(t.orm, async () => { + // Projection A — a wide fields set (id, client, user, scopes, audience, refresh_token_expires_at, revoked_at, parentToken). + await tokens.findByRefreshTokenHash('refresh-29'); + // Projection B — a narrow, DIFFERENT field set on the same row. + await tokens.findParent(id); + // The intended write, inside the request's own transaction. + await uow.transactional(async () => { + await tokens.revokeById(id); + }); + }); + + const row = testDb.prepare('SELECT revoked_at, scopes FROM oauth_tokens WHERE id = ?').get(id) as { revoked_at: string | null; scopes: string }; + expect(row.revoked_at).not.toBeNull(); // the nativeUpdate must stick + expect(row.scopes).toBe('["kept-scope"]'); // untouched by either read + }); + }); +}); diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index 4bd4dfce4..f784fc096 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -59,7 +59,6 @@ function makePkce() { } import { OauthService } from '../../../src/nest/oauth/oauth.service'; -import { DatabaseService } from '../../../src/nest/database/database.service'; import { AuditService } from '../../../src/nest/audit/audit.service'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; import { getMcpSafeUrl } from '../../../src/app-config'; @@ -70,14 +69,22 @@ import { AuditLog } from '../../../src/db/entities/AuditLog.entity'; import type { AuditLogRepository } from '../../../src/db/repositories/AuditLog.repository'; import { Users } from '../../../src/db/entities/Users.entity'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; +import { OauthClients } from '../../../src/db/entities/OauthClients.entity'; +import type { OauthClientsRepository } from '../../../src/db/repositories/OauthClients.repository'; +import { OauthTokens } from '../../../src/db/entities/OauthTokens.entity'; +import type { OauthTokensRepository } from '../../../src/db/repositories/OauthTokens.repository'; +import { OauthConsents } from '../../../src/db/entities/OauthConsents.entity'; +import type { OauthConsentsRepository } from '../../../src/db/repositories/OauthConsents.repository'; -const dbs = new DatabaseService(testDb); // Stubbed rather than real: every case drives the MCP gate through this one // flag, exactly as the addons.bridge mock did before the fold. const addonsStub = { isAddonEnabled } as unknown as AddonsService; let svc: OauthService; let auditLogRepo: AuditLogRepository; let usersRepo: UsersRepository; +let clientsRepo: OauthClientsRepository; +let tokensRepo: OauthTokensRepository; +let consentsRepo: OauthConsentsRepository; let t: TestOrm; // Legacy free-function names delegating to the service, so the moved cases below @@ -109,7 +116,10 @@ beforeAll(async () => { t = await createTestOrm(testDb); auditLogRepo = t.repo(AuditLog); usersRepo = t.repo(Users); - svc = new OauthService(dbs, addonsStub, new AuditService(auditLogRepo, usersRepo)); + clientsRepo = t.repo(OauthClients); + tokensRepo = t.repo(OauthTokens); + consentsRepo = t.repo(OauthConsents); + svc = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo)); }); beforeEach(() => { @@ -1298,7 +1308,7 @@ describe('module-scoped OAuth state', () => { codeChallengeMethod: 'S256', })!; - const secondInstance = new OauthService(dbs, addonsStub, new AuditService(auditLogRepo, usersRepo)); + const secondInstance = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo)); expect(secondInstance.consumeAuthCode(code)?.userId).toBe(42); }); });