feat: cross-org delegation entity models and AuthInfo extensions

[ekremney]

Summary

Implements sections 2 and 3 of the cross-org delegation (Option 2a) design: entity models in spacecat-shared-data-access and AuthInfo extensions in spacecat-shared-http-utils.

Context: Cross-org delegation allows an agency/delegate IMS org to access sites belonging to another org, scoped per product code. This PR adds the data-access layer and auth utilities that downstream services (auth-service for token enrichment, api-service for access control) will consume.

spacecat-shared-data-access — New entities

SiteImsOrgAccess

Maps a delegate org to a site it can access, per product. Core delegation grant record.

• Schema (site-ims-org-access.schema.js):
  • belongs_to Site + belongs_to Organization (delegate org receiving access)
  • targetOrganizationId as addAttribute with UUID validation (not a second belongs_to Organization — avoids SchemaBuilder naming collision; inline comment explains)
  • productCode enum from Entitlement.PRODUCT_CODES (LLMO, ASO, ACO)
  • role enum: collaborator | agency | viewer with default agency (app-layer validation, not DB enum — easier to evolve without migrations)
  • grantedBy optional, validated via regex: ims:<userId> | slack:<slackUserId> | system
  • expiresAt optional ISO date
  • allIndex on organizationId for login-time enrichment queries
• Model (site-ims-org-access.model.js): DELEGATION_ROLES constant
• Collection (site-ims-org-access.collection.js):
  • Idempotent create: check-then-insert on (siteId, organizationId, productCode) via findByIndexKeys — returns existing record if match found (SiteEnrollment pattern)
  • 50-delegate-per-site limit: queries allBySiteId(), throws 409 if count >= 50
• Site schema: added has_many SiteImsOrgAccesses with removeDependents: true (app-layer cascade alongside SQL ON DELETE CASCADE)

AccessGrantLog

Immutable audit log for grant/revoke actions. Preserves forensic trail even after entity deletion.

• Schema (access-grant-log.schema.js):
  • .allowUpdates(false).allowRemove(false) — immutable at app layer
  • siteId and organizationId as TEXT addAttribute (not FK belongs_to) — UUIDs must survive entity deletion. FK + ON DELETE SET NULL would null the value, defeating the forensic purpose. App layer guarantees valid UUIDs at insert time.
  • action enum: grant | revoke (from AccessGrantLog.GRANT_ACTIONS)
  • performedBy required, validated via regex: ims:<userId> | slack:<slackUserId> | system
  • allIndex on organizationId
• Model (access-grant-log.model.js): GRANT_ACTIONS constant
• Collection (access-grant-log.collection.js): minimal, no custom methods needed

Registration & exports

• Both entities registered in EntityRegistry (entity.registry.js)
• Barrel exports in src/models/index.js
• TypeScript definitions in index.d.ts files, exported via src/models/index.d.ts

spacecat-shared-http-utils — AuthInfo extensions

Three new methods on AuthInfo (src/auth/auth-info.js) for cross-org JWT enrichment and access-check:

Method	Purpose
getDelegatedTenant(imsOrgId, productCode?)	Find a delegated tenant entry matching org + optional product. Strips @AdobeOrg suffix. Used by api-service hasAccess() for JWT gate check.
getDelegatedTenants()	Get all delegated tenant entries from JWT. Used by api-service to enumerate delegations.
getTenantIds()	Get IDs of all primary tenants. Convenience for deduplication (skip delegations targeting primary orgs).

hasOrganization() unchanged — continues checking primary tenants only.

Files changed (28 files, +1476 lines)

New source files (13):

• packages/spacecat-shared-data-access/src/models/site-ims-org-access/ — model, collection, schema, index.js, index.d.ts
• packages/spacecat-shared-data-access/src/models/access-grant-log/ — model, collection, schema, index.js, index.d.ts

Modified source files (5):

• entity.registry.js — register new entities
• src/models/index.js + index.d.ts — barrel exports
• src/models/site/site.schema.js — has_many SiteImsOrgAccesses
• src/auth/auth-info.js — 3 new methods

Test files (10):

