From 505dc808b3c0dc98413506446d368b91b56ff682 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 8 Aug 2016 12:01:25 +0200
Subject: [PATCH] Use a permissions of user to access all dependent projects
from CI jobs (this also includes a container images, and in future LFS files)
---
app/controllers/jwt_controller.rb | 18 +++++----
.../projects/git_http_client_controller.rb | 12 +++++-
.../projects/git_http_controller.rb | 2 +-
app/helpers/lfs_helper.rb | 16 +++++++-
app/models/ci/build.rb | 13 +++---
app/models/project.rb | 6 ---
app/policies/project_policy.rb | 15 +++++--
...ntainer_registry_authentication_service.rb | 40 +++++++++++++++++--
.../20160808085531_add_token_to_build.rb | 10 +++++
...0160808085602_add_index_for_build_token.rb | 12 ++++++
lib/ci/api/helpers.rb | 14 +++++--
lib/gitlab/auth.rb | 31 ++++++++++----
lib/gitlab/git_access.rb | 19 +++++++--
13 files changed, 163 insertions(+), 45 deletions(-)
create mode 100644 db/migrate/20160808085531_add_token_to_build.rb
create mode 100644 db/migrate/20160808085602_add_index_for_build_token.rb
diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 66ebdcc37a7..ca02df28b91 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -11,7 +11,7 @@ class JwtController < ApplicationController
service = SERVICES[params[:service]]
return head :not_found unless service
- result = service.new(@project, @user, auth_params).execute
+ result = service.new(@project, @user, auth_params).execute(access_type: @access_type)
render json: result, status: result[:http_status]
end
@@ -21,10 +21,10 @@ class JwtController < ApplicationController
def authenticate_project_or_user
authenticate_with_http_basic do |login, password|
# if it's possible we first try to authenticate project with login and password
- @project = authenticate_project(login, password)
+ @project, @user, @access_type = authenticate_build(login, password)
return if @project
- @user = authenticate_user(login, password)
+ @user, @access_type = authenticate_user(login, password)
return if @user
render_403
@@ -35,15 +35,17 @@ class JwtController < ApplicationController
params.permit(:service, :scope, :account, :client_id)
end
- def authenticate_project(login, password)
- if login == 'gitlab-ci-token'
- Project.with_builds_enabled.find_by(runners_token: password)
- end
+ def authenticate_build(login, password)
+ return unless login == 'gitlab-ci-token'
+ return unless password
+
+ build = Ci::Build.running.find_by(token: password)
+ return build.project, build.user, :restricted if build
end
def authenticate_user(login, password)
user = Gitlab::Auth.find_with_user_password(login, password)
Gitlab::Auth.rate_limit!(request.ip, success: user.present?, login: login)
- user
+ return user, :full
end
end
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index f5ce63fdfed..0f72dc8437c 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
include ActionController::HttpAuthentication::Basic
include KerberosSpnegoHelper
- attr_reader :user
+ attr_reader :user, :access_type
# Git clients will not know what authenticity token to send along
skip_before_action :verify_authenticity_token
@@ -34,6 +34,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
@user = auth_result.user
end
+ @access_type = auth_result.access_type
+
if ci? || user
return # Allow access
end
@@ -118,6 +120,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController
@ci.present?
end
+ def full?
+ @access_type == :full
+ end
+
+ def restricted?
+ @access_type == :restricted
+ end
+
def verify_workhorse_api!
Gitlab::Workhorse.verify_api_request!(request.headers)
end
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 9805705c4e3..d59a47417f4 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -86,7 +86,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
end
def access
- @access ||= Gitlab::GitAccess.new(user, project, 'http')
+ @access ||= Gitlab::GitAccess.new(user, project, 'http', access_type: access_type)
end
def access_check
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 5d82abfca79..625dfddcf8d 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,13 +25,25 @@ module LfsHelper
def lfs_download_access?
return false unless project.lfs_enabled?
- project.public? || ci? || (user && user.can?(:download_code, project))
+ project.public? || ci? || privileged_user_can_download_code? || restricted_user_can_download_code?
+ end
+
+ def privileged_user_can_download_code?
+ full? && user && user.can?(:download_code, project)
+ end
+
+ def restricted_user_can_download_code?
+ restricted? && user && user.can?(:restricted_download_code, project)
end
def lfs_upload_access?
return false unless project.lfs_enabled?
- user && user.can?(:push_code, project)
+ privileged_user_can_push_code?
+ end
+
+ def privileged_user_can_push_code?
+ full? && user && user.can?(:push_code, project)
end
def render_lfs_forbidden
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 61052437318..1c2e0f1edea 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -1,5 +1,7 @@
module Ci
class Build < CommitStatus
+ include TokenAuthenticatable
+
belongs_to :runner, class_name: 'Ci::Runner'
belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
belongs_to :erased_by, class_name: 'User'
@@ -23,7 +25,10 @@ module Ci
acts_as_taggable
+ add_authentication_token_field :token
+
before_save :update_artifacts_size, if: :artifacts_file_changed?
+ before_save :ensure_token
before_destroy { project }
after_create :execute_hooks
@@ -172,7 +177,7 @@ module Ci
end
def repo_url
- auth = "gitlab-ci-token:#{token}@"
+ auth = "gitlab-ci-token:#{ensure_token}@"
project.http_url_to_repo.sub(/^https?:\/\//) do |prefix|
prefix + auth
end
@@ -340,12 +345,8 @@ module Ci
)
end
- def token
- project.runners_token
- end
-
def valid_token?(token)
- project.valid_runners_token?(token)
+ self.token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token)
end
def has_tags?
diff --git a/app/models/project.rb b/app/models/project.rb
index a6de2c48071..d7cdf8775b3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1138,12 +1138,6 @@ class Project < ActiveRecord::Base
self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
end
- # TODO (ayufan): For now we use runners_token (backward compatibility)
- # In 8.4 every build will have its own individual token valid for time of build
- def valid_build_token?(token)
- self.builds_enabled? && self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
- end
-
def build_coverage_enabled?
build_coverage_regex.present?
end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index acf36d422d1..cda83bcc74a 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -64,6 +64,12 @@ class ProjectPolicy < BasePolicy
can! :read_deployment
end
+ # Permissions given when an user is direct member of a group
+ def restricted_reporter_access!
+ can! :restricted_download_code
+ can! :restricted_read_container_image
+ end
+
def developer_access!
can! :admin_merge_request
can! :update_merge_request
@@ -130,10 +136,11 @@ class ProjectPolicy < BasePolicy
def team_access!(user)
access = project.team.max_member_access(user.id)
- guest_access! if access >= Gitlab::Access::GUEST
- reporter_access! if access >= Gitlab::Access::REPORTER
- developer_access! if access >= Gitlab::Access::DEVELOPER
- master_access! if access >= Gitlab::Access::MASTER
+ guest_access! if access >= Gitlab::Access::GUEST
+ reporter_access! if access >= Gitlab::Access::REPORTER
+ restricted_reporter_access! if access >= Gitlab::Access::REPORTER
+ developer_access! if access >= Gitlab::Access::DEVELOPER
+ master_access! if access >= Gitlab::Access::MASTER
end
def archived_access!
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 6072123b851..270d5a11d9e 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -4,7 +4,9 @@ module Auth
AUDIENCE = 'container_registry'
- def execute
+ def execute(access_type: access_type)
+ @access_type = access_type
+
return error('not found', 404) unless registry.enabled
unless current_user || project
@@ -74,9 +76,9 @@ module Auth
case requested_action
when 'pull'
- requested_project == project || can?(current_user, :read_container_image, requested_project)
+ restricted_user_can_pull?(requested_project) || privileged_user_can_pull?(requested_project)
when 'push'
- requested_project == project || can?(current_user, :create_container_image, requested_project)
+ restricted_user_can_push?(requested_project) || privileged_user_can_push?(requested_project)
else
false
end
@@ -85,5 +87,37 @@ module Auth
def registry
Gitlab.config.registry
end
+
+ private
+
+ def restricted_user_can_pull?(requested_project)
+ return false unless restricted?
+
+ # Restricted can:
+ # 1. pull from it's own project (for ex. a build)
+ # 2. read images from dependent projects if he is a team member
+ requested_project == project || can?(current_user, :restricted_read_container_image, requested_project)
+ end
+
+ def privileged_user_can_pull?(requested_project)
+ full? && can?(current_user, :read_container_image, requested_project)
+ end
+
+ def restricted_user_can_push?(requested_project)
+ # Restricted can push only to project to from which he originates
+ restricted? && requested_project == project
+ end
+
+ def privileged_user_can_push?(requested_project)
+ full? && can?(current_user, :create_container_image, requested_project)
+ end
+
+ def full?
+ @access_type == :full
+ end
+
+ def restricted?
+ @access_type == :restricted
+ end
end
end
diff --git a/db/migrate/20160808085531_add_token_to_build.rb b/db/migrate/20160808085531_add_token_to_build.rb
new file mode 100644
index 00000000000..3ed2a103ae3
--- /dev/null
+++ b/db/migrate/20160808085531_add_token_to_build.rb
@@ -0,0 +1,10 @@
+class AddTokenToBuild < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ def change
+ add_column :ci_builds, :token, :string
+ end
+end
diff --git a/db/migrate/20160808085602_add_index_for_build_token.rb b/db/migrate/20160808085602_add_index_for_build_token.rb
new file mode 100644
index 00000000000..10ef42afce1
--- /dev/null
+++ b/db/migrate/20160808085602_add_index_for_build_token.rb
@@ -0,0 +1,12 @@
+class AddIndexForBuildToken < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def change
+ add_concurrent_index :ci_builds, :token, unique: true
+ end
+end
diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb
index bcabf7a21b2..411e0dea15e 100644
--- a/lib/ci/api/helpers.rb
+++ b/lib/ci/api/helpers.rb
@@ -14,12 +14,20 @@ module Ci
end
def authenticate_build_token!(build)
- token = (params[BUILD_TOKEN_PARAM] || env[BUILD_TOKEN_HEADER]).to_s
- forbidden! unless token && build.valid_token?(token)
+ forbidden! unless build_token_valid?
end
def runner_registration_token_valid?
- params[:token] == current_application_settings.runners_registration_token
+ ActiveSupport::SecurityUtils.variable_size_secure_compare(
+ params[:token],
+ current_application_settings.runners_registration_token)
+ end
+
+ def build_token_valid?
+ token = (params[BUILD_TOKEN_PARAM] || env[BUILD_TOKEN_HEADER]).to_s
+
+ # We require to also check `runners_token` to maintain compatibility with old version of runners
+ token && (build.valid_token?(token) || build.project.valid_runners_token?(token))
end
def update_runner_last_contact(save: true)
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 91f0270818a..e7bf8ee6166 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,6 +1,6 @@
module Gitlab
module Auth
- Result = Struct.new(:user, :type)
+ Result = Struct.new(:user, :type, :access_type)
class << self
def find_for_git_client(login, password, project:, ip:)
@@ -64,9 +64,7 @@ module Gitlab
underscored_service = matched_login['service'].underscore
- if underscored_service == 'gitlab_ci'
- project && project.valid_build_token?(password)
- elsif Service.available_services_names.include?(underscored_service)
+ if Service.available_services_names.include?(underscored_service)
# We treat underscored_service as a trusted input because it is included
# in the Service.available_services_names whitelist.
service = project.public_send("#{underscored_service}_service")
@@ -77,12 +75,13 @@ module Gitlab
def populate_result(login, password)
result =
+ build_access_token_check(login, password) ||
user_with_password_for_git(login, password) ||
oauth_access_token_check(login, password) ||
personal_access_token_check(login, password)
if result
- result.type = nil unless result.user
+ result.type = nil unless result.user && result.type != :ci
if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
result.type = :missing_personal_token
@@ -94,7 +93,7 @@ module Gitlab
def user_with_password_for_git(login, password)
user = find_with_user_password(login, password)
- Result.new(user, :gitlab_or_ldap) if user
+ Result.new(user, :gitlab_or_ldap, :full) if user
end
def oauth_access_token_check(login, password)
@@ -102,7 +101,7 @@ module Gitlab
token = Doorkeeper::AccessToken.by_token(password)
if token && token.accessible?
user = User.find_by(id: token.resource_owner_id)
- Result.new(user, :oauth)
+ Result.new(user, :oauth, :full)
end
end
end
@@ -111,7 +110,23 @@ module Gitlab
if login && password
user = User.find_by_personal_access_token(password)
validation = User.by_login(login)
- Result.new(user, :personal_token) if user == validation
+ Result.new(user, :personal_token, :full) if user == validation
+ end
+ end
+
+ def build_access_token_check(login, password)
+ return unless login == 'gitlab-ci-token'
+ return unless password
+
+ build = Ci::Build.running.find_by_token(password)
+ return unless build
+
+ if build.user
+ # If user is assigned to build, use restricted credentials of user
+ Result.new(build.user, :build, :restricted)
+ else
+ # Otherwise use generic CI credentials (backward compatibility)
+ Result.new(nil, :ci, :restricted)
end
end
end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 1882eb8d050..5bd0134ed45 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -5,12 +5,13 @@ module Gitlab
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
PUSH_COMMANDS = %w{ git-receive-pack }
- attr_reader :actor, :project, :protocol, :user_access
+ attr_reader :actor, :project, :protocol, :user_access, :access_type
- def initialize(actor, project, protocol)
+ def initialize(actor, project, protocol, access_type: access_type)
@actor = actor
@project = project
@protocol = protocol
+ @access_type = access_type
@user_access = UserAccess.new(user, project: project)
end
@@ -60,14 +61,26 @@ module Gitlab
end
def user_download_access_check
- unless user_access.can_do_action?(:download_code)
+ unless privileged_user_can_download_code? || restricted_user_can_download_code?
return build_status_object(false, "You are not allowed to download code from this project.")
end
build_status_object(true)
end
+ def privileged_user_can_download_code?
+ access_type == :full && user_access.can_do_action?(:download_code)
+ end
+
+ def restricted_user_can_download_code?
+ access_type == :restricted && user_access.can_do_action?(:restricted_download_code)
+ end
+
def user_push_access_check(changes)
+ unless access_type == :full
+ return build_status_object(false, "You are not allowed to upload code for this project.")
+ end
+
if changes.blank?
return build_status_object(true)
end
--
GitLab