Merge branch '912-fix-no-licence-notification-on-push' into 'master'

Diplay correct message in git clients when License is not configured

See merge request !1133

app/controllers/projects/git_http_controller.rb

@@ -76,7 +76,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access_denied_message
-    project.above_size_limit? ? access_check.message : 'Access denied'
+    access_check.message || 'Access denied'
   end
 
   def upload_pack_allowed?

app/helpers/license_helper.rb

 module LicenseHelper
+  include ActionView::Helpers::AssetTagHelper
+  include ActionView::Helpers::UrlHelper
+
+  delegate :new_admin_license_path, to: 'Gitlab::Routing.url_helpers'
+
   def current_active_user_count
     User.active.count
   end
@@ -12,28 +17,24 @@ module LicenseHelper
       if License.current
         yes_license_message(signed_in, is_admin)
       else
-        no_license_message(signed_in, is_admin)
+        no_license_message(is_admin)
       end
   end
 
   private
 
-  def no_license_message(signed_in, is_admin)
+  def no_license_message(is_admin)
     message = []
-
-    message << "No GitLab Enterprise Edition license has been provided yet."
-    message << "Pushing code and creation of issues and merge requests has been disabled."
-
+    message << 'No GitLab Enterprise Edition license has been provided yet.'
+    message << 'Pushing code and creation of issues and merge requests has been disabled.'
     message <<
       if is_admin
-        "#{link_to('Upload a license', new_admin_license_path)} in the admin area"
+        "#{link_to('Upload a license', new_admin_license_path)} in the admin area to activate this functionality."
       else
-        "Ask an admin to upload a license"
+        'Ask an admin to upload a license to activate this functionality.'
       end
 
-    message << "to activate this functionality."
-
-    content_tag(:p, message.join(" ").html_safe)
+    content_tag(:p, message.join(' ').html_safe)
   end
 
   def yes_license_message(signed_in, is_admin)
@@ -45,33 +46,33 @@ module LicenseHelper
 
     message = []
 
-    message << "The GitLab Enterprise Edition license"
-    message << (license.expired? ? "expired" : "will expire")
+    message << 'The GitLab Enterprise Edition license'
+    message << (license.expired? ? 'expired' : 'will expire')
     message << "on #{license.expires_at}."
 
     if license.expired? && license.will_block_changes?
-      message << "Pushing code and creation of issues and merge requests"
+      message << 'Pushing code and creation of issues and merge requests'
 
       message <<
         if license.block_changes?
-          "has been disabled."
+          'has been disabled.'
         else
           "will be disabled on #{license.block_changes_at}."
         end
 
       message <<
         if is_admin
-          "Upload a new license in the admin area"
+          'Upload a new license in the admin area'
         else
-          "Ask an admin to upload a new license"
+          'Ask an admin to upload a new license'
         end
 
-      message << "to"
-      message << (license.block_changes? ? "restore" : "ensure uninterrupted")
-      message << "service."
+      message << 'to'
+      message << (license.block_changes? ? 'restore' : 'ensure uninterrupted')
+      message << 'service.'
     end
 
-    message.join(" ")
+    message.join(' ')
   end
 
   extend self

lib/gitlab/git_access.rb

@@ -2,6 +2,7 @@
 # class return an instance of `GitlabAccessStatus`
 module Gitlab
   class GitAccess
+    include ActionView::Helpers::SanitizeHelper
     include PathLocksHelper
     UnauthorizedError = Class.new(StandardError)
 
@@ -148,7 +149,7 @@ module Gitlab
 
       if ::License.block_changes?
         message = ::LicenseHelper.license_message(signed_in: true, is_admin: (user && user.is_admin?))
-        raise UnauthorizedError, message
+        raise UnauthorizedError, strip_tags(message)
       end
 
       check_change_access!(changes)

spec/helpers/license_helper_spec.rb

@@ -4,19 +4,26 @@ describe LicenseHelper do
   describe '#license_message' do
     context 'no license installed' do
       before do
-        expect(License).to receive(:current).and_return(nil)
+        allow(License).to receive(:current).and_return(nil)
       end
 
-      it 'admin user' do
+      context 'admin user' do
+        let(:is_admin) { true }
+
+        it 'displays correct error message for admin user' do
           admin_msg = '<p>No GitLab Enterprise Edition license has been provided yet. Pushing code and creation of issues and merge requests has been disabled. <a href="/admin/license/new">Upload a license</a> in the admin area to activate this functionality.</p>'
 
-        expect(license_message(signed_in: true, is_admin: true)).to eq(admin_msg)
+          expect(license_message(signed_in: true, is_admin: is_admin)).to eq(admin_msg)
+        end
       end
 
-      it 'normal user' do
+      context 'normal user' do
+        let(:is_admin) { false }
+        it 'displays correct error message for normal user' do
           user_msg = '<p>No GitLab Enterprise Edition license has been provided yet. Pushing code and creation of issues and merge requests has been disabled. Ask an admin to upload a license to activate this functionality.</p>'
 
-        expect(license_message(signed_in: true, is_admin: false)).to eq(user_msg)
+          expect(license_message(signed_in: true, is_admin: is_admin)).to eq(user_msg)
+        end
       end
     end
   end

spec/requests/git_http_spec.rb

@@ -283,6 +283,24 @@ describe 'Git HTTP requests', lib: true do
         end
       end
 
+      context 'when license is not provided' do
+        let(:env) { { user: user.username, password: user.password } }
+
+        before do
+          project.team << [user, :master]
+        end
+
+        it 'responds with status 403' do
+          msg = 'No GitLab Enterprise Edition license has been provided yet. Pushing code and creation of issues and merge requests has been disabled. Ask an admin to upload a license to activate this functionality.'
+          allow(License).to receive(:current).and_return(false)
+
+          upload(path, env) do |response|
+            expect(response).to have_http_status(403)
+            expect(response.body).to eq(msg)
+          end
+        end
+      end
+
       context "when the project is private" do
         before do
           project.update_attribute(:visibility_level, Project::PRIVATE)