Skip to content

security(backend): enforce token scopes consistently across transfer and admin routes - #143

Open
woahwhattheheck wants to merge 13 commits into
RemitFlow:mainfrom
woahwhattheheck:latch/remitflow-125-token-scopes
Open

woahwhattheheck wants to merge 13 commits into
RemitFlow:mainfrom
woahwhattheheck:latch/remitflow-125-token-scopes

Conversation

@woahwhattheheck

@woahwhattheheck woahwhattheheck commented Sep 24, 2026 •

Copy link
Copy Markdown

Why

Closes #125.

Inconsistent route and service checks allowed token-scope rules to vary across transfer, user, audit, and admin operations. The bulk dispatcher also accepted inherited properties and coerced non-string actions. Explicitly malformed API_TOKENS configuration silently enabled public demo credentials, while array-shaped maps could create unintended numeric tokens and null maps caused request failures.

Changes

  • Define the canonical action/resource scope catalog and route matrix in src/config/scopes.js.
  • Enforce scopes at route middleware and, when an explicit auth context is supplied, at service boundaries through shared authz helpers. HTTP controllers pass authFromRequest(req); trusted internal callers retain the documented omitted-context behavior.
  • Require admin:read for diagnostics; map the existing admin key onto that scope.
  • Provide POST /api/transfers/bulk for up to 50 IDs with per-ID outcomes. Validate action type and own membership before dispatch: only claim, cancel, archive, and unarchive run. Invalid actions return 400 before transfer mutation.
  • Return the same 404 message for malformed and missing transfer IDs in direct and bulk paths.
  • Validate explicitly supplied API_TOKENS before application startup: require an object with non-empty, unpadded keys and arrays of catalogued scopes. Reject malformed JSON, blank values, wrong shapes, and unknown scopes with a generic error that does not print credential keys or values.
  • Store tokens in a map without inherited properties. Unconfigured constructor, toString, and proto names receive the documented unknown-token 401 response on user and admin routes.
  • Document the scope matrix, admin credentials, bulk contract, and token configuration behavior in the existing docs, README, and changelog.
  • Document the service integration requirement, the existing archive/create argument positions, and recognized API-token precedence over a simultaneous legacy admin header. This aligns the contributor guide with the existing implementation.

Compatibility and boundaries

  • Transfer reads retain the shared-data demo model: any transfers:read token can read records. This contribution does not introduce account tenancy.
  • Legacy ADMIN_API_KEY credentials remain accepted and map to admin:read.
  • Trusted internal service callers may omit auth; HTTP controllers pass an explicit auth context.
  • Scope checks precede bulk action/ID validation. Valid actions retain lifecycle, batch-limit, and per-ID error behavior.
  • Demo credentials remain available only when API_TOKENS is absent, preserving the documented local-development mode. An explicitly empty value now stops startup. A valid empty object disables API-token credentials while the separately configured legacy admin key retains its behavior.
  • Existing valid token maps retain their grants. No dependency, provider, account, or deployment changes are included.

Documentation continuation at b0c161f

Documentation head: b0c161f
Sole parent: c147a2e
Tree: 6c546ffc026b270c3cf23925089ce7f908d2a26c

Only README.md and docs/SCOPE_MATRIX.md change (+47/-6). The guide now explains that assertScopes skips null/undefined auth, so a request adapter must preserve its route middleware and pass authFromRequest(req) to the service. It also describes the existing 403 result when a recognized, under-scoped API bearer token is supplied alongside a valid legacy admin header.

All 175 other tracked leaf blobs and modes are unchanged, including every route, service, test, manifest and lockfile. The complete two-file contents, Git blobs, sole-parent commit/tree/message, original branch, PR head and one-commit diff were read back through the provider API. Documentation examples were reconciled directly with the current auth helper, transfer controller/service and admin middleware.

This continuation is API-only. No new native HTTP flow, maintained suite, renderer, syntax command, install or external-provider request was attempted while the execution environment was offline. The retained execution evidence below belongs to the unchanged implementation at the parent head; it is not presented as a new run.

