Skip to content
Snippets Groups Projects
Commit 96d99847 authored by Shinya Maeda's avatar Shinya Maeda :two:
Browse files

Optimize AuthorizeProxyUserService with user_access authorizations

parent 32cab9c4
No related branches found
No related tags found
2 merge requests!122597doc/gitaly: Remove references to removed metrics,!116905Optimize AuthorizeProxyUserService with user_access authorizations
......@@ -2,6 +2,8 @@
module Clusters
class Agent < ApplicationRecord
include FromUnion
self.table_name = 'cluster_agents'
INACTIVE_AFTER = 1.hour.freeze
......@@ -68,6 +70,15 @@ def ci_access_authorized_for?(user)
).exists?
end
# As of today, all config values of associated authorization rows have the same value.
# See `UserAccess::RefreshService` for more information.
def user_access_config
self.class.from_union(
user_access_project_authorizations.select('config').limit(1),
user_access_group_authorizations.select('config').limit(1)
).compact.first&.config
end
private
def all_ci_access_authorized_projects_for(user)
......
......@@ -13,12 +13,12 @@ def initialize(current_user, agent)
def execute
return forbidden unless user_access_config.present?
access_as = user_access_config[:access_as]
access_as = user_access_config['access_as']
return forbidden unless access_as.present?
return forbidden if access_as.size != 1
if authorizations = handle_access(access_as, user_access_config)
return success(payload: authorizations)
if payload = handle_access(access_as)
return success(payload: payload)
end
forbidden
......@@ -29,15 +29,20 @@ def execute
attr_reader :current_user, :agent
# Override in EE
def handle_access(access_as, user_access)
access_as_agent(user_access) if access_as.key?(:agent)
def handle_access(access_as)
access_as_agent if access_as.key?('agent')
end
def authorizations
@authorizations ||= ::Clusters::Agents::Authorizations::UserAccess::Finder
.new(current_user, agent: agent).execute
end
def response_base
{
agent: {
id: agent.id,
config_project: { id: agent.project.id }
config_project: { id: agent.project_id }
},
user: {
id: current_user.id,
......@@ -46,44 +51,14 @@ def response_base
}
end
def access_as_agent(user_access)
projects = authorized_projects(user_access)
groups = authorized_groups(user_access)
return unless projects.size + groups.size > 0
def access_as_agent
return if authorizations.empty?
response_base.merge(access_as: { agent: {} })
end
def authorized_projects(user_access)
strong_memoize_with(:authorized_projects, user_access) do
user_access.fetch(:projects, [])
.first(::Clusters::Agents::Authorizations::CiAccess::RefreshService::AUTHORIZED_ENTITY_LIMIT)
.map { |project| ::Project.find_by_full_path(project[:id]) }
.select { |project| current_user.can?(:use_k8s_proxies, project) }
end
end
def authorized_groups(user_access)
strong_memoize_with(:authorized_groups, user_access) do
user_access.fetch(:groups, [])
.first(::Clusters::Agents::Authorizations::CiAccess::RefreshService::AUTHORIZED_ENTITY_LIMIT)
.map { |group| ::Group.find_by_full_path(group[:id]) }
.select { |group| current_user.can?(:use_k8s_proxies, group) }
end
end
def user_access_config
# TODO: Read the configuration from the database once it has been
# indexed. See https://gitlab.com/gitlab-org/gitlab/-/issues/389430
branch = agent.project.default_branch_or_main
path = ".gitlab/agents/#{agent.name}/config.yaml"
config_yaml = agent.project.repository
&.blob_at_branch(branch, path)
&.data
return unless config_yaml.present?
config = YAML.safe_load(config_yaml, aliases: true, symbolize_names: true)
config[:user_access]
agent.user_access_config
end
strong_memoize_attr :user_access_config
......
......@@ -7,40 +7,54 @@ module AuthorizeProxyUserService
extend ::Gitlab::Utils::Override
override :handle_access
def handle_access(access_as, user_access)
super || (access_as.key?(:user) && access_as_user(user_access))
def handle_access(access_as)
super || (access_as.key?('user') && access_as_user)
end
def access_as_user(user_access)
projects = authorized_projects(user_access)
groups = authorized_groups(user_access)
return unless projects.size + groups.size > 0
private
def access_as_user
return if authorizations.empty?
response_base.merge(
access_as: {
user: {
# FIXME: N+1 queries on projects and groups
# see https://gitlab.com/gitlab-org/gitlab/-/issues/393336
projects: projects.map { |p| { id: p.id, roles: project_roles(p) } },
groups: groups.map { |g| { id: g.id, roles: group_roles(g) } }
projects: projects_payload,
groups: groups_payload
}
}
)
end
def project_roles(project)
user_access_level = current_user.max_member_access_for_project(project.id)
::Gitlab::Access.sym_options_with_owner
.select { |_role, role_access_level| role_access_level <= user_access_level }
.map(&:first)
def projects_payload
project_authorizations.map do |authorization|
{ id: authorization.project_id, roles: roles(authorization.access_level) }
end
end
def groups_payload
group_authorizations.map do |authorization|
{ id: authorization.group_id, roles: roles(authorization.access_level) }
end
end
def group_roles(group)
user_access_level = current_user.max_member_access_for_group(group.id)
def roles(access_level)
::Gitlab::Access.sym_options_with_owner
.select { |_role, role_access_level| role_access_level <= user_access_level }
.select { |_role, role_access_level| role_access_level <= access_level }
.map(&:first)
end
def project_authorizations
@project_authorizations ||= authorizations.select do |authorization|
authorization.is_a?(::Clusters::Agents::Authorizations::UserAccess::ProjectAuthorization)
end
end
def group_authorizations
@group_authorizations ||= authorizations.select do |authorization|
authorization.is_a?(::Clusters::Agents::Authorizations::UserAccess::GroupAuthorization)
end
end
end
end
end
......
......@@ -8,43 +8,40 @@
let(:service) { described_class.new(user, agent) }
let(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:group) { create(:group) }
let_it_be(:user_access_config) do
let_it_be(:organization) { create(:group) }
let_it_be(:configuration_project) { create(:project, group: organization) }
let_it_be(:agent) { create(:cluster_agent, name: 'the-agent', project: configuration_project) }
let_it_be(:deployment_project) { create(:project, group: organization) }
let_it_be(:deployment_group) { create(:group, parent: organization) }
let(:user_access_config) do
{
'user_access' => {
'access_as' => { 'user' => {} },
'projects' => [{ 'id' => project.full_path }],
'groups' => [{ 'id' => group.full_path }]
'projects' => [{ 'id' => deployment_project.full_path }],
'groups' => [{ 'id' => deployment_group.full_path }]
}
}
end
let_it_be(:configuration_project) do
create(
:project, :custom_repo,
files: {
".gitlab/agents/the-agent/config.yaml" => user_access_config.to_yaml
}
)
before do
Clusters::Agents::Authorizations::UserAccess::RefreshService.new(agent, config: user_access_config).execute
end
let_it_be(:agent) { create(:cluster_agent, name: 'the-agent', project: configuration_project) }
it 'returns forbidden when user has no access to any project', :aggregate_failures do
expect(service_response).to be_error
expect(service_response.reason).to eq :forbidden
end
it "returns the user's authorizations when they have access", :aggregate_failures do
project.add_member(user, :developer)
group.add_member(user, :maintainer)
deployment_project.add_member(user, :developer)
deployment_group.add_member(user, :maintainer)
expect(service_response).to be_success
expect(service_response.payload[:access_as]).to eq({
user: {
projects: [{ id: project.id, roles: %i[guest reporter developer] }],
groups: [{ id: group.id, roles: %i[guest reporter developer maintainer] }]
projects: [{ id: deployment_project.id, roles: %i[guest reporter developer] }],
groups: [{ id: deployment_group.id, roles: %i[guest reporter developer maintainer] }]
}
})
end
......
......@@ -234,4 +234,46 @@
end
end
end
describe '#user_access_config' do
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project) }
let_it_be_with_refind(:agent) { create(:cluster_agent, project: project) }
subject { agent.user_access_config }
it { is_expected.to be_nil }
context 'with user_access project authorizations' do
before do
create(:agent_user_access_project_authorization, agent: agent, project: project, config: config)
end
let(:config) { {} }
it { is_expected.to eq(config) }
context 'when access_as keyword exists' do
let(:config) { { 'access_as' => { 'agent' => {} } } }
it { is_expected.to eq(config) }
end
end
context 'with user_access group authorizations' do
before do
create(:agent_user_access_group_authorization, agent: agent, group: group, config: config)
end
let(:config) { {} }
it { is_expected.to eq(config) }
context 'when access_as keyword exists' do
let(:config) { { 'access_as' => { 'agent' => {} } } }
it { is_expected.to eq(config) }
end
end
end
end
......@@ -372,34 +372,28 @@ def new_token
ActionController::Base.new.send(:generate_csrf_token)
end
let_it_be(:project) { create(:project) }
let_it_be(:group) { create(:group) }
let_it_be(:user_access_config) do
let_it_be(:organization) { create(:group) }
let_it_be(:configuration_project) { create(:project, group: organization) }
let_it_be(:agent) { create(:cluster_agent, name: 'the-agent', project: configuration_project) }
let_it_be(:another_agent) { create(:cluster_agent) }
let_it_be(:deployment_project) { create(:project, group: organization) }
let_it_be(:deployment_group) { create(:group, parent: organization) }
let(:user_access_config) do
{
'user_access' => {
'access_as' => { 'agent' => {} },
'projects' => [{ 'id' => project.full_path }],
'groups' => [{ 'id' => group.full_path }]
'projects' => [{ 'id' => deployment_project.full_path }],
'groups' => [{ 'id' => deployment_group.full_path }]
}
}
end
let_it_be(:configuration_project) do
create(
:project, :custom_repo,
files: {
".gitlab/agents/the-agent/config.yaml" => user_access_config.to_yaml
}
)
end
let_it_be(:agent) { create(:cluster_agent, name: 'the-agent', project: configuration_project) }
let_it_be(:another_agent) { create(:cluster_agent) }
let(:user) { create(:user) }
before do
allow(::Gitlab::Kas).to receive(:enabled?).and_return true
Clusters::Agents::Authorizations::UserAccess::RefreshService.new(agent, config: user_access_config).execute
end
it 'returns 400 when cookie is invalid' do
......@@ -442,7 +436,7 @@ def new_token
end
it 'returns 200 when user has access' do
project.add_member(user, :developer)
deployment_project.add_member(user, :developer)
token = new_token
public_id = stub_user_session(user, token)
access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id)
......@@ -452,7 +446,7 @@ def new_token
end
it 'returns 401 when user has valid KAS cookie and CSRF token but has no access to requested agent' do
project.add_member(user, :developer)
deployment_project.add_member(user, :developer)
token = new_token
public_id = stub_user_session(user, token)
access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id)
......@@ -464,7 +458,7 @@ def new_token
it 'returns 401 when global flag is disabled' do
stub_feature_flags(kas_user_access: false)
project.add_member(user, :developer)
deployment_project.add_member(user, :developer)
token = new_token
public_id = stub_user_session(user, token)
access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id)
......@@ -474,7 +468,7 @@ def new_token
end
it 'returns 401 when user id is not found in session' do
project.add_member(user, :developer)
deployment_project.add_member(user, :developer)
token = new_token
public_id = stub_user_session_with_no_user_id(user, token)
access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id)
......
......@@ -8,29 +8,26 @@
let(:service) { described_class.new(user, agent) }
let(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:group) { create(:group) }
let_it_be(:user_access_config) do
let_it_be(:organization) { create(:group) }
let_it_be(:configuration_project) { create(:project, group: organization) }
let_it_be(:agent) { create(:cluster_agent, name: 'the-agent', project: configuration_project) }
let_it_be(:deployment_project) { create(:project, group: organization) }
let_it_be(:deployment_group) { create(:group, parent: organization) }
let(:user_access_config) do
{
'user_access' => {
'access_as' => { 'agent' => {} },
'projects' => [{ 'id' => project.full_path }],
'groups' => [{ 'id' => group.full_path }]
'projects' => [{ 'id' => deployment_project.full_path }],
'groups' => [{ 'id' => deployment_group.full_path }]
}
}
end
let_it_be(:configuration_project) do
create(
:project, :custom_repo,
files: {
".gitlab/agents/the-agent/config.yaml" => user_access_config.to_yaml
}
)
before do
Clusters::Agents::Authorizations::UserAccess::RefreshService.new(agent, config: user_access_config).execute
end
let_it_be(:agent) { create(:cluster_agent, name: 'the-agent', project: configuration_project) }
it 'returns forbidden when user has no access to any project', :aggregate_failures do
expect(service_response).to be_error
expect(service_response.reason).to eq :forbidden
......@@ -38,14 +35,14 @@
context 'when user is member of an authorized group' do
it 'authorizes developers', :aggregate_failures do
group.add_member(user, :developer)
deployment_group.add_member(user, :developer)
expect(service_response).to be_success
expect(service_response.payload[:user]).to include(id: user.id, username: user.username)
expect(service_response.payload[:agent]).to include(id: agent.id, config_project: { id: agent.project.id })
end
it 'does not authorize reporters', :aggregate_failures do
group.add_member(user, :reporter)
deployment_group.add_member(user, :reporter)
expect(service_response).to be_error
expect(service_response.reason).to eq :forbidden
end
......@@ -53,14 +50,14 @@
context 'when user is member of an authorized project' do
it 'authorizes developers', :aggregate_failures do
project.add_member(user, :developer)
deployment_project.add_member(user, :developer)
expect(service_response).to be_success
expect(service_response.payload[:user]).to include(id: user.id, username: user.username)
expect(service_response.payload[:agent]).to include(id: agent.id, config_project: { id: agent.project.id })
end
it 'does not authorize reporters', :aggregate_failures do
project.add_member(user, :reporter)
deployment_project.add_member(user, :reporter)
expect(service_response).to be_error
expect(service_response.reason).to eq :forbidden
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment