Merge branch 'admin-edit-identities' into 'master'

Admin can see, edit and remove user identities

Related to #1415 and https://dev.gitlab.org/gitlab/gitlabhq/issues/2224Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

See merge request !843

CHANGELOG

@@ -10,6 +10,7 @@ v 7.13.0 (unreleased)
   - Rename "Design" profile settings page to "Preferences".
   - Allow users to customize their default Dashboard page.
   - Update ssl_ciphers in Nginx example to remove DHE settings. This will deny forward secrecy for Android 2.3.7, Java 6 and OpenSSL 0.9.8
+  - Admin can edit and remove user identities
   - Convert CRLF newlines to LF when committing using the web editor.
   - API request /projects/:project_id/merge_requests?state=closed will return only closed merge requests without merged one. If you need ones that were merged - use state=merged.  
   - Allow Administrators to filter the user list by those with or without Two-factor Authentication enabled.

app/controllers/admin/identities_controller.rb

+class Admin::IdentitiesController < Admin::ApplicationController
+  before_action :user
+  before_action :identity, except: :index
+
+  def index
+    @identities = @user.identities
+  end
+
+  def edit
+  end
+
+  def update
+    if @identity.update_attributes(identity_params)
+      redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.'
+    else
+      render :edit
+    end
+  end
+
+  def destroy
+    if @identity.destroy
+      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.'
+    end
+  end
+
+  protected
+
+  def user
+    @user ||= User.find_by!(username: params[:user_id])
+  end
+
+  def identity
+    @identity ||= user.identities.find(params[:id])
+  end
+
+  def identity_params
+    params.require(:identity).permit(:provider, :extern_uid)
+  end
+end

app/controllers/admin/users_controller.rb

 class Admin::UsersController < Admin::ApplicationController
-  before_action :user, only: [:show, :edit, :update, :destroy]
+  before_action :user, except: [:index, :new, :create]
 
   def index
     @users = User.order_name_asc.filter(params[:filter])
@@ -9,8 +9,17 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def show
+  end
+
+  def projects
     @personal_projects = user.personal_projects
     @joined_projects = user.projects.joined(@user)
+  end
+
+  def groups
+  end
+
+  def keys
     @keys = user.keys
   end
 

app/views/admin/identities/_form.html.haml

+= form_for [:admin, @user, @identity], html: { class: 'form-horizontal fieldset-form' } do |f|
+  - if @identity.errors.any?
+    #error_explanation
+      .alert.alert-danger
+        - @identity.errors.full_messages.each do |msg|
+          %p= msg
+
+  .form-group
+    = f.label :provider, class: 'control-label'
+    .col-sm-10
+      = f.select :provider, Gitlab::OAuth::Provider.names, { allow_blank: false }, class: 'form-control'
+  .form-group
+    = f.label :extern_uid, "Identifier", class: 'control-label'
+    .col-sm-10
+      = f.text_field :extern_uid, required: true, class: 'form-control', required: true
+
+  .form-actions
+    = f.submit 'Save changes', class: "btn btn-save"
+

app/views/admin/identities/_identity.html.haml

+%tr
+  %td
+    = identity.provider
+  %td
+    = identity.extern_uid
+  %td
+    = link_to edit_admin_user_identity_path(@user, identity), class: 'btn btn-xs btn-grouped' do
+      Edit
+    = link_to [:admin, @user, identity], method: :delete,
+      class: 'btn btn-xs btn-danger',
+      data: { confirm: "Are you sure you want to remove this identity?" } do
+      Delete

app/views/admin/identities/edit.html.haml

+- page_title "Edit", @identity.provider, "Identities", @user.name, "Users"
+%h3.page-title
+  Edit identity for #{@user.name}
+%hr
+
+= render 'form'

app/views/admin/identities/index.html.haml

+- page_title "Identities", @user.name, "Users"
+= render 'admin/users/head'
+
+- if @identities.present?
+  %table.table
+    %thead
+      %tr
+        %th Provider
+        %th Identifier
+        %th
+    = render @identities
+- else
+  %h4 This user has no identities

app/views/admin/users/_head.html.haml

+%h3.page-title
+  = @user.name
+  - if @user.blocked?
+    %span.cred (Blocked)
+  - if @user.admin
+    %span.cred (Admin)
+
+  .pull-right
+    = link_to edit_admin_user_path(@user), class: "btn btn-grouped" do
+      %i.fa.fa-pencil-square-o
+      Edit
+%hr
+%ul.nav.nav-tabs
+  = nav_link(path: 'users#show') do
+    = link_to "Account", admin_user_path(@user)
+  = nav_link(path: 'users#groups') do
+    = link_to "Groups", groups_admin_user_path(@user)
+  = nav_link(path: 'users#projects') do
+    = link_to "Projects", projects_admin_user_path(@user)
+  = nav_link(path: 'users#keys') do
+    = link_to "SSH keys", keys_admin_user_path(@user)
+  = nav_link(controller: :identities) do
+    = link_to "Identities", admin_user_identities_path(@user)

app/views/admin/users/groups.html.haml

+- page_title "Groups", @user.name, "Users"
+= render 'admin/users/head'
+
+- if @user.group_members.present?
+  .panel.panel-default
+    .panel-heading Groups:
+    %ul.well-list
+      - @user.group_members.each do |group_member|
+        - group = group_member.group
+        %li.group_member
+          %span{class: ("list-item-name" unless group_member.owner?)}
+            %strong= link_to group.name, admin_group_path(group)
+          .pull-right
+            %span.light= group_member.human_access
+            - unless group_member.owner?
+              = link_to group_group_member_path(group, group_member), data: { confirm: remove_user_from_group_message(group, group_member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do
+                %i.fa.fa-times.fa-inverse
+- else
+  .nothing-here-block This user has no groups.

app/views/admin/users/keys.html.haml

+- page_title "Keys", @user.name, "Users"
+= render 'admin/users/head'
+= render 'profiles/keys/key_table', admin: true

app/views/admin/users/projects.html.haml

+- page_title "Projects", @user.name, "Users"
+= render 'admin/users/head'
+
+- if @user.groups.any?
+  .panel.panel-default
+    .panel-heading Group projects
+    %ul.well-list
+      - @user.groups.each do |group|
+        %li
+          %strong= group.name
+          – access to
+          #{pluralize(group.projects.count, 'project')}
+
+.row
+  .col-md-6
+    - if @personal_projects.present?
+      = render 'users/projects', projects: @personal_projects
+    - else
+      .nothing-here-block This user has no personal projects.
+
+
+  .col-md-6
+    .panel.panel-default
+      .panel-heading Joined projects (#{@joined_projects.count})
+      %ul.well-list
+        - @joined_projects.sort_by(&:name_with_namespace).each do |project|
+          - member = project.team.find_member(@user.id)
+          %li.project_member
+            .list-item-name
+              = link_to admin_namespace_project_path(project.namespace, project), class: dom_class(project) do
+                = project.name_with_namespace
+
+            - if member
+              .pull-right
+                - if member.owner?
+                  %span.light Owner
+                - else
+                  %span.light= member.human_access
+
+                  - if member.respond_to? :project
+                    = link_to namespace_project_project_member_path(project.namespace, project, member), data: { confirm: remove_from_project_team_message(project, member) }, remote: true, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from project' do
+                      %i.fa.fa-times
+

app/views/admin/users/show.html.haml

 - page_title @user.name, "Users"
-%h3.page-title
-  User:
-  = @user.name
-  - if @user.blocked?
-    %span.cred (Blocked)
-  - if @user.admin
-    %span.cred (Admin)
+= render 'admin/users/head'
 
-  .pull-right
-    = link_to edit_admin_user_path(@user), class: "btn btn-grouped" do
-      %i.fa.fa-pencil-square-o
-      Edit
-%hr
-%ul.nav.nav-tabs
-  %li.active
-    %a{"data-toggle" => "tab", href: "#account"} Account
-  %li
-    %a{"data-toggle" => "tab", href: "#profile"} Profile
-  %li
-    %a{"data-toggle" => "tab", href: "#groups"} Groups
+.row
+  .col-md-6
+    .panel.panel-default
+      .panel-heading
+        = @user.name
+      %ul.well-list
         %li
-    %a{"data-toggle" => "tab", href: "#projects"} Projects
+          = image_tag avatar_icon(@user.email, 60), class: "avatar s60"
         %li
-    %a{"data-toggle" => "tab", href: "#ssh-keys"} SSH keys
+          %span.light Profile page:
+          %strong
+            = link_to user_path(@user) do
+              = @user.username
+    = render 'users/profile', user: @user
 
-.tab-content
-  #account.tab-pane.active
-    .row
-      .col-md-6
     .panel.panel-default
       .panel-heading
         Account:
@@ -164,77 +152,3 @@
                 %strong #{@user.solo_owned_groups.map(&:name).join(', ')}
               %p
                 You must transfer ownership or delete these groups before you can delete this user.
-
-  #profile.tab-pane
-    .row
-      .col-md-6
-        .panel.panel-default
-          .panel-heading
-            = @user.name
-          %ul.well-list
-            %li
-              = image_tag avatar_icon(@user.email, 60), class: "avatar s60"
-            %li
-              %span.light Profile page:
-              %strong
-                = link_to user_path(@user) do
-                  = @user.username
-      .col-md-6
-        = render 'users/profile', user: @user
-
-  #groups.tab-pane
-    - if @user.group_members.present?
-      .panel.panel-default
-        .panel-heading Groups:
-        %ul.well-list
-          - @user.group_members.each do |group_member|
-            - group = group_member.group
-            %li.group_member
-              %span{class: ("list-item-name" unless group_member.owner?)}
-                %strong= link_to group.name, admin_group_path(group)
-              .pull-right
-                %span.light= group_member.human_access
-                - unless group_member.owner?
-                  = link_to group_group_member_path(group, group_member), data: { confirm: remove_user_from_group_message(group, group_member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do
-                    %i.fa.fa-times.fa-inverse
-    - else
-      .nothing-here-block This user has no groups.
-
-  #projects.tab-pane
-    - if @user.groups.any?
-      .panel.panel-default
-        .panel-heading Group projects
-        %ul.well-list
-          - @user.groups.each do |group|
-            %li
-              %strong= group.name
-              – access to
-              #{pluralize(group.projects.count, 'project')}
-
-    .row
-      .col-md-6
-        = render 'users/projects', projects: @personal_projects
-
-      .col-md-6
-        .panel.panel-default
-          .panel-heading Joined projects (#{@joined_projects.count})
-          %ul.well-list
-            - @joined_projects.sort_by(&:name_with_namespace).each do |project|
-              - member = project.team.find_member(@user.id)
-              %li.project_member
-                .list-item-name
-                  = link_to admin_namespace_project_path(project.namespace, project), class: dom_class(project) do
-                    = project.name_with_namespace
-
-                - if member
-                  .pull-right
-                    - if member.owner?
-                      %span.light Owner
-                    - else
-                      %span.light= member.human_access
-
-                      - if member.respond_to? :project
-                        = link_to namespace_project_project_member_path(project.namespace, project, member), data: { confirm: remove_from_project_team_message(project, member) }, remote: true, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from project' do
-                          %i.fa.fa-times
-  #ssh-keys.tab-pane
-    = render 'profiles/keys/key_table', admin: true

config/routes.rb

@@ -149,7 +149,12 @@ Gitlab::Application.routes.draw do
   namespace :admin do
     resources :users, constraints: { id: /[a-zA-Z.\/0-9_\-]+/ } do
       resources :keys, only: [:show, :destroy]
+      resources :identities, only: [:index, :edit, :update, :destroy]
+
       member do
+        get :projects
+        get :keys
+        get :groups
         put :team_update
         put :block
         put :unblock

features/admin/users.feature

@@ -28,7 +28,7 @@ Feature: Admin Users
     When I submit modified user
     Then I see user attributes changed
 
-@javascript
+  @javascript
   Scenario: Remove users secondary email
     Given I visit admin users page
     And I view the user with secondary email
@@ -40,8 +40,26 @@ Feature: Admin Users
     Given user "Pete" with ssh keys
     And I visit admin users page
     And click on user "Pete"
+    And click on ssh keys tab
     Then I should see key list
     And I click on the key title
     Then I should see key details
     And I click on remove key
     Then I should see the key removed
+
+  Scenario: Show user identities
+    Given user "Pete" with twitter account
+    And I visit "Pete" identities page in admin
+    Then I should see twitter details
+
+  Scenario: Update user identities
+    Given user "Pete" with twitter account
+    And I visit "Pete" identities page in admin
+    And I modify twitter identity
+    Then I should see twitter details updated
+
+  Scenario: Remove user identities
+    Given user "Pete" with twitter account
+    And I visit "Pete" identities page in admin
+    And I remove twitter identity
+    Then I should not see twitter details

features/steps/admin/users.rb

@@ -114,4 +114,45 @@ class Spinach::Features::AdminUsers < Spinach::FeatureSteps
   step 'I should see the key removed' do
     expect(page).not_to have_content 'ssh-rsa Key2'
   end
+
+  step 'user "Pete" with twitter account' do
+    @user = create(:user, name: 'Pete')
+    @user.identities.create!(extern_uid: '123456', provider: 'twitter')
+  end
+
+  step 'I visit "Pete" identities page in admin' do
+    allow(Gitlab::OAuth::Provider).to receive(:names).and_return(%w(twitter twitter_updated))
+    visit admin_user_identities_path(@user)
+  end
+
+  step 'I should see twitter details' do
+    expect(page).to have_content 'Pete'
+    expect(page).to have_content 'twitter'
+  end
+
+  step 'I modify twitter identity' do
+    find('.table').find(:link, 'Edit').click
+    fill_in 'identity_extern_uid', with: '654321'
+    select 'twitter_updated', from: 'identity_provider'
+    click_button 'Save changes'
+  end
+
+  step 'I should see twitter details updated' do
+    expect(page).to have_content 'Pete'
+    expect(page).to have_content 'twitter_updated'
+    expect(page).to have_content '654321'
+  end
+
+  step 'I remove twitter identity' do
+    click_link 'Delete'
+  end
+
+  step 'I should not see twitter details' do
+    expect(page).to have_content 'Pete'
+    expect(page).to_not have_content 'twitter'
+  end
+
+  step 'click on ssh keys tab' do
+    click_link 'SSH keys'
+  end
 end

lib/gitlab/o_auth/provider.rb

+module Gitlab
+  module OAuth
+    class Provider
+      def self.names
+        providers = []
+
+        Gitlab.config.ldap.servers.values.each do |server|
+          providers << server['provider_name']
+        end
+
+        Gitlab.config.omniauth.providers.each do |provider|
+          providers << provider['name']
+        end
+
+        providers
+      end
+    end
+  end
+end