From 1c0f885a5bc96b0fa00374870c31cca8ff185839 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Mon, 1 Apr 2019 22:13:53 -0400
Subject: [PATCH] FIX: double render error with delegated authentication

Makes sure delegated authentication is checked before other login redirects

Updates specs to cover login_required = true cases
---
 app/controllers/application_controller.rb    | 43 +++++++++++---------
 spec/requests/application_controller_spec.rb |  1 +
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 3f257137647c13..9c0beedf85a57a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -703,6 +703,29 @@ def destination_url
   def redirect_to_login_if_required
     return if request.format.json? && is_api?
 
+    # Used by clients authenticated via user API.
+    # Redirects to provided URL scheme if
+    # - request uses a valid public key and auth_redirect scheme
+    # - one_time_password scope is allowed
+    if !current_user &&
+      params.has_key?(:user_api_public_key) &&
+      params.has_key?(:auth_redirect)
+      begin
+        OpenSSL::PKey::RSA.new(params[:user_api_public_key])
+      rescue OpenSSL::PKey::RSAError
+        return render plain: I18n.t("user_api_key.invalid_public_key")
+      end
+
+      if UserApiKey.invalid_auth_redirect?(params[:auth_redirect])
+        return render plain: I18n.t("user_api_key.invalid_auth_redirect")
+      end
+
+      if UserApiKey.allowed_scopes.superset?(Set.new(["one_time_password"]))
+        redirect_to("#{params[:auth_redirect]}?otp=true")
+        return
+      end
+    end
+
     if !current_user && SiteSetting.login_required?
       flash.keep
       dont_cache_page
@@ -731,26 +754,6 @@ def redirect_to_login_if_required
         redirect_to path(redirect_path)
       end
     end
-
-    # Used by clients authenticated via user API.
-    # Redirects to provided URL scheme if
-    # - request uses a valid public key and auth_redirect scheme
-    # - one_time_password scope is allowed
-    if !current_user &&
-      params.has_key?(:user_api_public_key) &&
-      params.has_key?(:auth_redirect)
-      begin
-        OpenSSL::PKey::RSA.new(params[:user_api_public_key])
-      rescue OpenSSL::PKey::RSAError
-        return render plain: I18n.t("user_api_key.invalid_public_key")
-      end
-
-      if UserApiKey.invalid_auth_redirect?(params[:auth_redirect])
-        return render plain: I18n.t("user_api_key.invalid_auth_redirect")
-      end
-      redirect_to("#{params[:auth_redirect]}?otp=true") if UserApiKey.allowed_scopes.superset?(Set.new(["one_time_password"]))
-    end
-
   end
 
   def block_if_readonly_mode
diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb
index 83327e81cae346..2381b8432f7739 100644
--- a/spec/requests/application_controller_spec.rb
+++ b/spec/requests/application_controller_spec.rb
@@ -390,6 +390,7 @@
     end
 
     it 'redirects correctly with valid params' do
+      SiteSetting.login_required = true
       args[:user_api_public_key] = public_key
       args[:auth_redirect] = "discourse://auth_redirect"