Handle expired confirmation tokens

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 request.post?
-      if token.user.active?
+      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"

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

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."

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