Merge branch 'clean-oauth-username' into 'master'

Clean username acquired from OAuth/LDAP.

Fixes #1967.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/813

See merge request !1500

CHANGELOG

@@ -49,6 +49,7 @@ v 7.8.0 (unreleased)
   - 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.
   - Don't allow page to be scaled on mobile.
+  - 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

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
 
   #

lib/gitlab/oauth/user.rb

@@ -86,7 +86,7 @@ module Gitlab
       def user_attributes
         {
           name:                   auth_hash.name,
-          username: auth_hash.username,
+          username:               ::User.clean_username(auth_hash.username),
           email:                  auth_hash.email,
           password:               auth_hash.password,
           password_confirmation:  auth_hash.password

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'
     }

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) }