Skip to content

Commit c4da0e6

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-24579-v18-sweep-round15
2 parents 9f29264 + 98c3d00 commit c4da0e6

9 files changed

Lines changed: 838 additions & 33 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth): an administrator's impersonation session is not held by the target's forced password reset or password expiry (#24762)
6+
7+
Clause-②: no
8+
9+
Impersonating a user who must change their password used to strand the administrator: the impersonation session was refused `403 PASSWORD_RESET_REQUIRED` on every protected route, and the console showed the target's change-password screen. An impersonation session is minted from the administrator's own session, which has passed its own posture check, so it is not the user's password sign-in, and a `credential`-scope posture holds the password sign-in alone (`AUTH_GATE_SCOPES`).
10+
11+
- **What moves.** On a session that carries `impersonatedBy`, the session gate (`user.authGate`, stamped in `customSession`) now shows only the first `account`-scope posture the user holds. `PASSWORD_RESET_REQUIRED` and `PASSWORD_EXPIRED` no longer hold it. `MFA_REQUIRED` still does, so an impersonation session of a user who owes MFA enrolment is still refused `403 MFA_REQUIRED`, including when the user also owes a password change.
12+
- **Unchanged.** The user's own sign-in stays held until they change the password. The forced reset's revocation (`revokeSessionsAndKeys`, on by default) still ends every session of the user, impersonation sessions included. API keys, the print door's refusal of an impersonation session, and the impersonation door's own check of the administrator's posture are unchanged.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): a platform operator's tenant-scoped insert from a session with no active organization is refused 403 naming the missing organization, as a member's is, instead of a 500
6+
7+
Clause-②: no (no accept set moves: the insert was refused before and is refused now, with the ADR-0123 D2 answer in place of an internal error)
8+
9+
- **Before.** Under `group` or `isolated`, a platform operator (`admin_full_access`) whose session had no active organization got `500 ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` for a create on a tenant object whose posture lets it cross the organization wall (`access.default: 'private'`). The operator crosses the wall there, so the wall let the insert through, and the engine then refused the row as a system write with no organization.
10+
- **After.** That create gets `403 PERMISSION_DENIED` with the message a member gets in the same state. The message names the missing active organization and tells the user to join or select one.
11+
- **Unchanged.** Nothing was written before and nothing is written now. The refusal judges the row as it is about to be stored, the same row the engine's own organization check reads, so it covers exactly the inserts that answered 500 and no insert that was admitted. These are as before:
12+
- a row a `beforeInsert` hook places in an organization;
13+
- the operator's reads and updates across organizations;
14+
- inserts from an operator with an active organization;
15+
- inserts into objects that opted out of tenancy;
16+
- system contexts (which still meet the engine's own refusal);
17+
- every `single` answer.

‎content/docs/permissions/authentication.mdx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,12 @@ logged. While `mustChangePassword` is set, the user is blocked with
981981
`403 PASSWORD_RESET_REQUIRED` on every protected route until they complete a
982982
password change. It is the same gate as password expiry, under its own code,
983983
and like expiry it holds password sign-in only: the user's API keys are not
984-
held by it.
984+
held by it. Nor is an administrator's impersonation session, which is minted
985+
from the administrator's own session and is not the user's password sign-in:
986+
an administrator can impersonate the user to verify their access before the
987+
user has changed the password, while the user's own sign-in stays blocked.
988+
Enforced MFA (`MFA_REQUIRED`) holds the account, so it still holds an
989+
impersonation session of that user.
985990

986991
#### Set / Reset a Password
987992

‎packages/plugins/plugin-auth/src/auth-manager.test.ts‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3665,7 +3665,7 @@ describe('AuthManager', () => {
36653665
findOne: vi.fn(),
36663666
}, { permissions: [{ name: ADMIN_FULL_ACCESS, ...ADMIN_FULL_ACCESS_CAPABILITIES }] });
36673667

3668-
const getSessionCallback = async (dataEngine: any) => {
3668+
const getSessionCallback = async (dataEngine: any, extra: Record<string, unknown> = {}) => {
36693669
let capturedConfig: any;
36703670
(betterAuth as any).mockImplementation((config: any) => {
36713671
capturedConfig = config;
@@ -3677,6 +3677,7 @@ describe('AuthManager', () => {
36773677
secret: 'test-secret-at-least-32-chars-long',
36783678
baseUrl: 'http://localhost:3000',
36793679
dataEngine,
3680+
...extra,
36803681
});
36813682
await manager.getAuthInstance();
36823683
warnSpy.mockRestore();
@@ -3772,6 +3773,56 @@ describe('AuthManager', () => {
37723773
expect(result.user).toBe(user);
37733774
expect(result.user.positions).toBeUndefined();
37743775
});
3776+
3777+
// [#24762, ruling 6288774214 items 1–3] What `user.authGate` the producer
3778+
// stamps on an impersonation session (`session.impersonatedBy` set) of a
3779+
// held user: only an `account`-scope posture (`AUTH_GATE_SCOPES`) holds
3780+
// it, the first one the user's list reports. The user's own session keeps
3781+
// the list's first entry. The same cases on a real boot, through the
3782+
// transport seams' 403 envelope, are `@objectstack/verify`'s
3783+
// `held-session-impersonation-target.pin.test.ts`.
3784+
describe('the posture stamped on an impersonation session (ADR-0069, AUTH_GATE_SCOPES)', () => {
3785+
const LAPSED = () => new Date(Date.now() - 30 * 86_400_000).toISOString();
3786+
const IMPERSONATION = { impersonatedBy: 'u-admin' };
3787+
// Every posture computable at once: enforced MFA with no grace, and a
3788+
// one-day expiry. Each row below decides which postures its user holds.
3789+
const POLICY = { mfaRequired: true, mfaGracePeriodDays: 0, passwordExpiryDays: 1 };
3790+
const sessionGateFor = async (row: Record<string, unknown>, session: Record<string, unknown>) => {
3791+
const engine = makeDataEngine({ platformAdmin: false });
3792+
// The block's double, its `findOne` answering the posture read of the user row.
3793+
engine.findOne = vi.fn(async (object: string, q?: any) => {
3794+
assertEngineFindOnePredicate(object, q);
3795+
return object === 'sys_user' && q?.where?.id === 'u-1' ? { id: 'u-1', ...row } : null;
3796+
}) as any;
3797+
const callback = await getSessionCallback(engine, POLICY);
3798+
const result = await callback({ user: { id: 'u-1', email: 'a@b.com' }, session });
3799+
return result.user.authGate?.code;
3800+
};
3801+
const RESET = { must_change_password: true, two_factor_enabled: true, password_changed_at: new Date().toISOString() };
3802+
const EXPIRED = { must_change_password: false, two_factor_enabled: true, password_changed_at: LAPSED() };
3803+
const MFA = { must_change_password: false, two_factor_enabled: false, mfa_required_at: LAPSED(), password_changed_at: new Date().toISOString() };
3804+
const RESET_AND_MFA = { ...MFA, must_change_password: true };
3805+
3806+
it('a forced-reset user: their own session is held, an impersonation session of them is not', async () => {
3807+
expect(await sessionGateFor(RESET, {})).toBe('PASSWORD_RESET_REQUIRED');
3808+
expect(await sessionGateFor(RESET, IMPERSONATION)).toBeUndefined();
3809+
});
3810+
3811+
it('an expired-password user: the same, the other credential-scope posture', async () => {
3812+
expect(await sessionGateFor(EXPIRED, {})).toBe('PASSWORD_EXPIRED');
3813+
expect(await sessionGateFor(EXPIRED, IMPERSONATION)).toBeUndefined();
3814+
});
3815+
3816+
it('an MFA-held user: the account-scope posture holds the impersonation session too', async () => {
3817+
expect(await sessionGateFor(MFA, {})).toBe('MFA_REQUIRED');
3818+
expect(await sessionGateFor(MFA, IMPERSONATION)).toBe('MFA_REQUIRED');
3819+
});
3820+
3821+
it('a user held by both: their own session shows the reset, the impersonation session MFA_REQUIRED', async () => {
3822+
expect(await sessionGateFor(RESET_AND_MFA, {})).toBe('PASSWORD_RESET_REQUIRED');
3823+
expect(await sessionGateFor(RESET_AND_MFA, IMPERSONATION)).toBe('MFA_REQUIRED');
3824+
});
3825+
});
37753826
});
37763827

37773828
// ADR-0069 D1: breached-password rejection enables better-auth's native

‎packages/plugins/plugin-auth/src/auth-manager.ts‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import {
4848
API_KEY_DEFAULT_EXPIRY_DAYS,
4949
API_KEY_INACTIVITY_DAYS,
5050
API_KEY_MAX_EXPIRY_DAYS,
51+
accountScopeAuthGate,
5152
hasPlatformAdminStanding,
5253
resolveUserAuthzGrants,
5354
} from '@objectstack/core';
@@ -4593,10 +4594,16 @@ export class AuthManager {
45934594
// revokes the session in place when exceeded (next request → 401).
45944595
await this.enforceSessionControls((session as any)?.id, (session as any)?.createdAt);
45954596

4597+
// An impersonation session carries the administrator who started it
4598+
// (`impersonatedBy`, written by the admin plugin's impersonation and
4599+
// stored as `sys_session.impersonated_by`); which postures hold the
4600+
// session depends on it — see computeAuthGate().
4601+
const impersonatedBy = (session as any)?.impersonatedBy;
45964602
const authGate = await this.computeAuthGate(
45974603
user.id,
45984604
(session as any)?.activeOrganizationId,
45994605
(user as any)?.twoFactorEnabled === true,
4606+
{ impersonation: typeof impersonatedBy === 'string' && impersonatedBy.trim() !== '' },
46004607
);
46014608

46024609
return {
@@ -8897,15 +8904,36 @@ export class AuthManager {
88978904
/**
88988905
* ADR-0069 — the auth-policy gate posture for a SESSION: the first, most
88998906
* urgent posture {@link resolveUserAuthGates} reports for its user in its
8900-
* active organization, else undefined. Every scope holds a session, because
8901-
* a session IS the password sign-in (#23287, ruling C′ item 1).
8907+
* active organization that holds this session, else undefined.
8908+
*
8909+
* What a posture holds is its declared scope (`AUTH_GATE_SCOPES`, #23287,
8910+
* ruling C′ item 1): `account` holds every credential of the user,
8911+
* `credential` holds the password sign-in alone. Which postures hold a
8912+
* session therefore depends on what the session is:
8913+
*
8914+
* - a session the user signed in to themselves is held by every scope: it
8915+
* IS the password sign-in a `credential` posture holds;
8916+
* - an IMPERSONATION session (`options.impersonation`, read off
8917+
* `session.impersonatedBy`) is not the user's password sign-in. It is
8918+
* minted from the administrator's own session, which passed its own
8919+
* posture check at the impersonation door. So only an `account`-scope
8920+
* posture holds it: the first one the list reports, wherever it sits
8921+
* (`accountScopeAuthGate`, the reading the API-key admission seam gives
8922+
* a key, the other credential that is not the password sign-in). A target
8923+
* held by both the forced reset and enforced MFA answers `MFA_REQUIRED`
8924+
* on it, never nothing (#24762, ruling 6288774214 items 1 and 2).
8925+
*
8926+
* The other callers ask for a user's own session (`options` omitted), so
8927+
* their answer is the list's first entry, as it always was.
89028928
*/
89038929
private async computeAuthGate(
89048930
userId: string,
89058931
activeOrgId: string | undefined,
89068932
_twoFactorEnabledHint: boolean,
8933+
options: { impersonation?: boolean } = {},
89078934
): Promise<AuthGate | undefined> {
89088935
const gates = await this.resolveUserAuthGates(userId, { organizationId: activeOrgId });
8936+
if (options.impersonation === true) return accountScopeAuthGate(gates) ?? undefined;
89098937
return gates[0];
89108938
}
89118939

@@ -8914,7 +8942,8 @@ export class AuthManager {
89148942
* `userId` holds right now, most urgent first; `[]` when no policy holds
89158943
* them. The ONE per-user posture computation (`IAuthService.resolveUserAuthGates`):
89168944
*
8917-
* - the session gate takes its first entry ({@link computeAuthGate}, onto
8945+
* - the session gate takes its first entry, and on an impersonation session
8946+
* its first `account`-scope entry ({@link computeAuthGate}, onto
89188947
* `user.authGate` in `customSession`);
89198948
* - the API-key admission seam (`resolveApiKeyAdmission`, `@objectstack/core`)
89208949
* asks it for a key's owner and holds the key on an `account`-scope entry
@@ -8925,10 +8954,11 @@ export class AuthManager {
89258954
* organization may require MFA above the global floor.
89268955
*
89278956
* Order, most urgent first: the administrator's forced reset, then natural
8928-
* password expiry, then enforced MFA. The session shows the first; a key is
8929-
* held only by an `account`-scope entry, wherever it sits in the list — a
8930-
* user whose password expired AND who owes MFA enrolment holds their keys
8931-
* on the MFA entry even though their session shows the expiry.
8957+
* password expiry, then enforced MFA. The user's own session shows the
8958+
* first; a key, and an impersonation session, are held only by an
8959+
* `account`-scope entry, wherever it sits in the list — a user whose
8960+
* password expired AND who owes MFA enrolment holds their keys on the MFA
8961+
* entry even though their session shows the expiry.
89328962
*
89338963
* No-op (no read) when no gate feature is enabled. Fails OPEN on any lookup
89348964
* error, answering `[]` — a transient hiccup must never lock a compliant user

0 commit comments

Comments
 (0)