From bc7ef8e5b7a002ca6bc2d7a5e6be11b4a59b6710 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 29 Dec 2015 17:53:55 -0200
Subject: Add ldap_blocked as new state to users state machine

---
 app/models/user.rb       | 12 +++++++++++-
 spec/models/user_spec.rb | 44 ++++++++++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 46b36c605b0..67b47b0f329 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -198,16 +198,26 @@ class User < ActiveRecord::Base
       transition active: :blocked
     end
 
+    event :ldap_block do
+      transition active: :ldap_blocked
+    end
+
     event :activate do
       transition blocked: :active
     end
+
+    state :blocked, :ldap_blocked do
+      def blocked?
+        true
+      end
+    end
   end
 
   mount_uploader :avatar, AvatarUploader
 
   # Scopes
   scope :admins, -> { where(admin: true) }
-  scope :blocked, -> { with_state(:blocked) }
+  scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
   scope :active, -> { with_state(:active) }
   scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all }
   scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 3cd63b2b0e8..0bef68e2885 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -569,27 +569,39 @@ describe User, models: true do
     end
   end
 
-  describe :ldap_user? do
-    it "is true if provider name starts with ldap" do
-      user = create(:omniauth_user, provider: 'ldapmain')
-      expect( user.ldap_user? ).to be_truthy
-    end
+  context 'ldap synchronized user' do
+    describe :ldap_user? do
+      it 'is true if provider name starts with ldap' do
+        user = create(:omniauth_user, provider: 'ldapmain')
+        expect(user.ldap_user?).to be_truthy
+      end
 
-    it "is false for other providers" do
-      user = create(:omniauth_user, provider: 'other-provider')
-      expect( user.ldap_user? ).to be_falsey
+      it 'is false for other providers' do
+        user = create(:omniauth_user, provider: 'other-provider')
+        expect(user.ldap_user?).to be_falsey
+      end
+
+      it 'is false if no extern_uid is provided' do
+        user = create(:omniauth_user, extern_uid: nil)
+        expect(user.ldap_user?).to be_falsey
+      end
     end
 
-    it "is false if no extern_uid is provided" do
-      user = create(:omniauth_user, extern_uid: nil)
-      expect( user.ldap_user? ).to be_falsey
+    describe :ldap_identity do
+      it 'returns ldap identity' do
+        user = create :omniauth_user
+        expect(user.ldap_identity.provider).not_to be_empty
+      end
     end
-  end
 
-  describe :ldap_identity do
-    it "returns ldap identity" do
-      user = create :omniauth_user
-      expect(user.ldap_identity.provider).not_to be_empty
+    describe '#ldap_block' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain', name: 'John Smith') }
+
+      it 'blocks user flaging the action caming from ldap' do
+        user.ldap_block
+        expect(user.blocked?).to be_truthy
+        expect(user.ldap_blocked?).to be_truthy
+      end
     end
   end
 
-- 
cgit v1.2.1


From ba9855d4877998e3574907cc542fcab15a9d1353 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 29 Dec 2015 18:58:38 -0200
Subject: Prevent ldap_blocked users from being unblocked by the Admin UI

---
 app/assets/stylesheets/framework/buttons.scss   |  3 +++
 app/controllers/admin/users_controller.rb       |  4 ++-
 app/views/admin/users/index.html.haml           |  7 ++++-
 spec/controllers/admin/users_controller_spec.rb | 35 ++++++++++++++++++-------
 4 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index 97a94638847..e2376363485 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -132,6 +132,9 @@
       margin-right: 0px;
     }
   }
