Merge pull request #705 from wpp/refactor_users_controller
Refactor user controller, create action, mostly.
This commit is contained in:
commit
5b4fa68e8c
|
@ -140,83 +140,83 @@ class UsersController < ApplicationController
|
|||
|
||||
def create
|
||||
|
||||
if params[:password_confirmation] != honeypot_value || params[:challenge] != challenge_value.try(:reverse)
|
||||
if honeypot_or_challenge_fails?(params)
|
||||
# Don't give any indication that we caught you in the honeypot
|
||||
return render(json: {success: true, active: false, message: I18n.t("login.activate_email", email: params[:email]) })
|
||||
honey_pot_response = {
|
||||
success: true,
|
||||
active: false,
|
||||
message: I18n.t("login.activate_email", email: params[:email])
|
||||
}
|
||||
return render(json: honey_pot_response)
|
||||
end
|
||||
|
||||
user = User.new
|
||||
user.name = params[:name]
|
||||
user.email = params[:email]
|
||||
user.password = params[:password]
|
||||
user.username = params[:username]
|
||||
user = User.new_from_params(params)
|
||||
|
||||
auth = session[:authentication]
|
||||
if auth && auth[:email] == params[:email] && auth[:email_valid]
|
||||
if valid_session_authentication?(auth, params[:email])
|
||||
user.active = true
|
||||
end
|
||||
user.password_required! unless auth
|
||||
|
||||
DiscourseHub.register_nickname( user.username, user.email ) if user.valid? && SiteSetting.call_discourse_hub?
|
||||
if user.valid? && SiteSetting.call_discourse_hub?
|
||||
DiscourseHub.register_nickname(user.username, user.email)
|
||||
end
|
||||
|
||||
if user.save
|
||||
|
||||
msg = nil
|
||||
active_result = user.active?
|
||||
if active_result
|
||||
active_user = user.active?
|
||||
|
||||
if active_user
|
||||
# If the user is active (remote authorized email)
|
||||
if SiteSetting.must_approve_users?
|
||||
msg = I18n.t("login.wait_approval")
|
||||
active_result = false
|
||||
active_user = false
|
||||
else
|
||||
log_on_user(user)
|
||||
user.enqueue_welcome_message('welcome_user')
|
||||
msg = I18n.t("login.active")
|
||||
end
|
||||
|
||||
else
|
||||
msg = I18n.t("login.activate_email", email: user.email)
|
||||
Jobs.enqueue(:user_email, type: :signup, user_id: user.id, email_token: user.email_tokens.first.token)
|
||||
end
|
||||
|
||||
# Create auth records
|
||||
if auth.present?
|
||||
if auth[:twitter_user_id] && auth[:twitter_screen_name] && TwitterUserInfo.find_by_twitter_user_id(auth[:twitter_user_id]).nil?
|
||||
TwitterUserInfo.create(user_id: user.id, screen_name: auth[:twitter_screen_name], twitter_user_id: auth[:twitter_user_id])
|
||||
end
|
||||
|
||||
if auth[:facebook].present? && FacebookUserInfo.find_by_facebook_user_id(auth[:facebook][:facebook_user_id]).nil?
|
||||
FacebookUserInfo.create!(auth[:facebook].merge(user_id: user.id))
|
||||
end
|
||||
|
||||
if auth[:github_user_id] && auth[:github_screen_name] && GithubUserInfo.find_by_github_user_id(auth[:github_user_id]).nil?
|
||||
GithubUserInfo.create(user_id: user.id, screen_name: auth[:github_screen_name], github_user_id: auth[:github_user_id])
|
||||
end
|
||||
Jobs.enqueue(
|
||||
:user_email, type: :signup, user_id: user.id,
|
||||
email_token: user.email_tokens.first.token
|
||||
)
|
||||
end
|
||||
|
||||
# Create 3rd party auth records (Twitter, Facebook, GitHub)
|
||||
create_third_party_auth_records(user, auth) if auth.present?
|
||||
|
||||
# Clear authentication session.
|
||||
session[:authentication] = nil
|
||||
|
||||
# JSON result
|
||||
render json: {success: true, active: active_result, message: msg }
|
||||
render json: { success: true, active: active_user, message: msg }
|
||||
else
|
||||
render json: {success: false, message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n"))}
|
||||
render json: {
|
||||
success: false,
|
||||
message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n"))
|
||||
}
|
||||
end
|
||||
rescue ActiveRecord::StatementInvalid
|
||||
render json: {success: false, message: I18n.t("login.something_already_taken")}
|
||||
render json: { success: false, message: I18n.t("login.something_already_taken") }
|
||||
rescue DiscourseHub::NicknameUnavailable
|
||||
render json: {success: false, message: I18n.t("login.errors", errors:I18n.t("login.not_available", suggestion: User.suggest_username(params[:username])) )}
|
||||
render json: { success: false,
|
||||
message: I18n.t(
|
||||
"login.errors",
|
||||
errors:I18n.t(
|
||||
"login.not_available", suggestion: User.suggest_username(params[:username])
|
||||
)
|
||||
)
|
||||
}
|
||||
rescue RestClient::Forbidden
|
||||
render json: {errors: [I18n.t("discourse_hub.access_token_problem")]}
|
||||
render json: { errors: [I18n.t("discourse_hub.access_token_problem")] }
|
||||
end
|
||||
|
||||
def get_honeypot_value
|
||||
render json: {value: honeypot_value, challenge: challenge_value}
|
||||
end
|
||||
|
||||
|
||||
# all avatars are funneled through here
|
||||
def avatar
|
||||
|
||||
|
@ -224,13 +224,10 @@ class UsersController < ApplicationController
|
|||
# raise ActiveRecord::RecordNotFound
|
||||
|
||||
user = User.select(:email).where(username_lower: params[:username].downcase).first
|
||||
if user
|
||||
# for now we only support gravatar in square (redirect cached for a day), later we can use x-sendfile and/or a cdn to serve local
|
||||
size = params[:size].to_i
|
||||
size = 64 if size == 0
|
||||
size = 10 if size < 10
|
||||
size = 128 if size > 128
|
||||
|
||||
if user.present?
|
||||
# for now we only support gravatar in square (redirect cached for a day),
|
||||
# later we can use x-sendfile and/or a cdn to serve local
|
||||
size = determine_avatar_size(params[:size])
|
||||
url = user.avatar_template.gsub("{size}", size.to_s)
|
||||
expires_in 1.day
|
||||
redirect_to url
|
||||
|
@ -270,14 +267,18 @@ class UsersController < ApplicationController
|
|||
guardian.ensure_can_edit!(user)
|
||||
|
||||
# Raise an error if the email is already in use
|
||||
raise Discourse::InvalidParameters.new(:email) if User.where("lower(email) = ?", params[:email].downcase).exists?
|
||||
if User.where("lower(email) = ?", params[:email].downcase).exists?
|
||||
raise Discourse::InvalidParameters.new(:email)
|
||||
end
|
||||
|
||||
email_token = user.email_tokens.create(email: params[:email])
|
||||
Jobs.enqueue(:user_email,
|
||||
to_address: params[:email],
|
||||
type: :authorize_email,
|
||||
user_id: user.id,
|
||||
email_token: email_token.token)
|
||||
Jobs.enqueue(
|
||||
:user_email,
|
||||
to_address: params[:email],
|
||||
type: :authorize_email,
|
||||
user_id: user.id,
|
||||
email_token: email_token.token
|
||||
)
|
||||
|
||||
render nothing: true
|
||||
end
|
||||
|
@ -327,8 +328,8 @@ class UsersController < ApplicationController
|
|||
|
||||
results = UserSearch.search term, topic_id
|
||||
|
||||
render json: { users: results.as_json( only: [ :username, :name ],
|
||||
methods: :avatar_template ) }
|
||||
render json: { users: results.as_json(only: [ :username, :name ],
|
||||
methods: :avatar_template) }
|
||||
end
|
||||
|
||||
private
|
||||
|
@ -351,4 +352,58 @@ class UsersController < ApplicationController
|
|||
guardian.ensure_can_see!(user)
|
||||
user
|
||||
end
|
||||
|
||||
def honeypot_or_challenge_fails?(params)
|
||||
params[:password_confirmation] != honeypot_value ||
|
||||
params[:challenge] != challenge_value.try(:reverse)
|
||||
end
|
||||
|
||||
def valid_session_authentication?(auth, email)
|
||||
auth && auth[:email] == email && auth[:email_valid]
|
||||
end
|
||||
|
||||
def create_third_party_auth_records(user, auth)
|
||||
if twitter_auth?(auth)
|
||||
TwitterUserInfo.create(
|
||||
user_id: user.id,
|
||||
screen_name: auth[:twitter_screen_name],
|
||||
twitter_user_id: auth[:twitter_user_id]
|
||||
)
|
||||
end
|
||||
|
||||
if facebook_auth?(auth)
|
||||
FacebookUserInfo.create!(auth[:facebook].merge(user_id: user.id))
|
||||
end
|
||||
|
||||
if github_auth?(auth)
|
||||
GithubUserInfo.create(
|
||||
user_id: user.id,
|
||||
screen_name: auth[:github_screen_name],
|
||||
github_user_id: auth[:github_user_id]
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
def twitter_auth?(auth)
|
||||
auth[:twitter_user_id] && auth[:twitter_screen_name] &&
|
||||
TwitterUserInfo.find_by_twitter_user_id(auth[:twitter_user_id]).nil?
|
||||
end
|
||||
|
||||
def facebook_auth?(auth)
|
||||
auth[:facebook].present? &&
|
||||
FacebookUserInfo.find_by_facebook_user_id(auth[:facebook][:facebook_user_id]).nil?
|
||||
end
|
||||
|
||||
def github_auth?(auth)
|
||||
auth[:github_user_id] && auth[:github_screen_name] &&
|
||||
GithubUserInfo.find_by_github_user_id(auth[:github_user_id]).nil?
|
||||
end
|
||||
|
||||
def determine_avatar_size(size)
|
||||
size = size.to_i
|
||||
size = 64 if size == 0
|
||||
size = 10 if size < 10
|
||||
size = 128 if size > 128
|
||||
size
|
||||
end
|
||||
end
|
||||
|
|
|
@ -102,6 +102,15 @@ class User < ActiveRecord::Base
|
|||
find_available_username_based_on(name)
|
||||
end
|
||||
|
||||
def self.new_from_params(params)
|
||||
user = User.new
|
||||
user.name = params[:name]
|
||||
user.email = params[:email]
|
||||
user.password = params[:password]
|
||||
user.username = params[:username]
|
||||
user
|
||||
end
|
||||
|
||||
def self.create_for_email(email, opts={})
|
||||
username = suggest_username(email)
|
||||
|
||||
|
|
|
@ -277,12 +277,14 @@ describe UsersController do
|
|||
context 'when creating a non active user (unconfirmed email)' do
|
||||
it 'should enqueue a signup email' do
|
||||
Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup))
|
||||
xhr :post, :create, name: @user.name, username: @user.username, password: "strongpassword", email: @user.email
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
|
||||
it "doesn't send a welcome email" do
|
||||
User.any_instance.expects(:enqueue_welcome_message).with('welcome_user').never
|
||||
xhr :post, :create, name: @user.name, username: @user.username, password: "strongpassword", email: @user.email
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -294,7 +296,8 @@ describe UsersController do
|
|||
|
||||
it 'should enqueue a signup email' do
|
||||
User.any_instance.expects(:enqueue_welcome_message).with('welcome_user')
|
||||
xhr :post, :create, name: @user.name, username: @user.username, password: "strongpassword", email: @user.email
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
|
||||
it "should be logged in" do
|
||||
|
@ -309,6 +312,7 @@ describe UsersController do
|
|||
::JSON.parse(response.body)['active'].should == true
|
||||
end
|
||||
|
||||
|
||||
context 'when approving of users is required' do
|
||||
before do
|
||||
SiteSetting.expects(:must_approve_users).returns(true)
|
||||
|
@ -322,14 +326,51 @@ describe UsersController do
|
|||
it "doesn't return active in the JSON" do
|
||||
::JSON.parse(response.body)['active'].should == false
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
context 'authentication records for' do
|
||||
|
||||
before do
|
||||
SiteSetting.expects(:must_approve_users).returns(true)
|
||||
end
|
||||
|
||||
it 'should create twitter user info if none exists' do
|
||||
twitter_auth = { twitter_user_id: 42, twitter_screen_name: "bruce" }
|
||||
session[:authentication] = twitter_auth
|
||||
TwitterUserInfo.expects(:find_by_twitter_user_id).returns(nil)
|
||||
TwitterUserInfo.expects(:create)
|
||||
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
|
||||
it 'should create facebook user info if none exists' do
|
||||
fb_auth = { facebook: { facebook_user_id: 42} }
|
||||
session[:authentication] = fb_auth
|
||||
FacebookUserInfo.expects(:find_by_facebook_user_id).returns(nil)
|
||||
FacebookUserInfo.expects(:create!)
|
||||
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
|
||||
it 'should create github user info if none exists' do
|
||||
gh_auth = { github_user_id: 2, github_screen_name: "bruce" }
|
||||
session[:authentication] = gh_auth
|
||||
GithubUserInfo.expects(:find_by_github_user_id).returns(nil)
|
||||
GithubUserInfo.expects(:create)
|
||||
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
|
||||
context 'after success' do
|
||||
before do
|
||||
xhr :post, :create, name: @user.name, username: @user.username, password: "strongpassword", email: @user.email
|
||||
xhr :post, :create, name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email
|
||||
end
|
||||
|
||||
it 'should succeed' do
|
||||
|
@ -403,13 +444,22 @@ describe UsersController do
|
|||
it_should_behave_like 'failed signup'
|
||||
end
|
||||
|
||||
context 'when InvalidStatement is raised' do
|
||||
before do
|
||||
User.any_instance.stubs(:save).raises(ActiveRecord::StatementInvalid)
|
||||
context 'when an Exception is raised' do
|
||||
|
||||
[ ActiveRecord::StatementInvalid,
|
||||
DiscourseHub::NicknameUnavailable,
|
||||
RestClient::Forbidden ].each do |exception|
|
||||
before { User.any_instance.stubs(:save).raises(exception) }
|
||||
|
||||
let(:create_params) {
|
||||
{ name: @user.name, username: @user.username,
|
||||
password: "strongpassword", email: @user.email}
|
||||
}
|
||||
|
||||
it_should_behave_like 'failed signup'
|
||||
end
|
||||
let(:create_params) { {name: @user.name, username: @user.username, password: "strongpassword", email: @user.email} }
|
||||
it_should_behave_like 'failed signup'
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
context '.username' do
|
||||
|
@ -704,7 +754,9 @@ describe UsersController do
|
|||
|
||||
context 'not logged in' do
|
||||
it 'raises an error when not logged in' do
|
||||
lambda { xhr :put, :update, username: 'somename' }.should raise_error(Discourse::NotLoggedIn)
|
||||
expect do
|
||||
xhr :put, :update, username: 'somename'
|
||||
end.to raise_error(Discourse::NotLoggedIn)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -735,7 +787,6 @@ describe UsersController do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
describe "search_users" do
|
||||
|
|
Loading…
Reference in New Issue