From 846e581732e291f8927d04a5b1b40fe8f2688885 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 13:08:07 -0800
Subject: use a magic default :global symbol instead of nil

to make sure we mean the global permissions
---
 app/controllers/application_controller.rb  | 2 +-
 app/controllers/groups_controller.rb       | 2 +-
 app/models/ability.rb                      | 7 ++++---
 app/models/guest.rb                        | 2 +-
 app/models/user.rb                         | 4 ++--
 app/policies/base_policy.rb                | 9 +++++++--
 lib/api/helpers.rb                         | 4 ++--
 lib/api/users.rb                           | 2 +-
 lib/banzai/reference_parser/base_parser.rb | 2 +-
 lib/gitlab/allowable.rb                    | 2 +-
 10 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1c66c530cd2..9b381336fab 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -90,7 +90,7 @@ class ApplicationController < ActionController::Base
     current_application_settings.after_sign_out_path.presence || new_user_session_path
   end
 
-  def can?(object, action, subject)
+  def can?(object, action, subject = :global)
     Ability.allowed?(object, action, subject)
   end
 
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 4663b6e7fc6..05f9ee1ee90 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -118,7 +118,7 @@ class GroupsController < Groups::ApplicationController
   end
 
   def authorize_create_group!
-    unless can?(current_user, :create_group, nil)
+    unless can?(current_user, :create_group)
       return render_404
     end
   end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ad6c588202e..f3692a5a067 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -56,15 +56,16 @@ class Ability
       end
     end
 
-    def allowed?(user, action, subject)
+    def allowed?(user, action, subject = :global)
       allowed(user, subject).include?(action)
     end
 
-    def allowed(user, subject)
+    def allowed(user, subject = :global)
+      return BasePolicy::RuleSet.none if subject.nil?
       return uncached_allowed(user, subject) unless RequestStore.active?
 
       user_key = user ? user.id : 'anonymous'
-      subject_key = subject ? "#{subject.class.name}/#{subject.id}" : 'global'
+      subject_key = subject == :global ? 'global' : "#{subject.class.name}/#{subject.id}"
       key = "/ability/#{user_key}/#{subject_key}"
       RequestStore[key] ||= uncached_allowed(user, subject).freeze
     end
diff --git a/app/models/guest.rb b/app/models/guest.rb
index 01285ca1264..df287c277a7 100644
--- a/app/models/guest.rb
+++ b/app/models/guest.rb
@@ -1,6 +1,6 @@
 class Guest
   class << self
-    def can?(action, subject)
+    def can?(action, subject = :global)
       Ability.allowed?(nil, action, subject)
     end
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 76fb4cd470e..db3837ecdf2 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -563,14 +563,14 @@ class User < ActiveRecord::Base
   end
 
   def can_create_group?
-    can?(:create_group, nil)
+    can?(:create_group)
   end
 
   def can_select_namespace?
     several_namespaces? || admin
   end
 
-  def can?(action, subject)
+  def can?(action, subject = :global)
     Ability.allowed?(self, action, subject)
   end
 
diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb
index e07b144355a..8890409d056 100644
--- a/app/policies/base_policy.rb
+++ b/app/policies/base_policy.rb
@@ -12,6 +12,10 @@ class BasePolicy
       new(Set.new, Set.new)
     end
 
+    def self.none
+      empty.freeze
+    end
+
     def can?(ability)
       @can_set.include?(ability) && !@cannot_set.include?(ability)
     end
@@ -49,7 +53,8 @@ class BasePolicy
   end
 
   def self.class_for(subject)
-    return GlobalPolicy if subject.nil?
+    return GlobalPolicy if subject == :global
+    raise ArgumentError, 'no policy for nil' if subject.nil?
 
     if subject.class.try(:presenter?)
       subject = subject.subject
@@ -79,7 +84,7 @@ class BasePolicy
   end
 
   def abilities
-    return RuleSet.empty if @user && @user.blocked?
+    return RuleSet.none if @user && @user.blocked?
     return anonymous_abilities if @user.nil?
     collect_rules { rules }
   end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index a9b364da9e1..e5f5de2af57 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -116,7 +116,7 @@ module API
       forbidden! unless current_user.is_admin?
     end
 
-    def authorize!(action, subject = nil)
+    def authorize!(action, subject = :global)
       forbidden! unless can?(current_user, action, subject)
     end
 
@@ -134,7 +134,7 @@ module API
       end
     end
 
-    def can?(object, action, subject)
+    def can?(object, action, subject = :global)
       Ability.allowed?(object, action, subject)
     end
 
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 549003f576a..2d4d5a25221 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -45,7 +45,7 @@ module API
         use :pagination
       end
       get do
