Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isKeyObject check fails in Bun #623

Closed
blendsdk opened this issue Oct 2, 2023 · 6 comments
Closed

isKeyObject check fails in Bun #623

blendsdk opened this issue Oct 2, 2023 · 6 comments

Comments

@blendsdk
Copy link

blendsdk commented Oct 2, 2023

Hi,

I am using bun as runtime to authenticate against an OP. The authentication process breaks on callback and I get the 'what?!'
exception. After some debugging it looks like that isKeyObject is failing because the key keyObject is not crypto.KeyObject but crypto.CryptoKey!! I think this is a specific to bun.

After I comment keystore.js->239-242 everything works. Any advice?

Debug information:

      if (!isKeyObject(keyObject)) {
        console.log(keyObject);
        throw new Error('what?!');
      }
CryptoKey {
  type: "public",
  extractable: false,
  algorithm: {
    name: "RSASSA-PKCS1-v1_5",
    modulusLength: 2048,
    publicExponent: Uint8Array(3) [ 1, 0, 1 ],
    hash: {
      name: "SHA-256"
    }
  },
  usages: [ "verify" ]
}

reference:
https://github.com/panva/node-openid-client/blob/0f7d3b4f03c799fa74c480e7ee9d32f33344f9df/lib/helpers/is_key_object.js#L4C1-L4C1

https://github.com/panva/node-openid-client/blob/0f7d3b4f03c799fa74c480e7ee9d32f33344f9df/lib/helpers/keystore.js#L239C13-L239C13

@blendsdk blendsdk added the triage label Oct 2, 2023
@panva
Copy link
Owner

panva commented Oct 2, 2023

This is happening because jose, the dependency used to import JWK and almost all other crypto involved, is a universal module and it has a "bun" target. Unfortunately, it also has a "require" target that, despite openid-client being CJS and using require(), bun ignores and loads the universal code which works with Web Crypto API as runtime for crypto, not node's node:crypto.

The problem at the moment is that Bun, from my POV at least, doesn't use the correct target of a dependency which is being loaded through its CJS and node compatibility modes. I suggest you bring this up with Bun as well, you're not the first one to have this issue.

If I have the time in the coming weeks/months I will try and ensure that the entire openid-client codebase works regardless of either KeyObject or CryptoKey coming back from jose, but that is not a simple task and also not a priority for me at the moment. This is a Node.js module afterall (see FAQ in the readme), not one written with other runtimes in mind.

Now even if Bun did load the correct node:crypto based target it lacks the required node APIs anyway, see https://bun.sh/docs/runtime/nodejs-apis#node-crypto.

cc @Jarred-Sumner

@Jarred-Sumner
Copy link

This should really be fixed on Bun's end and not something @panva should have to spend time dealing with

@cirospaciari is almost done with KeyObject and that might help with this particular case

@panva
Copy link
Owner

panva commented Oct 2, 2023

@Jarred-Sumner Thank you for the response, filling in KeyObject won't help on its own so long as Bun doesn't load the "require" target of jose when require()d from openid-client.

@panva
Copy link
Owner

panva commented Oct 2, 2023

So it's either

a) have Bun use the correct require target over others when running a CJS module written for node and fill in crypto APIs such as KeyObject (there could be others)

or

b) I take the time to update the codebase to handle both KeyObject or CryptoKey coming back from jose

panva added a commit that referenced this issue Oct 3, 2023
This attempts to work around:

- missing Node.js APIs in Bun
- Bun's bugs in url.parse(..., true)
- Bun loading `jose`'s bun target instead of the require one

It is not possible to run openid-client's test suite due to other Bun
Node.js compatibility bugs which is why this is "experimental"

Refs #622
Refs #623
panva added a commit that referenced this issue Oct 3, 2023
This attempts to work around:

- missing Node.js APIs in Bun
- Bun's bugs in url.parse(..., true)
- Bun loading `jose`'s bun target instead of the require one

It is not possible to run openid-client's test suite due to other Bun
Node.js compatibility bugs which is why this is "experimental"

Refs #622
Refs #623
panva added a commit that referenced this issue Oct 3, 2023
This attempts to work around:

- missing Node.js APIs in Bun
- Bun's bugs in url.parse(..., true)
- Bun loading `jose`'s bun target instead of the require one

It is not possible to run openid-client's test suite due to other Bun
Node.js compatibility bugs which is why this is "experimental"

Refs #622
Refs #623
@panva
Copy link
Owner

panva commented Oct 3, 2023

I've released https://github.com/panva/node-openid-client/releases/tag/v5.6.0

This attempts to work around:

  • missing Node.js APIs in Bun
  • Bun's bugs in url.parse(..., true)
  • Bun loading jose's bun target instead of the require one

It is not possible to run openid-client's test suite due to other Bun Node.js compatibility bugs which is why this is "experimental" and this for now remains a Node.js only module.

Please let me know if you encounter any new issues when running openid-client in Bun.

@panva panva closed this as not planned Won't fix, can't repro, duplicate, stale Oct 3, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2024
@panva
Copy link
Owner

panva commented Oct 7, 2024

@panva panva removed the triage label Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants