From f25eda13fa76a817731457e62b277b423fd2d9c7 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 26 May 2021 09:47:44 +0100 Subject: [PATCH] FIX: Make UI match server behavior for external-auth invites (#13113) There are two methods which the server uses to verify an invite is being redeemed with a matching email: 1) The email token, supplied via a `?t=` parameter 2) The validity of the email, as provided by the auth provider Only one of these needs to be true for the invite to be redeemed successfully on the server. The frontend logic was previously only checking (2). This commit updates the frontend logic to match the server. This commit does not affect the invite redemption logic. It only affects the 'show' endpoint, and the UI. --- .../discourse/app/controllers/invites-show.js | 18 ++++-- .../tests/acceptance/invite-accept-test.js | 59 ++++++++++++++++++- app/controllers/invites_controller.rb | 8 ++- config/locales/client.en.yml | 1 + spec/requests/invites_controller_spec.rb | 16 +++++ 5 files changed, 96 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/invites-show.js b/app/assets/javascripts/discourse/app/controllers/invites-show.js index 3c3658b06fc..49f9f7998be 100644 --- a/app/assets/javascripts/discourse/app/controllers/invites-show.js +++ b/app/assets/javascripts/discourse/app/controllers/invites-show.js @@ -29,6 +29,7 @@ export default Controller.extend( invitedBy: readOnly("model.invited_by"), email: alias("model.email"), hiddenEmail: alias("model.hidden_email"), + emailVerifiedByLink: alias("model.email_verified_by_link"), accountUsername: alias("model.username"), passwordRequired: notEmpty("accountPassword"), successMessage: null, @@ -127,14 +128,16 @@ export default Controller.extend( "rejectedEmails.[]", "authOptions.email", "authOptions.email_valid", - "hiddenEmail" + "hiddenEmail", + "emailVerifiedByLink" ) emailValidation( email, rejectedEmails, externalAuthEmail, externalAuthEmailValid, - hiddenEmail + hiddenEmail, + emailVerifiedByLink ) { if (hiddenEmail) { return EmberObject.create({ @@ -157,12 +160,12 @@ export default Controller.extend( }); } - if (externalAuthEmail) { + if (externalAuthEmail && externalAuthEmailValid) { const provider = this.createAccount.authProviderDisplayName( this.get("authOptions.auth_provider") ); - if (externalAuthEmail === email && externalAuthEmailValid) { + if (externalAuthEmail === email) { return EmberObject.create({ ok: true, reason: I18n.t("user.email.authenticated", { @@ -179,6 +182,13 @@ export default Controller.extend( } } + if (emailVerifiedByLink) { + return EmberObject.create({ + ok: true, + reason: I18n.t("user.email.authenticated_by_invite"), + }); + } + if (emailValid(email)) { return EmberObject.create({ ok: true, diff --git a/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js b/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js index 73ec2d4c00c..ea41e3b5048 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/invite-accept-test.js @@ -22,7 +22,7 @@ function setAuthenticationData(hooks, json) { }); } -function preloadInvite({ link = false } = {}) { +function preloadInvite({ link = false, email_verified_by_link = false } = {}) { const info = { invited_by: { id: 123, @@ -32,6 +32,7 @@ function preloadInvite({ link = false } = {}) { title: "team", }, username: "invited", + email_verified_by_link: email_verified_by_link, }; if (link) { @@ -360,3 +361,59 @@ acceptance( }); } ); + +acceptance( + "Email Invite link with valid authentication data, valid email token, unverified authentication email", + function (needs) { + needs.settings({ enable_local_logins: false }); + + setAuthenticationData(needs.hooks, { + auth_provider: "facebook", + email: "foobar@example.com", + email_valid: false, + username: "foobar", + name: "barfoo", + }); + + test("confirm form and buttons", async function (assert) { + preloadInvite({ email_verified_by_link: true }); + + await visit("/invites/myvalidinvitetoken"); + + assert.ok(!exists("#new-account-email"), "does not show email field"); + + assert.equal( + queryAll("#account-email-validation").text().trim(), + I18n.t("user.email.authenticated_by_invite") + ); + }); + } +); + +acceptance( + "Email Invite link with valid authentication data, no email token, unverified authentication email", + function (needs) { + needs.settings({ enable_local_logins: false }); + + setAuthenticationData(needs.hooks, { + auth_provider: "facebook", + email: "foobar@example.com", + email_valid: false, + username: "foobar", + name: "barfoo", + }); + + test("confirm form and buttons", async function (assert) { + preloadInvite({ email_verified_by_link: false }); + + await visit("/invites/myvalidinvitetoken"); + + assert.ok(!exists("#new-account-email"), "does not show email field"); + + assert.equal( + queryAll("#account-email-validation").text().trim(), + I18n.t("user.email.ok") + ); + }); + } +); diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 722fa3572eb..94d889c7ad9 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -31,6 +31,11 @@ class InvitesController < ApplicationController 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 info = { @@ -38,7 +43,8 @@ class InvitesController < ApplicationController email: email, hidden_email: hidden_email, username: hidden_email ? '' : UserNameSuggester.suggest(invite.email), - is_invite_link: invite.is_invite_link? + is_invite_link: invite.is_invite_link?, + email_verified_by_link: email_verified_by_link } if staged_user = User.where(staged: true).with_email(invite.email).first diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ad257fd1bd8..a5c6b13662e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1311,6 +1311,7 @@ en: invalid: "Please enter a valid email address" authenticated: "Your email has been authenticated by %{provider}" invite_auth_email_invalid: "Your invitation email does not match the email authenticated by %{provider}" + authenticated_by_invite: "Your email has been authenticated by the invitation" frequency_immediately: "We'll email you immediately if you haven't read the thing we're emailing you about." frequency: one: "We'll only email you if we haven't seen you in the last minute." diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 6dc26b8a240..0858261d1a1 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -41,6 +41,22 @@ describe InvitesController do end end + it 'includes token validity boolean' do + get "/invites/#{invite.invite_key}" + 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['email_verified_by_link']).to eq(false) + end + + get "/invites/#{invite.invite_key}?t=#{invite.email_token}" + 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['email_verified_by_link']).to eq(true) + end + end + it 'fails for logged in users' do sign_in(Fabricate(:user))