+  &.disabled {
+    pointer-events: auto !important;
+  }
 }
 
 .btn-block {
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index d7c927d444c..87f4fb455b8 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -40,7 +40,9 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def unblock
-    if user.activate
+    if user.ldap_blocked?
+      redirect_back_or_admin_user(alert: "This user cannot be unlocked manually from GitLab")
+    elsif user.activate
       redirect_back_or_admin_user(notice: "Successfully unblocked")
     else
       redirect_back_or_admin_user(alert: "Error occurred. User was not unblocked")
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index a92c9c152b9..911c4d0cf12 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -90,7 +90,12 @@
               &nbsp;
               = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-xs"
               - unless user == current_user
-                - if user.blocked?
+                - if user.ldap_blocked?
+                  = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn btn-xs btn-success disabled' do
+                    %i.fa.fa-lock
+                    Unblock
+                  = ''
+                - elsif user.blocked?
                   = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
                 - else
                   = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 8b7af4d3a0a..5b1f65d7aff 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -34,17 +34,34 @@ describe Admin::UsersController do
   end
 
   describe 'PUT unblock/:id' do
-    let(:user) { create(:user) }
-
-    before do
-      user.block
+    context 'ldap blocked users' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain') }
+
+      before do
+        user.ldap_block
+      end
+
+      it 'will not unblock user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_truthy
+        expect(flash[:alert]).to eq 'This user cannot be unlocked manually from GitLab'
+      end
     end
 
-    it 'unblocks user' do
-      put :unblock, id: user.username
-      user.reload
-      expect(user.blocked?).to be_falsey
-      expect(flash[:notice]).to eq 'Successfully unblocked'
+    context 'manually blocked users' do
+      let(:user) { create(:user) }
+
+      before do
+        user.block
+      end
+
+      it 'unblocks user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_falsey
+        expect(flash[:notice]).to eq 'Successfully unblocked'
+      end
     end
   end
 
-- 
cgit v1.2.1


From 6e7db8e23e169bcbf0847ece27b9e44e00ae572b Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 30 Dec 2015 16:52:02 -0200
Subject: Prevent ldap_blocked users from being blocked/unblocked by the API

---
 doc/api/users.md                |  6 ++++--
 lib/api/users.rb                | 12 ++++++++----
 spec/requests/api/users_spec.rb | 23 ++++++++++++++++++-----
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/doc/api/users.md b/doc/api/users.md
index 773fe36d277..b7fc903825e 100644
--- a/doc/api/users.md
+++ b/doc/api/users.md
@@ -558,7 +558,8 @@ Parameters:
 
 - `uid` (required) - id of specified user
 
-Will return `200 OK` on success, or `404 User Not Found` is user cannot be found.
+Will return `200 OK` on success, `404 User Not Found` is user cannot be found or 
+`403 Forbidden` when trying to block an already blocked user by LDAP synchronization.
 
 ## Unblock user
 
@@ -572,4 +573,5 @@ Parameters:
 
 - `uid` (required) - id of specified user
 
-Will return `200 OK` on success, or `404 User Not Found` is user cannot be found.
+Will return `200 OK` on success, `404 User Not Found` is user cannot be found or
+`403 Forbidden` when trying to unblock a user blocked by LDAP synchronization.
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 0d7813428e2..01fd90139b0 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -284,10 +284,12 @@ module API
         authenticated_as_admin!
         user = User.find_by(id: params[:id])
 
-        if user
+        if !user
+          not_found!('User')
+        elsif !user.ldap_blocked?
           user.block
         else
-          not_found!('User')
+          forbidden!('LDAP blocked users cannot be modified by the API')
         end
       end
 
@@ -299,10 +301,12 @@ module API
         authenticated_as_admin!
         user = User.find_by(id: params[:id])
 
-        if user
+        if !user
+          not_found!('User')
+        elsif !user.ldap_blocked?
           user.activate
         else
-          not_found!('User')
+          forbidden!('LDAP blocked users cannot be unblocked by the API')
         end
       end
     end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 4f278551d07..b82c5c7685f 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -8,6 +8,8 @@ describe API::API, api: true  do
   let(:key)   { create(:key, user: user) }
   let(:email)   { create(:email, user: user) }
   let(:omniauth_user) { create(:omniauth_user) }
+  let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') }
+  let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
 
   describe "GET /users" do
     context "when unauthenticated" do
@@ -783,6 +785,12 @@ describe API::API, api: true  do
       expect(user.reload.state).to eq('blocked')
     end
 
+    it 'should not re-block ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/block", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+    end
+
     it 'should not be available for non admin users' do
       put api("/users/#{user.id}/block", user)
       expect(response.status).to eq(403)
@@ -797,7 +805,9 @@ describe API::API, api: true  do
   end
 
   describe 'PUT /user/:id/unblock' do
+    let(:blocked_user)  { create(:user, state: 'blocked') }
     before { admin }
+
     it 'should unblock existing user' do
       put api("/users/#{user.id}/unblock", admin)
       expect(response.status).to eq(200)
@@ -805,12 +815,15 @@ describe API::API, api: true  do
     end
 
     it 'should unblock a blocked user' do
-      put api("/users/#{user.id}/block", admin)
-      expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('blocked')
-      put api("/users/#{user.id}/unblock", admin)
+      put api("/users/#{blocked_user.id}/unblock", admin)
       expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('active')
+      expect(blocked_user.reload.state).to eq('active')
+    end
+
+    it 'should not unblock ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/unblock", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
     end
 
     it 'should not be available for non admin users' do
-- 
cgit v1.2.1


From d6dc088affeee4568e771e1d7894e0bcdb955af8 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 30 Dec 2015 20:56:26 -0200
Subject: LDAP synchronization block/unblock new states

---
 lib/gitlab/ldap/access.rb           |  6 +++---
 spec/lib/gitlab/ldap/access_spec.rb | 34 ++++++++++++++--------------------
 2 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index c438a3d167b..76cb48d7aa6 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -37,15 +37,15 @@ module Gitlab
 
           # Block user in GitLab if he/she was blocked in AD
           if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter)
