diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index 86ed9cc75..fc9b9ce86 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -145,10 +145,15 @@ class DiscourseSingleSignOn < SingleSignOn user.name = name || User.suggest_name(username.blank? ? email : username) end - if (SiteSetting.sso_overrides_avatar && avatar_url.present? && ( - sso_record.external_avatar_url != avatar_url)) || avatar_force_update + avatar_missing = user.uploaded_avatar_id.nil? || !Upload.exists?(user.uploaded_avatar_id) - UserAvatar.import_url_for_user(avatar_url, user) + if (avatar_missing || avatar_force_update || SiteSetting.sso_overrides_avatar) && avatar_url.present? + + avatar_changed = sso_record.external_avatar_url != avatar_url + + if avatar_force_update || avatar_changed || avatar_missing + UserAvatar.import_url_for_user(avatar_url, user) + end end # change external attributes for sso record diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb index 47aeca7ef..6c01e380e 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_single_sign_on_spec.rb @@ -294,6 +294,16 @@ describe DiscourseSingleSignOn do # initial creation ... expect(avatar_id).to_not eq(nil) + # junk avatar id should be updated + old_id = user.uploaded_avatar_id + Upload.destroy(old_id) + + user = sso.lookup_or_create_user(ip_address) + avatar_id = user.uploaded_avatar_id + + expect(avatar_id).to_not eq(nil) + expect(old_id).to_not eq(avatar_id) + FileHelper.stubs(:download) { raise "should not be called" } sso.avatar_url = "https://some.new/avatar.png" user = sso.lookup_or_create_user(ip_address)