@@ -114,10 +114,13 @@ class AccountController < ApplicationController email = user[:email] reset_password = User.reset_password(email,username) if reset_password[0] - Notifier.deliver_forgot_password_email(username,reset_password[1],reset_password[2]) - flash[:notice]="A new password has been emailed to you."[:new_password_emailed] + new_password=reset_password[1] + user=reset_password[2] + email=user.email + Notifier.deliver_forgot_password_email(new_password,user) + flash.now[:notice]="A new password has been emailed to you at #{email}."[:new_password_emailed] else - flash[:notice]=reset_password[1] + flash.now[:notice]=reset_password[1] end end end @@ -182,10 +185,10 @@ class AccountController < ApplicationController protected def password_authentication(username, password) user = User.authenticate(username,password) - unless user.nil? - successful_login(user) + if user[0] + successful_login(user[1]) else - failed_login "Invalid login or password"[] + failed_login(user[1]) end end @@ -220,7 +223,6 @@ class AccountController < ApplicationController def successful_login(user, new_openid_user = false) set_current_user(user) flash[:notice] = "Logged in successfully"[:logged_in] - # TODO - user.failed_logins = 0; user.save # could catch the fact that they are a new openid user here and redirect somewhere else if you wanted if user.is_admin? && ( session[:return_to].nil? || session[:return_to].empty?) # if we're an admin we STILL would love a return, thank you very much! redirect_to :controller => 'admin', :action => 'index', :protocol => "http://" @@ -230,7 +232,6 @@ class AccountController < ApplicationController end def failed_login(message) - # TODO - user.failed_logins += 1; user.save # TODO - send an email to an admin if user.failed_logins > 10 # Smells like a dictionary attack! flash[:warning] = message redirect_to :action => 'login' app/controllers/account_controller.rb @@ -3,11 +3,11 @@ class Notifier < ActionMailer::Base @@from = $WEBSITE_EMAIL_FROM_ADDRESS @@test_recipient = "junk@example.com" # testing only if needed - def forgot_password_email(username,password,email) + def forgot_password_email(new_password,user) subject "EOL Forgot Password" - recipients email + recipients user.email from @@from - body :username => username,:password=>password + body :user => user,:new_password=>new_password end def agent_forgot_password_email(agent, new_password) app/models/notifier.rb @@ -21,7 +21,7 @@ class User < ActiveRecord::Base before_save {|obj| obj.credentials = '' if obj.credentials.nil?} # TODO Move this into the check_curator_status before_save method validates_presence_of :username, :if => :not_openid? - validates_length_of :username, :within => 4..16, :if => :not_openid? + validates_length_of :username, :within => 4..32, :if => :not_openid? validates_length_of :entered_password, :within => 4..16, :if => :not_openid?, :on => :create validates_presence_of :given_name @@ -143,23 +143,46 @@ class User < ActiveRecord::Base end def self.authenticate(username,password) - user = User.find_by_username_and_active(username,true) - # if we don't have any matching username for an active account, return nothing - return nil if user.blank? + # try username first + user = User.find_by_username_and_active(username,true) + if !user.blank? && user.hashed_password==User.hash_password(password) + user.reset_login_attempts # found a matching username and password matched! + return true,user + elsif !user.blank? # found a matching username, but password didn't match! + user.invalid_login_attempt + return false,"Invalid login or password"[] + end + + # no match with username, next try email address, which is not necessarily unique in database + users=User.find_all_by_email_and_active(username,true) + return false,"Invalid login or password"[] if users.blank? # no email match either, returning nothing + + users.each do |u| # check all users with matching email addresses to see if one of them matches the password + if u.hashed_password==User.hash_password(password) + u.reset_login_attempts # found a match with email and password + return true,u + else + u.invalid_login_attempt # log the bad attempt for this user! + end + end - # if we have a matching username active account, confirm the password too - if user.hashed_password==User.hash_password(password) - user.update_attributes(:failed_login_attempts=>0) # reset the user's failed login attempts - return user + if users.size > 1 + return false,"The email address is not unique - you must enter a username"[] # more than 1 email address with no matching passwords else - # user matched but not password, so log a failed login attempt and return nothing - user.update_attributes(:failed_login_attempts=>user.failed_login_attempts+1) - return nil + return false,"Invalid login or password"[] # no matches yet again :( end end + def reset_login_attempts + self.update_attributes(:failed_login_attempts=>0) # reset the user's failed login attempts + end + + def invalid_login_attempt + self.update_attributes(:failed_login_attempts=>self.failed_login_attempts+1) + end + # I wanted to centralize this call, so we can quickly change from one kind of hashing to another. def self.hash_password(raw) Digest::MD5.hexdigest(raw) @@ -191,7 +214,7 @@ class User < ActiveRecord::Base 8.times { new_password << chars[rand(chars.size)] } new_guy[0].password = new_password if new_guy[0].save - return true, new_password, new_guy[0].email + return true, new_password, new_guy[0] else return false, "Sorry, a problem occurred updating your account - please try again later."[:problem_updating_account] end app/models/user.rb @@ -13,7 +13,7 @@ - <%= "Enter the email address OR username (or both) you registered with to retrieve your password"[:forgot_password_message] %>: + <%= "Enter the email address OR username (or both) you registered with to reset your password"[:forgot_password_message] %>: <% form_for :user do |form| -%> <fieldset> app/views/account/forgot_password.html.erb @@ -24,14 +24,14 @@ <% form_for :user, :url => {:action=>'authenticate'} do |form| -%> <fieldset> - <label for="user_username"><%= "Username"[] %></label> + <label for="user_username"><%= "Username"[] %> <%="or"[]%> <%= "Email Address"[] %>:</label> <%= form.text_field 'username', :size=>'16',:maxlength=>'30' %> - - <label for="user_password"><%= "Password"[] %></label> + + <label for="user_password"><%= "Password"[] %>:</label> <%= form.password_field 'password', :size=>'16',:maxlength=>'30' %> - - or - + + <%="or"[]%> + <div id="openid"> <label for="openid_url"><%= "OpenID"[] %></label> <%= text_field_tag 'openid_url', {},{:size=>100,:maxlength=>250,:class=>'openid'} %> app/views/account/login.html.erb @@ -33,7 +33,9 @@ function check_passwords() { <div id="page-content" class="clearfix">  <div id="full-page-content"> - <%= eol_lang_error_messages_for :user %> + + <%=eol_lang_error_messages_for :user if request.post?%> + <% form_for :user, :html=>{:class=>"warning-warn invalid-invalid styleLabelOnErr"} do |f| -%> @@ -41,9 +43,9 @@ function check_passwords() { <legend><%= "Account Information"[] %></legend> - <label>Username:</label> -  - <%= f.text_field :username, {:maxlength=>16, :onblur=>'JavaScript:check_username();'} %> + <label>Username: (4 - 32 characters)</label> +  + <%= f.text_field :username, {:maxlength=>32, :onblur=>'JavaScript:check_username();'} %> <% if @user.openid? %> @@ -53,7 +55,7 @@ function check_passwords() { <% else %> <%="Only enter a new password if you wish to change it."[:only_enter_new_password_if_needed]%> - <label for="password"><%="New Password"[]%></label> + <label for="password"><%="New Password"[]%> (4 - 16 characters)</label> <%= f.password_field :entered_password,{:maxlength=>16} %> <label for="password_confirmation"><%="Confirm New Password"[]%></label> app/views/account/profile.html.erb @@ -51,9 +51,9 @@ <%=@verification_did_not_match%> - <label for="user_username" title="<%= "Please enter your username"[] %>"><%= "Username"[] %> (4 - 16 characters)* (<%= "Required"[] %>)</label> -  - <%= f.text_field :username, {:maxlength=>16, :onblur=>'JavaScript:check_username();'} %> + <label for="user_username" title="<%= "Please enter your username"[] %>"><%= "Username"[] %> (4 - 32 characters)* (<%= "Required"[] %>)</label> +  + <%= f.text_field :username, {:maxlength=>32, :onblur=>'JavaScript:check_username();'} %> <label for="user_entered_password" title="<%= "Please enter a password"[] %>"><%= "Password"[] %> (4 - 16 characters)* (<%= "Required"[] %>)</label> app/views/account/signup.html.erb @@ -1,11 +1,10 @@ - <%= eol_lang_error_messages_for :user %> - + <table> <tr> <td>Username:</td> - <td><%=f.text_field :username,{:size=>'50',:maxlength=>'16'}%></td> + <td><%=f.text_field :username,{:size=>'50',:maxlength=>'32'}%></td> </tr> <% if @user.openid?%> <tr> app/views/administrator/user/_form.html.erb @@ -2,5 +2,6 @@ You have indicated that you have forgotten your password. A new password has b for you and is shown below. Please log into the website at http://www.eol.org/login and then visit your profile page at http://www.eol.org/profile to change your password. -Username: <%=@username%> -Password: <%=@password%> \ No newline at end of file +Username: <%=@user.username%> +Email: <%=@user.email%> +Password: <%=@new_password%> \ No newline at end of file app/views/notifier/forgot_password_email.html.erb @@ -64,7 +64,7 @@ ActionController::Routing::Routes.draw do |map| map.external_link 'external_link',:controller=>'application', :action=>'external_link' - map.root :controller => 'content' + map.root :controller => 'content', :action=>'index' map.search 'search', :controller => 'taxa', :action => 'search' map.connect 'search/:id', :controller => 'taxa', :action => 'search' @@ -82,7 +82,16 @@ ActionController::Routing::Routes.draw do |map| map.connect '/taxon_concepts/:taxon_concept_id/comments/', :controller => 'comments', :action => 'index', :conditions => {:method => :get} map.connect '/taxon_concepts/:taxon_concept_id/comments/', :controller => 'comments', :action => 'create', :conditions => {:method => :post} + + map.admin 'admin', :controller=>'admin', :action=>'index' + map.admin 'content_partner', :controller=>'content_partner', :action=>'index' + + # this represents a URL with just a random namestring -- send to search page (e.g. www.eol.org/animalia) + map.connect ':id', :controller=>'taxa', :action=>'search' + # Install the default routes as the lowest priority. map.connect ':controller/:action/:id' map.connect ':controller/:action/:id.:format' + + end config/routes.rb @@ -342,8 +342,8 @@ forgot_password_message: Enter the email address or username (or both) you regis could_not_locate_account: Sorry, but we could not locate your account. #Sorry, but your email address is not unique - you must also specify a username. must_specify_username_too: Sorry, but your email address is not unique - you must also specify a username. -# A new password has been emailed to you. -new_password_emailed: A new password has been emailed to you. +# A new password has been emailed to you at {email}. +new_password_emailed: A new password has been emailed to you at {email}. #Sorry, a problem occurred updating your account - please try again later. problem_updating_account: Sorry, a problem occurred updating your account - please try again later. taxon_group_of_interest: Taxon group of interest lang/de.yml @@ -346,8 +346,8 @@ forgot_password_message: Enter the email address or username (or both) you regis could_not_locate_account: Sorry, but we could not locate your account. #Sorry, but your email address is not unique - you must also specify a username. must_specify_username_too: Sorry, but your email address is not unique - you must also specify a username. -# A new password has been emailed to you. -new_password_emailed: A new password has been emailed to you. +# A new password has been emailed to you at {email}. +new_password_emailed: A new password has been emailed to you at {email}.. #Sorry, a problem occurred updating your account - please try again later. problem_updating_account: Sorry, a problem occurred updating your account - please try again later. taxon_group_of_interest: Taxon group of interest lang/fr.yml @@ -346,8 +346,8 @@ forgot_password_message: Enter the email address or username (or both) you regis could_not_locate_account: Sorry, but we could not locate your account. #Sorry, but your email address is not unique - you must also specify a username. must_specify_username_too: Sorry, but your email address is not unique - you must also specify a username. -# A new password has been emailed to you. -new_password_emailed: Новый пароль был отправлен на ваш e-mail адрес. +# A new password has been emailed to you at {email}.. +new_password_emailed: Новый пароль был отправлен на ваш e-mail адрес {email}. #Sorry, a problem occurred updating your account - please try again later. problem_updating_account: Sorry, a problem occurred updating your account - please try again later. taxon_group_of_interest: Taxon group of interest lang/ru.yml @@ -396,8 +396,8 @@ forgot_password_message: Enter the email address or username (or both) you regis could_not_locate_account: Sorry, but we could not locate your account. #Sorry, but your email address is not unique - you must also specify a username. must_specify_username_too: Sorry, but your email address is not unique - you must also specify a username. -# A new password has been emailed to you. -new_password_emailed: A new password has been emailed to you. +# A new password has been emailed to you at {email}. +new_password_emailed: A new password has been emailed to you at {email}. #Sorry, a problem occurred updating your account - please try again later. problem_updating_account: Sorry, a problem occurred updating your account - please try again later. taxon_group_of_interest: Taxon group of interest lang/translation_template.yml @@ -7,16 +7,35 @@ describe User do @user.should_not be_a_new_record end - it 'should authenticate existing user with correct password' do - User.authenticate( @user.username, @user.password ).id.should == @user.id + it 'should authenticate existing user with correct password, returning true and user back' do + success,user=User.authenticate( @user.username, @user.password ) + success.should be_true + user.id.should == @user.id + end + + it 'should authenticate existing user with correct email address and password, returning true and user back' do + success,user=User.authenticate( @user.email, @user.password ) + success.should be_true + user.id.should == @user.id end - it 'should return nil for non-existing user' do - User.authenticate('idontexistATALL', @user.password).should be_nil + it 'should authenticate existing user with a duplicate email address, assuming password is correct, returning true and user back' do + user2 = User.gen :username => 'SpongeBob', :password => 'squarepants', :email=> @user.email + success,user=User.authenticate( user2.email, user2.password ) + success.should be_true + user.id.should == user2.id + end + + it 'should return false as first return value for non-existing user' do + success,message=User.authenticate('idontexistATALL', @user.password) + success.should be_false + message.should == 'Invalid login or password' end - it 'should return nil for user with incorrect password' do - User.authenticate(@user.username, 'totally wrong password').should be_nil + it 'should return false as first return value for user with incorrect password' do + success,message=User.authenticate(@user.username, 'totally wrong password') + success.should be_false + message.should == 'Invalid login or password' end it 'should fail to change password if the account is not found' do @@ -44,11 +63,11 @@ describe User do # TODO this API is very smelly. User#reset_password returns different kinds of # results depending on whether the reset succeeded or failed. very frustrating. # this would be a good candicate for refactoring. - success, password, email = User.reset_password @user.email, '' + success, password, user = User.reset_password @user.email, '' success.should be_true password.should_not == @user.password - email.should == @user.email + user.email.should == @user.email # confirm things really changed properly, in the database User.find(@user.id).hashed_password.should == User.hash_password(password) @@ -56,11 +75,11 @@ describe User do end it 'should be able to reset the password on an account if both email address and username are entered' do - success, password, email = User.reset_password @user.email, @user.username + success, password, user = User.reset_password @user.email, @user.username success.should be_true password.should_not == @user_password - email.should == @user.email + user.email.should == @user.email User.find(@user.id).hashed_password.should == User.hash_password(password) User.find(@user.id).hashed_password.should_not == @user.hashed_password spec/models/user_spec.rb 5fc90294b0460c28435bee91b7d057ff8bf2d5e4 peter peter@78829999-583a-0410-bd01-8a9b849fd409 http://github.com/eol/eol/commit/85439ecb9b1ab9c64b6e2239150d2077110382bb 85439ecb9b1ab9c64b6e2239150d2077110382bb

2009-05-28T11:17:08-07:00

allow users to login with their email address as well as their usernames; fix some small bugs with the forgot password email not sending them their usernames in the body of the email; add default routing for taxa friendly URLs (like www.eol.org/animalia) git-svn-id: file:///data/subversion/eol/trunk@610 78829999-583a-0410-bd01-8a9b849fd409 409eacb1ac4b998ce2574e225c390d663d639e69 peter peter@78829999-583a-0410-bd01-8a9b849fd409