Make getting a user by the username case insensitive

4 files changed, 210 insertions, 9 deletions

diff --git a/spec/finders/user_finder_spec.rb b/spec/finders/user_finder_spec.rb
index e53aa50dd33..4771b878b8e 100644
--- a/spec/finders/user_finder_spec.rb
+++ b/spec/finders/user_finder_spec.rb
@@ -3,40 +3,176 @@
 require 'spec_helper'
 
 describe UserFinder do
-  describe '#execute' do
+  set(:user) { create(:user) }
+
+  describe '#find_by_id' do
+    context 'when the user exists' do
+      it 'returns the user' do
+        found = described_class.new(user.id).find_by_id
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user exists (id as string)' do
+      it 'returns the user' do
+        found = described_class.new(user.id.to_s).find_by_id
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user does not exist' do
+      it 'returns nil' do
+        found = described_class.new(1).find_by_id
+
+        expect(found).to be_nil
+      end
+    end
+  end
+
+  describe '#find_by_username' do
     context 'when the user exists' do
       it 'returns the user' do
-        user = create(:user)
-        found = described_class.new(id: user.id).execute
+        found = described_class.new(user.username).find_by_username
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user does not exist' do
+      it 'returns nil' do
+        found = described_class.new("non_existent_username").find_by_username
+
+        expect(found).to be_nil
+      end
+    end
+  end
+
+  describe '#find_by_id_or_username' do
+    context 'when the user exists (id)' do
+      it 'returns the user' do
+        found = described_class.new(user.id).find_by_id_or_username
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user exists (id as string)' do
+      it 'returns the user' do
+        found = described_class.new(user.id.to_s).find_by_id_or_username
 
         expect(found).to eq(user)
       end
     end
 
+    context 'when the user exists (username)' do
+      it 'returns the user' do
+        found = described_class.new(user.username).find_by_id_or_username
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user does not exist (username)' do
+      it 'returns nil' do
+        found = described_class.new("non_existent_username").find_by_id_or_username
+
+        expect(found).to be_nil
+      end
+    end
+
     context 'when the user does not exist' do
       it 'returns nil' do
-        found = described_class.new(id: 1).execute
+        found = described_class.new(1).find_by_id_or_username
 
         expect(found).to be_nil
       end
     end
   end
 
-  describe '#execute!' do
+  describe '#find_by_id!' do
+    context 'when the user exists' do
+      it 'returns the user' do
+        found = described_class.new(user.id).find_by_id!
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user exists (id as string)' do
+      it 'returns the user' do
+        found = described_class.new(user.id.to_s).find_by_id!
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user does not exist' do
+      it 'raises ActiveRecord::RecordNotFound' do
+        finder = described_class.new(1)
+
+        expect { finder.find_by_id! }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+
+  describe '#find_by_username!' do
     context 'when the user exists' do
       it 'returns the user' do
-        user = create(:user)
-        found = described_class.new(id: user.id).execute!
+        found = described_class.new(user.username).find_by_username!
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user does not exist' do
+      it 'raises ActiveRecord::RecordNotFound' do
+        finder = described_class.new("non_existent_username")
+
+        expect { finder.find_by_username! }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+
+  describe '#find_by_id_or_username!' do
+    context 'when the user exists (id)' do
+      it 'returns the user' do
+        found = described_class.new(user.id).find_by_id_or_username!
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user exists (id as string)' do
+      it 'returns the user' do
+        found = described_class.new(user.id.to_s).find_by_id_or_username!
 
         expect(found).to eq(user)
       end
     end
 
+    context 'when the user exists (username)' do
+      it 'returns the user' do
+        found = described_class.new(user.username).find_by_id_or_username!
+
+        expect(found).to eq(user)
+      end
+    end
+
+    context 'when the user does not exist (username)' do
+      it 'raises ActiveRecord::RecordNotFound' do
+        finder = described_class.new("non_existent_username")
+
+        expect { finder.find_by_id_or_username! }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
     context 'when the user does not exist' do
       it 'raises ActiveRecord::RecordNotFound' do
-        finder = described_class.new(id: 1)
+        finder = described_class.new(1)
 
-        expect { finder.execute! }.to raise_error(ActiveRecord::RecordNotFound)
+        expect { finder.find_by_id_or_username! }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
   end
