SECURITY: Fix invite link email validation (#18817)

See https://github.com/discourse/discourse/security/advisories/GHSA-x8w7-rwmr-w278

Co-authored-by: Martin Brennan <martin@discourse.org>

app/assets/javascripts/discourse/app/controllers/invites-show.js

@@ -1,4 +1,4 @@
-import { alias, not, or, readOnly } from "@ember/object/computed";
+import { alias, bool, not, readOnly } from "@ember/object/computed";
 import Controller, { inject as controller } from "@ember/controller";
 import DiscourseURL from "discourse/lib/url";
 import EmberObject from "@ember/object";
@@ -29,6 +29,9 @@ export default Controller.extend(
     invitedBy: readOnly("model.invited_by"),
     email: alias("model.email"),
     accountEmail: alias("email"),
+    existingUserId: readOnly("model.existing_user_id"),
+    existingUserCanRedeem: readOnly("model.existing_user_can_redeem"),
+    existingUserRedeeming: bool("existingUserId"),
     hiddenEmail: alias("model.hidden_email"),
     emailVerifiedByLink: alias("model.email_verified_by_link"),
     differentExternalEmail: alias("model.different_external_email"),
@@ -40,13 +43,6 @@ export default Controller.extend(
     authOptions: null,
     inviteImageUrl: getUrl("/images/envelope.svg"),
     isInviteLink: readOnly("model.is_invite_link"),
-    submitDisabled: or(
-      "emailValidation.failed",
-      "usernameValidation.failed",
-      "passwordValidation.failed",
-      "nameValidation.failed",
-      "userFieldsValidation.failed"
-    ),
     rejectedEmails: null,
 
     init() {
@@ -81,6 +77,15 @@ export default Controller.extend(
       });
     },
 
+    @discourseComputed("existingUserId")
+    subheaderMessage(existingUserId) {
+      if (existingUserId) {
+        return I18n.t("invites.existing_user_can_redeem");
+      } else {
+        return I18n.t("create_account.subheader_title");
+      }
+    },
+
     @discourseComputed("email")
     yourEmailMessage(email) {
       return I18n.t("invites.your_email", { email });
@@ -100,6 +105,37 @@ export default Controller.extend(
       );
     },
 
+    @discourseComputed(
+      "emailValidation.failed",
+      "usernameValidation.failed",
+      "passwordValidation.failed",
+      "nameValidation.failed",
+      "userFieldsValidation.failed",
+      "existingUserRedeeming",
+      "existingUserCanRedeem"
+    )
+    submitDisabled(
+      emailValidationFailed,
+      usernameValidationFailed,
+      passwordValidationFailed,
+      nameValidationFailed,
+      userFieldsValidationFailed,
+      existingUserRedeeming,
+      existingUserCanRedeem
+    ) {
+      if (existingUserRedeeming) {
+        return !existingUserCanRedeem;
+      }
+
+      return (
+        emailValidationFailed ||
+        usernameValidationFailed ||
+        passwordValidationFailed ||
+        nameValidationFailed ||
+        userFieldsValidationFailed
+      );
+    },
+
     @discourseComputed(
       "externalAuthsEnabled",
       "externalAuthsOnly",
@@ -118,13 +154,20 @@ export default Controller.extend(
     @discourseComputed(
       "externalAuthsOnly",
       "authOptions",
-      "emailValidation.failed"
+      "emailValidation.failed",
+      "existingUserRedeeming"
     )
-    shouldDisplayForm(externalAuthsOnly, authOptions, emailValidationFailed) {
+    shouldDisplayForm(
+      externalAuthsOnly,
+      authOptions,
+      emailValidationFailed,
+      existingUserRedeeming
+    ) {
       return (
         (this.siteSettings.enable_local_logins ||
           (externalAuthsOnly && authOptions && !emailValidationFailed)) &&
-        !this.siteSettings.enable_discourse_connect
+        !this.siteSettings.enable_discourse_connect &&
+        !existingUserRedeeming
       );
     },
 

app/assets/javascripts/discourse/app/templates/invites/show.hbs

@@ -4,7 +4,7 @@
       <h1 class="login-title">{{this.welcomeTitle}}</h1>
       <img src={{this.wavingHandURL}} alt="" class="waving-hand">
       {{#unless this.successMessage}}
-        <p class="login-subheader">{{i18n "create_account.subheader_title"}}</p>
+        <p class="login-subheader">{{this.subheaderMessage}}</p>
       {{/unless}}
     </div>
 
@@ -131,6 +131,13 @@
                 {{/if}}
               </form>
             {{/if}}
+            {{#if this.existingUserRedeeming}}
+              {{#if this.existingUserCanRedeem}}
+                <DButton @class="btn-primary" @action={{action "submit"}} @type="submit" @disabled={{this.submitDisabled}} @label="invites.accept_invite" />
+              {{else}}
+                <div class="alert alert-error">{{i18n "invites.existing_user_cannot_redeem"}}</div>
+              {{/if}}
+            {{/if}}
           {{/if}}
         </div>
       </div>

app/controllers/invites_controller.rb

@@ -12,7 +12,6 @@ class InvitesController < ApplicationController
 
   before_action :ensure_invites_allowed, only: [:show, :perform_accept_invitation]
   before_action :ensure_new_registrations_allowed, only: [:show, :perform_accept_invitation]
-  before_action :ensure_not_logged_in, only: :perform_accept_invitation
 
   def show
     expires_now
@@ -22,90 +21,9 @@ class InvitesController < ApplicationController
     invite = Invite.find_by(invite_key: params[:id])
 
     if invite.present? && invite.redeemable?
-      if current_user
+      show_invite(invite)
-        redeemed = false
-
-        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
-
-        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 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
-          email = invite.email
-        else
-          different_external_email = true
-        end
-      end
-
-      email_verified_by_link = invite.email_token.present? && params[:t] == invite.email_token
-
-      if email_verified_by_link
-        email = invite.email
-      end
-
-      hidden_email = email != invite.email
-
-      if hidden_email || invite.email.nil?
-        username = ""
-      else
-        username = UserNameSuggester.suggest(invite.email)
-      end
-
-      info = {
-        invited_by: UserNameSerializer.new(invite.invited_by, scope: guardian, root: false),
-        email: email,
-        hidden_email: hidden_email,
-        username: username,
-        is_invite_link: invite.is_invite_link?,
-        email_verified_by_link: email_verified_by_link
-      }
-
-      if different_external_email
-        info[:different_external_email] = true
-      end
-
-      if staged_user = User.where(staged: true).with_email(invite.email).first
-        info[:username] = staged_user.username
-        info[:user_fields] = staged_user.user_fields
-      end
-
-      store_preloaded("invite_info", MultiJson.dump(info))
-
-      secure_session["invite-key"] = invite.invite_key
-
-      render layout: 'application'
     else
-      flash.now[:error] = if invite.blank?
+      show_irredeemable_invite(invite)
-        I18n.t('invite.not_found', base_url: Discourse.base_url)
-      elsif invite.redeemed?
-        if invite.is_invite_link?
-          I18n.t('invite.not_found_template_link', site_name: SiteSetting.title, base_url: Discourse.base_url)
-        else
-          I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)
-        end
-      elsif invite.expired?
-        I18n.t('invite.expired', base_url: Discourse.base_url)
-      end
-
-      render layout: 'no_ember'
     end
   rescue RateLimiter::LimitExceeded => e
     flash.now[:error] = e.description
@@ -277,24 +195,33 @@ class InvitesController < ApplicationController
     params.permit(:email, :username, :name, :password, :timezone, :email_token, user_custom_fields: {})
 
     invite = Invite.find_by(invite_key: params[:id])
+    redeeming_user = current_user
 
     if invite.present?
       begin
         attrs = {
-          username: params[:username],
-          name: params[:name],
-          password: params[:password],
-          user_custom_fields: params[:user_custom_fields],
           ip_address: request.remote_ip,
           session: session
         }
 
-        if invite.is_invite_link?
+        if redeeming_user
-          params.require(:email)
+          attrs[:redeeming_user] = redeeming_user
-          attrs[:email] = params[:email]
         else
-          attrs[:email] = invite.email
+          attrs[:username] = params[:username]
-          attrs[:email_token] = params[:email_token] if params[:email_token].present?
+          attrs[:name] = params[:name]
+          attrs[:password] = params[:password]
+          attrs[:user_custom_fields] = params[:user_custom_fields]
+
+          # If the invite is not scoped to an email then we allow the
+          # user to provide it themselves
+          if invite.is_invite_link?
+            params.require(:email)
+            attrs[:email] = params[:email]
+          else
+            # Otherwise we always use the email from the invitation.
+            attrs[:email] = invite.email
+            attrs[:email_token] = params[:email_token] if params[:email_token].present?
+          end
         end
 
         user = invite.redeem(**attrs)
@@ -306,7 +233,10 @@ class InvitesController < ApplicationController
         return render json: failed_json.merge(message: I18n.t('invite.not_found_json')), status: 404
       end
 
-      log_on_user(user) if user.active? && user.guardian.can_access_forum?
+      if !redeeming_user && user.active? && user.guardian.can_access_forum?
+        log_on_user(user)
+      end
+
       user.update_timezone_if_missing(params[:timezone])
       post_process_invite(user)
       create_topic_invite_notifications(invite, user)
@@ -316,6 +246,10 @@ class InvitesController < ApplicationController
 
       if user.present?
         if user.active? && user.guardian.can_access_forum?
+          if redeeming_user
+            response[:message] = I18n.t("invite.existing_user_success")
+          end
+
           if user.guardian.can_see?(topic)
             response[:redirect_to] = path(topic.relative_url)
           else
@@ -424,6 +358,84 @@ class InvitesController < ApplicationController
 
   private
 
+  def show_invite(invite)
+    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
+        email = invite.email
+      else
+        different_external_email = true
+      end
+    end
+
+    email_verified_by_link = invite.email_token.present? && params[:t] == invite.email_token
+
+    if email_verified_by_link
+      email = invite.email
+    end
+
+    hidden_email = email != invite.email
+
+    if hidden_email || invite.email.nil?
+      username = ""
+    else
+      username = UserNameSuggester.suggest(invite.email)
+    end
+
+    info = {
+      invited_by: UserNameSerializer.new(invite.invited_by, scope: guardian, root: false),
+      email: email,
+      hidden_email: hidden_email,
+      username: username,
+      is_invite_link: invite.is_invite_link?,
+      email_verified_by_link: email_verified_by_link
+    }
+
+    if different_external_email
+      info[:different_external_email] = true
+    end
+
+    if staged_user = User.where(staged: true).with_email(invite.email).first
+      info[:username] = staged_user.username
+      info[:user_fields] = staged_user.user_fields
+    end
+
+    if current_user
+      info[:existing_user_id] = current_user.id
+      info[:existing_user_can_redeem] = invite.can_be_redeemed_by?(current_user)
+      info[:email] = current_user.email
+      info[:username] = current_user.username
+    end
+
+    store_preloaded("invite_info", MultiJson.dump(info))
+
+    secure_session["invite-key"] = invite.invite_key
+
+    render layout: 'application'
+  end
+
+  def show_irredeemable_invite(invite)
+    flash.now[:error] = \
+      if invite.blank?
+        I18n.t('invite.not_found', base_url: Discourse.base_url)
+      elsif invite.redeemed?
+        if invite.is_invite_link?
+          I18n.t('invite.not_found_template_link', site_name: SiteSetting.title, base_url: Discourse.base_url)
+        else
+          I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)
+        end
+      elsif invite.expired?
+        I18n.t('invite.expired', base_url: Discourse.base_url)
+      end
+
+    render layout: 'no_ember'
+  end
+
   def ensure_invites_allowed
     if (!SiteSetting.enable_local_logins && Discourse.enabled_auth_providers.count == 0 && !SiteSetting.enable_discourse_connect)
       raise Discourse::NotFound
@@ -438,14 +450,6 @@ class InvitesController < ApplicationController
     end
   end
 
-  def ensure_not_logged_in
-    if current_user
-      flash[:error] = I18n.t("login.already_logged_in")
-      render layout: 'no_ember'
-      false
-    end
-  end
-
   def groups_can_see_topic?(groups, topic)
     if topic&.read_restricted_category?
       topic_groups = topic.category.groups

app/controllers/session_controller.rb

@@ -159,7 +159,7 @@ class SessionController < ApplicationController
 
         if SiteSetting.must_approve_users? && !user.approved?
           if invite.present? && user.invited_user.blank?
-            redeem_invitation(invite, sso)
+            redeem_invitation(invite, sso, user)
           end
 
           if SiteSetting.discourse_connect_not_approved_url.present?
@@ -173,7 +173,7 @@ class SessionController < ApplicationController
         # the user has not already redeemed an invite
         # (covers the same SSO user visiting an invite link)
         elsif invite.present? && user.invited_user.blank?
-          redeem_invitation(invite, sso)
+          redeem_invitation(invite, sso, user)
 
           # we directly call user.activate here instead of going
           # through the UserActivator path because we assume the account
@@ -772,14 +772,15 @@ class SessionController < ApplicationController
     invite
   end
 
-  def redeem_invitation(invite, sso)
+  def redeem_invitation(invite, sso, redeeming_user)
     InviteRedeemer.new(
       invite: invite,
       username: sso.username,
       name: sso.name,
       ip_address: request.remote_ip,
       session: session,
-      email: sso.email
+      email: sso.email,
+      redeeming_user: redeeming_user
     ).redeem
     secure_session["invite-key"] = nil
 

app/models/email_token.rb

@@ -69,7 +69,7 @@ class EmailToken < ActiveRecord::Base
       user.create_reviewable if !skip_reviewable
       user.set_automatic_groups
       DiscourseEvent.trigger(:user_confirmed_email, user)
-      Invite.redeem_from_email(user.email) if scope == EmailToken.scopes[:signup]
+      Invite.redeem_for_existing_user(user) if scope == EmailToken.scopes[:signup]
 
       user.reload
     end

app/models/invite.rb

@@ -90,6 +90,22 @@ class Invite < ActiveRecord::Base
     end
   end
 
+  def email_matches?(email)
+    email.downcase == self.email.downcase
+  end
+
+  def domain_matches?(email)
+    _, domain = email.split('@')
+    self.domain == domain
+  end
+
+  def can_be_redeemed_by?(user)
+    return false if !self.redeemable?
+    return true if self.email.blank? && self.domain.blank?
+    return true if self.email.present? && email_matches?(user.email)
+    self.domain.present? && domain_matches?(user.email)
+  end
+
   def expired?
     expires_at < Time.zone.now
   end
@@ -172,7 +188,17 @@ 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, email_token: nil)
+  def redeem(
+    email: nil,
+    username: nil,
+    name: nil,
+    password: nil,
+    user_custom_fields: nil,
+    ip_address: nil,
+    session: nil,
+    email_token: nil,
+    redeeming_user: nil
+  )
     return if !redeemable?
 
     email = self.email if email.blank? && !is_invite_link?
@@ -186,14 +212,15 @@ class Invite < ActiveRecord::Base
       user_custom_fields: user_custom_fields,
       ip_address: ip_address,
       session: session,
-      email_token: email_token
+      email_token: email_token,
+      redeeming_user: redeeming_user
     ).redeem
   end
 
-  def self.redeem_from_email(email)
+  def self.redeem_for_existing_user(user)
-    invite = Invite.find_by(email: Email.downcase(email))
+    invite = Invite.find_by(email: Email.downcase(user.email))
     if invite.present? && invite.redeemable?
-      InviteRedeemer.new(invite: invite, email: invite.email).redeem
+      InviteRedeemer.new(invite: invite, redeeming_user: user).redeem
     end
     invite
   end

app/models/invite_redeemer.rb

@@ -1,6 +1,41 @@
 # frozen_string_literal: true
 
-InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_custom_fields, :ip_address, :session, :email_token, keyword_init: true) do
+class InviteRedeemer
+  attr_reader :invite,
+    :email,
+    :username,
+    :name,
+    :password,
+    :user_custom_fields,
+    :ip_address,
+    :session,
+    :email_token,
+    :redeeming_user
+
+  def initialize(
+    invite: nil,
+    email: nil,
+    username: nil,
+    name: nil,
+    password: nil,
+    user_custom_fields: nil,
+    ip_address: nil,
+    session: nil,
+    email_token: nil,
+    redeeming_user: nil)
+
+    @invite = invite
+    @email = email
+    @username = username
+    @name = name
+    @password = password
+    @user_custom_fields = user_custom_fields
+    @ip_address = ip_address
+    @session = session
+    @email_token = email_token
+    @redeeming_user = redeeming_user
+  end
+
   def redeem
     Invite.transaction do
       if can_redeem_invite? && mark_invite_redeemed
@@ -82,42 +117,64 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_
   private
 
   def can_redeem_invite?
-    return false unless invite.redeemable?
+    return false if !invite.redeemable?
 
-    # Invite has already been redeemed
+    # Invite has already been redeemed by anyone.
     if !invite.is_invite_link? && InvitedUser.exists?(invite_id: invite.id)
       return false
     end
 
-    validate_invite_email!
+    # Email will not be present if we are claiming an invite link, which
+    # does not have an email or domain scope on the invitation.
+    if email.present? || redeeming_user.present?
+      email_to_check = redeeming_user&.email || email
 
-    existing_user = get_existing_user
+      if invite.email.present? && !invite.email_matches?(email_to_check)
+        raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.not_matching_email'))
+      end
 
-    if existing_user.present? && InvitedUser.exists?(user_id: existing_user.id, invite_id: invite.id)
+      if invite.domain.present? && !invite.domain_matches?(email_to_check)
+        raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed'))
+      end
+    end
+
+    # Anon user is trying to redeem an invitation, if an existing user already
+    # redeemed it then we cannot redeem now.
+    redeeming_user ||= User.where(admin: false, staged: false).find_by_email(email)
+    if redeeming_user.present? && InvitedUser.exists?(user_id: redeeming_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
+    return @invited_user if defined?(@invited_user)
+
+    # The redeeming user is an already logged in user or a user who is
+    # activating their account who is redeeming the invite,
+    # which is valid for existing users to be invited to topics or groups.
+    if redeeming_user.present?
+      @invited_user = redeeming_user
+      return @invited_user
+    end
+
+    # If there was no logged in user then we must attempt to create
+    # one based on the provided params.
+    invited_user ||= InviteRedeemer.create_user_from_invite(
+      email: email,
+      invite: invite,
+      username: username,
+      name: name,
+      password: password,
+      user_custom_fields: user_custom_fields,
+      ip_address: ip_address,
+      session: session,
+      email_token: email_token
+    )
+    invited_user.send_welcome_message = false
+    @invited_user = invited_user
+    @invited_user
   end
 
   def process_invitation
@@ -138,28 +195,6 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_
     @invited_user_record.present?
   end
 
-  def get_invited_user
-    result = get_existing_user
-
-    result ||= InviteRedeemer.create_user_from_invite(
-      email: email,
-      invite: invite,
-      username: username,
-      name: name,
-      password: password,
-      user_custom_fields: user_custom_fields,
-      ip_address: ip_address,
-      session: session,
-      email_token: email_token
-    )
-    result.send_welcome_message = false
-    result
-  end
-
-  def get_existing_user
-    User.where(admin: false, staged: false).find_by_email(email)
-  end
-
   def add_to_private_topics_if_invited
     topic_ids = Topic.where(archetype: Archetype::private_message).includes(:invites).where(invites: { email: email }).pluck(:id)
     topic_ids.each do |id|

config/locales/client.en.yml

@@ -2089,6 +2089,8 @@ en:
       success: "Your account has been created and you're now logged in."
       name_label: "Name"
       password_label: "Password"
+      existing_user_can_redeem: "Redeem your invitation to a topic or group."
+      existing_user_cannot_redeem: "This invitation cannot be redeemed. Please ask the person who invited you to send you a new invitation."
 
     password_reset:
       continue: "Continue to %{site_name}"

config/locales/server.en.yml

@@ -266,6 +266,7 @@ en:
     max_redemptions_allowed_one: "for email invites should be 1."
     redemption_count_less_than_max: "should be less than %{max_redemptions_allowed}."
     email_xor_domain: "Email and domain fields are not allowed at the same time"
+    existing_user_success: "Invite redeemed successfully"
 
   bulk_invite:
     file_should_be_csv: "The uploaded file should be of csv format."

db/post_migrate/20221026035440_security_log_out_invite_redemption_invited_users.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class SecurityLogOutInviteRedemptionInvitedUsers < ActiveRecord::Migration[7.0]
+  def up
+    # 20220606061813 was added shortly before the vulnerability was introduced
+    vulnerable_since = DB.query_single("SELECT created_at FROM schema_migration_details WHERE version='20220606061813'")[0]
+
+    DB.exec(<<~SQL, vulnerable_since: vulnerable_since)
+      DELETE FROM user_auth_tokens
+      WHERE user_id IN (
+        SELECT DISTINCT user_id
+        FROM invited_users
+        JOIN users ON invited_users.user_id = users.id
+        WHERE invited_users.redeemed_at > :vulnerable_since
+      )
+    SQL
+
+    DB.exec(<<~SQL, vulnerable_since: vulnerable_since)
+      DELETE FROM user_api_keys
+      WHERE user_id IN (
+        SELECT DISTINCT user_id
+        FROM invited_users
+        JOIN users ON invited_users.user_id = users.id
+        WHERE invited_users.redeemed_at > :vulnerable_since
+      )
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

spec/models/invite_redeemer_spec.rb

@@ -242,6 +242,40 @@ RSpec.describe InviteRedeemer do
       expect(invite.invited_users.first).to be_present
     end
 
+    it "raises an error if the email does not match the invite email" do
+      redeemer = InviteRedeemer.new(invite: invite, email: "blah@test.com", username: username, name: name)
+      expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+    end
+
+    context "when a redeeming user is passed in" do
+      fab!(:redeeming_user) { Fabricate(:user, email: "foobar@example.com") }
+
+      it "raises an error if the email does not match the invite email" do
+        redeeming_user.update!(email: "foo@bar.com")
+        redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user)
+        expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+      end
+    end
+
+    context 'with domain' do
+      fab!(:invite) { Fabricate(:invite, email: nil, domain: "test.com") }
+
+      it "raises an error if the email domain does not match the invite domain" do
+        redeemer = InviteRedeemer.new(invite: invite, email: "blah@somesite.com", username: username, name: name)
+        expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.domain_not_allowed"))
+      end
+
+      context "when a redeeming user is passed in" do
+        fab!(:redeeming_user) { Fabricate(:user, email: "foo@test.com") }
+
+        it "raises an error if the user's email domain does not match the invite domain" do
+          redeeming_user.update!(email: "foo@bar.com")
+          redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user)
+          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.domain_not_allowed"))
+        end
+      end
+    end
+
     context 'with invite_link' do
       fab!(:invite_link) { Fabricate(:invite, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) }
       let(:invite_redeemer) { InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') }
@@ -257,7 +291,7 @@ RSpec.describe InviteRedeemer do
       end
 
       it "should not redeem the invite if InvitedUser record already exists for email" do
-        user = invite_redeemer.redeem
+        invite_redeemer.redeem
         invite_link.reload
 
         another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'foo@example.com')
@@ -266,14 +300,28 @@ RSpec.describe InviteRedeemer do
       end
 
       it "should redeem the invite if InvitedUser record does not exists for email" do
-        user = invite_redeemer.redeem
+        invite_redeemer.redeem
         invite_link.reload
 
         another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'bar@example.com')
         another_user = another_invite_redeemer.redeem
         expect(another_user.is_a?(User)).to eq(true)
       end
-    end
 
+      it "raises an error if the email is already being used by an existing user" do
+        Fabricate(:user, email: 'foo@example.com')
+        expect { invite_redeemer.redeem }.to raise_error(ActiveRecord::RecordInvalid, /Primary email has already been taken/)
+      end
+
+      context "when a redeeming user is passed in" do
+        fab!(:redeeming_user) { Fabricate(:user, email: 'foo@example.com') }
+
+        it "does not create a new user" do
+          expect do
+            InviteRedeemer.new(invite: invite_link, redeeming_user: redeeming_user).redeem
+          end.not_to change { User.count }
+        end
+      end
+    end
   end
 end

spec/models/invite_spec.rb

@@ -338,38 +338,38 @@ RSpec.describe Invite do
     end
   end
 
-  describe '#redeem_from_email' do
+  describe '#redeem_for_existing_user' do
     fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }
     fab!(:user) { Fabricate(:user, email: invite.email) }
 
     it 'redeems the invite from email' do
-      Invite.redeem_from_email(user.email)
+      Invite.redeem_for_existing_user(user)
       expect(invite.reload).to be_redeemed
     end
 
     it 'does not redeem the invite if email does not match' do
-      Invite.redeem_from_email('test2@example.com')
+      user.update!(email: 'test2@example.com')
+      Invite.redeem_for_existing_user(user)
       expect(invite.reload).not_to be_redeemed
     end
 
     it 'does not work with expired invites' do
       invite.update!(expires_at: 1.day.ago)
-      Invite.redeem_from_email(user.email)
+      Invite.redeem_for_existing_user(user)
       expect(invite).not_to be_redeemed
     end
 
     it 'does not work with deleted invites' do
       invite.trash!
-      Invite.redeem_from_email(user.email)
+      Invite.redeem_for_existing_user(user)
       expect(invite).not_to be_redeemed
     end
 
     it 'does not work with invalidated invites' do
       invite.update!(invalidated_at: 1.day.ago)
-      Invite.redeem_from_email(user.email)
+      Invite.redeem_for_existing_user(user)
       expect(invite).not_to be_redeemed
     end
-
   end
 
   describe 'scopes' do

spec/requests/invites_controller_spec.rb

@@ -78,63 +78,68 @@ RSpec.describe InvitesController do
         sign_in(user)
       end
 
-      it "redeems the invite when user's email matches invite's email before redirecting to secured topic url" do
+      it "shows the accept invite page when user's email matches the invite email" do
         invite.update_columns(email: user.email)
-        group.add_owner(invite.invited_by)
 
-        secured_category = Fabricate(:category)
+        get "/invites/#{invite.invite_key}"
-        secured_category.permissions = { group.name => :full }
+        expect(response.status).to eq(200)
-        secured_category.save!
+        expect(response.body).to have_tag(:script, with: { src: "/assets/discourse.js" })
+        expect(response.body).not_to include(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url))
 
-        topic = Fabricate(:topic, category: secured_category)
+        expect(response.body).to have_tag('div#data-preloaded') do |element|
-        TopicInvite.create!(invite: invite, topic: topic)
+          json = JSON.parse(element.current_scope.attribute('data-preloaded').value)
-        InvitedGroup.create!(invite: invite, group: group)
+          invite_info = JSON.parse(json['invite_info'])
-
+          expect(invite_info['username']).to eq(user.username)
-        expect do
+          expect(invite_info['email']).to eq(user.email)
-          get "/invites/#{invite.invite_key}"
+          expect(invite_info['existing_user_id']).to eq(user.id)
-        end.to change { InvitedUser.exists?(invite: invite, user: user) }.to(true)
+          expect(invite_info['existing_user_can_redeem']).to eq(true)
-
+        end
-        expect(response).to redirect_to(topic.url)
-        expect(user.reload.groups).to include(group)
-
-        expect(Notification.exists?(user: user, notification_type: Notification.types[:invited_to_topic], topic: topic))
-          .to eq(true)
-
-        expect(Notification.exists?(user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]))
-          .to eq(true)
       end
 
-      it "redeems the invite when user's email domain matches the domain an invite link is restricted to" do
+      it "shows the accept invite page 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)
 
-        expect do
+        get "/invites/#{invite.invite_key}"
-          get "/invites/#{invite.invite_key}"
+        expect(response.status).to eq(200)
-        end.to change { InvitedUser.exists?(invite: invite, user: user) }.to(true)
+        expect(response.body).to have_tag(:script, with: { src: "/assets/discourse.js" })
+        expect(response.body).not_to include(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url))
 
