Merge branch 'fp-exempt-public-projects-from-job-token-scope' into 'master'

Exempt public/internal projects from job token scope

See merge request gitlab-org/gitlab!66551

app/policies/project_policy.rb

@@ -533,7 +533,7 @@ class ProjectPolicy < BasePolicy
     enable :read_project_for_iids
   end
 
-  rule { ~project_allowed_for_job_token }.prevent_all
+  rule { ~public_project & ~internal_access & ~project_allowed_for_job_token }.prevent_all
 
   rule { can?(:public_access) }.policy do
     enable :read_package

doc/api/index.md

@@ -259,7 +259,7 @@ Refer to this feature's version history for more details.
 
 You can limit the access scope of a project's CI/CD job token to increase the
 job token's security. A job token might give extra permissions that aren't necessary
-to access specific resources. Limiting the job token access scope reduces the risk of a leaked
+to access specific private resources. Limiting the job token access scope reduces the risk of a leaked
 token being used to access private data that the user associated to the job can access.
 
 Control the job token access scope with an allowlist of other projects authorized
@@ -273,7 +273,9 @@ setting at all times, and configure the allowlist for cross-project access if ne
 
 For example, when the setting is enabled, jobs in a pipeline in project `A` have
 a `CI_JOB_TOKEN` scope limited to project `A`. If the job needs to use the token
-to make an API request to project `B`, then `B` must be added to the allowlist for `A`.
+to make an API request to a private project `B`, then `B` must be added to the allowlist for `A`.
+If project `B` is public or internal, it doesn't need to be added to the allowlist.
+The job token scope is only for controlling access to private projects.
 
 To enable and configure the job token scope limit:
 

spec/policies/project_policy_spec.rb

@@ -1419,66 +1419,65 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'when user is authenticated via CI_JOB_TOKEN', :request_store do
-    let(:current_user) { developer }
-    let(:job) { build_stubbed(:ci_build, project: scope_project, user: current_user) }
+    using RSpec::Parameterized::TableSyntax
 
-    before do
-      current_user.set_ci_job_token_scope!(job)
-      scope_project.update!(ci_job_token_scope_enabled: true)
+    where(:project_visibility, :user_role, :external_user, :scope_project_type, :token_scope_enabled, :result) do
+      :private  | :reporter | false | :same      | true  | true
+      :private  | :reporter | false | :same      | false | true
+      :private  | :reporter | false | :different | true  | false
+      :private  | :reporter | false | :different | false | true
+      :private  | :guest    | false | :same      | true  | true
+      :private  | :guest    | false | :same      | false | true
+      :private  | :guest    | false | :different | true  | false
+      :private  | :guest    | false | :different | false | true
+
+      :internal | :reporter | false | :same      | true  | true
+      :internal | :reporter | true  | :same      | true  | true
+      :internal | :reporter | false | :same      | false | true
+      :internal | :reporter | false | :different | true  | true
+      :internal | :reporter | true  | :different | true  | false
+      :internal | :reporter | false | :different | false | true
+      :internal | :guest    | false | :same      | true  | true
+      :internal | :guest    | true  | :same      | true  | true
+      :internal | :guest    | false | :same      | false | true
+      :internal | :guest    | false | :different | true  | true
+      :internal | :guest    | true  | :different | true  | false
+      :internal | :guest    | false | :different | false | true
+
+      :public   | :reporter | false | :same      | true  | true
+      :public   | :reporter | false | :same      | false | true
+      :public   | :reporter | false | :different | true  | true
+      :public   | :reporter | false | :different | false | true
+      :public   | :guest    | false | :same      | true  | true
+      :public   | :guest    | false | :same      | false | true
+      :public   | :guest    | false | :different | true  | true
+      :public   | :guest    | false | :different | false | true
     end
 
-    context 'when accessing a private project' do
-      let(:project) { private_project }
-
-      context 'when the job token comes from the same project' do
-        let(:scope_project) { project }
-
-        it { is_expected.to be_allowed(:developer_access) }
-      end
-
-      context 'when the job token comes from another project' do
-        let(:scope_project) { create(:project, :private) }
-
-        before do
-          scope_project.add_developer(current_user)
-        end
-
-        it { is_expected.to be_disallowed(:guest_access) }
-
-        context 'when job token scope is disabled' do
-          before do
-            scope_project.update!(ci_job_token_scope_enabled: false)
-          end
+    with_them do
+      let(:current_user) { public_send(user_role) }
+      let(:project) { public_send("#{project_visibility}_project") }
+      let(:job) { build_stubbed(:ci_build, project: scope_project, user: current_user) }
 
-          it { is_expected.to be_allowed(:guest_access) }
+      let(:scope_project) do
+        if scope_project_type == :same
+          project
+        else
+          create(:project, :private)
         end
       end
-    end
-
-    context 'when accessing a public project' do
-      let(:project) { public_project }
 
