From 27d93b46582002bd07f672c8759a96771aa977f3 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 28 Nov 2022 15:15:00 +1000 Subject: [PATCH] FIX: Backport invite fixes from main (#19218) Backports the following: * 40e8912395ee2dc0f43c2b2a4504f4984c1f040d * bbcb69461fd85af0e1d883cfbc8240c200e748e9 Which were showing an error when users were trying to claim invites multiple times and a subsequent follow-up fix. --- .../discourse/app/controllers/invites-show.js | 3 + .../discourse/app/templates/invites/show.hbs | 2 +- app/controllers/invites_controller.rb | 10 +++ app/models/invite.rb | 5 ++ config/locales/client.en.yml | 1 - config/locales/server.en.yml | 2 + spec/models/invite_spec.rb | 89 ++++++++++++++++++- spec/requests/invites_controller_spec.rb | 32 +++++++ 8 files changed, 141 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/invites-show.js b/app/assets/javascripts/discourse/app/controllers/invites-show.js index 898f5963c2e..fef7f5b3319 100644 --- a/app/assets/javascripts/discourse/app/controllers/invites-show.js +++ b/app/assets/javascripts/discourse/app/controllers/invites-show.js @@ -31,6 +31,9 @@ export default Controller.extend( accountEmail: alias("email"), existingUserId: readOnly("model.existing_user_id"), existingUserCanRedeem: readOnly("model.existing_user_can_redeem"), + existingUserCanRedeemError: readOnly( + "model.existing_user_can_redeem_error" + ), existingUserRedeeming: bool("existingUserId"), hiddenEmail: alias("model.hidden_email"), emailVerifiedByLink: alias("model.email_verified_by_link"), diff --git a/app/assets/javascripts/discourse/app/templates/invites/show.hbs b/app/assets/javascripts/discourse/app/templates/invites/show.hbs index 35a788fca42..b222227f414 100644 --- a/app/assets/javascripts/discourse/app/templates/invites/show.hbs +++ b/app/assets/javascripts/discourse/app/templates/invites/show.hbs @@ -136,7 +136,7 @@ {{#if this.existingUserCanRedeem}} {{else}} -
{{i18n "invites.existing_user_cannot_redeem"}}
+
{{this.existingUserCanRedeemError}}
{{/if}} {{/if}} {{/if}} diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 57f30df171f..ba3c6c05700 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -391,6 +391,7 @@ class InvitesController < ApplicationController if current_user info[:existing_user_id] = current_user.id info[:existing_user_can_redeem] = invite.can_be_redeemed_by?(current_user) + info[:existing_user_can_redeem_error] = existing_user_can_redeem_error(invite) info[:email] = current_user.email info[:username] = current_user.username end @@ -467,4 +468,13 @@ class InvitesController < ApplicationController end end end + + def existing_user_can_redeem_error(invite) + return if invite.can_be_redeemed_by?(current_user) + if invite.invited_users.exists?(user: current_user) + I18n.t("invite.existing_user_already_redemeed") + else + I18n.t("invite.existing_user_cannot_redeem") + end + end end diff --git a/app/models/invite.rb b/app/models/invite.rb index 0377dfb0e2e..13d99a2b9e7 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -82,6 +82,10 @@ class Invite < ActiveRecord::Base !redeemed? && !expired? && !deleted_at? && !destroyed? && link_valid? end + def redeemed_by_user?(redeeming_user) + self.invited_users.exists?(user: redeeming_user) + end + def redeemed? if is_invite_link? redemption_count >= max_redemptions_allowed @@ -101,6 +105,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.email.blank? && self.domain.blank? return true if self.email.present? && email_matches?(user.email) self.domain.present? && domain_matches?(user.email) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4ff45e800e4..5d56d87aa7c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1989,7 +1989,6 @@ en: password_label: "Password" optional_description: "(optional)" 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}" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 270e3f4d159..cb05e87cf31 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -248,6 +248,8 @@ en:

Otherwise please Reset Password.

not_found_template_link: |

This invitation to %{site_name} can no longer be redeemed. Please ask the person who invited you to send you a new invitation.

+ existing_user_cannot_redeem: "This invitation cannot be redeemed. Please ask the person who invited you to send you a new invitation." + existing_user_already_redemeed: "You have already redeemed this invite link." user_exists: "There's no need to invite %{email}, they already have an account!" invite_exists: "You already invited %{email}." invalid_email: "%{email} isn't a valid email address." diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 56d09fe7413..99b9767ba54 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' 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" } @@ -481,4 +481,91 @@ describe Invite do expect(invite.warnings(admin.guardian)).to contain_exactly(I18n.t("invite.requires_groups", groups: group.name)) 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 d2564f68b2f..c3eaea4b9f7 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -117,6 +117,7 @@ describe InvitesController do 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) + expect(invite_info['existing_user_can_redeem_error']).to eq(I18n.t("invite.existing_user_cannot_redeem")) end end @@ -130,6 +131,37 @@ describe InvitesController do 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) + expect(invite_info['existing_user_can_redeem_error']).to eq(I18n.t("invite.existing_user_cannot_redeem")) + end + end + + it "does not allow the user to accept the invite when a multi-use invite link has already been redeemed by the user" do + invite.update!(email: nil, max_redemptions_allowed: 10) + expect(invite.redeem(redeeming_user: user)).not_to eq(nil) + + 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(false) + 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