Uniquify reserved word usernames on OAuth user creation

changelogs/unreleased/handle-reserved-words-for-oauth-usernames.yml

+---
+title: Uniquify reserved word usernames on OAuth user creation
+merge_request: 13244
+author: Robin Bobbitt

lib/gitlab/o_auth/user.rb

@@ -166,12 +166,17 @@ module Gitlab
         username ||= auth_hash.username
         email ||= auth_hash.email
+        valid_username = ::Namespace.clean_path(username)
+
+        uniquify = Uniquify.new
+        valid_username = uniquify.string(valid_username) { |s| !DynamicPathValidator.valid_user_path?(s) }
+
         name = auth_hash.name
-        name = ::Namespace.clean_path(username) if name.strip.empty?
+        name = valid_username if name.strip.empty?
         {
           name:                       name,
-          username:                   ::Namespace.clean_path(username),
+          username:                   valid_username,
           email:                      email,
           password:                   auth_hash.password,
           password_confirmation:      auth_hash.password,

spec/lib/gitlab/o_auth/user_spec.rb

@@ -457,4 +457,34 @@ describe Gitlab::OAuth::User do
       end
     end
   end
+
+  describe 'generating username' do
+    context 'when no collision with existing user' do
+      it 'generates the username with no counter' do
+        expect(gl_user.username).to eq('johngitlab-ETC')
+      end
+    end
+
+    context 'when collision with existing user' do
+      it 'generates the username with a counter' do
+        oauth_user.save
+        oauth_user2 = described_class.new(OmniAuth::AuthHash.new(uid: 'my-uid2', provider: provider, info: { nickname: 'johngitlab-ETC@othermail.com', email: 'john@othermail.com' }))
+
+        expect(oauth_user2.gl_user.username).to eq('johngitlab-ETC1')
+      end
+    end
+
+    context 'when username is a reserved word' do
+      let(:info_hash) do
+        {
+          nickname: 'admin@othermail.com',
+          email: 'admin@othermail.com'
+        }
+      end
+      
+      it 'generates the username with a counter' do
+        expect(gl_user.username).to eq('admin1')
+      end
+    end
+  end
 end