diff options
| -rw-r--r-- | app/controllers/user_controller.rb | 16 | ||||
| -rw-r--r-- | app/helpers/application_helper.rb | 3 | ||||
| -rw-r--r-- | app/models/user.rb | 26 | ||||
| -rw-r--r-- | app/views/layouts/default.rhtml | 2 | ||||
| -rw-r--r-- | app/views/user/signin.rhtml | 4 | ||||
| -rw-r--r-- | app/views/user/signup.rhtml | 4 | ||||
| -rw-r--r-- | config/environment.rb | 2 | ||||
| -rw-r--r-- | public/stylesheets/main.css | 9 | ||||
| -rw-r--r-- | todo.txt | 3 |
9 files changed, 45 insertions, 24 deletions
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 95f44e40f..79311753e 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: user_controller.rb,v 1.13 2007-11-06 18:09:36 francis Exp $ +# $Id: user_controller.rb,v 1.14 2007-11-07 10:26:29 francis Exp $ class UserController < ApplicationController # XXX See controllers/application.rb simplify_url_part for reverse of expression in SQL below @@ -33,8 +33,12 @@ class UserController < ApplicationController render :action => 'sign' return else - @user = User.authenticate(params[:user][:email], params[:user][:password]) - if @user + @user = User.authenticate_from_form(params[:user]) + if @user.errors.size > 0 + # Failed to authenticate + render :action => 'signin' + return + else # Successful login if @user.email_confirmed session[:user] = @user.id @@ -43,12 +47,6 @@ class UserController < ApplicationController send_confirmation_mail end return - else - # Failed to authenticate - flash[:error] = "Email or password not correct, please try again" - @user = User.new(params[:user]) - render :action => 'signin' - return end end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5ecf8f57b..a63d36b39 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -5,7 +5,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: application_helper.rb,v 1.12 2007-11-01 05:35:43 francis Exp $ +# $Id: application_helper.rb,v 1.13 2007-11-07 10:26:30 francis Exp $ module ApplicationHelper # URL generating functions are needed by all controllers (for redirects) @@ -29,7 +29,6 @@ module ApplicationHelper end error_messages = objects.map {|object| object.errors.full_messages.map {|msg| content_tag(:li, msg) } } content_tag(:div, - content_tag(:p, 'Please correct the following and try again.') << content_tag(:ul, error_messages), html ) diff --git a/app/models/user.rb b/app/models/user.rb index 986d5dc3d..51f47164e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,34 +4,42 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: user.rb,v 1.11 2007-11-01 16:14:43 francis Exp $ +# $Id: user.rb,v 1.12 2007-11-07 10:26:30 francis Exp $ require 'digest/sha1' class User < ActiveRecord::Base - validates_presence_of :email + validates_presence_of :email, :message => "^Please enter your email address" validates_uniqueness_of :email, :case_sensitive => false validates_presence_of :name + validates_presence_of :hashed_password, :message => "^Please enter a password" has_many :info_requests attr_accessor :password_confirmation - validates_confirmation_of :password + validates_confirmation_of :password, :message =>"^Please enter the same password twice" def validate - errors.add_to_base("Missing password") if hashed_password.blank? errors.add(:email, "doesn't look like a valid address") unless MySociety::Validate.is_valid_email(self.email) end - # Return user given login email and password - def self.authenticate(email, password) - user = self.find(:first, :conditions => [ 'email ilike ?', email ] ) # using ilike for case insensitive + # Return user given login email, password and other form parameters (e.g. name) + def self.authenticate_from_form(params) + auth_fail_message = "Email or password not recognised, please try again" + user = self.find(:first, :conditions => [ 'email ilike ?', params[:email] ] ) # using ilike for case insensitive if user - expected_password = encrypted_password(password, user.salt) + # There is user with email, check password + expected_password = encrypted_password(params[:password], user.salt) if user.hashed_password != expected_password - user = nil + user.errors.add_to_base(auth_fail_message) end + else + # No user of same email, make one (that we don't save in the database) + # for the forms code to use. + user = User.new(params) + # deliberately same message as above so as not to leak whether + user.errors.add_to_base(auth_fail_message) end user end diff --git a/app/views/layouts/default.rhtml b/app/views/layouts/default.rhtml index 4d06d4cd2..29629ae1b 100644 --- a/app/views/layouts/default.rhtml +++ b/app/views/layouts/default.rhtml @@ -48,7 +48,7 @@ <div id="content"> <div id="<%= controller.controller_name + "_" + controller.action_name %>"> <%= yield :layout %> - <?div> + </div> </div> </div> diff --git a/app/views/user/signin.rhtml b/app/views/user/signin.rhtml index 3dcfd198f..6487af84d 100644 --- a/app/views/user/signin.rhtml +++ b/app/views/user/signin.rhtml @@ -1 +1,5 @@ +<div id="sign_alone"> + <%= render :partial => 'signin' %> + +</div> diff --git a/app/views/user/signup.rhtml b/app/views/user/signup.rhtml index bcade0078..c4b197b38 100644 --- a/app/views/user/signup.rhtml +++ b/app/views/user/signup.rhtml @@ -1 +1,5 @@ +<div id="sign_alone"> + <%= render :partial => 'signup' %> + +</div> diff --git a/config/environment.rb b/config/environment.rb index 2b3211690..14bf2658e 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -68,6 +68,8 @@ end # Validation error messages ActiveRecord::Errors.default_error_messages[:blank] = "must be filled in" +# Use SPAN instead of DIV. See http://dev.rubyonrails.org/ticket/2210 +ActionView::Base.field_error_proc = Proc.new{ |html_tag, instance| %(<span class="fieldWithErrors">#{html_tag}</span>)} # Include your application configuration below diff --git a/public/stylesheets/main.css b/public/stylesheets/main.css index f7c08caa7..33e8720ee 100644 --- a/public/stylesheets/main.css +++ b/public/stylesheets/main.css @@ -126,7 +126,6 @@ a:hover, a:active { form { margin: 0; - padding: 1em; } .plaque { @@ -157,7 +156,7 @@ label { } .form_item_note { margin-left: 16.5em; - margin-top: -0.85em; + margin-top: -0.85em; font-style: italic; } @@ -190,6 +189,7 @@ label { border: solid 1px #cc0000; background-color: #ffcccc; } +div.fieldWithErrors { display:inline; } /* / - front page */ @@ -268,6 +268,11 @@ table#list_requests .odd { float: right; } +#sign_alone { + margin-left: 24%; + margin-right: 24%; +} + #sign_together label { width: 8em; } @@ -7,7 +7,8 @@ Next Send email to requestor telling them new information has come in Forgotten password link -Email has already been taken +Email has already been taken hasn't got a link to better place +Link from error page for signin/signup to go to other one Make it say "dear" as default letter |