Read about our upcoming Code of Conduct on this issue

Commit 78ca403c authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Mayra Cabrera
Browse files

Support Deploy Tokens properly without hacking abilities

parent cb3958a334fb
...@@ -29,6 +29,10 @@ def active? ...@@ -29,6 +29,10 @@ def active?
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 @@ def self.create_read_update_admin(name) ...@@ -145,7 +145,7 @@ def self.create_read_update_admin(name)
# 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 @@ def self.create_read_update_admin(name) ...@@ -179,7 +179,7 @@ def self.create_read_update_admin(name)
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 @@ def can_access?(requested_project, requested_action) ...@@ -109,7 +109,7 @@ def can_access?(requested_project, requested_action)
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 @@ def registry ...@@ -123,22 +123,33 @@ def registry
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 @@ def build_can_push?(requested_project) ...@@ -154,7 +165,7 @@ def build_can_push?(requested_project)
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 @@ def find_for_git_client(login, password, project:, ip:) ...@@ -26,7 +26,7 @@ def find_for_git_client(login, password, project:, ip:)
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 @@ def abilities_for_scopes(scopes) ...@@ -176,18 +176,18 @@ def abilities_for_scopes(scopes)
# 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 @@ def build_authentication_abilities ...@@ -242,7 +242,7 @@ def build_authentication_abilities
[ [
: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 @@ def ci? ...@@ -290,10 +290,10 @@ def ci?
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 @@ def operation ...@@ -195,7 +195,7 @@ def operation
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 @@ def operation ...@@ -310,7 +310,7 @@ def operation
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 @@ def build_authentication_abilities ...@@ -477,7 +477,7 @@ def build_authentication_abilities
[ [
: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 @@ ...@@ -28,7 +28,7 @@
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 @@ ...@@ -54,7 +54,7 @@
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 @@ ...@@ -373,7 +373,7 @@
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