tj/node-cookie-signature

Allow Buffer as `key`

Closed this issue · 2 comments

Or any other of the allowed types for the key parameter: <string> | <Buffer> | <TypedArray> | <DataView> | <KeyObject>

Cryptographically secure keys are usually binary encoded, converting them to strings is not always possible.

Looks like the only real technical blocker here would be to change the assertions from e.g.

if ('string' != typeof secret) throw new TypeError("Secret string must be provided.");

to something like:

if ('undefined' == typeof secret) throw new TypeError("Secret key must be provided.");

This seems relatively harmless to me but I'd want to get at least another set of eyes from downstream maintainers — e.g. @dougwilson — to make sure this wouldn't introduce any risk. (I'm struggling to find a reason not to change this but perhaps there's code out there that, say, for whatever reason, relies on the current assertion. Maybe a reasonable compromise would be to allow this flexibility in a new major release?)

Depending on how .createHmac treats null, you may have to do a check like secret == null (i.e. check for both undefined and null), but otherwise it seems to me the change is harmless and if it were me, I'd just file it under an enhancement (secret can now be anything .createHmac accepts vs just a string), especially since the behavior of a string would be unchanged.

The code is there (imo) as a guard to provide a useful error message. Not sure what .createHmac would do if a user supplied a function as the secret, but if it's a useful error, then probably just letting .createHmac handle it is good enough.