Skip to content

fix: passkey 文案 - #1936

Merged
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
seefs001:fix/passkey
Sep 30, 2025
Merged

fix: passkey 文案#1936
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
seefs001:fix/passkey

Conversation

@seefs001

@seefs001 seefs001 commented Sep 30, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes
    • Passkey settings now auto-fill Origins with the server address when left empty or set to "[]", ensuring a valid default for WebAuthn.
  • Documentation
    • Updated help text for Passkey settings:
      • rp_id: clarified that leaving it empty uses the server address (without http:// or https://).
      • origins: clarified that leaving it empty defaults to the server address; multiple Origins should be comma-separated (e.g., https://newapi.pro,https://newapi.com), must use HTTPS, and should not include square brackets.

@coderabbitai

coderabbitai Bot commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adds backend defaulting in GetPasskeySettings to set Origins to ServerAddress when empty or "[]", following existing RPID init. Updates frontend descriptions for passkey.rp_id and passkey.origins to clarify defaults and formatting requirements. No API signature or validation logic changes.

Changes

Cohort / File(s) Summary
Backend: Passkey settings defaulting
setting/system_setting/passkey.go
Adds post-RPID defaulting: if Origins is empty or equals "[]", set Origins to ServerAddress. No other control flow or error handling changes.
Frontend: Settings UI text updates
web/src/components/settings/SystemSetting.jsx
Revises extraText for passkey.rp_id and passkey.origins to explain defaults (server address), required formats (no http/https for rp_id; https scheme for origins), no brackets, examples provided. No functional changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant S as GetPasskeySettings (server)
  Note over S: Initialize settings

  C->>S: Request passkey settings
  activate S
  S->>S: Initialize/confirm RPID
  alt Origins empty or "[]"
    S->>S: Set Origins = ServerAddress
    Note right of S: New defaulting step
  else
    S->>S: Keep provided Origins
  end
  S-->>C: Return settings (RPID, Origins, ...)
  deactivate S
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitched my nose at configs plain,
Now Origins hop to server’s lane.
The fields align, no brackets stray,
With https to guide the way.
I stamp my paw—defaults set right,
Carrots cached, the passkeys tight. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “fix: passkey 文案” succinctly communicates that the pull request is updating the Passkey copy text, which directly reflects the user-facing text changes in the settings component.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

@Calcium-Ion
Calcium-Ion merged commit 3ba2aae into QuantumNous:main Sep 30, 2025
1 check was pending

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
web/src/components/settings/SystemSetting.jsx (1)

1114-1114: Verify passkey.origins default behavior and add input validation

  • Empty “Allowed Origins” now fall back to the server address instead of allowing any origin—confirm this change is intentional, that existing deployments have a migration path, and that release notes or a migration guide are in place.
  • No matching backend code or documentation was found—ensure server behavior and user-facing docs are updated to reflect this.
  • Recommend updating submitPasskeySettings() to validate the origins input (e.g. reject or warn on [/] or other invalid formats) before submission.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1bd791d and 0a6f39e.

📒 Files selected for processing (2)
  • setting/system_setting/passkey.go (1 hunks)
  • web/src/components/settings/SystemSetting.jsx (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
setting/system_setting/passkey.go (1)
setting/system_setting/system_setting_old.go (1)
  • ServerAddress (3-3)
🔇 Additional comments (1)
web/src/components/settings/SystemSetting.jsx (1)

1053-1053: Updated guidance correctly reflects RPID requirements.

The text now clearly states that empty values default to the server address and emphasizes that the RP ID cannot include protocol prefixes (http:// or https://), which aligns with WebAuthn specifications where the RP ID must be a domain name only.

Comment on lines +45 to +47
if defaultPasskeySettings.Origins == "" || defaultPasskeySettings.Origins == "[]" {
defaultPasskeySettings.Origins = ServerAddress
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

LGTM! Origins defaulting logic aligns with frontend guidance.

The logic correctly defaults Origins to ServerAddress when empty or when set to the literal string "[]". This matches the updated frontend text stating "为空则默认使用服务器地址" (defaults to server address when empty).

Verify that ServerAddress is properly validated elsewhere to ensure it contains the protocol (https://) as required by the WebAuthn spec and indicated in the UI text. Run this script to check how ServerAddress is initialized and validated:


🏁 Script executed:

#!/bin/bash
# Description: Verify ServerAddress initialization and validation

# Search for ServerAddress assignment and validation
rg -nP -C3 'ServerAddress\s*=' --type=go

# Search for any validation of ServerAddress format
rg -nP -C3 '(http://|https://).*ServerAddress|ServerAddress.*(http://|https://)' --type=go

Length of output: 2869


Enforce HTTPS scheme for ServerAddress
Add validation when setting ServerAddress (e.g., in model/option.go at the “ServerAddress” case) or before deriving the RPID (passkey.go:35-40) to reject or normalize any value that doesn’t start with “https://”, ensuring origins always comply with WebAuthn’s HTTPS requirement.

🤖 Prompt for AI Agents
In setting/system_setting/passkey.go around lines 45 to 47 (and the
ServerAddress handling in model/option.go at the “ServerAddress” case), enforce
that ServerAddress uses the HTTPS scheme by validating the value before using it
to derive origins/RPID: if the value does not start with "https://" either
reject it (return an error) or normalize it by prepending "https://" after
ensuring it is a valid URL; perform the check where ServerAddress is set
(model/option.go) or immediately before deriving RPID in passkey.go (lines
~35-40) so origins are always HTTPS-compliant, and surface an explicit
validation error if the scheme is not HTTPS rather than silently accepting or
using non-HTTPS values.

x22x22 pushed a commit to x22x22/new-api that referenced this pull request Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants