Merge branch 'security-462108/disclose-project-templates-17-3' into '17-3-stable-ee'

Prevent guest access to project templates

See merge request https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4449



Merged-by: GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>
Approved-by: Ash McKenzie <amckenzie@gitlab.com>
Reviewed-by: Ash McKenzie <amckenzie@gitlab.com>
Co-authored-by: Emma Park <epark@gitlab.com>

lib/api/project_templates.rb

@@ -14,6 +14,14 @@ class ProjectTemplates < ::API::Base
     feature_category :source_code_management
+    helpers do
+      def authorize_template_permissions!
+        permission = params[:type] == 'issues' ? :read_project : :download_code
+
+        authorize! permission, user_project, "Your current role does not have the required permissions to access the #{params[:type]} template. Contact your project administrator for assistance."
+      end
+    end
+
     params do
       requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project'
       requires :type, type: String, values: TEMPLATE_TYPES, desc: 'The type (dockerfiles|gitignores|gitlab_ci_ymls|licenses|issues|merge_requests) of the template'
@@ -32,6 +40,8 @@ class ProjectTemplates < ::API::Base
         use :pagination
       end
       get ':id/templates/:type' do
+        authorize_template_permissions!
+
         templates = TemplateFinder.all_template_names(user_project, params[:type]).values.flatten
         present paginate(::Kaminari.paginate_array(templates)), with: Entities::TemplatesList
@@ -60,6 +70,8 @@ class ProjectTemplates < ::API::Base
       end
       get ':id/templates/:type/:name', requirements: TEMPLATE_NAMES_ENDPOINT_REQUIREMENTS do
+        authorize_template_permissions!
+
         begin
           template = TemplateFinder.build(
             params[:type],

spec/requests/api/project_templates_spec.rb

@@ -5,13 +5,15 @@
 RSpec.describe API::ProjectTemplates, feature_category: :source_code_management do
   let_it_be(:public_project) { create(:project, :public, :repository, create_templates: :merge_request, path: 'path.with.dot') }
   let_it_be(:private_project) { create(:project, :private, :repository, create_templates: :issue) }
-  let_it_be(:developer) { create(:user) }
+  let_it_be(:reporter) { create(:user) }
+  let_it_be(:guest) { create(:user) }
   let(:url_encoded_path) { "#{public_project.namespace.path}%2F#{public_project.path}" }
   before do
     stub_feature_flags(remove_monitor_metrics: false)
-    private_project.add_developer(developer)
+    private_project.add_developer(reporter)
+    private_project.add_guest(guest)
   end
   shared_examples 'accepts project paths with dots' do
@@ -64,7 +66,7 @@
     end
     it 'returns issue templates' do
-      get api("/projects/#{private_project.id}/templates/issues", developer)
+      get api("/projects/#{private_project.id}/templates/issues", reporter)
       expect(response).to have_gitlab_http_status(:ok)
       expect(response).to include_pagination_headers
@@ -94,23 +96,58 @@
     end
     it 'permits access to a developer on a private project' do
-      get api("/projects/#{private_project.id}/templates/licenses", developer)
+      get api("/projects/#{private_project.id}/templates/licenses", reporter)
       expect(response).to have_gitlab_http_status(:ok)
       expect(response).to match_response_schema('public_api/v4/template_list')
     end
-  end
-  describe 'GET /projects/:id/templates/licenses' do
-    it 'returns key and name for the listed licenses' do
-      get api("/projects/#{public_project.id}/templates/licenses")
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(response).to match_response_schema('public_api/v4/template_list')
+    context 'when a guest has no permission to a template' do
+      it 'denies access to the dockerfile' do
+        get api("/projects/#{private_project.id}/templates/dockerfiles", guest)
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the dockerfiles template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the gitignore' do
+        get api("/projects/#{private_project.id}/templates/gitignores", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the gitignores template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the gitlab_ci_yml' do
+        get api("/projects/#{private_project.id}/templates/gitlab_ci_ymls", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the gitlab_ci_ymls template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the license' do
+        get api("/projects/#{private_project.id}/templates/licenses", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the licenses template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the merge request' do
+        get api("/projects/#{private_project.id}/templates/merge_requests", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the merge_requests template. Contact your project administrator for assistance.")
+      end
     end
-    it_behaves_like 'accepts project paths with dots' do
-      subject { get api("/projects/#{url_encoded_path}/templates/licenses") }
+    context 'when a guest has permission to an issues template' do
+      it 'returns an issue template' do
+        get api("/projects/#{private_project.id}/templates/issues", guest)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to include_pagination_headers
+        expect(response).to match_response_schema('public_api/v4/template_list')
+        expect(json_response.map { |t| t['key'] }).to match_array(%w[bug feature_proposal template_test (test)])
+      end
     end
   end
@@ -163,7 +200,7 @@
     end
     it 'returns a specific issue template' do
-      get api("/projects/#{private_project.id}/templates/issues/bug", developer)
+      get api("/projects/#{private_project.id}/templates/issues/bug", reporter)
       expect(response).to have_gitlab_http_status(:ok)
       expect(response).to match_response_schema('public_api/v4/template')
@@ -173,7 +210,7 @@
     context 'when issue template uses parentheses' do
       it 'returns a specific issue template' do
-        get api("/projects/#{private_project.id}/templates/issues/(test)", developer)
+        get api("/projects/#{private_project.id}/templates/issues/(test)", reporter)
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to match_response_schema('public_api/v4/template')
@@ -216,7 +253,7 @@
     end
     it 'permits access to a developer on a private project' do
-      get api("/projects/#{private_project.id}/templates/licenses/mit", developer)
+      get api("/projects/#{private_project.id}/templates/licenses/mit", reporter)
       expect(response).to have_gitlab_http_status(:ok)
       expect(response).to match_response_schema('public_api/v4/license')
@@ -237,6 +274,54 @@
     TemplateFinder::VENDORED_TEMPLATES.each do |template_type, _|
       it_behaves_like 'path traversal attempt', template_type
     end
+
+    context 'when a guest has no permission to a template' do
+      it 'denies access to the dockerfile' do
+        get api("/projects/#{private_project.id}/templates/dockerfiles/Binary", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the dockerfiles template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the gitignore' do
+        get api("/projects/#{private_project.id}/templates/gitignores/Actionscript", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the gitignores template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the gitlab_ci_yml' do
+        get api("/projects/#{private_project.id}/templates/gitlab_ci_ymls/Android", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the gitlab_ci_ymls template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the license' do
+        get api("/projects/#{private_project.id}/templates/licenses/mit", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the licenses template. Contact your project administrator for assistance.")
+      end
+
+      it 'denies access to the merge request' do
+        get api("/projects/#{private_project.id}/templates/merge_requests/feature_proposal", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq("403 Forbidden - Your current role does not have the required permissions to access the merge_requests template. Contact your project administrator for assistance.")
+      end
+    end
+
+    context 'when a guest has permission to an issues template' do
+      it 'returns an issue template' do
+        get api("/projects/#{private_project.id}/templates/issues/bug", guest)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('public_api/v4/template')
+        expect(json_response['name']).to eq('bug')
+        expect(json_response['content']).to eq('something valid')
+      end
+    end
   end
   describe 'GET /projects/:id/templates/licenses/:key' do