diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb index e383fe38ea6..9ba10487512 100644 --- a/app/controllers/admin/identities_controller.rb +++ b/app/controllers/admin/identities_controller.rb @@ -26,6 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController def update if @identity.update_attributes(identity_params) + RepairLdapBlockedUserService.new(@user, @identity).execute redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.' else render :edit @@ -34,6 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController def destroy if @identity.destroy + RepairLdapBlockedUserService.new(@user, @identity).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/models/identity.rb b/app/models/identity.rb index 8bcdc194953..830b99fa3f2 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -18,4 +18,8 @@ class Identity < ActiveRecord::Base validates :provider, presence: true validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider } validates :user_id, uniqueness: { scope: :provider } + + def is_ldap? + provider.starts_with?('ldap') + end end diff --git a/app/models/user.rb b/app/models/user.rb index 67b47b0f329..5eed9cf91c7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -196,6 +196,7 @@ class User < ActiveRecord::Base state_machine :state, initial: :active do event :block do transition active: :blocked + transition ldap_blocked: :blocked end event :ldap_block do diff --git a/app/services/repair_ldap_blocked_user_service.rb b/app/services/repair_ldap_blocked_user_service.rb new file mode 100644 index 00000000000..ceca15414e0 --- /dev/null +++ b/app/services/repair_ldap_blocked_user_service.rb @@ -0,0 +1,15 @@ +class RepairLdapBlockedUserService + attr_accessor :user, :identity + + def initialize(user, identity) + @user, @identity = user, identity + 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 + end +end diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb new file mode 100644 index 00000000000..c131d22a30a --- /dev/null +++ b/spec/controllers/admin/identities_controller_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Admin::IdentitiesController do + let(:admin) { create(:admin) } + before { sign_in(admin) } + + describe 'UPDATE identity' do + let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } + + it 'repairs ldap blocks' do + expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute) + + put :update, user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' } + end + end + + describe 'DELETE identity' do + let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } + + it 'repairs ldap blocks' do + expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute) + + delete :destroy, user_id: user.username, id: user.ldap_identity.id + end + end +end diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb new file mode 100644 index 00000000000..107bfc17782 --- /dev/null +++ b/spec/models/identity_spec.rb @@ -0,0 +1,38 @@ +# == Schema Information +# +# Table name: identities +# +# id :integer not null, primary key +# extern_uid :string(255) +# provider :string(255) +# user_id :integer +# created_at :datetime +# updated_at :datetime +# + +require 'spec_helper' + +RSpec.describe Identity, models: true do + + describe 'relations' do + it { is_expected.to belong_to(:user) } + end + + describe 'fields' do + it { is_expected.to respond_to(:provider) } + it { is_expected.to respond_to(:extern_uid) } + end + + describe '#is_ldap?' do + let(:ldap_identity) { create(:identity, provider: 'ldapmain') } + let(:other_identity) { create(:identity, provider: 'twitter') } + + it 'returns true if it is a ldap identity' do + expect(ldap_identity.is_ldap?).to be_truthy + end + + it 'returns false if it is not a ldap identity' do + expect(other_identity.is_ldap?).to be_falsey + end + end +end diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb new file mode 100644 index 00000000000..2a2114d038c --- /dev/null +++ b/spec/services/repair_ldap_blocked_user_service_spec.rb @@ -0,0 +1,23 @@ +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) } + + describe '#execute' do + it 'change to normal block after destroying last ldap identity' do + identity.destroy + service.execute + + expect(user.reload).not_to be_ldap_blocked + end + + it 'change to normal block after changing last ldap identity to another provider' do + identity.update_attribute(:provider, 'twitter') + service.execute + + expect(user.reload).not_to be_ldap_blocked + end + end +end