[BUG] Does not work with Bun

[madsbuch]

This library is dysfunctional in a Bun environment.

TypeError: Invalid public key
      at node:crypto:77:61
      at transformJwkToKeyObjectSync (/.../node_modules/aws-jwt-verify/dist/esm/jwt-rsa.js:348:27)
      at verifyDecomposedJwtSync (/.../node_modules/aws-jwt-verify/dist/esm/jwt-rsa.js:120:23)
      at verifySync (/.../node_modules/aws-jwt-verify/dist/esm/cognito-verifier.js:94:9)
      at /.../graphQlApp.ts:41:25

There is a parallel bug on the Bun bug tracker: https://github.com/oven-sh/bun/issues/8004

I realize that AWS might not feel inclined to support Bun, I just want make a notice that this issue is happening.

[hakanson]

I'm curious how our unit tests in bun behave that import public keys. In the node package.json is "test": "npm run test:unit", so an npm run test will output:

Test Suites: 1 skipped, 7 passed, 7 of 8 total
Tests:       4 skipped, 162 passed, 166 total
Snapshots:   0 total
Time:        29.46 s
Ran all test suites with tests matching "unit".

I haven't previously used bun, so dowloaded it, read https://bun.sh/guides/test/migrate-from-jest, and ran bun test (hoping for magic) but doesn't look like ran the same code.

 26 pass
 58 fail
 64 expect() calls
Ran 84 tests across 10 files. [22.44s]

Since you have bun experience, would you mind looking at how to run the equivalent of our unit tests?

    "test:unit": "jest --collect-coverage -t \"unit\" --testMatch '**/*.test.ts' --reporters=\"jest-junit\" --reporters=\"default\"",
    "test": "npm run test:unit",

[ottokruse]

Looks like this is our fault, see #155 But after fixing that maybe another reason pops up why this wouldn't work in Bun. Curious!

[ottokruse]

Gonna be after the weekend before we release a new version:) Bun users hold your breath

[ottokruse]

Should be fixed in v4.0.1

[ottokruse]

Let us know how you now fare with Bun @madsbuch

[madsbuch]

Awesome! Really fast turnaround time on this!

Overall it works now, I get full verifications through! I will revert back to this from the other implementation I had going.

The fetcher is quite wonky, but that can be fixed using the customer fetcher:

88 |  * HTTPS fetch errors
89 |  */
90 | export class FetchError extends JwtBaseError {
91 |     constructor(uri, msg) {
92 |         super(`Failed to fetch ${uri}: ${msg}`);
93 |     }
                                ^
error: Failed to fetch https://cognito-idp.XXX.amazonaws.com/XXX/.well-known/jwks.json: SyntaxError: JSON Parse error: Unexpected EOF
      at new JwtBaseError (:1:28)
      at new FetchError (/.../node_modules/aws-jwt-verify/dist/esm/error.js:93:10)
      at new NonRetryableFetchError (:1:28)
      at /.../node_modules/aws-jwt-verify/dist/esm/https-node.js:82:23

[ottokruse]

Ok, cool!

Any clue what is wrong with the fetcher, I see JSON Parse error: Unexpected EOF? Surely Bun supports JSON.parse, not sure what's wrong there

[madsbuch]

@ottokruse

I use this implementation which is stable:

    CognitoJwtVerifier.create(
      {
        userPoolId: config.aws.cognito.userPoolId,
        tokenUse: "access",
        clientId: config.aws.cognito.authorizedClients,
      },
      {
        jwksCache: new SimpleJwksCache({
          fetcher: {
            fetch: async (uri, req, data) =>
              (
                await fetch(uri, {
                  body: data,
                  ...req,
                })
              ).json(),
          },
        }),
      }
    )

I really appreciate how you have made that part pluggable. I love the "batteries included" aspect, but in particular when there are multiple runtimes (as in Bun, Node, Deno, etc.) it makes sense to hook into better solutions for each runtime,.

Anyways, I really appreciate the work here. I have fully gone back to using this package.

[ottokruse]

Cheers! And thanks for the feedback

And guess not weird that the default implementation for fetcher doesn't work, it uses the "old" NodeJS way of doing it, with eg a pipeline to collect the response, which is pretty NodeJS specific (although I guess Bun would want to support that, as they strive for great compatibility). Anyway nice that you have it working now with just "fetch".