-        expect(response).to redirect_to(topic.url)
+        expect(response.body).to have_tag('div#data-preloaded') do |element|
-        expect(user.reload.groups).to include(group)
+          json = JSON.parse(element.current_scope.attribute('data-preloaded').value)
+          invite_info = JSON.parse(json['invite_info'])
+          expect(invite_info['username']).to eq(user.username)
+          expect(invite_info['email']).to eq(user.email)
+          expect(invite_info['existing_user_id']).to eq(user.id)
+          expect(invite_info['existing_user_can_redeem']).to eq(true)
+        end
       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
+      it "does not allow the user to accept the invite when their email domain does not match the domain of the invite" do
         user.update!(email: "someguy@discourse.com")
         invite.update!(email: nil, domain: 'discourse.org')
 
-        expect { get "/invites/#{invite.invite_key}" }.not_to change { InvitedUser.count }
+        get "/invites/#{invite.invite_key}"
+        expect(response.status).to eq(200)
 
-        expect(response).to redirect_to("/")
+        expect(response.body).to have_tag('div#data-preloaded') do |element|
+          json = JSON.parse(element.current_scope.attribute('data-preloaded').value)
+          invite_info = JSON.parse(json['invite_info'])
+          expect(invite_info['existing_user_can_redeem']).to eq(false)
+        end
       end
 
