From 516b4c124831296a9280b9f04abcd865492eea1e Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Fri, 19 Jun 2015 16:31:36 -0400
Subject: [PATCH] Allow Admin to filter users by 2FA status
---
app/models/user.rb | 17 ++++++---
app/views/admin/users/index.html.haml | 8 +++++
spec/features/admin/admin_users_spec.rb | 40 +++++++++++++++++++++
spec/models/user_spec.rb | 46 +++++++++++++++++++------
4 files changed, 97 insertions(+), 14 deletions(-)
diff --git a/app/models/user.rb b/app/models/user.rb
index a2e2d220b3a..57a36a73ac5 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -193,11 +193,13 @@ class User < ActiveRecord::Base
mount_uploader :avatar, AvatarUploader
# Scopes
- scope :admins, -> { where(admin: true) }
+ scope :admins, -> { where(admin: true) }
scope :blocked, -> { with_state(: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)') }
+ scope :with_two_factor, -> { where('otp_required_for_login IS true') }
+ scope :without_two_factor, -> { where('otp_required_for_login IS NOT true') }
#
# Class methods
@@ -231,9 +233,16 @@ class User < ActiveRecord::Base
def filter(filter_name)
case filter_name
- when "admins"; self.admins
- when "blocked"; self.blocked
- when "wop"; self.without_projects
+ when 'admins'
+ self.admins
+ when 'blocked'
+ self.blocked
+ when 'two_factor_disabled'
+ self.without_two_factor
+ when 'two_factor_enabled'
+ self.with_two_factor
+ when 'wop'
+ self.without_projects
else
self.active
end
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index 45dee86b017..9c1bec7c84d 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -13,6 +13,14 @@
= link_to admin_users_path(filter: "admins") do
Admins
%small.pull-right= User.admins.count
+ %li.filter-two-factor-enabled{class: "#{'active' if params[:filter] == 'two_factor_enabled'}"}
+ = link_to admin_users_path(filter: 'two_factor_enabled') do
+ 2FA Enabled
+ %small.pull-right= User.with_two_factor.count
+ %li.filter-two-factor-disabled{class: "#{'active' if params[:filter] == 'two_factor_disabled'}"}
+ = link_to admin_users_path(filter: 'two_factor_disabled') do
+ 2FA Disabled
+ %small.pull-right= User.without_two_factor.count
%li{class: "#{'active' if params[:filter] == "blocked"}"}
= link_to admin_users_path(filter: "blocked") do
Blocked
diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb
index 7f5cb30cb94..86717761582 100644
--- a/spec/features/admin/admin_users_spec.rb
+++ b/spec/features/admin/admin_users_spec.rb
@@ -16,6 +16,46 @@ describe "Admin::Users", feature: true do
expect(page).to have_content(@user.email)
expect(page).to have_content(@user.name)
end
+
+ describe 'Two-factor Authentication filters' do
+ it 'counts users who have enabled 2FA' do
+ create(:user, two_factor_enabled: true)
+
+ visit admin_users_path
+
+ page.within('.filter-two-factor-enabled small') do
+ expect(page).to have_content('1')
+ end
+ end
+
+ it 'filters by users who have enabled 2FA' do
+ user = create(:user, two_factor_enabled: true)
+
+ visit admin_users_path
+ click_link '2FA Enabled'
+
+ expect(page).to have_content(user.email)
+ end
+
+ it 'counts users who have not enabled 2FA' do
+ create(:user, two_factor_enabled: false)
+
+ visit admin_users_path
+
+ page.within('.filter-two-factor-disabled small') do
+ expect(page).to have_content('2') # Including admin
+ end
+ end
+
+ it 'filters by users who have not enabled 2FA' do
+ user = create(:user, two_factor_enabled: false)
+
+ visit admin_users_path
+ click_link '2FA Disabled'
+
+ expect(page).to have_content(user.email)
+ end
+ end
end
describe "GET /admin/users/new" do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index fa7680fbbec..d86401c9fbb 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -308,18 +308,44 @@ describe User do
end
end
- describe 'filter' do
- before do
- User.delete_all
- @user = create :user
- @admin = create :user, admin: true
- @blocked = create :user, state: :blocked
+ describe '.filter' do
+ let(:user) { double }
+
+ it 'filters by active users by default' do
+ expect(User).to receive(:active).and_return([user])
+
+ expect(User.filter(nil)).to include user
end
- it { expect(User.filter("admins")).to eq([@admin]) }
- it { expect(User.filter("blocked")).to eq([@blocked]) }
- it { expect(User.filter("wop")).to include(@user, @admin, @blocked) }
- it { expect(User.filter(nil)).to include(@user, @admin) }
+ it 'filters by admins' do
+ expect(User).to receive(:admins).and_return([user])
+
+ expect(User.filter('admins')).to include user
+ end
+
+ it 'filters by blocked' do
+ expect(User).to receive(:blocked).and_return([user])
+
+ expect(User.filter('blocked')).to include user
+ end
+
+ it 'filters by two_factor_disabled' do
+ expect(User).to receive(:without_two_factor).and_return([user])
+
+ expect(User.filter('two_factor_disabled')).to include user
+ end
+
+ it 'filters by two_factor_enabled' do
+ expect(User).to receive(:with_two_factor).and_return([user])
+
+ expect(User.filter('two_factor_enabled')).to include user
+ end
+
+ it 'filters by wop' do
+ expect(User).to receive(:without_projects).and_return([user])
+
+ expect(User.filter('wop')).to include user
+ end
end
describe :not_in_project do
--
GitLab