FIX: Improve protection against problematic usernames (#8097)

app/controllers/users_controller.rb

@@ -141,6 +141,10 @@ class UsersController < ApplicationController
   def username
     params.require(:new_username)
 
+    if clashing_with_existing_route?(params[:new_username]) || User.reserved_username?(params[:new_username])
+      return render_json_error(I18n.t("login.reserved_username"))
+    end
+
     user = fetch_user_from_params
     guardian.ensure_can_edit_username!(user)
 
@@ -359,7 +363,7 @@ class UsersController < ApplicationController
       return fail_with("login.email_too_long")
     end
 
-    if User.reserved_username?(params[:username])
+    if clashing_with_existing_route?(params[:username]) || User.reserved_username?(params[:username])
       return fail_with("login.reserved_username")
     end
 
@@ -1355,4 +1359,18 @@ class UsersController < ApplicationController
     end
   end
 
+  def clashing_with_existing_route?(username)
+    normalized_username = User.normalize_username(username)
+    http_verbs = %w[GET POST PUT DELETE PATCH]
+    allowed_actions = %w[show update destroy]
+
+    http_verbs.any? do |verb|
+      begin
+        path = Rails.application.routes.recognize_path("/u/#{normalized_username}", method: verb)
+        allowed_actions.exclude?(path[:action])
+      rescue ActionController::RoutingError
+        false
+      end
+    end
+  end
 end

config/site_settings.yml

@@ -487,7 +487,7 @@ users:
   reserved_usernames:
     type: list
     list_type: compact
-    default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|hp|account-created|password-reset|admin-login|confirm-admin|account-created|activate-account|confirm-email-token|authorize-email"
+    default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|hp"
   min_password_length:
     client: true
     default: 10

spec/requests/users_controller_spec.rb

@@ -1005,11 +1005,16 @@ describe UsersController do
     end
 
     context 'with a reserved username' do
-      let(:create_params) { { name: @user.name, username: 'Reserved', email: @user.email, password: "x" * 20 } }
+      let(:create_params) { { name: @user.name, username: 'Reserved', email: @user.email, password: 'strongpassword' } }
       before { SiteSetting.reserved_usernames = 'a|reserved|b' }
       include_examples 'failed signup'
     end
 
+    context 'with a username that matches a user route' do
+      let(:create_params) { { name: @user.name, username: 'account-created', email: @user.email, password: 'strongpassword' } }
+      include_examples 'failed signup'
+    end
+
     context 'with a missing username' do
       let(:create_params) { { name: @user.name, email: @user.email, password: "x" * 20 } }
 
@@ -1197,6 +1202,23 @@ describe UsersController do
         expect(user.reload.username).to eq(new_username)
       end
 
+      it 'raises an error when the username clashes with an existing user route' do
+        put "/u/#{user.username}/preferences/username.json", params: { new_username: 'account-created' }
+
+        body = JSON.parse(response.body)
+
+        expect(body['errors'].first).to include(I18n.t('login.reserved_username'))
+      end
+
+      it 'raises an error when the username is in the reserved list' do
+        SiteSetting.reserved_usernames = 'reserved'
+
+        put "/u/#{user.username}/preferences/username.json", params: { new_username: 'reserved' }
+        body = JSON.parse(response.body)
+
+        expect(body['errors'].first).to include(I18n.t('login.reserved_username'))
+      end
+
       it 'should fail if the user is old' do
         # Older than the change period and >1 post
         user.created_at = Time.now - (SiteSetting.username_change_period + 1).days