Merge branch 'debian_unauthorized_triggers_auth' into 'master'

Fix Debian packages API to raise 401 Unauthorized instead of a 404 Not Found

See merge request gitlab-org/gitlab!51734

ee/lib/ee/api/helpers.rb

@@ -92,12 +92,11 @@ module EE
         # admin users can only access project if they are direct member
         ability = job_token_authentication? ? :build_read_project : :read_project
 
-        if can?(current_user, ability, project)
-          project
-        else
+        return project if can?(current_user, ability, project)
+        return unauthorized! if authenticate_non_public?
+
         not_found!('Project')
       end
-      end
 
       override :find_group!
       def find_group!(id)

lib/api/debian_group_packages.rb

@@ -8,6 +8,8 @@ module API
 
     resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       before do
+        require_packages_enabled!
+
         not_found! unless ::Feature.enabled?(:debian_packages, user_group)
 
         authorize_read_package!(user_group)

lib/api/debian_package_endpoints.rb

@@ -56,7 +56,7 @@ module API
           detail 'This feature was introduced in GitLab 13.5'
         end
 
-        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
         get 'Release.gpg' do
           not_found!
         end
@@ -66,7 +66,7 @@ module API
           detail 'This feature was introduced in GitLab 13.5'
         end
 
-        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
         get 'Release' do
           # https://gitlab.com/gitlab-org/gitlab/-/issues/5835#note_414103286
           'TODO Release'
@@ -77,7 +77,7 @@ module API
           detail 'This feature was introduced in GitLab 13.5'
         end
 
-        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
         get 'InRelease' do
           not_found!
         end
@@ -93,7 +93,7 @@ module API
             detail 'This feature was introduced in GitLab 13.5'
           end
 
-          route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+          route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
           get 'Packages' do
             # https://gitlab.com/gitlab-org/gitlab/-/issues/5835#note_414103286
             'TODO Packages'
@@ -116,7 +116,7 @@ module API
           detail 'This feature was introduced in GitLab 13.5'
         end
 
-        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+        route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
         get ':file_name', requirements: FILE_NAME_REQUIREMENTS do
           # https://gitlab.com/gitlab-org/gitlab/-/issues/5835#note_414103286
           'TODO File'

lib/api/debian_project_packages.rb

@@ -8,6 +8,8 @@ module API
 
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       before do
+        require_packages_enabled!
+
         not_found! unless ::Feature.enabled?(:debian_packages, user_project)
 
         authorize_read_package!
@@ -28,7 +30,7 @@ module API
             requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
           end
 
-          route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+          route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
           put do
             authorize_upload!(authorized_user_project)
             bad_request!('File is too large') if authorized_user_project.actual_limits.exceeded?(:debian_max_file_size, params[:file].size)
@@ -43,7 +45,7 @@ module API
           end
 
           # PUT {projects|groups}/:id/packages/debian/:file_name/authorize
