Merge pull request #3140 from fullfatthings/fix_missing_sso_avatar_bug

Fix bug when sso_overrides_avatar is true but no avatar_url is passed

app/models/discourse_single_sign_on.rb

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

spec/fabricators/single_sign_on_record_fabrictor.rb

@@ -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
+
+

spec/models/discourse_single_sign_on_spec.rb

@@ -103,4 +103,25 @@ describe DiscourseSingleSignOn do
     sso = DiscourseSingleSignOn.parse(payload)
     expect(sso.nonce).to_not be_nil
   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