diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 2c75d65911..bd21384ae7 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -20,39 +20,32 @@ class InvitesController < ApplicationController RateLimiter.new(nil, "invites-show-#{request.remote_ip}", 100, 1.minute).performed! invite = Invite.find_by(invite_key: params[:id]) + if invite.present? && invite.redeemable? if current_user - added_to_group = false + redeemed = false - if invite.groups.present? - invite_by_guardian = Guardian.new(invite.invited_by) - new_group_ids = invite.groups.pluck(:id) - current_user.group_users.pluck(:group_id) - new_group_ids.each do |id| - if group = Group.find_by(id: id) - if invite_by_guardian.can_edit_group?(group) - group.add(current_user) - added_to_group = true - end - end - end + begin + invite.redeem(email: current_user.email) + redeemed = true + rescue ActiveRecord::RecordNotSaved, Invite::UserExists + # This is not ideal but `Invite#redeem` raises either `Invite::UserExists` or `ActiveRecord::RecordNotSaved` + # error when it fails to redeem the invite. If redemption fails for a logged in user, we will just ignore it. end - create_topic_invite_notifications(invite, current_user) - - if topic = invite.topics.first - new_guardian = Guardian.new(current_user) - return redirect_to(topic.url) if new_guardian.can_see?(topic) - elsif added_to_group - return redirect_to(path("/")) + if redeemed && (topic = invite.topics.first) && current_user.guardian.can_see?(topic) + create_topic_invite_notifications(invite, current_user) + return redirect_to(topic.url) end - return ensure_not_logged_in + return redirect_to(path('/')) end email = Email.obfuscate(invite.email) # Show email if the user already authenticated their email different_external_email = false + if session[:authentication] auth_result = Auth::Result.from_session_data(session[:authentication], user: nil) if invite.email == auth_result.email @@ -63,6 +56,7 @@ class InvitesController < ApplicationController end email_verified_by_link = invite.email_token.present? && params[:t] == invite.email_token + if email_verified_by_link email = invite.email end diff --git a/app/models/invite.rb b/app/models/invite.rb index 1594a109ab..87936a2353 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -167,11 +167,8 @@ class Invite < ActiveRecord::Base 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) - raise UserExists.new I18n.t("invite_link.email_taken") - end - email = self.email if email.blank? && !is_invite_link? + InviteRedeemer.new( invite: self, email: email, diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index e046dd032c..1993ad01a8 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -1,10 +1,9 @@ # frozen_string_literal: true 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 - if invite_was_redeemed? + if can_redeem_invite? && mark_invite_redeemed process_invitation invited_user end @@ -19,13 +18,6 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ available_username = UserNameSuggester.suggest(email) end - if email.present? && invite.domain.present? - username, domain = email.split('@') - if domain.present? && invite.domain != domain - raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed')) - end - end - user = User.where(staged: true).with_email(email.strip.downcase).first user.unstage! if user user ||= User.new @@ -89,6 +81,39 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ private + def can_redeem_invite? + # Invite has already been redeemed + if !invite.is_invite_link? && InvitedUser.exists?(invite_id: invite.id) + return false + end + + validate_invite_email! + + existing_user = get_existing_user + + if existing_user.present? && InvitedUser.exists?(user_id: existing_user.id, invite_id: invite.id) + return false + end + + true + end + + def validate_invite_email! + return if email.blank? + + if invite.email.present? && email.downcase != invite.email.downcase + raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.not_matching_email')) + end + + if invite.domain.present? + username, domain = email.split('@') + + if domain.present? && invite.domain != domain + raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed')) + end + end + end + def invited_user @invited_user ||= get_invited_user end @@ -100,21 +125,9 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ notify_invitee end - def invite_was_redeemed? - mark_invite_redeemed - end - def mark_invite_redeemed - if !invite.is_invite_link? && InvitedUser.exists?(invite_id: invite.id) - return false - end - - existing_user = get_existing_user - if existing_user.present? && InvitedUser.exists?(user_id: existing_user.id, invite_id: invite.id) - return false - end - @invited_user_record = InvitedUser.create!(invite_id: invite.id, redeemed_at: Time.zone.now) + if @invited_user_record.present? Invite.increment_counter(:redemption_count, invite.id) delete_duplicate_invites @@ -125,6 +138,7 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ def get_invited_user result = get_existing_user + result ||= InviteRedeemer.create_user_from_invite( email: email, invite: invite, diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 7110cf1733..42132b2cd3 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -64,54 +64,71 @@ describe InvitesController do end end - it 'adds logged in users to invite groups' do - group = Fabricate(:group) - group.add_owner(invite.invited_by) - InvitedGroup.create!(group: group, invite: invite) + describe 'logged in user viewing an invite' do + fab!(:group) { Fabricate(:group) } - sign_in(user) + before do + sign_in(user) + end - get "/invites/#{invite.invite_key}" - expect(response).to redirect_to("/") - expect(user.reload.groups).to include(group) - end + it "redeems the invite when user's email matches invite's email before redirecting to secured topic url" do + invite.update_columns(email: user.email) + group.add_owner(invite.invited_by) - it 'redirects logged in users to invite topic if they can see it' do - topic = Fabricate(:topic) - TopicInvite.create!(topic: topic, invite: invite) + secured_category = Fabricate(:category) + secured_category.permissions = { group.name => :full } + secured_category.save! - sign_in(user) + topic = Fabricate(:topic, category: secured_category) + TopicInvite.create!(invite: invite, topic: topic) + InvitedGroup.create!(invite: invite, group: group) - get "/invites/#{invite.invite_key}" - expect(response).to redirect_to(topic.url) - expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1) - end + expect do + get "/invites/#{invite.invite_key}" + end.to change { InvitedUser.exists?(invite: invite, user: user) }.to(true) - it 'adds logged in user to group and redirects them to invite topic' do - group = Fabricate(:group) - group.add_owner(invite.invited_by) - secured_category = Fabricate(:category) - secured_category.permissions = { group.name => :full } - secured_category.save! - topic = Fabricate(:topic, category: secured_category) - TopicInvite.create!(invite: invite, topic: topic) - InvitedGroup.create!(invite: invite, group: group) + expect(response).to redirect_to(topic.url) + expect(user.reload.groups).to include(group) - sign_in(user) + expect(Notification.exists?(user: user, notification_type: Notification.types[:invited_to_topic], topic: topic)) + .to eq(true) - get "/invites/#{invite.invite_key}" - expect(user.reload.groups).to include(group) - expect(response).to redirect_to(topic.url) - expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1) - end + expect(Notification.exists?(user: invite.invited_by, notification_type: Notification.types[:invitee_accepted])) + .to eq(true) + end - it 'fails for logged in users' do - sign_in(Fabricate(:user)) + it "redeems the invite when user's email domain matches the domain an invite link is restricted to" do + invite.update!(email: nil, domain: 'discourse.org') + user.update!(email: "someguy@discourse.org") + topic = Fabricate(:topic) + TopicInvite.create!(invite: invite, topic: topic) + group.add_owner(invite.invited_by) + InvitedGroup.create!(invite: invite, group: group) - get "/invites/#{invite.invite_key}" - expect(response.status).to eq(200) - expect(response.body).to_not have_tag(:script, with: { src: '/assets/application.js' }) - expect(response.body).to include(I18n.t('login.already_logged_in')) + expect do + get "/invites/#{invite.invite_key}" + end.to change { InvitedUser.exists?(invite: invite, user: user) }.to(true) + + expect(response).to redirect_to(topic.url) + expect(user.reload.groups).to include(group) + end + + it "redirects to root if a logged in user tries to view an invite link restricted to a certain domain but user's email domain does not match" do + user.update!(email: "someguy@discourse.com") + invite.update!(email: nil, domain: 'discourse.org') + + expect { get "/invites/#{invite.invite_key}" }.to change { InvitedUser.count }.by(0) + + expect(response).to redirect_to("/") + end + + it "redirects to root if a tries to view an invite meant for a specific email that is not the user's" do + invite.update_columns(email: "notuseremail@discourse.org") + + expect { get "/invites/#{invite.invite_key}" }.to change { InvitedUser.count }.by(0) + + expect(response).to redirect_to("/") + end end it 'fails if invite does not exist' do