diff --git a/app/assets/javascripts/discourse/app/controllers/invites-show.js b/app/assets/javascripts/discourse/app/controllers/invites-show.js index f7cf803380f..3c3658b06fc 100644 --- a/app/assets/javascripts/discourse/app/controllers/invites-show.js +++ b/app/assets/javascripts/discourse/app/controllers/invites-show.js @@ -22,6 +22,8 @@ export default Controller.extend( NameValidation, UserFieldsValidation, { + queryParams: ["t"], + createAccount: controller(), invitedBy: readOnly("model.invited_by"), @@ -216,6 +218,8 @@ export default Controller.extend( if (this.isInviteLink) { data.email = this.email; + } else { + data.email_token = this.t; } ajax({ diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 3cbf6fcb837..1a86eb63af6 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -197,7 +197,7 @@ class InvitesController < ApplicationController # via the SessionController#sso_login route def perform_accept_invitation 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]) @@ -212,13 +212,13 @@ class InvitesController < ApplicationController session: session } - attrs[:email] = - if invite.is_invite_link? - params.require([:email]) - params[:email] - else - invite.email - end + if invite.is_invite_link? + params.require(:email) + attrs[:email] = params[:email] + else + attrs[:email] = invite.email + attrs[:email_token] = params[:email_token] if params[:email_token].present? + end user = invite.redeem(**attrs) rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e diff --git a/app/mailers/invite_mailer.rb b/app/mailers/invite_mailer.rb index 2f751e30f94..6f5f284422f 100644 --- a/app/mailers/invite_mailer.rb +++ b/app/mailers/invite_mailer.rb @@ -36,7 +36,7 @@ class InviteMailer < ActionMailer::Base template: sanitized_message ? 'custom_invite_mailer' : 'invite_mailer', inviter_name: inviter_name, 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_excerpt: topic_excerpt, site_description: SiteSetting.site_description, @@ -47,7 +47,7 @@ class InviteMailer < ActionMailer::Base template: sanitized_message ? 'custom_invite_forum_mailer' : 'invite_forum_mailer', inviter_name: inviter_name, 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_title: SiteSetting.title, user_custom_message: sanitized_message) diff --git a/app/models/invite.rb b/app/models/invite.rb index 109b767c535..76181639c9d 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -39,6 +39,12 @@ class Invite < ActiveRecord::Base self.expires_at ||= SiteSetting.invite_expiry_days.days.from_now end + before_save do + if will_save_change_to_email? + self.email_token = email.present? ? SecureRandom.hex : nil + end + end + before_validation do self.email = Email.downcase(email) unless email.nil? end @@ -85,8 +91,9 @@ class Invite < ActiveRecord::Base expires_at < Time.zone.now end - def link - "#{Discourse.base_url}/invites/#{invite_key}" + def link(with_email_token: false) + with_email_token ? "#{Discourse.base_url}/invites/#{invite_key}?t=#{email_token}" + : "#{Discourse.base_url}/invites/#{invite_key}" end def link_valid? @@ -167,7 +174,7 @@ class Invite < ActiveRecord::Base invite.reload 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? if is_invite_link? && UserEmail.exists?(email: email) @@ -183,7 +190,8 @@ class Invite < ActiveRecord::Base password: password, user_custom_fields: user_custom_fields, ip_address: ip_address, - session: session + session: session, + email_token: email_token ).redeem end diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 8c7b4aad303..f8c607afe17 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -1,6 +1,6 @@ # 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 Invite.transaction do @@ -14,7 +14,7 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ end # 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.unstage! if user @@ -76,7 +76,7 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ user.save! 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.activate end @@ -131,7 +131,8 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ password: password, user_custom_fields: user_custom_fields, ip_address: ip_address, - session: session + session: session, + email_token: email_token ) result.send_welcome_message = false result diff --git a/db/migrate/20210409142455_add_token_to_invites.rb b/db/migrate/20210409142455_add_token_to_invites.rb new file mode 100644 index 00000000000..43d1632a662 --- /dev/null +++ b/db/migrate/20210409142455_add_token_to_invites.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddTokenToInvites < ActiveRecord::Migration[6.0] + def change + add_column :invites, :email_token, :string + end +end diff --git a/plugins/discourse-narrative-bot/spec/requests/discobot_welcome_post_spec.rb b/plugins/discourse-narrative-bot/spec/requests/discobot_welcome_post_spec.rb index 334d16c2716..015879c52b8 100644 --- a/plugins/discourse-narrative-bot/spec/requests/discobot_welcome_post_spec.rb +++ b/plugins/discourse-narrative-bot/spec/requests/discobot_welcome_post_spec.rb @@ -29,16 +29,15 @@ describe "Discobot welcome post" do end 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 - invite - expect do put "/invites/show/#{invite.invite_key}.json", params: { username: 'somename', name: 'testing', - password: 'asodaasdaosdhq' + password: 'verystrongpassword', + email_token: invite.email_token } end.to change { User.count }.by(1) diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb index deaa6575706..aee4dcce2f4 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -10,9 +10,9 @@ describe InviteRedeemer do user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') expect(user.username).to eq('walter') expect(user.name).to eq('Walter White') - expect(user).to be_active expect(user.email).to eq('walter.white@email.com') expect(user.approved).to eq(true) + expect(user.active).to eq(false) end it "can set the password and ip_address" do @@ -52,7 +52,30 @@ describe InviteRedeemer do expect(user.approved).to eq(true) 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]) user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 9850aba37ec..fd722a15422 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -40,7 +40,23 @@ describe Invite do 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 invite = Invite.generate(user, email: 'TEST@EXAMPLE.COM') expect(invite.invite_key).to be_present @@ -59,9 +75,11 @@ describe Invite do end 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') + expect(invite.email).to eq('test@example.com') 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) 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.emailed_status).to eq(Invite.emailed_status_types[:not_required]) expect(invite.is_invite_link?).to eq(true) + expect(invite.email_token).to eq(nil) end it 'checks for max_redemptions_allowed range' do diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index bea2e20b7c0..f2001d53097 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -414,7 +414,7 @@ describe InvitesController do it 'logs in the user' 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 expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) @@ -427,7 +427,7 @@ describe InvitesController do end 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.parsed_body['redirect_to']).to eq(topic.relative_url) end @@ -553,7 +553,7 @@ describe InvitesController do it 'does not send an activation email and activates the user' 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) expect(response.status).to eq(200) @@ -565,6 +565,21 @@ describe InvitesController do expect(invited_user.active).to eq(true) expect(invited_user.email_confirmed?).to eq(true) 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 context 'user was invited via link' do