feat(oauth): add rotate_secret action for oauth applications#933
feat(oauth): add rotate_secret action for oauth applications#933skyfallwastaken merged 8 commits intohackclub:mainfrom
Conversation
95decca to
8e6fe24
Compare
Add POST routes and controller actions for both owner-facing and admin OAuth application secret rotation using Doorkeeper's renew_secret..
Add Rotate Secret button with confirmation dialog to both owner and admin show pages. Display rotated secret via flash with copy button..
8e6fe24 to
f18b05e
Compare
Pull request overview
Adds secret-rotation capability for OAuth applications so owners (on their own apps) and superadmins (on any app) can invalidate the current client secret and obtain a new one once via flash.
Changes:
- Added owner and admin POST routes for rotating an OAuth application’s client secret.
- Implemented
rotate_secretactions in the owner and admin controllers using Doorkeeper’srenew_secret. - Updated owner/admin show pages to include a “Rotate Secret” button; admin show additionally displays the rotated secret (from flash) with copy-to-clipboard UI.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| config/routes.rb | Adds rotate-secret endpoints for owner and admin contexts. |
| app/controllers/doorkeeper/applications_controller.rb | Adds owner rotate_secret action and includes it in set_application callback. |
| app/controllers/admin/oauth_applications_controller.rb | Adds admin rotate_secret action and includes it in set_application callback. |
| app/views/doorkeeper/applications/show.html.erb | Adds “Rotate Secret” button on owner app show page. |
| app/views/admin/oauth_applications/show.html.erb | Adds rotated-secret flash display, copy button, and “Rotate Secret” action button on admin show page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add explicit superadmin authorization check in rotate_secret action. The route constraint already limits access, but this adds defense in depth at the controller level to prevent privilege escalation.n.
- Remove duplicate stale lines in admin controller - Fix indentation in admin controller, both show views - Add superadmin guard to admin rotate_secret action - Use I18n for flash messages in doorkeeper controller - Add respond_to HTML/JSON branches matching existing patterns - Fix double space in before_action arrayarray
1f4a6a5 to
de86a50
Compare
b007205 to
8956906
Compare
| def rotate_secret | ||
| @application.renew_secret | ||
| if @application.save | ||
| flash[:notice] = I18n.t(:notice, scope: %i[doorkeeper flash applications rotate_secret]) | ||
| else | ||
| flash[:alert] = I18n.t(:alert, scope: %i[doorkeeper flash applications rotate_secret]) | ||
| end | ||
| redirect_to admin_oauth_application_path(@application) | ||
| end |
Bug: The rotate_secret action in Admin::OauthApplicationsController doesn't set flash[:application_secret], so the new secret is never displayed to the admin after rotation.
Severity: CRITICAL
Suggested Fix
In Admin::OauthApplicationsController#rotate_secret, after @application.save succeeds, add the line flash[:application_secret] = @application.plaintext_secret. This will store the newly generated secret in the flash session, making it available for display on the subsequent page load.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/controllers/admin/oauth_applications_controller.rb#L29-L37
Potential issue: The `Admin::OauthApplicationsController#rotate_secret` action calls
`@application.renew_secret` to generate a new secret but fails to store it in the flash
scope before redirecting. The corresponding view,
`admin/oauth_applications/show.html.erb`, relies on `flash[:application_secret]` to
display the new secret to the admin. Because the plaintext secret is only available
during the request in which it's generated, it becomes irretrievable after the redirect.
This means when an admin rotates a secret, the rotation succeeds but the new secret is
lost, preventing the admin from providing it to the application owner and breaking any
integrations using the old secret.
he old secret.
Adds the ability to rotate client secrets for existing OAuth applications. Available to app owners on their own apps and to superadmins on any app.
POST /oauth/applications/:id/rotate_secret(owner) andPOST /admin/oauth_applications/:id/rotate_secret(superadmin)rotate_secretaction to bothDoorkeeper::ApplicationsControllerandAdmin::OauthApplicationsControllerusing Doorkeeper's built-inrenew_secret