Clean username acquired from OAuth/LDAP.

Fixes #1967.

5 files changed, 33 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6a90320b8bc..0b369acf483 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -75,6 +75,7 @@ v 7.8.0 (unreleased)
   - Added support for firing system hooks on group create/destroy and adding/removing users to group (Boyan Tabakov)
   - Added persistent collapse button for left side nav bar (Jason Blanchard)
   - Prevent losing unsaved comments by automatically restoring them when comment page is loaded again.
+  - Clean the username acquired from OAuth/LDAP so it doesn't fail username validation and block signing up.
 
 v 7.7.2
   - Update GitLab Shell to version 2.4.2 that fixes a bug when developers can push to protected branch
diff --git a/app/models/user.rb b/app/models/user.rb
index 3a7dfabeafe..d7f688ec138 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -243,6 +243,22 @@ class User < ActiveRecord::Base
     def build_user(attrs = {})
       User.new(attrs)
     end
+
+    def clean_username(username)
+      username.gsub!(/@.*\z/,             "")
+      username.gsub!(/\.git\z/,           "")
+      username.gsub!(/\A-/,               "")
+      username.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
+
+      counter = 0
+      base = username
+      while by_login(username).present?
+        counter += 1 
+        username = "#{base}#{counter}"
+      end
+
+      username
+    end
   end
 
   #
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 6861427864e..9f55e8c4950 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -85,11 +85,11 @@ module Gitlab
 
       def user_attributes
         {
-          name: auth_hash.name,
-          username: auth_hash.username,
-          email: auth_hash.email,
-          password: auth_hash.password,
-          password_confirmation: auth_hash.password
+          name:                   auth_hash.name,
+          username:               ::User.clean_username(auth_hash.username),
+          email:                  auth_hash.email,
+          password:               auth_hash.password,
+          password_confirmation:  auth_hash.password
         }
       end
 
diff --git a/spec/lib/gitlab/oauth/user_spec.rb b/spec/lib/gitlab/oauth/user_spec.rb
index 88307515789..2680794a747 100644
--- a/spec/lib/gitlab/oauth/user_spec.rb
+++ b/spec/lib/gitlab/oauth/user_spec.rb
@@ -8,7 +8,7 @@ describe Gitlab::OAuth::User do
   let(:auth_hash) { double(uid: uid, provider: provider, info: double(info_hash)) }
   let(:info_hash) do
     {
-      nickname: 'john',
+      nickname: '-john+gitlab-ETC%.git@gmail.com',
       name: 'John',
       email: 'john@mail.com'
     }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 629d51b960d..7473054f481 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -301,6 +301,16 @@ describe User do
     end
   end
 
+  describe ".clean_username" do
+
+    let!(:user1) { create(:user, username: "johngitlab-etc") }
+    let!(:user2) { create(:user, username: "JohnGitLab-etc1") }
+
+    it "cleans a username and makes sure it's available" do
+      expect(User.clean_username("-john+gitlab-ETC%.git@gmail.com")).to eq("johngitlab-ETC2")
+    end
+  end
+
   describe 'all_ssh_keys' do
     it { should have_many(:keys).dependent(:destroy) }