diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index da400520d2c..7a6225a15ac 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/config/site_settings.yml b/config/site_settings.yml index 231e25d170a..3b090200a27 100644 --- a/config/site_settings.yml +++ b/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 diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index c4b7d75ae96..ae5e2795bf7 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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