Merge branch 'rate-limit-profile-update-username' into 'master'

Rate limit update username action

See merge request gitlab-org/gitlab!77221

app/controllers/profiles_controller.rb

@@ -6,6 +6,9 @@ class ProfilesController < Profiles::ApplicationController
 
   before_action :user
   before_action :authorize_change_username!, only: :update_username
+  before_action only: :update_username do
+    check_rate_limit!(:profile_update_username, scope: current_user) if Feature.enabled?(:rate_limit_profile_update_username, default_enabled: :yaml)
+  end
   skip_before_action :require_email, only: [:show, :update]
   before_action do
     push_frontend_feature_flag(:webauthn, default_enabled: :yaml)

config/feature_flags/development/rate_limit_profile_update_username.yml

+---
+name: rate_limit_profile_update_username
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77221
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349132
+milestone: '14.7'
+type: development
+group: group::optimize
+default_enabled: false

lib/gitlab/application_rate_limiter.rb

@@ -51,6 +51,7 @@ module Gitlab
           web_hook_calls:               { interval: 1.minute },
           users_get_by_id:              { threshold: 10, interval: 1.minute },
           profile_resend_email_confirmation:  { threshold: 5, interval: 1.minute },
+          profile_update_username:            { threshold: 10, interval: 1.minute },
           update_environment_canary_ingress:  { threshold: 1, interval: 1.minute },
           auto_rollback_deployment:           { threshold: 1, interval: 3.minutes },
           user_email_lookup:            { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute }

spec/controllers/profiles_controller_spec.rb

@@ -153,9 +153,12 @@ RSpec.describe ProfilesController, :request_store do
     let(:gitlab_shell) { Gitlab::Shell.new }
     let(:new_username) { generate(:username) }
 
-    it 'allows username change' do
+    before do
       sign_in(user)
+      allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false)
+    end
 
+    it 'allows username change' do
       put :update_username,
         params: { user: { username: new_username } }
 
@@ -166,8 +169,6 @@ RSpec.describe ProfilesController, :request_store do
     end
 
     it 'updates a username using JSON request' do
-      sign_in(user)
-
       put :update_username,
           params: {
             user: { username: new_username }
@@ -179,8 +180,6 @@ RSpec.describe ProfilesController, :request_store do
     end
 
     it 'renders an error message when the username was not updated' do
-      sign_in(user)
-
       put :update_username,
           params: {
             user: { username: 'invalid username.git' }
@@ -192,8 +191,6 @@ RSpec.describe ProfilesController, :request_store do
     end
 
     it 'raises a correct error when the username is missing' do
-      sign_in(user)
-
       expect { put :update_username, params: { user: { gandalf: 'you shall not pass' } } }
         .to raise_error(ActionController::ParameterMissing)
     end
@@ -202,8 +199,6 @@ RSpec.describe ProfilesController, :request_store do
       it 'moves dependent projects to new namespace' do
         project = create(:project_empty_repo, :legacy_storage, namespace: namespace)
 
-        sign_in(user)
-
         put :update_username,
           params: { user: { username: new_username } }
 
@@ -220,8 +215,6 @@ RSpec.describe ProfilesController, :request_store do
 
         before_disk_path = project.disk_path
 
-        sign_in(user)
-
         put :update_username,
           params: { user: { username: new_username } }
 
@@ -232,5 +225,18 @@ RSpec.describe ProfilesController, :request_store do
         expect(before_disk_path).to eq(project.disk_path)
       end
     end
+
+    context 'when the rate limit is reached' do
+      it 'does not update the username and returns status 429 Too Many Requests' do
+        expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:profile_update_username, scope: user).and_return(true)
+
+        expect do
+          put :update_username,
+            params: { user: { username: new_username } }
+        end.not_to change { user.reload.username }
+
+        expect(response).to have_gitlab_http_status(:too_many_requests)
+      end
+    end
   end
 end