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