FIX: move hp request from /users to /token (#10795)

`hp` is a valid username and we should not prevent users from registering it.
This commit is contained in:
Krzysztof Kotlarek 2020-10-02 09:01:40 +10:00 committed by GitHub
parent 29f7e0689f
commit 5cf411c3ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 32 additions and 35 deletions

View File

@ -229,7 +229,7 @@ export default Controller.extend(
return this._hpPromise; return this._hpPromise;
} }
this._hpPromise = ajax(userPath("hp.json")) this._hpPromise = ajax("/session/hp.json")
.then((json) => { .then((json) => {
this._challengeDate = new Date(); this._challengeDate = new Date();
// remove 30 seconds for jitter, make sure this works for at least // remove 30 seconds for jitter, make sure this works for at least

View File

@ -47,6 +47,9 @@ class ApplicationController < ActionController::Base
after_action :dont_cache_page after_action :dont_cache_page
after_action :conditionally_allow_site_embedding after_action :conditionally_allow_site_embedding
HONEYPOT_KEY ||= 'HONEYPOT_KEY'
CHALLENGE_KEY ||= 'CHALLENGE_KEY'
layout :set_layout layout :set_layout
def has_escaped_fragment? def has_escaped_fragment?
@ -833,6 +836,14 @@ class ApplicationController < ActionController::Base
protected protected
def honeypot_value
secure_session[HONEYPOT_KEY] ||= SecureRandom.hex
end
def challenge_value
secure_session[CHALLENGE_KEY] ||= SecureRandom.hex
end
def render_post_json(post, add_raw: true) def render_post_json(post, add_raw: true)
post_serializer = PostSerializer.new(post, scope: guardian, root: false) post_serializer = PostSerializer.new(post, scope: guardian, root: false)
post_serializer.add_raw = add_raw post_serializer.add_raw = add_raw

View File

@ -451,6 +451,17 @@ class SessionController < ApplicationController
end end
end end
def get_honeypot_value
secure_session.set(HONEYPOT_KEY, honeypot_value, expires: 1.hour)
secure_session.set(CHALLENGE_KEY, challenge_value, expires: 1.hour)
render json: {
value: honeypot_value,
challenge: challenge_value,
expires_in: SecureSession.expiry
}
end
protected protected
def check_local_login_allowed(user: nil, check_login_via_email: false) def check_local_login_allowed(user: nil, check_login_via_email: false)

View File

@ -35,7 +35,6 @@ class UsersController < ApplicationController
skip_before_action :verify_authenticity_token, only: [:create] skip_before_action :verify_authenticity_token, only: [:create]
skip_before_action :redirect_to_login_if_required, only: [:check_username, skip_before_action :redirect_to_login_if_required, only: [:check_username,
:create, :create,
:get_honeypot_value,
:account_created, :account_created,
:activate_account, :activate_account,
:perform_account_activation, :perform_account_activation,
@ -643,17 +642,6 @@ class UsersController < ApplicationController
} }
end end
def get_honeypot_value
secure_session.set(HONEYPOT_KEY, honeypot_value, expires: 1.hour)
secure_session.set(CHALLENGE_KEY, challenge_value, expires: 1.hour)
render json: {
value: honeypot_value,
challenge: challenge_value,
expires_in: SecureSession.expiry
}
end
def password_reset_show def password_reset_show
expires_now expires_now
token = params[:token] token = params[:token]
@ -1522,19 +1510,6 @@ class UsersController < ApplicationController
end end
end end
HONEYPOT_KEY ||= 'HONEYPOT_KEY'
CHALLENGE_KEY ||= 'CHALLENGE_KEY'
protected
def honeypot_value
secure_session[HONEYPOT_KEY] ||= SecureRandom.hex
end
def challenge_value
secure_session[CHALLENGE_KEY] ||= SecureRandom.hex
end
private private
def password_reset_find_user(token, committing_change:) def password_reset_find_user(token, committing_change:)

View File

@ -13,7 +13,7 @@
<%= preload_script "ember_jquery" %> <%= preload_script "ember_jquery" %>
<%= preload_script "vendor" %> <%= preload_script "vendor" %>
<%= render_google_universal_analytics_code %> <%= render_google_universal_analytics_code %>
<%= tag.meta id: 'data-activate-account', data: { path: path('/u/hp') } %> <%= tag.meta id: 'data-activate-account', data: { path: path('/session/hp') } %>
<%- end %> <%- end %>
<%= preload_script "activate-account" %> <%= preload_script "activate-account" %>

View File

@ -358,6 +358,7 @@ Discourse::Application.routes.draw do
get "session/sso_provider" => "session#sso_provider" get "session/sso_provider" => "session#sso_provider"
get "session/current" => "session#current" get "session/current" => "session#current"
get "session/csrf" => "session#csrf" get "session/csrf" => "session#csrf"
get "session/hp" => "session#get_honeypot_value"
get "session/email-login/:token" => "session#email_login_info" get "session/email-login/:token" => "session#email_login_info"
post "session/email-login/:token" => "session#email_login" post "session/email-login/:token" => "session#email_login"
get "session/otp/:token" => "session#one_time_password", constraints: { token: /[0-9a-f]+/ } get "session/otp/:token" => "session#one_time_password", constraints: { token: /[0-9a-f]+/ }
@ -406,7 +407,6 @@ Discourse::Application.routes.draw do
put "#{root_path}/second_factors_backup" => "users#create_second_factor_backup" put "#{root_path}/second_factors_backup" => "users#create_second_factor_backup"
put "#{root_path}/update-activation-email" => "users#update_activation_email" put "#{root_path}/update-activation-email" => "users#update_activation_email"
get "#{root_path}/hp" => "users#get_honeypot_value"
post "#{root_path}/email-login" => "users#email_login" post "#{root_path}/email-login" => "users#email_login"
get "#{root_path}/admin-login" => "users#admin_login" get "#{root_path}/admin-login" => "users#admin_login"
put "#{root_path}/admin-login" => "users#admin_login" put "#{root_path}/admin-login" => "users#admin_login"

View File

@ -522,7 +522,7 @@ users:
reserved_usernames: reserved_usernames:
type: list type: list
list_type: compact list_type: compact
default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|hp" default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support"
min_password_length: min_password_length:
client: true client: true
default: 10 default: 10

View File

@ -9,7 +9,7 @@ describe UsersController do
describe "#full account registration flow" do describe "#full account registration flow" do
it "will correctly handle honeypot and challenge" do it "will correctly handle honeypot and challenge" do
get '/u/hp.json' get '/session/hp.json'
expect(response.status).to eq(200) expect(response.status).to eq(200)
json = response.parsed_body json = response.parsed_body
@ -584,7 +584,7 @@ describe UsersController do
describe '#create' do describe '#create' do
def honeypot_magic(params) def honeypot_magic(params)
get '/u/hp.json' get '/session/hp.json'
json = response.parsed_body json = response.parsed_body
params[:password_confirmation] = json["value"] params[:password_confirmation] = json["value"]
params[:challenge] = json["challenge"].reverse params[:challenge] = json["challenge"].reverse
@ -1297,6 +1297,8 @@ describe UsersController do
before do before do
UsersController.any_instance.stubs(:honeypot_value).returns("abc") UsersController.any_instance.stubs(:honeypot_value).returns("abc")
UsersController.any_instance.stubs(:challenge_value).returns("efg") UsersController.any_instance.stubs(:challenge_value).returns("efg")
SessionController.any_instance.stubs(:honeypot_value).returns("abc")
SessionController.any_instance.stubs(:challenge_value).returns("efg")
end end
let!(:staged) { Fabricate(:staged, email: "staged@account.com", active: true) } let!(:staged) { Fabricate(:staged, email: "staged@account.com", active: true) }

View File

@ -2,7 +2,7 @@
module IntegrationHelpers module IntegrationHelpers
def create_user def create_user
get "/u/hp.json" get "/session/hp.json"
expect(response.status).to eq(200) expect(response.status).to eq(200)

View File

@ -416,7 +416,7 @@ export function applyDefaultHandlers(pretender) {
pretender.post("/u/action/send_activation_email", success); pretender.post("/u/action/send_activation_email", success);
pretender.put("/u/update-activation-email", success); pretender.put("/u/update-activation-email", success);
pretender.get("/u/hp.json", function () { pretender.get("/session/hp.json", function () {
return response({ return response({
value: "32faff1b1ef1ac3", value: "32faff1b1ef1ac3",
challenge: "61a3de0ccf086fb9604b76e884d75801", challenge: "61a3de0ccf086fb9604b76e884d75801",

View File

@ -59,14 +59,12 @@ QUnit.test("isInternal on subfolder install", (assert) => {
QUnit.test("userPath", (assert) => { QUnit.test("userPath", (assert) => {
assert.equal(userPath(), "/u"); assert.equal(userPath(), "/u");
assert.equal(userPath("eviltrout"), "/u/eviltrout"); assert.equal(userPath("eviltrout"), "/u/eviltrout");
assert.equal(userPath("hp.json"), "/u/hp.json");
}); });
QUnit.test("userPath with prefix", (assert) => { QUnit.test("userPath with prefix", (assert) => {
setPrefix("/forum"); setPrefix("/forum");
assert.equal(userPath(), "/forum/u"); assert.equal(userPath(), "/forum/u");
assert.equal(userPath("eviltrout"), "/forum/u/eviltrout"); assert.equal(userPath("eviltrout"), "/forum/u/eviltrout");
assert.equal(userPath("hp.json"), "/forum/u/hp.json");
}); });
QUnit.test("routeTo with prefix", async (assert) => { QUnit.test("routeTo with prefix", async (assert) => {