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

Unsuccessful Callback to Unknow client side error #622

Comments

@BRAVO68WEB
Copy link

BRAVO68WEB commented Sep 16, 2023

Describe the bug

To Reproduce
Client configuration:

{
    "client_secret": process.env.KEYCLOAK_CLIENT_SECRET,
    "default_max_age": 3600000,
    "realm": "master",
    "auth-server-url": process.env.KEYCLOAK_URL,
    "ssl-required": "external",
    "resource": process.env.KEYCLOAK_CLIENT_ID,
    "verify-token-audience": true,
    "credentials": {
        "secret": process.env.KEYCLOAK_CLIENT_SECRET
    },
    "confidential-port": 0,
    "policy-enforcer": {
        "credentials": {}
    },
    "token_endpoint_auth_method": 'client_secret_jwt'
}

Steps to reproduce the behaviour:

  1. Initiate Login
  2. Login as user
  3. Wait for Callback

Expected behaviour
It works well, I should be able to get access_token after a successfull callback

Environment:

  • openid-client version: v5.5.0
  • node version: v18.17.1

Additional context
Add any other context about the problem here.

  • the bug is happening on latest openid-client too.
  • i have searched the issues tracker on github for similar issues and couldn't find anything related.

Solution

  • Commented the Code and it worked without a problem
// if (!isKeyObject(keyObject)) {
//   throw new Error('what?!');
// }
  • I want to know why this check was created first place
  • If this is expected as solution, can I create a PR? 🐱
@BRAVO68WEB BRAVO68WEB changed the title Unsuccesful Callback to Unknow client side error Unsuccessful Callback to Unknow client side error Sep 16, 2023
@panva
Copy link
Owner

panva commented Sep 16, 2023

Hi @BRAVO68WEB

Are you running your code in Node.js, or some other runtime like Bun or Deno?

@BRAVO68WEB
Copy link
Author

BRAVO68WEB commented Sep 16, 2023

Bun

Let me check with node too.

@BRAVO68WEB
Copy link
Author

Yep, I just now tried with node, it worked.

But, still can tell me why is that check present there?

@panva
Copy link
Owner

panva commented Sep 16, 2023

Well, this is a Node.js-only module. https://github.com/panva/node-openid-client/blob/main/README.md#how-do-i-use-it-outside-of-nodejs

How do I use it outside of Node.js

It is only built for Node.js. Other javascript runtimes are not supported. I recommend panva/oauth4webapi, or a derivate thereof, if you're looking for a similarly compliant and certified client software that's not dependent on the Node.js runtime builtins.

I can explain what's happening. jose dependency, which is used to import the JWK key material is a universal library.

So what happens is that Bun resolves jose's bun entrypoint which uses native WebCryptoAPI and ends up with a CryptoKey, not node's own crypto.KeyObject. So the check is doing exactly what it should be doing, ensuring it's working with node's crypto KeyObject.

For now I can only say that this module is meant to be used only in Node.js and this is not a bug. That being said, I can look into testing the library in Bun and/or Deno as well but it's not a priority for me.

@panva panva removed the triage label Sep 16, 2023
@BRAVO68WEB
Copy link
Author

Thank you @panva for clearing my doubt pretty fast 😸.

@panva
Copy link
Owner

panva commented Sep 16, 2023

@Jarred-Sumner is there anything that can be done to have bun force to resolve a given dependency using a different algorithm (see my comment above)?

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.

@BRAVO68WEB
Copy link
Author

@panva I will try, and check back to you

@BRAVO68WEB
Copy link
Author

Hey @panva it works like a charm.
But currently facing a small issue relates to jose.

[0.04ms] ".env"
[1]    30000 segmentation fault  bun run -b --port 4000 --hot index.ts

I get this error when try to verify JWT with createRemoteJWKSet and jwtVerify from jose.

Waiting for your PR auth0/node-jwks-rsa#374 to get merged, then it will work fine.

@panva
Copy link
Owner

panva commented Oct 4, 2023

unlikely to be jose related, just jose triggered, please open an issue with bun. https://github.com/oven-sh/bun/issues

@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2024
@panva
Copy link
Owner

panva commented Oct 7, 2024

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.