At this documentation revision, CI run 37172150829 is completed/action_required, with zero jobs returned by the jobs endpoint. The PR remains open and unmerged. Maintainer CI approval and acceptance remain outstanding.

Retained validation at c147a2e

  • Ordinary full npm test: 298 passed, 0 failed, 0 skipped on Node 24.19.0 with the matching lockfile.
  • Sixteen new failure-mode cases failed on original head e732094: 13 invalid configuration profiles and 3 inherited-name HTTP cases. They now pass; valid-map and empty-map controls pass as well.
  • Actual production-mode local Express execution covered 9 configuration scenarios. Before repair, malformed/empty explicit maps admitted the public demo admin credential, and an array-shaped map admitted token 0: diagnostics returned 200 and user creation returned 201. All 5 invalid profiles in that HTTP reproduction now reject startup before a listener exists. Valid-map and absent-variable controls retain 200/201; an empty object and unconfigured inherited name return 401/401.
  • The existing scope matrix, cross-surface, bulk-action, malformed-ID, audit authorization, service-boundary, and legacy-admin checks remain green in the full suite. Earlier bulk-dispatch regressions and contributor history are preserved.
  • JavaScript syntax and git diff checks passed. The four-file continuation, exact tree, and all four blobs read back from the same fork branch after a nonforce update.

At that parent head, CI run 37116422606 reported action_required. Hosted CI success, sponsor acceptance, deployment, and live Stellar settlement are not claimed. All HTTP reproduction used local fixtures and closed its servers.

Later source continuations

Current original PR head: 38d34851ddfa7e66c4206db5d124aa720d32e9e9. These changes preserve the original scope catalog, bulk contract, token-map validation and contributor history.

  • 3f83f390 makes bulk failures follow the central ApiError boundary. Unexpected thrown values cannot supply public status or message, and null/string failures remain per-item while independent rows continue. Intentional API conflicts and the canonical 404 remain. One completed native execution used the actual createApp/Express application over loopback: the new cases had 2 passes and 6 failures on the previous source; repaired maintained tokenScopes plus the new regressions had 33 passes, zero failures or skips. Source-bound report.
  • 368f8b5f validates the claim transition before adapter work and commits terminal state only after adapter success. Adapter failure leaves the pending record and timestamp unchanged for retry; successful independent bulk rows remain committed. Two focused actual-production-module recovery cases failed before and passed after on Node 24.19.0. That isolated execution used the maintained mock adapter failure and an external inert dotenv loader; it was not an HTTP, full-suite or live-chain run.
  • 05a6279b enforces audit:read for an explicit auth context in getEntriesForResource before looking up a present, missing or empty resource. Omitted/null trusted contexts retain compatibility. Three focused cases passed through the actual audit, authz and configuration modules with a synthetic store on Node 24.19.0 and an external inert dotenv loader; no HTTP or full suite was run for this continuation.
  • 2834ab83 enforces users:read in exported findUser before lookup when auth is supplied. Existing and missing IDs with insufficient scope produce the same 403 without unauthorized store lookup; authorized and trusted calls retain their results. The two maintained service cases changed from 1 pass / 1 failure to 2 passes / zero failures or skips on Node 24.19.0 with the actual service/store/authz/config/audit/UUID modules and the external inert dotenv loader. This was not a provider, HTTP or full-suite execution.
  • The cursor continuation through 38d34851 adds optional auth to transferService.positionKeyAt(seq, auth) and auditService.positionKeyAt(seq, resourceId, auth), requiring transfers:read and audit:read respectively. Both history controllers pass their already-built auth context. Trusted internal calls without auth remain supported. The existing tokenScopes regression source includes one additional cursor-context case. No repository suite was executed for this latest continuation.

Current validation boundary

