FIX: don't redirect incorrectly after full screen login (#7170)

Fixes two issues:
1. Redirecting to an external origin's path after login did not work
2. User would be erroneously redirected to the external origin after logout

https://meta.discourse.org/t/109755

app/controllers/users/omniauth_callbacks_controller.rb

@@ -57,17 +57,18 @@ class Users::OmniauthCallbacksController < ApplicationController
       rescue URI::Error
       end
 
-      if parsed
+      if parsed && (parsed.host == nil || parsed.host == Discourse.current_hostname)
-        @origin = "#{parsed.path}?#{parsed.query}"
+        @origin = "#{parsed.path}"
+        @origin << "?#{parsed.query}" if parsed.query
       end
     end
 
     if @origin.blank?
       @origin = Discourse.base_uri("/")
-    else
-      @auth_result.destination_url = origin
     end
 
+    @auth_result.destination_url = origin
+
     if @auth_result.failed?
       flash[:error] = @auth_result.failed_reason.html_safe
       return render('failure')

lib/auth/default_current_user_provider.rb

@@ -226,6 +226,7 @@ class Auth::DefaultCurrentUserProvider
       @user_token.destroy
     end
 
+    cookies.delete('authentication_data')
     cookies.delete(TOKEN_COOKIE)
   end
 

spec/requests/omniauth_callbacks_controller_spec.rb

@@ -338,6 +338,61 @@ RSpec.describe Users::OmniauthCallbacksController do
           expect(response_body["awaiting_activation"]).to eq(true)
         end
       end
+
+      context 'with full screen login' do
+        before do
+          cookies['fsl'] = true
+        end
+
+        it "doesn't attempt redirect to external origin" do
+          get "/auth/google_oauth2?origin=https://example.com/external"
+          get "/auth/google_oauth2/callback"
+
+          expect(response.status).to eq 302
+          expect(response.location).to eq "http://test.localhost/"
+        end
+
+        it "redirects to internal origin" do
+          get "/auth/google_oauth2?origin=http://test.localhost/t/123"
+          get "/auth/google_oauth2/callback"
+
+          expect(response.status).to eq 302
+          expect(response.location).to eq "http://test.localhost/t/123"
+        end
+
+        it "redirects to relative origin" do
+          get "/auth/google_oauth2?origin=/t/123"
+          get "/auth/google_oauth2/callback"
+
+          expect(response.status).to eq 302
+          expect(response.location).to eq "http://test.localhost/t/123"
+        end
+
+        it "redirects with query" do
+          get "/auth/google_oauth2?origin=/t/123?foo=bar"
+          get "/auth/google_oauth2/callback"
+
+          expect(response.status).to eq 302
+          expect(response.location).to eq "http://test.localhost/t/123?foo=bar"
+        end
+
+        it "removes authentication_data cookie on logout" do
+          get "/auth/google_oauth2?origin=https://example.com/external"
+          get "/auth/google_oauth2/callback"
+
+          provider = log_in_user(Fabricate(:user))
+
+          expect(cookies['authentication_data']).to be
+
+          log_out_user(provider)
+
+          expect(cookies['authentication_data']).to be_nil
+        end
+
+        after do
+          cookies.delete('fsl')
+        end
+      end
     end
 
     context 'when attempting reconnect' do

spec/support/helpers.rb

@@ -14,6 +14,11 @@ module Helpers
   def log_in_user(user)
     provider = Discourse.current_user_provider.new(request.env)
     provider.log_on_user(user, session, cookies)
+    provider
+  end
+
+  def log_out_user(provider)
+    provider.log_off_user(session, cookies)
   end
 
   def fixture_file(filename)