Skip to content

fix: single identity owner, hardened recovery/throttling, firestore parity, release gate - #38

Open
tnramalho wants to merge 6 commits into
agent/rockets-auth-hardeningfrom
feat/auth-identity-ownership
Open

fix: single identity owner, hardened recovery/throttling, firestore parity, release gate#38
tnramalho wants to merge 6 commits into
agent/rockets-auth-hardeningfrom
feat/auth-identity-ownership

Conversation

@tnramalho

Copy link
Copy Markdown
Collaborator

Summary

Closes the open review findings from the four-PR stack (#34/#36/#37/#35) as one reviewable slice on top of agent/rockets-auth-hardening. Every fix is the "best-for-the-case" option agreed during triage, not the cheapest patch. Three cold-review rounds (fresh agents, no context) ran against the diff; their findings are folded in.

What changed, by area

Auth composition — single identity owner + enforced boundary

  • AuthBootstrap splits into identity (the persistence the whole adapter chain authenticates into — at most one owner per app) and contributes (guard preferences that may coexist). Two owners, or ownership sliced across integrations, now throw at composition naming the adapters.
  • Contributed identity.resources are folded into the app's resources[]: the same reference is skipped (idempotent), the same entity with a different config throws naming both sides — instead of the opaque registered twice boot failure a migrating app used to hit.
  • The server strips identity/contributes before handing bootstraps to core; RocketsCoreModule now rejects a bootstrap that still carries them, so core never silently drops integration-owned defaults.
  • Guard invariant (from the base branch) retained: a contributed enableGlobalGuard: false is honored only with a declared replacement (providesAppGuard) or an explicit app opt-out — otherwise it fails loudly.

Auth recovery + throttling

  • Password/login recovery respond immediately and run notification work as a logged floating promise. Uniform response timing removes the account-enumeration side channel; a slow mailer no longer blocks the request; previously-swallowed errors are logged with context.
  • The app-wide APP_GUARD throttler is replaced by a guard scoped to the auth-owned public routes (signup, login, recovery, otp, invitation acceptance) — the library no longer rate-limits the host app's own routes.
  • Throttling runs on two dimensions at once: a coarse per-IP ceiling (1000/min, the prior default — account rotation cannot exceed it) plus fine per-(ip, account) limits, so an attacker only ever throttles themselves and cannot lock a victim out of login. throttling: false disables both.

Firestore repository parity

  • firestoreValuesEqual: NaN matches NaN (the server compiles == NaN to the IS_NAN unary filter) and +0 === -0 — verified against @google-cloud/firestore.
  • compareStrings compares by code points (UTF-8 byte order), matching server sort for astral characters.
  • getAll is chunked (300/batch): id lists over 500 work again instead of throwing.
  • Duplicate-id create() throws one typed FirestoreDuplicateIdException from the backend contract (both backends), extending RepositoryQueryException so the permeator maps it to 409, not a generic 500.

Release tooling

  • verify-package-artifacts.mjs validates every main/types/exports/bin/files target against the npm pack --dry-run tarball (plus a source-path guard, since npm auto-includes main even from src/).
  • firebase-tools leaves root devDependencies (~398 packages); the emulator runs via pinned npx in one place, and SKIP_EMULATOR skips it locally (never in CI).
  • release-readiness.yml keeps only the gates ci-pr-test does not already run.
  • Stale rule docs fixed (deleted swagger.json, vitest .ts → .mts).

Reviewer notes / decisions worth confirming

  1. throttling: false and the per-IP ceiling — a consumer who passes a custom throttling array without a getTracker on their own ip entry loses the volume ceiling (their config choice). Only the built-in default is protected.
  2. Recovery is fire-and-forget — in-flight notification work is lost on a process crash; this is the deliberate timing-attack mitigation, and why the flow e2e now polls.
  3. release-readiness on push to main now runs only the gates, not unit/e2e (those ran at PR time via ci-pr-test).
  4. Authenticated CRUD routes (admin roles, user metadata) are no longer blanket-throttled — they sit behind auth; only the public auth surface is rate-limited.

Verification

  • yarn build, yarn typecheck:spec, yarn test (642), yarn test:e2e (169), yarn lint — all green.
  • The intermittent 2-suite e2e flake (password used too recently / rotating victim) is the documented host memory-pressure pattern (CHANGELOG.md), confirmed by a clean 169/169 re-run — no assertions weakened.
  • Three cold-review rounds; every confirmed finding fixed (fail-open coverage on signup/invitation, per-IP ceiling, an inverted NaN premise, stale docs, under-asserted test).

🤖 Generated with Claude Code

tnramalho and others added 6 commits August 10, 2026 15:32
userMetadata, repository, and resources are not mergeable defaults —
they are a singular ownership claim over the one user space the whole
adapter chain authenticates into. Move them from the per-integration
contributes bag to AuthBootstrap.identity, and enforce a single identity
owner per app at composition time (two owners, or ownership sliced
across integrations, now throw naming the adapters involved).

contributes keeps only the guard preferences (enableGlobalGuard,
providesAppGuard); the guard-swap invariant is unchanged.
defineRocketsAuth input API is untouched — only its returned bootstrap
shape moves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…emantics

- firestoreValuesEqual: NaN matches NaN (server compiles == NaN to the IS_NAN
  unary filter) and +0 equals -0, instead of Object.is on both.
- compareStrings compares by code points (UTF-8 byte order), matching server
  sort for astral characters.
- getAll is chunked (300/batch), so id lists over 500 work again instead of
  throwing; the borrowed batch-write cap is removed.
- create() rejects duplicate ids through one typed FirestoreDuplicateIdException
  in the backend contract (both backends), extending RepositoryQueryException so
  the permeator maps it to 409 instead of a generic 500.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Password/login recovery respond immediately and run notification work as a
  logged floating promise: uniform response timing removes the account-
  enumeration side channel, and a slow mailer no longer blocks the request.
  Swallowed recovery errors now log the error object with context.
- Replace the app-wide APP_GUARD throttler with a guard scoped to the auth-
  owned public routes (signup, login, recovery, otp, invitation acceptance),
  so the library no longer rate-limits the host app's own routes.
- Throttle on two dimensions at once: a coarse per-IP ceiling (1000/min, the
  prior default — account rotation cannot exceed it) plus fine per-(ip,account)
  limits, so an attacker only throttles themselves and cannot lock a victim out
  of login. throttling: false disables both.
- Add a boot test asserting the upstream string DI tokens still resolve.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- resolveRocketsComposition folds an integration's identity.resources into the
  app resources: the same reference is skipped (idempotent), the same entity
  defined twice with different config throws naming both sides, instead of a
  later opaque 'registered twice' boot failure.
- The server strips identity/contributes before handing bootstraps to core, and
  RocketsCoreModule now rejects a bootstrap that still carries them — core never
  silently drops integration-owned defaults.
- Document the boundary on the core README auth-option row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- verify-package-artifacts.mjs checks every main/types/exports/bin/files target
  against the npm pack --dry-run tarball file list (plus a source-path guard, as
  npm auto-includes main even from src/), so a manifest pointing at unpublished
  source fails the gate.
- firebase-tools drops from root devDependencies; the emulator runs via pinned
  npx in one place, and locally SKIP_EMULATOR skips it (never in CI).
- release-readiness workflow keeps only the publish-consumption gates that
  ci-pr-test does not already run.
- Fix stale rule docs: editing-guidelines (deleted swagger.json) and
  build-test-lint (vitest .ts -> .mts).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…wnership

* agent/rockets-auth-hardening:
  fix: match Firestore scalar range semantics
  feat: define server composition APIs (#34)

Conflicts:
	packages/rockets-core/src/domain/interfaces/auth-bootstrap.interface.ts
	packages/rockets-server/src/rockets.module-definition.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant