mirror of
https://github.com/discourse/discourse.git
synced 2025-02-25 18:55:32 -06:00
better consistency around email case sensitivity
This commit is contained in:
parent
c6ab9fec9d
commit
b24c1a1ad9
@ -18,7 +18,7 @@ Discourse.ForgotPasswordView = Discourse.ModalBodyView.extend({
|
|||||||
submit: function() {
|
submit: function() {
|
||||||
|
|
||||||
Discourse.ajax(Discourse.getURL("/session/forgot_password"), {
|
Discourse.ajax(Discourse.getURL("/session/forgot_password"), {
|
||||||
data: { username: this.get('accountEmailOrUsername') },
|
data: { login: this.get('accountEmailOrUsername') },
|
||||||
type: 'POST'
|
type: 'POST'
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -3,13 +3,13 @@ class SessionController < ApplicationController
|
|||||||
def create
|
def create
|
||||||
requires_parameter(:login, :password)
|
requires_parameter(:login, :password)
|
||||||
|
|
||||||
login = params[:login].downcase
|
login = params[:login]
|
||||||
login = login[1..-1] if login[0] == "@"
|
login = login[1..-1] if login[0] == "@"
|
||||||
|
|
||||||
if login =~ /@/
|
if login =~ /@/
|
||||||
@user = User.where(email: login).first
|
@user = User.where(email: Email.downcase(login)).first
|
||||||
else
|
else
|
||||||
@user = User.where(username_lower: login).first
|
@user = User.where(username_lower: login.downcase).first
|
||||||
end
|
end
|
||||||
|
|
||||||
if @user.present?
|
if @user.present?
|
||||||
@ -37,9 +37,9 @@ class SessionController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def forgot_password
|
def forgot_password
|
||||||
requires_parameter(:username)
|
requires_parameter(:login)
|
||||||
|
|
||||||
user = User.where('username_lower = :username or email = :username', username: params[:username].downcase).first
|
user = User.where('username_lower = :username or email = :email', username: params[:login].downcase, email: Email.downcase(params[:login])).first
|
||||||
if user.present?
|
if user.present?
|
||||||
email_token = user.email_tokens.create(email: user.email)
|
email_token = user.email_tokens.create(email: user.email)
|
||||||
Jobs.enqueue(:user_email, type: :forgot_password, user_id: user.id, email_token: email_token.token)
|
Jobs.enqueue(:user_email, type: :forgot_password, user_id: user.id, email_token: email_token.token)
|
||||||
|
@ -265,16 +265,17 @@ class UsersController < ApplicationController
|
|||||||
requires_parameter(:email)
|
requires_parameter(:email)
|
||||||
user = fetch_user_from_params
|
user = fetch_user_from_params
|
||||||
guardian.ensure_can_edit!(user)
|
guardian.ensure_can_edit!(user)
|
||||||
|
lower_email = Email.downcase(params[:email])
|
||||||
|
|
||||||
# Raise an error if the email is already in use
|
# Raise an error if the email is already in use
|
||||||
if User.where("lower(email) = ?", params[:email].downcase).exists?
|
if User.where("email = ?", lower_email).exists?
|
||||||
raise Discourse::InvalidParameters.new(:email)
|
raise Discourse::InvalidParameters.new(:email)
|
||||||
end
|
end
|
||||||
|
|
||||||
email_token = user.email_tokens.create(email: params[:email])
|
email_token = user.email_tokens.create(email: lower_email)
|
||||||
Jobs.enqueue(
|
Jobs.enqueue(
|
||||||
:user_email,
|
:user_email,
|
||||||
to_address: params[:email],
|
to_address: lower_email,
|
||||||
type: :authorize_email,
|
type: :authorize_email,
|
||||||
user_id: user.id,
|
user_id: user.id,
|
||||||
email_token: email_token.token
|
email_token: email_token.token
|
||||||
|
@ -11,13 +11,12 @@ class Invite < ActiveRecord::Base
|
|||||||
|
|
||||||
acts_as_paranoid
|
acts_as_paranoid
|
||||||
|
|
||||||
|
|
||||||
before_create do
|
before_create do
|
||||||
self.invite_key ||= SecureRandom.hex
|
self.invite_key ||= SecureRandom.hex
|
||||||
end
|
end
|
||||||
|
|
||||||
before_save do
|
before_save do
|
||||||
self.email = email.downcase
|
self.email = Email.downcase(email)
|
||||||
end
|
end
|
||||||
|
|
||||||
validate :user_doesnt_already_exist
|
validate :user_doesnt_already_exist
|
||||||
@ -26,7 +25,7 @@ class Invite < ActiveRecord::Base
|
|||||||
def user_doesnt_already_exist
|
def user_doesnt_already_exist
|
||||||
@email_already_exists = false
|
@email_already_exists = false
|
||||||
return if email.blank?
|
return if email.blank?
|
||||||
if User.where("lower(email) = ?", email.downcase).exists?
|
if User.where("email = ?", Email.downcase(email)).exists?
|
||||||
@email_already_exists = true
|
@email_already_exists = true
|
||||||
errors.add(:email)
|
errors.add(:email)
|
||||||
end
|
end
|
||||||
|
@ -398,11 +398,9 @@ class Topic < ActiveRecord::Base
|
|||||||
# Invite a user by email and return the invite. Return the previously existing invite
|
# Invite a user by email and return the invite. Return the previously existing invite
|
||||||
# if already exists. Returns nil if the invite can't be created.
|
# if already exists. Returns nil if the invite can't be created.
|
||||||
def invite_by_email(invited_by, email)
|
def invite_by_email(invited_by, email)
|
||||||
lower_email = email.downcase
|
lower_email = Email.downcase(email)
|
||||||
|
|
||||||
invite = Invite.with_deleted.where('invited_by_id = ? and email = ?', invited_by.id, lower_email).first
|
invite = Invite.with_deleted.where('invited_by_id = ? and email = ?', invited_by.id, lower_email).first
|
||||||
|
|
||||||
|
|
||||||
if invite.blank?
|
if invite.blank?
|
||||||
invite = Invite.create(invited_by: invited_by, email: lower_email)
|
invite = Invite.create(invited_by: invited_by, email: lower_email)
|
||||||
unless invite.valid?
|
unless invite.valid?
|
||||||
|
@ -193,7 +193,9 @@ class User < ActiveRecord::Base
|
|||||||
end
|
end
|
||||||
|
|
||||||
def self.find_by_username_or_email(username_or_email)
|
def self.find_by_username_or_email(username_or_email)
|
||||||
where("username_lower = :user or lower(username) = :user or lower(email) = :user or lower(name) = :user", user: username_or_email.downcase)
|
lower_user = username_or_email.downcase
|
||||||
|
lower_email = Email.downcase(username_or_email)
|
||||||
|
where("username_lower = :user or lower(username) = :user or email = :email or lower(name) = :user", user: lower_user, email: lower_email)
|
||||||
end
|
end
|
||||||
|
|
||||||
# tricky, we need our bus to be subscribed from the right spot
|
# tricky, we need our bus to be subscribed from the right spot
|
||||||
|
@ -1,8 +1,8 @@
|
|||||||
require 'mail'
|
require 'mail'
|
||||||
|
|
||||||
module Email
|
module Email
|
||||||
|
|
||||||
def self.is_valid?(email)
|
def self.is_valid?(email)
|
||||||
|
|
||||||
parser = Mail::RFC2822Parser.new
|
parser = Mail::RFC2822Parser.new
|
||||||
parser.root = :addr_spec
|
parser.root = :addr_spec
|
||||||
result = parser.parse(email)
|
result = parser.parse(email)
|
||||||
@ -12,4 +12,9 @@ module Email
|
|||||||
result && result.respond_to?(:domain) && result.domain.dot_atom_text.elements.size > 1
|
result && result.respond_to?(:domain) && result.domain.dot_atom_text.elements.size > 1
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def self.downcase(email)
|
||||||
|
return email unless Email.is_valid?(email)
|
||||||
|
email.gsub(/^(.+@{1})([^@]+)$/) { $1 + $2.downcase }
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
@ -3,20 +3,39 @@ require 'email'
|
|||||||
|
|
||||||
describe Email do
|
describe Email do
|
||||||
|
|
||||||
|
describe "is_valid?" do
|
||||||
|
|
||||||
|
it 'treats a good email as valid' do
|
||||||
|
Email.is_valid?('sam@sam.com').should be_true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'treats a bad email as invalid' do
|
||||||
|
Email.is_valid?('sam@sam').should be_false
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'allows museum tld' do
|
||||||
|
Email.is_valid?('sam@nic.museum').should be_true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not think a word is an email' do
|
||||||
|
Email.is_valid?('sam').should be_false
|
||||||
|
end
|
||||||
|
|
||||||
it 'should treat a good email as valid' do
|
|
||||||
Email.is_valid?('sam@sam.com').should be_true
|
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'should treat a bad email as invalid' do
|
describe "downcase" do
|
||||||
Email.is_valid?('sam@sam').should be_false
|
|
||||||
|
it 'downcases only the host part' do
|
||||||
|
Email.downcase('SAM@GMAIL.COM').should == 'SAM@gmail.com'
|
||||||
|
Email.downcase('sam@GMAIL.COM').should_not == 'SAM@gmail.com'
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'leaves invalid emails untouched' do
|
||||||
|
Email.downcase('SAM@GMAILCOM').should == 'SAM@GMAILCOM'
|
||||||
|
Email.downcase('samGMAIL.COM').should == 'samGMAIL.COM'
|
||||||
|
Email.downcase('sam@GM@AIL.COM').should == 'sam@GM@AIL.COM'
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'should allow museum tld' do
|
|
||||||
Email.is_valid?('sam@nic.museum').should be_true
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'should not think a word is an email' do
|
|
||||||
Email.is_valid?('sam').should be_false
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
@ -119,12 +119,12 @@ describe SessionController do
|
|||||||
|
|
||||||
context 'for a non existant username' do
|
context 'for a non existant username' do
|
||||||
it "doesn't generate a new token for a made up username" do
|
it "doesn't generate a new token for a made up username" do
|
||||||
lambda { xhr :post, :forgot_password, username: 'made_up'}.should_not change(EmailToken, :count)
|
lambda { xhr :post, :forgot_password, login: 'made_up'}.should_not change(EmailToken, :count)
|
||||||
end
|
end
|
||||||
|
|
||||||
it "doesn't enqueue an email" do
|
it "doesn't enqueue an email" do
|
||||||
Jobs.expects(:enqueue).with(:user_mail, anything).never
|
Jobs.expects(:enqueue).with(:user_mail, anything).never
|
||||||
xhr :post, :forgot_password, username: 'made_up'
|
xhr :post, :forgot_password, login: 'made_up'
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -132,12 +132,12 @@ describe SessionController do
|
|||||||
let(:user) { Fabricate(:user) }
|
let(:user) { Fabricate(:user) }
|
||||||
|
|
||||||
it "generates a new token for a made up username" do
|
it "generates a new token for a made up username" do
|
||||||
lambda { xhr :post, :forgot_password, username: user.username}.should change(EmailToken, :count)
|
lambda { xhr :post, :forgot_password, login: user.username}.should change(EmailToken, :count)
|
||||||
end
|
end
|
||||||
|
|
||||||
it "enqueues an email" do
|
it "enqueues an email" do
|
||||||
Jobs.expects(:enqueue).with(:user_email, has_entries(type: :forgot_password, user_id: user.id))
|
Jobs.expects(:enqueue).with(:user_email, has_entries(type: :forgot_password, user_id: user.id))
|
||||||
xhr :post, :forgot_password, username: user.username
|
xhr :post, :forgot_password, login: user.username
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -36,7 +36,6 @@ describe Invite do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
||||||
context 'to a topic' do
|
context 'to a topic' do
|
||||||
let!(:topic) { Fabricate(:topic) }
|
let!(:topic) { Fabricate(:topic) }
|
||||||
let(:inviter) { topic.user }
|
let(:inviter) { topic.user }
|
||||||
@ -97,8 +96,12 @@ describe Invite do
|
|||||||
topic.invite_by_email(inviter, 'iceking@adventuretime.ooo').should == @invite
|
topic.invite_by_email(inviter, 'iceking@adventuretime.ooo').should == @invite
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'matches case insensitively' do
|
it 'matches case insensitively for the domain part' do
|
||||||
topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo').should == @invite
|
topic.invite_by_email(inviter, 'iceking@ADVENTURETIME.ooo').should == @invite
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'matches case sensitively for the local part' do
|
||||||
|
topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo').should_not == @invite
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user