-      it "redirects to root if a tries to view an invite meant for a specific email that is not the user's" do
+      it "does not allow the user to accept the invite when their email does not match the invite" do
         invite.update_columns(email: "notuseremail@discourse.org")
 
-        expect { get "/invites/#{invite.invite_key}" }.not_to change { InvitedUser.count }
+        get "/invites/#{invite.invite_key}"
+        expect(response.status).to eq(200)
 
-        expect(response).to redirect_to("/")
+        expect(response.body).to have_tag('div#data-preloaded') do |element|
+          json = JSON.parse(element.current_scope.attribute('data-preloaded').value)
+          invite_info = JSON.parse(json['invite_info'])
+          expect(invite_info['existing_user_can_redeem']).to eq(false)
+        end
       end
     end
 
@@ -848,8 +853,9 @@ RSpec.describe InvitesController do
       fab!(:invite) { Fabricate(:invite, email: nil, emailed_status: Invite.emailed_status_types[:not_required]) }
 
       it 'sends an activation email and does not activate the user' do
-        expect { put "/invites/show/#{invite.invite_key}.json", params: { email: 'test@example.com', password: 'verystrongpassword' } }
+        expect {
-          .not_to change { UserAuthToken.count }
+          put "/invites/show/#{invite.invite_key}.json", params: { email: 'test@example.com', password: 'verystrongpassword' }
+        }.not_to change { UserAuthToken.count }
 
         expect(response.status).to eq(200)
         expect(response.parsed_body['message']).to eq(I18n.t('invite.confirm_email'))
