feat: implement auth guards and conditional rendering entry button for virtual classroom#51
Merged
A1L13N merged 3 commits intoalphaonelabs:mainfrom Apr 23, 2026
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 7 seconds. ⌛ 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughClient-side auth was added to the classroom join flow and UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Classroom as classroom_poc.html
participant Storage as localStorage/sessionStorage
participant Login as login.html
User->>Classroom: Click "Join Classroom"
Classroom->>Storage: call isAuthenticated() -> read edu_token, edu_user
alt authenticated
Classroom->>Classroom: proceed with joinClassroom() flow (connect, seating, websocket)
else not authenticated
Classroom->>Classroom: show warning toast
Classroom->>Storage: set post_login_redirect = currentPath+query (sessionStorage)
Classroom->>User: delay 800ms then redirect to /login.html
User->>Login: visits login.html (with optional next=)
Login->>Storage: on successful login store edu_token, edu_user (localStorage)
Login->>Login: showToast("Redirecting...") and call handlePostLoginRedirect()
Login->>Storage: read and clear post_login_redirect (sessionStorage)
Login->>User: redirect to stored URL or /dashboard.html
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/classroom_poc.html`:
- Around line 238-249: The getAuth() and isAuthenticated() functions are
duplicated; extract them into a single shared module (e.g., create /js/auth.js)
that exports getAuth and isAuthenticated, update classroom_poc.html to load that
script instead of defining the functions, and remove the local definitions;
ensure the exported getAuth() returns the same token/user shape and the
consuming code still calls getAuth() and isAuthenticated() (function names:
getAuth, isAuthenticated) so both pages use the single source of truth.
- Around line 429-434: Replace the blocking alert in joinClassroom() with a
non-blocking UX: call the existing showToast() helper to display the "must be
signed in" message, store the intended classroom path (e.g.,
window.location.pathname + search or the roomId) into sessionStorage under a key
like "post_login_redirect", then set window.location.href = '/login.html';
update login.html's post-login flow to check
sessionStorage.getItem('post_login_redirect') and redirect there after
successful login (and clear the key). Ensure references: joinClassroom,
showToast, and the sessionStorage key "post_login_redirect".
In `@public/index.html`:
- Around line 156-163: The login flow and visibility suffer two issues: clicking
the "Login to Join Classroom" anchor (id="join-not-logged-in") sends users to
login.html which currently redirects to dashboard, losing the intended classroom
target, and the logged-in button (id="join-logged-in") can briefly flash for
logged-out users; fix by making the classroom link include a next query param
(e.g., ?next=<url-encoded-current-target>) or by storing the target in
sessionStorage before navigating so login.html can read and redirect to that
next value instead of dashboard, and change the initial markup/CSS to hide both
anchors (or the container) by default and let your existing DOMContentLoaded/js
visibility logic reveal only the correct element (referencing ids join-logged-in
and join-not-logged-in and the login redirect logic in login.html) so logged-out
users never see the wrong button briefly.
In `@public/js/layout.js`:
- Around line 183-195: The DOMContentLoaded handler that reads localStorage and
manipulates loggedInEl/notLoggedInEl can throw when those elements are absent;
instead, move this logic into the existing initializer (merge with the current
DOMContentLoaded handler) and use the isAuthenticated() helper rather than
re-reading localStorage, then guard element access by checking const loggedInEl
= document.getElementById('join-logged-in') and const notLoggedInEl =
document.getElementById('join-not-logged-in') for null before touching
.style.display so pages without the buttons won't error.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3b95952-6a1e-441f-aea2-e38921760136
📒 Files selected for processing (3)
public/classroom_poc.htmlpublic/index.htmlpublic/js/layout.js
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/login.html`:
- Around line 320-323: The code double-decodes the next parameter by calling
decodeURIComponent on params.get('next'); update the post-login redirect logic
to store the raw params.get('next') value (no decodeURIComponent) into
sessionStorage under 'post_login_redirect' (i.e., change the
sessionStorage.setItem call to use params.get('next') directly) to avoid
malformed URI errors and preserve encoded percent literals.
- Around line 273-285: The showToast function is vulnerable to HTML injection
because it assigns the msg via innerHTML; update showToast to construct DOM
nodes safely instead of using innerHTML: create the wrapper div (toast) as
before, create an <i> element and set its className to "fas fa-check-circle
mt-0.5 flex-shrink-0", create a <span> for the message and set its textContent
to the msg argument (not innerHTML), append these children to toast, then append
toast to document.body and keep the existing timeout/remove logic; this ensures
msg is XSS-safe while preserving the toast styling and behavior.
- Around line 298-312: handlePostLoginRedirect currently redirects to whatever
is in sessionStorage['post_login_redirect'] allowing open-redirects; change it
to validate the stored value before navigating: when reading redirectUrl (from
sessionStorage.getItem('post_login_redirect')), construct a URL object with
location.origin as the base (new URL(redirectUrl, location.origin)) and only
allow the redirect if url.origin === location.origin and url.protocol is http:
or https: and url.pathname starts with '/'; also explicitly reject values
starting with '//' or 'javascript:' and fallback to the default
'/dashboard.html' if validation fails, then remove the stored item and perform
the safe navigation in handlePostLoginRedirect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 15d376a3-6142-4104-801e-6bba2daf69e4
📒 Files selected for processing (4)
public/classroom_poc.htmlpublic/index.htmlpublic/js/layout.jspublic/login.html
Comment on lines
+298
to
+312
| // Handle redirect after successful login | ||
| function handlePostLoginRedirect() { | ||
| // Check if there's a stored redirect URL | ||
| const redirectUrl = sessionStorage.getItem('post_login_redirect'); | ||
|
|
||
| if (redirectUrl) { | ||
| // Clear it so it doesn't trigger again | ||
| sessionStorage.removeItem('post_login_redirect'); | ||
| // Redirect to the original destination | ||
| window.location.href = redirectUrl; | ||
| } else { | ||
| // Default redirect | ||
| window.location.href = '/dashboard.html'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Open-redirect risk via post_login_redirect.
handlePostLoginRedirect() navigates to whatever URL sits in sessionStorage['post_login_redirect'] without validating that it's same-origin. Combined with the ?next= handler at lines 321‑323, an attacker can craft https://yoursite/login.html?next=https%3A%2F%2Fevil.example%2Fphish — after a legitimate login, the user is silently bounced to evil.example, which is a classic phishing vector.
Recommend constraining to same-origin relative paths only:
🛡️ Suggested guard
function handlePostLoginRedirect() {
- // Check if there's a stored redirect URL
const redirectUrl = sessionStorage.getItem('post_login_redirect');
-
- if (redirectUrl) {
- // Clear it so it doesn't trigger again
- sessionStorage.removeItem('post_login_redirect');
- // Redirect to the original destination
- window.location.href = redirectUrl;
- } else {
- // Default redirect
- window.location.href = '/dashboard.html';
- }
+ sessionStorage.removeItem('post_login_redirect');
+ // Only honor same-origin, path-only redirects to prevent open-redirect abuse.
+ const safe = redirectUrl && /^\/(?!\/)/.test(redirectUrl);
+ window.location.href = safe ? redirectUrl : '/dashboard.html';
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@public/login.html` around lines 298 - 312, handlePostLoginRedirect currently
redirects to whatever is in sessionStorage['post_login_redirect'] allowing
open-redirects; change it to validate the stored value before navigating: when
reading redirectUrl (from sessionStorage.getItem('post_login_redirect')),
construct a URL object with location.origin as the base (new URL(redirectUrl,
location.origin)) and only allow the redirect if url.origin === location.origin
and url.protocol is http: or https: and url.pathname starts with '/'; also
explicitly reject values starting with '//' or 'javascript:' and fallback to the
default '/dashboard.html' if validation fails, then remove the stored item and
perform the safe navigation in handlePostLoginRedirect.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces necessary authentication guards and UX improvements for the Virtual Classroom entry points to prevent unauthorized access and console errors.
Conditional UI (Homepage): Conditionally rendering the
Join Lobby Classroombutton. Unauthenticated users will now see a prompt to login instead of redirecting to the classroom.Classroom Auth Guard: Added validation logic to the frontend. If a user attempts to bypass the homepage and directly access the classroom URL without an active session, they are alerted and automatically redirected to the login page.
Logged In:
Logged Out:
alert:
Purpose
Implements client-side authentication guards and conditional UI for the Virtual Classroom entry flow to prevent unauthorized access and provide clearer UX for authenticated vs unauthenticated users.
Key Changes
Impact
Notes