Handle expired confirmation tokens

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index fdef4ea04de9da617499567b76330001d0c1b3ff..d89d483f2a72ab5f490a469307cff9daf33accaf 100644 (file)
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -304,10 +304,14 @@ class UserController < ApplicationController
   end
 
   def confirm
-    if request.post? && (token = UserToken.find_by_token(params[:confirm_string]))
-      if token.user.active?
+    if request.post?
+      token = UserToken.find_by_token(params[:confirm_string])
+      if token && token.user.active?
         flash[:error] = t('user.confirm.already active')
         redirect_to :action => 'login'
+      elsif !token || token.expired?
+        flash[:error] = t('user.confirm.unknown token')
+        redirect_to :action => 'confirm'
       else
         user = token.user
         user.status = "active"

diff --git a/app/models/user_token.rb b/app/models/user_token.rb
index 9a754d3442c5deeec13b6b35be7e6e59013dc772..3060b33ea17830012d0b65d864af56aa4dcbc989 100644 (file)
--- a/app/models/user_token.rb
+++ b/app/models/user_token.rb
@@ -5,6 +5,10 @@ class UserToken < ActiveRecord::Base
 
   after_initialize :set_defaults
 
+  def expired?
+    expiry < Time.now
+  end
+
 private
 
   def set_defaults

diff --git a/config/locales/en.yml b/config/locales/en.yml
index f3f4ac8dcf24085246eef5baff772f9f3ab7a4f4..9709b87781ccd27c05ff775237d5db3bc3dffc0f 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1880,7 +1880,7 @@ en:
       press confirm button: "Press the confirm button below to activate your account."
       button: Confirm
       already active: "This account has already been confirmed."
-      unknown token: "That token doesn't seem to exist."
+      unknown token: "That confirmation code has expired or does not exist."
       reconfirm_html: "If you need us to resend the confirmation email, <a href=\"%{reconfirm}\">click here</a>."
     confirm_resend:
       success: "We've sent a new confirmation note to %{email} and as soon as you confirm your account you'll be able to get mapping.<br /><br />If you use an antispam system which sends confirmation requests then please make sure you whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests."

diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb
index 8b223882940bef2d30df1b55ca413a37ee64f1cd..75fd34f0f1457851eb8264bbf73a3e36de933103 100644 (file)
--- a/test/functional/user_controller_test.rb
+++ b/test/functional/user_controller_test.rb
@@ -319,6 +319,30 @@ class UserControllerTest < ActionController::TestCase
     assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
   end
 
+  def test_user_confirm_expired_token
+    user = users(:inactive_user)
+    token = user.tokens.new
+    token.expiry = 1.day.ago
+    token.save!
+
+    @request.cookies["_osm_session"] = user.display_name
+    post :confirm, :confirm_string => token.token
+
+    assert_redirected_to :action => 'confirm'
+    assert_match /expired/, flash[:error]
+  end
+
+  def test_user_already_confirmed
+    user = users(:normal_user)
+    token = user.tokens.create
+
+    @request.cookies["_osm_session"] = user.display_name
+    post :confirm, :confirm_string => token.token
+
+    assert_redirected_to :action => 'login'
+    assert_match /confirmed/, flash[:error]
+  end
+
   def test_user_terms_new_user
     get :terms, {}, { "new_user" => User.new }
     assert_response :success