mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 03:07:17 -05:00
FIX: Delete destination_url cookie when it's used to set origin param during redirect to social auth (#36194)
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]
This commit is contained in:
@@ -42,14 +42,15 @@ class Users::OmniauthCallbacksController < ApplicationController
|
||||
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
|
||||
|
||||
@@ -438,6 +438,36 @@ shared_examples "social authentication scenarios" do
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user