-            user.block
+            user.ldap_block
             false
           else
-            user.activate if user.blocked? && !ldap_config.block_auto_created_users
+            user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
             true
           end
         else
           # Block the user if they no longer exist in LDAP/AD
-          user.block 
+          user.ldap_block
           false
         end
       rescue
diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb
index a628d0c0157..f58d70e809c 100644
--- a/spec/lib/gitlab/ldap/access_spec.rb
+++ b/spec/lib/gitlab/ldap/access_spec.rb
@@ -13,64 +13,59 @@ describe Gitlab::LDAP::Access, lib: true do
       end
 
       it { is_expected.to be_falsey }
-      
+
       it 'should block user in GitLab' do
         access.allowed?
         expect(user).to be_blocked
+        expect(user).to be_ldap_blocked
       end
     end
 
     context 'when the user is found' do
       before do
-        allow(Gitlab::LDAP::Person).
-          to receive(:find_by_dn).and_return(:ldap_user)
+        allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
       end
 
       context 'and the user is disabled via active directory' do
         before do
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(true)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true)
         end
 
         it { is_expected.to be_falsey }
 
-        it "should block user in GitLab" do
+        it 'should block user in GitLab' do
           access.allowed?
           expect(user).to be_blocked
+          expect(user).to be_ldap_blocked
         end
       end
 
       context 'and has no disabled flag in active diretory' do
         before do
           user.block
-
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(false)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false)
         end
 
         it { is_expected.to be_truthy }
 
         context 'when auto-created users are blocked' do
-
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(true)
+            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true)
           end
 
-          it "does not unblock user in GitLab" do
+          it 'does not unblock user in GitLab' do
             access.allowed?
             expect(user).to be_blocked
+            expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic
           end
         end
 
-        context "when auto-created users are not blocked" do
-
+        context 'when auto-created users are not blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(false)
+            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false)
           end
 
