Skip to content

Commit 3092798

Browse files
committed
refactor(core,objectql): ask main's one ban predicate instead of a second copy
#30287 landed isUserBanInForce and USER_BAN_FIELDS in security/user-ban.ts while this branch carried its own; the hand-over and the reference check now read that one, so a ban means one thing at every door. Claude-Session: https://claude.ai/code/session_01Utp6CZavYYLh5k5r6jhgCF Co-authored-by: Claude <noreply@anthropic.com>
1 parent 31ad53b commit 3092798

4 files changed

Lines changed: 4 additions & 48 deletions

File tree

‎packages/core/src/security/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,6 @@ export {
544544
USER_ASSIGNED_WORK_FIELD_CAP,
545545
USER_WORK_TRANSFER_ROW_CAP,
546546
USER_WORK_FAILURE_SAMPLE,
547-
isUserBanInForce,
548547
userAssignedWorkFields,
549548
countUserAssignedWork,
550549
transferUserAssignedWork,

‎packages/core/src/security/user-assigned-work.test.ts‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,36 +4,12 @@ import { describe, it, expect } from 'vitest';
44
import { assertEngineUpdateDispatch } from '@objectstack/metadata-core';
55
import {
66
countUserAssignedWork,
7-
isUserBanInForce,
87
transferUserAssignedWork,
98
userAssignedWorkFields,
109
UserWorkTransferRefusal,
1110
type UserWorkEngine,
1211
} from './user-assigned-work.js';
1312

14-
const NOW = Date.parse('2026-11-05T12:00:00Z');
15-
16-
describe('[#30205] isUserBanInForce', () => {
17-
it('a ban with no expiry, or one still ahead, is in force', () => {
18-
expect(isUserBanInForce({ banned: true }, NOW)).toBe(true);
19-
expect(isUserBanInForce({ banned: true, ban_expires: null }, NOW)).toBe(true);
20-
expect(isUserBanInForce({ banned: true, ban_expires: '2026-11-06T00:00:00Z' }, NOW)).toBe(true);
21-
expect(isUserBanInForce({ banned: true, ban_expires: new Date(NOW + 1000) }, NOW)).toBe(true);
22-
});
23-
24-
it('an expired ban, an unbanned user and a non-row are not', () => {
25-
expect(isUserBanInForce({ banned: true, ban_expires: '2026-11-04T00:00:00Z' }, NOW)).toBe(false);
26-
expect(isUserBanInForce({ banned: false }, NOW)).toBe(false);
27-
expect(isUserBanInForce({ banned: null }, NOW)).toBe(false);
28-
expect(isUserBanInForce({}, NOW)).toBe(false);
29-
expect(isUserBanInForce(null, NOW)).toBe(false);
30-
});
31-
32-
it('an expiry that does not parse cannot prove the ban lifted', () => {
33-
expect(isUserBanInForce({ banned: true, ban_expires: 'not a date' }, NOW)).toBe(true);
34-
});
35-
});
36-
3713
const ticket = {
3814
name: 'ticket',
3915
fields: {

‎packages/core/src/security/user-assigned-work.ts‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
*/
5252

5353
import { referenceTargetOf, unprovisionedInjectedColumns } from '@objectstack/spec/data';
54+
import { isUserBanInForce, USER_BAN_FIELDS } from './user-ban.js';
5455

5556
/** The platform user object every user reference targets. */
5657
const USER_OBJECT = 'sys_user';
@@ -64,26 +65,6 @@ export const USER_WORK_TRANSFER_ROW_CAP = 500;
6465
/** At most this many failed rows are described per (object, field) pair. */
6566
export const USER_WORK_FAILURE_SAMPLE = 10;
6667

67-
/**
68-
* Is this user row's ban in force?
69-
*
70-
* `banned` is `true` and the ban has no expiry, or one still in the future.
71-
* better-auth leaves `banned` true past `ban_expires` until the user's next
72-
* sign-in clears it, and such a user CAN sign in again, so an expired ban is
73-
* not in force. A row that is not an object, or has no `banned: true`, is not
74-
* banned.
75-
*/
76-
export function isUserBanInForce(row: unknown, now: number = Date.now()): boolean {
77-
if (!row || typeof row !== 'object') return false;
78-
const { banned, ban_expires: expires } = row as { banned?: unknown; ban_expires?: unknown };
79-
if (banned !== true) return false;
80-
if (expires === null || expires === undefined || expires === '') return true;
81-
const at = expires instanceof Date ? expires.getTime() : new Date(expires as string | number).getTime();
82-
// An expiry that does not parse cannot prove the ban lifted.
83-
if (Number.isNaN(at)) return true;
84-
return at > now;
85-
}
86-
8768
/** One user reference a record can hold work through. */
8869
export interface UserAssignedWorkField {
8970
object: string;
@@ -283,7 +264,7 @@ export async function transferUserAssignedWork(
283264
const elevated = { ...((context as object | undefined) ?? {}), isSystem: true };
284265
const [target] = await engine.find(USER_OBJECT, {
285266
where: { id: toUserId },
286-
fields: ['id', 'banned', 'ban_expires'],
267+
fields: [...USER_BAN_FIELDS],
287268
limit: 1,
288269
context: elevated,
289270
});

‎packages/objectql/src/engine-referential-integrity.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252

5353
import { referenceTargetOf, type EngineQueryOptions } from '@objectstack/spec/data';
5454
import type { ExecutionContext } from '@objectstack/spec/kernel';
55-
import { isUserBanInForce, keepsFlowWriteReferenceCheck } from '@objectstack/core';
55+
import { isUserBanInForce, keepsFlowWriteReferenceCheck, USER_BAN_FIELDS } from '@objectstack/core';
5656
import { ValidationError, buildFieldError } from './validation/record-validator.js';
5757
import type { ObjectQL } from './engine.js';
5858

@@ -328,7 +328,7 @@ async function bannedUserIds(
328328
try {
329329
const rows = await this.find(BAN_CHECKED_USER_OBJECT, {
330330
where: { id: asked.length === 1 ? asked[0] : { $in: asked } },
331-
fields: ['id', 'banned', 'ban_expires'],
331+
fields: [...USER_BAN_FIELDS],
332332
context: { ...(context ?? {}), isSystem: true } as ExecutionContext,
333333
} as EngineQueryOptions) as Array<Record<string, unknown>>;
334334
const banned = new Set<string>();

0 commit comments

Comments
 (0)