diff options
author | Luke Duncalfe <lduncalfe@eml.cc> | 2019-05-23 07:33:11 +0300 |
---|---|---|
committer | Luke Duncalfe <lduncalfe@eml.cc> | 2019-06-10 23:21:04 +0300 |
commit | ba377e91e1179b5b1124df1fcdda22c1b63e82a1 (patch) | |
tree | 3b4d44618b710f02055374154119f4f3123bb2dd /spec | |
parent | e5dcd1101b027d35ca23123c0712a483629b9bf6 (diff) |
Authorize access before serving project template
Previously, if a user was a guest member of a private project, they
could access the merge request template as we were not checking
permission-levels of the user.
When a issue template is asked for, the user must have :read_issue for
the project; or :read_merge_request when a merge request template is
asked for.
We also now rescue_from FileNotFoundError and handle as 404. This is
because RepoTemplateFinder can raise a FileNotFoundError exception,
which Rails previously handled as a 500.
Handling these in a way that is consistent with
ActiveRecord::RecordNotFound exceptions, within controllers that
inherit from Projects::ApplicationController at least, and returning a
404.
https://gitlab.com/gitlab-org/gitlab-ce/issues/54943
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/templates_controller_spec.rb | 110 | ||||
-rw-r--r-- | spec/routing/project_routing_spec.rb | 20 |
2 files changed, 101 insertions, 29 deletions
diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index bebf17728c0..9e7d34b10c0 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -3,49 +3,101 @@ require 'spec_helper' describe Projects::TemplatesController do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, :private) } let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:file_path_1) { '.gitlab/issue_templates/bug.md' } + let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' } + let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' } let(:body) { JSON.parse(response.body) } + let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') } + let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') } - before do - project.add_developer(user) - sign_in(user) - end + describe '#show' do + shared_examples 'renders issue templates as json' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) - before do - project.add_user(user, Gitlab::Access::MAINTAINER) - project.repository.create_file(user, file_path_1, 'something valid', - message: 'test 3', branch_name: 'master') - end + expect(response.status).to eq(200) + expect(body['name']).to eq('issue_template') + expect(body['content']).to eq('issue content') + end + end - describe '#show' do - it 'renders template name and content as json' do - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json) + shared_examples 'renders merge request templates as json' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) + + expect(response.status).to eq(200) + expect(body['name']).to eq('merge_request_template') + expect(body['content']).to eq('merge request content') + end + end + + shared_examples 'renders 404 when requesting an issue template' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) + + expect(response.status).to eq(404) + end + end + + shared_examples 'renders 404 when requesting a merge request template' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) - expect(response.status).to eq(200) - expect(body["name"]).to eq("bug") - expect(body["content"]).to eq("something valid") + expect(response.status).to eq(404) + end end - it 'renders 404 when unauthorized' do - sign_in(user2) - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json) + shared_examples 'renders 404 when params are invalid' do + it 'does not route when the template type is invalid' do + expect do + get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template', project_id: project }, format: :json) + end.to raise_error(ActionController::UrlGenerationError) + end + + it 'renders 404 when the format type is invalid' do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :html) + + expect(response.status).to eq(404) + end + + it 'renders 404 when the key is unknown' do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'unknown_template', project_id: project }, format: :json) - expect(response.status).to eq(404) + expect(response.status).to eq(404) + end end - it 'renders 404 when template type is not found' do - sign_in(user) - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) + context 'when the user is not a member of the project' do + before do + sign_in(user) + end - expect(response.status).to eq(404) + include_examples 'renders 404 when requesting an issue template' + include_examples 'renders 404 when requesting a merge request template' + include_examples 'renders 404 when params are invalid' end - it 'renders 404 without errors' do - sign_in(user) - expect { get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) }.not_to raise_error + context 'when user is a member of the project' do + before do + project.add_developer(user) + sign_in(user) + end + + include_examples 'renders issue templates as json' + include_examples 'renders merge request templates as json' + include_examples 'renders 404 when params are invalid' + end + + context 'when user is a guest of the project' do + before do + project.add_guest(user) + sign_in(user) + end + + include_examples 'renders issue templates as json' + include_examples 'renders 404 when requesting a merge request template' + include_examples 'renders 404 when params are invalid' end end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 83775b1040e..6dde40d1cb6 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -693,4 +693,24 @@ describe 'project routing' do it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/settings/repository", "/gitlab/gitlabhq/-/settings/repository" end + + describe Projects::TemplatesController, 'routing' do + describe '#show' do + def show_with_template_type(template_type) + "/gitlab/gitlabhq/templates/#{template_type}/template_name" + end + + it 'routes when :template_type is `merge_request`' do + expect(get(show_with_template_type('merge_request'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'merge_request', key: 'template_name', format: 'json') + end + + it 'routes when :template_type is `issue`' do + expect(get(show_with_template_type('issue'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'issue', key: 'template_name', format: 'json') + end + + it 'routes to application#route_not_found when :template_type is unknown' do + expect(get(show_with_template_type('invalid'))).to route_to('application#route_not_found', unmatched_route: 'gitlab/gitlabhq/templates/invalid/template_name') + end + end + end end |