diff options
| author | Luke Duncalfe <lduncalfe@eml.cc> | 2019-05-23 16:33:11 +1200 |
|---|---|---|
| committer | Luke Duncalfe <lduncalfe@eml.cc> | 2019-06-12 19:37:31 +1200 |
| commit | 136db4dd54c4aadce6c0426bd7ca6308f96a32da (patch) | |
| tree | 079d40e749de8eb91c274a7581634a452cb5ecb2 /app/controllers/projects | |
| parent | e3eeb779d72006b9fbbaecf9f1d8fbd52a7d6383 (diff) | |
| download | gitlab-ce-136db4dd54c4aadce6c0426bd7ca6308f96a32da.tar.gz | |
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 'app/controllers/projects')
| -rw-r--r-- | app/controllers/projects/application_controller.rb | 5 | ||||
| -rw-r--r-- | app/controllers/projects/templates_controller.rb | 17 |
2 files changed, 20 insertions, 2 deletions
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 781eac7f080..9ffde6afdb7 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -13,6 +13,11 @@ class Projects::ApplicationController < ApplicationController helper_method :repository, :can_collaborate_with_project?, :user_access + rescue_from Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError do |exception| + log_exception(exception) + render_404 + end + private def project diff --git a/app/controllers/projects/templates_controller.rb b/app/controllers/projects/templates_controller.rb index 7ceea4e5b96..f987033a26c 100644 --- a/app/controllers/projects/templates_controller.rb +++ b/app/controllers/projects/templates_controller.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true class Projects::TemplatesController < Projects::ApplicationController - before_action :authenticate_user!, :get_template_class + before_action :authenticate_user! + before_action :authorize_can_read_issuable! + before_action :get_template_class def show template = @template_type.find(params[:key], project) @@ -13,9 +15,20 @@ class Projects::TemplatesController < Projects::ApplicationController private + # User must have: + # - `read_merge_request` to see merge request templates, or + # - `read_issue` to see issue templates + # + # Note params[:template_type] has a route constraint to limit it to + # `merge_request` or `issue` + def authorize_can_read_issuable! + action = [:read_, params[:template_type]].join + + authorize_action!(action) + end + def get_template_class template_types = { issue: Gitlab::Template::IssueTemplate, merge_request: Gitlab::Template::MergeRequestTemplate }.with_indifferent_access @template_type = template_types[params[:template_type]] - render json: [], status: :not_found unless @template_type end end |