@@ -870,6 +876,24 @@ RSpec.describe InvitesController do
         expect(job_args['user_id']).to eq(invited_user.id)
         expect(EmailToken.hash_token(job_args['email_token'])).to eq(tokens.first.token_hash)
       end
+
+      it "does not automatically log in the user if their email matches an existing user's and shows an error" do
+        Fabricate(:user, email: 'test@example.com')
+        put "/invites/show/#{invite.invite_key}.json", params: { email: 'test@example.com', password: 'verystrongpassword' }
+        expect(session[:current_user_id]).to be_blank
+        expect(response.status).to eq(412)
+        expect(response.parsed_body['message']).to include("Primary email has already been taken")
+        expect(invite.reload.redemption_count).to eq(0)
+      end
+
+      it "does not automatically log in the user if their email matches an existing admin's and shows an error" do
+        Fabricate(:admin, email: 'test@example.com')
+        put "/invites/show/#{invite.invite_key}.json", params: { email: 'test@example.com', password: 'verystrongpassword' }
+        expect(session[:current_user_id]).to be_blank
+        expect(response.status).to eq(412)
+        expect(response.parsed_body['message']).to include("Primary email has already been taken")
+        expect(invite.reload.redemption_count).to eq(0)
+      end
     end
 
     context 'when new registrations are disabled' do
