Merge pull request #5809 from techAPJ/invite-redeem-fixes

FIX: better handling of invite links after they are redeemed

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

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
 

app/views/invites/show.html.erb

@@ -1,7 +1,7 @@
 <div id='simple-container'>
   <%if flash[:error]%>
     <div class='alert alert-error'>
-      <%=flash[:error]%>
+      <%=flash[:error].html_safe%>
     </div>
   <%end%>
 </div>

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: |
+      <p>Your invite to <a href="%{base_url}">%{site_name}</a> has already been redeemed.</p>
+
+      <p>If you remember your password you can <a href="%{base_url}/login">Login</a>.</p>
+
+      <p>Otherwise please <a href="%{base_url}/password-reset">Reset Password</a>.</p>
     user_exists: "There's no need to invite <b>%{email}</b>, they <a href='/u/%{username}/summary'>already have an account!</a>"
 
   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."
 

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

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

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

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
 

spec/models/invite_spec.rb

@@ -301,25 +301,8 @@ describe Invite do
         end
 
         context 'again' do
-          context "without a passthrough" do
+          it 'will not redeem twice' do
-            before do
+            expect(invite.redeem).to be_blank
-              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
           end
         end
       end

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