Feature Changes (detailed in description) - #1371
Conversation
New /support page: a topic list (currently just "Password Reset") that deep-links via ?topic=reset straight into a "Let's Reset Your Password" wizard, skipping the topic list -- used by "Forgot your password?" links elsewhere so /support can grow to cover more than password reset later without adding friction. The wizard animates through GitHub ID entry -> school Google OAuth sign-in -> new password, calling the new Spring OAuth-verified reset endpoints (/mvc/person/reset/oauth/verify, /mvc/person/reset/oauth/complete). The Google ID token is only relayed raw to the backend; the digit-match/identity check happens server-side, not in this client code. login.md: "Forgot your password?" now links to /support?topic=reset instead of an embedded reset panel on the login page itself. profile.html: removed the old "New Password" field, which let a logged-in user overwrite their password with zero verification (no current-password check, no reset token). Replaced with a "Forgot your password?" link into the same verified reset flow. Cleaned up saveChanges()'s now-dead password-save branch and the logout alert text that referenced it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a "Request a Ticket Instead" button to the OAuth password-reset wizard, shown when the reset rate limit is hit; it posts to Spring's new /mvc/person/reset/ticket endpoint so an admin can grant more attempts. Also: styles the .password-length validation class (was referenced but never defined), and de-dupes the Google OAuth client_id into a single GOOGLE_CLIENT_ID export in config.js instead of separate copies in login.md and support.md.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new /support page that owns a verified, multi-step password reset flow (GitHub ID → Google school account verification → password set), and removes the previously unverified password change path from the profile page. It also centralizes the Google OAuth client id in a shared config export and updates entry points (login/profile) to route users into the new reset flow.
Changes:
- Added
navigation/authentication/support.mdimplementing a password reset wizard with OAuth verification, rate-limit ticket fallback, and client-side password validation. - Updated login and profile UX to link “Forgot your password?” to
/support?topic=reset, removing the unverified profile password change. - Centralized
GOOGLE_CLIENT_IDinassets/js/api/config.jsand added missing.password-lengthstyling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| navigation/authentication/support.md | New Support page and password reset wizard (OAuth verify/complete + ticket fallback). |
| navigation/authentication/login.md | Adds “Forgot your password?” link and imports shared GOOGLE_CLIENT_ID. |
| assets/js/api/config.js | Exports GOOGLE_CLIENT_ID for reuse across OAuth flows. |
| _sass/open-coding/elements/forms/passwordvalidation.scss | Adds .password-length style used by password validation UI. |
| _layouts/profile.html | Removes unverified password field; replaces with link into verified reset flow and updates logout alert copy. |
Suppressed comments (1)
navigation/authentication/support.md:113
- These password inputs rely on placeholder text as their only accessible name. Add explicit labels or
aria-labels so screen readers can announce them reliably.
<input type="password" id="resetNewPassword" placeholder="New Password" required>
</div>
<div class="form-group">
<input type="password" id="resetConfirmPassword" placeholder="Confirm New Password" required>
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function showResetOAuthStatus(message, isError = false) { | ||
| const statusDiv = document.getElementById('reset-oauth-status'); | ||
| statusDiv.innerHTML = `<div class="${isError ? 'oauth-error' : 'oauth-success'}">${message}</div>`; | ||
| } |
| if (window.google && window.google.accounts) { | ||
| window.google.accounts.id.initialize({ | ||
| client_id: GOOGLE_CLIENT_ID, | ||
| callback: handleGoogleResetSignIn | ||
| }); | ||
| window.google.accounts.id.renderButton( | ||
| document.getElementById('reset-g_id_signin_container'), | ||
| { type: 'standard', size: 'large', theme: 'filled_blue', text: 'signin_with', shape: 'rectangular' } | ||
| ); | ||
| } | ||
| } |
| if (password.length < 8) { | ||
| alert('Password must be at least 8 characters long.'); | ||
| return; | ||
| } | ||
| if (password !== confirmPassword) { | ||
| alert('Passwords do not match. Please try again.'); | ||
| return; | ||
| } |
| <!-- Landing view: list of support topics --> | ||
| <div id="support-topics-container" style="max-width: 700px; margin: 0 auto; padding: 0 1.5rem;"> | ||
| <ul class="support-topic-list"> | ||
| <li class="support-topic-item" onclick="openSupportTopic('reset')">Password Reset</li> |
| <hr> | ||
| <div id="reset-step-uid" class="support-step active"> | ||
| <div class="form-group"> | ||
| <input type="text" id="resetUid" placeholder="GitHub ID" required> |
| <style> | ||
| .support-topic-list { | ||
| list-style: none; | ||
| padding: 0; | ||
| margin: 0; |
|
|
||
| <script type="module"> | ||
| import { login, pythonURI, javaURI, fetchOptions } from '{{site.baseurl}}/assets/js/api/config.js'; | ||
| import { login, pythonURI, javaURI, fetchOptions, GOOGLE_CLIENT_ID } from '{{site.baseurl}}/assets/js/api/config.js'; |
|
This does not look like something we should publish in GitHub: 65827797404-ccjleg7jg4g2an8ddpmhnlca4ii2gk8q.apps.googleusercontent.com We should have a UI and workflow review prior to pull. |
- showResetOAuthStatus(): build the status message via createElement/ textContent instead of innerHTML string interpolation. - startOAuthReset(): show an inline error if the Google GSI script hasn't loaded, and clear the sign-in container before rendering so re-entering the step doesn't stack a second button. - submitOAuthResetPassword(): route password/mismatch errors through the existing inline validation message instead of alert(). - All .support-topic-item <li>s: add role="button" tabindex="0" plus a delegated keydown listener, so they're keyboard-activatable. - resetUid/resetNewPassword/resetConfirmPassword: add matching aria-labels (previously placeholder-only accessible names). - Move support.md's inline <style> block and inline style="..." attributes into a new SCSS partial (elements/forms/support.scss), wired into the same import chain oauth.scss/passwordvalidation.scss already use. Verified by building with Jekyll and checking the compiled stylesheet for the new classes. - login.md's signup Google client id, hardcoded in data-client_id (duplicating the same literal in config.js's GOOGLE_CLIENT_ID), now reads from one shared _config.yml value via Liquid. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- showResetOAuthStatus(): build the status message via createElement/ textContent instead of innerHTML string interpolation. - startOAuthReset(): show an inline error if the Google GSI script hasn't loaded, and clear the sign-in container before rendering so re-entering the step doesn't stack a second button. - submitOAuthResetPassword(): route password/mismatch errors through the existing inline validation message instead of alert(). - .support-topic-item <li>: add role="button" tabindex="0" plus a delegated keydown listener, so it's keyboard-activatable. - resetUid/resetNewPassword/resetConfirmPassword: add matching aria-labels (previously placeholder-only accessible names). - Move the inline <style> block and inline style="..." attributes into the same SCSS partial used on pwd-reset-support (elements/forms/support.scss). - login.md's signup Google client id now reads from one shared _config.yml value via Liquid instead of being hardcoded (this branch's login.md already imported GOOGLE_CLIENT_ID from config.js, so this applied cleanly -- p1-forgot-password-button's older variant, which hardcodes a local copy instead of importing, was left alone rather than backported into). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- showResetOAuthStatus(): build the status message via createElement/ textContent instead of innerHTML string interpolation. - startOAuthReset(): show an inline error if the Google GSI script hasn't loaded, and clear the sign-in container before rendering so re-entering the step doesn't stack a second button. - submitOAuthResetPassword(): route password/mismatch errors through the existing inline validation message instead of alert(). - .support-topic-item <li>: add role="button" tabindex="0" plus a delegated keydown listener, so it's keyboard-activatable. - resetUid/resetNewPassword/resetConfirmPassword: add matching aria-labels (previously placeholder-only accessible names). - Move the inline <style> block and inline style="..." attributes into the same SCSS partial used on pwd-reset-support/p2-support-page (elements/forms/support.scss). - login.md's signup Google client id now reads from one shared _config.yml value via Liquid instead of being hardcoded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
These techniques are not allowed in production systems. You need a .env that contains keys. export const GOOGLE_CLIENT_ID = "65827797404-ccjleg7jg4g2an8ddxxxxxxxxxx.apps.googleusercontent.com"; |
- showResetOAuthStatus(): build the status message via createElement/ textContent instead of innerHTML string interpolation. - startOAuthReset(): show an inline error if the Google GSI script hasn't loaded, and clear the sign-in container before rendering so re-entering the step doesn't stack a second button. - submitOAuthResetPassword(): route password/mismatch errors through the existing inline validation message instead of alert(). - .support-topic-item <li>: add role="button" tabindex="0" plus a delegated keydown listener, so it's keyboard-activatable. - resetUid/resetNewPassword/resetConfirmPassword: add matching aria-labels (previously placeholder-only accessible names). - Move the inline <style> block and inline style="..." attributes into the same SCSS partial used on pwd-reset-support/p2-support-page (elements/forms/support.scss). - login.md's signup Google client id now reads from one shared _config.yml value via Liquid instead of being hardcoded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
John Mortensen's review on PR Open-Coding-Society#1371: "These techniques are not allowed in production systems. You need a .env that contains keys." -- the client id was hardcoded in _config.yml (a tracked file), which config.js's GOOGLE_CLIENT_ID and login.md's data-client_id both read via Liquid. That made it a single source of truth, but that source was still committed to a public repo. _config.yml no longer sets google_client_id at all. It's injected at build time into a gitignored override config instead, using the exact mechanism the CI workflow already had for baseurl (_config.override.yml, generated fresh each run) rather than inventing a new one: - Locally: scripts/generate_local_config_override.sh reads .env (see the new .env.example) and writes _config.local.yml -- the file Makefile's jekyll-serve already referenced in a comment as supported but never actually wired up. Both jekyll-serve and start-dev.sh now run the script and pass --config _config.yml,_config.local.yml. - In CI: the existing "Compute and apply baseurl" step now also writes google_client_id into _config.override.yml, sourced from a GOOGLE_CLIENT_ID repository secret. That secret doesn't exist yet -- needs a repo admin to add it in Settings > Secrets and variables > Actions before the deployed site's Google sign-in button will render; the build itself succeeds either way. Verified locally: generated _config.local.yml from a real .env, rebuilt Jekyll with the merged config, confirmed both config.js's GOOGLE_CLIENT_ID and login.md's data-client_id still resolve to the real value at runtime, and confirmed the literal id string no longer appears anywhere in _config.yml, config.js, or the navigation/authentication/*.md files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
John Mortensen's review on PR Open-Coding-Society#1371: - "This does not look like something we should publish in GitHub: 65827797404-ccjleg7jg4g2an8ddpmhnlca4ii2gk8q.apps.googleusercontent.com We should have a UI and workflow review prior to pull." - "These techniques are not allowed in production systems. You need a .env that contains keys." The client id was hardcoded in two places -- assets/js/api/config.js's GOOGLE_CLIENT_ID and login.md's data-client_id -- both now read {{ site.google_client_id }} via Liquid instead, one source of truth between the two. _config.yml itself no longer sets google_client_id at all -- that's the actual fix John is asking for, not just deduplicating the literal. It's injected at build time into a gitignored override config instead: - Locally: new scripts/generate_local_config_override.sh reads .env (see the new .env.example) and writes _config.local.yml -- the file Makefile's jekyll-serve already referenced in a comment as supported but never actually wired up. jekyll-serve now runs the script and passes --config _config.yml,_config.local.yml. - In CI: the existing "Compute and apply baseurl" step in .github/workflows/jekyll-gh-pages.yml now also writes google_client_id into _config.override.yml (the same file it already generates for baseurl), sourced from a GOOGLE_CLIENT_ID repository secret. That secret doesn't exist yet -- needs a repo admin to add it in Settings > Secrets and variables > Actions before the deployed site's Google sign-in button will render; the build itself succeeds either way. Verified locally: generated _config.local.yml from a real .env, rebuilt Jekyll with the merged config, confirmed both config.js's GOOGLE_CLIENT_ID and login.md's data-client_id still resolve to the real value at runtime, and confirmed the literal id string no longer appears anywhere in tracked source (_config.yml, config.js, or navigation/authentication/*.md). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
See previous comment. |
…sword-button pages: Point "Forgot your password?" at the verified reset flow (1/3 split of #1371)
pages: Add /support page with OAuth reset wizard (2/3 split of #1371)
tl;dr Changes
pages: Add a /support page with the verified password reset flow
What this does
Adds a
/supportpage that owns password reset, and removes the unverified passwordchange that used to live on the profile page.
Changes
New page:
navigation/authentication/support.mdwithout restructuring the page).
?topic=resetdeep links straight into the reset wizard and skips the topic list,so links from elsewhere on the site do not cost the user an extra click.
account, then set the new password. It calls the new Spring endpoints
POST /mvc/person/reset/oauth/verifyandPOST /mvc/person/reset/oauth/complete.check (matching the last 5 digits of the school email against the account's student ID)
happens server side, not here.
button that posts to
POST /mvc/person/reset/ticketso an admin can grant more attempts.This button does not require the OAuth step to have succeeded, since the whole point is
that the user cannot get through it right now.
navigation/authentication/login.md/support?topic=resetinstead of an embedded resetpanel on the login page.
GOOGLE_CLIENT_IDimport instead of its own hardcoded copy._layouts/profile.htmlwith no verification at all: no current password check and no reset token. Replaced with a
"Forgot your password?" link into the verified flow.
saveChanges()and updated the logout alerttext that referenced it.
assets/js/api/config.jsGOOGLE_CLIENT_IDonce, so the client id is not copy-pasted into login.md andsupport.md separately.
assets/style/elements/forms/passwordvalidation.scss.password-lengthstyle. It was already being applied by the validation codebut never defined.
Testing
Load
/support?topic=reset, enter a GitHub ID, sign in with a@stu.powayusd.comaccountwhose trailing 5 digits match that account's student ID, set a new password, and confirm the
new password works on
/login. Repeat the verify step past the rate limit to see the ticketbutton appear.