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

Implement KeyObject (node:crypto) #2036

Closed
Jarred-Sumner opened this issue Feb 10, 2023 · 32 comments · Fixed by #5940
Closed

Implement KeyObject (node:crypto) #2036

Jarred-Sumner opened this issue Feb 10, 2023 · 32 comments · Fixed by #5940
Assignees
Labels
atw enhancement New feature or request node.js Compatibility with Node.js APIs

Comments

@Jarred-Sumner
Copy link
Collaborator

This blocks the popular jsonwebtoken module.

From Discord:
image

https://discord.com/channels/876711213126520882/887787428973281300/1073538772840480789

@Jarred-Sumner Jarred-Sumner added the node.js Compatibility with Node.js APIs label Feb 10, 2023
@Electroid Electroid added the enhancement New feature or request label Feb 10, 2023
@ceopaludetto
Copy link

Any updates? This blocks @nestjs/jwt module as well

@chen-rn
Copy link

chen-rn commented Sep 11, 2023

I believe this is also blocking all Firebase projects at the moment :(

@endel
Copy link

endel commented Sep 12, 2023

firebase-admin seems to be affected too:

Error: Right hand side of instanceof is not an object
    at new FirebaseError (node_modules/firebase-admin/lib/utils/error.js:30:13)
    at new PrefixedFirebaseError (node_modules/firebase-admin/lib/utils/error.js:65:12)
    at new FirebaseAuthError (node_modules/firebase-admin/lib/utils/error.js:154:13)
    at mapJwtErrorToAuthError (node_modules/firebase-admin/lib/auth/token-verifier.js:273:104)
    at <anonymous> (node_modules/firebase-admin/lib/auth/token-verifier.js:246:9)
    at processTicksAndRejections (:1:1302)

@ehuff700
Copy link

What needs to be done for this to get fixed? This has been broken since Feb of this year lol.

@gingermusketeer
Copy link
Contributor

gingermusketeer commented Sep 14, 2023

Digging into this a little I found that the existing node crypto is implemented here.

Looking at the code in node for crypto it looks like the web standard class CryptoKey which is available in Bun is implemented as a thin wrapper around KeyObject. Perhaps the reverse could be implemented and Bun could expose KeyObject as a thin wrapper around CryptoKey.

Would this be a sane approach?

@ehuff700
Copy link

@gingermusketeer I feel like the better option would be to migrate away from crypto-browserify and simply re export everything in node:crypto until Bun can natively support those APIs. If I understand the purpose of node-fallbacks it's to provide wrappers for default node APIs where bun doesn't have support (please correct me if I am I in err)

@gingermusketeer
Copy link
Contributor

re export everything in node:crypto

@ehuff700 how would this work? I don't think node:crypto is accessible from bun.

@skeie
Copy link

skeie commented Sep 15, 2023

It also blocks passport-magic-login

@iturgeonK
Copy link

I figured I might as well link to the source, the screenshot in the report is referencing this line in jsonwebtoken (https://github.com/auth0/node-jsonwebtoken/blob/master/sign.js#L114), which is the same reson I landed here.

There is also an issue on jsonwebtoken pertaining to compatability with bun: auth0/node-jsonwebtoken#939

@carlos12fds151
Copy link

Waiting for this

@andres-linares
Copy link

Waiting too

@Electroid Electroid added the atw label Sep 19, 2023
@josebruno2020
Copy link

Waiting too. I came because of jsonwebtoken module

@Neiz-Kap
Copy link

I answered here and it may help you:
auth0/node-jsonwebtoken#863 (comment)

@diavrank
Copy link

Same issue here from my Nestjs app

    err: {
      "type": "Error",
      "message": "\u001b[38;5;3m[GlobalExceptionFilter] \u001b[39m\u001b[35mGET: \u001b[0m\u001b[31mRight hand side of instanceof is not an object\u001b[39m\n{\n    \"domain\": \"\",\n    \"identifier\": \"/accounts/me\"\n}",
      "stack":
          TypeError: Right hand side of instanceof is not an object
              at /Users/myuser/Projects/nestjs/api/node_modules/jsonwebtoken/verify.js:120:10
              at getSecret (/Users/myuser/Projects/nestjs/api/node_modules/jsonwebtoken/verify.js:96:51)
              at /Users/myuser/Projects/nestjs/api/node_modules/jsonwebtoken/verify.js:101:7
              at /Users/myuser/Projects/nestjs/api/node_modules/passport-jwt/lib/verify_jwt.js:3:59
              at /Users/myuser/Projects/nestjs/api/node_modules/passport-jwt/lib/strategy.js:104:16
              at /Users/myuser/Projects/nestjs/api/node_modules/passport-jwt/lib/strategy.js:39:42
              at /Users/myuser/Projects/nestjs/api/node_modules/passport-jwt/lib/strategy.js:99:8
              at attempt (/Users/myuser/Projects/nestjs/api/node_modules/passport/lib/middleware/authenticate.js:369:8)
              at authenticate (/Users/myuser/Projects/nestjs/api/node_modules/passport/lib/middleware/authenticate.js:369:8)
              at /Users/myuser/Projects/nestjs/api/node_modules/@nestjs/passport/dist/auth.guard.js:95:63
              at new Promise (:1:21)
              at /Users/myuser/Projects/nestjs/api/node_modules/@nestjs/passport/dist/auth.guard.js:88:8
              at /Users/myuser/Projects/nestjs/api/node_modules/@nestjs/passport/dist/auth.guard.js:49:24
              at fulfilled (/Users/myuser/Projects/nestjs/api/node_modules/@nestjs/passport/dist/auth.guard.js:16:68)
              at processTicksAndRejections (:55:77)
    }

Is there an estimated date for the fix? I just wanna know when to continue with the migration from Node to Bun.

@Zack-Heisnberg
Copy link

and i am done already from bun sadly :') ,

No firebase
No nestjs
No jsonwebtoken
so it's kinda useless as it is now +80%of projects need this and the issue since feb with +1.7K issues

bun need sponsores and more contributions :') , had to go back to node & pnpm

@chen-rn
Copy link

chen-rn commented Sep 24, 2023

bun need sponsores and more contributions :') , had to go back to node & pnpm

The PNPM/package manager side of bun should still work well! bun install is quite a bit faster than pnpm with the only notable caveat being storage cost. Though I personally have a habit of clearing old projects and cloning them from Github anyway so it doesn't matter too much for me!

@coding-agent
Copy link

coding-agent commented Sep 24, 2023

Hi,

I have the same issue using NestJS, It was working with node before migrating to bun.

I ran "bun i @nestjs/jwt@9.0.0" an it worked! I believe it might work for most of the previous issues related to nestjs commented too. Also I needed to downgrade passwort-jwt with "bun i passport-jwt@4.0.0"

Hope this comment helps.

Thank you :)

@pedrohaccorsi
Copy link

any news here? Im facing the same issue with bun and jsonwebtoken :c

@fahadali503
Copy link

Hi,

I have the same issue using NestJS, It was working with node before migrating to bun.

I ran "bun i @nestjs/jwt@9.0.0" an it worked! I believe it might work for most of the previous issues related to nestjs commented too. Also I needed to downgrade passwort-jwt with "bun i passport-jwt@4.0.0"

Hope this comment helps.

Thank you :)

