summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/finders/users_finder.rb4
-rw-r--r--app/policies/base_policy.rb7
-rw-r--r--app/policies/global_policy.rb15
-rw-r--r--app/policies/user_policy.rb7
-rw-r--r--changelogs/unreleased/34141-allow-unauthenticated-access-to-the-users-api.yml4
-rw-r--r--lib/api/helpers.rb6
-rw-r--r--lib/api/users.rb19
-rw-r--r--spec/requests/api/users_spec.rb35
8 files changed, 78 insertions, 19 deletions
diff --git a/app/finders/users_finder.rb b/app/finders/users_finder.rb
index dbd50d1db7c..07deceb827b 100644
--- a/app/finders/users_finder.rb
+++ b/app/finders/users_finder.rb
@@ -60,13 +60,13 @@ class UsersFinder
end
def by_external_identity(users)
- return users unless current_user.admin? && params[:extern_uid] && params[:provider]
+ return users unless current_user&.admin? && params[:extern_uid] && params[:provider]
users.joins(:identities).merge(Identity.with_extern_uid(params[:provider], params[:extern_uid]))
end
def by_external(users)
- return users = users.where.not(external: true) unless current_user.admin?
+ return users = users.where.not(external: true) unless current_user&.admin?
return users unless params[:external]
users.external
diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb
index 191c2e78a08..a605a3457c8 100644
--- a/app/policies/base_policy.rb
+++ b/app/policies/base_policy.rb
@@ -1,6 +1,8 @@
require_dependency 'declarative_policy'
class BasePolicy < DeclarativePolicy::Base
+ include Gitlab::CurrentSettings
+
desc "User is an instance admin"
with_options scope: :user, score: 0
condition(:admin) { @user&.admin? }
@@ -10,4 +12,9 @@ class BasePolicy < DeclarativePolicy::Base
with_options scope: :user, score: 0
condition(:can_create_group) { @user&.can_create_group }
+
+ desc "The application is restricted from public visibility"
+ condition(:restricted_public_level, scope: :global) do
+ current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
+ end
end
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index 535faa922dd..7767d3cccd5 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -11,10 +11,17 @@ class GlobalPolicy < BasePolicy
with_options scope: :user, score: 0
condition(:access_locked) { @user.access_locked? }
- rule { anonymous }.prevent_all
+ rule { anonymous }.policy do
+ prevent :log_in
+ prevent :access_api
+ prevent :access_git
+ prevent :receive_notifications
+ prevent :use_quick_actions
+ prevent :create_group
+ prevent :log_in
+ end
rule { default }.policy do
- enable :read_users_list
enable :log_in
enable :access_api
enable :access_git
@@ -37,4 +44,8 @@ class GlobalPolicy < BasePolicy
rule { access_locked }.policy do
prevent :log_in
end
+
+ rule { ~restricted_public_level }.policy do
+ enable :read_users_list
+ end
end
diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
index 0181ddf85e0..0905ddd9b38 100644
--- a/app/policies/user_policy.rb
+++ b/app/policies/user_policy.rb
@@ -1,11 +1,4 @@
class UserPolicy < BasePolicy
- include Gitlab::CurrentSettings
-
- desc "The application is restricted from public visibility"
- condition(:restricted_public_level, scope: :global) do
- current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
- end
-
desc "The current user is the user in question"
condition(:user_is_self, score: 0) { @subject == @user }
diff --git a/changelogs/unreleased/34141-allow-unauthenticated-access-to-the-users-api.yml b/changelogs/unreleased/34141-allow-unauthenticated-access-to-the-users-api.yml
new file mode 100644
index 00000000000..a3ade8db214
--- /dev/null
+++ b/changelogs/unreleased/34141-allow-unauthenticated-access-to-the-users-api.yml
@@ -0,0 +1,4 @@
+---
+title: Allow unauthenticated access to the /api/v4/users API
+merge_request: 12445
+author:
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 2c73a6fdc4e..a3aec8889d7 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 request_matches_route?(method, route)
+ request.request_method == method && request.path == route
+ end
end
end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index f9555842daf..bad4d76b428 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 request_matches_route?('GET', '/api/v4/users')
end
resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
@@ -51,15 +51,22 @@ 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 = can?(current_user, :read_users_list)
+
+ # 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.
+ authorized &&= params[:username].present? if current_user.blank?
+
+ forbidden!("Not authorized to access /api/v4/users") unless authorized
+
+ entity = current_user&.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 c0174b304c8..aa95ae8c7cc 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)
+
+ stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+
+ 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
View Lorry Controller Status