From 965ac3567b4c4b110d3d3909de4d5f43804a0300 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Wed, 12 Feb 2020 16:03:25 -0700 Subject: [PATCH] FIX: Handle SSO Provider Parse exception Prevent unnecessary 500 errors from appearing in the logs and return a 422 response instead. --- app/controllers/session_controller.rb | 8 ++++++++ spec/requests/session_controller_spec.rb | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index e9cfa31aa5b..c8696e05816 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -47,6 +47,14 @@ class SessionController < ApplicationController rescue SingleSignOnProvider::BlankSecret render plain: I18n.t("sso.missing_secret"), status: 400 return + rescue SingleSignOnProvider::ParseError => e + if SiteSetting.verbose_sso_logging + Rails.logger.warn("Verbose SSO log: Signature parse error\n\n#{e.message}\n\n#{sso&.diagnostics}") + end + + # Do NOT pass the error text to the client, it would give them the correct signature + render plain: I18n.t("sso.login_error"), status: 422 + return end if sso.return_sso_url.blank? diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index c70448b4e7a..3ddbb02a389 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -1006,7 +1006,7 @@ RSpec.describe SessionController do it "it fails to log in if secret is wrong" do get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload("secretForRandomSite")) - expect(response.status).to eq(500) + expect(response.status).to eq(422) end it "fails with a nice error message if secret is blank" do