-        unless can?(current_user, :read_users_list, nil)
+        unless can?(current_user, :read_users_list)
           render_api_error!("Not authorized.", 403)
         end
 
diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb
index 2058a58d0ae..b121c37c5d0 100644
--- a/lib/banzai/reference_parser/base_parser.rb
+++ b/lib/banzai/reference_parser/base_parser.rb
@@ -210,7 +210,7 @@ module Banzai
           grouped_objects_for_nodes(nodes, Project, 'data-project')
       end
 
-      def can?(user, permission, subject)
+      def can?(user, permission, subject = :global)
         Ability.allowed?(user, permission, subject)
       end
 
diff --git a/lib/gitlab/allowable.rb b/lib/gitlab/allowable.rb
index f48abcc86d5..e4f7cad2b79 100644
--- a/lib/gitlab/allowable.rb
+++ b/lib/gitlab/allowable.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Allowable
-    def can?(user, action, subject)
+    def can?(user, action, subject = :global)
       Ability.allowed?(user, action, subject)
     end
   end
-- 
cgit v1.2.1


From d9cfed07cd1048328499ae06807368be29bdeb2c Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 13:09:23 -0800
Subject: add User#internal? and some global permissions

---
 app/models/user.rb            | 5 ++++-
 app/policies/global_policy.rb | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index db3837ecdf2..fa6f336f6b5 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -350,12 +350,15 @@ class User < ActiveRecord::Base
     def ghost
       unique_internal(where(ghost: true), 'ghost', 'ghost%s@example.com') do |u|
         u.bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.'
-        u.state = :blocked
         u.name = 'Ghost User'
       end
     end
   end
 
+  def internal?
+    ghost?
+  end
+
   #
   # Instance methods
   #
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index 3c2fbe6b56b..a1946584dd0 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -4,5 +4,11 @@ class GlobalPolicy < BasePolicy
 
     can! :create_group if @user.can_create_group
     can! :read_users_list
+
+    unless @user.blocked? || @user.internal?
+      can! :log_in
+      can! :access_api
+      can! :access_git
+    end
   end
 end
-- 
cgit v1.2.1


From 0ea04cc5bfcc125875a6e0f46702389f0e2e19c0 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 13:19:52 -0800
Subject: use the policy stack to protect logins

---
 app/controllers/application_controller.rb                 | 2 +-
 app/controllers/concerns/authenticates_with_two_factor.rb | 7 +++----
 app/policies/global_policy.rb                             | 2 +-
 lib/api/helpers.rb                                        | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9b381336fab..b7ce081a5cd 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base
     token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence
     user = User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string)
 
-    if user
+    if user && can?(user, :log_in)
       # Notice we are passing store false, so the user is not
       # actually stored in the session and a token is needed
       # for every request. If you want the token to work as a
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index 4c497711fc0..ea441b1736b 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -23,7 +23,7 @@ module AuthenticatesWithTwoFactor
   #
   # Returns nil
   def prompt_for_two_factor(user)
-    return locked_user_redirect(user) if user.access_locked?
+    return locked_user_redirect(user) unless user.can?(:log_in)
 
     session[:otp_user_id] = user.id
     setup_u2f_authentication(user)
@@ -37,10 +37,9 @@ module AuthenticatesWithTwoFactor
 
   def authenticate_with_two_factor
     user = self.resource = find_user
+    return locked_user_redirect(user) unless user.can?(:log_in)
 
-    if user.access_locked?
-      locked_user_redirect(user)
-    elsif user_params[:otp_attempt].present? && session[:otp_user_id]
+    if user_params[:otp_attempt].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_otp(user)
     elsif user_params[:device_response].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_u2f(user)
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index a1946584dd0..e1a527b95bf 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -6,7 +6,7 @@ class GlobalPolicy < BasePolicy
     can! :read_users_list
 
     unless @user.blocked? || @user.internal?
-      can! :log_in
+      can! :log_in unless @user.access_locked?
       can! :access_api
       can! :access_git
     end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index e5f5de2af57..bd22b82476b 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -97,7 +97,7 @@ module API
     end
 
     def authenticate!
-      unauthorized! unless current_user
+      unauthorized! unless current_user && can?(current_user, :access_api)
     end
 
     def authenticate_non_get!
-- 
cgit v1.2.1


From dfe41c1556a5e31480a230e13033dd523ef51ba3 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 13:35:37 -0800
Subject: protect internal users from impersonation

