Create an external users tab on Admin user list

Also incorporates the review into this, mainly spec changes.

8 files changed, 57 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 07b187b31a2..f9c6f6e23e9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -30,6 +30,7 @@ v 8.6.0 (unreleased)
   - Add main language of a project in the list of projects (Tiago Botelho)
   - Add ability to show archived projects on dashboard, explore and group pages
   - Move group activity to separate page
+  - Create external users which are excluded of internal and private projects unless access was explicitly granted
 
 v 8.5.5
   - Ensure removing a project removes associated Todo entries
diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index 711d9019a3d..4a6c2fbd71c 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -40,18 +40,23 @@ class ProjectsFinder
   private
 
   def group_projects(current_user, group)
-    if current_user
+    return [group.projects.public_only] unless current_user
+
+    if current_user.external?
       [
         group_projects_for_user(current_user, group),
-        group.projects.public_and_internal_only
+        group.projects.public_only
       ]
     else
-      [group.projects.public_only]
+      [
+        group_projects_for_user(current_user, group),
+        group.projects.public_and_internal_only
+      ]
     end
   end
 
   def all_projects(current_user)
-    return [Project.public_only] unless current_user
+    return [public_projects] unless current_user
 
     if current_user.external?
       [current_user.authorized_projects, public_projects]
diff --git a/app/models/user.rb b/app/models/user.rb
index adff238c60f..34f1cfc0be6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -229,6 +229,7 @@ class User < ActiveRecord::Base
   # Scopes
   scope :admins, -> { where(admin: true) }
   scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
+  scope :external, -> { where(external: true) }
   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)') }
@@ -284,6 +285,8 @@ class User < ActiveRecord::Base
         self.with_two_factor
       when 'wop'
         self.without_projects
+      when 'external'
+        self.external
       else
         self.active
       end
@@ -855,9 +858,7 @@ class User < ActiveRecord::Base
   def ensure_external_user_rights
     return unless self.external?
 
-    self.can_create_team    = false
     self.can_create_group   = false
     self.projects_limit     = 0
-    self.hide_project_limit = true
   end
 end
diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml
index 5506827edc4..b910040a16a 100644
--- a/app/views/admin/users/_form.html.haml
+++ b/app/views/admin/users/_form.html.haml
@@ -58,13 +58,14 @@
         = f.label :admin, class: 'control-label'
         - if current_user == @user
           .col-sm-10= f.check_box :admin, disabled: true
-          .col-sm-10 You cannot remove your own admin rights
+          .col-sm-10 You cannot remove your own admin rights.
         - else
           .col-sm-10= f.check_box :admin
 
       .form-group
         = f.label :external, class: 'control-label'
         .col-sm-10= f.check_box :external
+        .col-sm-10 External users can not see internal or private projects unless access is explicitly granted. Also, external user can not create projects or groups.
 
     %fieldset
       %legend Profile
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index b6b1168bd37..4394e181008 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -19,6 +19,10 @@
       = link_to admin_users_path(filter: 'two_factor_disabled') do
         2FA Disabled
         %small.badge= number_with_delimiter(User.without_two_factor.count)
+    %li.filter-external{class: "#{'active' if params[:filter] == 'external'}"}
+      = link_to admin_users_path(filter: 'external') do
+        External
+        %small.badge= number_with_delimiter(User.external.count)
     %li{class: "#{'active' if params[:filter] == "blocked"}"}
       = link_to admin_users_path(filter: "blocked") do
         Blocked
diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb
index 31bd80e7ccb..f88c591d897 100644
--- a/spec/features/security/project/internal_access_spec.rb
+++ b/spec/features/security/project/internal_access_spec.rb
@@ -8,10 +8,12 @@ describe "Internal Project Access", feature: true  do
   let(:master) { create(:user) }
   let(:guest) { create(:user) }
   let(:reporter) { create(:user) }
+  let(:external_team_member) { create(:user, external: true) }
 
   before do
     # full access
     project.team << [master, :master]
+    project.team << [external_team_member, :master]
 
     # readonly
     project.team << [reporter, :reporter]
@@ -35,6 +37,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -47,6 +50,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -59,6 +63,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -71,6 +76,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -83,6 +89,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -95,6 +102,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -108,6 +116,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -120,6 +129,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -132,6 +142,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -144,6 +155,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -157,6 +169,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -169,6 +182,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -181,6 +195,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -193,6 +208,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -205,6 +221,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -222,6 +239,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -239,6 +257,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_allowed_for guest }
     it { is_expected.to be_allowed_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -251,6 +270,7 @@ describe "Internal Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 end
diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb
index 13ff29c98ae..19f287ce7a4 100644
--- a/spec/features/security/project/private_access_spec.rb
+++ b/spec/features/security/project/private_access_spec.rb
@@ -8,10 +8,12 @@ describe "Private Project Access", feature: true  do
   let(:master)   { create(:user) }
   let(:guest)    { create(:user) }
   let(:reporter) { create(:user) }
+  let(:external_team_member) { create(:user, external: true) }
 
   before do
     # full access
     project.team << [master, :master]
+    project.team << [external_team_member, :master]
 
     # readonly
     project.team << [reporter, :reporter]
@@ -35,6 +37,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -47,6 +50,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -59,6 +63,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -70,6 +75,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_allowed_for :admin }
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -82,6 +88,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -94,6 +101,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -107,6 +115,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -119,6 +128,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -131,6 +141,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -143,6 +154,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -156,6 +168,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -168,6 +181,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -180,6 +194,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -197,6 +212,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -214,6 +230,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 
@@ -226,6 +243,7 @@ describe "Private Project Access", feature: true  do
     it { is_expected.to be_denied_for guest }
     it { is_expected.to be_denied_for :user }
     it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for external_team_member }
     it { is_expected.to be_denied_for :visitor }
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8642654ad33..ca0c84c8632 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -217,7 +217,6 @@ describe User, models: true do
       it "sets other properties aswell" do
         expect(external_user.can_create_team).to be_falsey
         expect(external_user.can_create_group).to be_falsey
-        expect(external_user.hide_project_limit).to be_truthy
         expect(external_user.projects_limit).to be 0
       end
     end