FEATURE: Auto-activate users invited by email (#12675)

When invited by email, users will receive an invite URL which contains
a token. If that token is present when the invite is redeemed, their
account will be automatically activated.
This commit is contained in:
Dan Ungureanu 2021-04-14 12:15:56 +03:00 committed by GitHub
parent e4e2c7c66f
commit 528cfea079
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 105 additions and 29 deletions

View File

@ -22,6 +22,8 @@ export default Controller.extend(
NameValidation, NameValidation,
UserFieldsValidation, UserFieldsValidation,
{ {
queryParams: ["t"],
createAccount: controller(), createAccount: controller(),
invitedBy: readOnly("model.invited_by"), invitedBy: readOnly("model.invited_by"),
@ -216,6 +218,8 @@ export default Controller.extend(
if (this.isInviteLink) { if (this.isInviteLink) {
data.email = this.email; data.email = this.email;
} else {
data.email_token = this.t;
} }
ajax({ ajax({

View File

@ -197,7 +197,7 @@ class InvitesController < ApplicationController
# via the SessionController#sso_login route # via the SessionController#sso_login route
def perform_accept_invitation def perform_accept_invitation
params.require(:id) params.require(:id)
params.permit(:email, :username, :name, :password, :timezone, user_custom_fields: {}) params.permit(:email, :username, :name, :password, :timezone, :email_token, user_custom_fields: {})
invite = Invite.find_by(invite_key: params[:id]) invite = Invite.find_by(invite_key: params[:id])
@ -212,13 +212,13 @@ class InvitesController < ApplicationController
session: session session: session
} }
attrs[:email] = if invite.is_invite_link?
if invite.is_invite_link? params.require(:email)
params.require([:email]) attrs[:email] = params[:email]
params[:email] else
else attrs[:email] = invite.email
invite.email attrs[:email_token] = params[:email_token] if params[:email_token].present?
end end
user = invite.redeem(**attrs) user = invite.redeem(**attrs)
rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e

View File

@ -36,7 +36,7 @@ class InviteMailer < ActionMailer::Base
template: sanitized_message ? 'custom_invite_mailer' : 'invite_mailer', template: sanitized_message ? 'custom_invite_mailer' : 'invite_mailer',
inviter_name: inviter_name, inviter_name: inviter_name,
site_domain_name: Discourse.current_hostname, site_domain_name: Discourse.current_hostname,
invite_link: "#{Discourse.base_url}/invites/#{invite.invite_key}", invite_link: invite.link(with_email_token: true),
topic_title: topic_title, topic_title: topic_title,
topic_excerpt: topic_excerpt, topic_excerpt: topic_excerpt,
site_description: SiteSetting.site_description, site_description: SiteSetting.site_description,
@ -47,7 +47,7 @@ class InviteMailer < ActionMailer::Base
template: sanitized_message ? 'custom_invite_forum_mailer' : 'invite_forum_mailer', template: sanitized_message ? 'custom_invite_forum_mailer' : 'invite_forum_mailer',
inviter_name: inviter_name, inviter_name: inviter_name,
site_domain_name: Discourse.current_hostname, site_domain_name: Discourse.current_hostname,
invite_link: "#{Discourse.base_url}/invites/#{invite.invite_key}", invite_link: invite.link(with_email_token: true),
site_description: SiteSetting.site_description, site_description: SiteSetting.site_description,
site_title: SiteSetting.title, site_title: SiteSetting.title,
user_custom_message: sanitized_message) user_custom_message: sanitized_message)

View File

@ -39,6 +39,12 @@ class Invite < ActiveRecord::Base
self.expires_at ||= SiteSetting.invite_expiry_days.days.from_now self.expires_at ||= SiteSetting.invite_expiry_days.days.from_now
end end
before_save do
if will_save_change_to_email?
self.email_token = email.present? ? SecureRandom.hex : nil
end
end
before_validation do before_validation do
self.email = Email.downcase(email) unless email.nil? self.email = Email.downcase(email) unless email.nil?
end end
@ -85,8 +91,9 @@ class Invite < ActiveRecord::Base
expires_at < Time.zone.now expires_at < Time.zone.now
end end
def link def link(with_email_token: false)
"#{Discourse.base_url}/invites/#{invite_key}" with_email_token ? "#{Discourse.base_url}/invites/#{invite_key}?t=#{email_token}"
: "#{Discourse.base_url}/invites/#{invite_key}"
end end
def link_valid? def link_valid?
@ -167,7 +174,7 @@ class Invite < ActiveRecord::Base
invite.reload invite.reload
end end
def redeem(email: nil, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil, session: nil) def redeem(email: nil, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil, session: nil, email_token: nil)
return if !redeemable? return if !redeemable?
if is_invite_link? && UserEmail.exists?(email: email) if is_invite_link? && UserEmail.exists?(email: email)
@ -183,7 +190,8 @@ class Invite < ActiveRecord::Base
password: password, password: password,
user_custom_fields: user_custom_fields, user_custom_fields: user_custom_fields,
ip_address: ip_address, ip_address: ip_address,
session: session session: session,
email_token: email_token
).redeem ).redeem
end end

View File

@ -1,6 +1,6 @@
# frozen_string_literal: true # frozen_string_literal: true
InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_custom_fields, :ip_address, :session, keyword_init: true) do InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_custom_fields, :ip_address, :session, :email_token, keyword_init: true) do
def redeem def redeem
Invite.transaction do Invite.transaction do
@ -14,7 +14,7 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_
end end
# extracted from User cause it is very specific to invites # extracted from User cause it is very specific to invites
def self.create_user_from_invite(email:, invite:, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil, session: nil) def self.create_user_from_invite(email:, invite:, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil, session: nil, email_token: nil)
user = User.where(staged: true).with_email(email.strip.downcase).first user = User.where(staged: true).with_email(email.strip.downcase).first
user.unstage! if user user.unstage! if user
@ -76,7 +76,7 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_
user.save! user.save!
authenticator.finish authenticator.finish
if invite.emailed_status != Invite.emailed_status_types[:not_required] && email == invite.email if invite.emailed_status != Invite.emailed_status_types[:not_required] && email == invite.email && invite.email_token.present? && email_token == invite.email_token
user.email_tokens.create!(email: user.email) user.email_tokens.create!(email: user.email)
user.activate user.activate
end end
@ -131,7 +131,8 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_
password: password, password: password,
user_custom_fields: user_custom_fields, user_custom_fields: user_custom_fields,
ip_address: ip_address, ip_address: ip_address,
session: session session: session,
email_token: email_token
) )
result.send_welcome_message = false result.send_welcome_message = false
result result

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddTokenToInvites < ActiveRecord::Migration[6.0]
def change
add_column :invites, :email_token, :string
end
end

