FIX: Delete destination_url cookie when it's used to set origin param during redirect to social auth (#36194)

nullchristo · web-flow · commit aa157f5ea541 · 2025-11-25T09:15:03.000-06:00
When we navigate out to a social auth method, we set the origin param on that request to the value of the destination_url. Upon returning to Discourse, that origin param is returned and [we use it to set](https://github.com/discourse/discourse/blob/4dd3becec10c5b245ca122e684244f604b5c550a/config/initializers/009-omniauth.rb#L24) server_session["destination_url"]. Then during the [omniauth callback](https://github.com/discourse/discourse/blob/4dd3becec10c5b245ca122e684244f604b5c550a/app/controllers/users/omniauth_callbacks_controller.rb#L43), we use the server_session["destination_url"] value to redirect back to the page the destination_url cookie originally pointed to. The problem is that in this scenario, the destination_url cookie never gets deleted. This was causing a bug with Discourse ID in a corner case where [the discourse-login plugin sets the destination_url cookie](https://github.com/discourse-org/discourse-login/blob/646e32f4a66acab3692360188364815e5ff08fcf/config/initializers/doorkeeper.rb#L38) to a url that contains an OAuth state parameter which can become stale, and the cookie keeps getting re-used for redirection but not deleted, resulting in an error message "Sorry, the authorization timed out, or you have switched browsers. Please try again." which the user can't escape. The solution is to make sure that when we return to the omniauth callback we always clean up both the server_session[:destination_url] and cookies[:destination_url]
diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -42,14 +42,15 @@ def complete
     session.delete(:destination_url) # Clean up old values. TODO: Remove after March 2026
     if server_session[:destination_url].present?
       preferred_origin = server_session[:destination_url]
-      server_session.delete(:destination_url)
     elsif SiteSetting.enable_discourse_connect_provider && payload = cookies.delete(:sso_payload)
       preferred_origin = session_sso_provider_url + "?" + payload
     elsif cookies[:destination_url].present?
       preferred_origin = cookies[:destination_url]
-      cookies.delete(:destination_url)
     end
 
+    server_session.delete(:destination_url)
+    cookies.delete(:destination_url)
+
     if preferred_origin.present?
       parsed =
         begin
diff --git a/spec/system/social_authentication_spec.rb b/spec/system/social_authentication_spec.rb
@@ -438,6 +438,36 @@
         expect(page).to have_css(".header-dropdown-toggle.current-user")
       end
 
+      it "removes destination_url cookie if present after setting up redirect" do
+        mock_facebook_auth
+        visit("/")
+
+        category = Fabricate(:category)
+
+        signup_page.open
+
+        # Manually set the destination_url, as if it were set by a plugin
+        page.driver.with_playwright_page do |pw_page|
+          pw_page.context.add_cookies(
+            [{ url: pw_page.url, name: :destination_url, value: category.url }],
+          )
+
+          cookie_found =
+            pw_page.context.cookies.any? { |cookie| cookie["name"] == "destination_url" }
+          expect(cookie_found).not_to be_falsey
+        end
+
+        signup_page.click_social_button("facebook")
+        expect(page).to have_current_path(category.url)
+
+        # Ensure the destination_url cookie was removed after being used
+        page.driver.with_playwright_page do |pw_page|
+          cookie_found =
+            pw_page.context.cookies.any? { |cookie| cookie["name"] == "destination_url" }
+          expect(cookie_found).to be_falsey
+        end
+      end
+
       context "with a suspended user" do
         before do
           user.suspended_till = 2.years.from_now