• Unit tests: model, collection, schema for both entities (6 files) — all code paths covered
• IT tests: test/it/site-ims-org-access/ and test/it/access-grant-log/ (2 files)
• AuthInfo test additions (1 file)
• Fixtures: 2 fixture files + index update + seed priority update

Design decisions

Decision	Rationale
targetOrganizationId via addAttribute not belongs_to	SchemaBuilder uses model name as FK column prefix — second belongs_to Organization would collide with the delegate org's organization_id
role as app-layer enum, not DB enum	Evolving enum values without SQL migration; role enforcement deferred to Phase 2
grantedBy/performedBy as TEXT with prefix convention	Slack bot has no IMS user ID — slack:<slackUserId> captures actor identity across all surfaces
AccessGrantLog TEXT columns (not FK)	FK + ON DELETE SET NULL nulls the UUID on entity deletion, defeating forensic audit. TEXT preserves values permanently
Idempotent create + 50-delegate limit	Follows SiteEnrollment pattern; limit prevents unbounded delegation fan-out
removeDependents: true on Site→SiteImsOrgAccesses	App-layer cascade — grants cleaned up when site is removed

Related PRs

• Design doc: spacecat-auth-service#496 (merged)
• SQL migrations: mysticat-data-service#171
• Next: auth-service token enrichment (section 4), api-service access control (section 5)

Test plan

• spacecat-shared-data-access unit tests: 1638 passing, 100% line/statement coverage
• spacecat-shared-http-utils unit tests: 166 passing, 100% line/statement coverage
• IT tests written for both entities (CRUD, idempotent create, immutability, relationship queries)
• CI pipeline passes

🤖 Generated with Claude Code

[solaris007]

Hey @ekremney,

Strengths

Strong implementation overall. Both entities follow the BaseModel/BaseCollection/Schema pattern exactly, consistent with existing entities like SiteEnrollment, and will be immediately legible to anyone familiar with the codebase.

• access-grant-log.schema.js:17-18: .allowUpdates(false).allowRemove(false) at the schema level makes immutability explicit, not just conventional. The TEXT-column choice for siteId/organizationId correctly preserves forensic value after entity deletion - and the inline comments explain the rationale clearly.
• site-ims-org-access.collection.js:29-37: Idempotent create via findByIndexKeys mirrors the SiteEnrollment pattern.
• auth-info.js:104-131: Clean stateless accessors. getDelegatedTenant() correctly strips @AdobeOrg consistent with hasOrganization().
• access-grant-log.schema.js:14: The PERFORMED_BY_PATTERN regex (ims:|slack:|system) makes actor attribution parseable and extensible without future migrations.
• site-ims-org-access.schema.js:63-67: The addAttribute workaround for the SchemaBuilder naming collision is pragmatic and well-documented.
• Test coverage is thorough at both unit and integration levels - schema validation, idempotency, immutability rejection, and AuthInfo edge cases are all exercised.

Issues

Critical (Must Fix)

1. getDelegatedTenant() crashes on null/undefined input - packages/spacecat-shared-http-utils/src/auth/auth-info.js (getDelegatedTenant)

imsOrgId.split('@') throws TypeError: Cannot read properties of null (reading 'split') when imsOrgId is null or undefined. This method will be called with user-derived data in auth middleware - a malformed token or missing claim is a realistic production input.

Fix:

getDelegatedTenant(imsOrgId, productCode) {
  if (!imsOrgId) return undefined;
  const [id] = String(imsOrgId).split('@');
  ...
}

Also add tests for null, undefined, and empty string inputs.

Important (Should Fix)

2. Optional productCode in getDelegatedTenant() is a scope-widening risk - packages/spacecat-shared-http-utils/src/auth/auth-info.js (getDelegatedTenant)

When productCode is omitted, the method returns the first delegated tenant matching the org ID regardless of product scope:

return delegated.find((dt) => dt.id === id && (!productCode || dt.productCode === productCode));

Delegation grants are scoped per product - that is the core of the cross-org model. A caller omitting productCode could match an LLMO delegation when checking ASO access. The method name gives no signal that omitting productCode widens the match. Consider making productCode required, or adding a separate hasDelegationForOrg(imsOrgId) method with a name that makes the broader scope explicit.

