|
18 | 18 | * 2. the guard KEEPS refusing on an engine that refuses a principal-less, |
19 | 19 | * non-system context — its reads are not one, so the catch below never |
20 | 20 | * turns the refusal into a skipped guard; |
21 | | - * 3. the catches around the two reads are unchanged: a read that throws ends |
22 | | - * the hook without refusing, as it did before. That is pre-existing |
23 | | - * behaviour, pinned as it stands, not endorsed here. |
| 21 | + * 3. the guard FAILS CLOSED when a read it makes cannot answer (#21941): a |
| 22 | + * read that throws is refused with better-auth's `SERVICE_UNAVAILABLE` |
| 23 | + * (503) — never the guard's own `FORBIDDEN` (403), and never "ends the hook |
| 24 | + * without refusing" — while an engine that does not register |
| 25 | + * `sys_environment` (the open-source composition) declares the guard |
| 26 | + * inapplicable without reading anything. The real ObjectQL registry |
| 27 | + * answers that question in the last section. |
24 | 28 | */ |
25 | 29 |
|
26 | 30 | import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; |
| 31 | +import { ObjectQL, assertEngineFindOnePredicate } from '@objectstack/objectql'; |
27 | 32 | import { AuthManager } from './auth-manager'; |
| 33 | +import { authIdentityObjects } from './manifest'; |
28 | 34 |
|
29 | 35 | vi.mock('better-auth', () => ({ |
30 | 36 | betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })), |
@@ -172,27 +178,149 @@ describe('organization slug guard — on an engine that refuses a principal-less |
172 | 178 | }); |
173 | 179 | }); |
174 | 180 |
|
175 | | -describe('organization slug guard — the catches around the reads are unchanged (pre-existing)', () => { |
176 | | - it('an organization read that throws ends the hook without refusing, as before', async () => { |
| 181 | +/** |
| 182 | + * A read that could not answer, asserted on its envelope: better-auth's |
| 183 | + * `SERVICE_UNAVAILABLE` / 503, naming the object whose read failed — and |
| 184 | + * explicitly NOT the guard's `FORBIDDEN` / 403, which is a verdict about the |
| 185 | + * slug change that the guard never reached. |
| 186 | + */ |
| 187 | +async function expectReadFaultRefusal(attempt: Promise<unknown>, object: string) { |
| 188 | + const err = (await attempt.then( |
| 189 | + () => undefined, |
| 190 | + (e: unknown) => e, |
| 191 | + )) as { status?: unknown; statusCode?: unknown; body?: { message?: unknown }; cause?: unknown } | undefined; |
| 192 | + expect(err, 'the slug change was let through on a read that could not answer').toBeTruthy(); |
| 193 | + expect(err?.statusCode).toBe(503); |
| 194 | + expect(err?.status).toBe('SERVICE_UNAVAILABLE'); |
| 195 | + expect(String(err?.body?.message)).toContain(`\`${object}\``); |
| 196 | + return err; |
| 197 | +} |
| 198 | + |
| 199 | +/** The registration answer an ObjectQL engine gives through `getSchema`. */ |
| 200 | +const schemaFor = (...registered: string[]) => vi.fn((object: string) => (registered.includes(object) ? { name: object } : undefined)); |
| 201 | + |
| 202 | +/** |
| 203 | + * An engine double that answers the registration question (`getSchema`) beside |
| 204 | + * its two reads. Its `findOne` keeps ObjectQL's own predicate contract |
| 205 | + * (`assertEngineFindOnePredicate`), so it is no looser than the engine it |
| 206 | + * stands in for. |
| 207 | + */ |
| 208 | +function registeringEngine( |
| 209 | + registered: string[], |
| 210 | + answers: { findOne: () => Promise<unknown>; find: () => Promise<unknown> }, |
| 211 | +) { |
| 212 | + return { |
| 213 | + getSchema: schemaFor(...registered), |
| 214 | + findOne: vi.fn(async (object: string, q?: Query) => { |
| 215 | + assertEngineFindOnePredicate(object, q as never); |
| 216 | + return answers.findOne(); |
| 217 | + }), |
| 218 | + find: vi.fn(answers.find), |
| 219 | + }; |
| 220 | +} |
| 221 | + |
| 222 | +describe('organization slug guard — fails closed when a read cannot answer (#21941)', () => { |
| 223 | + it('an organization read that throws is refused (503), and the environment read is never made', async () => { |
| 224 | + // SUPERSEDED PIN, quoted — what the guard answered before: |
| 225 | + // await expect(update('acme-new')).resolves.toBeUndefined(); // ends the hook without refusing |
177 | 226 | const engine = { |
178 | 227 | findOne: vi.fn(async () => { |
179 | 228 | throw new Error('store unavailable'); |
180 | 229 | }), |
181 | 230 | find: vi.fn(async () => ENVS), |
182 | 231 | }; |
183 | 232 | const update = await slugGuard(engine); |
184 | | - await expect(update('acme-new')).resolves.toBeUndefined(); |
| 233 | + const err = await expectReadFaultRefusal(update('acme-new'), 'sys_organization'); |
| 234 | + expect((err?.cause as Error | undefined)?.message).toBe('store unavailable'); |
185 | 235 | expect(engine.find).not.toHaveBeenCalled(); |
186 | 236 | }); |
187 | 237 |
|
188 | | - it('an environment read that throws ends the hook without refusing, as before', async () => { |
| 238 | + it('an environment read that throws is refused (503) — on an engine that registers the object', async () => { |
| 239 | + // SUPERSEDED PIN, quoted — `find` threw and the hook resolved `undefined`. |
| 240 | + const engine = registeringEngine(['sys_organization', 'sys_environment'], { |
| 241 | + findOne: async () => ORG, |
| 242 | + find: async () => { |
| 243 | + throw new Error('connection reset'); |
| 244 | + }, |
| 245 | + }); |
| 246 | + const update = await slugGuard(engine); |
| 247 | + await expectReadFaultRefusal(update('acme-new'), 'sys_environment'); |
| 248 | + expect(engine.find).toHaveBeenCalledTimes(1); |
| 249 | + }); |
| 250 | + |
| 251 | + it('an environment read that throws on an engine whose registry cannot be asked is refused too — it is asked, and the fault refuses', async () => { |
189 | 252 | const engine = { |
190 | 253 | findOne: vi.fn(async () => ORG), |
191 | 254 | find: vi.fn(async () => { |
192 | 255 | throw new Error('object sys_environment is not registered'); |
193 | 256 | }), |
194 | 257 | }; |
195 | 258 | const update = await slugGuard(engine); |
| 259 | + await expectReadFaultRefusal(update('acme-new'), 'sys_environment'); |
| 260 | + }); |
| 261 | + |
| 262 | + it('a healthy read on a registered object still refuses the change while an active environment references the org, and allows it otherwise', async () => { |
| 263 | + const refusing = registeringEngine(['sys_organization', 'sys_environment'], { |
| 264 | + findOne: async () => ORG, |
| 265 | + find: async () => ENVS, |
| 266 | + }); |
| 267 | + await expectSlugRefusal((await slugGuard(refusing))('acme-new')); |
| 268 | + const allowing = registeringEngine(['sys_organization', 'sys_environment'], { |
| 269 | + findOne: async () => ORG, |
| 270 | + find: async () => [{ id: 'e2', status: 'archived' }], |
| 271 | + }); |
| 272 | + await expect((await slugGuard(allowing))('acme-new')).resolves.toBeUndefined(); |
| 273 | + }); |
| 274 | + |
| 275 | + it('an engine that does not register `sys_environment` declares the guard inapplicable — nothing is read', async () => { |
| 276 | + const engine = registeringEngine(['sys_organization'], { |
| 277 | + findOne: async () => { |
| 278 | + throw new Error('must not be read'); |
| 279 | + }, |
| 280 | + find: async () => { |
| 281 | + throw new Error('must not be read'); |
| 282 | + }, |
| 283 | + }); |
| 284 | + const update = await slugGuard(engine); |
| 285 | + await expect(update('acme-new')).resolves.toBeUndefined(); |
| 286 | + expect(engine.getSchema).toHaveBeenCalledWith('sys_environment'); |
| 287 | + expect(engine.findOne).not.toHaveBeenCalled(); |
| 288 | + expect(engine.find).not.toHaveBeenCalled(); |
| 289 | + }); |
| 290 | +}); |
| 291 | + |
| 292 | +describe('organization slug guard — the real ObjectQL registry answers the composition question', () => { |
| 293 | + /** A real engine holding exactly the identity objects this package registers — no `sys_environment`. */ |
| 294 | + function engineWithAuthObjects(): ObjectQL { |
| 295 | + const engine = new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {}, child() { return this; } } } as never); |
| 296 | + for (const object of authIdentityObjects) { |
| 297 | + engine.registry.registerObject(object as never, '@objectstack/plugin-auth'); |
| 298 | + } |
| 299 | + return engine; |
| 300 | + } |
| 301 | + |
| 302 | + it('this package registers no `sys_environment`, so on its own object set the guard does not apply and reads nothing', async () => { |
| 303 | + const engine = engineWithAuthObjects(); |
| 304 | + expect(engine.getSchema('sys_organization'), 'POSITIVE CONTROL: the registry does answer for an object this package registers').toBeTruthy(); |
| 305 | + expect(engine.getSchema('sys_environment')).toBeUndefined(); |
| 306 | + const findOne = vi.spyOn(engine, 'findOne'); |
| 307 | + const find = vi.spyOn(engine, 'find'); |
| 308 | + const update = await slugGuard(engine); |
196 | 309 | await expect(update('acme-new')).resolves.toBeUndefined(); |
| 310 | + expect(findOne).not.toHaveBeenCalled(); |
| 311 | + expect(find).not.toHaveBeenCalled(); |
| 312 | + }); |
| 313 | + |
| 314 | + it('once `sys_environment` is registered the guard reads, and a real engine fault on that read refuses (503)', async () => { |
| 315 | + // No driver is registered, so the engine itself cannot serve the read: a |
| 316 | + // genuine fault of the real engine, not a double's. |
| 317 | + const engine = engineWithAuthObjects(); |
| 318 | + engine.registry.registerObject( |
| 319 | + { name: 'sys_environment', label: 'Environment', fields: { organization_id: { name: 'organization_id', type: 'text' } } } as never, |
| 320 | + '@objectstack/test-cloud-objects', |
| 321 | + ); |
| 322 | + expect(engine.getSchema('sys_environment')).toBeTruthy(); |
| 323 | + const update = await slugGuard(engine); |
| 324 | + await expectReadFaultRefusal(update('acme-new'), 'sys_organization'); |
197 | 325 | }); |
198 | 326 | }); |
0 commit comments