It worked, thank you

@diavrank
Copy link

It worked for me as well!!

@diavrank
Copy link

diavrank commented Sep 28, 2023

False :/ , I just realized that I ran the wrong command:

"start:dev": "NODE_ENV=localdev nest start --watch \"bun run\"",

It was missing the --exec flag . It seems the problem comes from passport-jwt dependency which is needed for @nestjs/passport

@carlos12fds151
Copy link

I'm using jose with Elysia and is working well for me

`import * as jose from 'jose';

const secret = new TextEncoder().encode('123456...EXAMPLE')

export const sign = async (payload: any, expiration: string) => {

const alg = 'HS256'
  
const jwt = await new jose.SignJWT({ payload })
    .setProtectedHeader({ alg })
    .setExpirationTime(expiration)
    .sign(secret)
  
return jwt;

};

export const verify = async (token: any) => {
try {
if (!token) return 'Token Expired';

    const verified:any = await jose.jwtVerify(token, secret);

    return verified.payload;

} catch (error) {
    return 'Token Expired';
}

}

export const jwtMiddleware = async (req: any) => {
const authorizationHeader = req.headers['authorization'];

if (!authorizationHeader || !authorizationHeader.startsWith('Bearer ')) {
    return new Response(JSON.stringify({ 'error': 'Unauthorized' }), { status: 401, headers: {
        'Content-Type': 'application/json',
    }});
}

const token = authorizationHeader.substring(7);

const verified:any = await verify(token);

if (verified === 'Token Expired') {
    return new Response(JSON.stringify({ 'error': 'Unauthorized' }), { status: 401, headers: {
        'Content-Type': 'application/json',
    }});
}

}`

