Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| app.get('/sso', async (request: FastifyRequest, reply: FastifyReply) => { | ||
| if (!isAuth0Configured()) { | ||
| return reply.status(501).send({ | ||
| error: { code: 'SSO_NOT_CONFIGURED', message: 'SSO is not configured on this server' }, | ||
| }); | ||
| } | ||
|
|
||
| const querySchema = z.object({ | ||
| connection: z.string().optional(), | ||
| }); | ||
| const query = querySchema.safeParse(request.query); | ||
| const connection = query.success ? query.data.connection : undefined; | ||
|
|
||
| const state = crypto.randomUUID(); | ||
|
|
||
| reply.setCookie('sso_state', state, { | ||
| httpOnly: true, | ||
| secure: isProduction, | ||
| sameSite: 'lax', | ||
| maxAge: 600, | ||
| path: '/', | ||
| }); | ||
|
|
||
| const authUrl = getAuth0AuthorizeUrl(state, connection); | ||
| logger.debug({ connection }, 'Redirecting to Auth0 SSO'); | ||
|
|
||
| return reply.redirect(authUrl); | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, the /sso route should be configured with rate limiting using the same mechanism already used for /sso/callback. Fastify’s rate limit plugin supports per-route configuration via a config.rateLimit object in the route options. The best low-impact fix is therefore to wrap the /sso handler in a route definition that includes config: { rateLimit: { ... } }, mirroring the existing callback configuration but possibly with slightly different limits appropriate for an initiation endpoint.
Concretely, in apps/server/src/routes/auth.ts, edit the app.get('/sso', ...) declaration (around lines 225–253) to add a second argument: a route options object with a config.rateLimit property, before the async handler function. For example, configure a modest per-IP rate limit such as max: 30, timeWindow: '1 minute'. No new imports are needed because Fastify picks up these options via the already-registered rate-limit plugin (as evidenced by the /sso/callback route using the same pattern). The rest of the handler body (Auth0 configuration check, cookie setting, redirect) remains unchanged.
| @@ -223,35 +223,43 @@ | ||
| // ─── Auth0 SSO Routes ─────────────────────────────────────────────── | ||
|
|
||
| // GET /sso - Redirect to Auth0 for SSO login | ||
| app.get('/sso', async (request: FastifyRequest, reply: FastifyReply) => { | ||
| if (!isAuth0Configured()) { | ||
| return reply.status(501).send({ | ||
| error: { code: 'SSO_NOT_CONFIGURED', message: 'SSO is not configured on this server' }, | ||
| app.get( | ||
| '/sso', | ||
| { | ||
| config: { | ||
| rateLimit: { max: 30, timeWindow: '1 minute' }, | ||
| }, | ||
| }, | ||
| async (request: FastifyRequest, reply: FastifyReply) => { | ||
| if (!isAuth0Configured()) { | ||
| return reply.status(501).send({ | ||
| error: { code: 'SSO_NOT_CONFIGURED', message: 'SSO is not configured on this server' }, | ||
| }); | ||
| } | ||
|
|
||
| const querySchema = z.object({ | ||
| connection: z.string().optional(), | ||
| }); | ||
| } | ||
| const query = querySchema.safeParse(request.query); | ||
| const connection = query.success ? query.data.connection : undefined; | ||
|
|
||
| const querySchema = z.object({ | ||
| connection: z.string().optional(), | ||
| }); | ||
| const query = querySchema.safeParse(request.query); | ||
| const connection = query.success ? query.data.connection : undefined; | ||
| const state = crypto.randomUUID(); | ||
|
|
||
| const state = crypto.randomUUID(); | ||
| reply.setCookie('sso_state', state, { | ||
| httpOnly: true, | ||
| secure: isProduction, | ||
| sameSite: 'lax', | ||
| maxAge: 600, | ||
| path: '/', | ||
| }); | ||
|
|
||
| reply.setCookie('sso_state', state, { | ||
| httpOnly: true, | ||
| secure: isProduction, | ||
| sameSite: 'lax', | ||
| maxAge: 600, | ||
| path: '/', | ||
| }); | ||
| const authUrl = getAuth0AuthorizeUrl(state, connection); | ||
| logger.debug({ connection }, 'Redirecting to Auth0 SSO'); | ||
|
|
||
| const authUrl = getAuth0AuthorizeUrl(state, connection); | ||
| logger.debug({ connection }, 'Redirecting to Auth0 SSO'); | ||
| return reply.redirect(authUrl); | ||
| } | ||
| ); | ||
|
|
||
| return reply.redirect(authUrl); | ||
| }); | ||
|
|
||
| // GET /sso/callback - Auth0 SSO callback | ||
| app.get( | ||
| '/sso/callback', |
| async (request: FastifyRequest, reply: FastifyReply) => { | ||
| if (!isAuth0Configured()) { | ||
| return reply.status(501).send({ | ||
| error: { code: 'SSO_NOT_CONFIGURED', message: 'SSO is not configured' }, | ||
| }); | ||
| } | ||
|
|
||
| const callbackSchema = z.object({ | ||
| code: z.string().min(1), | ||
| state: z.string().min(1), | ||
| error: z.string().optional(), | ||
| error_description: z.string().optional(), | ||
| }); | ||
|
|
||
| const result = callbackSchema.safeParse(request.query); | ||
| if (!result.success) { | ||
| throw new ValidationError('Invalid SSO callback parameters'); | ||
| } | ||
|
|
||
| const { code, state, error, error_description } = result.data; | ||
|
|
||
| /* Validate CSRF state */ | ||
| const storedState = request.cookies.sso_state; | ||
| if (!state || !storedState || state !== storedState) { | ||
| logger.warn('SSO state mismatch'); | ||
| return reply.status(400).send({ | ||
| error: { code: 'INVALID_STATE', message: 'Invalid SSO state parameter' }, | ||
| }); | ||
| } | ||
| reply.clearCookie('sso_state'); | ||
|
|
||
| if (error) { | ||
| logger.warn({ error, error_description }, 'Auth0 SSO error'); | ||
| return reply.status(400).send({ | ||
| error: { code: 'SSO_ERROR', message: error_description ?? error }, | ||
| }); | ||
| } | ||
|
|
||
| /* Exchange code for tokens and fetch user info */ | ||
| const tokens = await exchangeAuth0Code(code); | ||
| const userInfo = await getAuth0UserInfo(tokens.access_token); | ||
|
|
||
| const db = getDb(); | ||
|
|
||
| /* Find existing user by email or create a new one */ | ||
| let user = await db.user.findFirst({ | ||
| where: { email: userInfo.email }, | ||
| }); | ||
|
|
||
| if (!user) { | ||
| user = await db.user.create({ | ||
| data: { | ||
| githubId: `auth0_${userInfo.sub}`, | ||
| username: userInfo.nickname ?? userInfo.email.split('@')[0] ?? 'user', | ||
| displayName: userInfo.name || null, | ||
| avatarUrl: userInfo.picture || null, | ||
| email: userInfo.email, | ||
| tier: 'TEAM', | ||
| }, | ||
| }); | ||
|
|
||
| logger.info({ userId: user.id, email: userInfo.email }, 'New SSO user created'); | ||
| } | ||
|
|
||
| /* Generate JWT */ | ||
| const token = app.jwt.sign( | ||
| { | ||
| userId: user.id, | ||
| username: user.username, | ||
| tier: user.tier, | ||
| }, | ||
| { expiresIn: '7d' } | ||
| ); | ||
|
|
||
| logger.info({ userId: user.id }, 'SSO authentication successful'); | ||
|
|
||
| /* Redirect to web app with token */ | ||
| const webUrl = new URL('/dashboard', env.WEB_APP_URL); | ||
| webUrl.searchParams.set('token', token); | ||
| return reply.redirect(webUrl.toString()); | ||
| } |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
Generally, the fix is to ensure that this authorization route is explicitly rate-limited at the Fastify route level, using the standard rateLimit option that works with the @fastify/rate-limit plugin. This avoids reliance on non-standard configuration keys that static analysis might miss and guarantees that the handler cannot be abused for denial-of-service via excessive Auth0 callbacks.
Concretely, in apps/server/src/routes/auth.ts, modify the /sso/callback route definition to move the existing config.rateLimit configuration into the top-level rateLimit property of the route options object:
app.get(
'/sso/callback',
{
rateLimit: {
max: 10,
timeWindow: '1 minute',
},
},
async (request, reply) => { ... }
);This keeps the same limits (10 requests per minute per IP, assuming default plugin behavior) while using the canonical configuration shape recognized by @fastify/rate-limit. No other logic changes are needed. Because we must not change imports beyond well-known libraries, and we cannot see plugin registration in this file, we limit ourselves to changing only the route options object; the actual plugin registration (e.g., app.register(require('@fastify/rate-limit'), ...)) would remain elsewhere in the codebase.
| @@ -256,8 +256,9 @@ | ||
| app.get( | ||
| '/sso/callback', | ||
| { | ||
| config: { | ||
| rateLimit: { max: 10, timeWindow: '1 minute' }, | ||
| rateLimit: { | ||
| max: 10, | ||
| timeWindow: '1 minute', | ||
| }, | ||
| }, | ||
| async (request: FastifyRequest, reply: FastifyReply) => { |
| app.get('/sso/status', async (_request: FastifyRequest, reply: FastifyReply) => { | ||
| return reply.send({ | ||
| enabled: isAuth0Configured(), | ||
| }); | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, the /sso/status route should be explicitly rate-limited, just like /sso/callback. In Fastify, this is typically done via the config.rateLimit option on the route definition (assuming the fastify-rate-limit plugin is registered at the app level). We will wrap the handler in the object-based app.get(path, options, handler) form and add a conservative rate limit configuration.
Concretely, in apps/server/src/routes/auth.ts, locate the app.get('/sso/status', async (_request: FastifyRequest, reply: FastifyReply) => { ... }); definition near lines 346–351. Replace it with a three-argument call: the same path, an options object containing config: { rateLimit: { max: 60, timeWindow: '1 minute' } } (or similar reasonable values), and the existing async handler function unchanged. No new imports or helper methods are required, because the file already uses config.rateLimit for /sso/callback, implying the necessary plugin is configured elsewhere.
| @@ -344,9 +344,17 @@ | ||
| ); | ||
|
|
||
| // GET /sso/status - Check if SSO is available | ||
| app.get('/sso/status', async (_request: FastifyRequest, reply: FastifyReply) => { | ||
| return reply.send({ | ||
| enabled: isAuth0Configured(), | ||
| }); | ||
| }); | ||
| app.get( | ||
| '/sso/status', | ||
| { | ||
| config: { | ||
| rateLimit: { max: 60, timeWindow: '1 minute' }, | ||
| }, | ||
| }, | ||
| async (_request: FastifyRequest, reply: FastifyReply) => { | ||
| return reply.send({ | ||
| enabled: isAuth0Configured(), | ||
| }); | ||
| } | ||
| ); | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796f2423cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // TODO: Send invitation email with token | ||
| // Send invitation email (non-blocking) | ||
| const joinUrl = `${env.WEB_APP_URL}/dashboard/teams/${teamId}/join?token=${token}`; |
There was a problem hiding this comment.
This link targets /dashboard/teams/${teamId}/join, but the web app only has apps/web/src/app/dashboard/teams/page.tsx and apps/web/src/app/dashboard/teams/[id]/page.tsx, so recipients who click the email land on a 404 and cannot redeem the invitation token. The new email invitation flow is therefore broken for real users.
Useful? React with 👍 / 👎.
| <div className="flex min-h-screen bg-background"> | ||
| <Sidebar isConnected={isConnected} /> | ||
| <main className="flex-1 min-w-0">{children}</main> | ||
| <ConflictToastListener /> |
There was a problem hiding this comment.
Adding this listener mounts another useWebSocket instance while DashboardShell already keeps one active, so one page now creates two simultaneous sockets for the same user. The server side WebSocket handler keeps a single connection per user and closes the previous one when a new one appears, which makes these two clients repeatedly disconnect/reconnect each other and causes unstable realtime behavior.
Useful? React with 👍 / 👎.
|
|
||
| /* Redirect to web app with token */ | ||
| const webUrl = new URL('/dashboard', env.WEB_APP_URL); | ||
| webUrl.searchParams.set('token', token); |
There was a problem hiding this comment.
Putting the signed session token into ?token= exposes a bearer credential in URL logs, browser history, and potential Referer leakage before client code strips it. This creates a concrete token disclosure path for every SSO login and should be replaced with a safer handoff (for example, HttpOnly cookie or one-time code exchange).
Useful? React with 👍 / 👎.
| const callbackSchema = z.object({ | ||
| code: z.string().min(1), | ||
| state: z.string().min(1), |
There was a problem hiding this comment.
The callback schema requires code even when Auth0 returns an OAuth error response (error/error_description with no code), so those valid failure callbacks are rejected as Invalid SSO callback parameters before your explicit if (error) handling runs. Users then get the wrong failure path instead of the intended SSO error response.
Useful? React with 👍 / 👎.
1 participant
No description provided.