Use `build_read_container_image` and use `build_download_code`

4 files changed, 37 insertions, 42 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