diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb index 9ba10487512a78649aa493da622df950b7eb0e27..79a53556f0a30700bf9ed1b79c4df99efcb1c22c 100644 --- a/app/controllers/admin/identities_controller.rb +++ b/app/controllers/admin/identities_controller.rb @@ -26,7 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController def update if @identity.update_attributes(identity_params) - RepairLdapBlockedUserService.new(@user, @identity).execute + RepairLdapBlockedUserService.new(@user).execute redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.' else render :edit @@ -35,7 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController def destroy if @identity.destroy - RepairLdapBlockedUserService.new(@user, @identity).execute + RepairLdapBlockedUserService.new(@user).execute redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully removed.' else redirect_to admin_user_identities_path(@user), alert: 'Failed to remove user identity.' diff --git a/app/services/repair_ldap_blocked_user_service.rb b/app/services/repair_ldap_blocked_user_service.rb index ceca15414e03a03bb7f7415714bcd6a9b4eea742..863cef7ff61fcd73cd6f5ab4173c0cb8a0cdaf5a 100644 --- a/app/services/repair_ldap_blocked_user_service.rb +++ b/app/services/repair_ldap_blocked_user_service.rb @@ -1,15 +1,17 @@ class RepairLdapBlockedUserService - attr_accessor :user, :identity + attr_accessor :user - def initialize(user, identity) - @user, @identity = user, identity + def initialize(user) + @user = user end def execute - if identity.destroyed? - user.block if identity.is_ldap? && user.ldap_blocked? && !user.ldap_user? - else - user.block if !identity.is_ldap? && user.ldap_blocked? && !user.ldap_user? - end + user.block if ldap_hard_blocked? + end + + private + + def ldap_hard_blocked? + user.ldap_blocked? && !user.ldap_user? end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 76cb48d7aa6fa3ac89fe1663b987d3813a1e78a1..ebd9260ad5d3ec276c29e40b206d558687ccf902 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -40,7 +40,9 @@ module Gitlab user.ldap_block false else - user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? + if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? + user.activate + end true end else diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb index 2a2114d038c9a612cbb448f14c44f9d13d672a49..ce7d1455975409772068666a304866fb68402e84 100644 --- a/spec/services/repair_ldap_blocked_user_service_spec.rb +++ b/spec/services/repair_ldap_blocked_user_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe RepairLdapBlockedUserService, services: true do let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } let(:identity) { user.ldap_identity } - subject(:service) { RepairLdapBlockedUserService.new(user, identity) } + subject(:service) { RepairLdapBlockedUserService.new(user) } describe '#execute' do it 'change to normal block after destroying last ldap identity' do