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
discourse · Apr 2, 2019 · 1c0f885 · 1c0f885
1 parent 254de64
commit 1c0f885
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 20 deletions.
diff --git 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
@@ -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"