The earlier full-suite and HTTP results above remain pinned to their recorded revisions. They are not combined into a new total and do not establish full-suite acceptance at 38d34851. At the current head, upstream CI run 37211981375 reports action_required with zero jobs, and no fork pull-request run was returned. The original PR remains open; hosted approval, deployment and live settlement remain separate.

woahwhattheheck and others added 13 commits September 24, 2026 18:24
Add a canonical scope catalog, re-check scopes at service boundaries, gate
admin diagnostics on explicit admin:read (while mapping the legacy admin key),
and introduce bulk transfer mutations with non-enumerating per-id errors so
malformed and missing ids cannot be told apart.

Closes RemitFlow#125
Reject inherited action names and non-string values before handler lookup.
Preserve scope checks, unsupported-string errors, and valid bulk behavior.

Five HTTP regressions reproduce false success, a coerced cancellation,
and a malformed-action 500 on the prior source. Full npm test: 281 pass.
Fail startup for malformed or invalid supplied token maps instead of enabling
public demo credentials. Validate configured scopes using the existing catalog
and keep token lookup limited to explicit entries. Preserve absent-variable
demo mode, valid maps, and the legacy admin credential behavior.

Validation: 16 new failure-mode cases failed on original source; npm test now
passes all 298 tests without skips. Actual local production-mode Express runs
confirmed malformed/empty maps previously granted demo admin access and
array-shaped maps granted token 0. All invalid configurations now stop startup;
valid-map and absent-variable controls still pass. No dependency changes or
external provider calls.
Validate the lifecycle transition before adapter work and apply state only after it succeeds. Preserve write-scope checks, terminal conflicts, and independent bulk results.

Focused production-module execution: two recovery cases failed before and pass after, including retry, redaction, terminal-state and under-scope controls. Node 24.19.0; existing simulated adapter with injected failure, external no-env dotenv loader. No HTTP, live-chain or full-suite result is claimed.
getEntriesForResource ignored a supplied authorization context while the
other audit readers enforced audit:read. Accept the optional context and
check it before inspecting the resource, using the existing shared guard.
Keep the documented null/undefined trusted internal behavior, resource
filtering and newest-first ordering.

On parent 368f8b5 the actual service
returned an audit row to an explicit transfers:read context while its
getEntries sibling returned 403. One new maintained regression fails on
the original and passes after the repair; its two existing resource-reader
controls also pass (3/3, zero skipped). It covers uniform scope denial for
present/missing/empty resources, audit:read access, trusted callers, and
unchanged ordering.

Executed with Node 24.19.0 on actual audit/index/authz/config modules and
synthetic in-memory entries. Only unavailable dotenv initialization used
an external inert no-.env shim; source/config/dependencies unchanged.
The current HTTP controller already uses guarded query/count readers;
this change makes the exported direct service reader consistent. No HTTP,
full-suite or CI result is claimed. Existing transfer recovery preserved.

Reproduce with installed project dependencies:
node --test --test-name-pattern='^getEntriesForResource' test/audit.test.js
Issue RemitFlow#125 requires scope enforcement at service boundaries. The exported findUser helper ignored an explicitly supplied context, unlike listUsers and getUserOrThrow. Require users:read before looking up any record while retaining the documented trusted behavior for omitted/null contexts.

Add two maintained Node service regressions. On parent 05a6279 they produced 1 pass / 1 fail (the direct lookup did not reject an under-scoped caller). With this repair they produce 2 pass / 0 fail / 0 skipped, including missing/existing-ID parity, zero unauthorized lookups, authorized readers and trusted-call compatibility.

Execution: Node v24.19.0; node --require /dev/shm/harbor-6907-rfb143-audit/preload-dotenv.cjs --test test/userScope.test.js. That external execution-only shim made unavailable dotenv.config inert; actual service, store, authorization, scope catalog, audit and UUID modules ran unchanged. No .env file, HTTP/server, hosted CI, chain or full-suite result is claimed. Prior transfer recovery and audit-resource scope fixes are preserved.
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.

security(backend): enforce token scopes consistently across transfer and admin routes

1 participant