feat(ui): add secret regeneration page and client credentials flow for user apps#121
Conversation
- Add user_app_secret.templ with proper title, security warning, and copy-to-clipboard support - Replace reuse of UserAppCreated template with dedicated UserAppSecret template in handler Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds a dedicated UI page for user-owned app client-secret regeneration so the post-regeneration experience matches the admin flow and avoids the misleading “App Registered”/pending-approval messaging.
Changes:
- Introduces
UserAppSecrettempl page to display the regenerated secret with security warning and copy-to-clipboard affordance. - Updates
RegenerateAppSecrethandler to render the new template instead of reusingUserAppCreated.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/templates/user_app_secret.templ | New dedicated secret-regeneration template for user apps (mirrors admin secret page UX). |
| internal/handlers/user_client.go | Switches regenerate-secret handler to render UserAppSecret. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| templates.RenderTempl( | ||
| c, | ||
| http.StatusOK, | ||
| templates.UserAppCreated(templates.UserClientCreatedPageProps{ | ||
| templates.UserAppSecret(templates.UserClientCreatedPageProps{ | ||
| BaseProps: templates.BaseProps{CSRFToken: middleware.GetCSRFToken(c)}, |
There was a problem hiding this comment.
GetClient is called after regenerating the secret and its error is ignored; if it fails (and returns nil), clientToDisplay will produce nil and the template render will likely panic when dereferencing props.Client. Handle the error (or fall back to the already-loaded client) before rendering the response.
| @@ -0,0 +1,62 @@ | |||
| package templates | |||
|
|
|||
| templ UserAppSecret(props UserClientCreatedPageProps) { | |||
There was a problem hiding this comment.
The new UserAppSecret page uses UserClientCreatedPageProps, which is now misleading since it’s not only used for the post-create flow. Consider introducing a dedicated props type (e.g., UserClientSecretPageProps) or renaming the existing type to reflect both create and regenerate use cases to keep template APIs self-describing.
| templ UserAppSecret(props UserClientCreatedPageProps) { | |
| type UserClientSecretPageProps = UserClientCreatedPageProps | |
| templ UserAppSecret(props UserClientSecretPageProps) { |
- Handle GetClient error after regeneration with fallback to pre-loaded client - Add UserClientSecretPageProps type alias for self-describing template API Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Show Client Credentials Flow checkbox in user app form for confidential clients - Parse and pass EnableClientCredentialsFlow in CreateApp and UpdateApp handlers - Add EnableClientCredentialsFlow to UserUpdateClientRequest - Enforce confidential-only constraint in UserUpdateClient service method Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| enableClientCredentials := req.EnableClientCredentialsFlow && | ||
| client.ClientType == ClientTypeConfidential | ||
| client.EnableClientCredentialsFlow = enableClientCredentials | ||
| client.GrantTypes = buildGrantTypes( | ||
| req.EnableDeviceFlow, | ||
| req.EnableAuthCodeFlow, | ||
| enableClientCredentials, | ||
| ) |
There was a problem hiding this comment.
UserUpdateClient now supports EnableClientCredentialsFlow, but the function’s “at least one grant type” validation earlier still only considers device/auth-code. This makes it impossible to save/update a client configured with only Client Credentials (e.g., CC-only client created successfully will fail future updates). Update the validation to include req.EnableClientCredentialsFlow (and keep the confidential-only restriction) so it matches the grant-types rebuilding logic.
| ShowClientCredentials: true, | ||
| ScopePresetsOnly: true, |
There was a problem hiding this comment.
This changes the user app form to expose Client Credentials Flow controls (ShowClientCredentials: true). That’s a functional behavior change (users can request/enable CC flow) and isn’t mentioned in the PR description/title, which frames this as only a dedicated secret-regeneration page. Either update the PR description to explicitly include this feature (and rationale/implications) or split it into a separate PR.
- Add EnableClientCredentialsFlow to the "at least one grant type" check in UserUpdateClient so CC-only confidential clients pass validation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !req.EnableDeviceFlow && !req.EnableAuthCodeFlow && !req.EnableClientCredentialsFlow { | ||
| return ErrAtLeastOneGrantRequired | ||
| } |
There was a problem hiding this comment.
UserUpdateClient validates ErrAtLeastOneGrantRequired using the raw request flags, but later forces Client Credentials off for public clients. A crafted request like client_type=public + enable_client_credentials_flow=true (and other flows false) will pass validation but end up with no enabled grants/empty GrantTypes. Consider either (a) validating client-type/grant compatibility up front and returning an error when CC is requested for public clients, or (b) re-checking the final computed enable flags (EnableDeviceFlow || EnableAuthCodeFlow || enableClientCredentials) before saving.
- Add ErrClientCredentialsRequireConfidential validation in UserUpdateClient before computing grant types, preventing public+CC-only from saving with empty grants Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Normalize client type early in UserUpdateClient to avoid inconsistency between validation and persistence - Add 3 tests: CC flow for confidential, CC rejected for public, CC-only confidential client Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… admin UpdateClient - Add upfront ErrClientCredentialsRequireConfidential validation in CreateClient and admin UpdateClient, consistent with UserUpdateClient - Normalize client type early in all three paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Extract shared secret-regeneration card into SecretRegeneratedCard in client_shared.templ, parameterized by breadcrumb, labels, and paths - Refactor AdminClientSecret and UserAppSecret to use the shared component - Eliminates duplication and prevents long-term drift Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… tests - Update ShowClientCredentials comment from "(admin only)" to "(confidential clients)" to reflect current behavior - Add CreateClient tests for CC flow: confidential success and public rejection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| A new client secret has been successfully generated for "<strong>{ props.ClientName }</strong>" | ||
| </div> | ||
| <div class="warning-box warning-box-enhanced"> | ||
| <div class="warning-icon">⚠</div> |
There was a problem hiding this comment.
The warning icon is purely decorative but will likely be announced by screen readers as a “warning sign” character. Consider adding aria-hidden="true" to the .warning-icon element (or alternatively role="img" + an explicit aria-label) so the accessible name comes from the nearby “Important Security Notice” text instead of the glyph.
| <div class="warning-icon">⚠</div> | |
| <div class="warning-icon" aria-hidden="true">⚠</div> |
| IsEdit bool | ||
| NameLabel string // Display label: "App Name" (user) or "Client Name" (admin) | ||
| ShowClientCredentials bool // Show Client Credentials Flow checkbox (admin only) | ||
| ShowClientCredentials bool // Show Client Credentials Flow checkbox (confidential clients) |
There was a problem hiding this comment.
The field comment for ShowClientCredentials is a bit misleading: the checkbox is rendered based on this flag, but the confidential-vs-public restriction is enforced separately in the template JS (it’s still shown, just disabled for public). Suggest updating the comment to reflect that this flag controls rendering, not the client-type constraint itself.
| ShowClientCredentials bool // Show Client Credentials Flow checkbox (confidential clients) | |
| ShowClientCredentials bool // Control rendering of the Client Credentials Flow checkbox; client-type (confidential vs public) restrictions are enforced in the template JS (e.g. disabling for public clients) |
…tials comment - Add aria-hidden="true" to decorative warning icon in SecretRegeneratedCard - Clarify ShowClientCredentials comment to reflect rendering vs JS enforcement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Introduce core.ClientType with IsValid, OrDefault, and NormalizeClientType - Replace raw string constants with typed ClientType across services and handlers - Simplify normalize blocks to one-liner OrDefault calls - Remove redundant enableClientCredentials guard after early-return validation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Test plan
Generated with Claude Code