---
 app/controllers/admin/users_controller.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 7ffde71c3b1..7f86723b921 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -32,6 +32,10 @@ class Admin::UsersController < Admin::ApplicationController
     if user.blocked?
       flash[:alert] = "You cannot impersonate a blocked user"
 
+      redirect_to admin_user_path(user)
+    elsif user.internal?
+      flash[:alert] = "You cannot impersonate an internal user"
+
       redirect_to admin_user_path(user)
     else
       session[:impersonator_id] = current_user.id
-- 
cgit v1.2.1


From 145f6fd0b9bb2428ee9f5445efe7cb0b91ca8712 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 13:37:35 -0800
Subject: protect git access through the policy infra

---
 lib/gitlab/user_access.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 6ce9b229294..3078b134bb3 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -63,7 +63,7 @@ module Gitlab
     private
 
     def no_user_or_blocked?
-      user.nil? || user.blocked?
+      user.nil? || !user.can?(:access_git)
     end
   end
 end
-- 
cgit v1.2.1


From 153d3e57285f2526bc5371dc7109ac9be07ae132 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 13:38:01 -0800
Subject: don't require passwords for internal users

since they can't log in
---
 app/models/user.rb | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index fa6f336f6b5..d725d37a355 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -958,6 +958,14 @@ class User < ActiveRecord::Base
     self.admin = (new_level == 'admin')
   end
 
+  protected
+
+  # override, from Devise::Validatable
+  def password_required?
+    return false if internal?
+    super
+  end
+
   private
 
   def ci_projects_union
@@ -1058,7 +1066,6 @@ class User < ActiveRecord::Base
 
     scope.create(
       username: username,
-      password: Devise.friendly_token,
       email: email,
       &creation_block
     )
-- 
cgit v1.2.1


From bb0cba920ae5ce46ecc80907c445ddc85f3b4ed1 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 14:15:27 -0800
Subject: don't require ghost users to be blocked

---
 app/models/user.rb       |  7 -------
 spec/models/user_spec.rb | 16 ----------------
 2 files changed, 23 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index d725d37a355..520baf6e659 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -126,7 +126,6 @@ class User < ActiveRecord::Base
   validate :unique_email, if: ->(user) { user.email_changed? }
   validate :owns_notification_email, if: ->(user) { user.notification_email_changed? }
   validate :owns_public_email, if: ->(user) { user.public_email_changed? }
-  validate :ghost_users_must_be_blocked
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   before_validation :generate_password, on: :create
@@ -455,12 +454,6 @@ class User < ActiveRecord::Base
     errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
   end
 
-  def ghost_users_must_be_blocked
-    if ghost? && !blocked?
-      errors.add(:ghost, 'cannot be enabled for a user who is not blocked.')
-    end
-  end
-
   def update_emails_with_primary_email
     primary_email_record = emails.find_by(email: email)
     if primary_email_record
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index adb5b538922..e1dafe3aa72 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -210,22 +210,6 @@ describe User, models: true do
         end
       end
     end
-
-    describe 'ghost users' do
-      it 'does not allow a non-blocked ghost user' do
-        user = build(:user, :ghost)
-        user.state = 'active'
-
-        expect(user).to be_invalid
-      end
-
-      it 'allows a blocked ghost user' do
-        user = build(:user, :ghost)
-        user.state = 'blocked'
-
-        expect(user).to be_valid
-      end
-    end
   end
 
   describe "scopes" do
-- 
cgit v1.2.1


From f7a111e97656a441e1c78e9ed81d4212f46dfb23 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 28 Feb 2017 15:24:04 -0800
Subject: use policies to protect sending email

---
 app/policies/global_policy.rb        | 1 +
 app/services/notification_service.rb | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index e1a527b95bf..cb72c2b4590 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -9,6 +9,7 @@ class GlobalPolicy < BasePolicy
       can! :log_in unless @user.access_locked?
       can! :access_api
       can! :access_git
+      can! :receive_notifications
     end
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index fbad85d310e..d12692ecc90 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -465,7 +465,7 @@ class NotificationService
     end
 
     users = users.to_a.compact.uniq
-    users = users.reject(&:blocked?)
+    users = users.select { |u| u.can?(:receive_notifications) }
 
     users.reject do |user|
       global_notification_setting = user.global_notification_setting
-- 
cgit v1.2.1


From a5c05544cf572613cebe2249cd7bb97b2ef508e7 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Wed, 1 Mar 2017 11:39:18 -0800
Subject: fix a brittle stub

true is neither nil nor a user and doesn't make sense as the return
value of `current_user`
---
 spec/requests/api/helpers_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index a89676fec93..988a57a80ea 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -436,7 +436,7 @@ describe API::Helpers, api: true do
 
     context 'current_user is present' do
       before do
