Merge branch 'fix-unathorized-cloning' into 'security'

Ensure external users are not able to clone disabled repositories.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23788

See merge request !2017

5 files changed, 15 insertions, 12 deletions

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 383e184d796..3f41916e6d3 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -21,10 +21,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   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)
 
@@ -41,6 +37,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController
         send_final_spnego_response
         return # Allow access
       end
+    elsif project && download_request? && Guest.can?(:download_code, project)
+      @authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code])
+
+      return # Allow access
     end
 
     send_challenges
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 662d38b10a5..13caeb42d40 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -78,11 +78,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   def upload_pack_allowed?
     return false unless Gitlab.config.gitlab_shell.upload_pack
 
-    if user
-      access_check.allowed?
-    else
-      ci? || project.public?
-    end
+    access_check.allowed? || ci?
   end
 
   def access
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 95b60aeab5f..d3966ba1f10 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -1,6 +1,6 @@
 module LfsHelper
   include Gitlab::Routing.url_helpers
-  
+
   def require_lfs_enabled!
     return if Gitlab.config.lfs.enabled
 
@@ -27,7 +27,7 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    project.public? || ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
+    ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
   end
 
   def user_can_download_code?
diff --git a/app/models/guest.rb b/app/models/guest.rb
new file mode 100644
index 00000000000..01285ca1264
--- /dev/null
+++ b/app/models/guest.rb
@@ -0,0 +1,7 @@
+class Guest
+  class << self
+    def can?(action, subject)
+      Ability.allowed?(nil, action, subject)
+    end
+  end
+end
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 8ea88da8a53..3fc1c70be75 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -76,7 +76,7 @@ module Auth
 
       case requested_action
       when 'pull'
-        requested_project.public? || build_can_pull?(requested_project) || user_can_pull?(requested_project)
+        build_can_pull?(requested_project) || user_can_pull?(requested_project)
       when 'push'
         build_can_push?(requested_project) || user_can_push?(requested_project)
       else