diff --git a/spec/finders/users_finder_spec.rb b/spec/finders/users_finder_spec.rb
index 4249c52c481..fecf97dc641 100644
--- a/spec/finders/users_finder_spec.rb
+++ b/spec/finders/users_finder_spec.rb
@@ -22,6 +22,12 @@ describe UsersFinder do
         expect(users).to contain_exactly(user1)
       end
 
+      it 'filters by username (case insensitive)' do
+        users = described_class.new(user, username: 'joHNdoE').execute
+
+        expect(users).to contain_exactly(user1)
+      end
+
       it 'filters by search' do
         users = described_class.new(user, search: 'orando').execute
 
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index 0a789d58fd8..cca449e9e56 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -368,6 +368,14 @@ describe API::Helpers do
                 it_behaves_like 'successful sudo'
               end
 
+              context 'when providing username (case insensitive)' do
+                before do
+                  env[API::Helpers::SUDO_HEADER] = user.username.upcase
+                end
+
+                it_behaves_like 'successful sudo'
+              end
+
               context 'when providing user ID' do
                 before do
                   env[API::Helpers::SUDO_HEADER] = user.id.to_s
@@ -386,6 +394,14 @@ describe API::Helpers do
                 it_behaves_like 'successful sudo'
               end
 
+              context 'when providing username (case insensitive)' do
+                before do
+                  set_param(API::Helpers::SUDO_PARAM, user.username.upcase)
+                end
+
+                it_behaves_like 'successful sudo'
+              end
+
               context 'when providing user ID' do
                 before do
                   set_param(API::Helpers::SUDO_PARAM, user.id.to_s)
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 09c1d016081..e6d01c9689f 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -51,6 +51,15 @@ describe API::Users do
         expect(json_response[0]['username']).to eq(user.username)
       end
 
+      it "returns the user when a valid `username` parameter is passed (case insensitive)" do
+        get api("/users"), username: user.username.upcase
+
+        expect(response).to match_response_schema('public_api/v4/user/basics')
+        expect(json_response.size).to eq(1)
+        expect(json_response[0]['id']).to eq(user.id)
+        expect(json_response[0]['username']).to eq(user.username)
+      end
+
       it "returns an empty response when an invalid `username` parameter is passed" do
         get api("/users"), username: 'invalid'
 
@@ -132,6 +141,14 @@ describe API::Users do
         expect(json_response.first['username']).to eq(omniauth_user.username)
       end
 
+      it "returns one user (case insensitive)" do
+        get api("/users?username=#{omniauth_user.username.upcase}", user)
+
+        expect(response).to match_response_schema('public_api/v4/user/basics')
+        expect(response).to include_pagination_headers
+        expect(json_response.first['username']).to eq(omniauth_user.username)
+      end
+
       it "returns a 403 when non-admin user searches by external UID" do
         get api("/users?extern_uid=#{omniauth_user.identities.first.extern_uid}&provider=#{omniauth_user.identities.first.provider}", user)
 
@@ -343,6 +360,12 @@ describe API::Users do
         let(:path) { "/users/#{user.username}/status" }
       end
     end
+
+    context 'when finding the user by username (case insensitive)' do
+      it_behaves_like 'rendering user status' do
+        let(:path) { "/users/#{user.username.upcase}/status" }
+      end
+    end
   end
 
   describe "POST /users" do
@@ -528,6 +551,18 @@ describe API::Users do
         expect(json_response['message']).to eq('Username has already been taken')
       end
 
+      it 'returns 409 conflict error if same username exists (case insensitive)' do
+        expect do
+          post api('/users', admin),
+               name: 'foo',
+               email: 'foo@example.com',
+               password: 'password',
+               username: 'TEST'
+        end.to change { User.count }.by(0)
+        expect(response).to have_gitlab_http_status(409)
+        expect(json_response['message']).to eq('Username has already been taken')
+      end
+
       it 'creates user with new identity' do
         post api("/users", admin), attributes_for(:user, provider: 'github', extern_uid: '67890')
 
@@ -749,6 +784,14 @@ describe API::Users do
         expect(response).to have_gitlab_http_status(409)
         expect(@user.reload.username).to eq(@user.username)
       end
+
+      it 'returns 409 conflict error if username taken (case insensitive)' do
+        @user_id = User.all.last.id
+        put api("/users/#{@user.id}", admin), username: 'TEST'
+
+        expect(response).to have_gitlab_http_status(409)
+        expect(@user.reload.username).to eq(@user.username)
+      end
     end
   end