Add latest changes from gitlab-org/security/gitlab@15-7-stable-ee

7 files changed, 80 insertions, 42 deletions

diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 09419a4589d..66f715f32af 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -52,6 +52,8 @@ class UploadsController < ApplicationController
       # access to itself when a secret is given.
       # For instance, user avatars are readable by anyone,
       # while temporary, user snippet uploads are not.
+      return false if !current_user && public_visibility_restricted?
+
       !secret? || can?(current_user, :update_user, model)
     when Appearance
       true
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 858c145de3f..8eea995529c 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -273,6 +273,9 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
   rule { can?(:admin_group) & resource_access_token_feature_available }.policy do
     enable :read_resource_access_tokens
     enable :destroy_resource_access_tokens
+  end
+
+  rule { can?(:admin_group) & resource_access_token_creation_allowed }.policy do
     enable :admin_setting_to_allow_project_access_token_creation
   end
 
@@ -338,12 +341,16 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
     true
   end
 
+  def resource_access_token_create_feature_available?
+    true
+  end
+
   def can_read_group_member?
     !(@subject.private? && access_level == GroupMember::NO_ACCESS)
   end
 
   def resource_access_token_creation_allowed?
-    resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
+    resource_access_token_create_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
   end
 
   def valid_dependency_proxy_deploy_token
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 2a13fafa313..fd3dbb54d57 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -157,7 +157,9 @@ class ProjectPolicy < BasePolicy
   condition(:service_desk_enabled) { @subject.service_desk_enabled? }
 
   with_scope :subject
-  condition(:resource_access_token_feature_available) { resource_access_token_feature_available? }
+  condition(:resource_access_token_feature_available) do
+    resource_access_token_feature_available?
+  end
   condition(:resource_access_token_creation_allowed) { resource_access_token_creation_allowed? }
 
   # We aren't checking `:read_issue` or `:read_merge_request` in this case
@@ -922,12 +924,16 @@ class ProjectPolicy < BasePolicy
     true
   end
 
+  def resource_access_token_create_feature_available?
+    true
+  end
+
   def resource_access_token_creation_allowed?
     group = project.group
 
     return true unless group # always enable for projects in personal namespaces
 
-    resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
+    resource_access_token_create_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
   end
 
   def project
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 2f23d47029c..d52306ef805 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -2,6 +2,8 @@
 
 module ErrorTracking
   class ListProjectsService < ErrorTracking::BaseService
+    MASKED_TOKEN_REGEX = /\A\*+\z/.freeze
+
     private
 
     def perform
@@ -20,23 +22,31 @@ module ErrorTracking
 
     def project_error_tracking_setting
       (super || project.build_error_tracking_setting).tap do |setting|
+        url_changed = !setting.api_url&.start_with?(params[:api_host])
+
         setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from(
           api_host: params[:api_host],
           organization_slug: 'org',
           project_slug: 'proj'
         )
 
-        setting.token = token(setting)
+        setting.token = token(setting, url_changed)
         setting.enabled = true
       end
     end
     strong_memoize_attr :project_error_tracking_setting
 
-    def token(setting)
+    def token(setting, url_changed)
+      return if url_changed && masked_token?
+
       # Use param token if not masked, otherwise use database token
-      return params[:token] unless /\A\*+\z/.match?(params[:token])
+      return params[:token] unless masked_token?
 
       setting.token
     end
+
+    def masked_token?
+      MASKED_TOKEN_REGEX.match?(params[:token])
+    end
   end
 end
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index e128db8d1c1..3e9c56d3274 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -268,17 +268,35 @@ RSpec.describe UploadsController do
       end
 
       context "when not signed in" do
-        it "responds with status 200" do
-          get :show, params: { model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png" }
+        context "when restricted visibility level is not set to public" do
+          before do
+            stub_application_setting(restricted_visibility_levels: [])
+          end
 
-          expect(response).to have_gitlab_http_status(:ok)
+          it "responds with status 200" do
+            get :show, params: { model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png" }
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+
+          it_behaves_like 'content publicly cached' do
+            subject do
+              get :show, params: { model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png' }
+
+              response
+            end
+          end
         end
 
-        it_behaves_like 'content publicly cached' do
-          subject do
-            get :show, params: { model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png' }
+        context "when restricted visibility level is set to public" do
+          before do
+            stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+          end
 
-            response
+          it "responds with status 401" do
+            get :show, params: { model: "user", mounted_as: "avatar", id: user.id, filename: "dk.png" }
+
+            expect(response).to have_gitlab_http_status(:unauthorized)
           end
         end
       end
diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb
index ce391bd1ca0..8408adcc21d 100644
--- a/spec/services/error_tracking/list_projects_service_spec.rb
+++ b/spec/services/error_tracking/list_projects_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe ErrorTracking::ListProjectsService do
+RSpec.describe ErrorTracking::ListProjectsService, feature_category: :integrations do
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project) }
 
@@ -51,15 +51,33 @@ RSpec.describe ErrorTracking::ListProjectsService do
       end
 
       context 'masked param token' do
-        let(:params) { ActionController::Parameters.new(token: "*********", api_host: new_api_host) }
+        let(:params) { ActionController::Parameters.new(token: "*********", api_host: api_host) }
 
-        before do
-          expect(error_tracking_setting).to receive(:list_sentry_projects)
+        context 'with the current api host' do
+          let(:api_host) { 'https://sentrytest.gitlab.com' }
+
+          before do
+            expect(error_tracking_setting).to receive(:list_sentry_projects)
             .and_return({ projects: [] })
+          end
+
+          it 'uses database token' do
+            expect { subject.execute }.not_to change { error_tracking_setting.token }
+          end
         end
 
-        it 'uses database token' do
-          expect { subject.execute }.not_to change { error_tracking_setting.token }
+        context 'with a new api host' do
+          let(:api_host) { new_api_host }
+
+          it 'returns an error' do
+            expect(result[:message]).to start_with('Token is a required field')
+            expect(error_tracking_setting).not_to be_valid
+            expect(error_tracking_setting).not_to receive(:list_sentry_projects)
+          end
+
+          it 'resets the token' do
+            expect { subject.execute }.to change { error_tracking_setting.token }.from(token).to(nil)
+          end
         end
       end
 
diff --git a/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb b/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb
index 337ad024fc0..cc91b73449a 100644
--- a/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb
+++ b/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb
@@ -71,26 +71,3 @@ RSpec.shared_examples 'Self-managed Core resource access tokens' do
     end
   end
 end
-
-RSpec.shared_examples 'GitLab.com Core resource access tokens' do
-  before do
-    allow(::Gitlab).to receive(:com?).and_return(true)
-    stub_ee_application_setting(should_check_namespace_plan: true)
-  end
-
-  context 'with owner access' do
-    let(:current_user) { owner }
-
-    context 'create resource access tokens' do
-      it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
-    end
-
-    context 'read resource access tokens' do
-      it { is_expected.not_to be_allowed(:read_resource_access_tokens) }
-    end
-
-    context 'destroy resource access tokens' do
-      it { is_expected.not_to be_allowed(:destroy_resource_access_tokens) }
-    end
-  end
-end