Conversation
| : null; | ||
|
|
||
| const displayName = name ?? email?.split("@")[0] ?? "User"; | ||
| const displayName = userInfo?.name ?? "User"; |
There was a problem hiding this comment.
Email fallback for
displayName dropped
The old code used email?.split("@")[0] as a secondary fallback when name wasn't present. Now that userInfo already contains email (since the /userinfo endpoint returns it for the email scope), the fallback can be restored without any extra requests.
| const displayName = userInfo?.name ?? "User"; | |
| const displayName = userInfo?.name ?? userInfo?.email?.split("@")[0] ?? "User"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/auth-auth0/server/src/index.ts
Line: 111
Comment:
**Email fallback for `displayName` dropped**
The old code used `email?.split("@")[0]` as a secondary fallback when `name` wasn't present. Now that `userInfo` already contains `email` (since the `/userinfo` endpoint returns it for the `email` scope), the fallback can be restored without any extra requests.
```suggestion
const displayName = userInfo?.name ?? userInfo?.email?.split("@")[0] ?? "User";
```
How can I resolve this? If you propose a fix, please make it concise.| oauthMetadata: { | ||
| issuer: env.SERVER_URL, | ||
| authorization_endpoint: `https://${env.AUTH0_DOMAIN}/authorize?audience=${encodeURIComponent(env.AUTH0_AUDIENCE)}`, | ||
| authorization_endpoint: `https://${env.AUTH0_DOMAIN}/authorize?audience=${encodeURIComponent(env.AUTH0_AUDIENCE)}&scope=openid%20profile%20email`, |
There was a problem hiding this comment.
Hardcoded
scope in authorization_endpoint may conflict with client-supplied scopes
Baking scope=openid%20profile%20email directly into the authorization_endpoint URL means that when an MCP client constructs the authorization URL, it will typically append its own scope parameter (per the OAuth spec), resulting in a URL with two scope params (e.g. ?...&scope=openid%20profile%20email&scope=openid). RFC 6749 doesn't define behaviour for duplicate query parameters, and different implementations handle this differently.
A more standard pattern is to leave the base endpoint clean and rely on scopes_supported to advertise what scopes are available — the client will then request them:
| authorization_endpoint: `https://${env.AUTH0_DOMAIN}/authorize?audience=${encodeURIComponent(env.AUTH0_AUDIENCE)}&scope=openid%20profile%20email`, | |
| authorization_endpoint: `https://${env.AUTH0_DOMAIN}/authorize?audience=${encodeURIComponent(env.AUTH0_AUDIENCE)}`, |
The scopes_supported metadata already advertises ["openid", "profile", "email"], which tells conforming clients which scopes to request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/auth-auth0/server/src/index.ts
Line: 39
Comment:
**Hardcoded `scope` in `authorization_endpoint` may conflict with client-supplied scopes**
Baking `scope=openid%20profile%20email` directly into the `authorization_endpoint` URL means that when an MCP client constructs the authorization URL, it will typically append its own `scope` parameter (per the OAuth spec), resulting in a URL with two `scope` params (e.g. `?...&scope=openid%20profile%20email&scope=openid`). RFC 6749 doesn't define behaviour for duplicate query parameters, and different implementations handle this differently.
A more standard pattern is to leave the base endpoint clean and rely on `scopes_supported` to advertise what scopes are available — the client will then request them:
```suggestion
authorization_endpoint: `https://${env.AUTH0_DOMAIN}/authorize?audience=${encodeURIComponent(env.AUTH0_AUDIENCE)}`,
```
The `scopes_supported` metadata already advertises `["openid", "profile", "email"]`, which tells conforming clients which scopes to request.
How can I resolve this? If you propose a fix, please make it concise.
Fixed
nameandemailfrom the Access Token, but these will never be returned in an AT according to the OAuth/OpenID spec. Thesearch-coffee-paristool now fetches the/userinfoendpoint using the Access Token to get more user information, like email and name.Auth.jsto use the@auth0/auth0-api-jspackage, which has a usefulverifyAccessToken()method and avoids possible mistakes in trying to validate tokens.client_idorazpclaim as theclientIdinstead of thesub. Auth0 returns either aclient_idorazpclaim in AT, depending on which AT profile is configured.client_id: the Client ID for which an access Token is issued, RFC 9068.azp: Authorized party, the Client ID for which an Access Token is issued. (not standardised for Access Tokens)sub: Subject, the user identifier for which an Access Token is issued.Greptile Summary
This PR fixes the Auth0 example by correctly fetching user identity information from the
/userinfoendpoint (rather than from Access Token claims which aren't present per the OAuth/OpenID spec), switching to@auth0/auth0-api-jsfor token verification, and properly mappingclientIdto theclient_id/azpclaim andscopesto the token'sscopeclaim.Key changes:
auth.ts: Replaces manualjose-based JWT verification withApiClient.verifyAccessToken()from@auth0/auth0-api-js.clientIdis now correctly sourced fromclient_id/azpand scopes from thescopeclaim.index.ts: Tool handler now calls/userinfowith the Access Token to retrievename/emailfor the display name.scopes_supportedmetadata updated to includeprofileandemail.package.json: Adds@auth0/auth0-api-js; however the now-unusedjosepackage was not removed.Findings:
josedependency is no longer used after the rewrite but remains inpackage.json.displayNamefallback (email?.split("@")[0]) was dropped —userInfo.emailis available from the/userinforesponse and could be restored as a secondary fallback before"User".scope=openid%20profile%20emailis hardcoded in theauthorization_endpointURL; OAuth clients typically append their ownscopeparameter, which may result in duplicatescopequery params with undefined precedence across implementations.Confidence Score: 5/5
Safe to merge; all findings are minor style/cleanup items that do not affect correctness of the core auth flow.
All three issues identified are P2: an unused dependency leftover, a slightly degraded display-name fallback, and a potentially confusing (but typically harmless) double-scope URL pattern. None block functionality or introduce a security regression.
No files require special attention, though
package.jsonshould have the unusedjosedependency removed for cleanliness.Important Files Changed
Comments Outside Diff (1)
examples/auth-auth0/package.json, line 19 (link)josedependencyjoseis still listed as a dependency but is no longer imported anywhere in the codebase after the rewrite to@auth0/auth0-api-js. It can be removed.(Delete the
"jose": "^6.2.1",line entirely.)Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Cleanup" | Re-trigger Greptile
(4/5) You can add custom instructions or style guidelines for the agent here!