diff options
-rw-r--r-- | changelogs/unreleased/handle-reserved-words-for-oauth-usernames.yml | 4 | ||||
-rw-r--r-- | lib/gitlab/o_auth/user.rb | 9 | ||||
-rw-r--r-- | spec/lib/gitlab/o_auth/user_spec.rb | 30 |
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 |