-          route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
+          route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth, authenticate_non_public: true
           put 'authorize' do
             authorize_workhorse!(
               subject: authorized_user_project,

lib/api/go_proxy.rb

@@ -30,20 +30,6 @@ module API
         str.gsub(/![[:alpha:]]/) { |s| s[1..].upcase }
       end
 
-      def find_project!(id)
-        # based on API::Helpers::Packages::BasicAuthHelpers#authorized_project_find!
-
-        project = find_project(id)
-
-        return project if project && can?(current_user, :read_project, project)
-
-        if current_user
-          not_found!('Project')
-        else
-          unauthorized!
-        end
-      end
-
       def find_module
         not_found! unless Feature.enabled?(:go_proxy, user_project)
 
@@ -74,7 +60,7 @@ module API
       requires :id, type: String, desc: 'The ID of a project'
       requires :module_name, type: String, desc: 'Module name', coerce_with: ->(val) { CGI.unescape(val) }
     end
-    route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
+    route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true, authenticate_non_public: true
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       before do
         authorize_read_package!

lib/api/helpers.rb

@@ -119,12 +119,11 @@ module API
     def find_project!(id)
       project = find_project(id)
 
-      if can?(current_user, :read_project, project)
-        project
-      else
+      return project if can?(current_user, :read_project, project)
+      return unauthorized! if authenticate_non_public?
+
       not_found!('Project')
     end
-    end
 
     # rubocop: disable CodeReuse/ActiveRecord
     def find_group(id)
@@ -139,12 +138,11 @@ module API
     def find_group!(id)
       group = find_group(id)
 
-      if can?(current_user, :read_group, group)
-        group
-      else
+      return group if can?(current_user, :read_group, group)
+      return unauthorized! if authenticate_non_public?
+
       not_found!('Group')
     end
-    end
 
     def check_namespace_access(namespace)
       return namespace if can?(current_user, :read_namespace, namespace)
@@ -657,6 +655,10 @@ module API
       Gitlab::Shell.secret_token
     end
 
+    def authenticate_non_public?
+      route_authentication_setting[:authenticate_non_public] && !current_user
+    end
+
     def send_git_blob(repository, blob)
       env['api.format'] = :txt
       content_type 'text/plain'

spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb

@@ -201,7 +201,7 @@ RSpec.shared_examples 'rejects Debian access with unknown project id' do
     let(:project) { double(id: non_existing_record_id) }
 
     context 'as anonymous' do
-      it_behaves_like 'Debian project repository GET request', :anonymous, true, :not_found, nil
+      it_behaves_like 'Debian project repository GET request', :anonymous, true, :unauthorized, nil
     end
 
     context 'as authenticated user' do
@@ -228,13 +228,13 @@ RSpec.shared_examples 'Debian project repository GET endpoint' do |success_statu
       'PUBLIC'  | :anonymous  | false | true  | success_status | success_body
       'PRIVATE' | :developer  | true  | true  | success_status | success_body
       'PRIVATE' | :guest      | true  | true  | :forbidden     | nil
-      'PRIVATE' | :developer  | true  | false | :not_found     | nil
-      'PRIVATE' | :guest      | true  | false | :not_found     | nil
+      'PRIVATE' | :developer  | true  | false | :unauthorized  | nil
+      'PRIVATE' | :guest      | true  | false | :unauthorized  | nil
       'PRIVATE' | :developer  | false | true  | :not_found     | nil
       'PRIVATE' | :guest      | false | true  | :not_found     | nil
-      'PRIVATE' | :developer  | false | false | :not_found     | nil
-      'PRIVATE' | :guest      | false | false | :not_found     | nil
-      'PRIVATE' | :anonymous  | false | true  | :not_found     | nil
+      'PRIVATE' | :developer  | false | false | :unauthorized  | nil
+      'PRIVATE' | :guest      | false | false | :unauthorized  | nil
+      'PRIVATE' | :anonymous  | false | true  | :unauthorized  | nil
     end
 
     with_them do
@@ -263,13 +263,13 @@ RSpec.shared_examples 'Debian project repository PUT endpoint' do |success_statu
       'PUBLIC'  | :anonymous  | false | true  | :unauthorized  | nil
       'PRIVATE' | :developer  | true  | true  | success_status | nil
       'PRIVATE' | :guest      | true  | true  | :forbidden     | nil
-      'PRIVATE' | :developer  | true  | false | :not_found     | nil
-      'PRIVATE' | :guest      | true  | false | :not_found     | nil
+      'PRIVATE' | :developer  | true  | false | :unauthorized  | nil
+      'PRIVATE' | :guest      | true  | false | :unauthorized  | nil
       'PRIVATE' | :developer  | false | true  | :not_found     | nil
       'PRIVATE' | :guest      | false | true  | :not_found     | nil
-      'PRIVATE' | :developer  | false | false | :not_found     | nil
-      'PRIVATE' | :guest      | false | false | :not_found     | nil
-      'PRIVATE' | :anonymous  | false | true  | :not_found     | nil
+      'PRIVATE' | :developer  | false | false | :unauthorized  | nil
+      'PRIVATE' | :guest      | false | false | :unauthorized  | nil
+      'PRIVATE' | :anonymous  | false | true  | :unauthorized  | nil
     end
 
     with_them do
@@ -321,7 +321,7 @@ RSpec.shared_examples 'rejects Debian access with unknown group id' do
     let(:group) { double(id: non_existing_record_id) }
 
     context 'as anonymous' do
-      it_behaves_like 'Debian group repository GET request', :anonymous, true, :not_found, nil
+      it_behaves_like 'Debian group repository GET request', :anonymous, true, :unauthorized, nil
     end
 
     context 'as authenticated user' do
@@ -348,13 +348,13 @@ RSpec.shared_examples 'Debian group repository GET endpoint' do |success_status,
       'PUBLIC'  | :anonymous  | false | true  | success_status | success_body
       'PRIVATE' | :developer  | true  | true  | success_status | success_body
       'PRIVATE' | :guest      | true  | true  | :forbidden     | nil
-      'PRIVATE' | :developer  | true  | false | :not_found     | nil
-      'PRIVATE' | :guest      | true  | false | :not_found     | nil
+      'PRIVATE' | :developer  | true  | false | :unauthorized  | nil
+      'PRIVATE' | :guest      | true  | false | :unauthorized  | nil
       'PRIVATE' | :developer  | false | true  | :not_found     | nil
       'PRIVATE' | :guest      | false | true  | :not_found     | nil
-      'PRIVATE' | :developer  | false | false | :not_found     | nil
-      'PRIVATE' | :guest      | false | false | :not_found     | nil
-      'PRIVATE' | :anonymous  | false | true  | :not_found     | nil
+      'PRIVATE' | :developer  | false | false | :unauthorized  | nil
+      'PRIVATE' | :guest      | false | false | :unauthorized  | nil
+      'PRIVATE' | :anonymous  | false | true  | :unauthorized  | nil
     end
 
     with_them do