From f7fb04a084013990392bfbf3d49604719c550eb2 Mon Sep 17 00:00:00 2001 From: alexcos20 Date: Mon, 17 Aug 2026 11:15:31 +0300 Subject: [PATCH 1/3] Authenticate `PolicyServerPassthrough` / `initializePSVerification`, and stop logging credentials --- docs/API.md | 344 +++++++++++------ docs/PolicyServer.md | 46 +++ src/@types/commands.ts | 7 + .../core/handler/coreHandlersRegistry.ts | 9 +- src/components/core/handler/policyServer.ts | 60 ++- src/components/httpRoutes/policyServer.ts | 9 +- src/components/httpRoutes/validateCommands.ts | 29 ++ src/test/unit/policyServer.test.ts | 348 ++++++++++++++++++ src/test/unit/validateCommands.test.ts | 124 +++++++ src/utils/constants.ts | 2 + 10 files changed, 850 insertions(+), 128 deletions(-) create mode 100644 src/test/unit/policyServer.test.ts create mode 100644 src/test/unit/validateCommands.test.ts diff --git a/docs/API.md b/docs/API.md index bd7386756..249e19cbe 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1204,20 +1204,43 @@ returns the current indexing queue, as an array of objects #### Description -Forwards request to PolicyServer (if any) +Forwards request to PolicyServer (if any). + +This endpoint is authenticated. The caller must provide either an `Authorization` header +carrying an auth token, or a `nonce` + `signature` pair. `consumerAddress` is always required. Requests without valid credentials +are rejected with `401`. + +The node verifies the caller, then injects the **verified** `consumerAddress` (overwriting +any value present inside `policyServerPassthrough`), along with `authorization`, `nonce`, +`signature`, the resolved `ddo` and the node's own `nodeAddress`, into the payload it +forwards to the PolicyServer. + +#### Request Headers + +| name | required | description | +| ------------- | -------- | -------------------------------------------------------------- | +| Authorization | | auth token; required unless `nonce` + `signature` are supplied | #### Parameters -| name | type | required | description | -| ----------------------- | ------ | -------- | ---------------------------------------------- | -| command | string | v | command name | -| node | string | | if not present it means current node | -| policyServerPassthrough | any | | command and params for PolicyServer (see docs) | +| name | type | required | description | +| ----------------------- | ------ | -------- | ---------------------------------------------------- | +| command | string | v | command name | +| node | string | | if not present it means current node | +| policyServerPassthrough | object | v | command and params for PolicyServer (see docs) | +| consumerAddress | string | v | the caller's address | +| nonce | string | | required unless an `Authorization` token is supplied | +| signature | string | | required unless an `Authorization` token is supplied | + +The signed message is `consumerAddress + nonce + "PolicyServerPassthrough"`. #### HTTP Example ```json { + "consumerAddress": "0x9876543210fedcba9876543210fedcba98765432", + "nonce": "1", + "signature": "0x123", "policyServerPassthrough": { "action": "newDDO", "rawDDO": {}, @@ -1234,6 +1257,9 @@ Forwards request to PolicyServer (if any) { "command": "PolicyServerPassthrough", "node": "PeerId", + "consumerAddress": "0x9876543210fedcba9876543210fedcba98765432", + "nonce": "1", + "signature": "0x123", "policyServerPassthrough": { "action": "newDDO", "rawDDO": {}, @@ -1244,6 +1270,68 @@ Forwards request to PolicyServer (if any) } ``` +#### Responses + +| code | description | +| ---- | --------------------------------------------------------------- | +| 200 | PolicyServer allowed the request; its response body is returned | +| 400 | missing/invalid parameters | +| 401 | missing or invalid authentication | + +--- + +## initializePSVerification + +### `HTTP` POST /api/services/initializePSVerification + +### `P2P` command: PolicyServerInitialize + +#### Description + +Asks the PolicyServer to start a verification flow (`initiate` action) for a given +asset/service and consumer. Authenticated the same way as `PolicyServerPassthrough`: an +`Authorization` header or a `nonce` + `signature` pair. + +This is a distinct command from `PolicyServerPassthrough`, so the signed message uses its +own command string: `consumerAddress + nonce + "PolicyServerInitialize"`. + +The verified `consumerAddress` is the one forwarded to the PolicyServer; the caller's +`authorization`, `nonce` and `signature` are added to the `policyServer` object. + +#### Parameters + +| name | type | required | description | +| --------------- | ------ | -------- | ---------------------------------------------------- | +| documentId | string | v | the asset DID | +| serviceId | string | v | the service id within the asset | +| consumerAddress | string | v | the caller's address | +| policyServer | any | v | free-form data passed to the PolicyServer | +| nonce | string | | required unless an `Authorization` token is supplied | +| signature | string | | required unless an `Authorization` token is supplied | + +#### HTTP Example + +```json +{ + "documentId": "did:op:1234", + "serviceId": "0", + "consumerAddress": "0x9876543210fedcba9876543210fedcba98765432", + "nonce": "1", + "signature": "0x123", + "policyServer": {} +} +``` + +#### Responses + +| code | description | +| ---- | --------------------------------- | +| 200 | PolicyServer response body | +| 400 | missing/invalid parameters | +| 401 | missing or invalid authentication | +| 404 | asset not found | +| 503 | DDO database not available | + --- ## Fetch Config @@ -1359,7 +1447,13 @@ Returns indexed Escrow contract events. The indexer matches Escrow logs by topic #### Request (POST /directCommand) ```json -{ "command": "getEscrowEvents", "chainId": 8996, "eventType": "Deposit", "offset": 0, "size": 50 } +{ + "command": "getEscrowEvents", + "chainId": 8996, + "eventType": "Deposit", + "offset": 0, + "size": 50 +} ``` #### Response @@ -1503,23 +1597,23 @@ starts a free compute job and returns jobId if succesfull #### Parameters -| name | type | required | description | -| --------------------------- | ------ | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| command | string | v | command name | -| node | string | | if not present it means current node | -| consumerAddress | string | v | consumer address | -| signature | string | v | signature (msg=String(nonce) ) | -| nonce | string | v | nonce for the request | -| datasets | object | | list of ComputeAsset to be used as inputs | -| algorithm | object | | ComputeAlgorithm definition | -| environment | string | v | compute environment to use | -| resources | object | | optional list of required resources | -| metadata | object | | optional metadata for the job, data provided by the user | -| additionalViewers | object | | optional array of addresses that are allowed to fetch the result | -| queueMaxWaitTime | number | | optional max time in seconds a job can wait in the queue before being started | -| encryptedDockerRegistryAuth | string | | Ecies encrypted docker auth schema for image (see [Private Docker Registries with Per-Job Authentication](../env.md#private-docker-registries-with-per-job-authentication)) | -| output | string | | Ecies encrypted with instructions for uploading compute results (see [C2D result upload to remote storage](../Storage.md#c2d-result-upload-to-remote-storage)) | -| outputBucketId | string | | persistent-storage bucket id; the bucket is mounted at /data/outputs and results are stored there as individual files. Mutually exclusive with `output` (see [persistent storage](../persistentStorage.md#using-a-bucket-for-compute-job-outputs)) | +| name | type | required | description | +| --------------------------- | ------ | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| command | string | v | command name | +| node | string | | if not present it means current node | +| consumerAddress | string | v | consumer address | +| signature | string | v | signature (msg=String(nonce) ) | +| nonce | string | v | nonce for the request | +| datasets | object | | list of ComputeAsset to be used as inputs | +| algorithm | object | | ComputeAlgorithm definition | +| environment | string | v | compute environment to use | +| resources | object | | optional list of required resources | +| metadata | object | | optional metadata for the job, data provided by the user | +| additionalViewers | object | | optional array of addresses that are allowed to fetch the result | +| queueMaxWaitTime | number | | optional max time in seconds a job can wait in the queue before being started | +| encryptedDockerRegistryAuth | string | | Ecies encrypted docker auth schema for image (see [Private Docker Registries with Per-Job Authentication](../env.md#private-docker-registries-with-per-job-authentication)) | +| output | string | | Ecies encrypted with instructions for uploading compute results (see [C2D result upload to remote storage](../Storage.md#c2d-result-upload-to-remote-storage)) | +| outputBucketId | string | | persistent-storage bucket id; the bucket is mounted at /data/outputs and results are stored there as individual files. Mutually exclusive with `output` (see [persistent storage](../persistentStorage.md#using-a-bucket-for-compute-job-outputs)) | #### Request @@ -1655,9 +1749,9 @@ Create a new persistent storage bucket. Bucket ownership is set to the request ` #### Request Headers -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| Authorization | string | | auth token (optional; depends on node auth configuration) | +| name | type | required | description | +| ------------- | ------ | -------- | --------------------------------------------------------- | +| Authorization | string | | auth token (optional; depends on node auth configuration) | #### Request Body @@ -1690,13 +1784,13 @@ List buckets for a given `owner`. Results are filtered by bucket access lists fo #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | consumer address | +| name | type | required | description | +| --------------- | ------ | -------- | -------------------------------------------------- | +| consumerAddress | string | v | consumer address | | signature | string | v | signed message (consumerAddress + nonce + command) | -| nonce | string | v | request nonce | -| chainId | number | v | chain id (used by auth/signature checks) | -| owner | string | v | bucket owner to filter by | +| nonce | string | v | request nonce | +| chainId | number | v | chain id (used by auth/signature checks) | +| owner | string | v | bucket owner to filter by | #### Response (200) @@ -1721,11 +1815,11 @@ List files in a bucket. #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | consumer address | +| name | type | required | description | +| --------------- | ------ | -------- | -------------------------------------------------- | +| consumerAddress | string | v | consumer address | | signature | string | v | signed message (consumerAddress + nonce + command) | -| nonce | string | v | request nonce | +| nonce | string | v | request nonce | #### Response (200) @@ -1750,11 +1844,11 @@ Return the `fileObject` for a specific file in a bucket (useful for passing refe #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | consumer address | +| name | type | required | description | +| --------------- | ------ | -------- | -------------------------------------------------- | +| consumerAddress | string | v | consumer address | | signature | string | v | signed message (consumerAddress + nonce + command) | -| nonce | string | v | request nonce | +| nonce | string | v | request nonce | #### Response (200) @@ -1776,11 +1870,11 @@ Upload a file to a bucket. The request body is treated as raw bytes. #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | consumer address | +| name | type | required | description | +| --------------- | ------ | -------- | -------------------------------------------------- | +| consumerAddress | string | v | consumer address | | signature | string | v | signed message (consumerAddress + nonce + command) | -| nonce | string | v | request nonce | +| nonce | string | v | request nonce | #### Request Body @@ -1807,12 +1901,12 @@ Delete a file from a bucket. #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | consumer address | +| name | type | required | description | +| --------------- | ------ | -------- | -------------------------------------------------- | +| consumerAddress | string | v | consumer address | | signature | string | v | signed message (consumerAddress + nonce + command) | -| nonce | string | v | request nonce | -| chainId | number | v | chain id (used by auth/signature checks) | +| nonce | string | v | request nonce | +| chainId | number | v | chain id (used by auth/signature checks) | #### Response (200) @@ -1847,37 +1941,37 @@ charged to the authenticated `consumerAddress`. Operator-published blueprint. Secret `envVars` values are never returned — only their keys via `envVarKeys`. -| property | type | description | -| ------------------------- | ----------------------------- | ----------- | -| id | string | template id (`[a-z0-9][a-z0-9_-]{0,63}`) | -| name / description | string | human-readable labels | -| image | string | base image | -| tag / checksum / dockerfile | string | image spec — exactly one | -| exposedPorts | number[] | container ports to forward | -| envVarKeys | string[] | keys of operator-set env vars (values never returned) | -| userConfigurableEnvVars | object[] | `{ key, validation?, sensitive? }` passed via `userData` | -| command / entrypoint | string[] | Docker CMD / ENTRYPOINT overrides | -| requiredResources | object[] | resources the service MUST have to run | -| recommendedResources | object[] | resources for best performance | +| property | type | description | +| --------------------------- | -------- | -------------------------------------------------------- | +| id | string | template id (`[a-z0-9][a-z0-9_-]{0,63}`) | +| name / description | string | human-readable labels | +| image | string | base image | +| tag / checksum / dockerfile | string | image spec — exactly one | +| exposedPorts | number[] | container ports to forward | +| envVarKeys | string[] | keys of operator-set env vars (values never returned) | +| userConfigurableEnvVars | object[] | `{ key, validation?, sensitive? }` passed via `userData` | +| command / entrypoint | string[] | Docker CMD / ENTRYPOINT overrides | +| requiredResources | object[] | resources the service MUST have to run | +| recommendedResources | object[] | resources for best performance | #### `ServiceJob` (returned by start / status / extend / restart / stop) The encrypted `userData` is never returned. Key fields: -| property | type | description | -| ------------- | -------- | ----------- | -| serviceId | string | unique id of the running service | -| environment | string | envId the service runs on | -| owner | string | consumerAddress | -| status | number | `10` Starting, `20` Locking, `11` PullImage, `13` BuildImage, `30` Claiming, `40` Running, `12` PullImageFailed, `14` BuildImageFailed, `15` VulnerableImage, `50` Stopping, `70` Stopped, `75` Expired, `99` Error | -| statusText | string | human-readable status | -| dateCreated | string | ISO timestamp | -| expiresAt | number | Unix ms timestamp when the paid window ends | -| duration | number | requested seconds | -| endpoints | object[] | `{ containerPort, hostPort, url }` per exposed port | -| resources | object[] | `{ id, amount, price }` | -| payment | object | initial start payment record | -| extendPayments | object[] | one entry per successful extend | +| property | type | description | +| -------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| serviceId | string | unique id of the running service | +| environment | string | envId the service runs on | +| owner | string | consumerAddress | +| status | number | `10` Starting, `20` Locking, `11` PullImage, `13` BuildImage, `30` Claiming, `40` Running, `12` PullImageFailed, `14` BuildImageFailed, `15` VulnerableImage, `50` Stopping, `70` Stopped, `75` Expired, `99` Error | +| statusText | string | human-readable status | +| dateCreated | string | ISO timestamp | +| expiresAt | number | Unix ms timestamp when the paid window ends | +| duration | number | requested seconds | +| endpoints | object[] | `{ containerPort, hostPort, url }` per exposed port | +| resources | object[] | `{ id, amount, price }` | +| payment | object | initial start payment record | +| extendPayments | object[] | one entry per successful extend | --- @@ -1891,8 +1985,8 @@ List the operator-published service templates (sanitized). Not authenticated. #### Query Parameters -| name | type | required | description | -| ------- | ------ | -------- | ----------- | +| name | type | required | description | +| ------- | ------ | -------- | -------------------------------------------------- | | chainId | number | | filter to templates whose envs price on this chain | #### Response (200) @@ -1906,7 +2000,10 @@ List the operator-published service templates (sanitized). Not authenticated. "tag": "latest", "exposedPorts": [8888], "userConfigurableEnvVars": [{ "key": "JUPYTER_TOKEN", "sensitive": true }], - "requiredResources": [{ "id": "cpu", "min": 1 }, { "id": "ram", "min": 2 }] + "requiredResources": [ + { "id": "cpu", "min": 1 }, + { "id": "ram", "min": 2 } + ] } ] ``` @@ -1944,25 +2041,28 @@ should keep watching `serviceStatus`, not just stop once they first see `Running "image": "nginxinc/nginx-unprivileged", "tag": "alpine", "exposedPorts": [8080], - "resources": [{ "id": "cpu", "amount": 1 }, { "id": "ram", "amount": 1 }], + "resources": [ + { "id": "cpu", "amount": 1 }, + { "id": "ram", "amount": 1 } + ], "duration": 3600, "userData": "", "payment": { "chainId": 8996, "token": "0x..." } } ``` -| field | type | required | description | -| --------------------- | -------- | -------- | ----------- | -| environment | string | v | envId to run on (services must be enabled on it) | -| image | string | v | base image | -| tag / checksum / dockerfile | string | | image spec — at most one; `dockerfile` requires `allowImageBuild` | -| additionalDockerFiles | object | | filename → content; only with `dockerfile` | -| dockerCmd / dockerEntrypoint | string[] | | container CMD / ENTRYPOINT overrides | -| exposedPorts | number[] | | container ports to publish | -| resources | object[] | | `{ id, amount }` requested resources | -| duration | number | v | seconds; capped by `serviceOnDemand.maxDurationSeconds` | -| userData | string | | ECIES-encrypted (to the node pubkey) JSON of env vars | -| payment | object | v | `{ chainId, token }` | +| field | type | required | description | +| ---------------------------- | -------- | -------- | ----------------------------------------------------------------- | +| environment | string | v | envId to run on (services must be enabled on it) | +| image | string | v | base image | +| tag / checksum / dockerfile | string | | image spec — at most one; `dockerfile` requires `allowImageBuild` | +| additionalDockerFiles | object | | filename → content; only with `dockerfile` | +| dockerCmd / dockerEntrypoint | string[] | | container CMD / ENTRYPOINT overrides | +| exposedPorts | number[] | | container ports to publish | +| resources | object[] | | `{ id, amount }` requested resources | +| duration | number | v | seconds; capped by `serviceOnDemand.maxDurationSeconds` | +| userData | string | | ECIES-encrypted (to the node pubkey) JSON of env vars | +| payment | object | v | `{ chainId, token }` | #### Response (200) @@ -2002,12 +2102,12 @@ by the authenticated `consumerAddress` are returned. #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | owner address | -| nonce | string | v | request nonce | +| name | type | required | description | +| --------------- | ------ | -------- | ------------------------------------------------------------ | +| consumerAddress | string | v | owner address | +| nonce | string | v | request nonce | | signature | string | v | signed message (or use an `Authorization` auth-token header) | -| serviceId | string | | filter to a single service; omit to list all owned services | +| serviceId | string | | filter to a single service; omit to list all owned services | #### Response (200) @@ -2030,14 +2130,14 @@ shared pools): `Running`/`Restarting`/`Stopping`, the mid-start pipeline states, #### Query Parameters -| name | type | required | description | -| ----------------- | ------- | -------- | ----------- | -| consumerAddress | string | v | caller identity (any consumer) | -| nonce | string | v | request nonce | -| signature | string | v | signed message (or use an `Authorization` auth-token header) | -| status | number | | filter to ONE specific `ServiceStatusNumber` (any status, incl. `75` Expired); takes precedence over `includeAllStatuses` | -| includeAllStatuses | boolean | | `true` returns services in every status instead of only the resource-holding set | -| fromTimestamp | string | | only services created at/after this moment — ISO date (`2026-01-15T00:00:00Z`) or Unix timestamp (seconds or milliseconds) | +| name | type | required | description | +| ------------------ | ------- | -------- | -------------------------------------------------------------------------------------------------------------------------- | +| consumerAddress | string | v | caller identity (any consumer) | +| nonce | string | v | request nonce | +| signature | string | v | signed message (or use an `Authorization` auth-token header) | +| status | number | | filter to ONE specific `ServiceStatusNumber` (any status, incl. `75` Expired); takes precedence over `includeAllStatuses` | +| includeAllStatuses | boolean | | `true` returns services in every status instead of only the resource-holding set | +| fromTimestamp | string | | only services created at/after this moment — ISO date (`2026-01-15T00:00:00Z`) or Unix timestamp (seconds or milliseconds) | #### Response (200) @@ -2139,17 +2239,17 @@ RESPEC mode (restart on a new image spec — `image` required, plus at most one } ``` -| name | type | required | description | -| --- | --- | --- | --- | -| serviceId | string | v | the service to restart | -| image | string | RESPEC | base image name (build label when `dockerfile` is set). Required as soon as any container param is present | -| tag | string | | pull by `name:tag`; mutually exclusive with `checksum`/`dockerfile` | -| checksum | string | | pull by digest `sha256:<64 hex>`; mutually exclusive with `tag`/`dockerfile` | -| dockerfile | string | | build from an inline Dockerfile; requires `allowImageBuild` on the environment; mutually exclusive with `tag`/`checksum` | -| additionalDockerFiles | object | | extra `filename → content` files for the build context (only with `dockerfile`) | -| userData | string | | ECIES-encrypted (to the node public key) JSON → the container's env-var map | -| dockerCmd | string[] | | exact container command (Docker exec-form CMD override) | -| dockerEntrypoint | string[] | | container ENTRYPOINT override | +| name | type | required | description | +| --------------------- | -------- | -------- | ------------------------------------------------------------------------------------------------------------------------ | +| serviceId | string | v | the service to restart | +| image | string | RESPEC | base image name (build label when `dockerfile` is set). Required as soon as any container param is present | +| tag | string | | pull by `name:tag`; mutually exclusive with `checksum`/`dockerfile` | +| checksum | string | | pull by digest `sha256:<64 hex>`; mutually exclusive with `tag`/`dockerfile` | +| dockerfile | string | | build from an inline Dockerfile; requires `allowImageBuild` on the environment; mutually exclusive with `tag`/`checksum` | +| additionalDockerFiles | object | | extra `filename → content` files for the build context (only with `dockerfile`) | +| userData | string | | ECIES-encrypted (to the node public key) JSON → the container's env-var map | +| dockerCmd | string[] | | exact container command (Docker exec-form CMD override) | +| dockerEntrypoint | string[] | | container ENTRYPOINT override | #### Response (200) @@ -2208,12 +2308,12 @@ container is kept around until `stop`/`restart`, so its logs remain fetchable fo #### Query Parameters -| name | type | required | description | -| --------------- | ------ | -------- | ----------- | -| consumerAddress | string | v | owner address | -| nonce | string | v | request nonce | -| signature | string | v | signed message (or use an `Authorization` auth-token header) | -| serviceId | string | v | the service to stream logs for | +| name | type | required | description | +| --------------- | ------ | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| consumerAddress | string | v | owner address | +| nonce | string | v | request nonce | +| signature | string | v | signed message (or use an `Authorization` auth-token header) | +| serviceId | string | v | the service to stream logs for | | since | string | | lower time bound for returned logs. Either a Unix timestamp in seconds (e.g. `1735689600`), or a relative duration counted back from now (e.g. `30s`, `45m`, `2h`, `7d`). Omit to get the full history since container start, then follow live. | #### Response (200) diff --git a/docs/PolicyServer.md b/docs/PolicyServer.md index be49539c3..92ad2024f 100644 --- a/docs/PolicyServer.md +++ b/docs/PolicyServer.md @@ -134,3 +134,49 @@ Called whenever a new decrypt command is received by Ocean Node "policyServer": {} } ``` + +## Passthrough and caller identity + +`POST /api/services/PolicyServerPassthrough` lets a caller send an arbitrary payload +straight to the PolicyServer, and `POST /api/services/initializePSVerification` starts an +`initiate` flow. Both are **authenticated by Ocean Node**: the caller must supply either an +`Authorization` header carrying an auth token, or a `nonce` + `signature` pair, together +with `consumerAddress`. Unauthenticated requests get a `401` and never reach the +PolicyServer. + +Because the passthrough body is forwarded verbatim, Ocean Node **overwrites** the identity +fields after it has verified the caller. The payload the PolicyServer receives therefore +always carries: + +| field | set by | meaning | +| --------------- | ---------- | ------------------------------------------------------------------ | +| consumerAddress | Ocean Node | the address this node verified — trustworthy, not caller-controlled | +| authorization | Ocean Node | the caller's auth token, relayed as received | +| nonce | Ocean Node | the caller's nonce (already consumed by this node) | +| signature | Ocean Node | the caller's signature, so the PolicyServer can re-verify it | +| ddo | Ocean Node | the DDO resolved from `documentId`, or `null` if not found | +| nodeAddress | Ocean Node | the address of the node making the request | + +Everything else in the payload — including `action` — is caller-supplied and must be +treated as untrusted input. + +Each endpoint is its own command, and the command string is part of the signed message, so a +signature is scoped to one endpoint and cannot be replayed against the other: + +| endpoint | command | signed message | +| ------------------------------------- | -------------------------- | ----------------------------------------------------- | +| `/api/services/PolicyServerPassthrough` | `PolicyServerPassthrough` | `consumerAddress + nonce + "PolicyServerPassthrough"` | +| `/api/services/initializePSVerification` | `PolicyServerInitialize` | `consumerAddress + nonce + "PolicyServerInitialize"` | + +A PolicyServer can independently recompute and verify either one. Note that for +`initializePSVerification` the credentials arrive nested inside the `policyServer` object +rather than at the top level. + +> **A caller controls `action`.** A passthrough payload can claim `"action": "download"` or +> `"action": "startCompute"` and look much like the ones Ocean Node itself sends for those +> commands. The `consumerAddress` is trustworthy, but the action is not — do not grant a +> passthrough request the same authority as a node-initiated one. + +> **The caller's auth token leaves the node.** `authorization` is relayed to the +> PolicyServer so it can run its own checks, so `POLICY_SERVER_URL` should be an HTTPS +> endpoint the operator controls. diff --git a/src/@types/commands.ts b/src/@types/commands.ts index 358cab0f0..3a1bea1ca 100644 --- a/src/@types/commands.ts +++ b/src/@types/commands.ts @@ -323,6 +323,11 @@ export interface JobStatus { export interface PolicyServerPassthroughCommand extends Command { policyServerPassthrough?: any + // caller identity, verified by this node before anything is forwarded to the policy server. + // either the inherited "authorization" token, or consumerAddress + nonce + signature + consumerAddress?: string + nonce?: string + signature?: string } export interface PolicyServerInitializeCommand extends Command { @@ -330,6 +335,8 @@ export interface PolicyServerInitializeCommand extends Command { serviceId?: string consumerAddress?: string policyServer?: any + nonce?: string + signature?: string } export interface CreateAuthTokenCommand extends Command { diff --git a/src/components/core/handler/coreHandlersRegistry.ts b/src/components/core/handler/coreHandlersRegistry.ts index c9c322b25..5af02f38a 100644 --- a/src/components/core/handler/coreHandlersRegistry.ts +++ b/src/components/core/handler/coreHandlersRegistry.ts @@ -11,7 +11,10 @@ import { } from './ddoHandler.js' import { DownloadHandler } from './downloadHandler.js' import { FileInfoHandler } from './fileInfoHandler.js' -import { PolicyServerPassthroughHandler } from './policyServer.js' +import { + PolicyServerPassthroughHandler, + PolicyServerInitializeHandler +} from './policyServer.js' import { EncryptHandler, EncryptFileHandler } from './encryptHandler.js' import { FeesHandler } from './feesHandler.js' import { BaseHandler, CommandHandler } from './handler.js' @@ -129,6 +132,10 @@ export class CoreHandlersRegistry { PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, new PolicyServerPassthroughHandler(node) ) + this.registerCoreHandler( + PROTOCOL_COMMANDS.POLICY_SERVER_INITIALIZE, + new PolicyServerInitializeHandler(node) + ) this.registerCoreHandler(PROTOCOL_COMMANDS.VALIDATE_DDO, new ValidateDDOHandler(node)) this.registerCoreHandler( diff --git a/src/components/core/handler/policyServer.ts b/src/components/core/handler/policyServer.ts index 98aad2ef6..ca30ae23f 100644 --- a/src/components/core/handler/policyServer.ts +++ b/src/components/core/handler/policyServer.ts @@ -4,6 +4,7 @@ import { PolicyServerInitializeCommand } from '../../../@types/commands.js' import { Readable } from 'stream' +import { isAddress } from 'ethers' import { CommandHandler } from './handler.js' import { ValidateParams, @@ -20,7 +21,17 @@ export class PolicyServerPassthroughHandler extends CommandHandler { return buildInvalidRequestMessage( 'Invalid Request: missing policyServerPassthrough field!' ) - const validation = validateCommandParameters(command, []) // all optional? weird + // we inject fields into this object below, so it has to be an object + if (typeof command.policyServerPassthrough !== 'object') + return buildInvalidRequestMessage( + 'Invalid Request: "policyServerPassthrough" must be an object!' + ) + const validation = validateCommandParameters(command, ['consumerAddress']) + if (validation.valid && !isAddress(command.consumerAddress)) { + return buildInvalidRequestMessage( + 'Parameter : "consumerAddress" is not a valid web3 address' + ) + } return validation } @@ -29,6 +40,17 @@ export class PolicyServerPassthroughHandler extends CommandHandler { if (this.shouldDenyTaskHandling(validationResponse)) { return validationResponse } + // same auth contract as startCompute: an authorization token, or nonce + signature + const authValidationResponse = await this.validateTokenOrSignature( + task.authorization, + task.consumerAddress, + task.nonce, + task.signature, + task.command + ) + if (authValidationResponse.status.httpStatus !== 200) { + return authValidationResponse + } task.policyServerPassthrough.ddo = null // resolve DDO first try { @@ -41,6 +63,13 @@ export class PolicyServerPassthroughHandler extends CommandHandler { `PolicyServerPassthroughHandler: DDO not found for documentId ${task.policyServerPassthrough.documentId}: ${error.message}` ) } + // the passthrough payload is forwarded verbatim, so every identity field has to be + // (re)written here, after validation. otherwise a caller could forge consumerAddress + // and impersonate the typed actions (download, startCompute, ...) + task.policyServerPassthrough.consumerAddress = authValidationResponse.consumerAddress + task.policyServerPassthrough.authorization = task.authorization + task.policyServerPassthrough.nonce = task.nonce + task.policyServerPassthrough.signature = task.signature // policyServer check const policyServer = new PolicyServer() const policyStatus = await policyServer.passThrough(task.policyServerPassthrough) @@ -71,7 +100,12 @@ export class PolicyServerInitializeHandler extends CommandHandler { 'documentId', 'serviceId', 'consumerAddress' - ]) // all optional? weird + ]) + if (validation.valid && !isAddress(command.consumerAddress)) { + return buildInvalidRequestMessage( + 'Parameter : "consumerAddress" is not a valid web3 address' + ) + } return validation } @@ -80,6 +114,17 @@ export class PolicyServerInitializeHandler extends CommandHandler { if (this.shouldDenyTaskHandling(validationResponse)) { return validationResponse } + // same auth contract as startCompute: an authorization token, or nonce + signature + const authValidationResponse = await this.validateTokenOrSignature( + task.authorization, + task.consumerAddress, + task.nonce, + task.signature, + task.command + ) + if (authValidationResponse.status.httpStatus !== 200) { + return authValidationResponse + } // resolve DDO first try { const database = await this.getOceanNode().getDatabase() @@ -98,12 +143,19 @@ export class PolicyServerInitializeHandler extends CommandHandler { } // policyServer check const policyServer = new PolicyServer() + // forward the address this node actually verified, plus the caller credentials, + // so the policy server can run its own additional checks const policyStatus = await policyServer.initializePSVerification( task.documentId, ddo, task.serviceId, - task.consumerAddress, - task.policyServer + authValidationResponse.consumerAddress, + { + ...task.policyServer, + authorization: task.authorization, + nonce: task.nonce, + signature: task.signature + } ) if (!policyStatus.success) { return { diff --git a/src/components/httpRoutes/policyServer.ts b/src/components/httpRoutes/policyServer.ts index 8b2bf3350..1bbb0fa86 100644 --- a/src/components/httpRoutes/policyServer.ts +++ b/src/components/httpRoutes/policyServer.ts @@ -21,6 +21,10 @@ PolicyServerPassthroughRoute.post( const response = await new PolicyServerPassthroughHandler(req.oceanNode).handle({ command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, policyServerPassthrough: req.body.policyServerPassthrough, + consumerAddress: req.body.consumerAddress, + nonce: req.body.nonce, + signature: req.body.signature, + authorization: req.headers?.authorization, caller: req.caller }) if (response.stream) { @@ -49,11 +53,14 @@ PolicyServerPassthroughRoute.post( ) try { const response = await new PolicyServerInitializeHandler(req.oceanNode).handle({ - command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + command: PROTOCOL_COMMANDS.POLICY_SERVER_INITIALIZE, documentId: req.body.documentId, serviceId: req.body.serviceId, consumerAddress: req.body.consumerAddress, policyServer: req.body.policyServer, + nonce: req.body.nonce, + signature: req.body.signature, + authorization: req.headers?.authorization, caller: req.caller }) if (response.stream) { diff --git a/src/components/httpRoutes/validateCommands.ts b/src/components/httpRoutes/validateCommands.ts index f8bb365cb..df0d995f5 100644 --- a/src/components/httpRoutes/validateCommands.ts +++ b/src/components/httpRoutes/validateCommands.ts @@ -2,6 +2,7 @@ import { PROTOCOL_COMMANDS, SUPPORTED_PROTOCOL_COMMANDS } from '../../utils/cons import { P2PCommandResponse } from '../../@types/OceanNode.js' import { Command } from '../../@types/commands.js' import { CORE_LOGGER } from '../../utils/logging/common.js' +import { isDefined } from '../../utils/util.js' import { ReadableString } from '../P2P/handlers.js' export type ValidateParams = { @@ -10,6 +11,24 @@ export type ValidateParams = { status?: number } +// credentials present on (almost) any command. these must never reach the logs, on any +// command, since they are what authorizes the request in the first place +const SENSITIVE_COMMAND_FIELDS = [ + 'authorization', // auth token (JWT), usually taken from the Authorization header + 'signature', // consumer signature authorizing this command + 'aes_encrypted_key', // download: encrypted key material + 'encryptedDockerRegistryAuth' // compute: encrypted docker registry credentials +] + +// "token" is an ERC20 address on most commands (escrow, compute payment) and only a +// credential on the auth-token commands, so it is redacted per-command instead +const SENSITIVE_TOKEN_COMMANDS: string[] = [ + PROTOCOL_COMMANDS.INVALIDATE_AUTH_TOKEN, + PROTOCOL_COMMANDS.VALIDATE_AUTH_TOKEN +] + +const REDACTED = '[REDACTED]' + // add others when we add suppor // request level validation, just check if we have a "command" field and its a supported one @@ -56,6 +75,16 @@ export function validateCommandParameters( logCommandData.rawData = [] } + // never log the caller's credentials, whatever the command is + for (const field of SENSITIVE_COMMAND_FIELDS) { + if (isDefined(logCommandData[field])) { + logCommandData[field] = REDACTED + } + } + if (SENSITIVE_TOKEN_COMMANDS.includes(commandStr) && isDefined(logCommandData.token)) { + logCommandData.token = REDACTED + } + CORE_LOGGER.info( `Checking received command data for Command "${commandStr}": ${JSON.stringify( logCommandData, diff --git a/src/test/unit/policyServer.test.ts b/src/test/unit/policyServer.test.ts new file mode 100644 index 000000000..7ff9abbe9 --- /dev/null +++ b/src/test/unit/policyServer.test.ts @@ -0,0 +1,348 @@ +import { assert, expect } from 'chai' +import sinon from 'sinon' +import { PROTOCOL_COMMANDS } from '../../utils/constants.js' +import { PolicyServer } from '../../components/policyServer/index.js' +import { + PolicyServerPassthroughHandler, + PolicyServerInitializeHandler +} from '../../components/core/handler/policyServer.js' + +const CONSUMER = '0x0000000000000000000000000000000000000abc' +const VICTIM = '0x0000000000000000000000000000000000000def' +const TOKEN = 'eyJhbGciOiJIUzI1NiJ9.fake.token' +const DDO = { id: 'did:op:1234', nftAddress: '0xnft' } + +interface FakeOpts { + // what Auth.validateAuthenticationOrToken resolves to + authResult?: any + // false => OceanNode.getAuth() returns undefined (auth component not wired up) + authConfigured?: boolean + ddo?: any +} + +function buildFakes(opts: FakeOpts = {}) { + const validateAuthenticationOrToken = sinon + .stub() + .resolves(opts.authResult ?? { valid: true, error: '', address: CONSUMER }) + + const retrieve = sinon.stub().resolves(opts.ddo === undefined ? DDO : opts.ddo) + + const node: any = { + getRequestMap: () => new Map(), + getConfig: (): any => ({ rateLimit: undefined as number | undefined }), + getDatabase: () => Promise.resolve({ ddo: { retrieve } }), + getAuth: () => + opts.authConfigured === false ? undefined : { validateAuthenticationOrToken } + } + + return { node, validateAuthenticationOrToken, retrieve } +} + +function passthroughTask(overrides: any = {}) { + return { + command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + policyServerPassthrough: { action: 'newDDO', documentId: DDO.id }, + consumerAddress: CONSUMER, + nonce: '1', + signature: '0xsignature', + ...overrides + } +} + +function initializeTask(overrides: any = {}) { + return { + command: PROTOCOL_COMMANDS.POLICY_SERVER_INITIALIZE, + documentId: DDO.id, + serviceId: 'service-1', + consumerAddress: CONSUMER, + policyServer: { some: 'blob' }, + nonce: '1', + signature: '0xsignature', + ...overrides + } +} + +describe('PolicyServerPassthroughHandler', () => { + afterEach(() => sinon.restore()) + + describe('parameter validation (runs before auth)', () => { + it('rejects a missing policyServerPassthrough field (400)', async () => { + const { node, validateAuthenticationOrToken } = buildFakes() + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ policyServerPassthrough: undefined }) + ) + expect(response.status.httpStatus).to.equal(400) + expect(response.status.error).to.contain('missing policyServerPassthrough') + // validation must short-circuit before we ever touch the auth component + assert(validateAuthenticationOrToken.notCalled, 'auth should not be reached') + }) + + it('rejects a non-object policyServerPassthrough (400)', async () => { + const { node } = buildFakes() + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ policyServerPassthrough: 'not-an-object' }) + ) + expect(response.status.httpStatus).to.equal(400) + expect(response.status.error).to.contain('must be an object') + }) + + it('rejects a missing consumerAddress (400)', async () => { + const { node } = buildFakes() + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ consumerAddress: undefined }) + ) + expect(response.status.httpStatus).to.equal(400) + expect(response.status.error).to.contain('consumerAddress') + }) + + it('rejects a malformed consumerAddress (400)', async () => { + const { node } = buildFakes() + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ consumerAddress: 'not-an-address' }) + ) + expect(response.status.httpStatus).to.equal(400) + expect(response.status.error).to.contain('not a valid web3 address') + }) + }) + + describe('authentication', () => { + it('rejects an unauthenticated request (401)', async () => { + const { node } = buildFakes({ + authResult: { + valid: false, + error: + 'Invalid authentication, you need to provide either a token or an address, signature, message and nonce' + } + }) + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough') + + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ nonce: undefined, signature: undefined }) + ) + + expect(response.status.httpStatus).to.equal(401) + assert(passThrough.notCalled, 'must not reach the policy server') + }) + + it('rejects an invalid signature (401)', async () => { + const { node } = buildFakes({ + authResult: { valid: false, error: 'Invalid signature' } + }) + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough') + + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask() + ) + + expect(response.status.httpStatus).to.equal(401) + expect(response.status.error).to.equal('Invalid signature') + assert(passThrough.notCalled, 'must not reach the policy server') + }) + + it('returns 401 when the auth component is not configured', async () => { + const { node } = buildFakes({ authConfigured: false }) + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask() + ) + expect(response.status.httpStatus).to.equal(401) + expect(response.status.error).to.equal('Auth not configured') + }) + + it('passes the auth header and the nonce/signature triple to the auth component', async () => { + const { node, validateAuthenticationOrToken } = buildFakes() + sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ + success: true, + message: 'ok', + httpStatus: 200 + }) + + await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ authorization: TOKEN }) + ) + + const args = validateAuthenticationOrToken.firstCall.args[0] + expect(args.token).to.equal(TOKEN) + expect(args.address).to.equal(CONSUMER) + expect(args.nonce).to.equal('1') + expect(args.signature).to.equal('0xsignature') + // the signed message is scoped to the command string + expect(args.command).to.equal(PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH) + }) + }) + + describe('forwarding to the policy server', () => { + it('forwards the verified consumerAddress and the caller credentials', async () => { + const { node } = buildFakes() + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ + success: true, + message: 'ok', + httpStatus: 200 + }) + + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ authorization: TOKEN }) + ) + + expect(response.status.httpStatus).to.equal(200) + const forwarded = passThrough.firstCall.args[0] + expect(forwarded.consumerAddress).to.equal(CONSUMER) + expect(forwarded.authorization).to.equal(TOKEN) + expect(forwarded.nonce).to.equal('1') + expect(forwarded.signature).to.equal('0xsignature') + // the node still resolves the DDO server-side + expect(forwarded.ddo).to.deep.equal(DDO) + // and the caller's own payload is preserved + expect(forwarded.action).to.equal('newDDO') + }) + + it('overwrites a forged consumerAddress inside the payload with the verified one', async () => { + // auth succeeds for CONSUMER, but the caller tries to pass VICTIM off as the consumer + const { node } = buildFakes() + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ + success: true, + message: 'ok', + httpStatus: 200 + }) + + await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ + policyServerPassthrough: { + action: 'download', + documentId: DDO.id, + consumerAddress: VICTIM + } + }) + ) + + expect(passThrough.firstCall.args[0].consumerAddress).to.equal(CONSUMER) + }) + + it('uses the address the token resolves to, not the one the caller claims', async () => { + // token path: Auth returns the address encoded in the JWT + const { node } = buildFakes({ + authResult: { valid: true, error: '', address: CONSUMER } + }) + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ + success: true, + message: 'ok', + httpStatus: 200 + }) + + await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ consumerAddress: VICTIM, authorization: TOKEN }) + ) + + expect(passThrough.firstCall.args[0].consumerAddress).to.equal(CONSUMER) + }) + + it('forwards even when the DDO cannot be resolved (ddo stays null)', async () => { + const { node } = buildFakes({ ddo: null }) + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ + success: true, + message: 'ok', + httpStatus: 200 + }) + + await new PolicyServerPassthroughHandler(node).handle(passthroughTask()) + + expect(passThrough.firstCall.args[0].ddo).to.equal(null) + expect(passThrough.firstCall.args[0].consumerAddress).to.equal(CONSUMER) + }) + + it('propagates a policy server denial', async () => { + const { node } = buildFakes() + sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ + success: false, + message: 'denied by policy', + httpStatus: 403 + }) + + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask() + ) + + expect(response.status.httpStatus).to.equal(403) + expect(response.status.error).to.equal('denied by policy') + expect(response.stream).to.equal(null) + }) + }) +}) + +describe('PolicyServerInitializeHandler', () => { + afterEach(() => sinon.restore()) + + it('rejects a malformed consumerAddress (400)', async () => { + const { node } = buildFakes() + const response = await new PolicyServerInitializeHandler(node).handle( + initializeTask({ consumerAddress: 'not-an-address' }) + ) + expect(response.status.httpStatus).to.equal(400) + expect(response.status.error).to.contain('not a valid web3 address') + }) + + it('scopes the signature to its own command, not to PolicyServerPassthrough', async () => { + const { node, validateAuthenticationOrToken } = buildFakes() + sinon + .stub(PolicyServer.prototype, 'initializePSVerification') + .resolves({ success: true, message: 'ok', httpStatus: 200 }) + + await new PolicyServerInitializeHandler(node).handle(initializeTask()) + + expect(validateAuthenticationOrToken.firstCall.args[0].command).to.equal( + PROTOCOL_COMMANDS.POLICY_SERVER_INITIALIZE + ) + expect(validateAuthenticationOrToken.firstCall.args[0].command).to.not.equal( + PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH + ) + }) + + it('rejects an unauthenticated request (401)', async () => { + const { node } = buildFakes({ + authResult: { valid: false, error: 'Invalid signature' } + }) + const initialize = sinon.stub(PolicyServer.prototype, 'initializePSVerification') + + const response = await new PolicyServerInitializeHandler(node).handle( + initializeTask() + ) + + expect(response.status.httpStatus).to.equal(401) + assert(initialize.notCalled, 'must not reach the policy server') + }) + + it('forwards the verified consumerAddress and the caller credentials', async () => { + const { node } = buildFakes() + const initialize = sinon + .stub(PolicyServer.prototype, 'initializePSVerification') + .resolves({ success: true, message: 'ok', httpStatus: 200 }) + + const response = await new PolicyServerInitializeHandler(node).handle( + initializeTask({ consumerAddress: VICTIM, authorization: TOKEN }) + ) + + expect(response.status.httpStatus).to.equal(200) + const [documentId, ddo, serviceId, consumerAddress, policyServer] = + initialize.firstCall.args + expect(documentId).to.equal(DDO.id) + expect(ddo).to.deep.equal(DDO) + expect(serviceId).to.equal('service-1') + // verified address wins over the one the caller supplied + expect(consumerAddress).to.equal(CONSUMER) + expect(policyServer.some).to.equal('blob') + expect(policyServer.authorization).to.equal(TOKEN) + expect(policyServer.nonce).to.equal('1') + expect(policyServer.signature).to.equal('0xsignature') + }) + + it('returns 404 when the DDO does not exist', async () => { + const { node } = buildFakes({ ddo: null }) + const initialize = sinon.stub(PolicyServer.prototype, 'initializePSVerification') + + const response = await new PolicyServerInitializeHandler(node).handle( + initializeTask() + ) + + expect(response.status.httpStatus).to.equal(404) + assert(initialize.notCalled, 'must not reach the policy server') + }) +}) diff --git a/src/test/unit/validateCommands.test.ts b/src/test/unit/validateCommands.test.ts new file mode 100644 index 000000000..5d025fd19 --- /dev/null +++ b/src/test/unit/validateCommands.test.ts @@ -0,0 +1,124 @@ +import { expect, assert } from 'chai' +import sinon from 'sinon' +import { PROTOCOL_COMMANDS } from '../../utils/constants.js' +import { CORE_LOGGER } from '../../utils/logging/common.js' +import { validateCommandParameters } from '../../components/httpRoutes/validateCommands.js' + +const TOKEN = 'eyJhbGciOiJIUzI1NiJ9.fake.token' +const SIGNATURE = '0xdeadbeefsignature' +const ERC20 = '0x0000000000000000000000000000000000000abc' + +// validateCommandParameters logs the whole command for every request, so anything that +// authorizes the request must be redacted before it reaches the logs +describe('validateCommandParameters credential redaction', () => { + let info: sinon.SinonStub + + beforeEach(() => { + info = sinon.stub(CORE_LOGGER, 'info') + }) + afterEach(() => sinon.restore()) + + function loggedCommand(): string { + assert(info.called, 'the command should have been logged') + return info.firstCall.args[0] as string + } + + it('redacts the authorization token, signature and encrypted key material', () => { + const command = { + command: PROTOCOL_COMMANDS.DOWNLOAD, + documentId: 'did:op:1234', + consumerAddress: ERC20, + nonce: '1', + authorization: TOKEN, + signature: SIGNATURE, + aes_encrypted_key: 'secret-key-material' + } + + const validation = validateCommandParameters(command, []) + expect(validation.valid).to.equal(true) + + const logged = loggedCommand() + expect(logged).to.not.contain(TOKEN) + expect(logged).to.not.contain(SIGNATURE) + expect(logged).to.not.contain('secret-key-material') + expect(logged).to.contain('[REDACTED]') + // non-sensitive fields are still logged, so the logs stay useful + expect(logged).to.contain('did:op:1234') + expect(logged).to.contain('"nonce": "1"') + }) + + it('redacts encryptedDockerRegistryAuth on compute commands', () => { + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.COMPUTE_START, + consumerAddress: ERC20, + encryptedDockerRegistryAuth: 'registry-secret' + }, + [] + ) + expect(loggedCommand()).to.not.contain('registry-secret') + }) + + it('redacts the auth token on invalidateAuthToken', () => { + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.INVALIDATE_AUTH_TOKEN, + address: ERC20, + nonce: '1', + token: TOKEN + }, + [] + ) + expect(loggedCommand()).to.not.contain(TOKEN) + }) + + it('redacts the auth token on validateAuthToken', () => { + validateCommandParameters( + { command: PROTOCOL_COMMANDS.VALIDATE_AUTH_TOKEN, token: TOKEN }, + [] + ) + expect(loggedCommand()).to.not.contain(TOKEN) + }) + + it('keeps "token" readable where it is an ERC20 address, not a credential', () => { + validateCommandParameters( + { command: PROTOCOL_COMMANDS.GET_ESCROW_EVENTS, chainId: 8996, token: ERC20 }, + [] + ) + expect(loggedCommand()).to.contain(ERC20) + }) + + it('does not mutate the command it was given', () => { + const command = { + command: PROTOCOL_COMMANDS.DOWNLOAD, + authorization: TOKEN, + signature: SIGNATURE + } + validateCommandParameters(command, []) + // redaction happens on a copy - handlers still need the real credentials + expect(command.authorization).to.equal(TOKEN) + expect(command.signature).to.equal(SIGNATURE) + }) + + it('redacts even when the command carries a non-cloneable stream', () => { + // forces the shallow-clone fallback path (structuredClone throws on functions) + const command: any = { + command: PROTOCOL_COMMANDS.DOWNLOAD, + authorization: TOKEN, + notCloneable: () => 'boom' + } + validateCommandParameters(command, []) + expect(loggedCommand()).to.not.contain(TOKEN) + expect(command.authorization).to.equal(TOKEN) + }) + + it('still reports missing required fields', () => { + const validation = validateCommandParameters( + { command: PROTOCOL_COMMANDS.DOWNLOAD, authorization: TOKEN }, + ['documentId'] + ) + expect(validation.valid).to.equal(false) + expect(validation.status).to.equal(400) + expect(validation.reason).to.contain('documentId') + }) +}) diff --git a/src/utils/constants.ts b/src/utils/constants.ts index 507b1b1bb..94e5784bb 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -30,6 +30,7 @@ export const PROTOCOL_COMMANDS = { HANDLE_INDEXING_THREAD: 'handleIndexingThread', COLLECT_FEES: 'collectFees', POLICY_SERVER_PASSTHROUGH: 'PolicyServerPassthrough', + POLICY_SERVER_INITIALIZE: 'PolicyServerInitialize', GET_P2P_PEER: 'getP2PPeer', GET_P2P_PEERS: 'getP2PPeers', GET_P2P_NETWORK_STATS: 'getP2PNetworkStats', @@ -90,6 +91,7 @@ export const SUPPORTED_PROTOCOL_COMMANDS: string[] = [ PROTOCOL_COMMANDS.HANDLE_INDEXING_THREAD, PROTOCOL_COMMANDS.COLLECT_FEES, PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + PROTOCOL_COMMANDS.POLICY_SERVER_INITIALIZE, PROTOCOL_COMMANDS.GET_P2P_PEER, PROTOCOL_COMMANDS.GET_P2P_PEERS, PROTOCOL_COMMANDS.GET_P2P_NETWORK_STATS, From 28d3b2d67ca3d857dc2a1957f1ede031d8bfbf19 Mon Sep 17 00:00:00 2001 From: alexcos20 Date: Mon, 17 Aug 2026 11:32:01 +0300 Subject: [PATCH 2/3] fix --- src/components/core/handler/policyServer.ts | 8 +- src/components/httpRoutes/validateCommands.ts | 62 ++++++++++-- src/test/unit/policyServer.test.ts | 15 +++ src/test/unit/validateCommands.test.ts | 98 +++++++++++++++++++ 4 files changed, 172 insertions(+), 11 deletions(-) diff --git a/src/components/core/handler/policyServer.ts b/src/components/core/handler/policyServer.ts index ca30ae23f..a80dd3c2d 100644 --- a/src/components/core/handler/policyServer.ts +++ b/src/components/core/handler/policyServer.ts @@ -21,8 +21,12 @@ export class PolicyServerPassthroughHandler extends CommandHandler { return buildInvalidRequestMessage( 'Invalid Request: missing policyServerPassthrough field!' ) - // we inject fields into this object below, so it has to be an object - if (typeof command.policyServerPassthrough !== 'object') + // we inject fields into this object below, so it has to be a keyed object. arrays are + // objects too, and would be forwarded as {"0":..,"1":..} with no action + if ( + typeof command.policyServerPassthrough !== 'object' || + Array.isArray(command.policyServerPassthrough) + ) return buildInvalidRequestMessage( 'Invalid Request: "policyServerPassthrough" must be an object!' ) diff --git a/src/components/httpRoutes/validateCommands.ts b/src/components/httpRoutes/validateCommands.ts index df0d995f5..0b9055b08 100644 --- a/src/components/httpRoutes/validateCommands.ts +++ b/src/components/httpRoutes/validateCommands.ts @@ -29,6 +29,51 @@ const SENSITIVE_TOKEN_COMMANDS: string[] = [ const REDACTED = '[REDACTED]' +/** + * Returns a copy of the payload with every credential field redacted, at any depth. + * + * This must not mutate its input: the clone the caller hands us can be a *shallow* copy + * (the fallback path below), so nested objects are still shared with the real command and + * the handlers still need the actual credentials. So we rebuild containers instead of + * writing into them. + * + * Only plain objects and arrays are walked - Buffers, typed arrays, Dates, streams and the + * like are passed through untouched. + */ +function redactSensitiveFields( + value: any, + redactToken: boolean, + seen: WeakSet +): any { + if (value === null || typeof value !== 'object') { + return value + } + if (seen.has(value)) { + return '[CIRCULAR]' + } + if (Array.isArray(value)) { + seen.add(value) + const copy = value.map((item) => redactSensitiveFields(item, redactToken, seen)) + seen.delete(value) + return copy + } + const proto = Object.getPrototypeOf(value) + if (proto !== Object.prototype && proto !== null) { + return value + } + seen.add(value) + const copy: any = {} + for (const [key, item] of Object.entries(value)) { + if (SENSITIVE_COMMAND_FIELDS.includes(key) || (redactToken && key === 'token')) { + copy[key] = isDefined(item) ? REDACTED : item + } else { + copy[key] = redactSensitiveFields(item, redactToken, seen) + } + } + seen.delete(value) + return copy +} + // add others when we add suppor // request level validation, just check if we have a "command" field and its a supported one @@ -75,15 +120,14 @@ export function validateCommandParameters( logCommandData.rawData = [] } - // never log the caller's credentials, whatever the command is - for (const field of SENSITIVE_COMMAND_FIELDS) { - if (isDefined(logCommandData[field])) { - logCommandData[field] = REDACTED - } - } - if (SENSITIVE_TOKEN_COMMANDS.includes(commandStr) && isDefined(logCommandData.token)) { - logCommandData.token = REDACTED - } + // never log the caller's credentials, whatever the command is. credentials also show up + // nested (the free-form "policyServer" / "policyServerPassthrough" blobs), so this walks + // the whole payload + logCommandData = redactSensitiveFields( + logCommandData, + SENSITIVE_TOKEN_COMMANDS.includes(commandStr), + new WeakSet() + ) CORE_LOGGER.info( `Checking received command data for Command "${commandStr}": ${JSON.stringify( diff --git a/src/test/unit/policyServer.test.ts b/src/test/unit/policyServer.test.ts index 7ff9abbe9..05709cfa7 100644 --- a/src/test/unit/policyServer.test.ts +++ b/src/test/unit/policyServer.test.ts @@ -86,6 +86,21 @@ describe('PolicyServerPassthroughHandler', () => { expect(response.status.error).to.contain('must be an object') }) + it('rejects an array policyServerPassthrough (400)', async () => { + // typeof [] === 'object', so an array would otherwise be forwarded as + // {"0":..,"1":..} with no action + const { node } = buildFakes() + const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough') + + const response = await new PolicyServerPassthroughHandler(node).handle( + passthroughTask({ policyServerPassthrough: ['a', 'b'] }) + ) + + expect(response.status.httpStatus).to.equal(400) + expect(response.status.error).to.contain('must be an object') + assert(passThrough.notCalled, 'must not reach the policy server') + }) + it('rejects a missing consumerAddress (400)', async () => { const { node } = buildFakes() const response = await new PolicyServerPassthroughHandler(node).handle( diff --git a/src/test/unit/validateCommands.test.ts b/src/test/unit/validateCommands.test.ts index 5d025fd19..adae4af85 100644 --- a/src/test/unit/validateCommands.test.ts +++ b/src/test/unit/validateCommands.test.ts @@ -88,6 +88,104 @@ describe('validateCommandParameters credential redaction', () => { expect(loggedCommand()).to.contain(ERC20) }) + it('redacts credentials nested inside the policyServerPassthrough blob', () => { + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + consumerAddress: ERC20, + policyServerPassthrough: { + action: 'download', + documentId: 'did:op:1234', + authorization: TOKEN, + signature: SIGNATURE + } + }, + [] + ) + const logged = loggedCommand() + expect(logged).to.not.contain(TOKEN) + expect(logged).to.not.contain(SIGNATURE) + // the rest of the blob is still logged + expect(logged).to.contain('did:op:1234') + expect(logged).to.contain('download') + }) + + it('redacts credentials nested inside the policyServer blob', () => { + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.POLICY_SERVER_INITIALIZE, + documentId: 'did:op:1234', + serviceId: '0', + consumerAddress: ERC20, + policyServer: { authorization: TOKEN, signature: SIGNATURE } + }, + [] + ) + const logged = loggedCommand() + expect(logged).to.not.contain(TOKEN) + expect(logged).to.not.contain(SIGNATURE) + }) + + it('redacts credentials nested arbitrarily deep, including inside arrays', () => { + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + consumerAddress: ERC20, + policyServerPassthrough: { + nested: { deeper: [{ authorization: TOKEN }, { signature: SIGNATURE }] } + } + }, + [] + ) + const logged = loggedCommand() + expect(logged).to.not.contain(TOKEN) + expect(logged).to.not.contain(SIGNATURE) + }) + + it('does not mutate nested objects it shares with the caller', () => { + // the shallow-clone fallback path shares nested references with the original, so + // redaction must never write into them - handlers still need the real credentials + const nested = { action: 'download', authorization: TOKEN, signature: SIGNATURE } + const command: any = { + command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + consumerAddress: ERC20, + policyServerPassthrough: nested, + notCloneable: () => 'boom' // forces the shallow-clone fallback + } + validateCommandParameters(command, []) + expect(loggedCommand()).to.not.contain(TOKEN) + expect(nested.authorization).to.equal(TOKEN) + expect(nested.signature).to.equal(SIGNATURE) + }) + + it('survives a circular payload', () => { + const blob: any = { authorization: TOKEN } + blob.self = blob + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, + consumerAddress: ERC20, + policyServerPassthrough: blob + }, + [] + ) + const logged = loggedCommand() + expect(logged).to.not.contain(TOKEN) + expect(logged).to.contain('[CIRCULAR]') + }) + + it('keeps "token" readable when nested as an ERC20 payment address', () => { + validateCommandParameters( + { + command: PROTOCOL_COMMANDS.COMPUTE_START, + consumerAddress: ERC20, + payment: { chainId: 8996, token: ERC20 } + }, + [] + ) + expect(loggedCommand()).to.contain(ERC20) + }) + it('does not mutate the command it was given', () => { const command = { command: PROTOCOL_COMMANDS.DOWNLOAD, From efb30ef54a919574a578aeabf9e256adec6c03f3 Mon Sep 17 00:00:00 2001 From: George Adrian Vitelus <49452066+AdriGeorge@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:20:45 +0300 Subject: [PATCH 3/3] Refactor PolicyServerPassthrough to remove authentication requirements and update related tests (#1450) --- docs/API.md | 43 ++----- docs/PolicyServer.md | 51 ++------ src/@types/commands.ts | 5 - src/components/core/handler/policyServer.ts | 26 +--- src/components/httpRoutes/policyServer.ts | 4 - src/test/unit/policyServer.test.ts | 129 ++------------------ 6 files changed, 32 insertions(+), 226 deletions(-) diff --git a/docs/API.md b/docs/API.md index 249e19cbe..e2ee4e873 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1206,41 +1206,22 @@ returns the current indexing queue, as an array of objects Forwards request to PolicyServer (if any). -This endpoint is authenticated. The caller must provide either an `Authorization` header -carrying an auth token, or a `nonce` + `signature` pair. `consumerAddress` is always required. Requests without valid credentials -are rejected with `401`. - -The node verifies the caller, then injects the **verified** `consumerAddress` (overwriting -any value present inside `policyServerPassthrough`), along with `authorization`, `nonce`, -`signature`, the resolved `ddo` and the node's own `nodeAddress`, into the payload it -forwards to the PolicyServer. - -#### Request Headers - -| name | required | description | -| ------------- | -------- | -------------------------------------------------------------- | -| Authorization | | auth token; required unless `nonce` + `signature` are supplied | +This endpoint is not authenticated. The node forwards the caller-supplied payload and adds +the resolved `ddo` plus its own `nodeAddress`. Caller-supplied identity fields are not +verified. #### Parameters -| name | type | required | description | -| ----------------------- | ------ | -------- | ---------------------------------------------------- | -| command | string | v | command name | -| node | string | | if not present it means current node | -| policyServerPassthrough | object | v | command and params for PolicyServer (see docs) | -| consumerAddress | string | v | the caller's address | -| nonce | string | | required unless an `Authorization` token is supplied | -| signature | string | | required unless an `Authorization` token is supplied | - -The signed message is `consumerAddress + nonce + "PolicyServerPassthrough"`. +| name | type | required | description | +| ----------------------- | ------ | -------- | ---------------------------------------------- | +| command | string | v | command name | +| node | string | | if not present it means current node | +| policyServerPassthrough | object | v | command and params for PolicyServer (see docs) | #### HTTP Example ```json { - "consumerAddress": "0x9876543210fedcba9876543210fedcba98765432", - "nonce": "1", - "signature": "0x123", "policyServerPassthrough": { "action": "newDDO", "rawDDO": {}, @@ -1257,9 +1238,6 @@ The signed message is `consumerAddress + nonce + "PolicyServerPassthrough"`. { "command": "PolicyServerPassthrough", "node": "PeerId", - "consumerAddress": "0x9876543210fedcba9876543210fedcba98765432", - "nonce": "1", - "signature": "0x123", "policyServerPassthrough": { "action": "newDDO", "rawDDO": {}, @@ -1276,7 +1254,6 @@ The signed message is `consumerAddress + nonce + "PolicyServerPassthrough"`. | ---- | --------------------------------------------------------------- | | 200 | PolicyServer allowed the request; its response body is returned | | 400 | missing/invalid parameters | -| 401 | missing or invalid authentication | --- @@ -1289,8 +1266,8 @@ The signed message is `consumerAddress + nonce + "PolicyServerPassthrough"`. #### Description Asks the PolicyServer to start a verification flow (`initiate` action) for a given -asset/service and consumer. Authenticated the same way as `PolicyServerPassthrough`: an -`Authorization` header or a `nonce` + `signature` pair. +asset/service and consumer. This endpoint requires an `Authorization` header or a +`nonce` + `signature` pair. This is a distinct command from `PolicyServerPassthrough`, so the signed message uses its own command string: `consumerAddress + nonce + "PolicyServerInitialize"`. diff --git a/docs/PolicyServer.md b/docs/PolicyServer.md index 92ad2024f..b458193ae 100644 --- a/docs/PolicyServer.md +++ b/docs/PolicyServer.md @@ -135,48 +135,21 @@ Called whenever a new decrypt command is received by Ocean Node } ``` -## Passthrough and caller identity +## Passthrough and initialization authentication `POST /api/services/PolicyServerPassthrough` lets a caller send an arbitrary payload -straight to the PolicyServer, and `POST /api/services/initializePSVerification` starts an -`initiate` flow. Both are **authenticated by Ocean Node**: the caller must supply either an -`Authorization` header carrying an auth token, or a `nonce` + `signature` pair, together -with `consumerAddress`. Unauthenticated requests get a `401` and never reach the -PolicyServer. - -Because the passthrough body is forwarded verbatim, Ocean Node **overwrites** the identity -fields after it has verified the caller. The payload the PolicyServer receives therefore -always carries: - -| field | set by | meaning | -| --------------- | ---------- | ------------------------------------------------------------------ | -| consumerAddress | Ocean Node | the address this node verified — trustworthy, not caller-controlled | -| authorization | Ocean Node | the caller's auth token, relayed as received | -| nonce | Ocean Node | the caller's nonce (already consumed by this node) | -| signature | Ocean Node | the caller's signature, so the PolicyServer can re-verify it | -| ddo | Ocean Node | the DDO resolved from `documentId`, or `null` if not found | -| nodeAddress | Ocean Node | the address of the node making the request | - -Everything else in the payload — including `action` — is caller-supplied and must be -treated as untrusted input. - -Each endpoint is its own command, and the command string is part of the signed message, so a -signature is scoped to one endpoint and cannot be replayed against the other: - -| endpoint | command | signed message | -| ------------------------------------- | -------------------------- | ----------------------------------------------------- | -| `/api/services/PolicyServerPassthrough` | `PolicyServerPassthrough` | `consumerAddress + nonce + "PolicyServerPassthrough"` | -| `/api/services/initializePSVerification` | `PolicyServerInitialize` | `consumerAddress + nonce + "PolicyServerInitialize"` | - -A PolicyServer can independently recompute and verify either one. Note that for -`initializePSVerification` the credentials arrive nested inside the `policyServer` object -rather than at the top level. +straight to the PolicyServer without authentication. Ocean Node adds the resolved `ddo` +and its own `nodeAddress`, but it does not verify or inject caller identity. Every field in +the passthrough payload, including `action` and `consumerAddress`, must therefore be treated +as untrusted input. + +`POST /api/services/initializePSVerification` remains authenticated. The caller supplies +either an `Authorization` header or a `nonce` + `signature` pair together with +`consumerAddress`. Its signed message is +`consumerAddress + nonce + "PolicyServerInitialize"`, and its credentials are forwarded +inside the `policyServer` object. > **A caller controls `action`.** A passthrough payload can claim `"action": "download"` or > `"action": "startCompute"` and look much like the ones Ocean Node itself sends for those -> commands. The `consumerAddress` is trustworthy, but the action is not — do not grant a +> commands. Neither the action nor caller identity is verified — do not grant a > passthrough request the same authority as a node-initiated one. - -> **The caller's auth token leaves the node.** `authorization` is relayed to the -> PolicyServer so it can run its own checks, so `POLICY_SERVER_URL` should be an HTTPS -> endpoint the operator controls. diff --git a/src/@types/commands.ts b/src/@types/commands.ts index 3a1bea1ca..83b24ca88 100644 --- a/src/@types/commands.ts +++ b/src/@types/commands.ts @@ -323,11 +323,6 @@ export interface JobStatus { export interface PolicyServerPassthroughCommand extends Command { policyServerPassthrough?: any - // caller identity, verified by this node before anything is forwarded to the policy server. - // either the inherited "authorization" token, or consumerAddress + nonce + signature - consumerAddress?: string - nonce?: string - signature?: string } export interface PolicyServerInitializeCommand extends Command { diff --git a/src/components/core/handler/policyServer.ts b/src/components/core/handler/policyServer.ts index a80dd3c2d..43100d611 100644 --- a/src/components/core/handler/policyServer.ts +++ b/src/components/core/handler/policyServer.ts @@ -30,13 +30,7 @@ export class PolicyServerPassthroughHandler extends CommandHandler { return buildInvalidRequestMessage( 'Invalid Request: "policyServerPassthrough" must be an object!' ) - const validation = validateCommandParameters(command, ['consumerAddress']) - if (validation.valid && !isAddress(command.consumerAddress)) { - return buildInvalidRequestMessage( - 'Parameter : "consumerAddress" is not a valid web3 address' - ) - } - return validation + return validateCommandParameters(command, []) } async handle(task: PolicyServerPassthroughCommand): Promise { @@ -44,17 +38,6 @@ export class PolicyServerPassthroughHandler extends CommandHandler { if (this.shouldDenyTaskHandling(validationResponse)) { return validationResponse } - // same auth contract as startCompute: an authorization token, or nonce + signature - const authValidationResponse = await this.validateTokenOrSignature( - task.authorization, - task.consumerAddress, - task.nonce, - task.signature, - task.command - ) - if (authValidationResponse.status.httpStatus !== 200) { - return authValidationResponse - } task.policyServerPassthrough.ddo = null // resolve DDO first try { @@ -67,13 +50,6 @@ export class PolicyServerPassthroughHandler extends CommandHandler { `PolicyServerPassthroughHandler: DDO not found for documentId ${task.policyServerPassthrough.documentId}: ${error.message}` ) } - // the passthrough payload is forwarded verbatim, so every identity field has to be - // (re)written here, after validation. otherwise a caller could forge consumerAddress - // and impersonate the typed actions (download, startCompute, ...) - task.policyServerPassthrough.consumerAddress = authValidationResponse.consumerAddress - task.policyServerPassthrough.authorization = task.authorization - task.policyServerPassthrough.nonce = task.nonce - task.policyServerPassthrough.signature = task.signature // policyServer check const policyServer = new PolicyServer() const policyStatus = await policyServer.passThrough(task.policyServerPassthrough) diff --git a/src/components/httpRoutes/policyServer.ts b/src/components/httpRoutes/policyServer.ts index 1bbb0fa86..74bfc5555 100644 --- a/src/components/httpRoutes/policyServer.ts +++ b/src/components/httpRoutes/policyServer.ts @@ -21,10 +21,6 @@ PolicyServerPassthroughRoute.post( const response = await new PolicyServerPassthroughHandler(req.oceanNode).handle({ command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, policyServerPassthrough: req.body.policyServerPassthrough, - consumerAddress: req.body.consumerAddress, - nonce: req.body.nonce, - signature: req.body.signature, - authorization: req.headers?.authorization, caller: req.caller }) if (response.stream) { diff --git a/src/test/unit/policyServer.test.ts b/src/test/unit/policyServer.test.ts index 05709cfa7..a8766cd95 100644 --- a/src/test/unit/policyServer.test.ts +++ b/src/test/unit/policyServer.test.ts @@ -42,9 +42,6 @@ function passthroughTask(overrides: any = {}) { return { command: PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH, policyServerPassthrough: { action: 'newDDO', documentId: DDO.id }, - consumerAddress: CONSUMER, - nonce: '1', - signature: '0xsignature', ...overrides } } @@ -65,16 +62,14 @@ function initializeTask(overrides: any = {}) { describe('PolicyServerPassthroughHandler', () => { afterEach(() => sinon.restore()) - describe('parameter validation (runs before auth)', () => { + describe('parameter validation', () => { it('rejects a missing policyServerPassthrough field (400)', async () => { - const { node, validateAuthenticationOrToken } = buildFakes() + const { node } = buildFakes() const response = await new PolicyServerPassthroughHandler(node).handle( passthroughTask({ policyServerPassthrough: undefined }) ) expect(response.status.httpStatus).to.equal(400) expect(response.status.error).to.contain('missing policyServerPassthrough') - // validation must short-circuit before we ever touch the auth component - assert(validateAuthenticationOrToken.notCalled, 'auth should not be reached') }) it('rejects a non-object policyServerPassthrough (400)', async () => { @@ -100,94 +95,13 @@ describe('PolicyServerPassthroughHandler', () => { expect(response.status.error).to.contain('must be an object') assert(passThrough.notCalled, 'must not reach the policy server') }) - - it('rejects a missing consumerAddress (400)', async () => { - const { node } = buildFakes() - const response = await new PolicyServerPassthroughHandler(node).handle( - passthroughTask({ consumerAddress: undefined }) - ) - expect(response.status.httpStatus).to.equal(400) - expect(response.status.error).to.contain('consumerAddress') - }) - - it('rejects a malformed consumerAddress (400)', async () => { - const { node } = buildFakes() - const response = await new PolicyServerPassthroughHandler(node).handle( - passthroughTask({ consumerAddress: 'not-an-address' }) - ) - expect(response.status.httpStatus).to.equal(400) - expect(response.status.error).to.contain('not a valid web3 address') - }) }) - describe('authentication', () => { - it('rejects an unauthenticated request (401)', async () => { - const { node } = buildFakes({ - authResult: { - valid: false, - error: - 'Invalid authentication, you need to provide either a token or an address, signature, message and nonce' - } - }) - const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough') - - const response = await new PolicyServerPassthroughHandler(node).handle( - passthroughTask({ nonce: undefined, signature: undefined }) - ) - - expect(response.status.httpStatus).to.equal(401) - assert(passThrough.notCalled, 'must not reach the policy server') - }) - - it('rejects an invalid signature (401)', async () => { - const { node } = buildFakes({ + describe('forwarding to the policy server', () => { + it('forwards the payload without authentication', async () => { + const { node, validateAuthenticationOrToken } = buildFakes({ authResult: { valid: false, error: 'Invalid signature' } }) - const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough') - - const response = await new PolicyServerPassthroughHandler(node).handle( - passthroughTask() - ) - - expect(response.status.httpStatus).to.equal(401) - expect(response.status.error).to.equal('Invalid signature') - assert(passThrough.notCalled, 'must not reach the policy server') - }) - - it('returns 401 when the auth component is not configured', async () => { - const { node } = buildFakes({ authConfigured: false }) - const response = await new PolicyServerPassthroughHandler(node).handle( - passthroughTask() - ) - expect(response.status.httpStatus).to.equal(401) - expect(response.status.error).to.equal('Auth not configured') - }) - - it('passes the auth header and the nonce/signature triple to the auth component', async () => { - const { node, validateAuthenticationOrToken } = buildFakes() - sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ - success: true, - message: 'ok', - httpStatus: 200 - }) - - await new PolicyServerPassthroughHandler(node).handle( - passthroughTask({ authorization: TOKEN }) - ) - - const args = validateAuthenticationOrToken.firstCall.args[0] - expect(args.token).to.equal(TOKEN) - expect(args.address).to.equal(CONSUMER) - expect(args.nonce).to.equal('1') - expect(args.signature).to.equal('0xsignature') - // the signed message is scoped to the command string - expect(args.command).to.equal(PROTOCOL_COMMANDS.POLICY_SERVER_PASSTHROUGH) - }) - }) - - describe('forwarding to the policy server', () => { - it('forwards the verified consumerAddress and the caller credentials', async () => { - const { node } = buildFakes() const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ success: true, message: 'ok', @@ -195,23 +109,17 @@ describe('PolicyServerPassthroughHandler', () => { }) const response = await new PolicyServerPassthroughHandler(node).handle( - passthroughTask({ authorization: TOKEN }) + passthroughTask() ) expect(response.status.httpStatus).to.equal(200) const forwarded = passThrough.firstCall.args[0] - expect(forwarded.consumerAddress).to.equal(CONSUMER) - expect(forwarded.authorization).to.equal(TOKEN) - expect(forwarded.nonce).to.equal('1') - expect(forwarded.signature).to.equal('0xsignature') - // the node still resolves the DDO server-side + assert(validateAuthenticationOrToken.notCalled, 'auth should not be called') expect(forwarded.ddo).to.deep.equal(DDO) - // and the caller's own payload is preserved expect(forwarded.action).to.equal('newDDO') }) - it('overwrites a forged consumerAddress inside the payload with the verified one', async () => { - // auth succeeds for CONSUMER, but the caller tries to pass VICTIM off as the consumer + it('preserves caller-supplied payload fields', async () => { const { node } = buildFakes() const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ success: true, @@ -229,25 +137,7 @@ describe('PolicyServerPassthroughHandler', () => { }) ) - expect(passThrough.firstCall.args[0].consumerAddress).to.equal(CONSUMER) - }) - - it('uses the address the token resolves to, not the one the caller claims', async () => { - // token path: Auth returns the address encoded in the JWT - const { node } = buildFakes({ - authResult: { valid: true, error: '', address: CONSUMER } - }) - const passThrough = sinon.stub(PolicyServer.prototype, 'passThrough').resolves({ - success: true, - message: 'ok', - httpStatus: 200 - }) - - await new PolicyServerPassthroughHandler(node).handle( - passthroughTask({ consumerAddress: VICTIM, authorization: TOKEN }) - ) - - expect(passThrough.firstCall.args[0].consumerAddress).to.equal(CONSUMER) + expect(passThrough.firstCall.args[0].consumerAddress).to.equal(VICTIM) }) it('forwards even when the DDO cannot be resolved (ddo stays null)', async () => { @@ -261,7 +151,6 @@ describe('PolicyServerPassthroughHandler', () => { await new PolicyServerPassthroughHandler(node).handle(passthroughTask()) expect(passThrough.firstCall.args[0].ddo).to.equal(null) - expect(passThrough.firstCall.args[0].consumerAddress).to.equal(CONSUMER) }) it('propagates a policy server denial', async () => {