@@ -889,15 +913,75 @@ RSpec.describe InvitesController do
 
     context 'when user is already logged in' do
       fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }
-      fab!(:user) { sign_in(Fabricate(:user)) }
+      fab!(:user) { Fabricate(:user, email: 'test@example.com') }
+      fab!(:group) { Fabricate(:group) }
 
-      it 'does not redeem the invite' do
+      before { sign_in(user) }
+
+      it 'redeems the invitation and creates the invite accepted notification' do
         put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
         expect(response.status).to eq(200)
+        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
         invite.reload
-        expect(invite.invited_users).to be_blank
+        expect(invite.invited_users.first.user).to eq(user)
-        expect(invite.redeemed?).to be_falsey
+        expect(invite.redeemed?).to be_truthy
-        expect(response.body).to include(I18n.t('login.already_logged_in', current_user: user.username))
+        expect(
+          Notification.exists?(
+            user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
+          )
+        ).to eq(true)
+      end
+
+      it 'redirects to the first topic the user was invited to and creates the topic notification' do
+        topic = Fabricate(:topic)
+        TopicInvite.create!(invite: invite, topic: topic)
+        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+        expect(response.status).to eq(200)
+        expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+        expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
+      end
+
+      it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
+        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)
+
+        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+        expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+        invite.reload
+        expect(invite.redeemed?).to be_truthy
+        expect(user.reload.groups).to include(group)
+        expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
+      end
+
+      it "does not try to log in the user automatically" do
+        expect do
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+        end.not_to change { UserAuthToken.count }
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+      end
+
+      it "errors if the user's email doesn't match the invite email" do
+        user.update!(email: "blah@test.com")
+        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+        expect(response.status).to eq(412)
+        expect(response.parsed_body["message"]).to eq(I18n.t("invite.not_matching_email"))
+      end
+
+      it "errors if the user's email domain doesn't match the invite domain" do
+        user.update!(email: "blah@test.com")
+        invite.update!(email: nil, domain: "example.com")
+        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+        expect(response.status).to eq(412)
+        expect(response.parsed_body["message"]).to eq(I18n.t("invite.domain_not_allowed"))
       end
     end