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

Generic OAuth error "due to an internal error" for "invalid_client_metadata" #3096

Closed
Gregoor opened this issue Nov 25, 2024 · 12 comments · Fixed by #3135
Closed

Generic OAuth error "due to an internal error" for "invalid_client_metadata" #3096

Gregoor opened this issue Nov 25, 2024 · 12 comments · Fixed by #3135
Labels
bug Something isn't working

Comments

@Gregoor
Copy link

Gregoor commented Nov 25, 2024

Hi there 👋🏼

Describe the bug

In prod OAuth fails for me with the following (it works in dev):

⨯ Error: OAuth "invalid_client_metadata" error: Failed to fetch client information due to an internal error
    at h.request (/var/task/web/.next/server/chunks/533.js:60:238928)
    at async c.authorize (/var/task/web/.next/server/chunks/533.js:60:227047)
    ... {
  response: Response {
    status: 400,
    statusText: 'Bad Request',
    headers: Headers {
      date: 'Mon, 25 Nov 2024 00:39:58 GMT',
      'content-type': 'application/json',
      'transfer-encoding': 'chunked',
      connection: 'keep-alive',
      'x-powered-by': 'Express',
      'access-control-allow-origin': '*',
      'access-control-allow-headers': '*',
      'cache-control': 'no-store',
      pragma: 'no-cache',
      'dpop-nonce': 'Jw-9j3pBbjXCne6NHHT01L4Yq3xGRWheJ45l-blIVMA',
      'access-control-expose-headers': 'DPoP-Nonce',
      vary: 'accept-encoding',
      'content-encoding': 'br'
    },
    body: undefined,
    bodyUsed: true,
    ok: false,
    redirected: false,
    type: 'basic',
    url: 'https://bsky.social/oauth/par'
  },
  payload: [Object],
  error: 'invalid_client_metadata',
  errorDescription: 'Failed to fetch client information due to an internal error',
  digest: '1499086769'
}

To Reproduce

This is my OAuth config:

const IS_DEV = process.env.NODE_ENV == "development";
const ORIGIN = IS_DEV ? "http://127.0.0.1:3000" : "https://skylights.my";

const abs = (s: string) => `${ORIGIN}/${s}`;

const SCOPE = "atproto transition:generic";
const REDIRECT_URI = abs("atproto-oauth-callback");

export const authClient = new NodeOAuthClient({
  clientMetadata: {
    client_id: IS_DEV
      ? `http://localhost?redirect_uri=${enc(REDIRECT_URI)}&scope=${enc(SCOPE)}`
      : `${ORIGIN}/oauth/client-metadata.json`,
    client_name: "Skylights",
    client_uri: ORIGIN,
    tos_uri: abs("tos"),
    policy_uri: abs("policy"),
    redirect_uris: [REDIRECT_URI],
    grant_types: ["authorization_code", "refresh_token"],
    response_types: ["code"],
    application_type: "web",
    scope: SCOPE,
    token_endpoint_auth_method: "none",
    dpop_bound_access_tokens: true,
  },
  ...stores
});

Hope I gave enough info, please let me know if not!

@Gregoor Gregoor added the bug Something isn't working label Nov 25, 2024
@mikestaub
Copy link

I get a 404 when I curl that endpoint, it needs to return the client-metadata.json content and it must match with what is passed to the NodeOAuthClient:

curl -vL https://skylights.my/client-metadata.json

I built a passport strategy you might want to try with your node app, or at least look at the working example: https://github.com/mikestaub/passport-atprotocol

@Gregoor
Copy link
Author

Gregoor commented Nov 25, 2024

Oops, I did a bad job of keeping this example in sync while I was trying things today. The new metadata URL is:
https://www.skylights.my/oauth/client-metadata.json (which does go through now, and the problem persists).

Thanks for the library as well, I will give it a read!

@Gregoor
Copy link
Author

Gregoor commented Nov 26, 2024

I thought the lack of keysets and a token endpoint auth method might be the culprit so, thanks to Tyler from Sill, I now have a locally working version with keys that looks like this:

const privateKeyPKCS8 = Buffer.from(
  process.env.PRIVATE_KEY_ES256_B64 as string,
  "base64",
).toString();
const privateKey = await JoseKey.fromImportable(privateKeyPKCS8, "key1");

const isDev = process.env.NODE_ENV == "development";
const origin = isDev ? "http://127.0.0.1:3000" : "https://skylights.my";

