diff --git a/app/controllers/concerns/dependency_proxy/group_access.rb b/app/controllers/concerns/dependency_proxy/group_access.rb
index e9fb2563e42e1ee02cbd84f54d9fbbdc08c94b59..76d6535a21f261fab6d994c216fbfecaae0f2edd 100644
--- a/app/controllers/concerns/dependency_proxy/group_access.rb
+++ b/app/controllers/concerns/dependency_proxy/group_access.rb
@@ -11,13 +11,45 @@ module GroupAccess
private
+ def auth_user_or_token
+ if defined?(personal_access_token) && personal_access_token && auth_user.is_a?(::User) &&
+ (
+ (auth_user.project_bot? && auth_user.resource_bot_resource.is_a?(::Group)) ||
+ auth_user.human? ||
+ auth_user.service_account?
+ )
+ personal_access_token
+ else
+ auth_user
+ end
+ end
+
def verify_dependency_proxy_available!
render_404 unless group&.dependency_proxy_feature_available?
end
+ # TODO: Split the authorization logic into dedicated methods
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/452145
def authorize_read_dependency_proxy!
+ if Feature.enabled?(:packages_dependency_proxy_pass_token_to_policy, group)
+ if auth_user_or_token.is_a?(User)
+ authorize_read_dependency_proxy_for_users!
+ else
+ authorize_read_dependency_proxy_for_tokens!
+ end
+ else
+ authorize_read_dependency_proxy_for_users!
+ end
+ end
+
+ def authorize_read_dependency_proxy_for_users!
access_denied! unless can?(auth_user, :read_dependency_proxy, group)
end
+
+ def authorize_read_dependency_proxy_for_tokens!
+ access_denied! unless can?(auth_user_or_token, :read_dependency_proxy,
+ group&.dependency_proxy_for_containers_policy_subject)
+ end
end
end
diff --git a/app/controllers/groups/dependency_proxy/application_controller.rb b/app/controllers/groups/dependency_proxy/application_controller.rb
index a5266d89ddbee076ea91397a05b2aac6faa71451..b1bc53507a0ee0067aee3b42371e0603928594a0 100644
--- a/app/controllers/groups/dependency_proxy/application_controller.rb
+++ b/app/controllers/groups/dependency_proxy/application_controller.rb
@@ -19,15 +19,18 @@ def authenticate_user_from_jwt_token!
authenticate_with_http_token do |token, _|
@authentication_result = EMPTY_AUTH_RESULT
- user_or_deploy_token = ::DependencyProxy::AuthTokenService.user_or_deploy_token_from_jwt(token)
-
- case user_or_deploy_token
- when User
- @authentication_result = Gitlab::Auth::Result.new(user_or_deploy_token, nil, :user, [])
- sign_in(user_or_deploy_token) unless user_or_deploy_token.project_bot? ||
- user_or_deploy_token.service_account?
- when DeployToken
- @authentication_result = Gitlab::Auth::Result.new(user_or_deploy_token, nil, :deploy_token, [])
+ if Feature.enabled?(:packages_dependency_proxy_pass_token_to_policy, group_from_params)
+ user_or_token = ::DependencyProxy::AuthTokenService.user_or_token_from_jwt(token)
+ sign_in_and_setup_authentication_result(user_or_token)
+ else
+ user_or_token = ::DependencyProxy::AuthTokenService.user_or_deploy_token_from_jwt(token)
+ case user_or_token
+ when User
+ @authentication_result = Gitlab::Auth::Result.new(user_or_token, nil, :user, [])
+ sign_in(user_or_token) unless user_or_token.project_bot? || user_or_token.service_account?
+ when DeployToken
+ @authentication_result = Gitlab::Auth::Result.new(user_or_token, nil, :deploy_token, [])
+ end
end
end
@@ -36,11 +39,32 @@ def authenticate_user_from_jwt_token!
private
+ attr_reader :personal_access_token
+
+ def group_from_params
+ GroupsFinder.new(nil, { search: params[:group_id] }).execute.first
+ end
+
def request_bearer_token!
# unfortunately, we cannot use https://api.rubyonrails.org/classes/ActionController/HttpAuthentication/Token.html#method-i-authentication_request
response.headers['WWW-Authenticate'] = ::DependencyProxy::Registry.authenticate_header
render plain: '', status: :unauthorized
end
+
+ # When we rollout packages_dependency_proxy_pass_token_to_policy,
+ # we can move the body of this method inline, inside authenticate_user_from_jwt_token!
+ def sign_in_and_setup_authentication_result(user_or_token)
+ case user_or_token
+ when User
+ @authentication_result = Gitlab::Auth::Result.new(user_or_token, nil, :user, [])
+ sign_in(user_or_token)
+ when PersonalAccessToken
+ @authentication_result = Gitlab::Auth::Result.new(user_or_token.user, nil, :personal_access_token, [])
+ @personal_access_token = user_or_token
+ when DeployToken
+ @authentication_result = Gitlab::Auth::Result.new(user_or_token, nil, :deploy_token, [])
+ end
+ end
end
end
end
diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index e6e232cfbc38fc93fd8c5791e0297771aac60568..de7fe52e3db5dadca84c889039f88d2e45d8cd2c 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -33,9 +33,10 @@ def authenticate_project_or_user
@authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_only_authentication_abilities)
authenticate_with_http_basic do |login, password|
- @raw_token = password
@authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, request: request)
+ @raw_token = password if @authentication_result.type == :personal_access_token
+
if @authentication_result.failed?
log_authentication_failed(login, @authentication_result)
render_access_denied
diff --git a/app/models/group.rb b/app/models/group.rb
index 0668458b8e2b38638c13b4b4ef4051bd064296c4..cac322ea7ebe9e255451ebffc1c4363765503a7c 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -971,6 +971,10 @@ def packages_policy_subject
::Packages::Policies::Group.new(self)
end
+ def dependency_proxy_for_containers_policy_subject
+ ::Packages::Policies::DependencyProxy::Group.new(self)
+ end
+
def update_two_factor_requirement_for_members
hierarchy_members.find_each(&:update_two_factor_requirement)
end
diff --git a/app/models/packages/policies/dependency_proxy/group.rb b/app/models/packages/policies/dependency_proxy/group.rb
new file mode 100644
index 0000000000000000000000000000000000000000..00986e6f474ba6608a8ca93736c7b55b1b927abc
--- /dev/null
+++ b/app/models/packages/policies/dependency_proxy/group.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+# We use this class, in conjunction with the
+# Group#dependency_proxy_for_containers_policy_subject method,
+# to specify a custom policy class for DependencyProxy.
+# A similar pattern was used in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90963
+module Packages
+ module Policies
+ module DependencyProxy
+ class Group
+ attr_reader :group
+
+ delegate :dependency_proxy_feature_available?, :full_path, :licensed_feature_available?,
+ :max_member_access_for_user, :member?, :owned_by?, :public?, :root_ancestor,
+ :root_ancestor_ip_restrictions, to: :group
+
+ def initialize(group)
+ @group = group
+ end
+ end
+ end
+ end
+end
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 560625f6541db011767692a95018a988819112ee..45f02b438c5fb35b27ac7e63b5c2dec86c4c627d 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -462,6 +462,7 @@ def resource_access_token_creation_allowed?
resource_access_token_create_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
end
+ # We can remove this when we rollout the feature flag packages_dependency_proxy_pass_token_to_policy
def valid_dependency_proxy_deploy_token
@user.is_a?(DeployToken) && @user&.valid_for_dependency_proxy? && @user&.has_access_to_group?(@subject)
end
diff --git a/app/policies/packages/policies/dependency_proxy/group_policy.rb b/app/policies/packages/policies/dependency_proxy/group_policy.rb
new file mode 100644
index 0000000000000000000000000000000000000000..11e57a2d33782258b3dbe7ec129f9d281758e1f6
--- /dev/null
+++ b/app/policies/packages/policies/dependency_proxy/group_policy.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+# The policies defined in GroupPolicy is used in GraphQL requests
+# With a GraphQL request, the user is always a human User
+#
+# With JWT requests, we can be dealing with any of the following:
+# - a PrAT for a human
+# - a PrAT for a service account
+# - a GrAT
+# - a Group DeployToken
+#
+# We use this custom policy class for JWT requests
+module Packages
+ module Policies
+ module DependencyProxy
+ class GroupPolicy < ::GroupPolicy
+ overrides(:read_dependency_proxy)
+
+ desc "Deploy token with read access to dependency proxy"
+ condition(:read_dependency_proxy_deploy_token) do
+ @user.is_a?(DeployToken) && @user&.valid_for_dependency_proxy? && @user&.has_access_to_group?(@subject.group)
+ end
+
+ desc "Personal access or group access token with read access to dependency proxy"
+ condition(:read_dependency_proxy_personal_access_token) do
+ user_is_personal_access_token? &&
+ (
+ user.user.human? ||
+ user.user.service_account? ||
+ (user.user.project_bot? && user.user.resource_bot_resource.is_a?(::Group))
+ ) &&
+ (access_level(for_any_session: true) >= GroupMember::GUEST)
+ end
+
+ condition(:dependency_proxy_disabled, scope: :subject) do
+ !@subject.dependency_proxy_feature_available?
+ end
+
+ rule { dependency_proxy_disabled }.prevent :read_dependency_proxy
+
+ rule do
+ read_dependency_proxy_personal_access_token | read_dependency_proxy_deploy_token
+ end.enable :read_dependency_proxy
+
+ rule do
+ ~read_dependency_proxy_personal_access_token & ~read_dependency_proxy_deploy_token
+ end.prevent :read_dependency_proxy
+
+ def access_level(for_any_session: false)
+ return GroupMember::NO_ACCESS if @user.nil?
+
+ @access_level ||= lookup_access_level!(for_any_session: for_any_session)
+ end
+
+ def lookup_access_level!(_)
+ @subject.max_member_access_for_user(@user.user)
+ end
+
+ def user_is_personal_access_token?
+ user.is_a?(PersonalAccessToken)
+ end
+ end
+ end
+ end
+end
+
+Packages::Policies::DependencyProxy::GroupPolicy.prepend_mod_with('Packages::Policies::DependencyProxy::GroupPolicy')
diff --git a/app/services/auth/dependency_proxy_authentication_service.rb b/app/services/auth/dependency_proxy_authentication_service.rb
index 29f5a50d809ffb5f53a6c5a55dc0fb8f7714dd0d..2ba9da159c939798a6908ebd70aa792c5aa453d5 100644
--- a/app/services/auth/dependency_proxy_authentication_service.rb
+++ b/app/services/auth/dependency_proxy_authentication_service.rb
@@ -65,6 +65,8 @@ def authorized_token
JSONWebToken::HMACToken.new(self.class.secret).tap do |token|
token['user_id'] = current_user.id if current_user
token['deploy_token'] = deploy_token.token if deploy_token
+ token['personal_access_token'] = raw_token if personal_access_token_user?
+ token['group_access_token'] = raw_token if group_access_token_user?
token.expire_time = self.class.token_expire_at
end
end
@@ -76,5 +78,13 @@ def deploy_token
def raw_token
params[:raw_token]
end
+
+ def group_access_token_user?
+ raw_token && current_user&.project_bot? && current_user.resource_bot_resource.is_a?(Group)
+ end
+
+ def personal_access_token_user?
+ raw_token && current_user && (current_user.human? || current_user.service_account?)
+ end
end
end
diff --git a/app/services/dependency_proxy/auth_token_service.rb b/app/services/dependency_proxy/auth_token_service.rb
index c6c9eb534bbb26cae28d04886c3f78d0638c3fcd..a4034894e13d614d78246eacc860451e189ba81b 100644
--- a/app/services/dependency_proxy/auth_token_service.rb
+++ b/app/services/dependency_proxy/auth_token_service.rb
@@ -12,6 +12,10 @@ def execute
JSONWebToken::HMACToken.decode(token, ::Auth::DependencyProxyAuthenticationService.secret).first
end
+ # Rename to make it obvious how it's used in Gitlab::Auth::RequestAuthenticator
+ # which is to return an <object>.<id> that is used as a rack-attack discriminator
+ # that way it cannot be confused with `.user_or_token_from_jwt`
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/454518
def self.user_or_deploy_token_from_jwt(raw_jwt)
token_payload = self.new(raw_jwt).execute
@@ -23,5 +27,34 @@ def self.user_or_deploy_token_from_jwt(raw_jwt)
rescue JWT::DecodeError, JWT::ExpiredSignature, JWT::ImmatureSignature
nil
end
+
+ def self.user_or_token_from_jwt(raw_jwt)
+ token_payload = self.new(raw_jwt).execute
+
+ if token_payload['personal_access_token']
+ get_personal_access_token(token_payload['personal_access_token'])
+ elsif token_payload['group_access_token']
+ # a group access token is a personal access token in disguise
+ get_personal_access_token(token_payload['group_access_token'])
+ elsif token_payload['deploy_token']
+ get_deploy_token(token_payload['deploy_token'])
+ elsif token_payload['user_id']
+ get_user(token_payload['user_id'])
+ end
+ rescue JWT::DecodeError, JWT::ExpiredSignature, JWT::ImmatureSignature
+ nil
+ end
+
+ def self.get_user(user_id)
+ User.find(user_id)
+ end
+
+ def self.get_personal_access_token(raw_token)
+ PersonalAccessTokensFinder.new(state: 'active').find_by_token(raw_token)
+ end
+
+ def self.get_deploy_token(raw_token)
+ DeployToken.active.find_by_token(raw_token)
+ end
end
end
diff --git a/config/feature_flags/gitlab_com_derisk/packages_dependency_proxy_pass_token_to_policy.yml b/config/feature_flags/gitlab_com_derisk/packages_dependency_proxy_pass_token_to_policy.yml
new file mode 100644
index 0000000000000000000000000000000000000000..9131f3f01d791d27fff2939dfd2cd3ca2780f715
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/packages_dependency_proxy_pass_token_to_policy.yml
@@ -0,0 +1,9 @@
+---
+name: packages_dependency_proxy_pass_token_to_policy
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/434291
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141358
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/441588
+milestone: '17.0'
+group: group::container registry
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/ee/app/controllers/concerns/ee/dependency_proxy/group_access.rb b/ee/app/controllers/concerns/ee/dependency_proxy/group_access.rb
index 0a832f8988a645d603196f20f86de89e1dc56e04..f07eeed9d0fe9c13d8b1839c3a474213d21b32e6 100644
--- a/ee/app/controllers/concerns/ee/dependency_proxy/group_access.rb
+++ b/ee/app/controllers/concerns/ee/dependency_proxy/group_access.rb
@@ -8,6 +8,8 @@ module GroupAccess
private
+ # Rename this to authorize_read_dependency_proxy_for_tokens!
+ # when we rollout FF packages_dependency_proxy_pass_token_to_policy
override :authorize_read_dependency_proxy!
def authorize_read_dependency_proxy!
if ip_restricted?(group)
diff --git a/ee/app/policies/ee/packages/policies/dependency_proxy/group_policy.rb b/ee/app/policies/ee/packages/policies/dependency_proxy/group_policy.rb
new file mode 100644
index 0000000000000000000000000000000000000000..6545e582dcbf8ae06fc45c8c0eb99ff59419eb65
--- /dev/null
+++ b/ee/app/policies/ee/packages/policies/dependency_proxy/group_policy.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module EE
+ module Packages
+ module Policies
+ module DependencyProxy
+ module GroupPolicy
+ extend ActiveSupport::Concern
+ extend ::Gitlab::Utils::Override
+
+ prepended do
+ with_scope :user
+ condition(:auditor, score: 0) { false }
+
+ condition(:no_active_sso_session, scope: :subject) do
+ policy_user = user.user
+
+ ::Gitlab::Auth::GroupSaml::SessionEnforcer.new(policy_user, subject).access_restricted?
+ end
+ end
+
+ # This is a copy of lookup_access_level! in ee/app/policies/ee/group_policy.rb
+ override :lookup_access_level!
+ def lookup_access_level!(for_any_session: false)
+ if for_any_session
+ return ::GroupMember::NO_ACCESS if no_active_sso_session?
+ elsif needs_new_sso_session?
+ return ::GroupMember::NO_ACCESS
+ end
+
+ super
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
index 19ce13bd85c133975e9e388d33a97655a71a31f7..1e64b1fefe6eb1795af1c41e3ff11e28d538ad40 100644
--- a/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
+++ b/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
@@ -122,7 +122,10 @@
it_behaves_like 'returning response status', :not_found
context 'when user is a deploy token' do
- let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
+ let_it_be(:deploy_token) do
+ create(:deploy_token, user: user, read_package_registry: true, write_package_registry: true)
+ end
+
let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token, group: group) }
let(:authenticated_subject) { deploy_token }
diff --git a/ee/spec/policies/packages/policies/dependency_proxy/group_policy_spec.rb b/ee/spec/policies/packages/policies/dependency_proxy/group_policy_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0aba2fdd8a62185d36e59bb85ec093107a1b64a9
--- /dev/null
+++ b/ee/spec/policies/packages/policies/dependency_proxy/group_policy_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Policies::DependencyProxy::GroupPolicy, feature_category: :container_registry do
+ include_context 'GroupPolicy context'
+
+ subject do
+ described_class.new(auth_token, group.dependency_proxy_for_containers_policy_subject)
+ end
+
+ let_it_be(:current_user) { guest }
+ let_it_be(:auth_token) { create(:personal_access_token, user: current_user) }
+
+ before do
+ stub_config(dependency_proxy: { enabled: true }, registry: { enabled: true })
+ end
+
+ context 'when there is no active sso session' do
+ before do
+ allow(::Gitlab::Auth::GroupSaml::SsoEnforcer)
+ .to receive(:access_restricted?)
+ .and_return(true)
+ end
+
+ it { is_expected.to be_disallowed(:guest_access) }
+ end
+end
diff --git a/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb b/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb
index d9cfc91b33e8dd68450945a3735baecd806d7e75..b8726bbfd5dff0373c47441bc9867aca572e056e 100644
--- a/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb
+++ b/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Groups::DependencyProxyAuthController do
+RSpec.describe Groups::DependencyProxyAuthController, feature_category: :container_registry do
include DependencyProxyHelpers
describe 'GET #authenticate' do
@@ -33,21 +33,57 @@
end
context 'group bot user' do
- let_it_be(:user) { create(:user, :project_bot) }
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ let_it_be(:user) { create(:user, :project_bot) }
- it { is_expected.to have_gitlab_http_status(:success) }
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it { is_expected.to have_gitlab_http_status(:success) }
+ end
+
+ context 'with packages_dependency_proxy_pass_token_to_policy enabled' do
+ let_it_be(:bot_user) { create(:user, :project_bot) }
+ let_it_be(:user) { create(:personal_access_token, user: bot_user) }
+
+ it { is_expected.to have_gitlab_http_status(:success) }
+ end
end
context 'service account user' do
- let_it_be(:user) { create(:user, :service_account) }
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ let_it_be(:user) { create(:user, :service_account) }
- it { is_expected.to have_gitlab_http_status(:success) }
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it { is_expected.to have_gitlab_http_status(:success) }
+ end
+
+ context 'with packages_dependency_proxy_pass_token_to_policy enabled' do
+ let_it_be(:service_account_user) { create(:user, :service_account) }
+ let_it_be(:user) { create(:personal_access_token, user: service_account_user) }
+
+ it { is_expected.to have_gitlab_http_status(:success) }
+ end
end
context 'deploy token' do
let_it_be(:user) { create(:deploy_token) }
- it { is_expected.to have_gitlab_http_status(:success) }
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it { is_expected.to have_gitlab_http_status(:success) }
+ end
+
+ context 'with packages_dependency_proxy_pass_token_to_policy enabled' do
+ it { is_expected.to have_gitlab_http_status(:success) }
+ end
end
end
diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
index 9b9772484350a088c07a6909fe35b54a3cffbec5..34ca70e17fec5fe1edac868061b2338a971ab1a8 100644
--- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
+++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
@@ -62,6 +62,8 @@
context 'with invalid group access token' do
let_it_be(:user) { create(:user, :project_bot) }
+ let_it_be(:token) { create(:personal_access_token, user: user, scopes: [Gitlab::Auth::READ_API_SCOPE]) }
+ let_it_be(:jwt) { build_jwt(token) }
context 'not under the group' do
it { is_expected.to have_gitlab_http_status(:not_found) }
@@ -82,8 +84,6 @@
end
context 'with insufficient scopes' do
- let_it_be(:pat) { create(:personal_access_token, user: user, scopes: [Gitlab::Auth::READ_API_SCOPE]) }
-
it { is_expected.to have_gitlab_http_status(:not_found) }
context 'packages_dependency_proxy_containers_scope_check disabled' do
@@ -193,7 +193,19 @@
token.update_column(:scopes, Gitlab::Auth::REGISTRY_SCOPES)
end
- it_behaves_like 'sends Workhorse instructions'
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it_behaves_like 'sends Workhorse instructions'
+ end
+
+ context 'with packages_dependency_proxy_pass_token_to_policy enabled' do
+ let_it_be(:jwt) { build_jwt(token) }
+
+ it_behaves_like 'sends Workhorse instructions'
+ end
end
context 'with a deploy token' do
@@ -293,6 +305,15 @@
it_behaves_like 'a successful manifest pull'
it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest', false
+ context 'when packages_dependency_proxy_pass_token_to_policy is disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_containers_scope_check: false)
+ end
+
+ it_behaves_like 'a successful manifest pull'
+ it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest', false
+ end
+
context 'with workhorse response' do
let(:pull_response) { { status: :success, manifest: nil, from_cache: false } }
@@ -324,6 +345,14 @@
it_behaves_like 'a successful manifest pull'
+ context 'when packages_dependency_proxy_pass_token_to_policy is disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_containers_scope_check: false)
+ end
+
+ it_behaves_like 'a successful manifest pull'
+ end
+
context 'pulling from a subgroup' do
let_it_be_with_reload(:parent_group) { create(:group) }
let_it_be_with_reload(:group) { create(:group, parent: parent_group) }
@@ -344,8 +373,21 @@
group.add_guest(user)
end
- it_behaves_like 'a successful manifest pull'
- it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest', false
+ context 'when packages_dependency_proxy_pass_token_to_policy is disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it_behaves_like 'a successful manifest pull'
+ it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest', false
+ end
+
+ context 'when packages_dependency_proxy_pass_token_to_policy is enabled' do
+ let_it_be(:jwt) { build_jwt(token) }
+
+ it_behaves_like 'a successful manifest pull'
+ it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest', false
+ end
end
end
@@ -367,6 +409,14 @@ def get_manifest(tag)
it_behaves_like 'without a token'
it_behaves_like 'without permission'
+ context 'when packages_dependency_proxy_pass_token_to_policy is disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_containers_scope_check: false)
+ end
+
+ it { is_expected.to have_gitlab_http_status(:not_found) }
+ end
+
context 'a valid user' do
before do
group.add_guest(user)
diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index 1eae4ae3c15bb7050c2937acb7141b907e2758d2..4862e12f02f9acc06738ef1b7f7d2c8817b96ecf 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -1114,6 +1114,7 @@
end
end
+ # This block can be removed when packages_dependency_proxy_pass_token_to_policy is rolled out
describe 'dependency proxy' do
RSpec.shared_examples 'disabling admin_package feature flag' do
before do
diff --git a/spec/policies/packages/policies/dependency_proxy/group_policy_spec.rb b/spec/policies/packages/policies/dependency_proxy/group_policy_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..15b14d69ab0df243a8224d4113138900af39e1d8
--- /dev/null
+++ b/spec/policies/packages/policies/dependency_proxy/group_policy_spec.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Policies::DependencyProxy::GroupPolicy, feature_category: :system_access do
+ subject { described_class.new(auth_token, group.dependency_proxy_for_containers_policy_subject) }
+
+ let_it_be(:guest) { create(:user) }
+ let_it_be(:non_group_member) { create(:user) }
+ let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only) }
+ let_it_be(:current_user) { guest }
+
+ before do
+ group.add_guest(guest)
+ end
+
+ describe 'dependency proxy' do
+ shared_examples 'disallows dependency proxy read access' do
+ it { is_expected.to be_disallowed(:read_dependency_proxy) }
+ end
+
+ shared_examples 'allows dependency proxy read access' do
+ it { is_expected.to be_allowed(:read_dependency_proxy) }
+ end
+
+ context 'with feature disabled' do
+ let_it_be(:auth_token) { create(:personal_access_token, user: current_user) }
+
+ before do
+ stub_config(dependency_proxy: { enabled: false })
+ end
+
+ it_behaves_like 'disallows dependency proxy read access'
+ end
+
+ context 'with feature enabled' do
+ let_it_be(:auth_token) { create(:personal_access_token, user: current_user) }
+
+ before do
+ stub_config(dependency_proxy: { enabled: true }, registry: { enabled: true })
+ end
+
+ context 'with a human user personal access token' do
+ subject { described_class.new(auth_token, group.dependency_proxy_for_containers_policy_subject) }
+
+ context 'when not a member of the group' do
+ let_it_be(:current_user) { non_group_member }
+ let_it_be(:auth_token) { create(:personal_access_token, user: current_user) }
+
+ it_behaves_like 'disallows dependency proxy read access'
+ end
+
+ context 'when a member of the group' do
+ let_it_be(:current_user) { guest }
+ let_it_be(:auth_token) { create(:personal_access_token, user: current_user) }
+
+ it_behaves_like 'allows dependency proxy read access'
+ end
+ end
+
+ context 'with a deploy token user' do
+ before do
+ create(:group_deploy_token, group: group, deploy_token: auth_token)
+ end
+
+ context 'with insufficient scopes' do
+ let_it_be(:auth_token) { create(:deploy_token, :group, user: current_user) }
+
+ it_behaves_like 'disallows dependency proxy read access'
+ end
+
+ context 'with sufficient scopes' do
+ let_it_be(:auth_token) { create(:deploy_token, :group, :dependency_proxy_scopes) }
+
+ it_behaves_like 'allows dependency proxy read access'
+ end
+ end
+
+ context 'with a group access token user' do
+ let_it_be(:bot_user) { create(:user, :project_bot) }
+ let_it_be(:auth_token) do
+ create(:personal_access_token, user: bot_user, scopes: [Gitlab::Auth::READ_API_SCOPE])
+ end
+
+ context 'when not a member of the group' do
+ it_behaves_like 'disallows dependency proxy read access'
+ end
+
+ context 'when a member of the group' do
+ before do
+ group.add_guest(bot_user)
+ end
+
+ it_behaves_like 'allows dependency proxy read access'
+ end
+ end
+ end
+ end
+end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index 992c5ab0b136199829953f15c19714017ed6efcb..539055d103e5abeaf7f17eaf72fea483f79160c0 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -92,7 +92,7 @@
context 'project with enabled CI' do
subject! { get '/jwt/auth', params: parameters, headers: headers }
- it { expect(service_class).to have_received(:new).with(project, user, ActionController::Parameters.new(parameters.merge(auth_type: :build, raw_token: build.token)).permit!) }
+ it { expect(service_class).to have_received(:new).with(project, user, ActionController::Parameters.new(parameters.merge(auth_type: :build)).permit!) }
it_behaves_like 'user logging'
end
@@ -119,7 +119,7 @@
.with(
nil,
nil,
- ActionController::Parameters.new(parameters.merge(deploy_token: deploy_token, auth_type: :deploy_token, raw_token: deploy_token.token)).permit!
+ ActionController::Parameters.new(parameters.merge(deploy_token: deploy_token, auth_type: :deploy_token)).permit!
)
end
@@ -160,7 +160,7 @@
subject! { get '/jwt/auth', params: parameters, headers: headers }
- it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters.merge(auth_type: :gitlab_or_ldap, raw_token: user.password)).permit!) }
+ it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters.merge(auth_type: :gitlab_or_ldap)).permit!) }
it_behaves_like 'rejecting a blocked user'
@@ -180,7 +180,7 @@
ActionController::Parameters.new({ service: service_name, scopes: %w[scope1 scope2] }).permit!
end
- it { expect(service_class).to have_received(:new).with(nil, user, service_parameters.merge(auth_type: :gitlab_or_ldap, raw_token: user.password)) }
+ it { expect(service_class).to have_received(:new).with(nil, user, service_parameters.merge(auth_type: :gitlab_or_ldap)) }
it_behaves_like 'user logging'
end
@@ -197,7 +197,7 @@
ActionController::Parameters.new({ service: service_name, scopes: %w[scope1 scope2] }).permit!
end
- it { expect(service_class).to have_received(:new).with(nil, user, service_parameters.merge(auth_type: :gitlab_or_ldap, raw_token: user.password)) }
+ it { expect(service_class).to have_received(:new).with(nil, user, service_parameters.merge(auth_type: :gitlab_or_ldap)) }
end
context 'when user has 2FA enabled' do
diff --git a/spec/services/auth/dependency_proxy_authentication_service_spec.rb b/spec/services/auth/dependency_proxy_authentication_service_spec.rb
index e81f59cff39cf30211674ec37f38789200553203..2cd3aad9aa818b5ae5f691d78efe67e34edda48a 100644
--- a/spec/services/auth/dependency_proxy_authentication_service_spec.rb
+++ b/spec/services/auth/dependency_proxy_authentication_service_spec.rb
@@ -6,7 +6,7 @@
let_it_be(:user) { create(:user) }
let_it_be(:params) { {} }
- let(:authentication_abilities) { nil }
+ let(:authentication_abilities) { [] }
let(:service) { described_class.new(nil, user, params) }
before do
@@ -48,31 +48,57 @@
end
context 'with a deploy token' do
+ let(:user) { nil }
let_it_be(:deploy_token) { create(:deploy_token, :group, :dependency_proxy_scopes) }
let_it_be(:params) { { deploy_token: deploy_token } }
it_behaves_like 'returning a token with an encoded field', 'deploy_token'
+
+ context 'with packages_dependency_proxy_containers_scope_check disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_containers_scope_check: false)
+ end
+
+ it_behaves_like 'returning a token with an encoded field', 'deploy_token'
+ end
+
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it_behaves_like 'returning a token with an encoded field', 'deploy_token'
+ end
end
context 'with a human user' do
it_behaves_like 'returning a token with an encoded field', 'user_id'
- end
- context 'all other user types' do
- User::USER_TYPES.except(:human, :project_bot).each_value do |user_type|
- context "with user_type #{user_type}" do
- before do
- user.update!(user_type: user_type)
- end
-
- it_behaves_like 'returning a token with an encoded field', 'user_id'
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
end
+
+ it_behaves_like 'returning a token with an encoded field', 'user_id'
end
end
+ context 'with a personal access token user' do
+ let_it_be_with_reload(:token) { create(:personal_access_token, user: user) }
+ let_it_be(:params) { { raw_token: token.token } }
+
+ it_behaves_like 'returning a token with an encoded field', 'personal_access_token'
+ end
+
context 'with a group access token' do
let_it_be(:user) { create(:user, :project_bot) }
+ let_it_be(:group) { create(:group) }
let_it_be_with_reload(:token) { create(:personal_access_token, user: user) }
+ let_it_be(:params) { { raw_token: token.token } }
+
+ before_all do
+ group.add_guest(user)
+ end
context 'with insufficient authentication abilities' do
it_behaves_like 'returning', status: 403, message: 'access forbidden'
@@ -82,7 +108,7 @@
stub_feature_flags(packages_dependency_proxy_containers_scope_check: false)
end
- it_behaves_like 'returning a token with an encoded field', 'user_id'
+ it_behaves_like 'returning a token with an encoded field', 'group_access_token'
end
end
@@ -92,7 +118,15 @@
subject { service.execute(authentication_abilities: authentication_abilities) }
- it_behaves_like 'returning a token with an encoded field', 'user_id'
+ it_behaves_like 'returning a token with an encoded field', 'group_access_token'
+
+ context 'with packages_dependency_proxy_pass_token_to_policy disabled' do
+ before do
+ stub_feature_flags(packages_dependency_proxy_pass_token_to_policy: false)
+ end
+
+ it_behaves_like 'returning a token with an encoded field', 'user_id'
+ end
context 'revoked' do
before do
@@ -112,6 +146,18 @@
end
end
+ context 'all other user types' do
+ User::USER_TYPES.except(:human, :project_bot).each_value do |user_type|
+ context "with user_type #{user_type}" do
+ before do
+ user.update!(user_type: user_type)
+ end
+
+ it_behaves_like 'returning a token with an encoded field', 'user_id'
+ end
+ end
+ end
+
def decode(token)
DependencyProxy::AuthTokenService.new(token).execute
end
diff --git a/spec/services/dependency_proxy/auth_token_service_spec.rb b/spec/services/dependency_proxy/auth_token_service_spec.rb
index 2612c5765a49f7d4d860667c9ff19b1d0dcbb0ac..43a672032b93290f428d4ef34ae6d4dd91379754 100644
--- a/spec/services/dependency_proxy/auth_token_service_spec.rb
+++ b/spec/services/dependency_proxy/auth_token_service_spec.rb
@@ -5,8 +5,36 @@
include DependencyProxyHelpers
let_it_be(:user) { create(:user) }
+ let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
+ let_it_be(:group_access_token) { create(:personal_access_token, user: user) }
let_it_be(:deploy_token) { create(:deploy_token) }
+ shared_examples 'handling token errors' do
+ context 'with a decoding error' do
+ before do
+ allow(JWT).to receive(:decode).and_raise(JWT::DecodeError)
+ end
+
+ it { is_expected.to eq(nil) }
+ end
+
+ context 'with an immature signature error' do
+ before do
+ allow(JWT).to receive(:decode).and_raise(JWT::ImmatureSignature)
+ end
+
+ it { is_expected.to eq(nil) }
+ end
+
+ context 'with an expired signature error' do
+ it 'returns nil' do
+ travel_to(Time.zone.now + Auth::DependencyProxyAuthenticationService.token_expire_at + 1.minute) do
+ expect(subject).to eq(nil)
+ end
+ end
+ end
+ end
+
describe '.user_or_deploy_token_from_jwt' do
subject { described_class.user_or_deploy_token_from_jwt(token.encoded) }
@@ -72,4 +100,84 @@
it { is_expected.to eq(nil) }
end
end
+
+ describe '.user_or_token_from_jwt' do
+ subject { described_class.user_or_token_from_jwt(token.encoded) }
+
+ context 'with a user' do
+ let_it_be(:token) { build_jwt(user) }
+
+ it { is_expected.to eq(user) }
+
+ context 'with an invalid user id' do
+ let_it_be(:token) { build_jwt { |jwt| jwt['user_id'] = 'this_is_not_a_user_id' } }
+
+ it 'raises an not found error' do
+ expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+
+ it_behaves_like 'handling token errors'
+ end
+
+ context 'with a personal access token' do
+ let_it_be(:token) { build_jwt(personal_access_token) }
+
+ it { is_expected.to eq(personal_access_token) }
+
+ context 'with an inactive token' do
+ before do
+ personal_access_token.revoke!
+ end
+
+ it { is_expected.to eq(nil) }
+ end
+
+ context 'with an invalid token' do
+ let_it_be(:token) { build_jwt { |jwt| jwt['personal_access_token'] = 'this_is_not_a_token' } }
+
+ it { is_expected.to eq(nil) }
+ end
+ end
+
+ context 'with a group access token' do
+ let_it_be(:token) { build_jwt(group_access_token) }
+
+ it { is_expected.to eq(group_access_token) }
+
+ context 'with an inactive token' do
+ before do
+ group_access_token.revoke!
+ end
+
+ it { is_expected.to eq(nil) }
+ end
+
+ context 'with an invalid token' do
+ let_it_be(:token) { build_jwt { |jwt| jwt['group_access_token'] = 'this_is_not_a_token' } }
+
+ it { is_expected.to eq(nil) }
+ end
+ end
+
+ context 'with a deploy token' do
+ let_it_be(:token) { build_jwt(deploy_token) }
+
+ it { is_expected.to eq(deploy_token) }
+
+ context 'with an invalid token' do
+ let_it_be(:token) { build_jwt { |jwt| jwt['deploy_token'] = 'this_is_not_a_token' } }
+
+ it { is_expected.to eq(nil) }
+ end
+
+ it_behaves_like 'handling token errors'
+ end
+
+ context 'with an empty token payload' do
+ let_it_be(:token) { build_jwt(nil) }
+
+ it { is_expected.to eq(nil) }
+ end
+ end
end
diff --git a/spec/support/helpers/dependency_proxy_helpers.rb b/spec/support/helpers/dependency_proxy_helpers.rb
index 75dc09ec159e32b8fb3dd1064f21d0b1e6422524..ae54c64a4d972684b3d543174787c67990ef22f5 100644
--- a/spec/support/helpers/dependency_proxy_helpers.rb
+++ b/spec/support/helpers/dependency_proxy_helpers.rb
@@ -32,13 +32,14 @@ def stub_blob_download(image, blob_sha, status = 200, body = '123456')
.to_return(status: status, body: body)
end
- def build_jwt(user = nil, expire_time: nil)
+ def build_jwt(user_or_token = nil, expire_time: nil)
JSONWebToken::HMACToken.new(::Auth::DependencyProxyAuthenticationService.secret).tap do |jwt|
if block_given?
yield(jwt)
else
- jwt['user_id'] = user.id if user.is_a?(User)
- jwt['deploy_token'] = user.token if user.is_a?(DeployToken)
+ jwt['user_id'] = user_or_token.id if user_or_token.is_a?(User)
+ jwt['personal_access_token'] = user_or_token.token if user_or_token.is_a?(PersonalAccessToken)
+ jwt['deploy_token'] = user_or_token.token if user_or_token.is_a?(DeployToken)
jwt.expire_time = expire_time || jwt.issued_at + 1.minute
end
end