feat: consumer controller apis

[ravverma]

@ -0,0 +1,56 @@

PR Summary – Consumer Management API

Summary

Adds a new Consumer Management API for registering and managing API consumers (Technical Accounts). This enables service-to-service integrations to be tracked, permissioned, and lifecycle-managed via a set of RESTful endpoints.

New Endpoints

Method	Path	Description
GET	/consumers	List all registered consumers
GET	/consumers/:consumerId	Get a consumer by consumer ID (primary key)
GET	/consumers/by-client-id/:clientId	Get a consumer by IMS OAuth client ID
POST	/consumers/register	Register a new consumer using a TA access token
PATCH	/consumers/:consumerId	Update a consumer's name, capabilities, or status
POST	/consumers/:consumerId/revoke	Revoke a consumer

Key Design Decisions

• URL param alignment: Resource lookups use :consumerId (primary key) for get/update/revoke. A separate route GET /consumers/by-client-id/:clientId supports lookup by IMS client_id and uses Consumer.findByClientId(clientId).
• IMS token validation on register: The register endpoint validates the TA access token via imsClient.validateAccessToken(). The validation response includes a token object with client_id, user_id, and org; identity is taken from that payload (no separate profile API). If the response lacks token, the request fails with 400.
• Immutable fields: clientId, technicalAccountId, and imsOrgId are immutable after registration. Attempts to modify them in update return 400 with a clear error.
• Explicit revoke flow: update does not accept revokedAt or REVOKED status. Revocation is a dedicated POST /consumers/:consumerId/revoke operation. Once revoked, no further updates are permitted.
• Access control: All consumer endpoints require S2S Admin via hasS2SAdminAccess() on AccessControlUtil (delegates to authInfo.isS2SAdmin()).
• Audit trail: All mutating operations (register, update, revoke) log an info message and send a Slack notification to channel C0AFRHVRMPT with operation details (including a delta of changes for updates).
• updatedBy: Every mutation records who performed it, from the caller's auth profile email (falls back to 'system').
• Error responses: 4xx/5xx responses return a JSON body with a message field (in addition to the x-error header) so clients can read the error message from the body.
• Data access: Lookup by consumer ID uses Consumer.findById(consumerId); lookup by IMS client ID uses Consumer.findByClientId(clientId), consistent with patterns used in other controllers.

Files Changed

• src/controllers/consumers.js — New controller (factory function pattern): getAll, getByConsumerId, getByClientId, register, update, revoke.
• src/dto/consumer.js — New DTO for API response serialization (includes consumerId, clientId, technicalAccountId, imsOrgId, consumerName, capabilities, status, revokedAt, createdAt, updatedAt, updatedBy).
• src/routes/index.js — Six consumer route definitions (including GET /consumers/by-client-id/:clientId).
• src/index.js — Controller instantiation and wiring.
• src/support/access-control-util.js — Added hasS2SAdminAccess().
• test/controllers/consumers.test.js — Unit tests for all endpoints, access control, validation, error paths, and edge cases (including getByClientId, token payload validation, and error body message).
• test/dto/consumer.test.js — DTO serialization tests.
• test/support/access-control-util.test.js — Test for hasS2SAdminAccess().
• test/routes/index.test.js — Consumer routes included in route segregation test.
• test/controllers/fixes.test.js — Fixed EntityRegistry constructor call for @adobe/spacecat-shared-data-access@2.109.0.
• docs/access-control-matrix.md — Capabilities matrix (Admin vs LLMO Administrator); S2S/Consumer not in matrix per request.
• package.json — Bumped spacecat-shared-data-access and spacecat-shared-http-utils as needed.

Test Plan

• All consumer controller tests pass (getAll, getByConsumerId, getByClientId, register, update, revoke).
• Access control enforced — forbidden for non–S2S-admin callers.
• Immutable field rejection tested for all three fields individually and combined.
• Revoked consumer blocks further updates.
• Slack notification failure does not break the operation.
• Register uses token payload from validateAccessToken; missing or invalid token payload returns 400 with message in body.
• DTO serialization verified (including consumerId).
• Route segregation test updated and passing.
• Fixes controller tests fixed for updated data-access dependency.
• Error responses include message in JSON body for 4xx/5xx.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

This PR will trigger a minor release when merged.

[solaris007]

Hey @ravverma

Solid foundation for consumer lifecycle management. The factory pattern is consistent with the codebase, immutable field protection is clean, revocation as a separate lifecycle action is the right design, and Slack notification resilience (fire-and-forget) is correct. Test coverage is broad at 827 lines covering happy paths, validation, forbidden access, DB failures, and edge cases.