const abs = (s: string) => `${origin}/${s}`;
const enc = encodeURIComponent;

const SCOPE = "atproto transition:generic";
const REDIRECT_URI = abs("oauth/atproto-callback");

export const authClient = new NodeOAuthClient({
  clientMetadata: {
    client_id: isDev
      ? `http://localhost?redirect_uri=${enc(REDIRECT_URI)}&scope=${enc(SCOPE)}`
      : abs("oauth/client-metadata.json"),
    client_name: "Skylights",
    client_uri: origin,
    redirect_uris: [REDIRECT_URI],
    grant_types: ["authorization_code", "refresh_token"],
    response_types: ["code"],
    application_type: "web",
    scope: SCOPE,
    token_endpoint_auth_method: "private_key_jwt",
    token_endpoint_auth_signing_alg: "ES256",
    dpop_bound_access_tokens: true,
    jwks_uri: abs("oauth/jwks.json"),
  },

  keyset: [privateKey],
  
  ...stores
});

Unfortunately it still fails with the exact same error on prod :(

@Gregoor Gregoor changed the title Generic OAuth error "invalid_client_metadata" Generic OAuth error " Nov 26, 2024
@Gregoor Gregoor changed the title Generic OAuth error " Generic OAuth error "due to an internal error" for "invalid_client_metadata" Nov 26, 2024
@Gregoor
Copy link
Author

Gregoor commented Nov 26, 2024

I am wondering if it would be possible to add info about which FetchError failed, maybe adding status code and path wouldn't be too much of an information leak?

if (err instanceof FetchError) {
const message =
err instanceof FetchResponseError || err.statusCode !== 500
? // Only expose 500 message if it was generated on another server
`Failed to fetch client information: ${err.message}`
: `Failed to fetch client information due to an internal error`
throw new InvalidClientMetadataError(message, err)
}

@matthieusieben would you be open to such a PR of mine (I saw you touching these lines last, hope it's not inappropriate that I tag you)?

Update: Sorry that didn't make sense, just realized that the original error is bubbled out as well.

@mikestaub
Copy link

invalid_client_metadata likely means they simply do not match. Can you try logging out the JSON of your client metadata on the server and then diffing it with the json response from your public endpoint?

@Gregoor
Copy link
Author

Gregoor commented Nov 27, 2024

I just logged authClient.clientMetadata and compared it with my endpoint's output. They are both exactly this:

{
    "application_type": "web",
    "client_id": "https://skylights.my/oauth/client-metadata.json",
    "client_name": "Skylights",
    "client_uri": "https://skylights.my",
    "dpop_bound_access_tokens": true,
    "grant_types": [
        "authorization_code",
        "refresh_token"
    ],
    "jwks_uri": "https://skylights.my/oauth/jwks.json",
    "redirect_uris": [
        "https://skylights.my/oauth/atproto-callback"
    ],
    "response_types": [
        "code"
    ],
    "scope": "atproto transition:generic",
    "token_endpoint_auth_method": "private_key_jwt",
    "token_endpoint_auth_signing_alg": "ES256"
}

Thanks for the idea, happy to investigate any avenue at this point 😵‍💫

@mikestaub
Copy link

strange, that looks good to me. The last thing I would try is removing the "refresh_token" from the metadata

@Gregoor
Copy link
Author

Gregoor commented Nov 27, 2024

No luck with that one either, appreciate the attempt tho!

@mikestaub
Copy link

How did you generate your private key? Here is a working example: https://github.com/mikestaub/passport-atprotocol/blob/main/scripts/generate-keys.js

@Gregoor
Copy link
Author

Gregoor commented Nov 28, 2024

A friend helped me solve it! It was something stupid that felt unrelated to me. I though my main domain was the one without WWW and that the one with redirected towards it. Turned out it was the other way around.

So the obtuse error because instead of getting a 200 for the client metadata it got a 308. It would be very helpful if that error would bubble out though, so I'll keep this issue open.

Thank you for holding my hand @mikestaub!

@matthieusieben
Copy link
Contributor

Closing as #3135 will improve error reporting (in particular in the case of an invalid redirect).

@matthieusieben
Copy link
Contributor

Thansk for the report

matthieusieben added a commit that referenced this issue Nov 29, 2024
* Improve messaging of client metadata loading errors

Fixes #3096

* Support parsing of more fetch() errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants