From bbcb69461fd85af0e1d883cfbc8240c200e748e9 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 25 Nov 2022 11:57:04 +1000 Subject: [PATCH] FIX: Existing users were mistakenly unable to redeem invite (#19191) Follow up to 40e8912395ee2dc0f43c2b2a4504f4984c1f040d In this previous commit I introduced a bug that prevented a legitimate case for an existing user to redeem an invite, where the email/domain were both blank and the invite was still redeemable by the user. Fixes the issue and adds more specs for that case. --- app/models/invite.rb | 1 + spec/models/invite_spec.rb | 89 +++++++++++++++++++++++- spec/requests/invites_controller_spec.rb | 14 ++++ 3 files changed, 103 insertions(+), 1 deletion(-) diff --git a/app/models/invite.rb b/app/models/invite.rb index 48b0efe072..2712760ab1 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -114,6 +114,7 @@ class Invite < ActiveRecord::Base def can_be_redeemed_by?(user) return false if !self.redeemable? return false if redeemed_by_user?(user) + return true if self.domain.blank? && self.email.blank? return true if self.email.present? && email_matches?(user.email) self.domain.present? && domain_matches?(user.email) end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 0a1f9de9f3..815feef803 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe Invite do - fab!(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user, email: "existinguser@invitetest.com") } let(:xss_email) { "email@test.com" } let(:escaped_email) { "<b onmouseover=alert('wufff!')>email</b><script>alert('test');</script>@test.com" } @@ -469,4 +469,91 @@ RSpec.describe Invite do expect(invite.invalidated_at).to be_nil end end + + describe "#can_be_redeemed_by?" do + context "for invite links" do + fab!(:invite) { Fabricate(:invite, email: nil, domain: nil, max_redemptions_allowed: 1) } + + it "returns false if invite is already redeemed" do + invite.update!(redemption_count: 1) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if the invite is expired" do + invite.update!(expires_at: 10.days.ago) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if invite is deleted" do + invite.trash! + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if invite is invalidated" do + invite.update!(invalidated_at: 1.day.ago) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if the user already redeemed it" do + InvitedUser.create(user: user, invite: invite) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if domain does not match user email" do + invite.update!(domain: "zzzzz.com") + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns true if domain does match user email" do + invite.update!(domain: "invitetest.com") + expect(invite.can_be_redeemed_by?(user)).to eq(true) + end + + it "returns true by default if all other conditions are met and domain and invite are blank" do + expect(invite.can_be_redeemed_by?(user)).to eq(true) + end + end + + context "for email invites" do + fab!(:invite) do + invite = Fabricate(:invite, email: "otherexisting@invitetest.com", domain: nil) + user.update!(email: "otherexisting@invitetest.com") + invite + end + + it "returns false if invite is already redeemed" do + InvitedUser.create(user: Fabricate(:user), invite: invite) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if the invite is expired" do + invite.update!(expires_at: 10.days.ago) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if invite is deleted" do + invite.trash! + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if invite is invalidated" do + invite.update!(invalidated_at: 1.day.ago) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if the user already redeemed it" do + InvitedUser.create(user: user, invite: invite) + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns false if email does not match user email" do + invite.update!(email: "blahblah@test.com") + expect(invite.can_be_redeemed_by?(user)).to eq(false) + end + + it "returns true if email does match user email" do + expect(invite.can_be_redeemed_by?(user)).to eq(true) + end + end + end end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index cebeeab647..60dc16785d 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -158,6 +158,20 @@ RSpec.describe InvitesController do expect(invite_info['existing_user_can_redeem_error']).to eq(I18n.t("invite.existing_user_already_redemeed")) end end + + it "allows the user to accept the invite when its an invite link that they have not redeemed" do + invite.update!(email: nil, max_redemptions_allowed: 10) + + get "/invites/#{invite.invite_key}" + expect(response.status).to eq(200) + + 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_id']).to eq(user.id) + expect(invite_info['existing_user_can_redeem']).to eq(true) + end + end end it 'fails if invite does not exist' do