-        expect_any_instance_of(self.class).to receive(:current_user).and_return(true)
+        expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(User.new)
       end
 
       it 'does not raise an error' do
-- 
cgit v1.2.1


From f1d3a92bd64e9a503335d089057cca559a74b5ef Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Thu, 2 Mar 2017 17:43:05 -0800
Subject: spec the behavior of nil subjects

---
 spec/models/ability_spec.rb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index 30f8fdf91b2..92d70cfc64c 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -1,6 +1,12 @@
 require 'spec_helper'
 
 describe Ability, lib: true do
+  context 'using a nil subject' do
+    it 'is always empty' do
+      expect(Ability.allowed(nil, nil).to_set).to be_empty
+    end
+  end
+
   describe '.can_edit_note?' do
     let(:project) { create(:empty_project) }
     let(:note) { create(:note_on_issue, project: project) }
-- 
cgit v1.2.1


From 275a46c52338ab3bfb4da73431465d742060e3ea Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Thu, 2 Mar 2017 17:49:23 -0800
Subject: spec the new behavior of .class_for

and more robustly spec the ancestor behavior
---
 spec/policies/base_policy_spec.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb
index 63acc0b68cd..02acdcb36df 100644
--- a/spec/policies/base_policy_spec.rb
+++ b/spec/policies/base_policy_spec.rb
@@ -1,17 +1,19 @@
 require 'spec_helper'
 
 describe BasePolicy, models: true do
-  let(:build) { Ci::Build.new }
-
   describe '.class_for' do
     it 'detects policy class based on the subject ancestors' do
-      expect(described_class.class_for(build)).to eq(Ci::BuildPolicy)
+      expect(described_class.class_for(GenericCommitStatus.new)).to eq(CommitStatusPolicy)
     end
 
     it 'detects policy class for a presented subject' do
-      presentee = Ci::BuildPresenter.new(build)
+      presentee = Ci::BuildPresenter.new(Ci::Build.new)
 
       expect(described_class.class_for(presentee)).to eq(Ci::BuildPolicy)
     end
+
+    it 'uses GlobalPolicy when :global is given' do
+      expect(described_class.class_for(:global)).to eq(GlobalPolicy)
+    end
   end
 end
-- 
cgit v1.2.1


From 8f057a5109687016fe72c6512fa0d4ea2354731f Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 7 Mar 2017 15:27:59 -0800
Subject: add a spec that the ghost user cannot log in

---
 spec/features/login_spec.rb | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index ae609160e18..f32d1f78b40 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -48,6 +48,18 @@ feature 'Login', feature: true do
     end
   end
 
+  describe 'with the ghost user' do
+    it 'disallows login' do
+      login_with(User.ghost)
+
+      expect(page).to have_content('Invalid Login or password.')
+    end
+
+    it 'does not update Devise trackable attributes' do
+      expect { login_with(User.ghost) }.not_to change { User.ghost.reload.sign_in_count }
+    end
+  end
+
   describe 'with two-factor authentication' do
     def enter_code(code)
       fill_in 'user_otp_attempt', with: code
-- 
cgit v1.2.1


From b88314f4ad955897dc737b6e9515b43dc9d97422 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 7 Mar 2017 19:05:01 -0800
Subject: consolidate the error handling for #impersonate

---
 app/controllers/admin/users_controller.rb | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 7f86723b921..205cf0490ab 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -29,15 +29,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def impersonate
-    if user.blocked?
-      flash[:alert] = "You cannot impersonate a blocked user"
-
-      redirect_to admin_user_path(user)
-    elsif user.internal?
-      flash[:alert] = "You cannot impersonate an internal user"
-
-      redirect_to admin_user_path(user)
-    else
+    if !can?(user, :log_in)
       session[:impersonator_id] = current_user.id
 
       warden.set_user(user, scope: :user)
@@ -47,6 +39,17 @@ class Admin::UsersController < Admin::ApplicationController
       flash[:alert] = "You are now impersonating #{user.username}"
 
       redirect_to root_path
+    else
+      flash[:alert] =
+        if user.blocked?
+          "You cannot impersonate a blocked user"
+        elsif user.internal?
+          "You cannot impersonate an internal user"
+        else
+          "You cannot impersonate a user who cannot log in"
+        end
+
+      redirect_to admin_user_path(user)
     end
   end
 
-- 
cgit v1.2.1


From ef4ba8d3ce32d4e8af34ac86b64e5cf7a85ed926 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Tue, 7 Mar 2017 19:06:13 -0800
Subject: add .internal and .non_internal scopes

