summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changelogs/unreleased/secutity-404-difference.yml5
-rw-r--r--config/routes.rb1
-rw-r--r--config/routes/unmatched_project.rb18
-rw-r--r--spec/requests/git_http_spec.rb8
-rw-r--r--spec/routing/git_http_routing_spec.rb21
-rw-r--r--spec/routing/project_routing_spec.rb69
-rw-r--r--spec/support/matchers/route_to_route_not_found_matcher.rb15
-rw-r--r--spec/support/shared_examples/routing/git_http_routing_shared_examples.rb54
8 files changed, 185 insertions, 6 deletions
diff --git a/changelogs/unreleased/secutity-404-difference.yml b/changelogs/unreleased/secutity-404-difference.yml
new file mode 100644
index 00000000000..0c09f2da9df
--- /dev/null
+++ b/changelogs/unreleased/secutity-404-difference.yml
@@ -0,0 +1,5 @@
+---
+title: Add routes for unmatched url for not-get requests
+merge_request:
+author:
+type: security
diff --git a/config/routes.rb b/config/routes.rb
index 91d1a817175..31e483df326 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -275,6 +275,7 @@ Rails.application.routes.draw do
draw :dashboard
draw :user
draw :project
+ draw :unmatched_project
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/210024
scope as: 'deprecated' do
diff --git a/config/routes/unmatched_project.rb b/config/routes/unmatched_project.rb
new file mode 100644
index 00000000000..b4fe243c7b0
--- /dev/null
+++ b/config/routes/unmatched_project.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+scope(path: '*namespace_id',
+ as: :namespace,
+ namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do
+ scope(path: ':project_id',
+ constraints: { project_id: Gitlab::PathRegex.project_route_regex },
+ as: :project) do
+ post '*all', to: 'application#route_not_found'
+ put '*all', to: 'application#route_not_found'
+ patch '*all', to: 'application#route_not_found'
+ delete '*all', to: 'application#route_not_found'
+ post '/', to: 'application#route_not_found'
+ put '/', to: 'application#route_not_found'
+ patch '/', to: 'application#route_not_found'
+ delete '/', to: 'application#route_not_found'
+ end
+end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index bc89dc2fa77..1ee3e36be8b 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -159,13 +159,17 @@ RSpec.describe 'Git HTTP requests' do
context "POST git-upload-pack" do
it "fails to find a route" do
- expect { clone_post(repository_path) }.to raise_error(ActionController::RoutingError)
+ clone_post(repository_path) do |response|
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
end
end
context "POST git-receive-pack" do
it "fails to find a route" do
- expect { push_post(repository_path) }.to raise_error(ActionController::RoutingError)
+ push_post(repository_path) do |response|
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
end
end
end
diff --git a/spec/routing/git_http_routing_spec.rb b/spec/routing/git_http_routing_spec.rb
index e3cc1440a9e..79d392e4132 100644
--- a/spec/routing/git_http_routing_spec.rb
+++ b/spec/routing/git_http_routing_spec.rb
@@ -7,6 +7,10 @@ RSpec.describe 'git_http routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/gitlab-test.git' }
end
+
+ it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+ let(:path) { '/gitlab-org/gitlab-test.git' }
+ end
end
describe 'wiki repositories' do
@@ -14,6 +18,7 @@ RSpec.describe 'git_http routing' do
let(:path) { '/gitlab-org/gitlab-test.wiki.git' }
it_behaves_like 'git repository routes'
+ it_behaves_like 'git repository routes with fallback for git-upload-pack'
describe 'redirects', type: :request do
let(:web_path) { '/gitlab-org/gitlab-test/-/wikis' }
@@ -37,12 +42,20 @@ RSpec.describe 'git_http routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org.wiki.git' }
end
+
+ it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+ let(:path) { '/gitlab-org.wiki.git' }
+ end
end
context 'in child group' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/child.wiki.git' }
end
+
+ it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+ let(:path) { '/gitlab-org/child.wiki.git' }
+ end
end
end
@@ -51,12 +64,20 @@ RSpec.describe 'git_http routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/snippets/123.git' }
end
+
+ it_behaves_like 'git repository routes without fallback' do
+ let(:path) { '/snippets/123.git' }
+ end
end
context 'project snippet' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
end
+
+ it_behaves_like 'git repository routes with fallback' do
+ let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
+ end
end
end
end
diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb
index 29e5c1b4bae..f7ed8d7d5dc 100644
--- a/spec/routing/project_routing_spec.rb
+++ b/spec/routing/project_routing_spec.rb
@@ -876,4 +876,73 @@ RSpec.describe 'project routing' do
)
end
end
+
+ context 'with a non-existent project' do
+ it 'routes to 404 with get request' do
+ expect(get: "/gitlab/not_exist").to route_to(
+ 'application#route_not_found',
+ unmatched_route: 'gitlab/not_exist'
+ )
+ end
+
+ it 'routes to 404 with delete request' do
+ expect(delete: "/gitlab/not_exist").to route_to(
+ 'application#route_not_found',
+ namespace_id: 'gitlab',
+ project_id: 'not_exist'
+ )
+ end
+
+ it 'routes to 404 with post request' do
+ expect(post: "/gitlab/not_exist").to route_to(
+ 'application#route_not_found',
+ namespace_id: 'gitlab',
+ project_id: 'not_exist'
+ )
+ end
+
+ it 'routes to 404 with put request' do
+ expect(put: "/gitlab/not_exist").to route_to(
+ 'application#route_not_found',
+ namespace_id: 'gitlab',
+ project_id: 'not_exist'
+ )
+ end
+
+ context 'with route to some action' do
+ it 'routes to 404 with get request to' do
+ expect(get: "/gitlab/not_exist/some_action").to route_to(
+ 'application#route_not_found',
+ unmatched_route: 'gitlab/not_exist/some_action'
+ )
+ end
+
+ it 'routes to 404 with delete request' do
+ expect(delete: "/gitlab/not_exist/some_action").to route_to(
+ 'application#route_not_found',
+ namespace_id: 'gitlab',
+ project_id: 'not_exist',
+ all: 'some_action'
+ )
+ end
+
+ it 'routes to 404 with post request' do
+ expect(post: "/gitlab/not_exist/some_action").to route_to(
+ 'application#route_not_found',
+ namespace_id: 'gitlab',
+ project_id: 'not_exist',
+ all: 'some_action'
+ )
+ end
+
+ it 'routes to 404 with put request' do
+ expect(put: "/gitlab/not_exist/some_action").to route_to(
+ 'application#route_not_found',
+ namespace_id: 'gitlab',
+ project_id: 'not_exist',
+ all: 'some_action'
+ )
+ end
+ end
+ end
end
diff --git a/spec/support/matchers/route_to_route_not_found_matcher.rb b/spec/support/matchers/route_to_route_not_found_matcher.rb
new file mode 100644
index 00000000000..4105f0f9191
--- /dev/null
+++ b/spec/support/matchers/route_to_route_not_found_matcher.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+RSpec::Matchers.define :route_to_route_not_found do
+ match do |actual|
+ expect(actual).to route_to(controller: 'application', action: 'route_not_found')
+ rescue RSpec::Expectations::ExpectationNotMetError => e
+ # `route_to` matcher requires providing all params for exact match. As we use it in shared examples and we provide different paths,
+ # this matcher checks if provided route matches controller and action, without checking params.
+ expect(e.message).to include("-{\"controller\"=>\"application\", \"action\"=>\"route_not_found\"}\n+{\"controller\"=>\"application\", \"action\"=>\"route_not_found\",")
+ end
+
+ failure_message do |_|
+ "expected #{actual} to route to route_not_found"
+ end
+end
diff --git a/spec/support/shared_examples/routing/git_http_routing_shared_examples.rb b/spec/support/shared_examples/routing/git_http_routing_shared_examples.rb
index b0e1e942d81..f924da37f4f 100644
--- a/spec/support/shared_examples/routing/git_http_routing_shared_examples.rb
+++ b/spec/support/shared_examples/routing/git_http_routing_shared_examples.rb
@@ -16,10 +16,6 @@ RSpec.shared_examples 'git repository routes' do
expect(get("#{container_path}/info/refs?service=git-upload-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-upload-pack")
expect(get("#{container_path}/info/refs?service=git-receive-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-receive-pack")
end
-
- it 'does not redirect other requests' do
- expect(post("#{container_path}/git-upload-pack")).not_to be_routable
- end
end
it 'routes LFS endpoints' do
@@ -35,6 +31,56 @@ RSpec.shared_examples 'git repository routes' do
expect(get("#{path}/gitlab-lfs/objects/#{oid}")).to route_to('repositories/lfs_storage#download', oid: oid, **params)
expect(put("#{path}/gitlab-lfs/objects/#{oid}/456/authorize")).to route_to('repositories/lfs_storage#upload_authorize', oid: oid, size: '456', **params)
expect(put("#{path}/gitlab-lfs/objects/#{oid}/456")).to route_to('repositories/lfs_storage#upload_finalize', oid: oid, size: '456', **params)
+ end
+end
+
+RSpec.shared_examples 'git repository routes without fallback' do
+ let(:container_path) { path.delete_suffix('.git') }
+
+ context 'requests without .git format' do
+ it 'does not redirect other requests' do
+ expect(post("#{container_path}/git-upload-pack")).not_to be_routable
+ end
+ end
+
+ it 'routes LFS endpoints for unmatched routes' do
+ oid = generate(:oid)
+
+ expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
+ expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable
+ expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).not_to be_routable
+ end
+end
+
+RSpec.shared_examples 'git repository routes with fallback' do
+ let(:container_path) { path.delete_suffix('.git') }
+
+ context 'requests without .git format' do
+ it 'does not redirect other requests' do
+ expect(post("#{container_path}/git-upload-pack")).to route_to_route_not_found
+ end
+ end
+
+ it 'routes LFS endpoints' do
+ oid = generate(:oid)
+
+ expect(put("#{path}/gitlab-lfs/objects/foo")).to route_to_route_not_found
+ expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).to route_to_route_not_found
+ expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).to route_to_route_not_found
+ end
+end
+
+RSpec.shared_examples 'git repository routes with fallback for git-upload-pack' do
+ let(:container_path) { path.delete_suffix('.git') }
+
+ context 'requests without .git format' do
+ it 'does not redirect other requests' do
+ expect(post("#{container_path}/git-upload-pack")).to route_to_route_not_found
+ end
+ end
+
+ it 'routes LFS endpoints for unmatched routes' do
+ oid = generate(:oid)
expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable
View Lorry Controller Status