-
Notifications
You must be signed in to change notification settings - Fork 0
Add application management UI, implement OIDC Discovery endpoints, and OAuth flow pages #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
…pport Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
…information Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
|
@copilot 继续补充和完善剩余的功能,但需要说明的是:
|
|
@niltor Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
|
@copilot
|
…ate client integration guide Co-authored-by: niltor <8222201+niltor@users.noreply.github.com>
已完成!实现了以下内容(提交 16f9466): 1. OAuth流程Razor Pages ✅创建了3个Razor Pages用于OAuth流程(使用表单POST提交,非SPA):
所有页面都使用现代化UI设计,支持移动端,完整中文本地化。 2. 客户端对接指南 ✅创建了完整的对接文档 后端应用对接:
前端应用对接:
管理后台操作:
流程示例用户登录授权流程: 文档中包含前后端完整代码示例和管理后台配置步骤。 |
已完成所有要求(提交 16f9466): ✅ Razor Pages OAuth流程 - Login/Consent/Logout页面使用表单POST提交 所有OAuth页面已使用Razor Pages实现(非SPA),符合规范的表单提交方式。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds OAuth UI pages with Razor Pages, implements OIDC Discovery endpoints, improves documentation structure, and adds menu items for application and resource management.
Key Changes
- Added Razor Pages for OAuth flows (Login, Logout, Consent) with session-based authentication
- Implemented OIDC Discovery endpoints (/.well-known/openid-configuration, /.well-known/jwks, /connect/userinfo)
- Created DiscoveryManager and related DTOs for OIDC standard compliance
- Updated frontend menus and translations for application/resource management
- Cleaned up temporary documentation files and restructured README
- Downgraded .NET version from 10 to 9 temporarily
Reviewed Changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Services/ApiService/Program.cs | Added session support and Razor Pages configuration for OAuth UI |
| src/Services/ApiService/Pages/Account/*.cshtml.cs | Implemented Login, Logout, and Consent page models with session management |
| src/Services/ApiService/Controllers/DiscoveryController.cs | Added OIDC Discovery endpoints with comprehensive documentation |
| src/Modules/IdentityMod/Managers/DiscoveryManager.cs | Implemented business logic for OIDC configuration and JWKS generation |
| src/Services/ApiService/Controllers/OAuthController.cs | Updated to redirect to new Razor Pages and check consent |
| src/ClientApp/WebApp/src/assets/menus.json | Added application and resource management menu items |
| src/ClientApp/WebApp/src/assets/i18n/*.json | Added translations for client and resource management |
| global.json | Temporarily downgraded .NET version to 9.0.306 |
| README.md | Completely restructured with professional project overview |
| docs/*.md | Cleaned up temporary docs, added DEVELOPMENT-GUIDE.md and CLIENT-INTEGRATION-GUIDE.md |
Comments suppressed due to low confidence (2)
src/Modules/IdentityMod/Managers/DiscoveryManager.cs:1
- Magic number 256 should be extracted as a named constant. Define
private const int MinimumRsaKeySizeBytes = 256;with a comment explaining this represents 2048 bits minimum RSA key size for security compliance.
using Entity.CommonMod;
src/Services/ApiService/Controllers/OAuthController.cs:1
- The consent_granted flag can be easily bypassed by adding it to the URL. This should be validated using a signed token or server-side state (e.g., session or database) to prevent users from bypassing the consent screen by manually adding
consent_granted=trueto the authorization URL.
using IdentityMod;
| HttpContext.Session.SetString("UserId", user.Id.ToString()); | ||
| HttpContext.Session.SetString("UserName", user.UserName); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Session-based authentication is insecure for OAuth flows. Sessions can be hijacked, and this doesn't issue proper OAuth tokens (access_token, id_token). The login page should redirect back to /connect/authorize which will handle proper token issuance through the Authorization Code flow. Consider using ASP.NET Core Cookie Authentication with SignInAsync instead of raw session storage.
| var authorizeUrl = $"/connect/authorize?client_id={ClientId}&scope={Scope}&response_type={ResponseType}&redirect_uri={RedirectUri}"; | ||
|
|
||
| if (!string.IsNullOrEmpty(State)) | ||
| authorizeUrl += $"&state={State}"; |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL parameters are not being properly URL-encoded. Use Uri.EscapeDataString() or UriBuilder to prevent injection attacks. For example, redirect_uri and other parameters should be encoded: $\"&redirect_uri={Uri.EscapeDataString(RedirectUri)}\"
| options.IdleTimeout = TimeSpan.FromMinutes(30); | ||
| options.Cookie.HttpOnly = true; | ||
| options.Cookie.IsEssential = true; | ||
| options.Cookie.SameSite = SameSiteMode.Lax; |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SameSite=Lax may allow CSRF attacks in OAuth flows. For OAuth authorization endpoints, SameSite should be None with Secure flag, or use anti-forgery tokens. Consider using SameSiteMode.Strict or implementing proper CSRF protection with state parameter validation.
| if (!string.IsNullOrEmpty(ReturnUrl) && Url.IsLocalUrl(ReturnUrl)) | ||
| { | ||
| return Redirect(ReturnUrl); | ||
| } |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open redirect vulnerability. While Url.IsLocalUrl() prevents external redirects, the ReturnUrl from query parameters can still be manipulated. The code should validate that ReturnUrl contains expected OAuth parameters (client_id, response_type, etc.) or whitelist allowed redirect patterns to prevent redirect-based attacks.
| // TODO: Check if consent is required | ||
| // For now, auto-consent for demonstration | ||
| // Check if consent is required and not yet granted | ||
| var consentGranted = Request.Query.ContainsKey("consent_granted") && Request.Query["consent_granted"] == "true"; |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient use of 'ContainsKey' and indexer.
| var consentGranted = Request.Query.ContainsKey("consent_granted") && Request.Query["consent_granted"] == "true"; | |
| var consentGranted = Request.Query.TryGetValue("consent_granted", out var consentValue) && consentValue == "true"; |
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Login failed for user {Username}", Username); | ||
| ErrorMessage = "登录过程中发生错误,请稍后重试"; | ||
| return Page(); | ||
| } |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to load client {ClientId}", ClientId); | ||
| ClientName = ClientId; | ||
| } |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to load scope {Scope}", scopeName); | ||
| RequestedScopes.Add(new ScopeViewModel | ||
| { | ||
| Name = scopeName, | ||
| DisplayName = scopeName, | ||
| Description = GetDefaultScopeDescription(scopeName), | ||
| Required = IsDefaultRequiredScope(scopeName) | ||
| }); | ||
| } |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to process consent for client {ClientId}", ClientId); | ||
| return Page(); | ||
| } |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Error during logout"); | ||
| return Page(); | ||
| } |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
Adds application/client management to the frontend menu, implements missing OIDC standard endpoints required for client auto-discovery and JWT verification, and provides complete OAuth flow UI with Razor Pages for third-party client integration.
Frontend Changes
Backend: OIDC Discovery Implementation
Implements three standard OIDC endpoints:
New components:
DiscoveryManager: Generates OIDC config, converts DB signing keys to JWKS format, returns scope-filtered user claimsDiscoveryController: Exposes discovery endpoints with Host header injection protectionOidcConfigurationDto,JwksDto,UserInfoDto(OIDC spec compliant)Security improvements:
OAuth Flow UI (Razor Pages)
Implements complete OAuth authorization flow using traditional form-based submission (Razor Pages, not SPA) for third-party client integration:
New pages:
/Account/Login- User authentication page with modern UI, form validation, and "Remember Me" functionality/Account/Consent- Authorization consent page showing app details, requested scopes, and allow/deny actions/Account/Logout- Logout confirmation page with post-logout redirect supportFeatures:
Client Integration Guide
NEW:
docs/CLIENT-INTEGRATION-GUIDE.md- Comprehensive integration documentation (~400 lines):Backend Application Integration:
Frontend Application Integration:
Management Portal Guide:
Troubleshooting:
Documentation
Removed 14 temporary task summary documents.
Impact
OAuth Authorization Flow
Notes
Temporarily adjusted
global.jsonfrom .NET 10 to .NET 9 due to SDK availability in build environment.Original prompt
Note
Custom agent used: ater agent
如何更好的理解项目,并获得相关参考内容
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.