However, there are issues that need to be addressed before merging.

---

CRITICAL: Missing access control on getByConsumerId and getByClientId

getAll correctly gates on hasS2SAdminAccess(), but getByConsumerId (~line 128) and getByClientId (~line 155) have no access control whatsoever. Any authenticated user can look up any consumer's details - including clientId, technicalAccountId, imsOrgId, and capabilities - by guessing or enumerating IDs.

Both functions need the same guard:

if (!accessControlUtil.hasS2SAdminAccess()) {
  return forbidden('Only S2S admins can view consumers');
}

The tests confirm this gap: getByConsumerId and getByClientId describe blocks have no "returns forbidden for non-admin users" test, unlike all four other endpoints (getAll, register, update, revoke).

If the intent is to allow consumers to look up their own record, a narrower self-access check (e.g., caller's clientId matches the consumer's clientId) would be appropriate instead. Either way, the current state of zero access control on these endpoints is not correct.

---

IMPORTANT: Access token passed in request body

The register endpoint accepts accessToken in the JSON request body. This token will appear in any middleware-level request/body logging, APM traces, and error stack traces. Consider accepting it via the Authorization header instead (e.g., Authorization: Bearer <ta-token>), which logging frameworks typically redact by default. The code itself is careful (the log.info on ~line 199 logs consumerName but not the token), but middleware-level body logging is the concern.

---

IMPORTANT: Slack mrkdwn injection via unsanitized user input

User-controlled consumerName and capabilities values are interpolated directly into Slack mrkdwn messages. While backtick-wrapping provides some protection, a consumer name containing backticks breaks out of the code block. For example, `foo` *bold injection* would render as active Slack markup. This affects register (~line 252), update (~line 337), and revoke notifications.

This could be used to craft misleading admin notifications (e.g., making an action appear to be performed by a different person). Sanitize or escape mrkdwn special characters (*, _, `, ~, <, >) before interpolation.

---

IMPORTANT: No capability validation

capabilities accepts any array with Array.isArray(capabilities) && capabilities.length > 0. There is no check that elements are strings, are non-empty, match a known set, or are free of duplicates. capabilities: [null, 123, {}, ""] or capabilities: ["<script>alert(1)</script>"] would be stored verbatim. Same issue in the update path.

At minimum, validate that all elements are non-empty strings. Ideally validate against an enum of known capabilities, or document that capabilities are freeform and downstream must not trust them for authorization.

---

MINOR: TOCTOU race on register duplicate check

const existing = await Consumer.findByClientId(clientId);
if (existing) { throw ... }
const consumer = await Consumer.create({...});

A concurrent request could register the same clientId between the check and the create. If the data layer supports a unique constraint on clientId, that would be the proper guard. Otherwise, document this as a known limitation.

---

MINOR: Consumer enumeration via error messages

The notFound responses echo the lookup key: Consumer with consumerId ${consumerId} not found. Less critical given admin-only access (once the access control fix is applied), but for defense-in-depth, consider a generic "Consumer not found" message.

---

MINOR: getUpdatedBy fallback to 'system'

When the auth profile lacks an email, updatedBy is set to 'system'. This is ambiguous in audit trails - was this an actual system operation or a user whose profile lacked an email? Consider a more distinctive marker or logging a warning when the fallback is used.

---

MINOR: notifySlack pattern and channel validation

The notifySlack function uses Promise.resolve().then().catch() when try/catch would be simpler (the function is already async). Also, if ctx.env exists but S2S_SLACK_CHANNEL_ID is unset, postMessage gets called with channel: undefined. Validate the channel ID exists and log a specific warning if missing.

---

Process: Missing integration tests and OpenAPI specs

The repo requires integration tests for new/modified endpoints (test/it/) and OpenAPI-first API contract definitions (docs/openapi/). Neither is present in this PR. These can be follow-up PRs, but should be tracked.

[ravverma]

@solaris007 review comment addressed

Changes

API

• TA token: Move from request body to x-ta-access-token header for register
• Capability validation: Require non-empty strings and validate against entity:operation allowlist (from data layer)
• ValidationError: Return 400 for ValidationError from create/save

Security

• Slack mrkdwn: Escape user-controlled values in Slack notifications to avoid injection
• Access control: Enforce S2S admin for getByConsumerId and getByClientId

Tests

• Unit tests for capability validation, ValidationError, and pathInfo branch
• Integration tests for consumers (DynamoDB + PostgreSQL)
• Consumer seed data and wiring

Other

• Add docs/consumers-api-backlog.md
• Fix IT seed import paths and duplicate imports