-      context 'when the job token comes from the same project' do
-        let(:scope_project) { project }
-
-        it { is_expected.to be_allowed(:developer_access) }
+      before do
+        current_user.set_ci_job_token_scope!(job)
+        current_user.external = external_user
+        scope_project.update!(ci_job_token_scope_enabled: token_scope_enabled)
       end
 
-      context 'when the job token comes from another project' do
-        let(:scope_project) { create(:project, :private) }
-
-        before do
-          scope_project.add_developer(current_user)
-        end
-
-        it { is_expected.to be_disallowed(:public_access) }
-
-        context 'when job token scope is disabled' do
-          before do
-            scope_project.update!(ci_job_token_scope_enabled: false)
-          end
-
-          it { is_expected.to be_allowed(:public_access) }
+      it "enforces the expected permissions" do
+        if result
+          is_expected.to be_allowed("#{user_role}_access".to_sym)
+        else
+          is_expected.to be_disallowed("#{user_role}_access".to_sym)
         end
       end
     end

spec/requests/api/generic_packages_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe API::GenericPackages do
   let_it_be(:project_deploy_token_wo) { create(:project_deploy_token, deploy_token: deploy_token_wo, project: project) }
 
   let(:user) { personal_access_token.user }
-  let(:ci_build) { create(:ci_build, :running, user: user, project: project) }
+  let(:ci_build) { create(:ci_build, :running, user: user) }
   let(:snowplow_standard_context_params) { { user: user, project: project, namespace: project.namespace } }
 
   def auth_header

spec/requests/api/go_proxy_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe API::GoProxy do
   let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" }
 
   let_it_be(:oauth) { create :oauth_access_token, scopes: 'api', resource_owner: user }
-  let_it_be(:job) { create :ci_build, user: user, status: :running, project: project }
+  let_it_be(:job) { create :ci_build, user: user, status: :running }
   let_it_be(:pa_token) { create :personal_access_token, user: user }
 
   let_it_be(:modules) do

spec/requests/api/maven_packages_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe API::MavenPackages do
   let_it_be(:package_file) { package.package_files.with_file_name_like('%.xml').first }
   let_it_be(:jar_file) { package.package_files.with_file_name_like('%.jar').first }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
-  let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running, project: project) }
+  let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) }
   let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
   let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
   let_it_be(:deploy_token_for_group) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) }

spec/requests/api/npm_project_packages_spec.rb

@@ -78,7 +78,7 @@ RSpec.describe API::NpmProjectPackages do
 
       context 'with a job token for a different user' do
         let_it_be(:other_user) { create(:user) }
-        let_it_be_with_reload(:other_job) { create(:ci_build, :running, user: other_user, project: project) }
+        let_it_be_with_reload(:other_job) { create(:ci_build, :running, user: other_user) }
 
         let(:headers) { build_token_auth_header(other_job.token) }
 

spec/requests/api/pypi_packages_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe API::PypiPackages do
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
   let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
   let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
-  let_it_be(:job) { create(:ci_build, :running, user: user, project: project) }
+  let_it_be(:job) { create(:ci_build, :running, user: user) }
 
   let(:headers) { {} }
 

spec/requests/api/releases_spec.rb

@@ -811,7 +811,7 @@ RSpec.describe API::Releases do
     end
 
     context 'when using JOB-TOKEN auth' do
-      let(:job) { create(:ci_build, user: maintainer, project: project) }
+      let(:job) { create(:ci_build, user: maintainer) }
       let(:params) do
         {
           name: 'Another release',

spec/requests/api/rubygem_packages_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe API::RubygemPackages do
   let_it_be_with_reload(:project) { create(:project) }
   let_it_be(:personal_access_token) { create(:personal_access_token) }
   let_it_be(:user) { personal_access_token.user }
-  let_it_be(:job) { create(:ci_build, :running, user: user, project: project) }
+  let_it_be(:job) { create(:ci_build, :running, user: user) }
   let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
   let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
   let_it_be(:headers) { {} }

spec/requests/api/terraform/modules/v1/packages_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages do
   let_it_be(:package) { create(:terraform_module_package, project: project) }
   let_it_be(:personal_access_token) { create(:personal_access_token) }
   let_it_be(:user) { personal_access_token.user }
-  let_it_be(:job) { create(:ci_build, :running, user: user, project: project) }
+  let_it_be(:job) { create(:ci_build, :running, user: user) }
   let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
   let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
 

spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb

@@ -11,7 +11,7 @@ RSpec.shared_context 'conan api setup' do
   let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
 
   let(:project) { package.project }
-  let(:job) { create(:ci_build, :running, user: user, project: project) }
+  let(:job) { create(:ci_build, :running, user: user) }
   let(:job_token) { job.token }
   let(:auth_token) { personal_access_token.token }
   let(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }

spec/support/shared_contexts/requests/api/npm_packages_shared_context.rb

@@ -11,7 +11,7 @@ RSpec.shared_context 'npm api setup' do
   let_it_be(:package, reload: true) { create(:npm_package, project: project, name: "@#{group.path}/scoped_package") }
   let_it_be(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
-  let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running, project: project) }
+  let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) }
   let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) }
   let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }