Commit 72220a99 authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Mayra Cabrera

Support Deploy Tokens properly without hacking abilities

parent 171b2625
...@@ -29,6 +29,10 @@ class DeployToken < ActiveRecord::Base ...@@ -29,6 +29,10 @@ class DeployToken < ActiveRecord::Base
end end
def username def username
User.ghost.username "gitlab+deploy-token-#{id}"
end
def has_access_to?(project)
self.project == project
end end
end end
...@@ -145,7 +145,7 @@ class ProjectPolicy < BasePolicy ...@@ -145,7 +145,7 @@ class ProjectPolicy < BasePolicy
# These abilities are not allowed to admins that are not members of the project, # These abilities are not allowed to admins that are not members of the project,
# that's why they are defined separately. # that's why they are defined separately.
rule { guest & can?(:download_code) }.enable :build_download_code rule { guest & can?(:download_code) }.enable :build_download_code
rule { guest & can?(:read_container_image) }.enable :project_read_container_image rule { guest & can?(:read_container_image) }.enable :build_read_container_image
rule { can?(:reporter_access) }.policy do rule { can?(:reporter_access) }.policy do
enable :download_code enable :download_code
...@@ -179,7 +179,7 @@ class ProjectPolicy < BasePolicy ...@@ -179,7 +179,7 @@ class ProjectPolicy < BasePolicy
enable :fork_project enable :fork_project
enable :build_download_code enable :build_download_code
enable :project_read_container_image enable :build_read_container_image
enable :request_access enable :request_access
end end
......
...@@ -109,7 +109,7 @@ module Auth ...@@ -109,7 +109,7 @@ module Auth
case requested_action case requested_action
when 'pull' when 'pull'
build_can_pull?(requested_project) || user_can_pull?(requested_project) build_can_pull?(requested_project) || user_can_pull?(requested_project) || deploy_token_can_pull?(requested_project)
when 'push' when 'push'
build_can_push?(requested_project) || user_can_push?(requested_project) build_can_push?(requested_project) || user_can_push?(requested_project)
when '*' when '*'
...@@ -123,22 +123,33 @@ module Auth ...@@ -123,22 +123,33 @@ module Auth
Gitlab.config.registry Gitlab.config.registry
end end
def can_user?(ability, project)
current_user.is_a?(User) &&
can?(current_user, ability, project)
end
def build_can_pull?(requested_project) def build_can_pull?(requested_project)
# Build can: # Build can:
# 1. pull from its own project (for ex. a build) # 1. pull from its own project (for ex. a build)
# 2. read images from dependent projects if creator of build is a team member # 2. read images from dependent projects if creator of build is a team member
has_authentication_ability?(:project_read_container_image) && has_authentication_ability?(:build_read_container_image) &&
(requested_project == project || can?(current_user, :project_read_container_image, requested_project)) (requested_project == project || can_user?(:build_read_container_image, requested_project))
end end
def user_can_admin?(requested_project) def user_can_admin?(requested_project)
has_authentication_ability?(:admin_container_image) && has_authentication_ability?(:admin_container_image) &&
can?(current_user, :admin_container_image, requested_project) can_user?(:admin_container_image, requested_project)
end end
def user_can_pull?(requested_project) def user_can_pull?(requested_project)
has_authentication_ability?(:read_container_image) && has_authentication_ability?(:read_container_image) &&
can?(current_user, :read_container_image, requested_project) can_user?(:read_container_image, requested_project)
end
def deploy_token_can_pull?(requested_project)
has_authentication_ability?(:read_container_image) &&
current_user.is_a?(DeployToken) &&
current_user.has_access_to?(requested_project)
end end
## ##
...@@ -154,7 +165,7 @@ module Auth ...@@ -154,7 +165,7 @@ module Auth
def user_can_push?(requested_project) def user_can_push?(requested_project)
has_authentication_ability?(:create_container_image) && has_authentication_ability?(:create_container_image) &&
can?(current_user, :create_container_image, requested_project) can_user?(current_user, :create_container_image, requested_project)
end end
def error(code, status:, message: '') def error(code, status:, message: '')
......
...@@ -26,7 +26,7 @@ module Gitlab ...@@ -26,7 +26,7 @@ module Gitlab
lfs_token_check(login, password, project) || lfs_token_check(login, password, project) ||
oauth_access_token_check(login, password) || oauth_access_token_check(login, password) ||
personal_access_token_check(password) || personal_access_token_check(password) ||
deploy_token_check(project, password) || deploy_token_check(login, password) ||
user_with_password_for_git(login, password) || user_with_password_for_git(login, password) ||
Gitlab::Auth::Result.new Gitlab::Auth::Result.new
...@@ -176,18 +176,18 @@ module Gitlab ...@@ -176,18 +176,18 @@ module Gitlab
# Project is always sent when using read_scope, # Project is always sent when using read_scope,
# but is not sent when using read_registry scope # but is not sent when using read_registry scope
# (since jwt is not context aware of the project) # (since jwt is not context aware of the project)
def deploy_token_check(project, password) def deploy_token_check(login, password)
return unless password.present? return unless password.present?
token = token =
if project.present? DeployToken.active.find_by(token: password)
DeployToken.active.find_by(project: project, token: password)
else return unless token
DeployToken.active.find_by(token: password) return unless login != "gitlab+deploy-token-#{token.id}"
end
scopes = abilities_for_scopes(token.scopes)
if token && valid_scoped_token?(token, available_scopes) if valid_scoped_token?(token, scopes)
Gitlab::Auth::Result.new(token, token.project, :deploy_token, abilities_for_scopes(token.scopes)) Gitlab::Auth::Result.new(token, token.project, :deploy_token, scopes)
end end
end end
...@@ -242,7 +242,7 @@ module Gitlab ...@@ -242,7 +242,7 @@ module Gitlab
[ [
:read_project, :read_project,
:build_download_code, :build_download_code,
:project_read_container_image, :build_read_container_image,
:build_create_container_image :build_create_container_image
] ]
end end
......
...@@ -290,10 +290,10 @@ module Gitlab ...@@ -290,10 +290,10 @@ module Gitlab
def can_read_project? def can_read_project?
if deploy_key? if deploy_key?
deploy_key.has_access_to?(project) deploy_key.has_access_to?(project)
elsif deploy_token?
deploy_token.has_access_to?(project)
elsif user elsif user
user.can?(:read_project, project) user.can?(:read_project, project)
elsif deploy_token?
deploy_token.active? && deploy_token.project == project
elsif ci? elsif ci?
true # allow CI (build without a user) for backwards compatibility true # allow CI (build without a user) for backwards compatibility
end || Guest.can?(:read_project, project) end || Guest.can?(:read_project, project)
......
...@@ -195,7 +195,7 @@ describe Gitlab::Auth do ...@@ -195,7 +195,7 @@ describe Gitlab::Auth do
personal_access_token = create(:personal_access_token, scopes: ['read_registry']) personal_access_token = create(:personal_access_token, scopes: ['read_registry'])
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, [:read_project, :build_download_code, :project_read_container_image])) expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, [:read_project, :build_download_code, :build_read_container_image]))
end end
end end
...@@ -310,7 +310,7 @@ describe Gitlab::Auth do ...@@ -310,7 +310,7 @@ describe Gitlab::Auth do
end end
it 'succeeds if deploy token does have read_registry as scope' do it 'succeeds if deploy token does have read_registry as scope' do
abilities = %i(read_project build_download_code project_read_container_image) abilities = %i(read_project build_download_code build_read_container_image)
auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities) auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities)
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
...@@ -477,7 +477,7 @@ describe Gitlab::Auth do ...@@ -477,7 +477,7 @@ describe Gitlab::Auth do
[ [
:read_project, :read_project,
:build_download_code, :build_download_code,
:project_read_container_image, :build_read_container_image,
:build_create_container_image :build_create_container_image
] ]
end end
......
...@@ -28,7 +28,7 @@ describe ProjectPolicy do ...@@ -28,7 +28,7 @@ describe ProjectPolicy do
end end
let(:team_member_reporter_permissions) do let(:team_member_reporter_permissions) do
%i[build_download_code project_read_container_image] %i[build_download_code build_read_container_image]
end end
let(:developer_permissions) do let(:developer_permissions) do
...@@ -54,7 +54,7 @@ describe ProjectPolicy do ...@@ -54,7 +54,7 @@ describe ProjectPolicy do
let(:public_permissions) do let(:public_permissions) do
%i[ %i[
download_code fork_project read_commit_status read_pipeline download_code fork_project read_commit_status read_pipeline
read_container_image build_download_code project_read_container_image read_container_image build_download_code build_read_container_image
download_wiki_code download_wiki_code
] ]
end end
......
...@@ -373,7 +373,7 @@ describe Auth::ContainerRegistryAuthenticationService do ...@@ -373,7 +373,7 @@ describe Auth::ContainerRegistryAuthenticationService do
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
let(:authentication_abilities) do let(:authentication_abilities) do
[:project_read_container_image, :build_create_container_image] [:build_read_container_image, :build_create_container_image]
end end
before do before do
......
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