From b24913e5fa2880051c916cc92e9510cbc6ad79c0 Mon Sep 17 00:00:00 2001 From: AdriGeorge Date: Tue, 18 Aug 2026 13:05:07 +0300 Subject: [PATCH] Refactor PolicyServerPassthrough to remove authentication requirements and update related tests --- 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 () => {