diff options
| -rw-r--r-- | app/controllers/jwt_controller.rb | 18 | ||||
| -rw-r--r-- | app/helpers/lfs_helper.rb | 8 | ||||
| -rw-r--r-- | app/policies/project_policy.rb | 18 | ||||
| -rw-r--r-- | app/services/auth/container_registry_authentication_service.rb | 35 | ||||
| -rw-r--r-- | lib/gitlab/auth.rb | 16 | ||||
| -rw-r--r-- | lib/gitlab/git_access.rb | 8 |
6 files changed, 50 insertions, 53 deletions
diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 1b075cc5e2d..7bf534d8732 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(capabilities: @capabilities) + @@authentication_result ||= Gitlab::Auth.Result.new + + result = service.new(@authentication_result.project, @authentication_result.user, auth_params). + execute(capabilities: @authentication_result.capabilities || []) render json: result, status: result[:http_status] end @@ -20,18 +23,9 @@ class JwtController < ApplicationController def authenticate_project_or_user authenticate_with_http_basic do |login, password| - @auth_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip) - - @user = auth_result.user - @project = auth_result.project - @type = auth_result.type - @capabilities = auth_result.capabilities || [] - - if @user || @project - return # Allow access - end + @authentication_result = Gitlab::Auth.find_for_git_client(login, password, ip: request.ip) - render_403 + render_403 unless @authentication_result.success? end end diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index bee03ffb446..a2359d94443 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -25,15 +25,15 @@ module LfsHelper def lfs_download_access? return false unless project.lfs_enabled? - project.public? || ci? || privileged_user_can_download_code? || restricted_user_can_download_code? + project.public? || ci? || user_can_download_code? || build_can_download_code? end - def privileged_user_can_download_code? + def user_can_download_code? has_capability?(:download_code) && user && user.can?(:download_code, project) end - def restricted_user_can_download_code? - has_capability?(:restricted_download_code) && user && user.can?(:restricted_download_code, project) + def build_can_download_code? + has_capability?(:build_download_code) && user && user.can?(:build_download_code, project) end def lfs_upload_access? diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index cda83bcc74a..ce686af2ade 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -65,9 +65,9 @@ class ProjectPolicy < BasePolicy 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 + def team_member_reporter_access! + can! :build_download_code + can! :build_read_container_image end def developer_access! @@ -115,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! @@ -136,11 +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 - restricted_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 cba0e2297a8..ba0b60abfe4 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -76,9 +76,9 @@ module Auth case requested_action when 'pull' - restricted_user_can_pull?(requested_project) || privileged_user_can_pull?(requested_project) + build_can_pull?(requested_project) || user_can_pull?(requested_project) when 'push' - restricted_user_can_push?(requested_project) || privileged_user_can_push?(requested_project) + build_can_push?(requested_project) || user_can_push?(requested_project) else false end @@ -90,29 +90,28 @@ module Auth private - def restricted_user_can_pull?(requested_project) - # Restricted can: + def build_can_pull?(requested_project) + # Build 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 || - has_ability?(:restricted_read_container_image, requested_project) + # 2. read images from dependent projects if creator of build is a team member + @capabilities.include?(:build_read_container_image) && + (requested_project == project || can?(current_user, :build_read_container_image, requested_project)) end - def privileged_user_can_pull?(requested_project) - has_ability?(:read_container_image, requested_project) + def user_can_pull?(requested_project) + @capabilities.include?(:read_container_image) && + 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 - requested_project == project + def build_can_push?(requested_project) + # Build can push only to project to from which he originates + @capabilities.include?(:build_create_container_image) && + requested_project == project end - def privileged_user_can_push?(requested_project) - has_ability?(:create_container_image, requested_project) - end - - def has_ability?(ability, requested_project) - @capabilities.include?(ability) && can?(current_user, ability, requested_project) + def user_can_push?(requested_project) + @capabilities.include?(:create_container_image) && + can?(current_user, :create_container_image, requested_project) end end end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index a792db027ff..6a55c50c3f3 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -78,7 +78,7 @@ module Gitlab service = project.public_send("#{underscored_service}_service") if service && service.activated? && service.valid_token?(password) - Result.new(nil, project, :ci, restricted_capabilities) + Result.new(nil, project, :ci, build_capabilities) end end end @@ -124,25 +124,27 @@ module Gitlab if build.user # If user is assigned to build, use restricted credentials of user - Result.new(build.user, build.project, :build, restricted_capabilities) + Result.new(build.user, build.project, :build, build_capabilities) else # Otherwise use generic CI credentials (backward compatibility) - Result.new(nil, build.project, :ci, restricted_capabilities) + Result.new(nil, build.project, :ci, build_capabilities) end end private - def restricted_capabilities + def build_capabilities [ :read_project, - :restricted_download_code, - :restricted_read_container_image + :build_download_code, + :build_read_container_image, + :build_create_container_image ] end def read_capabilities - restricted_capabilities + [ + [ + :read_project, :download_code, :read_container_image ] diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 10ef4a1e3cf..63b707db814 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -61,19 +61,19 @@ module Gitlab end def user_download_access_check - unless privileged_user_can_download_code? || restricted_user_can_download_code? + unless user_can_download_code? || build_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? + def user_can_download_code? capabilities.include?(:download_code) && user_access.can_do_action?(:download_code) end - def restricted_user_can_download_code? - capabilities.include?(:restricted_download_code) && user_access.can_do_action?(:restricted_download_code) + def build_can_download_code? + capabilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code) end def user_push_access_check(changes) |