Fixed issue #1905: Stored Exchange remote_id in login causes sync to update wrong user.

app/assets/javascripts/app/controllers/_integration/exchange.coffee

@@ -454,9 +454,9 @@ class ConnectionWizard extends App.WizardModal
     for key in ['source', 'dest']
       if !_.isArray(attributes[key])
         attributes[key] = [attributes[key]]
-    attributes_local =
+
-      item_id: 'login'
+    attributes_local = {}
-    length = attributes.source.length-1
+    length           = attributes.source.length-1
     for count in [0..length]
       if attributes.source[count] && attributes.dest[count]
         attributes_local[attributes.source[count]] = attributes.dest[count]
@@ -465,18 +465,8 @@ class ConnectionWizard extends App.WizardModal
     @tryShow()
 
   buildRowsUserMap: (user_attribute_map) =>
-
+    el = []
-    # show static login row
-    userUidDisplayValue = @wizardConfig.wizardData.backend_attributes['item_id']
-    el = [
-      $(App.view('integration/ldap_user_attribute_row_read_only')(
-        key:   userUidDisplayValue,
-        value: 'Login'
-      ))
-    ]
-
     for source, dest of user_attribute_map
-      continue if source == 'item_id'
       continue if !(source of @wizardConfig.wizardData.backend_attributes)
       el.push @buildRowUserAttribute(source, dest)
     el

app/assets/javascripts/app/views/integration/ldap_user_attribute_row_read_only.jst.eco

@@ -1,6 +0,0 @@
-<tr class="js-entry">
-  <td style="max-width: 240px" class="settings-list-row-control">
-    <div class="u-textTruncate"><%= @key %></div>
-  <td class="settings-list-row-control">
-    <%= @value %>
-  <td class="settings-list-row-control">

db/migrate/20180327170847_issue_1905_exchange_login_from_remote_id.rb

@@ -0,0 +1,17 @@
+class Issue1905ExchangeLoginFromRemoteId < ActiveRecord::Migration[5.1]
+  def change
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    config = Import::Exchange.config
+    return if config.blank?
+    return if config[:attributes].blank?
+    return if config[:attributes][:item_id].blank?
+    return if config[:attributes][:item_id] != 'login'
+
+    config[:attributes].delete(:item_id)
+
+    Import::Exchange.config = config
+  end
+end

lib/sequencer/sequence/import/exchange/folder_contact.rb

@@ -8,7 +8,8 @@ class Sequencer
             [
               'Import::Exchange::FolderContact::RemoteId',
               'Import::Common::RemoteId::CaseSensitive',
-              'Import::Exchange::FolderContact::Mapping',
+              'Import::Exchange::FolderContact::Mapping::FromConfig',
+              'Import::Exchange::FolderContact::Mapping::Login',
               'Import::Common::Model::Skip::Blank::Mapped',
               'Common::ModelClass::User',
               'Import::Exchange::FolderContact::ExternalSyncSource',

lib/sequencer/unit/import/exchange/folder_contact/mapping.rb

@@ -1,27 +0,0 @@
-class Sequencer
-  class Unit
-    module Import
-      module Exchange
-        module FolderContact
-          class Mapping < Sequencer::Unit::Import::Common::Mapping::FlatKeys
-
-            uses :import_job
-
-            private
-
-            def mapping
-              from_import_job || ::Import::Exchange.config[:attributes]
-            end
-
-            def from_import_job
-              return if !state.provided?(:import_job)
-              payload = import_job.payload
-              return if payload.blank?
-              payload[:ews_attributes]
-            end
-          end
-        end
-      end
-    end
-  end
-end

lib/sequencer/unit/import/exchange/folder_contact/mapping/from_config.rb

@@ -0,0 +1,29 @@
+class Sequencer
+  class Unit
+    module Import
+      module Exchange
+        module FolderContact
+          module Mapping
+            class FromConfig < Sequencer::Unit::Import::Common::Mapping::FlatKeys
+
+              uses :import_job
+
+              private
+
+              def mapping
+                from_import_job || ::Import::Exchange.config[:attributes]
+              end
+
+              def from_import_job
+                return if !state.provided?(:import_job)
+                payload = import_job.payload
+                return if payload.blank?
+                payload[:ews_attributes]
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/sequencer/unit/import/exchange/folder_contact/mapping/login.rb

@@ -0,0 +1,25 @@
+class Sequencer
+  class Unit
+    module Import
+      module Exchange
+        module FolderContact
+          module Mapping
+            class Login < Sequencer::Unit::Import::Common::Mapping::FlatKeys
+              include ::Sequencer::Unit::Import::Common::Mapping::Mixin::ProvideMapped
+
+              uses :remote_id
+
+              def process
+                provide_mapped do
+                  {
+                    login: remote_id
+                  }
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/db/migrate/issue_1905_exchange_login_from_remote_id_spec.rb

@@ -0,0 +1,82 @@
+require 'rails_helper'
+
+RSpec.describe Issue1905ExchangeLoginFromRemoteId, type: :db_migration do
+
+  let(:backend) { ::Import::Exchange }
+
+  it 'removes :item_id from attributes' do
+
+    invalid_config = {
+      attributes: {
+        item_id: 'login',
+        some:    'other',
+      }
+    }
+
+    valid_config = ActiveSupport::HashWithIndifferentAccess.new(
+      attributes: {
+        some:    'other',
+      }
+    )
+
+    expect(backend).to receive(:config).and_return(invalid_config)
+    allow(backend).to receive(:config).and_call_original
+
+    migrate
+
+    expect(backend.config).to eq(valid_config)
+  end
+
+  context 'no changes' do
+
+    it 'performs no action for new systems', system_init_done: false do
+      expect(backend).not_to receive(:config)
+      migrate
+    end
+
+    shared_examples 'irrelevant config' do
+      it 'does not change the config' do
+        expect(backend).to receive(:config).and_return(config)
+        expect(backend).not_to receive(:config=)
+        migrate
+      end
+    end
+
+    context 'blank config' do
+      let(:config) { nil }
+      it_behaves_like 'irrelevant config'
+    end
+
+    context 'blank attributes' do
+      let(:config) do
+        {
+          some: 'config'
+        }
+      end
+      it_behaves_like 'irrelevant config'
+    end
+
+    context 'blank attribute :item_id' do
+      let(:config) do
+        {
+          attributes: {
+            some: 'mapping'
+          }
+        }
+      end
+      it_behaves_like 'irrelevant config'
+    end
+
+    context 'attribute :item_id not mapping to login' do
+
+      let(:config) do
+        {
+          attributes: {
+            item_id: 'other_local_attribute'
+          }
+        }
+      end
+      it_behaves_like 'irrelevant config'
+    end
+  end
+end