FIX: Existing users were mistakenly unable to redeem invite (#19191)

Follow up to 40e8912395

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.
This commit is contained in:
Martin Brennan 2022-11-25 11:57:04 +10:00 committed by GitHub
parent 755ca0fcbb
commit bbcb69461f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 103 additions and 1 deletions

View File

@ -114,6 +114,7 @@ class Invite < ActiveRecord::Base
def can_be_redeemed_by?(user) def can_be_redeemed_by?(user)
return false if !self.redeemable? return false if !self.redeemable?
return false if redeemed_by_user?(user) 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) return true if self.email.present? && email_matches?(user.email)
self.domain.present? && domain_matches?(user.email) self.domain.present? && domain_matches?(user.email)
end end

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.describe Invite do RSpec.describe Invite do
fab!(:user) { Fabricate(:user) } fab!(:user) { Fabricate(:user, email: "existinguser@invitetest.com") }
let(:xss_email) { "<b onmouseover=alert('wufff!')>email</b><script>alert('test');</script>@test.com" } let(:xss_email) { "<b onmouseover=alert('wufff!')>email</b><script>alert('test');</script>@test.com" }
let(:escaped_email) { "&lt;b onmouseover=alert(&#39;wufff!&#39;)&gt;email&lt;/b&gt;&lt;script&gt;alert(&#39;test&#39;);&lt;/script&gt;@test.com" } let(:escaped_email) { "&lt;b onmouseover=alert(&#39;wufff!&#39;)&gt;email&lt;/b&gt;&lt;script&gt;alert(&#39;test&#39;);&lt;/script&gt;@test.com" }
@ -469,4 +469,91 @@ RSpec.describe Invite do
expect(invite.invalidated_at).to be_nil expect(invite.invalidated_at).to be_nil
end end
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 end

View File

@ -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")) expect(invite_info['existing_user_can_redeem_error']).to eq(I18n.t("invite.existing_user_already_redemeed"))
end end
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 end
it 'fails if invite does not exist' do it 'fails if invite does not exist' do