fix: generate random OAuth state each login #256
Conversation
Fixes kossiitkgp#216 ## Changes - Added `generateRandomState()` using `crypto.getRandomValues()` for cryptographically secure random OAuth state - Modified `handleLogin()` to append unique `&state=${randomState}` to `GITHUB_OAUTH_URL` each login - Each "Student/Mentor Sign-up" click now generates **different 43-char state parameter** ## Why this matters Prevents OAuth CSRF attacks by ensuring `state` parameter is unique per login attempt [web:162] ## Testing - Click "Student Sign-up" → URL gets `&state=abcXYZ123random...` - Refresh → Click again → **NEW** `&state=def456different...`
|
@ProgrammerSONGBIT is attempting to deploy a commit to the jai's projects Team on Vercel. A member of the Team first needs to authorize it. |
👷 Deploy request for kwoc-2025 pending review.Visit the deploys page to approve it
|
dipamsen
left a comment
dipamsen
left a comment
There was a problem hiding this comment.
This PR does not completely implement #246. It generates a random state for logging in, but does not verify whether the state is correct after the redirect. See the description in the issue for more details.
Solution:
- Generate a random string when clicking on the login button
- Store the string in
localstorageand redirect to the oauth URL with this state- When the oauth login is complete and the user is redirected to
/oauthon the frontend, match these strings and only proceed to log in if they match- It would also be good to make the state a limited-time string so that a string generated previously cannot be used
Added localStorage handling for OAuth state and timestamp.
Added OAuth state validation to prevent CSRF attacks and handle state expiration.
|
Changed the files =
Tested = Tested:
|
src/pages/OAuth.tsx
Outdated
| // Check 1: State received from URL | ||
| if (!receivedState) { | ||
| setErr("No state parameter in OAuth callback. Please try logging in again."); | ||
| return false; | ||
| } | ||
|
|
||
| // Check 2: State was stored in localStorage | ||
| if (!storedState) { | ||
| setErr("No stored OAuth state found. Your session may have expired."); | ||
| return false; | ||
| } | ||
|
|
||
| // Check 3: States match (prevents CSRF attacks) | ||
| if (receivedState !== storedState) { | ||
| console.error("CSRF ATTACK DETECTED: OAuth state mismatch!", { |
There was a problem hiding this comment.
No need to provide such descriptive error messages to the user (specifically in setErr) -- the error shown to the user can simply be 'Something went wrong. Please try logging in again.'
src/pages/OAuth.tsx
Outdated
| return; | ||
| } | ||
|
|
||
| // Validate OAuth state (NEW - prevents CSRF attacks) |
1. Added simple error message 2. Comments are removed
|
Removed comments as per your feedback and added simple error messages. |
src/components/Home/HeroSection.tsx
Outdated
| auth.setUserType(userType as UserType); | ||
| window.location.href = GITHUB_OAUTH_URL; | ||
| }; | ||
| const handleLogin = (userType: string) => { |
There was a problem hiding this comment.
Instead of a separate function, can this be part of the auth context?
There was a problem hiding this comment.
Instead of a separate function, can this be part of the auth context?
Changed the auth tsx file
Removed GitHub OAuth logic and replaced with auth context method.
Added handleOAuthLogin method to AuthContext.
Added OAuth login handler and random state generation for CSRF protection in the auth file .
3 participants
Changes
generateRandomState()usingcrypto.getRandomValues()for cryptographically secure random OAuth statehandleLogin()to append unique&state=${randomState}toGITHUB_OAUTH_URLeach loginWhy this matters
Prevents OAuth CSRF attacks by ensuring
stateparameter is unique per login attempt [web:162]Testing
&state=abcXYZ123random...&state=def456different...