Skip to content

refactor: simplify session casting and improve type safety in MyMento… #3505

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

Merged
arkid15r merged 4 commits into from
Jan 24, 2026
Merged

refactor: simplify session casting and improve type safety in MyMento… #3505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
352282b
refactor: simplify session casting and improve type safety in MyMento…
ThetaLogN Jan 23, 2026
3482c4f
Update code
arkid15r Jan 24, 2026
2d23a67
Merge branch 'main' into refactor/mentorship-session-fix
arkid15r Jan 24, 2026
5777a8b
Update code
arkid15r Jan 24, 2026
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
8 changes: 5 additions & 3 deletions frontend/src/app/my/mentorship/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import SearchPageLayout from 'components/SearchPageLayout'
const MyMentorshipPage: React.FC = () => {
const router = useRouter()
const searchParams = useSearchParams()

const { data: session } = useSession()
const username = (session as ExtendedSession)?.user?.login
const extendedSession = session as ExtendedSession | null
const userName = extendedSession?.user?.login

const initialQuery = searchParams.get('q') || ''
const initialPage = Number.parseInt(searchParams.get('page') || '1', 10)
Expand Down Expand Up @@ -58,7 +60,7 @@ const MyMentorshipPage: React.FC = () => {
fetchPolicy: 'cache-and-network',
errorPolicy: 'all',
})
const isProjectLeader = (session as ExtendedSession)?.user.isLeader
const isProjectLeader = extendedSession?.user?.isLeader

