From b0847509536ff3b613525ad9236b26393b075e33 Mon Sep 17 00:00:00 2001
From: Leo McArdle <leo@mozilla.org.uk>
Date: Tue, 19 Mar 2019 12:39:13 +0000
Subject: [PATCH] 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
---
 .../users/omniauth_callbacks_controller.rb    |  9 +--
 lib/auth/default_current_user_provider.rb     |  1 +
 .../omniauth_callbacks_controller_spec.rb     | 55 +++++++++++++++++++
 spec/support/helpers.rb                       |  5 ++
 4 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 60bb2294ef3..7c4b9694a18 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -57,17 +57,18 @@ class Users::OmniauthCallbacksController < ApplicationController
       rescue URI::Error
       end
 
-      if parsed
-        @origin = "#{parsed.path}?#{parsed.query}"
+      if parsed && (parsed.host == nil || parsed.host == Discourse.current_hostname)
+        @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')
diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb
index 18e23848752..2efcde6bae5 100644
--- a/lib/auth/default_current_user_provider.rb
+++ b/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
 
diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb
index 8296ac3362e..9a48f5265f9 100644
--- a/spec/requests/omniauth_callbacks_controller_spec.rb
+++ b/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
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index 455230e1b48..11db6097697 100644
--- a/spec/support/helpers.rb
+++ b/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)