From 83245aa5084cefd13c5af1d28ab734d63536b06c Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 8 May 2018 20:12:58 +0530 Subject: [PATCH] FIX: better handling of invite links after they are redeemed FIX: deprecate invite_passthrough_hours setting --- app/controllers/invites_controller.rb | 4 +- app/models/invite_redeemer.rb | 6 --- app/views/invites/show.html.erb | 2 +- config/locales/server.en.yml | 7 +++- config/site_settings.yml | 1 - ...8142711_remove_invite_passthrough_hours.rb | 5 +++ spec/controllers/invites_controller_spec.rb | 27 ------------- spec/models/invite_redeemer_spec.rb | 1 - spec/models/invite_spec.rb | 21 +--------- spec/requests/invites_controller_spec.rb | 40 +++++++++++++++++++ 10 files changed, 56 insertions(+), 58 deletions(-) create mode 100644 db/migrate/20180508142711_remove_invite_passthrough_hours.rb create mode 100644 spec/requests/invites_controller_spec.rb diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 8773a349705..10933affcfe 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -19,7 +19,7 @@ class InvitesController < ApplicationController invite = Invite.find_by(invite_key: params[:id]) - if invite.present? + if invite.present? && !invite.redeemed? store_preloaded("invite_info", MultiJson.dump( invited_by: UserNameSerializer.new(invite.invited_by, scope: guardian, root: false), email: invite.email, @@ -28,7 +28,7 @@ class InvitesController < ApplicationController render layout: 'application' else - flash.now[:error] = I18n.t('invite.not_found') + flash.now[:error] = I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url) render layout: 'no_ember' end end diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 5bb6d0f1efe..904bf56bb07 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -8,12 +8,6 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end end - # If `invite_passthrough_hours` is defined, allow them to re-use the invite link - # to login again. - if invite.redeemed_at && invite.redeemed_at >= SiteSetting.invite_passthrough_hours.hours.ago - return get_existing_user - end - nil end diff --git a/app/views/invites/show.html.erb b/app/views/invites/show.html.erb index cc4b0acc1e7..125a9c775ef 100644 --- a/app/views/invites/show.html.erb +++ b/app/views/invites/show.html.erb @@ -1,7 +1,7 @@
<%if flash[:error]%>
- <%=flash[:error]%> + <%=flash[:error].html_safe%>
<%end%>
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f0602de0122..fb8ba2c4e42 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -171,6 +171,12 @@ en: invite: not_found: "Your invite token is invalid. Please contact the site's administrator." + not_found_template: | +

Your invite to %{site_name} has already been redeemed.

+ +

If you remember your password you can Login.

+ +

Otherwise please Reset Password.

user_exists: "There's no need to invite %{email}, they already have an account!" bulk_invite: @@ -1204,7 +1210,6 @@ en: new_version_emails: "Send an email to the contact_email address when a new version of Discourse is available." invite_expiry_days: "How long user invitation keys are valid, in days" - invite_passthrough_hours: "How long a user can use a previously redeemed invitation key to log in, in hours" invite_only: "Public registration is disabled, all new users must be explicitly invited by staff." diff --git a/config/site_settings.yml b/config/site_settings.yml index a397c4b2581..649cab0d98d 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -408,7 +408,6 @@ users: default: true invite_expiry_days: default: 30 - invite_passthrough_hours: 0 invites_per_page: client: true default: 40 diff --git a/db/migrate/20180508142711_remove_invite_passthrough_hours.rb b/db/migrate/20180508142711_remove_invite_passthrough_hours.rb new file mode 100644 index 00000000000..76f8f1f4634 --- /dev/null +++ b/db/migrate/20180508142711_remove_invite_passthrough_hours.rb @@ -0,0 +1,5 @@ +class RemoveInvitePassthroughHours < ActiveRecord::Migration[5.1] + def change + execute "DELETE FROM site_settings WHERE name = 'invite_passthrough_hours'" + end +end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 777a6777403..6433a2de7fd 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -2,33 +2,6 @@ require 'rails_helper' describe InvitesController do - context '.show' do - render_views - - it "shows error if invite not found" do - get :show, params: { id: 'nopeNOPEnope' } - - expect(response).to be_success - - body = response.body - - expect(body).to_not have_tag(:script, with: { src: '/assets/application.js' }) - expect(CGI.unescapeHTML(body)).to include(I18n.t('invite.not_found')) - end - - it "renders the accept invite page if invite exists" do - i = Fabricate(:invite) - get :show, params: { id: i.invite_key } - - expect(response).to be_success - - body = response.body - - expect(body).to have_tag(:script, with: { src: '/assets/application.js' }) - expect(CGI.unescapeHTML(body)).to_not include(I18n.t('invite.not_found')) - end - end - context '.destroy' do it 'requires you to be logged in' do diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb index dea7ea2945d..ac31409b522 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -130,7 +130,6 @@ describe InviteRedeemer do end it "only allows one user to be created per invite" do - SiteSetting.invite_passthrough_hours = 4800 user = invite_redeemer.redeem invite.reload diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index af5dc736eeb..f0411f4b1c6 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -301,25 +301,8 @@ describe Invite do end context 'again' do - context "without a passthrough" do - before do - SiteSetting.invite_passthrough_hours = 0 - end - - it 'will not redeem twice' do - expect(invite.redeem).to be_blank - end - end - - context "with a passthrough" do - before do - SiteSetting.invite_passthrough_hours = 1 - end - - it 'will not redeem twice' do - expect(invite.redeem).to be_present - expect(invite.redeem.email).to eq(user.email) - end + it 'will not redeem twice' do + expect(invite.redeem).to be_blank end end end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb new file mode 100644 index 00000000000..4f622c196ab --- /dev/null +++ b/spec/requests/invites_controller_spec.rb @@ -0,0 +1,40 @@ +require 'rails_helper' + +describe InvitesController do + + context 'show' do + let(:invite) { Fabricate(:invite) } + let(:user) { Fabricate(:coding_horror) } + + it "returns error if invite not found" do + get "/invites/nopeNOPEnope" + + expect(response).to be_success + + body = response.body + expect(body).to_not have_tag(:script, with: { src: '/assets/application.js' }) + expect(CGI.unescapeHTML(body)).to include(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)) + end + + it "renders the accept invite page if invite exists" do + get "/invites/#{invite.invite_key}" + + expect(response).to be_success + + body = response.body + expect(body).to have_tag(:script, with: { src: '/assets/application.js' }) + expect(CGI.unescapeHTML(body)).to_not include(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)) + end + + it "returns error if invite has already been redeemed" do + invite.update_attributes!(redeemed_at: 1.day.ago) + get "/invites/#{invite.invite_key}" + + expect(response).to be_success + + body = response.body + expect(body).to_not have_tag(:script, with: { src: '/assets/application.js' }) + expect(CGI.unescapeHTML(body)).to include(I18n.t('invite.not_found_template', site_name: SiteSetting.title, base_url: Discourse.base_url)) + end + end +end