Merge branch 'feature/ldap-sync-edgecases' into 'master'

LDAP Sync blocked user edgecases

Allow GitLab admins to block otherwise valid GitLab LDAP users
(https://gitlab.com/gitlab-org/gitlab-ce/issues/3462)

Based on the discussion on the original issue, we are going to differentiate "normal" block operations to the ldap automatic ones in order to make some decisions when its one or the other.

Expected behavior:

- [x] "ldap_blocked" users respond to both `blocked?` and `ldap_blocked?`
- [x] "ldap_blocked" users can't be unblocked by the Admin UI
- [x] "ldap_blocked" users can't be unblocked by the API
- [x] Block operations that are originated from LDAP synchronization will flag user as "ldap_blocked"
- [x] Only "ldap_blocked" users will be automatically unblocked by LDAP synchronization
- [x] When LDAP identity is removed, we should convert `ldap_blocked` into `blocked`
 
Mockup for the Admin UI with both "ldap_blocked" and normal "blocked" users:
![image](/uploads/4f56fc17b73cb2c9e2a154a22e7ad291/image.png)

There will be another MR for the EE version.

See merge request !2242

7 files changed, 60 insertions, 12 deletions

diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index 97a94638847..bb29829b7a1 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -131,6 +131,12 @@
     &:last-child {
       margin-right: 0px;
     }
+    &.btn-xs {
+      margin-right: 3px;
+    }
+  }
+  &.disabled {
+    pointer-events: auto !important;
   }
 }
 
diff --git a/app/controllers/admin/identities_controller.rb b/app/controllers/admin/identities_controller.rb
index e383fe38ea6..79a53556f0a 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).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).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/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/models/identity.rb b/app/models/identity.rb
index 8bcdc194953..e1915b079d4 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 ldap?
+    provider.starts_with?('ldap')
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 46b36c605b0..592468933ed 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -196,10 +196,22 @@ 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
+      transition active: :ldap_blocked
     end
 
     event :activate do
       transition blocked: :active
+      transition ldap_blocked: :active
+    end
+
+    state :blocked, :ldap_blocked do
+      def blocked?
+        true
+      end
     end
   end
 
@@ -207,7 +219,7 @@ class User < ActiveRecord::Base
 
   # 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/app/services/repair_ldap_blocked_user_service.rb b/app/services/repair_ldap_blocked_user_service.rb
new file mode 100644
index 00000000000..863cef7ff61
--- /dev/null
+++ b/app/services/repair_ldap_blocked_user_service.rb
@@ -0,0 +1,17 @@
+class RepairLdapBlockedUserService
+  attr_accessor :user
+
+  def initialize(user)
+    @user = user
+  end
+
+  def execute
+    user.block if ldap_hard_blocked?
+  end
+
+  private
+
+  def ldap_hard_blocked?
+    user.ldap_blocked? && !user.ldap_user?
+  end
+end
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index a92c9c152b9..8312642b6c3 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -88,14 +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.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"