vaTheo added a commit to vaTheo/BackEndAutobahn that referenced this issue Sep 29, 2023
@tonyea
Copy link

tonyea commented Sep 29, 2023

Switched to a different jwt library: https://www.npmjs.com/package/njwt

@coding-agent
Copy link

coding-agent commented Sep 29, 2023

Switched to a different jwt library: https://www.npmjs.com/package/njwt

It seem to be a good solution too.

The problem is that some important dependencies use jsonwebtoken in the background like "@nest/jsonwebtoken", "passport-jwt", "firebase-admin", "express-jwt" which impacts some projects.

@andres-linares-int
Copy link

Exactly. I am using passport in one of my projects and I had to use a custom strategy using jose instead of passport-jwt.
This ended up being kinda simple but I imagine some situations where a custom implementation is not feasible...

@tonyea
Copy link

tonyea commented Sep 29, 2023

Switched to a different jwt library: https://www.npmjs.com/package/njwt

It seem to be a good solution too.

The problem is that some important dependencies use jsonwebtoken in the background like "@nest/jsonwebtoken", "passport-jwt", "firebase-admin", "express-jwt" which impacts some projects.

Exactly. I am using passport in one of my projects and I had to use a custom strategy using jose instead of passport-jwt. This ended up being kinda simple but I imagine some situations where a custom implementation is not feasible...

Yea, even njwt borked for me on build. Jose seems to work for now.

@muchekechege
Copy link

muchekechege commented Oct 4, 2023

bun sucks.Errors everywhere bcrypto not working jwt and even spinning multiple a cluster

@DWboutin
Copy link

DWboutin commented Oct 6, 2023

@tonyea did you use passport-jwt in your project? I used jose to sign my token, but when verifying it with passport.authenticate('jwt', { session: false }), it does

TypeError: Right hand side of instanceof is not an object
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/jsonwebtoken/verify.js:120:10)
    at getSecret (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/jsonwebtoken/verify.js:96:51)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/jsonwebtoken/verify.js:101:7)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/verify_jwt.js:3:59)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/strategy.js:104:20)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/strategy.js:39:42)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/strategy.js:99:12)
    at attempt (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport/lib/middleware/authenticate.js:369:8)
    at authenticate (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport/lib/middleware/authenticate.js:369:8)
   ...

@danny-avila
Copy link

Exactly. I am using passport in one of my projects and I had to use a custom strategy using jose instead of passport-jwt. This ended up being kinda simple but I imagine some situations where a custom implementation is not feasible...

Do you mind sharing this? working on my own right now

@tonyea
Copy link

tonyea commented Oct 6, 2023

@tonyea did you use passport-jwt in your project? I used jose to sign my token, but when verifying it with passport.authenticate('jwt', { session: false }), it does

TypeError: Right hand side of instanceof is not an object
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/jsonwebtoken/verify.js:120:10)
    at getSecret (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/jsonwebtoken/verify.js:96:51)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/jsonwebtoken/verify.js:101:7)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/verify_jwt.js:3:59)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/strategy.js:104:20)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/strategy.js:39:42)
    at <anonymous> (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport-jwt/lib/strategy.js:99:12)
    at attempt (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport/lib/middleware/authenticate.js:369:8)
    at authenticate (/Users/mikaelboutin/Desktop/coding/bun-backend/node_modules/passport/lib/middleware/authenticate.js:369:8)
   ...

Sorry no, didn't use passport so wouldn't have good advice

@userunknownn
Copy link

I figured I might as well link to the source, the screenshot in the report is referencing this line in jsonwebtoken (https://github.com/auth0/node-jsonwebtoken/blob/master/sign.js#L114), which is the same reson I landed here.

There is also an issue on jsonwebtoken pertaining to compatability with bun: auth0/node-jsonwebtoken#939

Downgrading jwt to 8.5.1 worked for me, just as @AlexRoosWork replied on that issue

@skeie
Copy link

skeie commented Oct 13, 2023

Upgraded to bun 1.06 and it seems to be solved 🎉 ref #5940

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
atw enhancement New feature or request node.js Compatibility with Node.js APIs
Projects
None yet
Development

Successfully merging a pull request may close this issue.