3. Cascade delete silently removes grants without audit log entries - packages/spacecat-shared-data-access/src/models/site/site.schema.js (removeDependents)

removeDependents: true silently removes all SiteImsOrgAccess records when a Site is deleted. No revoke AccessGrantLog entries are created for these implicit revocations. Someone auditing "why did agency X lose access to site Y" will find nothing in the log - the exact forensic gap that the TEXT-not-FK design was built to avoid. Either add a model-level pre-remove hook that writes audit entries, or document this as a known Phase 1 gap with a tracking issue.

4. Missing targetOrganizationId in AccessGrantLog - packages/spacecat-shared-data-access/src/models/access-grant-log/access-grant-log.schema.js

The audit log records who requested access (organizationId, the delegate) and which site, but not the site-owning org (targetOrganizationId). After site deletion, the log cannot answer "which customer org did agency X have access to?" - exactly the forensic question a compliance auditor would ask. Add targetOrganizationId as a TEXT attribute with UUID validation, consistent with the existing siteId/organizationId approach.

5. Expired grants count toward the 50-delegate limit - packages/spacecat-shared-data-access/src/models/site-ims-org-access/site-ims-org-access.collection.js (limit check)

allBySiteId returns all grants including expired ones. A site that rotates agencies will accumulate expired grants that count toward the cap but provide no access. Over time the limit is exhausted by dead entries. Fix: filter expired grants in the limit check (existingGrants.filter(g => !g.getExpiresAt() || new Date(g.getExpiresAt()) > new Date())), or add a scheduled cleanup job, or at minimum document the behavior with a tracking issue.

6. TOCTOU race on idempotent create and 50-delegate limit - packages/spacecat-shared-data-access/src/models/site-ims-org-access/site-ims-org-access.collection.js (create method)

The findByIndexKeys check + allBySiteId count + super.create sequence is not atomic. Two concurrent requests can both pass the idempotency check (creating duplicates) or both pass the 50-limit check (exceeding it). For an authorization feature, the delegate cap is a trust boundary. At minimum, add a comment acknowledging the race window. Ideally, add a DB-level unique constraint on (siteId, organizationId, productCode) so duplicate inserts fail at the storage layer rather than producing silent duplicates.

7. No observability for delegation lifecycle events - packages/spacecat-shared-data-access/src/models/site-ims-org-access/site-ims-org-access.collection.js

The collection has zero log statements. For a security-relevant feature, you need application log entries for: new grant creation, idempotent return of existing grant, and limit rejection. The AccessGrantLog entity captures grant/revoke actions only if callers remember to write one - the collection itself should not be completely silent.

8. targetOrganizationId (and other identity fields) should be read-only on SiteImsOrgAccess - packages/spacecat-shared-data-access/src/models/site-ims-org-access/site-ims-org-access.schema.js and index.d.ts

setTargetOrganizationId() is exposed via the TypeScript interface (index.d.ts:296). The target org is part of the grant's identity - changing it is a different grant, not an update. Same applies to siteId, organizationId, and productCode. Add readOnly: true to these attribute definitions and remove the setters from the TypeScript interface.

9. Missing indexes: targetOrganizationId on SiteImsOrgAccess, siteId on AccessGrantLog

• SiteImsOrgAccess has no index on targetOrganizationId. The common admin query "show all delegations granted by org Z across all their sites" requires a full table scan.
• AccessGrantLog only indexes by organizationId. Querying "show me the audit trail for site X" - the most natural forensics query - also requires a full table scan. For an append-only log that never shrinks, this becomes expensive. Add siteId to an index, either as a composite sort key on the existing all-index or as a separate custom index.

Adding indexes to a new empty table is free; adding them after data accumulates requires a migration.

10. Plain Error instead of DataAccessError for the 50-delegate limit - packages/spacecat-shared-data-access/src/models/site-ims-org-access/site-ims-org-access.collection.js

The limit check throws new Error(...) with a manually set .status = 409. Callers catching DataAccessError will miss this. As a shared library, consumers depend on consistent typed errors.

11. role is unvalidated in AccessGrantLog - packages/spacecat-shared-data-access/src/models/access-grant-log/access-grant-log.schema.js:39-42

SiteImsOrgAccess constrains role to the DELEGATION_ROLES enum; AccessGrantLog accepts any string. A typo at write time produces a misleading audit entry. New entries should be validated against the current enum; historical entries with old values remain valid.

12. Test coverage gaps:

• No IT test for the 50-delegate limit in site-ims-org-access.test.js (limit check uses real queries that unit mocks cannot verify).
• No IT test for site deletion cascading to SiteImsOrgAccess records (removeDependents: true).
• The 409 error test in site-ims-org-access.collection.test.js checks the message but not err.status === 409 - if downstream code branches on that property, it needs asserting.
• Immutability tests in access-grant-log.test.js assert only that the promise rejects, not the specific error type or message.

Minor (Nice to Have)

• auth-info.js (getTenantIds): If a tenant object lacks an id, undefined is silently included in the returned array. Consider .filter((t) => t.id).map((t) => t.id).
• auth-info.js (getDelegatedTenant): Returns a mutable reference from the JWT profile. A caller that modifies the return value will mutate in-memory state for the rest of the request. Consider returning a shallow copy.
• access-grant-log.schema.test.js: The most important properties of this schema (allowUpdates, allowRemove) are verified only via IT tests, not asserted at the unit schema test level.
• site-ims-org-access.collection.js: allBySiteId fetches and deserializes up to 50 full objects just to count them. Fine with the current cap, but a COUNT query would be more efficient.
• access-grant-log/index.d.ts: findByOrganizationId returns a single record from an append-only log - "first match" semantics are surprising for a log. If this is "does any log exist for this org?", a name like existsByOrganizationId would be clearer.
• organizationId in SiteImsOrgAccess represents the delegate org, while targetOrganizationId is the site-owning org. The generic name will trip up new readers. belongs_to dictates the FK column name so a rename is not straightforward, but a JSDoc comment or model-level alias would help.

Recommendations

