Validate project templates before creating project

A project will no longer be created if an invalid project name is
provided. This makes it possible for the API and controllers to behave
in a consistent manner, since it's easy to make a mistake and use the
wrong template name.

app/services/projects/create_from_template_service.rb

@@ -9,7 +9,9 @@ module Projects
     end
 
     def execute
-      file = Gitlab::ProjectTemplate.find(template_name)&.file
+      return project unless validate_template!
+
+      file = built_in_template&.file
 
       override_params = params.dup
       params[:file] = file
@@ -24,6 +26,25 @@ module Projects
         params.delete(:template_name).presence
       end
     end
+
+    private
+
+    def validate_template!
+      return true if built_in_template
+
+      project.errors.add(:template_name, _("'%{template_name}' is unknown or invalid" % { template_name: template_name }))
+      false
+    end
+
+    def built_in_template
+      strong_memoize(:built_in_template) do
+        Gitlab::ProjectTemplate.find(template_name)
+      end
+    end
+
+    def project
+      @project ||= ::Project.new(namespace_id: params[:namespace_id])
+    end
   end
 end
 

ee/app/services/ee/projects/create_from_template_service.rb

@@ -9,11 +9,7 @@ module EE
       override :execute
       def execute
         return super unless use_custom_template?
-
-        if subgroup_id && !valid_project_namespace?
-          project.errors.add(:namespace, _("is not a descendant of the Group owning the template"))
-          return project
-        end
+        return project unless validate_group_template!
 
         override_params = params.dup
         params[:custom_template] = template_project if template_project
@@ -23,6 +19,18 @@ module EE
 
       private
 
+      def validate_group_template!
+        if subgroup_id && !valid_project_namespace?
+          project.errors.add(:namespace, _("is not a descendant of the Group owning the template"))
+          return false
+        end
+
+        return true if template_project.present?
+
+        project.errors.add(:template_name, _("'%{template_name}' is unknown or invalid" % { template_name: template_name }))
+        false
+      end
+
       def use_custom_template?
         strong_memoize(:use_custom_template) do
           template_name &&
@@ -51,10 +59,6 @@ module EE
         templates_owner.self_and_descendants.exists?(id: project.namespace_id)
       end
       # rubocop: enable CodeReuse/ActiveRecord
-
-      def project
-        @project ||= ::Project.new(namespace_id: params[:namespace_id])
-      end
     end
   end
 end

ee/spec/controllers/projects_controller_spec.rb

@@ -122,16 +122,19 @@ describe ProjectsController do
       end
 
       context 'when unlicensed' do
+        render_views
+
         before do
           stub_licensed_features(custom_project_templates: false)
+          project
+          project_template
         end
 
-        it 'creates the project from project template' do
-          post :create, params: { project: templates_params }
+        it 'does not create the project from project template' do
+          expect { post :create, params: { project: templates_params } }.not_to change { Project.count }
 
-          created_project = Project.find_by_path('foo')
-          expect(flash[:notice]).to eq "Project 'foo' was successfully created."
-          expect(created_project.repository.empty?).to be true
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.body).to match(/Template name .* is unknown or invalid/)
         end
       end
     end

ee/spec/requests/api/projects_spec.rb

@@ -203,6 +203,16 @@ describe API::Projects do
         project = Project.find(json_response['id'])
         expect(project.name).to eq(new_project_name)
       end
+
+      it 'returns a 400 error for an invalid template name' do
+        project_params[:template_name] = 'bogus-template'
+
+        expect { post api('/projects', user), params: project_params }
+          .not_to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response['message']['template_name']).to eq(["'bogus-template' is unknown or invalid"])
+      end
     end
 
     context 'with instance-level templates' do

ee/spec/services/projects/create_from_template_service_spec.rb

@@ -55,7 +55,7 @@ describe Projects::CreateFromTemplateService do
       after do
         project = subject.execute
 
-        expect(project).to be_saved
+        expect(project).not_to be_saved
         expect(project.repository.empty?).to be true
       end
 
@@ -80,9 +80,8 @@ describe Projects::CreateFromTemplateService do
       context 'when custom_project_template does not exist' do
         let(:project_name) { 'whatever' }
 
-        it 'creates an empty project' do
-          expect(::Projects::GitlabProjectsImportService)
-            .to receive(:new).with(user, hash_excluding(:custom_template), anything).and_call_original
+        it 'does not attempt to import a project' do
+          expect(::Projects::GitlabProjectsImportService).not_to receive(:new)
         end
       end
     end

locale/gitlab.pot

@@ -391,6 +391,9 @@ msgstr ""
 msgid "'%{source}' is not a import source"
 msgstr ""
 
+msgid "'%{template_name}' is unknown or invalid"
+msgstr ""
+
 msgid "(%d closed)"
 msgid_plural "(%d closed)"
 msgstr[0] ""

spec/requests/api/projects_spec.rb

@@ -642,6 +642,14 @@ describe API::Projects do
       expect(project.import_type).to eq('gitlab_project')
     end
 
+    it 'returns 400 for an invalid template' do
+      expect { post api('/projects', user), params: { template_name: 'unknown', name: 'rails-test' } }
+        .not_to change { Project.count }
+
+      expect(response).to have_gitlab_http_status(400)
+      expect(json_response['message']['template_name']).to eq(["'unknown' is unknown or invalid"])
+    end
+
     it 'disallows creating a project with an import_url and template' do
       project_params = { import_url: 'http://example.com', template_name: 'rails', name: 'rails-test' }
       expect { post api('/projects', user), params: project_params }

spec/services/projects/create_from_template_service_spec.rb

@@ -25,7 +25,7 @@ describe Projects::CreateFromTemplateService do
     subject.execute
   end
 
-  it 'returns the project thats created' do
+  it 'returns the project that is created' do
     project = subject.execute
 
     expect(project).to be_saved
@@ -37,7 +37,7 @@ describe Projects::CreateFromTemplateService do
     let(:project) { subject.execute }
 
     before do
-      expect(project).to be_saved
+      expect(project).not_to be_saved
     end
 
     it 'does not set import set import type' do