useEffect(() => {
if (programData?.myPrograms) {
Expand All @@ -81,7 +83,7 @@ const MyMentorshipPage: React.FC = () => {

const handleCreate = () => router.push('/my/mentorship/programs/create')

if (!username) {
if (!userName) {
return <LoadingSpinner />
}
Comment on lines +86 to 88
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for middleware files in the frontend app
echo "=== Looking for middleware files ==="
find frontend -name 'middleware.ts' -o -name 'middleware.js' 2>/dev/null

echo ""
echo "=== Checking root-level middleware ==="
find . -maxdepth 2 -name 'middleware.ts' -o -name 'middleware.js' 2>/dev/null | head -20

echo ""
echo "=== Directory structure of /my/ routes ==="
fd -t d 'my' frontend/src/app/ 2>/dev/null

echo ""
echo "=== Files in /my/ directory ==="
fd -t f frontend/src/app/my/ 2>/dev/null | head -20

Repository: OWASP/Nest

Length of output: 228

🏁 Script executed:

#!/bin/bash
# Check the actual page file to see the full implementation
echo "=== Content of frontend/src/app/my/mentorship/page.tsx ==="
cat -n frontend/src/app/my/mentorship/page.tsx | head -100

echo ""
echo "=== Checking for useSession and status usage in the file ==="
rg -n "useSession|status|redirect|authenticated" frontend/src/app/my/mentorship/page.tsx

Repository: OWASP/Nest

Length of output: 4325

🏁 Script executed:

#!/bin/bash
# Check how other /my/ pages handle authentication
echo "=== Looking for other pages in /my/ directory ==="
fd -t f '.tsx$' frontend/src/app/my/ --exec basename {} \; | sort -u

echo ""
echo "=== Checking auth patterns in other /my/ pages ==="
fd -t f '.tsx$' frontend/src/app/my/ -exec sh -c 'echo "=== {} ===" && grep -n "useSession\|status\|redirect\|LoadingSpinner" {} | head -15' \;

Repository: OWASP/Nest

Length of output: 397

🏁 Script executed:

#!/bin/bash
# Check if there's a layout file that might handle auth at the route level
echo "=== Checking for layout files in /my/ routes ==="
fd -t f 'layout.tsx' frontend/src/app/my/

echo ""
echo "=== Examining /my/ layout if it exists ==="
if [ -f "frontend/src/app/my/layout.tsx" ]; then
  cat -n frontend/src/app/my/layout.tsx
fi

if [ -f "frontend/src/app/my/mentorship/layout.tsx" ]; then
  echo "=== Examining /my/mentorship/ layout if it exists ==="
  cat -n frontend/src/app/my/mentorship/layout.tsx
fi

Repository: OWASP/Nest

Length of output: 146

Fix unauthenticated user handling with proper session status check.

Currently, the loading gate !userName cannot distinguish between the loading state (session data still loading) and the unauthenticated state (user not logged in). Since there's no middleware or layout-level route protection for /my/* routes, an unauthenticated user will see the spinner indefinitely.

Verify the session loading state by including status from useSession():

const { data: session, status } = useSession()

Then handle all three states explicitly: loading, unauthenticated, and authenticated. For the unauthenticated case, redirect to login or show an appropriate message instead of displaying the spinner.

🤖 Prompt for AI Agents 
In `@frontend/src/app/my/mentorship/page.tsx` around lines 86 - 88, The current
loading gate uses only userName which conflates a true loading state with
unauthenticated users; update the component to destructure status from
useSession() (const { data: session, status } = useSession()), stop using the
single check if (!userName) to show <LoadingSpinner />, and instead explicitly
handle status === 'loading' (show LoadingSpinner), status === 'unauthenticated'
(perform a redirect to login or render a clear unauthenticated message), and
status === 'authenticated' (render the normal UI using session/userName); adjust
references to userName and LoadingSpinner accordingly so unauthenticated users
don’t see an indefinite spinner.


Expand Down
4 changes: 2 additions & 2 deletions frontend/src/types/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ export type ExtendedProfile = {
export type ExtendedSession = Session & {
accessToken?: string
user?: Session['user'] & {
email?: string
expires?: string
image?: string
isLeader?: boolean
Comment on lines 10 to 14
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n frontend/src/types/auth.ts

Repository: OWASP/Nest

Length of output: 576

🌐 Web query:

next-auth v4.24.13 Session user email image type definitions TypeScript

💡 Result:

In next-auth v4.24.x (including v4.24.13), the default TypeScript types already define the session’s user as having (optional) name, email, and image:

  • User includes: name?: string | null, email?: string | null, image?: string | null (and id: string). [1]
  • To keep those defaults when you extend Session.user, intersect with DefaultSession["user"]. [2]

Create a declaration file (e.g. types/next-auth.d.ts) and add:

import NextAuth, { DefaultSession } from "next-auth"

declare module "next-auth" {
  interface Session {
    user: {
      // your custom fields here, e.g. id: string
    } & DefaultSession["user"] // keeps name/email/image types
  }
}

This ensures session.user.email and session.user.image are typed as string | null | undefined (unless you further narrow them in your own app). [2]

Sources:
[1] next-auth v4.24.x docs “Types” (User/email/image)
[2] NextAuth.js TypeScript docs (module augmentation + DefaultSession["user"])

🏁 Script executed:

rg "session\.user\.email|session\.user\.image" --type ts --type tsx -A 2 -B 2

Repository: OWASP/Nest

Length of output: 81

🏁 Script executed:

rg "ExtendedSession" --type ts --type tsx -A 3 | head -100

Repository: OWASP/Nest

Length of output: 81

🏁 Script executed:

rg "session\.user\.email|session\.user\.image" -A 2 -B 2

Repository: OWASP/Nest

Length of output: 36

🏁 Script executed:

rg "ExtendedSession" -A 3 | head -150

Repository: OWASP/Nest

Length of output: 10757

🏁 Script executed:

fd "\.(ts|tsx)$" frontend/src --type f | head -20

Repository: OWASP/Nest

Length of output: 933

🏁 Script executed:

rg "email" frontend/src --type-add 'ts:*.{ts,tsx}' -t ts | grep -i "session\|user\|email" | head -40

Repository: OWASP/Nest

Length of output: 20371

🏁 Script executed:

rg "\.image" frontend/src --type-add 'ts:*.{ts,tsx}' -t ts | head -40

Repository: OWASP/Nest

Length of output: 320

🏁 Script executed:

cat frontend/src/components/UserMenu.tsx | head -80

Repository: OWASP/Nest

Length of output: 2804

🏁 Script executed:

rg "session\.user\." frontend/src/components/UserMenu.tsx -B 2 -A 2

Repository: OWASP/Nest

Length of output: 36

Avoid narrowing email/image nullability from next-auth.

In next-auth v4.24.13, Session['user'] defines email and image as string | null. Redeclaring them as optional strings in the intersection intersects and narrows the type, removing null from the union. This masks the actual nullability from next-auth and can lead to unsafe assumptions about these fields.

🛠️ Proposed fix 
   user?: Session['user'] & {
-    email?: string
+    email?: Session['user']['email']
     expires?: string
-    image?: string
+    image?: Session['user']['image']
     isLeader?: boolean
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
user?: Session['user'] & {
email?: string
expires?: string
image?: string
isLeader?: boolean
user?: Session['user'] & {
email?: Session['user']['email']
expires?: string
image?: Session['user']['image']
isLeader?: boolean
🤖 Prompt for AI Agents 
In `@frontend/src/types/auth.ts` around lines 10 - 14, The intersection on the
user type is narrowing next-auth's nullability for email and image; update the
inline overrides in the user?: Session['user'] & { ... } declaration so that
email and image preserve next-auth's possible null (e.g., use email?: string |
null and image?: string | null) or remove those re-declarations entirely and
only add non-conflicting fields like isLeader?: boolean, ensuring you keep the
type name Session['user'] to locate the declaration.

isMentor?: boolean
isOwaspStaff?: boolean
login?: string
name?: string
email?: string
image?: string
}
}
Loading