Skip to content

Commit 0a367a4

Browse files
author
GitLab Bot
committed
Add latest changes from gitlab-org/gitlab@master
1 parent a50b8ce commit 0a367a4

11 files changed

+45
-28
lines changed

app/controllers/application_controller.rb

-4
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,6 @@ def self.endpoint_id_for_action(action_name)
9090
render_403
9191
end
9292

93-
rescue_from ActionController::InvalidAuthenticityToken do
94-
render_403
95-
end
96-
9793
rescue_from Browser::Error do |e|
9894
render plain: e.message, status: :forbidden
9995
end

app/controllers/graphql_controller.rb

+4-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ class GraphqlController < ApplicationController
1313
skip_before_action :verify_authenticity_token, if: -> {
1414
Feature.enabled?(:fix_graphql_csrf, Feature.current_request) && (current_user.nil? || sessionless_user?)
1515
}
16+
skip_before_action :check_two_factor_requirement, if: -> {
17+
Feature.enabled?(:fix_graphql_csrf, Feature.current_request) && sessionless_user?
18+
}
1619

1720
# Header can be passed by tests to disable SQL query limits.
1821
DISABLE_SQL_QUERY_LIMIT_HEADER = 'HTTP_X_GITLAB_DISABLE_SQL_QUERY_LIMIT'
@@ -106,7 +109,7 @@ def execute
106109
# `rescue_from StandardError` above would prevent these from bubbling up to ApplicationController.
107110
# These also return errors in a JSON format similar to GraphQL errors.
108111
rescue_from ActionController::InvalidAuthenticityToken do |exception|
109-
render_error(exception.message, status: :forbidden)
112+
render_error(exception.message, status: :unprocessable_entity)
110113
end
111114

112115
rescue_from Gitlab::Auth::TooManyIps do |exception|

app/workers/authorized_project_update/periodic_recalculate_worker.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ module AuthorizedProjectUpdate
44
class PeriodicRecalculateWorker
55
include ApplicationWorker
66

7-
data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency -- will change to sticky with feature-flag
7+
data_consistency :sticky, feature_flag: :change_data_consistency_for_permissions_workers
88

99
# This worker does not perform work scoped to a context
1010
include CronjobQueue # rubocop:disable Scalability/CronWorkerContext

app/workers/authorized_project_update/project_recalculate_per_user_worker.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
module AuthorizedProjectUpdate
44
class ProjectRecalculatePerUserWorker < ProjectRecalculateWorker
5-
data_consistency :always
5+
data_consistency :sticky, feature_flag: :change_data_consistency_for_permissions_workers
66

77
feature_category :permissions
88
urgency :high

app/workers/authorized_project_update/project_recalculate_worker.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ module AuthorizedProjectUpdate
44
class ProjectRecalculateWorker
55
include ApplicationWorker
66

7-
data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency -- will change to sticky with feature-flag
7+
data_consistency :sticky, feature_flag: :change_data_consistency_for_permissions_workers
88
include Gitlab::ExclusiveLeaseHelpers
99

1010
feature_category :permissions

app/workers/authorized_project_update/user_refresh_from_replica_worker.rb

+1-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class UserRefreshFromReplicaWorker
77
sidekiq_options retry: 3
88
feature_category :permissions
99
urgency :low
10-
data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency -- will change to sticky with feature-flag
10+
data_consistency :delayed, feature_flag: :change_data_consistency_for_permissions_workers
1111
queue_namespace :authorized_project_update
1212

1313
idempotent!
@@ -25,10 +25,6 @@ def perform(user_id)
2525

2626
private
2727

28-
# We use this approach instead of specifying `data_consistency :delayed` because these jobs
29-
# are enqueued in large numbers, and using `data_consistency :delayed`
30-
# does not allow us to deduplicate these jobs.
31-
# https://gitlab.com/gitlab-org/gitlab/-/issues/325291
3228
def use_replica_if_available(&block)
3329
::Gitlab::Database::LoadBalancing::SessionMap
3430
.with_sessions([::ApplicationRecord, ::Ci::ApplicationRecord])

app/workers/authorized_projects_worker.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
class AuthorizedProjectsWorker
44
include ApplicationWorker
55

6-
data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency -- will change to sticky with feature-flag
6+
data_consistency :sticky, feature_flag: :change_data_consistency_for_permissions_workers
77

88
sidekiq_options retry: 3
99

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
name: change_data_consistency_for_permissions_workers
3+
feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/462611
4+
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173210
5+
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/505413
6+
milestone: '17.7'
7+
group: group::authorization
8+
type: worker
9+
default_enabled: false

db/post_migrate/20240124043507_migrate_sidekiq_queued_and_future_jobs.rb

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ class MigrateSidekiqQueuedAndFutureJobs < Gitlab::Database::Migration[2.2]
44
milestone '16.10'
55
disable_ddl_transaction!
66

7+
restrict_gitlab_migration gitlab_schema: :gitlab_main
8+
79
class SidekiqMigrateJobs
810
LOG_FREQUENCY_QUEUES = 10
911
LOG_FREQUENCY = 1000

spec/controllers/application_controller_spec.rb

-13
Original file line numberDiff line numberDiff line change
@@ -1139,19 +1139,6 @@ def index
11391139
.to raise_error(ActionController::InvalidAuthenticityToken)
11401140
end
11411141
end
1142-
1143-
controller(described_class) do
1144-
self.allow_forgery_protection = true
1145-
skip_before_action :authenticate_user!
1146-
1147-
def create; end
1148-
end
1149-
1150-
it 'returns a 403' do
1151-
post :create
1152-
1153-
expect(response).to have_gitlab_http_status(:forbidden)
1154-
end
11551142
end
11561143

11571144
describe '#after_sign_in_path_for' do

spec/requests/api/graphql_spec.rb

+25-1
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@
242242

243243
post_graphql(query, headers: { 'X-CSRF-Token' => 'invalid' })
244244

245-
expect(response).to have_gitlab_http_status(:forbidden)
245+
expect(response).to have_gitlab_http_status(:unprocessable_entity)
246246
end
247247

248248
it 'authenticates a user with a valid session token' do
@@ -315,6 +315,30 @@
315315
expect(graphql_data['echo']).to eq("\"#{token.user.username}\" says: Hello world")
316316
end
317317

318+
context 'when two-factor authentication is required' do
319+
before do
320+
stub_application_setting(require_two_factor_authentication: true)
321+
end
322+
323+
it 'does not enforce 2FA' do
324+
post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token })
325+
326+
expect(graphql_data['echo']).to eq("\"#{token.user.username}\" says: Hello world")
327+
end
328+
329+
context 'when fix_graphql_csrf is disabled' do
330+
before do
331+
stub_feature_flags(fix_graphql_csrf: false)
332+
end
333+
334+
it 'does not enforce 2FA' do
335+
post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token })
336+
337+
expect(graphql_data['echo']).to eq("\"#{token.user.username}\" says: Hello world")
338+
end
339+
end
340+
end
341+
318342
context 'when user also has a valid session' do
319343
let_it_be(:other_user) { create(:user) }
320344

0 commit comments

Comments
 (0)