Allow unauthenticated access to the `/api/v4/users` API.

- The issue filtering frontend code needs access to this API for non-logged-in
  users + public projects. It uses the API to fetch information for a user by
  username.

- We don't authenticate this API anymore, but instead - if the `current_user` is
  not present:

  - Verify that the `username` parameter has been passed. This disallows an
    unauthenticated user from grabbing a list of all users on the instance. The
    `UsersFinder` class performs an exact match on the `username`, so we are
    guaranteed to get 0 or 1 users.
  - Verify that the resulting user (if any) is accessible to be viewed publicly
    by calling `can?(current_user, :read_user, user)`

4 files changed, 61 insertions, 10 deletions

diff --git a/app/finders/users_finder.rb b/app/finders/users_finder.rb
index dbd50d1db7c..0534317df8f 100644
--- a/app/finders/users_finder.rb
+++ b/app/finders/users_finder.rb
@@ -27,8 +27,11 @@ class UsersFinder
     users = by_search(users)
     users = by_blocked(users)
     users = by_active(users)
-    users = by_external_identity(users)
-    users = by_external(users)
+
+    if current_user
+      users = by_external_identity(users)
+      users = by_external(users)
+    end
 
     users
   end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 2c73a6fdc4e..1322afaa64f 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -407,5 +407,11 @@ module API
 
       exception.status == 500
     end
+
+    # Does the current route match the route identified by
+    # `description`?
+    def route_matches_description?(description)
+      options.dig(:route_options, :description) == description
+    end
   end
 end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index c10e3364382..34619c90d8b 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -4,7 +4,7 @@ module API
 
     before do
       allow_access_with_scope :read_user if request.get?
-      authenticate!
+      authenticate! unless route_matches_description?("Get the list of users")
     end
 
     resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
@@ -51,15 +51,26 @@ module API
         use :pagination
       end
       get do
-        unless can?(current_user, :read_users_list)
-          render_api_error!("Not authorized.", 403)
-        end
-
         authenticated_as_admin! if params[:external].present? || (params[:extern_uid].present? && params[:provider].present?)
 
         users = UsersFinder.new(current_user, params).execute
 
-        entity = current_user.admin? ? Entities::UserWithAdmin : Entities::UserBasic
+        authorized =
+          if current_user
+            can?(current_user, :read_users_list)
+          else
+            # When `current_user` is not present, require that the `username`
+            # parameter is passed, to prevent an unauthenticated user from accessing
+            # a list of all the users on the GitLab instance. `UsersFinder` performs
+            # an exact match on the `username` parameter, so we are guaranteed to
+            # get either 0 or 1 `users` here.
+            params[:username].present? &&
+              users.all? { |user| can?(current_user, :read_user, user) }
+          end
+
+        render_api_error!("Not authorized.", 403) unless authorized
+
+        entity = current_user.try(:admin?) ? Entities::UserWithAdmin : Entities::UserBasic
         present paginate(users), with: entity
       end
 
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 18000d91795..01541901330 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -13,9 +13,40 @@ describe API::Users do
 
   describe 'GET /users' do
     context "when unauthenticated" do
-      it "returns authentication error" do
+      it "returns authorization error when the `username` parameter is not passed" do
         get api("/users")
-        expect(response).to have_http_status(401)
+
+        expect(response).to have_http_status(403)
+      end
+
+      it "returns the user when a valid `username` parameter is passed" do
+        user = create(:user)
+
+        get api("/users"), username: user.username
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.size).to eq(1)
+        expect(json_response[0]['id']).to eq(user.id)
+        expect(json_response[0]['username']).to eq(user.username)
+      end
+
+      it "returns authorization error when the `username` parameter refers to an inaccessible user" do
+        user = create(:user)
+
+        expect(Ability).to receive(:allowed?).with(nil, :read_user, user).and_return(false)
+
+        get api("/users"), username: user.username
+
+        expect(response).to have_http_status(403)
+      end
+
+      it "returns an empty response when an invalid `username` parameter is passed" do
+        get api("/users"), username: 'invalid'
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.size).to eq(0)
       end
     end