Fix bug when sso_overrides_avatar is true but no avatar_url is passed

If a user has a current avatar, and sso_overrides_avatar is true, but no avatar_url is
passed in the sso attributes, the current code errors, as it tries to parse a nil
as a URL. It seems to me valid that a third party system may not pass an avatar_url in
some cases (e.g. avatars may not be mandatory, so not all users may have them)

This might warrant a discussion about what should happen in this case; maybe the current
avatar in discourse should be removed? This branch merely stops the login process erroring.
This commit is contained in:
Dan Singerman 2015-01-28 15:47:59 +00:00
parent 4c0129ccdd
commit dbf2f4efec
3 changed files with 32 additions and 1 deletions

View File

@ -125,10 +125,11 @@ class DiscourseSingleSignOn < SingleSignOn
user.name = User.suggest_name(name || username || email) user.name = User.suggest_name(name || username || email)
end end
if SiteSetting.sso_overrides_avatar && ( if SiteSetting.sso_overrides_avatar && avatar_url.present? && (
avatar_force_update == "true" || avatar_force_update == "true" ||
avatar_force_update.to_i != 0 || avatar_force_update.to_i != 0 ||
sso_record.external_avatar_url != avatar_url) sso_record.external_avatar_url != avatar_url)
begin begin
tempfile = FileHelper.download(avatar_url, 1.megabyte, "sso-avatar", true) tempfile = FileHelper.download(avatar_url, 1.megabyte, "sso-avatar", true)

View File

@ -0,0 +1,9 @@
Fabricator(:single_sign_on_record) do
user
external_id { sequence(:external_id) { |i| "ext_#{i}" } }
external_username { sequence(:username) { |i| "bruce#{i}" } }
external_email { sequence(:email) { |i| "bruce#{i}@wayne.com" } }
last_payload { sequence(:last_payload) { |i| "nonce=#{i}1870a940bbcbb46f06880ed338d58a07&name=" } }
end

View File

@ -103,4 +103,25 @@ describe DiscourseSingleSignOn do
sso = DiscourseSingleSignOn.parse(payload) sso = DiscourseSingleSignOn.parse(payload)
expect(sso.nonce).to_not be_nil expect(sso.nonce).to_not be_nil
end end
context 'when sso_overrides_avatar is enabled' do
before do
SiteSetting.sso_overrides_avatar = true
end
it "deal with no avatar url passed for an existing user with an avatar" do
sso_record = Fabricate(:single_sign_on_record, external_avatar_url: "http://example.com/an_image.png")
sso = DiscourseSingleSignOn.new
sso.username = "test"
sso.name = "test"
sso.email = sso_record.user.email
sso.external_id = sso_record.external_id
# Deliberately not setting avatar_url.
user = sso.lookup_or_create_user
expect(user).to_not be_nil
end
end
end end