Skip to content

LDAP sync job doesn't add new SSH keys if at least one key already exists

Summary

During to LDAP user sync job, GitLab also sync SSH keys from LDAP. But, if one of the keys already exists, job will raise error and skip synchronisation of all keys in array which is after existing keys.

Steps to reproduce

  1. Create user in LDAP and add one SSH key
  2. Sync users with GitLab
  3. Check that SSH key of the user exists in GitLab
  4. Add second SSH key to LDAP
  5. Sync users with GitLab, get error during the sync.

What is the current bug behavior?

Sync job raise error and don't add SSH keys

What is the expected correct behavior?

Add all SSH keys during the sync

Relevant logs and/or screenshots

Syncing user <username1>
Instantiating Gitlab::Auth::LDAP::Person with LDIF:
..
cn: username1
ipasshpubkey: ssh-rsa key-one
ipasshpubkey: ssh-rsa key-two
...
Gitlab::Auth::LDAP::Access: adding LDAP SSH key "ssh-rsa key-one" to <username1> (55)
Gitlab::Auth::LDAP::Access: failed to add LDAP SSH key "ssh-rsa key-one" to <username1> (55)
error messages: {:fingerprint=>["has already been taken"]}
Syncing user <username2>

Results of GitLab environment info

Expand for output related to GitLab environment info 
GitLab information
Version:        11.4.4-ee
Revision:       e325c5f2

Possible fixes

In that file update function add_new_ssh_keys like that (I'm not a Ruby programmer):

          def add_new_ssh_keys
            keys = ldap_user.ssh_keys - user.keys.ldap.pluck(:key)

            keys.each do |key|
              begin
                ::Rails.logger.info "#{self.class.name}: adding LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})"
                new_key = ::LDAPKey.new(title: "LDAP - #{ldap_config.sync_ssh_keys}", key: key)
                new_key.user = user

                unless new_key.save
                  ::Rails.logger.error "#{self.class.name}: failed to add LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})\n"\
                    "error messages: #{new_key.errors.messages}"
              rescue
                ::Rails.logger.warn "#{self.class.name}: try to add next LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})"
              end
            end
          end