Skip to content

feat: support guest session #8

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: support guest session #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3583cb2
feat: support guest session
jeremyphilemon Apr 9, 2025
fc78794
rework middleware logic
jeremyphilemon Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/(auth)/api/auth/guest/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { redirect } from 'next/navigation';
import { auth, signIn } from '@/app/(auth)/auth';

export async function GET() {
const session = await auth();

if (!session?.user?.id) {
await signIn('guest', { redirect: false });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Fixation Risk

Guest authentication doesn't invalidate existing session tokens. Attackers could potentially set a session identifier, then force guest auth to maintain control. This enables session fixation attacks.

// First destroy any existing session to prevent session fixation
if (session) {
  await signOut({ redirect: false });
}
await signIn('guest', { redirect: false });
Standards
  • CWE-384
  • OWASP-A07

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Error Handling

The guest authentication call lacks error handling. Failed authentication attempts will cause unhandled promise rejections, potentially crashing the application or leaving users in an inconsistent state.

Suggested change
await signIn('guest', { redirect: false });
try {
const result = await signIn('guest', { redirect: false });
if (!result?.ok) {
return new Response('Authentication failed', { status: 500 });
}
redirect('/');
} catch (error) {
console.error('Guest authentication failed:', error);
return new Response('Authentication failed', { status: 500 });
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • SRE-Error-Handling

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Error Handling

The guest authentication endpoint lacks error handling for the signIn operation. If authentication fails, the function will still redirect users, potentially leading to an inconsistent authentication state and service disruption.

    try {
      const result = await signIn('guest', { redirect: false });
      if (!result?.ok) {
        return new Response('Authentication failed', { status: 500 });
      }
      redirect('/');
    } catch (error) {
      console.error('Guest authentication failed:', error);
      return new Response('Authentication failed', { status: 500 });
    }
Commitable Suggestion
Suggested change
await signIn('guest', { redirect: false });
try {
const result = await signIn('guest', { redirect: false });
if (!result?.ok) {
return new Response('Authentication failed', { status: 500 });
}
redirect('/');
} catch (error) {
console.error('Guest authentication failed:', error);
return new Response('Authentication failed', { status: 500 });
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • SRE-Error-Handling

redirect('/');
Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guest Auth Loop

Redirecting to '/' after guest sign-in will trigger middleware again, potentially creating an infinite auth loop. The middleware will detect no session during redirect processing and send to /api/auth/guest again.

  if (!session?.user?.id) {
    const result = await signIn('guest', { redirect: false });
    // Check if sign-in was successful before redirecting
    if (result?.ok) {
      redirect('/');
    } else {
      return new Response('Authentication failed', { status: 500 });
    }
  }
Standards
  • Logic-Verification-Control-Flow
  • Business-Rule-Authentication-Flow

Comment on lines +8 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Error Handling

Guest sign-in lacks error handling. If signIn() fails, the code continues to redirect without handling the error, potentially causing navigation to authenticated routes with invalid session state.

try {
  await signIn('guest', { redirect: false });
  redirect('/');
} catch (error) {
  console.error('Failed to create guest session:', error);
  return new Response('Authentication failed', { status: 500 });
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • SRE-Error-Handling

Comment on lines +4 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Rate Limiting

Guest account creation lacks rate limiting by IP. Attackers can create unlimited guest accounts through API endpoint. This enables DoS attacks against the database.

import { rateLimit } from '@/lib/rate-limit';

// Create a rate limiter instance - 5 requests per minute per IP
const limiter = rateLimit({
  interval: 60 * 1000, // 1 minute
  uniqueTokenPerInterval: 500, // Max 500 users per interval
});

export async function GET(request: Request) {
  const ip = request.headers.get('x-forwarded-for') || 'anonymous';
  
  try {
    await limiter.check(5, ip); // 5 requests per minute per IP
    const session = await auth();

    if (!session?.user?.id) {
      await signIn('guest', { redirect: false });
      redirect('/');
    }

    return new Response('Unauthorized', { status: 401 });
  } catch {
    return new Response('Too Many Requests', { status: 429 });
  }
}
Standards
  • CWE-770
  • OWASP-A04

Comment on lines +8 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Error Handling

No error handling for signIn failure. If guest authentication fails, the error is not caught, potentially causing unhandled exceptions and service disruption.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • SRE-Error-Handling

Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Missing

After signing in as guest, the code immediately redirects without verifying the session was successfully created. This can lead to redirect loops if signIn fails but the redirect still executes.

Standards
  • Business-Rule-State-Consistency
  • Logic-Verification-Control-Flow

}
Comment on lines +4 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guest Auth Loop

The guest auth endpoint creates a potential redirect loop. If signIn fails, the code will redirect without handling the failure, potentially causing an infinite redirection cycle.

export async function GET() {
  const session = await auth();

  if (!session?.user?.id) {
    try {
      const result = await signIn('guest', { redirect: false });
      if (result?.error) {
        return new Response(`Authentication failed: ${result.error}`, { status: 500 });
      }
      redirect('/');
    } catch (error) {
      return new Response(`Authentication error: ${error.message}`, { status: 500 });
    }
  }
Standards
  • Logic-Verification-ErrorHandling
  • Control-Flow-Completeness


return new Response('Unauthorized', { status: 401 });
Comment on lines +4 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insufficient Error Handling

Guest authentication lacks error handling for signIn failures. If guest authentication fails, the error is not caught, potentially causing unhandled promise rejections and revealing implementation details.

export async function GET() {
  const session = await auth();

  if (!session?.user?.id) {
    try {
      await signIn('guest', { redirect: false });
      redirect('/');
    } catch (error) {
      console.error('Guest authentication failed:', error);
      return new Response('Authentication failed', { status: 500 });
    }
  }

  return new Response('Unauthorized', { status: 401 });
}
Standards
  • CWE-755
  • OWASP-A04

Comment on lines +4 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing CSRF Protection

Guest authentication endpoint lacks CSRF protection for automatic sign-in. Attacker could force authentication via CSRF attack. Enables unwanted account creation and potential tracking.

Suggested change
export async function GET() {
const session = await auth();
if (!session?.user?.id) {
await signIn('guest', { redirect: false });
redirect('/');
}
return new Response('Unauthorized', { status: 401 });
export async function GET(request: Request) {
const session = await auth();
// Already authenticated, no need to create guest session
if (session?.user?.id) {
return new Response('Already authenticated', { status: 200 });
}
// Verify state parameter to prevent CSRF
const url = new URL(request.url);
const stateParam = url.searchParams.get('state');
const { cookies } = request;
const stateCookie = cookies.get('guest_auth_state')?.value;
// Validate state parameter against cookie
if (!stateParam || !stateCookie || stateParam !== stateCookie) {
return new Response('Invalid authentication request', { status: 400 });
}
try {
const result = await signIn('guest', { redirect: false });
if (!result?.ok) {
return new Response('Authentication failed', { status: 500 });
}
// Clear the state cookie
const response = new Response(null, {
status: 302,
headers: { Location: '/' }
});
response.cookies.set('guest_auth_state', '', { maxAge: 0 });
return response;
} catch (error) {
console.error('Guest authentication failed:', error);
return new Response('Authentication failed', { status: 500 });
}
}
Standards
  • CWE-352
  • OWASP-A01

}
28 changes: 1 addition & 27 deletions app/(auth)/auth.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,5 @@ export const authConfig = {
// added later in auth.ts since it requires bcrypt which is only compatible with Node.js
// while this file is also used in non-Node.js environments
],
callbacks: {
authorized({ auth, request: { nextUrl } }) {
const isLoggedIn = !!auth?.user;
const isOnChat = nextUrl.pathname.startsWith('/');
const isOnRegister = nextUrl.pathname.startsWith('/register');
const isOnLogin = nextUrl.pathname.startsWith('/login');

if (isLoggedIn && (isOnLogin || isOnRegister)) {
return Response.redirect(new URL('/', nextUrl as unknown as URL));
}

if (isOnRegister || isOnLogin) {
return true; // Always allow access to register and login pages
}

if (isOnChat) {
if (isLoggedIn) return true;
return false; // Redirect unauthenticated users to login page
}

if (isLoggedIn) {
return Response.redirect(new URL('/', nextUrl as unknown as URL));
}

return true;
},
},
callbacks: {},
} satisfies NextAuthConfig;
24 changes: 18 additions & 6 deletions app/(auth)/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { compare } from 'bcrypt-ts';
import NextAuth, { type User, type Session } from 'next-auth';
import Credentials from 'next-auth/providers/credentials';

import { getUser } from '@/lib/db/queries';
import { createAnonymousUser, getUser } from '@/lib/db/queries';

import { authConfig } from './auth.config';

Expand All @@ -21,12 +21,24 @@ export const {
Credentials({
credentials: {},
async authorize({ email, password }: any) {
const users = await getUser(email);
if (users.length === 0) return null;
// biome-ignore lint: Forbidden non-null assertion.
const passwordsMatch = await compare(password, users[0].password!);
const [user] = await getUser(email);

if (!user) return null;
if (!user.password) return null;

const passwordsMatch = await compare(password, user.password);

if (!passwordsMatch) return null;
return users[0] as any;

return user;
},
}),
Credentials({
id: 'guest',
credentials: {},
async authorize() {
const [anonymousUser] = await createAnonymousUser();
return anonymousUser;
Comment on lines +39 to +41
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Predictable Anonymous Credentials

The guest authentication flow creates anonymous users without requiring any credentials or verification. This allows anyone to access the system as a guest user, potentially bypassing authentication controls and enabling automated abuse. Consider implementing additional verification steps like CAPTCHA for guest sessions.

      async authorize(credentials) {
        // Verify CAPTCHA token if provided
        if (credentials?.captchaToken) {
          try {
            const captchaResponse = await fetch('https://www.google.com/recaptcha/api/siteverify', {
              method: 'POST',
              headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
              body: `secret=${process.env.RECAPTCHA_SECRET_KEY}&response=${credentials.captchaToken}`
            });
            
            const captchaResult = await captchaResponse.json();
            if (!captchaResult.success) {
              console.error('CAPTCHA verification failed');
              return null;
            }
          } catch (error) {
            console.error('CAPTCHA verification error:', error);
            return null;
          }
        } else if (process.env.NODE_ENV === 'production') {
          // In production, require CAPTCHA
          console.error('Missing CAPTCHA token for guest authentication');
          return null;
        }
        
        const [anonymousUser] = await createAnonymousUser();
        return anonymousUser;
Commitable Suggestion
Suggested change
async authorize() {
const [anonymousUser] = await createAnonymousUser();
return anonymousUser;
async authorize(credentials) {
// Verify CAPTCHA token if provided
if (credentials?.captchaToken) {
try {
const captchaResponse = await fetch('https://www.google.com/recaptcha/api/siteverify', {
method: 'POST',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
body: `secret=${process.env.RECAPTCHA_SECRET_KEY}&response=${credentials.captchaToken}`
});
const captchaResult = await captchaResponse.json();
if (!captchaResult.success) {
console.error('CAPTCHA verification failed');
return null;
}
} catch (error) {
console.error('CAPTCHA verification error:', error);
return null;
}
} else if (process.env.NODE_ENV === 'production') {
// In production, require CAPTCHA
console.error('Missing CAPTCHA token for guest authentication');
return null;
}
const [anonymousUser] = await createAnonymousUser();
return anonymousUser;
Standards
  • CWE-287
  • OWASP-A07
  • NIST-SSDF-PW.4

},
}),
],
Expand Down
4 changes: 4 additions & 0 deletions app/(auth)/login/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { AuthForm } from '@/components/auth-form';
import { SubmitButton } from '@/components/submit-button';

import { login, type LoginActionState } from '../actions';
import { useSession } from 'next-auth/react';

export default function Page() {
const router = useRouter();
Expand All @@ -23,6 +24,8 @@ export default function Page() {
},
);

const { update: updateSession } = useSession();

useEffect(() => {
if (state.status === 'failed') {
toast({
Expand All @@ -36,6 +39,7 @@ export default function Page() {
});
} else if (state.status === 'success') {
setIsSuccessful(true);
updateSession();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Handling

The session update call lacks error handling. If the session update fails, the application will continue as if successful, potentially causing state inconsistency between client and server.

try {
  await updateSession();
} catch (error) {
  toast({
    type: 'error',
    description: 'Failed to update session. Please try again.'
  });
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Risk

Session update operation lacks error handling. Failed session updates may leave users in inconsistent authentication states, causing unexpected behavior and potential reliability issues.

Suggested change
updateSession();
try {
await updateSession();
} catch (error) {
console.error('Failed to update session:', error);
toast({
type: 'warning',
description: 'Session updated but refresh needed. Please reload the page.'
});
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

router.refresh();
Comment on lines +42 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Race

Session update and router refresh occur in parallel without awaiting session update completion. This can cause race conditions where navigation happens before session state is properly updated, leading to inconsistent authentication state.

await updateSession();
router.refresh();
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +42 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Failure

The updateSession() call doesn't handle promise rejection. If session update fails, the router.refresh() will execute anyway, potentially causing inconsistent state. This can lead to authentication state mismatch and application errors.

try {
  await updateSession();
  router.refresh();
} catch (error) {
  toast({
    type: 'error',
    description: 'Failed to update session. Please try again.'
  });
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +42 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Await Pattern

The updateSession() call is not awaited, which can lead to race conditions where navigation happens before session update completes. This may cause authentication state inconsistencies and unexpected behavior.

42:        setIsSuccessful(true);
43:        await updateSession();
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +42 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Race Condition

Session update occurs after UI state change without awaiting completion. If session update fails, UI may show success while authentication state remains inconsistent, causing navigation/permission errors.

await updateSession();
setIsSuccessful(true);
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +42 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Inefficiency

Calling updateSession() followed immediately by router.refresh() creates redundant rendering cycles. The updateSession() already triggers a revalidation of session data, making the manual router.refresh() unnecessary and potentially causing double-rendering. This can impact client-side performance, especially on lower-end devices.

Suggested change
updateSession();
router.refresh();
await updateSession();
Standards
  • ISO-IEC-25010-Performance-Time-Behaviour
  • Google-Core-Web-Vitals-INP

Comment on lines +42 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Failure

The updateSession function is called without awaiting its completion. This can lead to race conditions where the router refreshes before the session is fully updated, potentially causing authentication state inconsistencies.

      setIsSuccessful(true);
      await updateSession();
      router.refresh();
Commitable Suggestion
Suggested change
updateSession();
router.refresh();
setIsSuccessful(true);
await updateSession();
router.refresh();
Standards
  • ISO-IEC-25010-Reliability-Maturity
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • SRE-Error-Handling

}
}, [state.status]);
Expand Down
4 changes: 4 additions & 0 deletions app/(auth)/register/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { SubmitButton } from '@/components/submit-button';

import { register, type RegisterActionState } from '../actions';
import { toast } from '@/components/toast';
import { useSession } from 'next-auth/react';

export default function Page() {
const router = useRouter();
Expand All @@ -23,6 +24,8 @@ export default function Page() {
},
);

const { update: updateSession } = useSession();

useEffect(() => {
if (state.status === 'user_exists') {
toast({ type: 'error', description: 'Account already exists!' });
Expand All @@ -37,6 +40,7 @@ export default function Page() {
toast({ type: 'success', description: 'Account created successfully!' });

setIsSuccessful(true);
updateSession();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Await Pattern

The updateSession() function is likely asynchronous but called without await. This can lead to race conditions where the router.refresh() executes before session update completes, causing inconsistent state.

try {
  await updateSession();
} catch (error) {
  toast({
    type: 'error',
    description: 'Failed to update session. Please try again.'
  });
}
Standards
  • ISO-IEC-25010-Reliability-Maturity
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines 42 to +43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Update Failure

The updateSession function is called without awaiting its completion. This can lead to race conditions where the router refreshes before the session is fully updated, potentially causing authentication state inconsistencies.

        setIsSuccessful(true);
        await updateSession();
        router.refresh();
Commitable Suggestion
Suggested change
setIsSuccessful(true);
updateSession();
setIsSuccessful(true);
await updateSession();
router.refresh();
Standards
  • ISO-IEC-25010-Reliability-Maturity
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • SRE-Error-Handling

router.refresh();
Comment on lines 42 to 44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Session Update

Session update call is not awaited before router refresh. This can lead to navigation with stale session data, causing authentication state inconsistencies and potential user experience issues.

setIsSuccessful(true);
await updateSession();
router.refresh();
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines 42 to 44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Await Pattern

The updateSession() call is not awaited, which can lead to race conditions where navigation happens before session update completes. This may cause authentication state inconsistencies and unexpected behavior.

42:        setIsSuccessful(true);
43:        await updateSession();
44:        router.refresh();
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines 42 to 44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race Condition Risk

The updateSession() function is called without awaiting completion, causing a race condition. Router refresh may execute before session update completes, resulting in stale session data being used for rendering.

setIsSuccessful(true);
await updateSession();
router.refresh();
Standards
  • Logic-Verification-Async-Operations
  • Algorithm-Correctness-Execution-Order

}
}, [state]);
Expand Down
34 changes: 31 additions & 3 deletions app/(chat)/api/chat/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
UIMessage,
type UIMessage,
appendResponseMessages,
createDataStreamResponse,
smoothStream,
Expand All @@ -10,6 +10,7 @@ import { systemPrompt } from '@/lib/ai/prompts';
import {
deleteChatById,
getChatById,
getMessageCountByUserId,
saveChat,
saveMessages,
} from '@/lib/db/queries';
Expand All @@ -23,8 +24,12 @@ import { createDocument } from '@/lib/ai/tools/create-document';
import { updateDocument } from '@/lib/ai/tools/update-document';
import { requestSuggestions } from '@/lib/ai/tools/request-suggestions';
import { getWeather } from '@/lib/ai/tools/get-weather';
import { isProductionEnvironment } from '@/lib/constants';
import { anonymousRegex, isProductionEnvironment } from '@/lib/constants';
import { myProvider } from '@/lib/ai/providers';
import {
entitlementsByMembershipTier,
type MembershipTier,
} from '@/lib/ai/capabilities';

export const maxDuration = 60;

Expand All @@ -42,10 +47,33 @@ export async function POST(request: Request) {

const session = await auth();

if (!session || !session.user || !session.user.id) {
if (!session?.user?.id) {
return new Response('Unauthorized', { status: 401 });
}

const membershipTier: MembershipTier = anonymousRegex.test(
session.user.email ?? '',
)
? 'guest'
: 'free';
Comment on lines +54 to +58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Membership Tier Duplication

Duplicated user type detection logic appears in multiple components. This creates maintenance burden when user classification rules change and violates DRY principle.

// Import from a shared utility function
import { getUserMembershipTier } from '@/lib/auth/membership';

// Later in the code:
const membershipTier = getUserMembershipTier(session.user);
Standards
  • Clean-Code-DRY
  • Design-Pattern-Abstraction

Comment on lines +54 to +58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Authentication Check

Guest tier determination relies solely on email pattern. Attacker could forge emails matching anonymous pattern. Enables privilege escalation from guest to free tier.

Suggested change
const membershipTier: MembershipTier = anonymousRegex.test(
session.user.email ?? '',
)
? 'guest'
: 'free';
// Determine membership tier based on both email pattern and creation method
const isAnonymousEmail = anonymousRegex.test(session.user.email ?? '');
const authMethod = session.user.authMethod || 'credentials';
// Only consider as guest if both email matches pattern AND auth method is 'guest'
const membershipTier: MembershipTier = (isAnonymousEmail && authMethod === 'guest')
? 'guest'
: 'free';
Standards
  • CWE-287
  • OWASP-A01


const messageCount = await getMessageCountByUserId({
id: session.user.id,
differenceInHours: 24,
});

if (
messageCount >
entitlementsByMembershipTier[membershipTier].maxMessagesPerDay
) {
Comment on lines +60 to +68
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate Limiting Bottleneck

Rate limiting check occurs on every request, requiring a database query that counts messages. This creates a performance bottleneck as the database is queried before processing each chat message, adding latency to every request regardless of whether the user is near their limit.

    // Cache message count in memory with a TTL
    const cacheKey = `message_count:${session.user.id}`;
    const cachedCount = messageCountCache.get(cacheKey);
    
    let messageCount;
    if (cachedCount !== undefined) {
      messageCount = cachedCount + 1; // Increment for current message
    } else {
      messageCount = await getMessageCountByUserId({
        id: session.user.id,
        differenceInHours: 24,
      });
      // Cache with 5-minute TTL
      messageCountCache.set(cacheKey, messageCount, 5 * 60 * 1000);
    }
    
    if (
      messageCount >
      entitlementsByMembershipTier[membershipTier].maxMessagesPerDay
    ) {
Commitable Suggestion
Suggested change
const messageCount = await getMessageCountByUserId({
id: session.user.id,
differenceInHours: 24,
});
if (
messageCount >
entitlementsByMembershipTier[membershipTier].maxMessagesPerDay
) {
// Cache message count in memory with a TTL
const cacheKey = `message_count:${session.user.id}`;
const cachedCount = messageCountCache.get(cacheKey);
let messageCount;
if (cachedCount !== undefined) {
messageCount = cachedCount + 1; // Increment for current message
} else {
messageCount = await getMessageCountByUserId({
id: session.user.id,
differenceInHours: 24,
});
// Cache with 5-minute TTL
messageCountCache.set(cacheKey, messageCount, 5 * 60 * 1000);
}
if (
messageCount >
entitlementsByMembershipTier[membershipTier].maxMessagesPerDay
) {
Standards
  • ISO-IEC-25010-Performance-Efficiency-Time-Behavior
  • Optimization-Pattern-Caching
  • Algorithmic-Complexity-Database-Access

return new Response(
'You have exceeded your maximum number of messages for the day',
{
status: 429,
},
Comment on lines 47 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insufficient Rate Limiting for Guest Users

The PR implements rate limiting based on message count per user ID, but lacks IP-based rate limiting for guest account creation. An attacker could create unlimited anonymous accounts (each with its own message quota) by repeatedly calling the guest authentication endpoint, effectively bypassing the per-user message limit and potentially causing a denial of service.

Suggested change
const session = await auth();
if (!session || !session.user || !session.user.id) {
if (!session?.user?.id) {
return new Response('Unauthorized', { status: 401 });
}
const membershipTier: MembershipTier = anonymousRegex.test(
session.user.email ?? '',
)
? 'guest'
: 'free';
const messageCount = await getMessageCountByUserId({
id: session.user.id,
differenceInHours: 24,
});
if (
messageCount >
entitlementsByMembershipTier[membershipTier].maxMessagesPerDay
) {
return new Response(
'You have exceeded your maximum number of messages for the day',
{
status: 429,
},
// Add this to app/(auth)/api/auth/guest/route.ts after the imports
import { rateLimit } from '@/lib/rate-limit';
// Add this before the GET function
const limiter = rateLimit({
interval: 60 * 60 * 1000, // 1 hour
uniqueTokenPerInterval: 500, // Max 500 users per hour
});

Standard: CWE-770
Standard: OWASP Top 10 2021: A04 - Insecure Design

);
Comment on lines +60 to +74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Error Handling

The code doesn't handle potential errors from getMessageCountByUserId() which could throw exceptions as shown in its implementation. This can lead to unhandled promise rejections and server errors when database queries fail.

Standards
  • Logic-Verification-Error-Handling
  • Business-Rule-Robustness

}
Comment on lines +60 to +75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate Limiting Check

The rate limiting check occurs after database query execution, causing unnecessary database load when users have already exceeded their limits. This creates performance overhead as the system performs expensive message counting operations for every request, even when users are already over quota. Under high traffic, this can lead to database contention and increased latency for all users.

// Check cached rate limit status first before database query
const cacheKey = `rate_limit:${session.user.id}`;
const cachedCount = await cache.get(cacheKey);

if (cachedCount && parseInt(cachedCount) >= entitlementsByMembershipTier[membershipTier].maxMessagesPerDay) {
  return new Response(
    'You have exceeded your maximum number of messages for the day',
    { status: 429 }
  );
}

const messageCount = await getMessageCountByUserId({
  id: session.user.id,
  differenceInHours: 24,
});

// Update cache with latest count
await cache.set(cacheKey, messageCount.toString(), { ttl: 60 * 5 }); // 5 minute TTL

if (messageCount >= entitlementsByMembershipTier[membershipTier].maxMessagesPerDay) {
  return new Response(
    'You have exceeded your maximum number of messages for the day',
    { status: 429 }
  );
}
Standards
  • ISO-IEC-25010-Performance-Time-Behaviour
  • ISO-IEC-25010-Performance-Resource-Utilization
  • Netflix-Multi-Layer-Caching
  • Algorithm-Opt-Cache-First-Check

Comment on lines +54 to +75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insufficient Rate Limiting

Rate limiting uses only user ID without IP-based restrictions. Attackers could create multiple guest accounts to bypass per-user limits, enabling denial of service attacks.

const membershipTier: MembershipTier = anonymousRegex.test(
  session.user.email ?? '',
)
  ? 'guest'
  : 'free';

// Get client IP for rate limiting
const clientIp = request.headers.get('x-forwarded-for') || 'unknown';

// Check user-based rate limit
const messageCount = await getMessageCountByUserId({
  id: session.user.id,
  differenceInHours: 24,
});

// Check IP-based rate limit for anonymous users
let ipLimitExceeded = false;
if (membershipTier === 'guest') {
  ipLimitExceeded = await checkIpRateLimit(clientIp);
}

if (
  messageCount > entitlementsByMembershipTier[membershipTier].maxMessagesPerDay ||
  ipLimitExceeded
) {
  return new Response(
    'You have exceeded your maximum number of messages for the day',
    {
      status: 429,
    },
  );
}
Standards
  • CWE-770
  • OWASP-A04

Comment on lines +54 to +75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insufficient Rate Limiting

Rate limiting only checks message count without IP-based restrictions. Attackers can create unlimited guest accounts to bypass per-account message limits. Implement IP-based rate limiting for guest account creation.

Standards
  • CWE-770
  • OWASP-A04


const userMessage = getMostRecentUserMessage(messages);

if (!userMessage) {
Expand Down
3 changes: 2 additions & 1 deletion app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Geist, Geist_Mono } from 'next/font/google';
import { ThemeProvider } from '@/components/theme-provider';

import './globals.css';
import { SessionProvider } from 'next-auth/react';

export const metadata: Metadata = {
metadataBase: new URL('https://chat.vercel.ai'),
Expand Down Expand Up @@ -77,7 +78,7 @@ export default async function RootLayout({
disableTransitionOnChange
>
<Toaster position="top-center" />
{children}
<SessionProvider>{children}</SessionProvider>
</ThemeProvider>
</body>
</html>
Expand Down
71 changes: 55 additions & 16 deletions components/sidebar-user-nav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use client';

import { ChevronUp } from 'lucide-react';
import Image from 'next/image';
import type { User } from 'next-auth';
import { signOut } from 'next-auth/react';
import { signOut, useSession } from 'next-auth/react';
import { useTheme } from 'next-themes';

import {
Expand All @@ -17,26 +18,50 @@ import {
SidebarMenuButton,
SidebarMenuItem,
} from '@/components/ui/sidebar';
import { anonymousRegex } from '@/lib/constants';
import { useRouter } from 'next/navigation';
import { toast } from './toast';
import { LoaderIcon } from './icons';

export function SidebarUserNav({ user }: { user: User }) {
const router = useRouter();
const { data, status } = useSession();
const { setTheme, theme } = useTheme();

const isGuest = anonymousRegex.test(data?.user?.email ?? '');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anonymous User Detection

Direct regex testing in component creates tight coupling to user identification logic. This pattern makes changing the anonymous user detection mechanism difficult and violates separation of concerns.

// Extract anonymous user detection to a utility function
import { isAnonymousUser } from '@/lib/auth/utils';

// Later in component:
const isGuest = isAnonymousUser(data?.user?.email);
Standards
  • SOLID-SRP
  • Clean-Code-Abstraction


return (
<SidebarMenu>
<SidebarMenuItem>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<SidebarMenuButton className="data-[state=open]:bg-sidebar-accent bg-background data-[state=open]:text-sidebar-accent-foreground h-10">
<Image
src={`https://avatar.vercel.sh/${user.email}`}
alt={user.email ?? 'User Avatar'}
width={24}
height={24}
className="rounded-full"
/>
<span className="truncate">{user?.email}</span>
<ChevronUp className="ml-auto" />
</SidebarMenuButton>
{status === 'loading' ? (
<SidebarMenuButton className="data-[state=open]:bg-sidebar-accent bg-background data-[state=open]:text-sidebar-accent-foreground h-10 justify-between">
<div className="flex flex-row gap-2">
<div className="size-6 bg-zinc-500/30 rounded-full animate-pulse" />
<span className="bg-zinc-500/30 text-transparent rounded-md animate-pulse">
Loading auth status
</span>
</div>
<div className="animate-spin text-zinc-500">
<LoaderIcon />
</div>
</SidebarMenuButton>
) : (
<SidebarMenuButton className="data-[state=open]:bg-sidebar-accent bg-background data-[state=open]:text-sidebar-accent-foreground h-10">
<Image
src={`https://avatar.vercel.sh/${user.email}`}
Comment on lines +38 to +53
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Loading Race

User object accessed during loading state. When status is not 'loading', code assumes user object is available but doesn't verify, risking runtime errors if authentication is incomplete.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

alt={user.email ?? 'User Avatar'}
width={24}
height={24}
className="rounded-full"
/>
<span className="truncate">
{isGuest ? 'Guest' : user?.email}
</span>
<ChevronUp className="ml-auto" />
</SidebarMenuButton>
)}
Comment on lines +51 to +64
Copy link

@coderabbitai coderabbitai bot Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential null reference on user prop.

The code accesses user.email at lines 53-54 and 60 when status !== 'loading', but the user prop may be undefined or null if the session is unauthenticated. This could cause a runtime error.

Apply this diff to safely access user:

               <SidebarMenuButton className="data-[state=open]:bg-sidebar-accent bg-background data-[state=open]:text-sidebar-accent-foreground h-10">
                 <Image
-                  src={`https://avatar.vercel.sh/${user.email}`}
-                  alt={user.email ?? 'User Avatar'}
+                  src={`https://avatar.vercel.sh/${data?.user?.email ?? 'guest'}`}
+                  alt={data?.user?.email ?? 'User Avatar'}
                   width={24}
                   height={24}
                   className="rounded-full"
                 />
                 <span className="truncate">
-                  {isGuest ? 'Guest' : user?.email}
+                  {isGuest ? 'Guest' : data?.user?.email}
                 </span>
                 <ChevronUp className="ml-auto" />
               </SidebarMenuButton>
🤖 Prompt for AI Agents 
In components/sidebar-user-nav.tsx around lines 51 to 64, the code accesses
user.email directly which can be null/undefined for unauthenticated sessions;
change references to use safe checks and fallbacks (e.g., user?.email or a
placeholder string) and guard rendering where necessary: use optional chaining
for src and alt (or a default avatar URL/empty string) and keep the displayed
name as isGuest ? 'Guest' : user?.email ?? 'Unknown User' (or similar) so no
property access on null occurs and rendering remains safe.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have implemented checks in the backend and db schema holds contraint for user.user not null
So there will be no case for this one

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crimefighter18, thank you for the clarification! If the backend checks and database constraints guarantee that user.email is never null when this component renders, then the null safety concern is indeed not applicable here. I appreciate you explaining the architectural safeguards in place.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai We have implemented checks in the backend and db schema holds contraint for user.user not null
So there will be no case for this one

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crimefighter18, I've noted your clarification. Since the backend validation and database schema constraints ensure user.email is always non-null when this component renders, the null safety concern doesn't apply here. Thank you for explaining the architectural safeguards!

✏️ Learnings added 
Learnt from: crimefighter18
PR: coderabbit-test/ai-chatbot#8
File: components/sidebar-user-nav.tsx:51-64
Timestamp: 2025-10-06T09:20:43.245Z
Learning: In the ai-chatbot codebase (components/sidebar-user-nav.tsx), user.email is guaranteed to be non-null due to backend checks and database schema constraints (user.user not null constraint). Null safety checks for user.email are not needed in frontend components.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

</DropdownMenuTrigger>
<DropdownMenuContent
side="top"
Expand All @@ -54,12 +79,26 @@ export function SidebarUserNav({ user }: { user: User }) {
type="button"
className="w-full cursor-pointer"
onClick={() => {
signOut({
redirectTo: '/',
});
if (status === 'loading') {
toast({
type: 'error',
description:
'Checking authentication status, please try again!',
});

return;
}
Comment on lines +82 to +90
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session Status Race

Sign out button is clickable during 'loading' state but shows error toast instead of disabling the button. This creates a poor user experience where users can trigger actions that immediately fail with an error message.

Standards
  • Logic-Verification-UI-State-Consistency
  • Business-Rule-User-Feedback

Comment on lines +82 to +90
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading State Handling

The sign-out functionality shows an error toast when authentication is loading, but doesn't disable the button. Users can repeatedly click the button during loading, causing multiple error toasts and a confusing user experience.

Standards
  • ISO-IEC-25010-Reliability-Maturity
  • ISO-IEC-25010-Functional-Correctness-Appropriateness


if (isGuest) {
router.push('/login');
} else {
signOut({
redirectTo: '/',
});
}
Comment on lines +82 to +98
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent Session Handling

The code uses both router.push() for guests and signOut() with redirectTo for regular users, creating inconsistent session termination logic. This can lead to state inconsistencies where guest sessions aren't properly terminated before login.

Standards
  • Business-Rule-State-Consistency
  • Logic-Verification-Control-Flow

}}
>
Sign out
{isGuest ? 'Login to your account' : 'Sign out'}
</button>
</DropdownMenuItem>
</DropdownMenuContent>
Expand Down
33 changes: 33 additions & 0 deletions lib/ai/capabilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import type { ChatModel } from './models';

export type MembershipTier = 'guest' | 'free';

interface Entitlements {
maxMessagesPerDay: number;
chatModelsAvailable: Array<ChatModel['id']>;
}

export const entitlementsByMembershipTier: Record<
MembershipTier,
Entitlements
> = {
/*
* For users without an account
*/
guest: {
maxMessagesPerDay: 20,
chatModelsAvailable: ['chat-model', 'chat-model-reasoning'],
},

/*
* For user with an account
*/
free: {
maxMessagesPerDay: 100,
chatModelsAvailable: ['chat-model', 'chat-model-reasoning'],
},
Comment on lines +17 to +28
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded Rate Limits

Rate limits are hardcoded in the capabilities file. This reduces configurability and requires code changes to adjust limits, making it difficult to tune system behavior without deployment.

// Import configuration from environment or config file
import { config } from '../config';

/*
 * For users without an account
 */
guest: {
  maxMessagesPerDay: config.limits.guest.messagesPerDay || 20,
  chatModelsAvailable: config.models.guest.available || ['chat-model', 'chat-model-reasoning'],
},

/*
 * For user with an account
 */
Standards
  • Clean-Code-Configuration
  • Design-Pattern-Configuration

Comment on lines +10 to +28
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded Rate Limits

Rate limits are hardcoded in the application code rather than being configurable via environment variables. This reduces maintainability as changing limits requires code changes and redeployment instead of configuration updates.

Standards
  • Clean-Code-Configuration
  • Maintainability-Quality-Flexibility
  • Design-Pattern-Configuration


/*
* TODO: For users with an account and a paid membership
*/
};
2 changes: 1 addition & 1 deletion lib/ai/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const DEFAULT_CHAT_MODEL: string = 'chat-model';

interface ChatModel {
export interface ChatModel {
id: string;
name: string;
description: string;
Expand Down
2 changes: 2 additions & 0 deletions lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ export const isTestEnvironment = Boolean(
process.env.PLAYWRIGHT ||
process.env.CI_PLAYWRIGHT,
);

export const anonymousRegex = /^anonymous-\d+$/;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This regular expression is designed to match anonymous user emails created with Date.now(). This is coupled with the user creation logic in lib/db/queries.ts. If you update the email generation to use UUIDs for better uniqueness (as suggested in a separate comment), this regex will no longer match and will need to be updated.

A simple update could be to match any characters after anonymous-, or you could use a more specific regex for UUIDs.

Suggested change
export const anonymousRegex = /^anonymous-\d+$/;
export const anonymousRegex = /^anonymous-.+/;

Loading