---
 app/models/user.rb | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 520baf6e659..1081f103ea6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -354,8 +354,20 @@ class User < ActiveRecord::Base
     end
   end
 
+  def self.internal_attributes
+    [:ghost]
+  end
+
   def internal?
-    ghost?
+    self.class.internal_attributes.any? { |a| self[a] }
+  end
+
+  def self.internal
+    where(Hash[internal_attributes.zip([true] * internal_attributes.size)])
+  end
+
+  def self.non_internal
+    where(Hash[internal_attributes.zip([false] * internal_attributes.size)])
   end
 
   #
-- 
cgit v1.2.1


From 66f204e0f0dd74df3409547bea2cec98c8947f2c Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Thu, 9 Mar 2017 12:02:56 -0800
Subject: get the logic right :X

---
 app/controllers/admin/users_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 205cf0490ab..24504685e48 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -29,7 +29,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def impersonate
-    if !can?(user, :log_in)
+    if can?(user, :log_in)
       session[:impersonator_id] = current_user.id
 
       warden.set_user(user, scope: :user)
-- 
cgit v1.2.1


From 90d924dc390693892a596659aa1a38b0432f6e40 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Thu, 9 Mar 2017 13:59:19 -0800
Subject: reverse the logic and use a clearer name

---
 lib/gitlab/user_access.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 3078b134bb3..f260c0c535f 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -8,7 +8,7 @@ module Gitlab
     end
 
     def can_do_action?(action)
-      return false if no_user_or_blocked?
+      return false unless can_access_git?
 
       @permission_cache ||= {}
       @permission_cache[action] ||= user.can?(action, project)
@@ -19,7 +19,7 @@ module Gitlab
     end
 
     def allowed?
-      return false if no_user_or_blocked?
+      return false unless can_access_git?
 
       if user.requires_ldap_check? && user.try_obtain_ldap_lease
         return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -29,7 +29,7 @@ module Gitlab
     end
 
     def can_push_to_branch?(ref)
-      return false if no_user_or_blocked?
+      return false unless can_access_git?
 
       if project.protected_branch?(ref)
         return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user)
@@ -44,7 +44,7 @@ module Gitlab
     end
 
     def can_merge_to_branch?(ref)
-      return false if no_user_or_blocked?
+      return false unless can_access_git?
 
       if project.protected_branch?(ref)
         access_levels = project.protected_branches.matching(ref).map(&:merge_access_levels).flatten
@@ -55,15 +55,15 @@ module Gitlab
     end
 
     def can_read_project?
-      return false if no_user_or_blocked?
+      return false unless can_access_git?
 
       user.can?(:read_project, project)
     end
 
     private
 
-    def no_user_or_blocked?
-      user.nil? || !user.can?(:access_git)
+    def can_access_git?
+      user && user.can?(:access_git)
     end
   end
 end
-- 
cgit v1.2.1


From 00cf529a8dfb87dfdb0cc489a69c0043c646b345 Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Thu, 9 Mar 2017 14:06:05 -0800
Subject: mark internal users in the admin panel

and don't offer to impersonate them
---
 app/views/admin/users/_head.html.haml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml
index d20be373564..be41c33b853 100644
--- a/app/views/admin/users/_head.html.haml
+++ b/app/views/admin/users/_head.html.haml
@@ -2,11 +2,13 @@
   = @user.name
   - if @user.blocked?
     %span.cred (Blocked)
+  - if @user.internal?
+    %span.cred (Internal)
   - if @user.admin
     %span.cred (Admin)
 
   .pull-right
-    - unless @user == current_user || @user.blocked?
+    - if @user != current_user && @user.can?(:log_in)
       = link_to 'Impersonate', impersonate_admin_user_path(@user), method: :post, class: "btn btn-nr btn-grouped btn-info"
     = link_to edit_admin_user_path(@user), class: "btn btn-nr btn-grouped" do
       %i.fa.fa-pencil-square-o
-- 
cgit v1.2.1


From 90e11fb272cd30e7e61be16d862830f2b69a624a Mon Sep 17 00:00:00 2001
From: "http://jneen.net/" <jneen@jneen.net>
Date: Fri, 10 Mar 2017 11:39:54 -0800
Subject: allow internal attributes to have a nil value

---
 app/models/user.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 1081f103ea6..39c1281179b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -367,7 +367,7 @@ class User < ActiveRecord::Base
   end
 
   def self.non_internal
-    where(Hash[internal_attributes.zip([false] * internal_attributes.size)])
+    where(Hash[internal_attributes.zip([[false, nil]] * internal_attributes.size)])
   end
 
   #
-- 
cgit v1.2.1