SECURITY: properly validate return URL for SSO

Previously carefully crafted URLs could redirect off site

app/controllers/session_controller.rb

@@ -174,7 +174,7 @@ class SessionController < ApplicationController
           begin
             uri = URI(return_path)
             if (uri.hostname == Discourse.current_hostname)
-              return_path = uri.request_uri
+              return_path = uri.to_s
             elsif !SiteSetting.sso_allows_all_return_paths
               return_path = path("/")
             end
@@ -183,8 +183,10 @@ class SessionController < ApplicationController
           end
         end
 
-        # never redirects back to sso in an sso loop
+        # this can be done more surgically with a regex
-        if return_path.start_with?(path("/session/sso"))
+        # but it the edge case of never supporting redirects back to
+        # any url with `/session/sso` in it anywhere is reasonable
+        if return_path.include?(path("/session/sso"))
           return_path = path("/")
         end
 

spec/requests/session_controller_spec.rb

@@ -403,6 +403,17 @@ RSpec.describe SessionController do
       expect(logged_on_user.admin).to eq(true)
     end
 
+    it 'does not redirect offsite' do
+      sso = get_sso("#{Discourse.base_url}//site.com/xyz")
+      sso.external_id = '666'
+      sso.email = 'bob@bob.com'
+      sso.name = 'Sam Saffron'
+      sso.username = 'sam'
+
+      get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers
+      expect(response).to redirect_to("#{Discourse.base_url}//site.com/xyz")
+    end
+
     it 'redirects to a non-relative url' do
       sso = get_sso("#{Discourse.base_url}/b/")
       sso.external_id = '666' # the number of the beast