View File

@ -29,16 +29,15 @@ describe "Discobot welcome post" do
end end
context 'when user redeems an invite' do context 'when user redeems an invite' do
let(:invite) { Fabricate(:invite, invited_by: Fabricate(:admin), email: 'testing@gmail.com') } let!(:invite) { Fabricate(:invite, invited_by: Fabricate(:admin), email: 'testing@gmail.com') }
it 'should delay the welcome post until the user logs in' do it 'should delay the welcome post until the user logs in' do
invite
expect do expect do
put "/invites/show/#{invite.invite_key}.json", params: { put "/invites/show/#{invite.invite_key}.json", params: {
username: 'somename', username: 'somename',
name: 'testing', name: 'testing',
password: 'asodaasdaosdhq' password: 'verystrongpassword',
email_token: invite.email_token
} }
end.to change { User.count }.by(1) end.to change { User.count }.by(1)

View File

@ -10,9 +10,9 @@ describe InviteRedeemer do
user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White')
expect(user.username).to eq('walter') expect(user.username).to eq('walter')
expect(user.name).to eq('Walter White') expect(user.name).to eq('Walter White')
expect(user).to be_active
expect(user.email).to eq('walter.white@email.com') expect(user.email).to eq('walter.white@email.com')
expect(user.approved).to eq(true) expect(user.approved).to eq(true)
expect(user.active).to eq(false)
end end
it "can set the password and ip_address" do it "can set the password and ip_address" do
@ -52,7 +52,30 @@ describe InviteRedeemer do
expect(user.approved).to eq(true) expect(user.approved).to eq(true)
end end
it "should not activate user invited via links" do it "activates user invited via email with a token" do
invite = Fabricate(:invite, invited_by: Fabricate(:admin), email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:sent])
user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White', email_token: invite.email_token)
expect(user.username).to eq('walter')
expect(user.name).to eq('Walter White')
expect(user.email).to eq('walter.white@email.com')
expect(user.approved).to eq(true)
expect(user.active).to eq(true)
end
it "does not activate user invited via email with a wrong token" do
invite = Fabricate(:invite, invited_by: Fabricate(:user), email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:sent])
user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White', email_token: 'wrong_token')
expect(user.active).to eq(false)
end
it "does not activate user invited via email without a token" do
invite = Fabricate(:invite, invited_by: Fabricate(:user), email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:sent])
user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White')
expect(user.active).to eq(false)
end
it "does not activate user invited via links" do
invite = Fabricate(:invite, email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:not_required]) invite = Fabricate(:invite, email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:not_required])
user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White')

