From e6b3310577582fc702913ac084d41bdf7006439d Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 9 Nov 2018 14:27:36 +1100 Subject: [PATCH] FIX: never redirect back to `/sso` it will cause a loop If for any reason our return url is set to `/sso` bypass using it for login redirect --- app/controllers/session_controller.rb | 5 +++++ spec/requests/session_controller_spec.rb | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 557ac26d857..175844a8204 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -168,6 +168,11 @@ class SessionController < ApplicationController end end + # never redirects back to sso in an sso loop + if return_path.start_with?(path("/sso")) + return_path = path("/") + end + redirect_to return_path else render_sso_error(text: I18n.t("sso.not_found"), status: 500) diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index fe90aea97e0..96427a9d76f 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -303,6 +303,16 @@ RSpec.describe SessionController do end + it 'will never redirect back to /sso path' do + sso = get_sso("/sso?bla=1") + sso.email = user.email + sso.external_id = 'abc' + sso.username = 'sam' + + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + expect(response).to redirect_to('/') + end + it 'can take over an account' do sso = get_sso("/") user = Fabricate(:user)