From 8274fcee7dffdcaf70881b42043633d5270d1af4 Mon Sep 17 00:00:00 2001
From: Benjamin Chauvette <dev@bench.email>
Date: Wed, 28 Apr 2021 14:38:07 -0400
Subject: [PATCH] Remove Readonly<T> from AuthenticateHandler params

This commit removes the Readonly<T> wrappings around the `username` and
`password` params to the `AuthenticateHandler` interface, which were added in
 #596.

The motivation for this change is that when `password` is `Readonly<Buffer>`,
the type is incompatible with `crypto.timingSafeEqual`, which is the function
Aedes users should be using to compare raw, sensitive buffers with each other.
Because `Readonly<Buffer>` is incompatible with `crypto.timingSafeEqual`, users
end up having to cast with `password as Buffer`, which largely defeats the
purpose of marking it `Readonly` in the first place and introduces casting in
security-related areas of the code where it's not really needed in the first
place.

The error it gives is:

    No overload matches this call.
      Overload 1 of 2, '(a: ArrayBufferView, b: ArrayBufferView): boolean', gave the following error.
        Argument of type 'Readonly<Buffer>' is not assignable to parameter of type 'ArrayBufferView'.
          Type 'Readonly<Buffer>' is missing the following properties from type 'Float32Array': [Symbol.iterator], [Symbol.toStringTag]
      Overload 2 of 2, '(a: ArrayBufferView, b: ArrayBufferView): boolean', gave the following error.
        Argument of type 'Readonly<Buffer>' is not assignable to parameter of type 'ArrayBufferView'.ts(2769)

Removing it from `username` has no effect, because strings are already
immutable in JavaScript, and TypeScript will automatically treat it as if it
were just `string`.
---
 test/types/aedes.test-d.ts | 5 +++--
 types/instance.d.ts        | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/test/types/aedes.test-d.ts b/test/types/aedes.test-d.ts
index a9a09b74..1eb10e4a 100644
--- a/test/types/aedes.test-d.ts
+++ b/test/types/aedes.test-d.ts
@@ -1,4 +1,5 @@
 import { expectType } from 'tsd'
+import { timingSafeEqual } from 'crypto'
 import { Socket } from 'net'
 import type {
   Aedes,
@@ -27,8 +28,8 @@ const broker = Server({
       callback(new Error('connection error'), false)
     }
   },
-  authenticate: (client: Client, username: Readonly<string>, password: Readonly<Buffer>, callback) => {
-    if (username === 'test' && password === Buffer.from('test') && client.version === 4) {
+  authenticate: (client: Client, username: string, password: Buffer, callback) => {
+    if (username === 'test' && timingSafeEqual(password, Buffer.from('test')) && client.version === 4) {
       callback(null, true)
     } else {
       const error = new Error() as AuthenticateError
diff --git a/types/instance.d.ts b/types/instance.d.ts
index ca18155a..44898a68 100644
--- a/types/instance.d.ts
+++ b/types/instance.d.ts
@@ -27,8 +27,8 @@ type PreConnectHandler = (client: Client, packet: ConnectPacket, callback: (erro
 
 type AuthenticateHandler = (
   client: Client,
-  username: Readonly<string>,
-  password: Readonly<Buffer>,
+  username: string,
+  password: Buffer,
   done: (error: AuthenticateError | null, success: boolean | null) => void
 ) => void