regression: Update express OAuth route handlers on re-register

Merged
YA
3mo ago
+25-0

Proposed changes (including videos or screenshots)

OAuth services are re-registered at runtime when their settings change. The passport strategy is replaced correctly, but the express routes are only appended, the old handlers are also part of execution stack.

passportOAuthCallback isn't a request handler, but is a factory that gives out request handler function based on settings configs. Due to this the request handler always uses old handler with old settings within its closure.

The fix implemented in this PR removes old request handlers from express router stack before pushing new handlers. In future we can come up with a better fix to not sure factory and closures, where express route handlers can use new settings directly.

Issue(s)

Steps to test or reproduce

  • Go to Workspace → Settings → OAuth
  • Enable Use Modern OAuth Flow.
  • Set Login Style to Redirect and save the changes
  • Wait for the settings to be applied or select Refresh OAuth Services
  • Change Login Style to Popup and save the changes
  • Do not restart the Rocket.Chat server
  • Open the workspace URL in a fresh incognito window
  • Select the Custom OAuth provider
  • Complete authentication successfully

Further comments

CORE-2489

Summary by CodeRabbit

  • Bug Fixes
    • Improved OAuth provider reconfiguration by removing existing OAuth routes before registering updated Passport strategies.
    • Prevented duplicate or conflicting OAuth endpoints during OAuth service initialization.
    • Applied the same route cleanup behavior to Apple OAuth and custom OAuth providers.
  • New Features
    • Added a shared utility to remove OAuth routes for a specified provider.