1. Add a DB-level unique constraint on (siteId, organizationId, productCode) for SiteImsOrgAccess. The application-layer idempotency check is best-effort; a database constraint makes it authoritative.
2. Define an expiration cleanup strategy before this ships. Currently expiresAt is stored but never enforced in this layer. If enforcement lives in api-service's hasAccess(), document it explicitly so future service authors cannot miss it.
3. Document the deployment dependency: the mysticat-data-service SQL migration (PR #171) must be deployed and verified before any consuming service bumps to this shared library version.
4. Consider embedding AccessGrantLog writes in the collection (Phase 2): if SiteImsOrgAccessCollection.create() and remove() automatically wrote audit entries, audit completeness would be guaranteed by the data layer, not by caller discipline.

Assessment

Ready to merge? No.

Address the getDelegatedTenant null guard before any service deploys against this. The remaining important items - cascade audit gap, missing targetOrganizationId in the log, expired-grant limit exhaustion, missing indexes - are best addressed before the tables are populated in production, since adding them retroactively requires migrations.

The architecture is sound: the two-entity split (live grant + immutable audit log), TEXT columns for forensic durability, and the idempotency pattern are all the right calls. The issues are hardening gaps, not design rethinks.

Next Steps

1. Address the critical item: getDelegatedTenant null guard.
2. Address the 11 important items listed above.
3. Minor items are optional but worth tracking.

[github-actions]

This PR will trigger a minor release when merged.

[ekremney]

Hey @solaris007, thanks for the thorough review — really useful feedback. Addressed everything in 36d5f65. Here's a rundown:

Critical

1. getDelegatedTenant null guard — added if (!imsOrgId) return undefined + String() coercion. Also returns a shallow copy now to prevent callers from mutating in-memory state (minor item). Tests added for null, undefined, empty string, and the copy behaviour.

Important

2. Optional productCode scope-widening — documented explicitly in JSDoc rather than making it required, since there are legitimate callers that intentionally want any-product matching. The comment makes the broad-match behaviour visible.

3. Cascade delete audit gap — added a TODO(Phase 2 audit gap) comment on the has_many SiteImsOrgAccesses reference explaining exactly what's missing and why. Agreed this should be a pre-launch hook; tracked as a follow-up.

4. Missing targetOrganizationId in AccessGrantLog — added as a required TEXT/UUID attribute in the schema, TypeScript interface (getTargetOrganizationId()), and fixture data. The SQL migration already had the column.

5. Expired grants counting toward limit — limit check now filters to activeGrants (no expiresAt or future expiresAt). Added a unit test covering the boundary: 49 active + 1 expired does not hit the cap.

6. TOCTOU race — added an explicit comment on the non-atomic sequence. The DB UNIQUE constraint on (site_id, organization_id, product_code) (already in the migration) is the authoritative guard; the app-layer check is a fast-path only.

7. Observability — log.info on idempotent-return and successful create; log.warn on limit rejection.

8. Identity fields read-only — added readOnly: true to targetOrganizationId and productCode in the SiteImsOrgAccess schema (prevents setter generation). Removed setTargetOrganizationId and setProductCode from the TypeScript interface. siteId and organizationId are already guarded by the belongs_to FK mechanism.

9. Missing indexes — added allByTargetOrganizationId index on SiteImsOrgAccess and allBySiteId index on AccessGrantLog. Both indexes already exist in the SQL migration; the JS schema just needed to declare them.

10. DataAccessError instead of plain Error — collection now throws new DataAccessError(message) with .status = 409 set for backward-compat with callers that branch on that property.

11. role unvalidated in AccessGrantLog — changed from type: 'string' to type: Object.values(SiteImsOrgAccess.DELEGATION_ROLES).

12. Test coverage gaps:

• 409 test now asserts instanceof DataAccessError and err.status === 409
• Added expired-grant exclusion unit test
• IT: immutability tests now assert DataAccessError type; removed the setRole() call that would throw synchronously (setters aren't generated when allowsUpdates is false)
• IT: added 50-delegate limit test using a MAX_DELEGATES_PER_SITE override with try/finally cleanup
• IT: added allBySiteId test for AccessGrantLog
• Schema unit tests: added allowsUpdates()/allowsRemove() assertions and targetOrganizationId attribute tests

Minor

• getTenantIds — filters out tenant objects without an id
• getDelegatedTenant — shallow copy (see above)
• allowUpdates/allowRemove schema assertions — added to unit test level

One item I intentionally left for a follow-up: the IT test for site-deletion cascade (removeDependents). It requires creating a transient Site record mid-test, which is feasible but adds meaningful setup complexity; the TODO comment in site.schema.js tracks it.

[solaris007]

Hey @ekremney,

Previously Flagged Issues - Now Addressed

All critical and important findings from the prior review have been resolved or explicitly deferred:

• Critical #1 (getDelegatedTenant null crash): Fixed. Null guard + String() coercion + tests for null, undefined, empty string. Solid belt-and-suspenders implementation.
• Important #2 (productCode scope-widening): Mitigated via JSDoc documenting the broad-match behavior when productCode is omitted. Acceptable.
• Important #3 (cascade delete audit gap): Deferred with actionable TODO(Phase 2 audit gap) comment in site.schema.js. Fair deferral - pre-remove hook infrastructure doesn't exist yet.
• Important #4 (targetOrganizationId in AccessGrantLog): Fixed. Added as required TEXT/UUID attribute with index.
• Important #5 (expired grants toward 50 limit): Fixed. Limit check now filters to active grants. Unit and IT tests cover the boundary case.
• Important #6 (TOCTOU race): Acknowledged with documentation. DB-level unique constraint in the migration is the authoritative guard.
• Important #7 (no observability): Fixed. Three log statements: info on idempotent hit, warn on limit reached, info on new grant created.
• Important #8 (identity fields read-only): Fixed. targetOrganizationId and productCode have readOnly: true. Setters removed from TypeScript interface.
• Important #9 (missing indexes): Fixed. targetOrganizationId on SiteImsOrgAccess, siteId on AccessGrantLog. TypeScript definitions updated.
• Important #10 (plain Error): Fixed. Now DataAccessError with .status = 409. Tests verify both type and status.
• Important #11 (role unvalidated in AccessGrantLog): Fixed. Uses Object.values(SiteImsOrgAccess.DELEGATION_ROLES).
• Important #12 (test gaps): Mostly fixed - IT test for 50-limit, 409 status assertions, immutability error type assertions, schema constraint tests all added. Cascade-delete IT test deferred (acceptable).
• Minor items: getDelegatedTenant returns shallow copy, getTenantIds filters missing .id, schema tests verify allowsUpdates/allowsRemove, organizationId naming clarified via JSDoc.

Issues

Minor (Nice to Have)

• base.model.js catch clause silently skips removeDependents when the dependent table is absent from the PostgREST schema cache. This is motivated by a test environment issue (CI DB pre-dates the migration) but runs in production too. In practice this is defensible - if the table doesn't exist, there can't be orphaned records in it. The match pattern is specific ('Could not find the' + 'table'/'column' + 'schema cache'). However, adding a log.warn when the skip fires would make this observable in production rather than completely silent.

Assessment

Ready to merge? Yes.

Thorough response to review feedback - 13 of 14 findings addressed, with the remaining item (cascade audit gap) tracked via a clear TODO. The fixes are clean, well-tested, and follow established codebase patterns. The auth changes (null guard, shallow copy, JSDoc) are particularly well-implemented. The deferred items (cascade audit hook, TOCTOU hardening) are documented and bounded to Phase 2.

Note: CI Test job is currently failing but all tests pass (361 IT + 810 unit). The failure appears to be transient/infra-related, not caused by the PR changes.

[solaris007]

Hey @ekremney,

Previously Flagged Issues - Now Addressed

• base.model.js catch block coverage (Minor from prior review): Resolved. Two new tests added covering both the schema-cache-error swallow path and the re-throw path for non-schema-cache errors.

Issues

Minor (Nice to Have)

• Orphaned JSDoc block before #fetchGrantsWithTargetOrg (site-ims-org-access.collection.js ~lines 82-93): A full JSDoc block documenting allByOrganizationIdWithTargetOrganization sits before the private helper's own JSDoc. The same block is repeated before the actual public method. Looks like a copy-paste artifact from the refactoring - remove the first copy.
• Query builder mutation accumulates redundant ORDER clauses: #fetchGrantsWithTargetOrg calls query.order('id').range(...) on each pagination iteration. The Supabase builder's .order() appends rather than replaces, so after N pages the URL contains order=id.asc N times. Functionally harmless (PostgREST applies it idempotently, and most callers won't paginate past page 1), but fragile if the pattern is copied. Consider calling .order('id') once before the loop.
• No upper bound on organizationIds array: allByOrganizationIdsWithTargetOrganization passes the array directly into PostgREST's .in() filter. At ~80+ UUIDs the URL approaches typical length limits (~8KB), producing an opaque 414 rather than a clear error. Unlikely for the login-time use case (users rarely belong to that many orgs), but a defensive guard (reject arrays > 50-100 with a DataAccessError) would prevent surprises from future callers.

Assessment

Ready to merge? Yes.

CI is now fully green. The two new collection methods (allByOrganizationIdWithTargetOrganization, allByOrganizationIdsWithTargetOrganization) correctly serve the login-time JWT enrichment use case with single-query joins rather than N+1 lookups. The #fetchGrantsWithTargetOrg helper with offset-based pagination follows established patterns. The FK disambiguation hint is correct and necessary. Supporting changes (image bump, skip guard removal, fixture realignment, sentiment-topic cleanup, base.model coverage) are all clean.

[ekremney]

@solaris007 thanks for the thorough review! All three items addressed in the latest commits:

• Orphaned JSDoc — removed the duplicate block above #fetchGrantsWithTargetOrg
• Redundant ORDER clauses — moved .order('id') outside the pagination loop into orderedQuery
• No upper bound on organizationIds — added a guard that throws DataAccessError when the array exceeds MAX_DELEGATES_PER_SITE (50)

Also snuck in the self-delegation guard (organizationId === targetOrganizationId → 409) that was a known missing piece.

[solaris007]

🎉 This PR is included in version @adobe/spacecat-shared-data-access-v3.25.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[solaris007]

🎉 This PR is included in version @adobe/spacecat-shared-http-utils-v1.23.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