sync: dev to extern-contrib#979
Merged
Merged
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Contributor
Reviewer's GuideAdds SSO-based login support to the messages page, wires it into initialization and navigation, and introduces new legal/policy and footer content plus a link to the SSO center on the homepage. Sequence diagram for new SSO-based login flow on messages pagesequenceDiagram
actor User
participant Browser
participant SSOService
participant BackendAPI
User->>Browser: Click btn_sso_login
Browser->>Browser: startSsoLogin()
Browser->>Browser: generateUuidV4() for state
Browser->>Browser: Save state to localStorage
Browser->>SSOService: Redirect to OAUTH_AUTHORIZE_URL
User->>SSOService: Authenticate and authorize
SSOService-->>Browser: Redirect to OAUTH_REDIRECT_URI with code and state
Browser->>Browser: init() (on load)
Browser->>Browser: handleSsoCallback()
Browser->>Browser: Read code and state from URL
Browser->>Browser: Compare state with localStorage
alt State mismatch
Browser->>Browser: Clear code and state from URL
Browser->>User: showToast(统一认证登录失败: state 校验失败)
else State valid
Browser->>BackendAPI: POST API_BASE ExchangeSSOCode(Code, State, RedirectURI, ClientID)
BackendAPI-->>Browser: { Success, Data{Username, SessionID} }
alt Exchange or data invalid
Browser->>User: showToast(统一认证登录失败: 错误信息)
else Exchange success
Browser->>Browser: Remove saved state from localStorage
Browser->>Browser: saveSession(Username, SessionID)
Browser->>Browser: Clear code and state from URL
Browser->>User: showToast(统一认证登录成功)
Browser->>Browser: onLoggedIn()
end
end
Flow diagram for handleSsoCallback logicflowchart TD
Start["init() on page load"] --> CheckLoggedIn{Already_logged_in?}
CheckLoggedIn -->|Yes| EndLoggedIn["onLoggedIn()"]
CheckLoggedIn -->|No| HandleSSO["handleSsoCallback()"]
HandleSSO --> HasCode{URL_has_code?}
HasCode -->|No| ShowLogin["Show login screen"]
HasCode -->|Yes| LoadState["Load state from URL and localStorage"]
LoadState --> StateValid{state_equals_saved?}
StateValid -->|No| ClearBadState["Clear code and state from URL"]
ClearBadState --> ToastStateError["showToast(state 校验失败)"]
ToastStateError --> ShowLogin
StateValid -->|Yes| ExchangeCode["exchangeSsoCode(Code, State)"]
ExchangeCode --> ExchangeOk{Success and Data valid?}
ExchangeOk -->|No| ToastExchangeError["showToast(统一认证登录失败)"]
ToastExchangeError --> ShowLogin
ExchangeOk -->|Yes| SaveSession["saveSession(Username, SessionID)"]
SaveSession --> ClearParams["Clear code and state from URL"]
ClearParams --> ToastSuccess["showToast(统一认证登录成功)"]
ToastSuccess --> EndLoggedIn
ShowLogin --> EndAnon["Wait for user to choose login method"]
Flow diagram for new footer and policy navigation on homepageflowchart TD
Home["index.html homepage"] --> NavSSO["Navbar link 统一认证中心"]
Home --> Footer["Footer container"]
NavSSO -->|Click| OpenSSO["Open https://sso.xmoj-bbs.me in new tab"]
Footer --> ICPLink["萌ICP备20240425号 link"]
Footer --> PrivacyLink["privacy.html link"]
Footer --> TermsLink["terms.html link"]
Footer --> ChildLink["child-protection.html link"]
ICPLink -->|Click| OpenICP["Open icp.gov.moe record page"]
PrivacyLink -->|Click| OpenPrivacy["Open privacy.html"]
TermsLink -->|Click| OpenTerms["Open terms.html"]
ChildLink -->|Click| OpenChild["Open child-protection.html"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Deploying xmoj-script-dev-channel with
|
| Latest commit: |
f6b0dcc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9712c4c6.xmoj-script-dev-channel.pages.dev |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
handleSsoCallback, when thestatecheck fails you show a toast and strip query params but leaveSTORAGE_OAUTH_STATEuntouched; consider removing the stored state in this branch as well to avoid stale state values blocking future SSO attempts. - All OAuth-related values (authorize URL, client id, scope, redirect URI) are hard-coded in
messages.html; it may be more maintainable to centralize these into a single config object or injected config so they can be updated without touching UI logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `handleSsoCallback`, when the `state` check fails you show a toast and strip query params but leave `STORAGE_OAUTH_STATE` untouched; consider removing the stored state in this branch as well to avoid stale state values blocking future SSO attempts.
- All OAuth-related values (authorize URL, client id, scope, redirect URI) are hard-coded in `messages.html`; it may be more maintainable to centralize these into a single config object or injected config so they can be updated without touching UI logic.
## Individual Comments
### Comment 1
<location path="messages.html" line_range="1012-1021" />
<code_context>
+ return res.json();
+}
+
+async function handleSsoCallback() {
+ var url = new URL(location.href);
+ var code = url.searchParams.get('code');
+ var state = url.searchParams.get('state');
+ if (!code) return false;
+
+ var expectedState = localStorage.getItem(STORAGE_OAUTH_STATE);
+ if (!state || !expectedState || state !== expectedState) {
+ showToast('统一认证登录失败：state 校验失败', 'danger');
+ url.searchParams.delete('code');
+ url.searchParams.delete('state');
+ history.replaceState(null, '', url.pathname + (url.searchParams.toString() ? '?' + url.searchParams.toString() : ''));
+ return false;
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Cleaning the URL after SSO may drop the hash fragment and duplicates URL-rewrite logic across success and failure paths.
Both the state-mismatch and success branches call `history.replaceState` with `url.pathname + ...` but omit `url.hash`, which will drop any existing fragment (e.g. `#anchor`) that other code may rely on. The URL-rewrite logic that removes `code`/`state` and rebuilds the URL is also duplicated.
Consider extracting a helper like `clearSsoParams(url)` that:
- Removes `code` and `state` from `url.searchParams`.
- Rebuilds the URL including remaining query parameters and `url.hash`.
Then call it from both branches to avoid duplication and preserve consistent URL behavior.
Suggested implementation:
```
function clearSsoParams(url) {
url.searchParams.delete('code');
url.searchParams.delete('state');
var search = url.searchParams.toString();
var newUrl = url.pathname + (search ? '?' + search : '') + (url.hash || '');
history.replaceState(null, '', newUrl);
}
async function handleSsoCallback() {
```
```
var url = new URL(location.href);
var code = url.searchParams.get('code');
var state = url.searchParams.get('state');
if (!code) return false;
var expectedState = localStorage.getItem(STORAGE_OAUTH_STATE);
if (!state || !expectedState || state !== expectedState) {
showToast('统一认证登录失败：state 校验失败', 'danger');
clearSsoParams(url);
return false;
}
```
Search the rest of `handleSsoCallback` (and any related SSO-flow code) for other places where `code`/`state` are removed and `history.replaceState` is called with `url.pathname + ...` but without `url.hash`. Replace those sequences with `clearSsoParams(url);` to deduplicate the logic and ensure the hash fragment is preserved consistently on all SSO completion paths.
</issue_to_address>
### Comment 2
<location path="messages.html" line_range="967-976" />
<code_context>
});
});
+function generateUuidV4() {
+ if (window.crypto && window.crypto.randomUUID) return window.crypto.randomUUID();
+ if (window.crypto && window.crypto.getRandomValues) {
+ var bytes = new Uint8Array(16);
+ window.crypto.getRandomValues(bytes);
+ bytes[6] = (bytes[6] & 0x0f) | 0x40;
+ bytes[8] = (bytes[8] & 0x3f) | 0x80;
+ var hex = Array.from(bytes).map(function(b) { return b.toString(16).padStart(2, '0'); }).join('');
+ return [hex.slice(0, 8), hex.slice(8, 12), hex.slice(12, 16), hex.slice(16, 20), hex.slice(20)].join('-');
+ }
+ return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {
+ var r = Math.random() * 16 | 0;
+ var v = c === 'x' ? r : (r & 0x3 | 0x8);
+ return v.toString(16);
+ });
</code_context>
<issue_to_address>
**🚨 suggestion (security):** The `Math.random` fallback for `generateUuidV4` weakens the entropy of the OAuth `state` parameter on older browsers.
Using `crypto.randomUUID`/`getRandomValues` is appropriate, but falling back to `Math.random` for an OAuth `state` value weakens CSRF protection in older environments. For those environments, consider either treating missing `window.crypto` as unsupported (and failing with an error) or requiring at least `crypto.getRandomValues` and otherwise not proceeding, to avoid low‑entropy, more predictable tokens.
Suggested implementation:
```
function generateUuidV4() {
if (window.crypto && typeof window.crypto.randomUUID === 'function') {
return window.crypto.randomUUID();
}
if (window.crypto && typeof window.crypto.getRandomValues === 'function') {
var bytes = new Uint8Array(16);
window.crypto.getRandomValues(bytes);
// Per RFC 4122 section 4.4: set version to 4 and variant to RFC 4122
bytes[6] = (bytes[6] & 0x0f) | 0x40;
bytes[8] = (bytes[8] & 0x3f) | 0x80;
var hex = Array.from(bytes)
.map(function(b) { return b.toString(16).padStart(2, '0'); })
.join('');
return [
hex.slice(0, 8),
hex.slice(8, 12),
hex.slice(12, 16),
hex.slice(16, 20),
hex.slice(20)
].join('-');
}
// Do not fall back to Math.random for security-sensitive values like OAuth state
throw new Error('Secure random number generator not available; cannot generate secure UUID v4.');
}
```
If `generateUuidV4()` is used during OAuth flow (e.g., to generate the `state` parameter), callers should be prepared for it to throw in older/unsupported browsers. For a better UX, you may want to catch this error where `generateUuidV4()` is called and display a user-friendly message indicating that the browser is not supported for secure login.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="child-protection.html">
<violation number="1" location="child-protection.html:8">
P2: External CDN stylesheet is loaded without SRI, which weakens frontend supply-chain integrity protection.</violation>
</file>
<file name="messages.html">
<violation number="1" location="messages.html:977">
P1: The `Math.random` fallback produces predictable output (only ~52 bits of entropy) and is explicitly listed as insecure for OAuth `state` generation. Since `state` is the sole CSRF defense for the OAuth flow, this fallback should either require `crypto.getRandomValues` (supported since IE 11) or throw an error rather than silently downgrading to a weak source. Browsers lacking `window.crypto` are ancient enough that failing with a clear message is preferable to weakened CSRF protection.</violation>
<violation number="2" location="messages.html:1044">
P2: The `catch` block in `handleSsoCallback` does not clean the OAuth `code`/`state` query params from the URL or remove the stored OAuth state. If the code exchange fails, refreshing the page retriggers the same failed exchange in a loop. Both other exit paths (state-mismatch and success) already perform this cleanup.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
* switch endpoint to api.xmoj-script.uk for everything Agent-Logs-Url: https://github.com/XMOJ-Script-dev/XMOJ-Script/sessions/3a87c529-2f63-4617-8be5-5b9f3c33f751 Co-authored-by: PythonSmall-Q <106425289+PythonSmall-Q@users.noreply.github.com> * Update ASSET_BASE URL to use assets subdomain Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net> * 3.4.6 * Update version info to 3.4.6 * Update image upload URLs to new asset location Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net> * Update time and description of 3.4.6 * Update ServerURL for script debugging Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net> * Update time and description of 3.4.6 * Update SSO button for development version Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net> * Update time and description of 3.4.6 * Remove badge from messages link in navigation Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net> * Update time and description of 3.4.6 --------- Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PythonSmall-Q <106425289+PythonSmall-Q@users.noreply.github.com> Co-authored-by: Shan Wenxiao <seanoj_noreply@yeah.net> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
hendragon-bot
Bot
added
user-script
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="messages.html">
<violation number="1" location="messages.html:77">
P2: The new SSO tab key (`sso-develop`) does not match the tab router (`sso`), so clicking it hides all login panels and breaks the SSO login UI.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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
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.
sync-branches: New code has just landed in dev, so let's bring extern-contrib up to speed!
Summary by Sourcery
Add unified SSO-based login support and update the site footer and legal pages.
New Features:
Enhancements:
Summary by cubic
Adds unified SSO login on the Messages page and refreshes legal/navigation. Migrates all API/WebSocket and asset endpoints to xmoj-script.uk and bumps the script to 3.4.6.
New Features
messages.htmlwith OAuth (state validation,ExchangeSSOCodeexchange, saves session).privacy.html,terms.html,child-protection.html, and an integrated footer with ICP and legal links.Refactors
3.4.6.Written for commit f6b0dcc. Summary will update on new commits.