Merge branch '38126-security-username-change' into 'security-10-0'

Move project repositories between namespaces when renaming users (10.0)

See merge request gitlab/gitlabhq!2200

4 files changed, 50 insertions, 13 deletions

diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb
index 5ab5c80a2f5..b3020484738 100644
--- a/app/models/concerns/storage/legacy_namespace.rb
+++ b/app/models/concerns/storage/legacy_namespace.rb
@@ -7,6 +7,8 @@ module Storage
         raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has tags in container registry')
       end
 
+      expires_full_path_cache
+
       # Move the namespace directory in all storage paths used by member projects
       repository_storage_paths.each do |repository_storage_path|
         # Ensure old directory exists before moving it
diff --git a/changelogs/unreleased/38126-security-username-change.yml b/changelogs/unreleased/38126-security-username-change.yml
new file mode 100644
index 00000000000..6b0dfc3dca2
--- /dev/null
+++ b/changelogs/unreleased/38126-security-username-change.yml
@@ -0,0 +1,5 @@
+---
+title: Move project repositories between namespaces when renaming users
+merge_request:
+author:
+type: security
diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb
index b52b63e05a4..43abc3351f2 100644
--- a/spec/controllers/profiles_controller_spec.rb
+++ b/spec/controllers/profiles_controller_spec.rb
@@ -1,9 +1,10 @@
 require('spec_helper')
 
-describe ProfilesController do
-  describe "PUT update" do
-    it "allows an email update from a user without an external email address" do
-      user = create(:user)
+describe ProfilesController, :request_store do
+  let(:user) { create(:user) }
+
+  describe 'PUT update' do
+    it 'allows an email update from a user without an external email address' do
       sign_in(user)
 
       put :update,
@@ -15,7 +16,7 @@ describe ProfilesController do
       expect(user.unconfirmed_email).to eq('john@gmail.com')
     end
 
-    it "ignores an email update from a user with an external email address" do
+    it 'ignores an email update from a user with an external email address' do
       stub_omniauth_setting(sync_profile_from_provider: ['ldap'])
       stub_omniauth_setting(sync_profile_attributes: true)
 
@@ -32,7 +33,7 @@ describe ProfilesController do
       expect(ldap_user.unconfirmed_email).not_to eq('john@gmail.com')
     end
 
-    it "ignores an email and name update but allows a location update from a user with external email and name, but not external location" do
+    it 'ignores an email and name update but allows a location update from a user with external email and name, but not external location' do
       stub_omniauth_setting(sync_profile_from_provider: ['ldap'])
       stub_omniauth_setting(sync_profile_attributes: true)
 
@@ -51,4 +52,35 @@ describe ProfilesController do
       expect(ldap_user.location).to eq('City, Country')
     end
   end
+
+  describe 'PUT update_username' do
+    let(:namespace) { user.namespace }
+    let(:project) { create(:project_empty_repo, namespace: namespace) }
+    let(:gitlab_shell) { Gitlab::Shell.new }
+    let(:new_username) { 'renamedtosomethingelse' }
+
+    it 'allows username change' do
+      sign_in(user)
+
+      put :update_username,
+        user: { username: new_username }
+
+      user.reload
+
+      expect(response.status).to eq(302)
+      expect(user.username).to eq(new_username)
+    end
+
+    it 'moves dependent projects to new namespace' do
+      sign_in(user)
+
+      put :update_username,
+        user: { username: new_username }
+
+      user.reload
+
+      expect(response.status).to eq(302)
+      expect(gitlab_shell.exists?(project.repository_storage_path, "#{new_username}/#{project.path}.git")).to be_truthy
+    end
+  end
 end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 81d5ab7a6d3..0ff959395fa 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe Namespace do
   let!(:namespace) { create(:namespace) }
+  let(:gitlab_shell) { Gitlab::Shell.new }
 
   describe 'associations' do
     it { is_expected.to have_many :projects }
@@ -151,11 +152,10 @@ describe Namespace do
     end
   end
 
-  describe '#move_dir' do
+  describe '#move_dir', :request_store do
     before do
       @namespace = create :namespace
       @project = create(:project_empty_repo, namespace: @namespace)
-      allow(@namespace).to receive(:path_changed?).and_return(true)
     end
 
     it "raises error when directory exists" do
@@ -163,11 +163,9 @@ describe Namespace do
     end
 
     it "moves dir if path changed" do
-      new_path = @namespace.full_path + "_new"
-      allow(@namespace).to receive(:full_path_was).and_return(@namespace.full_path)
-      allow(@namespace).to receive(:full_path).and_return(new_path)
-      expect(@namespace).to receive(:remove_exports!)
-      expect(@namespace.move_dir).to be_truthy
+      @namespace.update_attributes(path: @namespace.full_path + '_new')
+
+      expect(gitlab_shell.exists?(@project.repository_storage_path, "#{@namespace.path}/#{@project.path}.git")).to be_truthy
     end
 
     context "when any project has container images" do