diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index f489de42864ec58c346dcf4c1f75de87e8dd1b5a..e1bfe92f61bb92bead25fba213120377fcaf370a 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -142,6 +142,14 @@ def embeddable? uploader && uploader.exists? && uploader.embeddable? end + def bypass_auth_checks_on_uploads? + if ::Feature.enabled?(:enforce_auth_checks_on_uploads, default_enabled: :yaml) + false + else + action_name == 'show' && embeddable? + end + end + def find_model nil end diff --git a/app/controllers/groups/uploads_controller.rb b/app/controllers/groups/uploads_controller.rb index 49249f87d31d92ba743f3c48876ba5b84dd0de25..387f7be56cd5bb51e83876bc54c3f97de3d86171 100644 --- a/app/controllers/groups/uploads_controller.rb +++ b/app/controllers/groups/uploads_controller.rb @@ -4,7 +4,7 @@ class Groups::UploadsController < Groups::ApplicationController include UploadsActions include WorkhorseRequest - skip_before_action :group, if: -> { action_name == 'show' && embeddable? } + skip_before_action :group, if: -> { bypass_auth_checks_on_uploads? } before_action :authorize_upload_file!, only: [:create, :authorize] before_action :verify_workhorse_api!, only: [:authorize] diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index c15768e7bbb63416547ec28d82edd0ddacf435cb..ed5bd73d6d14cf671d01a325af796169beba87f4 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/app/controllers/projects/uploads_controller.rb @@ -6,7 +6,7 @@ class Projects::UploadsController < Projects::ApplicationController # These will kick you out if you don't have access. skip_before_action :project, :repository, - if: -> { action_name == 'show' && embeddable? } + if: -> { bypass_auth_checks_on_uploads? } before_action :authorize_upload_file!, only: [:create, :authorize] before_action :verify_workhorse_api!, only: [:authorize] diff --git a/config/feature_flags/development/enforce_auth_checks_on_uploads.yml b/config/feature_flags/development/enforce_auth_checks_on_uploads.yml new file mode 100644 index 0000000000000000000000000000000000000000..d0cbe123a8f804e89ebcf32e7e2e02d37370b565 --- /dev/null +++ b/config/feature_flags/development/enforce_auth_checks_on_uploads.yml @@ -0,0 +1,8 @@ +--- +name: enforce_auth_checks_on_uploads +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80117 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352291 +milestone: '14.8' +type: development +group: group::code review +default_enabled: false diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb index 62b35923bcd267361b1a6b012083af03ee8d8f3d..5ed8dc7ce981a0908e71e217fd2e2a7ed9706389 100644 --- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb +++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb @@ -205,10 +205,30 @@ allow_any_instance_of(FileUploader).to receive(:image?).and_return(true) end - it "responds with status 200" do - show_upload + context "enforce_auth_checks_on_uploads feature flag" do + context "with flag enabled" do + before do + stub_feature_flags(enforce_auth_checks_on_uploads: true) + end - expect(response).to have_gitlab_http_status(:ok) + it "responds with status 302" do + show_upload + + expect(response).to have_gitlab_http_status(:redirect) + end + end + + context "with flag disabled" do + before do + stub_feature_flags(enforce_auth_checks_on_uploads: false) + end + + it "responds with status 200" do + show_upload + + expect(response).to have_gitlab_http_status(:ok) + end + end end end @@ -276,10 +296,30 @@ allow_any_instance_of(FileUploader).to receive(:image?).and_return(true) end - it "responds with status 200" do - show_upload + context "enforce_auth_checks_on_uploads feature flag" do + context "with flag enabled" do + before do + stub_feature_flags(enforce_auth_checks_on_uploads: true) + end + + it "responds with status 404" do + show_upload + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "with flag disabled" do + before do + stub_feature_flags(enforce_auth_checks_on_uploads: false) + end + + it "responds with status 200" do + show_upload - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end + end end end