-          it "should unblock user in GitLab" do
+          it 'should unblock user in GitLab' do
             access.allowed?
             expect(user).not_to be_blocked
           end
@@ -80,8 +75,7 @@ describe Gitlab::LDAP::Access, lib: true do
       context 'without ActiveDirectory enabled' do
         before do
           allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
-          allow_any_instance_of(Gitlab::LDAP::Config).
-            to receive(:active_directory).and_return(false)
+          allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false)
         end
 
         it { is_expected.to be_truthy }
-- 
cgit v1.2.1


From ec67e9be1d7486199b47e19c766202a8bfdefe93 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 6 Jan 2016 05:38:52 -0200
Subject: Repair ldap_blocked state when no ldap identity exist anymore

---
 app/controllers/admin/identities_controller.rb     |  2 ++
 app/models/identity.rb                             |  4 +++
 app/models/user.rb                                 |  1 +
 app/services/repair_ldap_blocked_user_service.rb   | 15 +++++++++
 .../admin/identities_controller_spec.rb            | 26 +++++++++++++++
 spec/models/identity_spec.rb                       | 38 ++++++++++++++++++++++
 .../repair_ldap_blocked_user_service_spec.rb       | 23 +++++++++++++
 7 files changed, 109 insertions(+)
 create mode 100644 app/services/repair_ldap_blocked_user_service.rb
 create mode 100644 spec/controllers/admin/identities_controller_spec.rb
 create mode 100644 spec/models/identity_spec.rb
 create mode 100644 spec/services/repair_ldap_blocked_user_service_spec.rb

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
-- 
cgit v1.2.1


From 47e4613f4adc2d6ef4b066a87ec772ef8044bdd5 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 7 Jan 2016 14:01:01 -0200
Subject: Code style fixes and some code simplified

---
 app/controllers/admin/identities_controller.rb         |  4 ++--
 app/services/repair_ldap_blocked_user_service.rb       | 18 ++++++++++--------
 lib/gitlab/ldap/access.rb                              |  4 +++-
 spec/services/repair_ldap_blocked_user_service_spec.rb |  2 +-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb
index 9ba10487512..79a53556f0a 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 ceca15414e0..863cef7ff61 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 76cb48d7aa6..ebd9260ad5d 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 2a2114d038c..ce7d1455975 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
-- 
cgit v1.2.1


From ac6a10f3e88c5d2081b8638df63016089517a844 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 12 Jan 2016 12:29:10 -0200
Subject: Codestyle changes

---
 app/models/identity.rb       | 2 +-
 lib/api/users.rb             | 6 +++---
 spec/models/identity_spec.rb | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/models/identity.rb b/app/models/identity.rb
index 830b99fa3f2..e1915b079d4 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -19,7 +19,7 @@ class Identity < ActiveRecord::Base
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
   validates :user_id, uniqueness: { scope: :provider }
 
-  def is_ldap?
+  def ldap?
     provider.starts_with?('ldap')
   end
 end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 01fd90139b0..fd2128bd179 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -303,10 +303,10 @@ module API
 
         if !user
           not_found!('User')
-        elsif !user.ldap_blocked?
-          user.activate
-        else
+        elsif user.ldap_blocked?
           forbidden!('LDAP blocked users cannot be unblocked by the API')
+        else
+          user.activate
         end
       end
     end
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
index 107bfc17782..5afe042e154 100644
--- a/spec/models/identity_spec.rb
+++ b/spec/models/identity_spec.rb
@@ -28,11 +28,11 @@ RSpec.describe Identity, models: true do
     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
+      expect(ldap_identity.ldap?).to be_truthy
     end
 
     it 'returns false if it is not a ldap identity' do
-      expect(other_identity.is_ldap?).to be_falsey
+      expect(other_identity.ldap?).to be_falsey
     end
   end
 end
-- 
cgit v1.2.1


From fc7e6e8ffcb76e92b625b6490a966c830adc750a Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 14 Jan 2016 03:08:40 -0200
Subject: Fix Admin/Users view to position buttons without spacing magic

---
 app/assets/stylesheets/framework/buttons.scss |  3 +++
 app/views/admin/users/index.html.haml         | 30 +++++++++++++--------------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index e2376363485..bb29829b7a1 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -131,6 +131,9 @@
     &:last-child {
       margin-right: 0px;
     }
+    &.btn-xs {
+      margin-right: 3px;
+    }
   }
   &.disabled {
     pointer-events: auto !important;
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index 911c4d0cf12..8312642b6c3 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -88,19 +88,19 @@
                 %i.fa.fa-envelope
                 = mail_to user.email, user.email, class: 'light'
               &nbsp;
-              = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-xs"
-              - unless user == current_user
-                - if user.ldap_blocked?
-                  = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn btn-xs btn-success disabled' do
-                    %i.fa.fa-lock
-                    Unblock
-                  = ''
-                - elsif user.blocked?
-                  = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
-                - else
-                  = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
-                - if user.access_locked?
-                  = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success", data: { confirm: 'Are you sure?' }
-                - if user.can_be_removed?
-                  = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
+              .pull-right
+                = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn-grouped btn btn-xs'
+                - unless user == current_user
+                  - if user.ldap_blocked?
+                    = link_to '#', title: 'Cannot unblock LDAP blocked users', data: {toggle: 'tooltip'}, class: 'btn-grouped btn btn-xs btn-success disabled' do
+                      %i.fa.fa-lock
+                      Unblock
+                  - elsif user.blocked?
+                    = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success'
+                  - else
+                    = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: 'btn-grouped btn btn-xs btn-warning'
+                  - if user.access_locked?
+                    = link_to 'Unlock', unlock_admin_user_path(user), method: :put, class: 'btn-grouped btn btn-xs btn-success', data: { confirm: 'Are you sure?' }
+                  - if user.can_be_removed?
+                    = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All issues, merge requests and groups linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: 'btn-grouped btn btn-xs btn-remove'
     = paginate @users, theme: "gitlab"
-- 
cgit v1.2.1


From dd6fc01ff8a073880b67a323a547edeb5d63f167 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 14 Jan 2016 03:31:27 -0200
Subject: fixed LDAP activation on login to use new ldap_blocked state

---
 app/models/user.rb                  | 1 +
 lib/gitlab/ldap/access.rb           | 4 +---
 spec/lib/gitlab/ldap/access_spec.rb | 5 ++---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 5eed9cf91c7..592468933ed 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -205,6 +205,7 @@ class User < ActiveRecord::Base
 
     event :activate do
       transition blocked: :active
+      transition ldap_blocked: :active
     end
 
     state :blocked, :ldap_blocked do
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index ebd9260ad5d..a659d179b5f 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -40,9 +40,7 @@ module Gitlab
             user.ldap_block
             false
           else
-            if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
-              user.activate
-            end
+            user.activate if user.ldap_blocked?
             true
           end
         else
diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb
index f58d70e809c..32a19bf344b 100644
--- a/spec/lib/gitlab/ldap/access_spec.rb
+++ b/spec/lib/gitlab/ldap/access_spec.rb
@@ -42,7 +42,6 @@ describe Gitlab::LDAP::Access, lib: true do
 
       context 'and has no disabled flag in active diretory' do
         before do
-          user.block
           allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false)
         end
 
@@ -50,7 +49,7 @@ describe Gitlab::LDAP::Access, lib: true do
 
         context 'when auto-created users are blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true)
+            user.block
           end
 
           it 'does not unblock user in GitLab' do
@@ -62,7 +61,7 @@ describe Gitlab::LDAP::Access, lib: true do
 
         context 'when auto-created users are not blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false)
+            user.ldap_block
           end
 
           it 'should unblock user in GitLab' do
-- 
cgit v1.2.1