Commit 254e029b authored by Douwe Maan's avatar Douwe Maan Committed by James Lopez

Merge branch 'cherry-pick-b01d0422' into 'security-10-5-ee'

Fix LDAP group sync no longer configurable for regular users (10.5 port)

See merge request gitlab/gitlab-ee!594
parent 0a328042
---
title: Fix LDAP group sync no longer configurable for regular users
merge_request:
author:
type: fixed
module API module API
class Ldap < Grape::API class Ldap < Grape::API
before { authenticated_as_admin! } # Admin users by default should be able to access these API endpoints.
# However, non-admin users can access these endpoints if the "Allow group
# owners to manage LDAP-related group settings" is enabled, and they own a
# group.
before { authenticated_with_ldap_admin_access! }
resource :ldap do resource :ldap do
helpers do helpers do
......
...@@ -20,6 +20,21 @@ module EE ...@@ -20,6 +20,21 @@ module EE
def check_project_feature_available!(feature) def check_project_feature_available!(feature)
not_found! unless user_project.feature_available?(feature) not_found! unless user_project.feature_available?(feature)
end end
# Normally, only admin users should have access to see LDAP
# groups. However, due to the "Allow group owners to manage LDAP-related
# group settings" setting, any group owner can sync LDAP groups with
# their project.
#
# In the future, we should also check that the user has access to manage
# a specific group so that we can use the Ability class.
def authenticated_with_ldap_admin_access!
authenticate!
forbidden! unless current_user.admin? ||
::Gitlab::CurrentSettings.current_application_settings
.allow_group_owners_to_manage_ldap
end
end end
end end
end end
...@@ -17,6 +17,7 @@ describe API::Ldap do ...@@ -17,6 +17,7 @@ describe API::Ldap do
allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true) allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
allow(Gitlab::Auth::LDAP::Adapter).to receive(:new).and_return(adapter) allow(Gitlab::Auth::LDAP::Adapter).to receive(:new).and_return(adapter)
allow(adapter).to receive_messages(groups: groups) allow(adapter).to receive_messages(groups: groups)
stub_application_setting(allow_group_owners_to_manage_ldap: false)
end end
describe "GET /ldap/groups" do describe "GET /ldap/groups" do
...@@ -34,6 +35,20 @@ describe API::Ldap do ...@@ -34,6 +35,20 @@ describe API::Ldap do
end end
end end
context 'when group owners are allowed to manage LDAP' do
before do
stub_application_setting(allow_group_owners_to_manage_ldap: true)
end
it "returns an array of ldap groups" do
get api("/ldap/groups", user)
expect(response.status).to eq 200
expect(json_response).to be_an Array
expect(json_response.length).to eq 2
expect(json_response.first['cn']).to eq 'developers'
end
end
context "when authenticated as admin" do context "when authenticated as admin" do
it "returns an array of ldap groups" do it "returns an array of ldap groups" do
get api("/ldap/groups", admin) get api("/ldap/groups", admin)
...@@ -60,6 +75,20 @@ describe API::Ldap do ...@@ -60,6 +75,20 @@ describe API::Ldap do
end end
end end
context 'when group owners are allowed to manage LDAP' do
before do
stub_application_setting(allow_group_owners_to_manage_ldap: true)
end
it "returns an array of ldap groups" do
get api("/ldap/ldapmain/groups", admin)
expect(response.status).to eq 200
expect(json_response).to be_an Array
expect(json_response.length).to eq 2
expect(json_response.first['cn']).to eq 'developers'
end
end
context "when authenticated as admin" do context "when authenticated as admin" do
it "returns an array of ldap groups" do it "returns an array of ldap groups" do
get api("/ldap/ldapmain/groups", admin) get api("/ldap/ldapmain/groups", admin)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment