Merge pull request ManageIQ#14142 from jvlcek/bz1400567_RFE_LDAP_no_d…

…isplayname

Ensure user name is set even when common LDAP attributes are missing.
(cherry picked from commit b502d51)

https://bugzilla.redhat.com/show_bug.cgi?id=1428859

app/models/authenticator/httpd.rb

@@ -47,10 +47,12 @@ def update_user_attributes(user, _username, identity)
       user_attrs, _membership_list = identity
 
       user.userid     = user_attrs[:username]
-      user.name       = user_attrs[:fullname]
       user.first_name = user_attrs[:firstname]
       user.last_name  = user_attrs[:lastname]
       user.email      = user_attrs[:email] unless user_attrs[:email].blank?
+      user.name       = user_attrs[:fullname]
+      user.name       = "#{user.first_name} #{user.last_name}" if user.name.blank?
+      user.name       = user.userid if user.name.blank?
     end
 
     private

app/models/authenticator/ldap.rb

@@ -131,11 +131,13 @@ def groups_for(obj)
 
     def update_user_attributes(user, _username, lobj)
       user.userid     = ldap.normalize(ldap.get_attr(lobj, :userprincipalname) || ldap.get_attr(lobj, :dn))
-      user.name       = ldap.get_attr(lobj, :displayname)
       user.first_name = ldap.get_attr(lobj, :givenname)
       user.last_name  = ldap.get_attr(lobj, :sn)
       email           = ldap.get_attr(lobj, :mail)
       user.email      = email unless email.blank?
+      user.name       = ldap.get_attr(lobj, :displayname)
+      user.name       = "#{user.first_name} #{user.last_name}" if user.name.blank?
+      user.name       = user.userid if user.name.blank?
     end
 
     REQUIRED_LDAP_USER_PROXY_KEYS = [:basedn, :bind_dn, :bind_pwd, :ldaphost, :ldapport, :mode]

spec/models/authenticator/httpd_spec.rb

@@ -295,6 +295,34 @@ def authenticate
             expect(MiqTask.status_error?(task.status)).to be_truthy
           end
         end
+
+        context "when fullname is blank" do
+          let(:username) { 'betty' }
+          let(:headers) do
+            super().merge('X-Remote-User-FullName'  => '',
+                          'X-Remote-User-FirstName' => 'Betty',
+                          'X-Remote-User-LastName'  => 'Boop',
+                          'X-Remote-User-Email'     => 'betty@example.com')
+          end
+
+          it "creates a new User with name set to FirstName + LastName" do
+            expect(-> { authenticate }).to change { User.where(:name => 'Betty Boop').count }.from(0).to(1)
+          end
+        end
+
+        context "when fullname, firstname and lastname are blank" do
+          let(:username) { 'sam' }
+          let(:headers) do
+            super().merge('X-Remote-User-FullName'  => '',
+                          'X-Remote-User-FirstName' => '',
+                          'X-Remote-User-LastName'  => '',
+                          'X-Remote-User-Email'     => 'sam@example.com')
+          end
+
+          it "creates a new User with name set to the userid" do
+            expect(-> { authenticate }).to change { User.where(:name => 'sam').count }.from(0).to(1)
+          end
+        end
       end
 
       describe ".user_attrs_from_external_directory" do

spec/models/authenticator/ldap_spec.rb

@@ -62,6 +62,8 @@ def normalize(dn)
       'rootdn' => {:password => 'verysecret'},
       'alice'  => alice_data,
       'bob'    => bob_data,
+      'betty'  => betty_data,
+      'sam'    => sam_data,
     }
   end
   let(:alice_data) do
@@ -86,6 +88,28 @@ def normalize(dn)
       :groups            => %w(wibble bubble),
     }
   end
+  let(:betty_data) do
+    {
+      :userprincipalname => 'betty',
+      :password          => 'secret',
+      :displayname       => nil,
+      :givenname         => 'Betty',
+      :sn                => 'Builderson',
+      :mail              => 'betty@example.com',
+      :groups            => %w(wibble bubble),
+    }
+  end
+  let(:sam_data) do
+    {
+      :userprincipalname => 'sam',
+      :password          => 'secret',
+      :displayname       => nil,
+      :givenname         => nil,
+      :sn                => nil,
+      :mail              => 'sam@example.com',
+      :groups            => %w(wibble bubble),
+    }
+  end
 
   before(:each) do
     allow(MiqLdap).to receive(:new) { FakeLdap.new(user_data) }
@@ -462,6 +486,22 @@ def authenticate
             expect(MiqTask.status_error?(task.status)).to be_truthy
           end
         end
+
+        context "when display name is blank" do
+          let(:username) { 'betty' }
+
+          it "creates a new User with name set to givenname + sn" do
+            expect(-> { authenticate }).to change { User.where(:name => 'Betty Builderson').count }.from(0).to(1)
+          end
+        end
+
+        context "when display name, givenname and sn are blank" do
+          let(:username) { 'sam' }
+
+          it "creates a new User with name set to the userid" do
+            expect(-> { authenticate }).to change { User.where(:name => 'sam').count }.from(0).to(1)
+          end
+        end
       end
     end
   end