Skip to content

Conversation

@ravverma
Copy link
Contributor

Summary

Add new Consumer entity to the data access layer for registering and managing technical accounts. This entity provides a structured way to track consumers with their associated client credentials, capabilities, and issuer information.

Changes

Consumer Entity Schema

  • consumerId (UUID): Primary key identifier
  • clientId (string): Client identifier, indexed for efficient lookups
  • consumerName (string): Consumer name/identifier
  • status (enum): Consumer status (ACTIVE, SUSPEND)
  • capabilities (string[]): Array of granted capabilities (required)
  • issuerId (enum): Adobe IMS Organization ID from allowed list

Validation & Constraints

  • Status Validation: Only allows ACTIVE or SUSPEND values
  • Issuer ID Validation:
    • Must match format: [a-z0-9]{24}@AdobeOrg
    • Must be from predefined allowed list (PRODUCTION, STAGE)
  • Required Fields: All fields are required

Database Indexes

  • Primary index: consumerId
  • Secondary index: issuerId - for querying consumers by issuer
  • Secondary index: clientId - for querying consumers by client

Collection Methods

Auto-generated query methods via BaseCollection:

  • findById(consumerId) - Find consumer by ID
  • allByIssuerId(issuerId) - Get all consumers for an issuer
  • allByClientId(clientId) - Get all consumers for a client
  • findByIssuerId(issuerId) - Find first consumer by issuer
  • findByClientId(clientId) - Find first consumer by client
  • Standard CRUD operations (create, update, delete)

Test Coverage

Unit Tests: 15 tests covering model static constants, getters, setters, and collection
Integration Tests: 9 tests covering CRUD operations, index queries, and validation
Fixtures: Sample test data for multiple consumer scenarios

Files Added

  • src/models/consumer/consumer.model.js
  • src/models/consumer/consumer.collection.js
  • src/models/consumer/consumer.schema.js
  • src/models/consumer/index.js
  • src/models/consumer/index.d.ts
  • test/fixtures/consumers.fixture.js
  • test/it/consumer/consumer.test.js
  • test/unit/models/consumer/consumer.model.test.js
  • test/unit/models/consumer/consumer.collection.test.js

Files Modified

  • src/models/base/entity.registry.js - Registered Consumer entity
  • src/models/index.js - Added Consumer exports
  • src/models/index.d.ts - Added Consumer type exports
  • test/fixtures/index.fixtures.js - Added consumer fixtures

@ravverma ravverma requested a review from solaris007 February 12, 2026 10:07
@github-actions
Copy link

This PR will trigger a minor release when merged.

Copy link
Member

@solaris007 solaris007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ravverma

Good first implementation - follows codebase patterns well, solid test coverage. However, there are several schema-level decisions that become expensive to change after data exists. Requesting changes on those before merge.

Must-Fix (before merge)

# Finding Rationale
1 Rename issuerId to imsOrgId Every other entity in the codebase uses imsOrgId for this concept. In OAuth/OIDC, "issuer" means the authorization server (IMS itself), not the org. The existing api-key.schema.js uses imsOrgId. This divergence will cause confusion. Schema rename is free now, painful after data exists.
2 SUSPEND -> SUSPENDED Entitlement and SiteEnrollment both use SUSPENDED. Status values describe state (adjective), not action (verb). Fixing after data is persisted requires migration.
3 Move ALLOWED_ISSUER_IDS out of source code Real IMS org IDs hardcoded in a public repo. These confirm which org IDs are authoritative for SpaceCat S2S auth. Move to env config or the existing Configuration entity. Use synthetic IDs in test fixtures.
4 Anchor the regex ISSUER_ID_REGEX = /[a-z0-9]{24}@AdobeOrg/i matches substrings. Should be /^[a-z0-9]{24}@AdobeOrg$/i.
5 Make clientId and imsOrgId immutable Identity-binding fields should be readOnly: true in the schema. Repointing a consumer to a different TA should require creating a new consumer, not updating an existing one. Exposing setClientId/setIssuerId allows mutation of security-critical identity bindings without explicit authorization checks.

Should-Fix (before auth-service integration)

# Finding Rationale
6 Add technicalAccountId field clientId identifies the credential/integration; technicalAccountId (the sub claim in the access token) identifies the service principal. Token validation should verify both match. Checking only client_id would allow any TA credential under the same integration to authenticate.
7 Add capability validation/enum Free-form string list with no allowlist. Define known capabilities (sites:read, sites:write, etc.) similar to ApiKey.SCOPE_NAMES. Without this, any string can be stored - including admin, *, or typos that silently grant no access.
8 Add REVOKED terminal status SUSPENDED is reversible. Incident response needs an irreversible state. The ApiKey entity handles this with a revokedAt timestamp - consider the same pattern.
9 Clarify clientId uniqueness Fixtures show duplicate clientIds (items 0 and 2 both have client-123-abc). If the login flow uses findByClientId, it returns only the first match - ambiguous. Document whether this is intentional, or add a composite uniqueness constraint (clientId + imsOrgId).
10 Add DEV environment Only PROD and STAGE org IDs listed. If DEV reuses STAGE, document it with a comment. If DEV has its own org, add it.

Nice-to-Have

  • Document the dual addAllIndex pattern (no other entity does this - worth a comment explaining why)
  • Remove or document ISSUER_ID_REGEX (dead code - schema uses enum validation, not regex)
  • More realistic fixture names (mystique, drs instead of consumer-456-def)
  • Add isActive() convenience method (similar to ApiKey.isValid())
  • Consider imsScopes field to track expected TA scopes for drift detection
  • Move the remove integration test to last position to avoid ordering fragility

Items 1-5 are the blockers - all schema-level decisions that are painful to change post-merge. Items 6-10 can land in a fast follow-up but ideally before the auth-service login flow is built on top of this entity.

@solaris007 solaris007 added the enhancement New feature or request label Feb 12, 2026
@ekremney ekremney self-requested a review as a code owner February 12, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants