refactor: Make signed request the final transformation

src/userlib/js/bootstrap/index.ts

@@ -87,7 +87,7 @@ if (host) {
 
 const agent = new HttpAgent({ host });
 agent.addTransform(makeNonceTransform());
-agent.addTransform(makeAuthTransform(keyPair));
+agent.setAuthTransform(makeAuthTransform(keyPair));
 
 window.icHttpAgent = agent;
 window.ic = { httpAgent: agent };

src/userlib/js/src/actor.test.ts

@@ -76,24 +76,26 @@ test('makeActor', async () => {
   ];
 
   const expectedCallRequest = {
-    request_type: SubmitRequestType.Call,
-    canister_id: canisterId,
-    method_name: methodName,
-    arg,
-    nonce: nonces[0],
+    content: {
+      request_type: SubmitRequestType.Call,
+      canister_id: canisterId,
+      method_name: methodName,
+      arg,
+      nonce: nonces[0],
+    },
     sender_pubkey: senderPubKey,
     sender_sig: senderSig,
   };
 
-  const expectedCallRequestId = await requestIdOf(expectedCallRequest);
+  const expectedCallRequestId = await requestIdOf(expectedCallRequest.content);
 
   let nonceCount = 0;
 
   const httpAgent = new HttpAgent({
     fetch: mockFetch,
   });
   httpAgent.addTransform(makeNonceTransform(() => nonces[nonceCount++]));
-  httpAgent.addTransform(
+  httpAgent.setAuthTransform(
     makeAuthTransform(
       {
         publicKey: senderPubKey,
@@ -109,6 +111,7 @@ test('makeActor', async () => {
   expect(reply).toEqual(IDL.decode([IDL.Text], expectedReplyArg)[0]);
 
   const { calls, results } = mockFetch.mock;
+
   expect(calls.length).toBe(4);
   expect(calls[0]).toEqual([
     '/api/v1/submit',
@@ -129,9 +132,11 @@ test('makeActor', async () => {
         'Content-Type': 'application/cbor',
       },
       body: cbor.encode({
-        request_type: 'request_status',
-        request_id: expectedCallRequestId,
-        nonce: nonces[1],
+        content: {
+          request_type: 'request_status',
+          request_id: expectedCallRequestId,
+          nonce: nonces[1],
+        },
         sender_pubkey: senderPubKey,
         sender_sig: senderSig,
       }),
@@ -145,9 +150,11 @@ test('makeActor', async () => {
       'Content-Type': 'application/cbor',
     },
     body: cbor.encode({
-      request_type: 'request_status',
-      request_id: expectedCallRequestId,
-      nonce: nonces[2],
+      content: {
+        request_type: 'request_status',
+        request_id: expectedCallRequestId,
+        nonce: nonces[2],
+      },
       sender_pubkey: senderPubKey,
       sender_sig: senderSig,
     }),
@@ -160,9 +167,11 @@ test('makeActor', async () => {
       'Content-Type': 'application/cbor',
     },
     body: cbor.encode({
-      request_type: 'request_status',
-      request_id: expectedCallRequestId,
-      nonce: nonces[3],
+      content: {
+        request_type: 'request_status',
+        request_id: expectedCallRequestId,
+        nonce: nonces[3],
+      },
       sender_pubkey: senderPubKey,
       sender_sig: senderSig,
     }),

src/userlib/js/src/auth.ts

@@ -1,6 +1,10 @@
 import { Buffer } from 'buffer/';
 import { sign as naclSign } from 'tweetnacl';
-import { HttpAgentRequest, HttpAgentRequestTransformFn } from './http_agent_types';
+import {
+  AuthHttpAgentRequestTransformFn,
+  HttpAgentRequest,
+  SignedHttpAgentRequest,
+} from './http_agent_types';
 import { RequestId, requestIdOf } from './request_id';
 import { BinaryBlob } from './types';
 
@@ -55,19 +59,22 @@ export type SigningConstructedFn = (
 export function makeAuthTransform(
   keyPair: KeyPair,
   senderSigFn: SigningConstructedFn = sign,
-): HttpAgentRequestTransformFn {
+): AuthHttpAgentRequestTransformFn {
   const { publicKey, secretKey } = keyPair;
   const signFn = senderSigFn(secretKey);
 
   const fn = async (r: HttpAgentRequest) => {
-    const requestId = await requestIdOf(r.body);
-    r.body.sender_pubkey = publicKey;
-    r.body.sender_sig = signFn(requestId);
+    const { body, ...fields } = r;
+    const requestId = await requestIdOf(body);
+    return {
+      ...fields,
+      body: {
+        content: body,
+        sender_pubkey: publicKey,
+        sender_sig: signFn(requestId),
+      },
+    } as SignedHttpAgentRequest;
   };
 
-  // Set priority low so other transforms run first. Signing should be done on
-  // the last request transformed.
-  fn.priority = -100;
-
   return fn;
 }

src/userlib/js/src/http_agent.test.ts

@@ -9,6 +9,7 @@ import {
   ReadRequestType,
   RequestStatusResponseReplied,
   RequestStatusResponseStatus,
+  Signed,
   SubmitRequestType,
 } from './http_agent_types';
 import { requestIdOf } from './request_id';
@@ -37,7 +38,7 @@ test('call', async () => {
     fetch: mockFetch,
   });
   httpAgent.addTransform(makeNonceTransform(() => nonce));
-  httpAgent.addTransform(makeAuthTransform(keyPair));
+  httpAgent.setAuthTransform(makeAuthTransform(keyPair));
 
   const methodName = 'greet';
   const arg = Buffer.from([]) as BinaryBlob;
@@ -62,13 +63,13 @@ test('call', async () => {
   // Just sanity checking our life.
   expect(verify(mockPartialsRequestId, senderSig, keyPair.publicKey)).toBe(true);
 
-  const expectedRequest: CallRequest = {
-    ...mockPartialRequest,
+  const expectedRequest: Signed<CallRequest> = {
+    content: mockPartialRequest,
     sender_pubkey: keyPair.publicKey,
     sender_sig: senderSig,
-  } as CallRequest;
+  } as Signed<CallRequest>;
 
-  const expectedRequestId = await requestIdOf(expectedRequest);
+  const expectedRequestId = await requestIdOf(expectedRequest.content);
   expect(expectedRequestId).toEqual(mockPartialsRequestId);
 
   const { calls, results } = mockFetch.mock;
@@ -117,7 +118,7 @@ test('requestStatus', async () => {
     fetch: mockFetch,
   });
   httpAgent.addTransform(makeNonceTransform(() => nonce));
-  httpAgent.addTransform(makeAuthTransform(keyPair, () => () => Buffer.from([0]) as SenderSig));
+  httpAgent.setAuthTransform(makeAuthTransform(keyPair, () => () => Buffer.from([0]) as SenderSig));
 
   const requestId = await requestIdOf({
     request_type: SubmitRequestType.Call,
@@ -132,9 +133,11 @@ test('requestStatus', async () => {
   });
 
   const expectedRequest = {
-    request_type: ReadRequestType.RequestStatus,
-    request_id: requestId,
-    nonce,
+    content: {
+      request_type: ReadRequestType.RequestStatus,
+      request_id: requestId,
+      nonce,
+    },
     sender_pubkey: senderPubKey,
     sender_sig: Buffer.from([0]) as SenderSig,
   };

src/userlib/js/src/http_agent.ts

@@ -4,6 +4,7 @@ import * as actor from './actor';
 import { CanisterId } from './canisterId';
 import * as cbor from './cbor';
 import {
+  AuthHttpAgentRequestTransformFn,
   Endpoint,
   HttpAgentReadRequest,
   HttpAgentRequest,
@@ -17,13 +18,14 @@ import {
   ReadResponse,
   RequestStatusResponse,
   ResponseStatusFields,
+  SignedHttpAgentRequest,
   SubmitRequest,
   SubmitRequestType,
   SubmitResponse,
 } from './http_agent_types';
 import * as IDL from './idl';
 import { requestIdOf } from './request_id';
-import { BinaryBlob, blobFromHex } from './types';
+import { BinaryBlob, blobFromHex, blobFromUint8Array } from './types';
 
 const API_VERSION = 'v1';
 
@@ -67,12 +69,14 @@ function getDefaultFetch() {
 // allowing extensions.
 export class HttpAgent {
   private readonly _pipeline: HttpAgentRequestTransformFn[] = [];
+  private _authTransform: AuthHttpAgentRequestTransformFn | null = null;
   private readonly _fetch: typeof fetch;
   private readonly _host: string = '';
 
   constructor(options: HttpAgentOptions = {}) {
     if (options.parent) {
       this._pipeline = [...options.parent._pipeline];
+      this._authTransform = options.parent._authTransform;
     }
     this._fetch = options.fetch || getDefaultFetch() || fetch.bind(global);
     if (options.host) {
@@ -90,6 +94,10 @@ export class HttpAgent {
     this._pipeline.splice(i >= 0 ? i : this._pipeline.length, 0, Object.assign(fn, { priority }));
   }
 
+  public setAuthTransform(fn: AuthHttpAgentRequestTransformFn) {
+    this._authTransform = fn;
+  }
+
   public async submit(submit: SubmitRequest): Promise<SubmitResponse> {
     const transformedRequest = (await this._transform({
       request: {
@@ -207,13 +215,19 @@ export class HttpAgent {
     return actor.makeActorFactory;
   }
 
-  protected _transform(request: HttpAgentRequest): Promise<HttpAgentRequest> {
+  protected _transform(
+    request: HttpAgentRequest,
+  ): Promise<HttpAgentRequest | SignedHttpAgentRequest> {
     let p = Promise.resolve(request);
 
     for (const fn of this._pipeline) {
       p = p.then(r => fn(r).then(r2 => r2 || r));
     }
 
-    return p;
+    if (this._authTransform != null) {
+      return p.then(this._authTransform);
+    } else {
+      return p;
+    }
   }
 }

src/userlib/js/src/http_agent_types.ts

@@ -27,11 +27,33 @@ export interface HttpAgentReadRequest extends HttpAgentBaseRequest {
   body: ReadRequest;
 }
 
+export type SignedHttpAgentRequest = SignedHttpAgentReadRequest | SignedHttpAgentSubmitRequest;
+
+export interface SignedHttpAgentSubmitRequest extends HttpAgentBaseRequest {
+  readonly endpoint: Endpoint.Submit;
+  body: Signed<SubmitRequest>;
+}
+
+export interface SignedHttpAgentReadRequest extends HttpAgentBaseRequest {
+  readonly endpoint: Endpoint.Read;
+  body: Signed<ReadRequest>;
+}
+
+export interface Signed<T> {
+  content: T;
+  sender_pubkey: BinaryBlob;
+  sender_sig: BinaryBlob;
+}
+
 export interface HttpAgentRequestTransformFn {
   (args: HttpAgentRequest): Promise<HttpAgentRequest | undefined | void>;
   priority?: number;
 }
 
+export type AuthHttpAgentRequestTransformFn =
+  (args: HttpAgentRequest) => Promise<SignedHttpAgentRequest>;
+
+
 export interface QueryFields {
   methodName: string;
   arg: BinaryBlob;

src/userlib/js/src/request_id.test.ts

@@ -70,12 +70,6 @@ test('requestIdOf', async () => {
     // D   I   D   L   \x00  \253  *
     // 68  73  68  76  0     253   42
     arg: Buffer.from([68, 73, 68, 76, 0, 253, 42]) as BinaryBlob,
-
-    // These fields are not included in the example provided in the spec but we
-    // provide them here to verify that they do not affect the request ID:
-    // "Remove the fields that are only used for authentication"
-    sender_pubkey: Buffer.alloc(32, 0) as SenderPubKey,
-    sender_sig: Buffer.alloc(64, 0) as SenderSig,
   };
 
   const requestId = await requestIdOf(request);

src/userlib/js/src/request_id.ts

@@ -50,15 +50,7 @@ const concat = (bs: BinaryBlob[]): BinaryBlob => {
 };
 
 export const requestIdOf = async (request: Record<string, any>): Promise<RequestId> => {
-  // While the type signature of this function ensures the fields we care about
-  // are present, it does not prevent additional fields from being provided,
-  // including the fields used for authentication that we must omit when
-  // calculating the request ID. This is by design, since requests are expected
-  // to have more than just the common fields. As a result, we need to explictly
-  // ignore the authentication fields.
-  const { sender_pubkey, sender_sig, ...fields } = request;
-
-  const hashed: Array<Promise<[BinaryBlob, BinaryBlob]>> = Object.entries(fields).map(
+  const hashed: Array<Promise<[BinaryBlob, BinaryBlob]>> = Object.entries(request).map(
     async ([key, value]: [string, unknown]) => {
       const hashedKey = await hashString(key);
       const hashedValue = await hashValue(value);