Skip to content

Commit

Permalink
Remove Readonly<T> from AuthenticateHandler params
Browse files Browse the repository at this point in the history
This commit removes the Readonly<T> wrappings around the `username` and
`password` params to the `AuthenticateHandler` interface, which were added in
 moscajs#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`.
  • Loading branch information
Benjamin Chauvette committed Apr 28, 2021
1 parent 6844e36 commit 8274fce
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
5 changes: 3 additions & 2 deletions test/types/aedes.test-d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expectType } from 'tsd'
import { timingSafeEqual } from 'crypto'
import { Socket } from 'net'
import type {
Aedes,
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions types/instance.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 8274fce

Please sign in to comment.