diff options
author | Rémy Coutable <remy@rymai.me> | 2016-09-19 16:36:54 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-09-19 16:36:54 +0300 |
commit | fe084819b4c0aa83ec80b5915e7b3f444b693e9f (patch) | |
tree | 3e0816289920f6e1ef4f9e3d4e189e7e53217f08 /app | |
parent | 1e72de669018252c2eb0bc086d66c74cfbbe1a0a (diff) | |
parent | 135be3cabb01ca3c825829f18ede4e8720383d7b (diff) |
Merge branch 'per-build-token-without-lfs' into 'master'
Make CI to use the permission of the user who is trigger the build
This is continuation of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5735, but with removed all LFS code that is added by: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6043.
This also incorporates most of LFS code added in !6043 to simplify further merge.
See merge request !6409
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/jwt_controller.rb | 36 | ||||
-rw-r--r-- | app/controllers/projects/builds_controller.rb | 6 | ||||
-rw-r--r-- | app/controllers/projects/git_http_client_controller.rb | 62 | ||||
-rw-r--r-- | app/controllers/projects/git_http_controller.rb | 2 | ||||
-rw-r--r-- | app/helpers/lfs_helper.rb | 12 | ||||
-rw-r--r-- | app/models/ci/build.rb | 30 | ||||
-rw-r--r-- | app/models/project.rb | 6 | ||||
-rw-r--r-- | app/policies/project_policy.rb | 17 | ||||
-rw-r--r-- | app/services/auth/container_registry_authentication_service.rb | 32 |
9 files changed, 138 insertions, 65 deletions
diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 66ebdcc37a7..06d96774754 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -11,7 +11,10 @@ class JwtController < ApplicationController service = SERVICES[params[:service]] return head :not_found unless service - result = service.new(@project, @user, auth_params).execute + @authentication_result ||= Gitlab::Auth::Result.new + + result = service.new(@authentication_result.project, @authentication_result.actor, auth_params). + execute(authentication_abilities: @authentication_result.authentication_abilities) render json: result, status: result[:http_status] end @@ -20,30 +23,23 @@ 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) - return if @project - - @user = authenticate_user(login, password) - return if @user + @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip) - render_403 + render_403 unless @authentication_result.success? && + (@authentication_result.actor.nil? || @authentication_result.actor.is_a?(User)) end + rescue Gitlab::Auth::MissingPersonalTokenError + render_missing_personal_token end - def auth_params - params.permit(:service, :scope, :account, :client_id) + def render_missing_personal_token + render plain: "HTTP Basic: Access denied\n" \ + "You have 2FA enabled, please use a personal access token for Git over HTTP.\n" \ + "You can generate one at #{profile_personal_access_tokens_url}", + status: 401 end - def authenticate_project(login, password) - if login == 'gitlab-ci-token' - Project.with_builds_enabled.find_by(runners_token: password) - end - 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 + def auth_params + params.permit(:service, :scope, :account, :client_id) end end diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index f13fb1df373..3b2e35a7a05 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -35,7 +35,11 @@ class Projects::BuildsController < Projects::ApplicationController respond_to do |format| format.html format.json do - render json: @build.to_json(methods: :trace_html) + render json: { + id: @build.id, + status: @build.status, + trace_html: @build.trace_html + } end end end diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index f5ce63fdfed..d1a2c52d80a 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -4,7 +4,11 @@ class Projects::GitHttpClientController < Projects::ApplicationController include ActionController::HttpAuthentication::Basic include KerberosSpnegoHelper - attr_reader :user + attr_reader :authentication_result + + delegate :actor, :authentication_abilities, to: :authentication_result, allow_nil: true + + alias_method :user, :actor # Git clients will not know what authenticity token to send along skip_before_action :verify_authenticity_token @@ -15,32 +19,25 @@ class Projects::GitHttpClientController < Projects::ApplicationController private def authenticate_user + @authentication_result = Gitlab::Auth::Result.new + if project && project.public? && download_request? return # Allow access end if allow_basic_auth? && basic_auth_provided? login, password = user_name_and_password(request) - auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) - - if auth_result.type == :ci && download_request? - @ci = true - elsif auth_result.type == :oauth && !download_request? - # Not allowed - elsif auth_result.type == :missing_personal_token - render_missing_personal_token - return # Render above denied access, nothing left to do - else - @user = auth_result.user - end - if ci? || user + if handle_basic_authentication(login, password) return # Allow access end elsif allow_kerberos_spnego_auth? && spnego_provided? - @user = find_kerberos_user + user = find_kerberos_user if user + @authentication_result = Gitlab::Auth::Result.new( + user, nil, :kerberos, Gitlab::Auth.full_authentication_abilities) + send_final_spnego_response return # Allow access end @@ -48,6 +45,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController send_challenges render plain: "HTTP Basic: Access denied\n", status: 401 + rescue Gitlab::Auth::MissingPersonalTokenError + render_missing_personal_token end def basic_auth_provided? @@ -114,8 +113,39 @@ class Projects::GitHttpClientController < Projects::ApplicationController render plain: 'Not Found', status: :not_found end + def handle_basic_authentication(login, password) + @authentication_result = Gitlab::Auth.find_for_git_client( + login, password, project: project, ip: request.ip) + + return false unless @authentication_result.success? + + if download_request? + authentication_has_download_access? + else + authentication_has_upload_access? + end + end + def ci? - @ci.present? + authentication_result.ci? && + authentication_project && + authentication_project == project + end + + def authentication_has_download_access? + has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code) + end + + def authentication_has_upload_access? + has_authentication_ability?(:push_code) + end + + def has_authentication_ability?(capability) + (authentication_abilities || []).include?(capability) + end + + def authentication_project + authentication_result.project end def verify_workhorse_api! diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 9805705c4e3..662d38b10a5 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', authentication_abilities: authentication_abilities) end def access_check diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index 5d82abfca79..8e827664681 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -25,13 +25,21 @@ module LfsHelper def lfs_download_access? return false unless project.lfs_enabled? - project.public? || ci? || (user && user.can?(:download_code, project)) + project.public? || ci? || user_can_download_code? || build_can_download_code? + end + + def user_can_download_code? + has_authentication_ability?(:download_code) && can?(user, :download_code, project) + end + + def build_can_download_code? + has_authentication_ability?(:build_download_code) && can?(user, :build_download_code, project) end def lfs_upload_access? return false unless project.lfs_enabled? - user && user.can?(:push_code, project) + has_authentication_ability?(:push_code) && can?(user, :push_code, project) end def render_lfs_forbidden diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d5724af4cce..dd984aef318 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 @@ -38,6 +43,7 @@ module Ci new_build.status = 'pending' new_build.runner_id = nil new_build.trigger_request_id = nil + new_build.token = nil new_build.save end @@ -176,7 +182,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 @@ -238,12 +244,7 @@ module Ci end def trace - trace = raw_trace - if project && trace.present? && project.runners_token.present? - trace.gsub(project.runners_token, 'xxxxxx') - else - trace - end + hide_secrets(raw_trace) end def trace_length @@ -256,6 +257,7 @@ module Ci def trace=(trace) recreate_trace_dir + trace = hide_secrets(trace) File.write(path_to_trace, trace) end @@ -269,6 +271,8 @@ module Ci def append_trace(trace_part, offset) recreate_trace_dir + trace_part = hide_secrets(trace_part) + File.truncate(path_to_trace, offset) if File.exist?(path_to_trace) File.open(path_to_trace, 'ab') do |f| f.write(trace_part) @@ -344,12 +348,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? @@ -491,5 +491,11 @@ module Ci pipeline.config_processor.build_attributes(name) end + + def hide_secrets(trace) + trace = Ci::MaskSecret.mask(trace, project.runners_token) if project + trace = Ci::MaskSecret.mask(trace, token) + trace + end end end diff --git a/app/models/project.rb b/app/models/project.rb index 8b5a6f167bd..d7f20070be0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1137,12 +1137,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..00c4c7b1440 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 team member of a project + def team_member_reporter_access! + can! :build_download_code + can! :build_read_container_image + end + def developer_access! can! :admin_merge_request can! :update_merge_request @@ -109,6 +115,8 @@ class ProjectPolicy < BasePolicy can! :read_commit_status can! :read_pipeline can! :read_container_image + can! :build_download_code + can! :build_read_container_image end def owner_access! @@ -130,10 +138,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 + team_member_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..98da6563947 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(authentication_abilities:) + @authentication_abilities = authentication_abilities || [] + 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) + requested_project.public? || build_can_pull?(requested_project) || user_can_pull?(requested_project) when 'push' - requested_project == project || can?(current_user, :create_container_image, requested_project) + build_can_push?(requested_project) || user_can_push?(requested_project) else false end @@ -85,5 +87,29 @@ module Auth def registry Gitlab.config.registry end + + def build_can_pull?(requested_project) + # Build can: + # 1. pull from its own project (for ex. a build) + # 2. read images from dependent projects if creator of build is a team member + @authentication_abilities.include?(:build_read_container_image) && + (requested_project == project || can?(current_user, :build_read_container_image, requested_project)) + end + + def user_can_pull?(requested_project) + @authentication_abilities.include?(:read_container_image) && + can?(current_user, :read_container_image, requested_project) + end + + def build_can_push?(requested_project) + # Build can push only to the project from which it originates + @authentication_abilities.include?(:build_create_container_image) && + requested_project == project + end + + def user_can_push?(requested_project) + @authentication_abilities.include?(:create_container_image) && + can?(current_user, :create_container_image, requested_project) + end end end |