DEV: reduce logging when no external id is specified
Previously we were returning an unknown sso error and logging a message when external id was blank. This noise is not needed.
This commit is contained in:
parent
236833ed5d
commit
0375a5ac0b
|
@ -254,6 +254,10 @@ class SessionController < ApplicationController
|
||||||
|
|
||||||
render_sso_error(text: text || I18n.t("sso.unknown_error"), status: 500)
|
render_sso_error(text: text || I18n.t("sso.unknown_error"), status: 500)
|
||||||
|
|
||||||
|
rescue DiscourseSingleSignOn::BlankExternalId
|
||||||
|
|
||||||
|
render_sso_error(text: I18n.t("sso.blank_id_error"), status: 500)
|
||||||
|
|
||||||
rescue => e
|
rescue => e
|
||||||
message = +"Failed to create or lookup user: #{e}."
|
message = +"Failed to create or lookup user: #{e}."
|
||||||
message << " "
|
message << " "
|
||||||
|
|
|
@ -15,26 +15,6 @@ describe Scheduler::Defer do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
class TrackingLogger < ::Logger
|
|
||||||
attr_reader :messages
|
|
||||||
def initialize
|
|
||||||
super(nil)
|
|
||||||
@messages = []
|
|
||||||
end
|
|
||||||
def add(*args, &block)
|
|
||||||
@messages << args
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def track_log_messages
|
|
||||||
old_logger = Rails.logger
|
|
||||||
logger = Rails.logger = TrackingLogger.new
|
|
||||||
yield logger.messages
|
|
||||||
logger.messages
|
|
||||||
ensure
|
|
||||||
Rails.logger = old_logger
|
|
||||||
end
|
|
||||||
|
|
||||||
before do
|
before do
|
||||||
@defer = DeferInstance.new
|
@defer = DeferInstance.new
|
||||||
@defer.async = true
|
@defer.async = true
|
||||||
|
|
|
@ -385,3 +385,26 @@ def silence_stdout
|
||||||
ensure
|
ensure
|
||||||
STDOUT.unstub(:write)
|
STDOUT.unstub(:write)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class TrackingLogger < ::Logger
|
||||||
|
attr_reader :messages
|
||||||
|
def initialize(level: nil)
|
||||||
|
super(nil)
|
||||||
|
@messages = []
|
||||||
|
@level = level
|
||||||
|
end
|
||||||
|
def add(*args, &block)
|
||||||
|
if !level || args[0].to_i >= level
|
||||||
|
@messages << args
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def track_log_messages(level: nil)
|
||||||
|
old_logger = Rails.logger
|
||||||
|
logger = Rails.logger = TrackingLogger.new(level: level)
|
||||||
|
yield logger.messages
|
||||||
|
logger.messages
|
||||||
|
ensure
|
||||||
|
Rails.logger = old_logger
|
||||||
|
end
|
||||||
|
|
|
@ -563,9 +563,13 @@ RSpec.describe SessionController do
|
||||||
sso.external_id = ' '
|
sso.external_id = ' '
|
||||||
sso.username = 'sam'
|
sso.username = 'sam'
|
||||||
|
|
||||||
get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers
|
messages = track_log_messages(level: Logger::WARN) do
|
||||||
|
get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers
|
||||||
|
end
|
||||||
|
|
||||||
|
expect(messages.length).to eq(0)
|
||||||
expect(response.status).to eq(500)
|
expect(response.status).to eq(500)
|
||||||
|
expect(response.body).to include(I18n.t('sso.blank_id_error'))
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'can handle invalid sso external ids due to banned word' do
|
it 'can handle invalid sso external ids due to banned word' do
|
||||||
|
@ -1165,12 +1169,9 @@ RSpec.describe SessionController do
|
||||||
SiteSetting.enable_local_logins_via_email = false
|
SiteSetting.enable_local_logins_via_email = false
|
||||||
end
|
end
|
||||||
it 'doesnt matter, logs in correctly' do
|
it 'doesnt matter, logs in correctly' do
|
||||||
events = DiscourseEvent.track_events do
|
post "/session.json", params: {
|
||||||
post "/session.json", params: {
|
login: user.username, password: 'myawesomepassword'
|
||||||
login: user.username, password: 'myawesomepassword'
|
}
|
||||||
}
|
|
||||||
end
|
|
||||||
|
|
||||||
expect(response.status).to eq(200)
|
expect(response.status).to eq(200)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue