Uniquify reserved word usernames on OAuth user creation

3 files changed, 41 insertions, 2 deletions

diff --git a/changelogs/unreleased/handle-reserved-words-for-oauth-usernames.yml b/changelogs/unreleased/handle-reserved-words-for-oauth-usernames.yml
new file mode 100644
index 00000000000..0d64844a2b8
--- /dev/null
+++ b/changelogs/unreleased/handle-reserved-words-for-oauth-usernames.yml
@@ -0,0 +1,4 @@
+---
+title: Uniquify reserved word usernames on OAuth user creation
+merge_request: 13244
+author: Robin Bobbitt
diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index 3f2bbd9f6a6..e8330917e91 100644
--- a/lib/gitlab/o_auth/user.rb
+++ b/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,
diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb
index 47aa19d5fd9..6c84c4a0c1e 100644
--- a/spec/lib/gitlab/o_auth/user_spec.rb
+++ b/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