pilcrowonpaper / oslo

A collection of auth-related utilities
https://oslo.js.org
MIT License
1.06k stars 35 forks source link

[oslo/crypto] Accept `TypedArray` in addition to `ArrayBuffer` #12

Closed binyamin closed 9 months ago

binyamin commented 10 months ago

Currently, the createJWT method only accepts the key as an ArrayBuffer. This is probably because many methods in oslo/crypto only accept an ArrayBuffer. However, the Web Crypto API often accepts TypedArray (eg. Uint8Array) and DataView as well.

My personal use-case is that I am using JWTs in cookie sessions. Instead of generating the JWT key at runtime, I want to save the JWT key as a string, so the sessions are valid even when I restart the server.

In any case, ArrayBuffer is harder to create/manipulate than Uint8Array.

Because the Web Crypto API is compatible either way, this shouldn't require any changes in logic (just type-defs).


PS: Thanks so much for your work here and with lucia. It means I have one less thing to worry about when implementing authentication.

pilcrowonpaper commented 10 months ago

Doesn't Uint8Array satisfy ArrayBuffer?

binyamin commented 10 months ago

Nope. Uint8Array can consume ArrayBuffer, but you can't actually write data to ArrayBuffer. So you can't create an ArrayBuffer from a string. See MDN on ArrayBuffer.

pilcrowonpaper commented 10 months ago

No I mean like ArrayBuffer includes Uint8Array, so you can pass Uint8Array to any function that accepts ArrayBuffer?

binyamin commented 10 months ago

I mean, practically speaking, many APIs (including Web Crypto) accept both ArrayBuffer and Uint8Array. But they're different structures, so typescript complains. The main problem is the docs & TS.

I can PR this, if you want, and you can see what I mean.

pilcrowonpaper commented 10 months ago

Can you provide an example? Most Web Crypto APIs return an ArrayBuffer so you can pass the result to Oslo, and most Oslo API returns ArrayBuffer or Uint8Array so you can pass the result to Web Crypto. I'm not sure where TS complains?

binyamin commented 10 months ago

Hmm. I tested this further. TLDR is that typescript's types are only compatible because they don't include more recent additions to ECMAScript. Bun's types are actually correct, which is where I got the error.

Officially, they are different structures, and you can see this with node:assert.

import { isTypedArray, isAnyArrayBuffer } from 'node:util/types';

const a = new ArrayBuffer(10);
console.error(isAnyArrayBuffer(a)); // true
console.error(isTypedArray(a)); // false

const b = new Uint8Array(10);
console.error(isAnyArrayBuffer(b)); // false
console.error(isTypedArray(b)); // true

In addition, there are some differences with the APIs, which you can see with Node.js v20.

const a = new ArrayBuffer(8, {
    maxByteLength: 16,
});
const b = new ArrayBuffer(8);
const c = new Uint8Array(8);

a.resize(12); // pass
b.resize(12); // error
c.resize(12); // error

However, neither Typescript nor Node.js includes these particular APIs in the types. I was using Bun, which does differentiate within the bun-types package.

pilcrowonpaper commented 10 months ago

Hm, that's interesting

Officially, they are different structures

That shouldn't matter from TS' perspective since it only cares that the method and property types match

pilcrowonpaper commented 9 months ago

Fixed with 0.26.1