View File

@ -40,7 +40,23 @@ describe Invite do
end end
end end
context '::generate' do context 'before_save' do
it 'regenerates the email token when email is changed' do
invite = Fabricate(:invite, email: 'test@example.com')
token = invite.email_token
invite.update!(email: 'test@example.com')
expect(invite.email_token).to eq(token)
invite.update!(email: 'test2@example.com')
expect(invite.email_token).not_to eq(token)
invite.update!(email: nil)
expect(invite.email_token).to eq(nil)
end
end
context '.generate' do
it 'saves an invites' do it 'saves an invites' do
invite = Invite.generate(user, email: 'TEST@EXAMPLE.COM') invite = Invite.generate(user, email: 'TEST@EXAMPLE.COM')
expect(invite.invite_key).to be_present expect(invite.invite_key).to be_present
@ -59,9 +75,11 @@ describe Invite do
end end
context 'via email' do context 'via email' do
it 'enqueues a job to email the invite' do it 'can be created and a job is enqueued to email the invite' do
invite = Invite.generate(user, email: 'test@example.com') invite = Invite.generate(user, email: 'test@example.com')
expect(invite.email).to eq('test@example.com')
expect(invite.emailed_status).to eq(Invite.emailed_status_types[:sending]) expect(invite.emailed_status).to eq(Invite.emailed_status_types[:sending])
expect(invite.email_token).not_to eq(nil)
expect(Jobs::InviteEmail.jobs.size).to eq(1) expect(Jobs::InviteEmail.jobs.size).to eq(1)
end end
@ -91,6 +109,7 @@ describe Invite do
expect(invite.expires_at.to_date).to eq(SiteSetting.invite_expiry_days.days.from_now.to_date) expect(invite.expires_at.to_date).to eq(SiteSetting.invite_expiry_days.days.from_now.to_date)
expect(invite.emailed_status).to eq(Invite.emailed_status_types[:not_required]) expect(invite.emailed_status).to eq(Invite.emailed_status_types[:not_required])
expect(invite.is_invite_link?).to eq(true) expect(invite.is_invite_link?).to eq(true)
expect(invite.email_token).to eq(nil)
end end
it 'checks for max_redemptions_allowed range' do it 'checks for max_redemptions_allowed range' do

View File

@ -414,7 +414,7 @@ describe InvitesController do
it 'logs in the user' do it 'logs in the user' do
events = DiscourseEvent.track_events do events = DiscourseEvent.track_events do
put "/invites/show/#{invite.invite_key}.json" put "/invites/show/#{invite.invite_key}.json", params: { email_token: invite.email_token }
end end
expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in)
@ -427,7 +427,7 @@ describe InvitesController do
end end
it 'redirects to the first topic the user was invited to' do it 'redirects to the first topic the user was invited to' do
put "/invites/show/#{invite.invite_key}.json" put "/invites/show/#{invite.invite_key}.json", params: { email_token: invite.email_token }
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(response.parsed_body['redirect_to']).to eq(topic.relative_url) expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
end end
@ -553,7 +553,7 @@ describe InvitesController do
it 'does not send an activation email and activates the user' do it 'does not send an activation email and activates the user' do
expect do expect do
put "/invites/show/#{invite.invite_key}.json", params: { password: 'verystrongpassword' } put "/invites/show/#{invite.invite_key}.json", params: { password: 'verystrongpassword', email_token: invite.email_token }
end.to change { UserAuthToken.count }.by(1) end.to change { UserAuthToken.count }.by(1)
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -565,6 +565,21 @@ describe InvitesController do
expect(invited_user.active).to eq(true) expect(invited_user.active).to eq(true)
expect(invited_user.email_confirmed?).to eq(true) expect(invited_user.email_confirmed?).to eq(true)
end end
it 'does not activate user if email token is missing' do
expect do
put "/invites/show/#{invite.invite_key}.json", params: { password: 'verystrongpassword' }
end.to change { UserAuthToken.count }.by(0)
expect(response.status).to eq(200)
expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0)
expect(Jobs::CriticalUserEmail.jobs.size).to eq(1)
invited_user = User.find_by_email(invite.email)
expect(invited_user.active).to eq(false)
expect(invited_user.email_confirmed?).to eq(false)
end
end end
context 'user was invited via link' do context 'user was invited via link' do