Add 2FA filter to group members page

* Show 2fa badge on a group members page
* Make group members page UI consistent with project members page
* Fix ambiguous sql in User.with/without_two_factor methods

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

6 files changed, 56 insertions, 15 deletions

diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 134b0dfc0db..ef3eba80154 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -11,13 +11,20 @@ class Groups::GroupMembersController < Groups::ApplicationController
                                   :override
 
   def index
+    can_manage_members = can?(current_user, :admin_group_member, @group)
+
     @sort = params[:sort].presence || sort_value_name
     @project = @group.projects.find(params[:project_id]) if params[:project_id]
 
     @members = GroupMembersFinder.new(@group).execute
-    @members = @members.non_invite unless can?(current_user, :admin_group, @group)
+    @members = @members.non_invite unless can_manage_members
     @members = @members.search(params[:search]) if params[:search].present?
     @members = @members.sort_by_attribute(@sort)
+
+    if can_manage_members && params[:two_factor].present?
+      @members = @members.filter_by_2fa(params[:two_factor])
+    end
+
     @members = @members.page(params[:page]).per(50)
     @members = present_members(@members.includes(:user))
 
diff --git a/app/models/member.rb b/app/models/member.rb
index eac4a22a03f..68572f2e33a 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -96,6 +96,17 @@ class Member < ActiveRecord::Base
       joins(:user).merge(User.search(query))
     end
 
+    def filter_by_2fa(value)
+      case value
+      when 'enabled'
+        left_join_users.merge(User.with_two_factor_indistinct)
+      when 'disabled'
+        left_join_users.merge(User.without_two_factor)
+      else
+        all
+      end
+    end
+
     def sort_by_attribute(method)
       case method.to_s
       when 'access_level_asc' then reorder(access_level: :asc)
diff --git a/app/models/user.rb b/app/models/user.rb
index a9cfd39f604..d74d5aade5a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -237,14 +237,18 @@ class User < ActiveRecord::Base
   scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) }
   scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) }
 
-  def self.with_two_factor
+  def self.with_two_factor_indistinct
     joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id")
-      .where("u2f.id IS NOT NULL OR otp_required_for_login = ?", true).distinct(arel_table[:id])
+      .where("u2f.id IS NOT NULL OR users.otp_required_for_login = ?", true)
+  end
+
+  def self.with_two_factor
+    with_two_factor_indistinct.distinct(arel_table[:id])
   end
 
   def self.without_two_factor
     joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id")
-      .where("u2f.id IS NULL AND otp_required_for_login = ?", false)
+      .where("u2f.id IS NULL AND users.otp_required_for_login = ?", false)
   end
 
   #
diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml
index ad9d5562ded..c8addc49117 100644
--- a/app/views/groups/group_members/index.html.haml
+++ b/app/views/groups/group_members/index.html.haml
@@ -1,10 +1,11 @@
 - page_title "Members"
+- can_manage_members = can?(current_user, :admin_group_member, @group)
 
 .project-members-page.prepend-top-default
   %h4
     Members
   %hr
-  - if can?(current_user, :admin_group_member, @group)
+  - if can_manage_members
     .project-members-new.append-bottom-default
       %p.clearfix
         Add new member to
@@ -13,20 +14,23 @@
 
     = render 'shared/members/requests', membership_source: @group, requesters: @requesters
 
-  .append-bottom-default.clearfix
+  .clearfix
     %h5.member.existing-title
       Existing members
-    = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form'  do
-      .form-group
-        = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false }
-        %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" }
-          = icon("search")
-        = render 'shared/members/sort_dropdown'
   .panel.panel-default
-    .panel-heading
-      Members with access to
-      %strong= @group.name
+    .panel-heading.flex-project-members-panel
+      %span.flex-project-title
+        Members with access to
+        %strong= @group.name
       %span.badge= @members.total_count
+      = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form flex-project-members-form'  do
+        .form-group
+          = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false }
+          %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" }
+            = icon("search")
+          - if can_manage_members
+            = render 'shared/members/filter_2fa_dropdown'
+          = render 'shared/members/sort_dropdown'
     %ul.content-list.members-list
       = render partial: 'shared/members/member', collection: @members, as: :member
     = paginate @members, theme: 'gitlab'
diff --git a/app/views/shared/members/_filter_2fa_dropdown.html.haml b/app/views/shared/members/_filter_2fa_dropdown.html.haml
new file mode 100644
index 00000000000..95c35c56b3c
--- /dev/null
+++ b/app/views/shared/members/_filter_2fa_dropdown.html.haml
@@ -0,0 +1,11 @@
+- filter = params[:two_factor] || 'everyone'
+- filter_options = { 'everyone' => 'Everyone', 'enabled' => 'Enabled', 'disabled' => 'Disabled' }
+.dropdown.inline.member-filter-2fa-dropdown
+  = dropdown_toggle('2FA: ' + filter_options[filter], { toggle: 'dropdown' })
+  %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable
+    %li.dropdown-header
+      Filter by two-factor authentication
+    - filter_options.each do |value, title|
+      %li
+        = link_to filter_group_project_member_path(two_factor: value), class: ("is-active" if filter == value) do
+          = title
diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml
index 1c139827acf..1961ad6d616 100644
--- a/app/views/shared/members/_member.html.haml
+++ b/app/views/shared/members/_member.html.haml
@@ -20,6 +20,10 @@
           %label.label.label-danger
             %strong Blocked
 
+        - if user.two_factor_enabled?
+          %label.label.label-info
+            2FA
+
         - if source.instance_of?(Group) && source != @group
           &middot;
           = link_to source.full_name, source, class: "member-group-link"