Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/import-protocol-implementor-typed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@objectstack/plugin-auth": patch
---

fix(plugin-auth): let `ImportProtocolLike` type the admin import protocol's members (#17422)

`admin-import-users.ts` is the only hand-written in-repo implementor of the runner's `ImportProtocolLike`, and it annotated all three required members `args: any`. An explicit parameter annotation wins over the contextual type, so #16952's newly declared request dialect held every implementor except this one — the one with a demonstrated history: before #16950 this file read `args?.query?.$filter ?? {}`, the runner moved to the canonical spelling, the read went `undefined`, and the `?? {}` default degraded the import's duplicate probe into match-everything, so `POST /api/v1/auth/admin/import-users` updated the wrong users without a sound.

The three annotations are deleted, so `findData` / `createData` / `updateData` are typed by the contract they implement. Measured: with the annotations gone, reading a retired wire alias (`args.query?.$filter`) is `TS2339 Property '$filter' does not exist on type 'QueryInput'`; with `args: any` restored the identical probe type-checks at exit 0.

`FindDataRequest` declares `query` optional, so `findData` now states its refusal in code — a thrown `Error` carrying the already-registered `INVALID_REQUEST` code — instead of relying on an incidental `TypeError` from a property read on `undefined`. No `??` fallback and no optional chaining were added: both spell match-everything, which is the defect this closes.

No API, request body, response shape or exported signature changes. A caller that reaches `findData` through `runImport` always supplies `query`, so no supported call moves; only a protocol call that was already failing now fails with a code attached.
36 changes: 36 additions & 0 deletions packages/plugins/plugin-auth/src/admin-import-users.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect, vi } from 'vitest';
import { readFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { assertEngineUpdateDispatch } from '@objectstack/objectql';
import { runAdminImportUsers, IMPORT_USERS_MAX_ROWS, type IdentityImportDeps } from './admin-import-users.js';
import type { AdminActor } from './admin-user-endpoints.js';

const ACTOR: AdminActor = { id: 'admin-1', email: 'admin@example.com' };

const HERE = dirname(fileURLToPath(import.meta.url));
const IMPORT_USERS_SOURCE = readFileSync(resolve(HERE, 'admin-import-users.ts'), 'utf8');

function makeRequest(body: unknown): Request {
return new Request('http://localhost/api/v1/auth/admin/import-users', {
method: 'POST',
Expand Down Expand Up @@ -624,3 +630,33 @@ describe('runAdminImportUsers — CSV payloads', () => {
expect(m.createUser.mock.calls.map((c) => c[0].body.email).sort()).toEqual(['c1@x.co', 'c2@x.co']);
});
});

/**
* [#17422] The IMPLEMENTOR half of #16952's contract.
*
* `ImportProtocolLike` types the three required members, but an EXPLICIT
* parameter annotation wins over a contextual type — so `findData(args: any)`
* opts this file back out of the contract while `tsc --noEmit` stays green.
* Measured on #17422 in this package: with the annotation restored, a probe
* reading the retired wire alias (`args.query?.$filter` — the pre-#16950 read
* whose `?? {}` default degraded the duplicate probe into match-everything)
* type-checks at exit 0; with the annotation gone the same probe is
* `TS2339 Property '$filter' does not exist on type 'QueryInput'`.
*
* ⇒ Nothing else in the repo can see that difference. The behavioural upsert
* tests above discriminate the CONSEQUENCE (ablated to the historical read,
* two of them go red) but not the opt-out itself: re-annotating the parameter
* leaves every one of them green and every gate green. This is that guard.
*/
describe('[#17422] the import protocol literal is typed BY `ImportProtocolLike`', () => {
it('binds the literal to the exported contract', () => {
expect(IMPORT_USERS_SOURCE).toContain('const protocol: ImportProtocolLike = {');
});

for (const member of ['findData', 'createData', 'updateData'] as const) {
it(`leaves \`${member}\`'s parameter unannotated, so the contract types it`, () => {
expect(IMPORT_USERS_SOURCE).toContain(`async ${member}(args) {`);
expect(IMPORT_USERS_SOURCE).not.toMatch(new RegExp(`async\\s+${member}\\s*\\(\\s*args\\s*:`));
});
}
});
29 changes: 23 additions & 6 deletions packages/plugins/plugin-auth/src/admin-import-users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,34 @@ export async function runAdminImportUsers(
// to `{}` stops constraining anything, so the duplicate probe matches rows
// it was given no key for and the upsert updates the WRONG user. One
// dialect, read straight — a request that arrives without a `query` is a
// caller defect and costs a loud TypeError, not a silent match-everything.
async findData(args: any) {
const where = args.query.where;
const limit = args.query.limit;
// caller defect and is refused loudly, not softened into match-everything.
//
// [#17422] The parameter is deliberately UNANNOTATED: `ImportProtocolLike`
// types it, and an explicit annotation here would win over that contextual
// type and opt this implementor back out of the contract (the runner's own
// docblock says so). `FindDataRequest` declares `query` OPTIONAL, so the
// contract makes this file write its refusal down instead of leaving it as
// an incidental TypeError from a property read on `undefined`.
// ⛔ Not `args.query ?? {}` and ⛔ not `args.query?.where`: both spell
// match-everything, which is the exact regression this protocol's history
// is about.
async findData(args) {
const query = args.query;
if (!query) {
throw Object.assign(
new Error('import-users: findData was called without a query — refusing to match every user'),
{ code: 'INVALID_REQUEST' },
);
}
const where = query.where;
const limit = query.limit;
return engine.find(args.object, { where, limit, context: SYSTEM_CTX } as any);
},

// One better-auth create per row — hashing + credential sys_account.
// Deliberately NO createManyData: there is no safe bulk primitive for
// identities, and scrypt dominates the cost anyway.
async createData(args: any) {
async createData(args) {
const data: Record<string, any> = args?.data ?? {};
const email: string = typeof data.email === 'string' && data.email.length > 0
? data.email
Expand Down Expand Up @@ -431,7 +448,7 @@ export async function runAdminImportUsers(

// Upsert updates touch PROFILE fields only — never email, never anything
// credential- or system-managed. An empty filtered patch is a no-op.
async updateData(args: any) {
async updateData(args) {
const patch: Record<string, any> = {};
for (const [k, v] of Object.entries(args?.data ?? {})) {
if (UPDATE_ALLOWED_FIELDS.has(k) && v !== undefined && v !== null && v !== '') patch[k] = v;
Expand Down
Loading