From 4f1de5faacb6824bad2624b75537e9f4ddbb1207 Mon Sep 17 00:00:00 2001
From: Will Starms <vilhelmen@gmail.com>
Date: Thu, 25 Aug 2016 11:48:08 -0500
Subject: [PATCH] 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                     |  1 +
 app/models/namespace.rb       | 14 ++++++--------
 lib/gitlab/regex.rb           |  4 ++--
 spec/features/groups_spec.rb  | 32 +++++++++++++++++++++++++++++++-
 spec/models/namespace_spec.rb |  1 +
 5 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 5e775cec6d4..00db2f2d40f 100644
--- a/CHANGELOG
+++ b/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)
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index b7f2b2bbe61..b67049f0f55 100644
--- a/app/models/namespace.rb
+++ b/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.
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 776bbcbb5d0..0d30e1bb92e 100644
--- a/lib/gitlab/regex.rb
+++ b/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
diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb
index 2d8b59472e8..c54ec2563ad 100644
--- a/spec/features/groups_spec.rb
+++ b/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
 
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 544920d1824..431b3e4435f 100644
--- a/spec/models/namespace_spec.rb
+++ b/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