feat: add config GUIs for RFC-conformance fields missing them - #27
Merged
Conversation
The RFC-conformance work (#26) wired several configurable fields through the backend but left two with no dashboard GUI: security.txt (RFC 9116) — security_contact / security_policy_url were added to SiteConfig and the admin PATCH allowlist, but the admin settings page had no fields, so /.well-known/security.txt could not be enabled from the UI. Add a "Security disclosure" card to the General tab. Per-app client auth (RFC 7591/7523) — token_endpoint_auth_method / jwks / jwks_uri (migration 0065) were settable only via Dynamic Client Registration; the apps API PATCH and App Detail did not expose them. Thread them through the apps PATCH handler (with DCR-matching validation) and safeApp, add them to the OAuthApp/CreateAppBody types, and surface a token endpoint auth-method dropdown plus jwks / jwks_uri inputs (shown for private_key_jwt) in App Detail settings. i18n (en + zh) and docs (admin.md, oauth.md + zh) updated to match. Verified: tsc -b, eslint, and a Vite production build all pass.
审查者指南完成 RFC 合规字段缺失的仪表板和 API 连接：管理员可以配置 RFC 9116 security.txt 元数据，应用所有者可以选择令牌端点身份验证方法，并管理带验证、持久化、本地化界面和更新文档的 private_key_jwt 密钥来源。 应用 private_key_jwt 配置的时序图sequenceDiagram
participant Owner as AppOwner
participant UI as AppDetail
participant API as AppsAPI
participant DB as Database
AppOwner->>AppDetail: Select private_key_jwt and enter jwks or jwks_uri
AppDetail->>AppsAPI: PATCH /apps/:id
AppsAPI->>AppsAPI: Validate token_endpoint_auth_method
AppsAPI->>AppsAPI: Validate jwks or jwks_uri
AppsAPI->>Database: UPDATE oauth_apps
Database-->>AppsAPI: Updated app
AppsAPI-->>AppDetail: safeApp with auth settings
AppDetail-->>AppOwner: Show saved configuration
文件级变更
提示和命令与 Sourcery 交互
自定义你的使用体验访问你的仪表板来：
获取帮助Original review guide in EnglishReviewer's GuideCompletes the missing dashboard and API plumbing for RFC-conformance fields: administrators can configure RFC 9116 security.txt metadata, while app owners can select token endpoint authentication methods and manage private_key_jwt key sources with validation, persistence, localized UI, and updated documentation. Sequence diagram for app private_key_jwt configurationsequenceDiagram
participant Owner as AppOwner
participant UI as AppDetail
participant API as AppsAPI
participant DB as Database
AppOwner->>AppDetail: Select private_key_jwt and enter jwks or jwks_uri
AppDetail->>AppsAPI: PATCH /apps/:id
AppsAPI->>AppsAPI: Validate token_endpoint_auth_method
AppsAPI->>AppsAPI: Validate jwks or jwks_uri
AppsAPI->>Database: UPDATE oauth_apps
Database-->>AppsAPI: Updated app
AppsAPI-->>AppDetail: safeApp with auth settings
AppDetail-->>AppOwner: Show saved configuration
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
你好——我发现了 2 个问题
给 AI 代理的提示
请处理本次代码审查中的评论：
## 单条评论
### 评论 1
<location path="src/pages/apps/AppDetail.tsx" line_range="1478" />
<code_context>
+ <Option value="client_secret_post">client_secret_post</Option>
+ <Option value="private_key_jwt">private_key_jwt</Option>
+ <Option value="none">none</Option>
+ </Dropdown>
+ </Field>
+ {form.token_endpoint_auth_method === "private_key_jwt" && (
</code_context>
<issue_to_address>
**issue (bug_risk)：** 在 App Detail 中选择 `none` 会将 `token_endpoint_auth_method = "none"` 保存下来，但令牌认证路径仍会对除 `private_key_jwt` 之外的所有方法调用 `clientSecretValid`。因此，机密客户端仍然需要其客户端密钥，而不是像 RFC 7591 对 `none` 的要求那样，在不进行客户端认证的情况下被接受。
**Triggers：** 当管理员为机密应用选择 `none`，但没有同时将应用设为公共应用时。
**Suggested fix：** 让令牌认证遵循已配置的方法，包括接受针对 `none` 的仅包含客户端 ID 的请求，或者将 `none` 与应用的公共客户端设置进行同步/校验。
</issue_to_address>
### 评论 2
<location path="src/pages/apps/AppDetail.tsx" line_range="1472-1478" />
<code_context>
+ }
+ >
+ <Option value="">{t("apps.authMethodDefault")}</Option>
+ <Option value="client_secret_basic">
+ client_secret_basic
+ </Option>
+ <Option value="client_secret_post">client_secret_post</Option>
+ <Option value="private_key_jwt">private_key_jwt</Option>
+ <Option value="none">none</Option>
+ </Dropdown>
+ </Field>
+ {form.token_endpoint_auth_method === "private_key_jwt" && (
</code_context>
<issue_to_address>
**issue (bug_risk)：** UI 将 `client_secret_basic`、`client_secret_post` 和 `none` 展示为不同的认证方法，但后端并未强制执行这些区别：对于所有非私钥方法，同一认证流程既接受 Basic 凭据，也接受 `client_id`/`client_secret` 表单参数。因此，存储的方法和发现元数据可能声明一种方法，而令牌、PAR 及相关端点却接受另一种方法。
**Triggers：** 当客户端被配置为使用 `client_secret_basic`、`client_secret_post` 或 `none`，却使用了不同的凭据传输方式时。
**Suggested fix：** 根据存储的方法分支处理认证，并拒绝通过未配置的传输方式提交的凭据。
</issue_to_address>帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/pages/apps/AppDetail.tsx" line_range="1478" />
<code_context>
+ <Option value="client_secret_post">client_secret_post</Option>
+ <Option value="private_key_jwt">private_key_jwt</Option>
+ <Option value="none">none</Option>
+ </Dropdown>
+ </Field>
+ {form.token_endpoint_auth_method === "private_key_jwt" && (
</code_context>
<issue_to_address>
**issue (bug_risk):** Selecting `none` in App Detail stores `token_endpoint_auth_method = "none"`, but the token authentication path still calls `clientSecretValid` for every method except `private_key_jwt`. Confidential clients therefore continue to require their client secret instead of being accepted without client authentication as RFC 7591 `none` requires.
**Triggers:** When an administrator selects `none` for a confidential app without also making the app public.
**Suggested fix:** Make token authentication honor the configured method, including accepting a client-id-only request for `none`, or synchronize/validate `none` with the app's public-client setting.
</issue_to_address>
### Comment 2
<location path="src/pages/apps/AppDetail.tsx" line_range="1472-1478" />
<code_context>
+ }
+ >
+ <Option value="">{t("apps.authMethodDefault")}</Option>
+ <Option value="client_secret_basic">
+ client_secret_basic
+ </Option>
+ <Option value="client_secret_post">client_secret_post</Option>
+ <Option value="private_key_jwt">private_key_jwt</Option>
+ <Option value="none">none</Option>
+ </Dropdown>
+ </Field>
+ {form.token_endpoint_auth_method === "private_key_jwt" && (
</code_context>
<issue_to_address>
**issue (bug_risk):** The UI presents `client_secret_basic`, `client_secret_post`, and `none` as distinct authentication methods, but the backend does not enforce those distinctions: the same authentication flow accepts either Basic credentials or `client_id`/`client_secret` form parameters for every non-private-key method. The stored method and discovery metadata can therefore claim one method while the token, PAR, and related endpoints accept another.
**Triggers:** When a client is configured with `client_secret_basic`, `client_secret_post`, or `none` and uses a different credential transport.
**Suggested fix:** Branch authentication by the stored method and reject credentials presented through an unconfigured transport.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <Option value="client_secret_post">client_secret_post</Option> | ||
| <Option value="private_key_jwt">private_key_jwt</Option> | ||
| <Option value="none">none</Option> | ||
| </Dropdown> |
There was a problem hiding this comment.
issue (bug_risk)： 在 App Detail 中选择 none 会将 token_endpoint_auth_method = "none" 保存下来，但令牌认证路径仍会对除 private_key_jwt 之外的所有方法调用 clientSecretValid。因此，机密客户端仍然需要其客户端密钥，而不是像 RFC 7591 对 none 的要求那样，在不进行客户端认证的情况下被接受。
Triggers： 当管理员为机密应用选择 none，但没有同时将应用设为公共应用时。
Suggested fix： 让令牌认证遵循已配置的方法，包括接受针对 none 的仅包含客户端 ID 的请求，或者将 none 与应用的公共客户端设置进行同步/校验。
Original comment in English
issue (bug_risk): Selecting none in App Detail stores token_endpoint_auth_method = "none", but the token authentication path still calls clientSecretValid for every method except private_key_jwt. Confidential clients therefore continue to require their client secret instead of being accepted without client authentication as RFC 7591 none requires.
Triggers: When an administrator selects none for a confidential app without also making the app public.
Suggested fix: Make token authentication honor the configured method, including accepting a client-id-only request for none, or synchronize/validate none with the app's public-client setting.
Comment on lines
+1472
to
+1478
| <Option value="client_secret_basic"> | ||
| client_secret_basic | ||
| </Option> | ||
| <Option value="client_secret_post">client_secret_post</Option> | ||
| <Option value="private_key_jwt">private_key_jwt</Option> | ||
| <Option value="none">none</Option> | ||
| </Dropdown> |
There was a problem hiding this comment.
issue (bug_risk)： UI 将 client_secret_basic、client_secret_post 和 none 展示为不同的认证方法，但后端并未强制执行这些区别：对于所有非私钥方法，同一认证流程既接受 Basic 凭据，也接受 client_id/client_secret 表单参数。因此，存储的方法和发现元数据可能声明一种方法，而令牌、PAR 及相关端点却接受另一种方法。
Triggers： 当客户端被配置为使用 client_secret_basic、client_secret_post 或 none，却使用了不同的凭据传输方式时。
Suggested fix： 根据存储的方法分支处理认证，并拒绝通过未配置的传输方式提交的凭据。
Original comment in English
issue (bug_risk): The UI presents client_secret_basic, client_secret_post, and none as distinct authentication methods, but the backend does not enforce those distinctions: the same authentication flow accepts either Basic credentials or client_id/client_secret form parameters for every non-private-key method. The stored method and discovery metadata can therefore claim one method while the token, PAR, and related endpoints accept another.
Triggers: When a client is configured with client_secret_basic, client_secret_post, or none and uses a different credential transport.
Suggested fix: Branch authentication by the stored method and reject credentials presented through an unconfigured transport.
|
Preview deployed: https://prism-preview.siiway.workers.dev (shared preview Worker + database, so the newest PR deploy is what is live there). |
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
1 participant
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.
The RFC-conformance work (#26) wired several configurable fields through the backend but left two with no dashboard GUI:
security.txt (RFC 9116) — security_contact / security_policy_url were added to SiteConfig and the admin PATCH allowlist, but the admin settings page had no fields, so /.well-known/security.txt could not be enabled from the UI. Add a "Security disclosure" card to the General tab.
Per-app client auth (RFC 7591/7523) — token_endpoint_auth_method / jwks / jwks_uri (migration 0065) were settable only via Dynamic Client Registration; the apps API PATCH and App Detail did not expose them. Thread them through the apps PATCH handler (with DCR-matching validation) and safeApp, add them to the OAuthApp/CreateAppBody types, and surface a token endpoint auth-method dropdown plus jwks / jwks_uri inputs (shown for private_key_jwt) in App Detail settings.
i18n (en + zh) and docs (admin.md, oauth.md + zh) updated to match. Verified: tsc -b, eslint, and a Vite production build all pass.
Sourcery 总结
启用 security.txt 的仪表板和 API 配置，以及每个应用的 OAuth 客户端身份验证设置。
新功能：
增强功能：
文档：
Original summary in English
Summary by Sourcery
Enable dashboard and API configuration for security.txt and per-application OAuth client authentication settings.
New Features:
Enhancements:
Documentation: