diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 2ba373125..e5a2f04bd 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -212,8 +212,8 @@ class UserController < ApplicationController else redirect_to :controller => 'site', :action => 'index' end - elsif User.authenticate(:username => email_or_display_name, :password => pass, :pending => true) - flash.now[:error] = t 'user.login.account not active' + elsif user = User.authenticate(:username => email_or_display_name, :password => pass, :pending => true) + flash.now[:error] = t 'user.login.account not active', :reconfirm => url_for(:action => 'confirm_resend', :display_name => user.display_name) elsif User.authenticate(:username => email_or_display_name, :password => pass, :suspended => true) webmaster = link_to t('user.login.webmaster'), "mailto:webmaster@openstreetmap.org" flash.now[:error] = t 'user.login.account suspended', :webmaster => webmaster @@ -248,29 +248,54 @@ class UserController < ApplicationController def confirm if request.post? - token = UserToken.find_by_token(params[:confirm_string]) - if token and !token.user.active? - @user = token.user - @user.status = "active" - @user.email_valid = true - @user.save! - referer = token.referer - token.destroy - session[:user] = @user.id - unless referer.nil? - flash[:notice] = t('user.confirm.success') - redirect_to referer + if token = UserToken.find_by_token(params[:confirm_string]) + if token.user.active? + flash[:error] = t('user.confirm.already active') + redirect_to :action => 'login' else - flash[:notice] = t('user.confirm.success') + "

" + t('user.confirm.before you start') - redirect_to :action => 'account', :display_name => @user.display_name + user = token.user + user.status = "active" + user.email_valid = true + user.save! + referer = token.referer + token.destroy + session[:user] = user.id + + unless referer.nil? + flash[:notice] = t('user.confirm.success') + redirect_to referer + else + flash[:notice] = t('user.confirm.success') + "

" + t('user.confirm.before you start') + redirect_to :action => 'account', :display_name => user.display_name + end end else - flash[:error] = t('user.confirm.failure') - redirect_to :action => 'login', :display_name => @user.display_name + user = User.find_by_display_name(params[:display_name]) + + if user and user.active? + flash[:error] = t('user.confirm.already active') + elsif user + flash[:error] = t('user.confirm.unknown token') + t('user.confirm.reconfirm', :reconfirm => url_for(:action => 'confirm_resend', :display_name => params[:display_name])) + else + flash[:error] = t('user.confirm.unknown token') + end + + redirect_to :action => 'login' end end end + def confirm_resend + if user = User.find_by_display_name(params[:display_name]) + Notifier.deliver_signup_confirm(user, user.tokens.create) + flash[:notice] = t 'user.confirm_resend.success', :email => user.email + else + flash[:notice] = t 'user.confirm_resend.failure', :name => params[:display_name] + end + + redirect_to :action => 'login' + end + def confirm_email if request.post? token = UserToken.find_by_token(params[:confirm_string]) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index a1fde49b3..e6058d4b7 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -4,6 +4,7 @@ class Notifier < ActionMailer::Base subject I18n.t('notifier.signup_confirm.subject') body :url => url_for(:host => SERVER_URL, :controller => "user", :action => "confirm", + :display_name => user.display_name, :confirm_string => token.token) end diff --git a/app/views/user/confirm.html.erb b/app/views/user/confirm.html.erb index c7894380c..408ba771f 100644 --- a/app/views/user/confirm.html.erb +++ b/app/views/user/confirm.html.erb @@ -7,6 +7,7 @@ $("content").style.display = "none";

<%= t 'user.confirm.press confirm button' %>

+
diff --git a/config/locales/en.yml b/config/locales/en.yml index 573c5794b..4e633b27b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1500,7 +1500,7 @@ en: remember: "Remember me:" lost password link: "Lost your password?" login_button: "Login" - account not active: "Sorry, your account is not active yet.
Please click on the link in the account confirmation email to activate your account." + account not active: "Sorry, your account is not active yet.
Please use the link in the account confirmation email to activate your account, or request a new confirmation email." account suspended: Sorry, your account has been suspended due to suspicious activity.
Please contact the {{webmaster}} if you wish to discuss this. webmaster: webmaster auth failure: "Sorry, could not log in with those details." @@ -1663,7 +1663,12 @@ en: button: Confirm success: "Confirmed your account, thanks for signing up!" before you start: "We know you're probably in a hurry to start mapping, but before you do you might like to fill in some more information about yourself in the form below." - failure: "A user account with this token has already been confirmed." + already active: "This account has already been confirmed." + unknown token: "That token doesn't seem to exist." + reconfirm: "If it's been a while since you signed up you might need to send yourself a new confirmation email." + 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.

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." + failure: "User {{name}} not found." confirm_email: heading: Confirm a change of email address press confirm button: "Press the confirm button below to confirm your new email address." diff --git a/config/routes.rb b/config/routes.rb index 4608fb32c..825413ff3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,6 +103,8 @@ ActionController::Routing::Routes.draw do |map| map.connect '/user/new', :controller => 'user', :action => 'new' map.connect '/user/terms', :controller => 'user', :action => 'terms' map.connect '/user/save', :controller => 'user', :action => 'save' + map.connect '/user/:display_name/confirm/resend', :controller => 'user', :action => 'confirm_resend' + map.connect '/user/:display_name/confirm', :controller => 'user', :action => 'confirm' map.connect '/user/confirm', :controller => 'user', :action => 'confirm' map.connect '/user/confirm-email', :controller => 'user', :action => 'confirm_email' map.connect '/user/go_public', :controller => 'user', :action => 'go_public' diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index 77d73552a..01a7ca649 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -104,7 +104,7 @@ class UserCreationTest < ActionController::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct - confirm_regex = Regexp.new("/user/confirm\\?confirm_string=([a-zA-Z0-9]*)") + confirm_regex = Regexp.new("/user/redirect_tester/confirm\\?confirm_string=([a-zA-Z0-9]*)") assert_match(confirm_regex, register_email.body) confirm_string = confirm_regex.match(register_email.body)[1]