Correct namespace validation to forbid bad names #21077

Adds .git and .atom to the master namespace regex
Updates existing group tests and adds two new ones
Updates path cleaning to also forbid .atom

CHANGELOG

@@ -30,6 +30,7 @@ v 8.13.0 (unreleased)
   - Allow the Koding integration to be configured through the API
   - Add new issue button to each list on Issues Board
   - Added soft wrap button to repository file/blob editor
+  - Update namespace validation to forbid reserved names (.git and .atom) (Will Starms)
   - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps)
   - Fix todos page mobile viewport layout (ClemMakesApps)
   - Fix inconsistent highlighting of already selected activity nav-links (ClemMakesApps)

app/models/namespace.rb

@@ -61,15 +61,13 @@ class Namespace < ActiveRecord::Base
     def clean_path(path)
       path = path.dup
       # Get the email username by removing everything after an `@` sign.
-      path.gsub!(/@.*\z/,             "")
-      # Usernames can't end in .git, so remove it.
-      path.gsub!(/\.git\z/,           "")
-      # Remove dashes at the start of the username.
-      path.gsub!(/\A-+/,              "")
-      # Remove periods at the end of the username.
-      path.gsub!(/\.+\z/,             "")
+      path.gsub!(/@.*\z/,                "")
       # Remove everything that's not in the list of allowed characters.
-      path.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
+      path.gsub!(/[^a-zA-Z0-9_\-\.]/,    "")
+      # Remove trailing violations ('.atom', '.git', or '.')
+      path.gsub!(/(\.atom|\.git|\.)*\z/, "")
+      # Remove leading violations ('-')
+      path.gsub!(/\A\-+/,                "")
       # Users with the great usernames of "." or ".." would end up with a blank username.
       # Work around that by setting their username to "blank", followed by a counter.

lib/gitlab/regex.rb

@@ -2,7 +2,7 @@ module Gitlab
   module Regex
     extend self
-    NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*[a-zA-Z0-9_\-]|[a-zA-Z0-9_])'.freeze
+    NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*[a-zA-Z0-9_\-]|[a-zA-Z0-9_])(?<!\.git|\.atom)'.freeze
     def namespace_regex
       @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
@@ -10,7 +10,7 @@ module Gitlab
     def namespace_regex_message
       "can contain only letters, digits, '_', '-' and '.'. " \
-      "Cannot start with '-' or end in '.'." \
+      "Cannot start with '-' or end in '.', '.git' or '.atom'." \
     end
     def namespace_name_regex

spec/features/groups_spec.rb

@@ -5,6 +5,12 @@ feature 'Group', feature: true do
     login_as(:admin)
   end
+  matcher :have_namespace_error_message do
+    match do |page|
+      page.has_content?("Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.', '.git' or '.atom'.")
+    end
+  end
+
   describe 'creating a group with space in group path' do
     it 'renders new group form with validation errors' do
       visit new_group_path
@@ -13,7 +19,31 @@ feature 'Group', feature: true do
       click_button 'Create group'
       expect(current_path).to eq(groups_path)
-      expect(page).to have_content("Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-' or end in '.'.")
+      expect(page).to have_namespace_error_message
+    end
+  end
+  
+  describe 'creating a group with .atom at end of group path' do
+    it 'renders new group form with validation errors' do
+      visit new_group_path
+      fill_in 'Group path', with: 'atom_group.atom'
+
+      click_button 'Create group'
+
+      expect(current_path).to eq(groups_path)
+      expect(page).to have_namespace_error_message
+    end
+  end
+  
+  describe 'creating a group with .git at end of group path' do
+    it 'renders new group form with validation errors' do
+      visit new_group_path
+      fill_in 'Group path', with: 'git_group.git'
+
+      click_button 'Create group'
+
+      expect(current_path).to eq(groups_path)
+      expect(page).to have_namespace_error_message
     end
   end

spec/models/namespace_spec.rb

@@ -114,6 +114,7 @@ describe Namespace, models: true do
     it "cleans the path and makes sure it's available" do
       expect(Namespace.clean_path("-john+gitlab-ETC%.git@gmail.com")).to eq("johngitlab-ETC2")
+      expect(Namespace.clean_path("--%+--valid_*&%name=.git.%.atom.atom.@email.com")).to eq("valid_name")
     end
   end
 end