Skip to content

feat(oauth): add rotate_secret action for oauth applications#933

Merged
skyfallwastaken merged 8 commits intohackclub:mainfrom
System-End:oauth-creds-rotate
Feb 15, 2026
Lines changed: 181 additions & 97 deletionsions
Merged

feat(oauth): add rotate_secret action for oauth applications#933
skyfallwastaken merged 8 commits intohackclub:mainfrom
System-End:oauth-creds-rotate

Conversation

@System-End
Copy link
Contributor

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.

  • Routes: Added POST /oauth/applications/:id/rotate_secret (owner) and POST /admin/oauth_applications/:id/rotate_secret (superadmin)
  • Controllers: Added rotate_secret action to both Doorkeeper::ApplicationsController and Admin::OauthApplicationsController using Doorkeeper's built-in renew_secret
  • Views: Added "Rotate Secret" button with confirmation dialog to both owner and admin show pages. New secret is flashed once with a copy-to-clipboard button.

Copilot AI review requested due to automatic review settings February 12, 2026 17:13

Verified

This commit was signed with the committer’s verified signature.
Add POST routes and controller actions for both owner-facing and admin
OAuth application secret rotation using Doorkeeper's renew_secret..

Verified

This commit was signed with the committer’s verified signature.
Add Rotate Secret button with confirmation dialog to both owner and
admin show pages. Display rotated secret via flash with copy button..
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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_secret actions in the owner and admin controllers using Doorkeeper’s renew_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.

Verified

This commit was signed with the committer’s verified signature.
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.

Verified

This commit was signed with the committer’s verified signature.
- 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
@System-End System-End force-pushed the oauth-creds-rotate branch 2 times, most recently from 1f4a6a5 to de86a50 Compare February 15, 2026 02:43

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Member

@skyfallwastaken skyfallwastaken left a comment

Choose a reason for hiding this comment

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

Thanks!

@skyfallwastaken skyfallwastaken enabled auto-merge (squash) February 15, 2026 13:28
@skyfallwastaken skyfallwastaken merged commit mit 5f5eb93 into hackclub:main Feb 15, 2026
7 checks passed
Comment on lines +29 to +37
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
Copy link

Choose a reason for hiding this comment

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

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.

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.

None yet

2 participants