Added check for authenticated users access to public pages#185
Added check for authenticated users access to public pages#185sumagnadas merged 5 commits intoOpenLake:mainfrom
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughThe changes introduce a new Changes
Sequence DiagramsequenceDiagram
actor User
participant App as App Router
participant PublicRoute as PublicRoute Guard
participant AuthContext as Auth Context
participant Login as Login Component
User->>App: Navigate to /login
App->>PublicRoute: Render PublicRoute wrapper
PublicRoute->>AuthContext: useAuth() for isAuthenticated & loading
alt User is loading
PublicRoute->>User: Show "Loading..." indicator
else User is authenticated
PublicRoute->>App: Navigate to "/"
App->>User: Redirect to home
else User is not authenticated
PublicRoute->>Login: Render children (Login component)
Login->>AuthContext: useAuth() for auth state
User->>Login: Submit login/Google auth
Login->>AuthContext: Update authentication tokens
Login->>User: Redirect to home after successful auth
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app/src/components/Heatmap.jsx`:
- Around line 11-21: The useEffect that calls fetchGitHubContributions() or
generateMockData() is missing the contributions prop in its dependency array,
causing stale mock heatmap data when contributions changes; update the effect
dependencies from [platform, username] to include contributions (i.e.,
[platform, username, contributions]) so the effect reruns when contributions
updates, ensuring generateMockData() and fetchGitHubContributions() produce
fresh data (refer to the useEffect block and the generateMockData and
fetchGitHubContributions functions).
In `@app/src/components/HomePage.jsx`:
- Around line 27-104: The code dereferences potentially null objects (userNames
and the prop passed into Cards), causing runtime errors; fix by guarding these
accesses: update the Cards signature to accept a safe default (e.g., function
Cards({ usernames } = { usernames: {} }) or destructure with usernames = {}),
and replace direct accesses like usernames.github?.contributions and
usernames.codeforces?.username with optional chaining and sensible defaults;
likewise, in HomePage replace direct userNames.username uses with
userNames?.username (or conditionally render the UI only when userNames is
non-null) so every dereference is null-safe and Heatmap/contribution props fall
back to 0 or empty string when absent.
In `@app/src/components/Login.jsx`:
- Around line 48-52: handleGoogleAuth currently calls SignInWithGoogle() without
inspecting its result and always calls navigate("/"); update handleGoogleAuth to
await and capture the return value from SignInWithGoogle(), verify the response
indicates a successful auth (e.g., truthy user object or success flag returned
by SignInWithGoogle), and only call navigate("/") when that check passes; keep
proper try/catch around the call to SignInWithGoogle() and in the error/failure
branch avoid navigation and surface an error state or message to the user.
In `@app/src/Context/AuthContext.jsx`:
- Around line 33-48: The effect that sets user and isAuthenticated runs only on
mount, so isAuthenticated gets stale; update the useEffect in AuthContext (the
useEffect that calls setUser, setIsAuthenticated, setLoading) to depend on
authTokens (add authTokens to the dependency array) or alternatively remove the
separate isAuthenticated state and derive it from authTokens (e.g., compute
isAuthenticated = Boolean(authTokens) or from authTokens.access) and remove
setIsAuthenticated usage; ensure the effect still calls
setUser(jwtDecode(authTokens.access)) when authTokens is present and clears
user/setIsAuthenticated when null.
🧹 Nitpick comments (1)
app/src/components/Heatmap.jsx (1)
23-46: Consider checkingresponse.okbefore parsing.This makes failure modes clearer and avoids treating non‑200 responses as valid data.
🔧 Suggested tweak
- const response = await fetch(`https://github-contributions-api.deno.dev/${username}.json`); - const data = await response.json(); + const response = await fetch(`https://github-contributions-api.deno.dev/${username}.json`); + if (!response.ok) { + throw new Error(`GitHub contributions fetch failed: ${response.status}`); + } + const data = await response.json();
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| if (platform === 'github' && username) { | ||
| await fetchGitHubContributions(); | ||
| } else { | ||
| generateMockData(); | ||
| } | ||
| }; | ||
|
|
||
| fetchData(); | ||
| }, [platform, username]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i heatmapRepository: OpenLake/Leaderboard-Pro
Length of output: 98
🏁 Script executed:
find . -name "Heatmap.jsx" -o -name "Heatmap.js"Repository: OpenLake/Leaderboard-Pro
Length of output: 100
🏁 Script executed:
wc -l app/src/components/Heatmap.jsxRepository: OpenLake/Leaderboard-Pro
Length of output: 102
🏁 Script executed:
cat -n app/src/components/Heatmap.jsxRepository: OpenLake/Leaderboard-Pro
Length of output: 10444
Include contributions in the effect dependency array.
The generateMockData function (lines 97-131) reads the contributions prop to determine mock data intensity levels. When the effect calls generateMockData() and contributions changes, the effect won't re-run without it in the dependency array, leaving the heatmap with stale data.
🔧 Suggested fix
- }, [platform, username]);
+ }, [platform, username, contributions]);🤖 Prompt for AI Agents
In `@app/src/components/Heatmap.jsx` around lines 11 - 21, The useEffect that
calls fetchGitHubContributions() or generateMockData() is missing the
contributions prop in its dependency array, causing stale mock heatmap data when
contributions changes; update the effect dependencies from [platform, username]
to include contributions (i.e., [platform, username, contributions]) so the
effect reruns when contributions updates, ensuring generateMockData() and
fetchGitHubContributions() produce fresh data (refer to the useEffect block and
the generateMockData and fetchGitHubContributions functions).
| async function handleGoogleAuth(e) { | ||
| e.preventDefault(); | ||
| await SignInWithGoogle(); | ||
| navigate("/"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -type f -name "Login.jsx" | head -20Repository: OpenLake/Leaderboard-Pro
Length of output: 123
🏁 Script executed:
cd /repo && find . -type f -name "*.jsx" -o -name "*.js" | grep -i login | head -20Repository: OpenLake/Leaderboard-Pro
Length of output: 123
🏁 Script executed:
cd /repo && rg "SignInWithGoogle" --type jsx --type js -A 3 -B 3 | head -100Repository: OpenLake/Leaderboard-Pro
Length of output: 123
🏁 Script executed:
find . -type f -name "Login.jsx"Repository: OpenLake/Leaderboard-Pro
Length of output: 98
🏁 Script executed:
find . -type f -name "*.jsx" -o -name "*.js" | grep -i loginRepository: OpenLake/Leaderboard-Pro
Length of output: 98
🏁 Script executed:
rg "SignInWithGoogle" -A 5 -B 2Repository: OpenLake/Leaderboard-Pro
Length of output: 2739
🏁 Script executed:
cat -n ./app/src/components/Login.jsx | head -60Repository: OpenLake/Leaderboard-Pro
Length of output: 2039
🏁 Script executed:
cat -n ./app/src/firebase/AuthContext.js | grep -A 10 "const SignInWithGoogle"Repository: OpenLake/Leaderboard-Pro
Length of output: 402
🏁 Script executed:
cat -n ./app/src/Context/AuthContext.jsx | grep -A 30 "const SignInWithGoogle"Repository: OpenLake/Leaderboard-Pro
Length of output: 1354
🏁 Script executed:
cat -n ./app/src/Context/AuthContext.jsx | sed -n '172,210p'Repository: OpenLake/Leaderboard-Pro
Length of output: 1628
Gate navigation on successful Google sign‑in response.
The SignInWithGoogle() function returns a response object, but the current handler doesn't capture or check it before navigating. If authentication fails in the try-catch block, the response may be undefined or invalid, causing users to navigate to protected routes and bounce back. Capture the return value and navigate only on success.
🔧 Suggested fix
async function handleGoogleAuth(e) {
e.preventDefault();
- await SignInWithGoogle();
- navigate("/");
+ const response = await SignInWithGoogle();
+ if (response) {
+ navigate("/");
+ }
}📝 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.
| async function handleGoogleAuth(e) { | |
| e.preventDefault(); | |
| await SignInWithGoogle(); | |
| navigate("/"); | |
| } | |
| async function handleGoogleAuth(e) { | |
| e.preventDefault(); | |
| const response = await SignInWithGoogle(); | |
| if (response) { | |
| navigate("/"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@app/src/components/Login.jsx` around lines 48 - 52, handleGoogleAuth
currently calls SignInWithGoogle() without inspecting its result and always
calls navigate("/"); update handleGoogleAuth to await and capture the return
value from SignInWithGoogle(), verify the response indicates a successful auth
(e.g., truthy user object or success flag returned by SignInWithGoogle), and
only call navigate("/") when that check passes; keep proper try/catch around the
call to SignInWithGoogle() and in the error/failure branch avoid navigation and
surface an error state or message to the user.
This comment has been minimized.
This comment has been minimized.
Refactor authentication state management by using a boolean derived from authTokens instead of a separate state variable.
This comment has been minimized.
This comment has been minimized.
|
Can you separate the commits whicrh are to be merged after pulling the current main branch. |
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.Unrecognized words (605)Some files were automatically ignored 🙈These sample patterns would exclude them: You should consider excluding directory paths (e.g. You should consider adding them to: File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept these unrecognized words as correct and update file exclusions, you could run the following commands... in a clone of the git@github.com:HarK-github/Leaderboard-Pro.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/OpenLake/Leaderboard-Pro/actions/runs/22309848367/attempts/1' &&
git commit -m 'Update check-spelling metadata'Available 📚 dictionaries could cover words not in the 📘 dictionary
Consider adding them (in with:
extra_dictionaries: |
cspell:django/dict/django.txt
cspell:software-terms/dict/softwareTerms.txt
cspell:python/src/common/extra.txt
cspell:npm/dict/npm.txt
cspell:html/dict/html.txtTo stop checking additional dictionaries, add (in check_extra_dictionaries: ""Warnings
|
| Count | |
|---|---|
| 15 | |
| 1 | |
| 5 | |
| 10 |
See
If you see a bunch of garbage
If it relates to a ...
well-formed pattern
See if there's a pattern that would match it.
If not, try writing one and adding it to the patterns.txt file.
Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
binary-ish string
Please add a file path to the excludes.txt file instead of just accepting the garbage.
File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).
2 participants
Description
Prevents authenticated users from accessing public routes like
/login and /register by introducing a PublicRoute wrapper.
Changes
PublicRoutecomponentAuthContextType of Change
Checklist
Summary by CodeRabbit