diff options
| -rw-r--r-- | app/controllers/application.rb | 7 | ||||
| -rw-r--r-- | app/controllers/request_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/user_controller.rb | 65 | ||||
| -rw-r--r-- | app/models/post_redirect.rb | 13 | ||||
| -rw-r--r-- | app/models/user_mailer.rb | 26 | ||||
| -rw-r--r-- | app/views/user/confirm.rhtml | 14 | ||||
| -rw-r--r-- | app/views/user/signin.rhtml | 10 | ||||
| -rw-r--r-- | app/views/user/signup.rhtml | 5 | ||||
| -rw-r--r-- | app/views/user_mailer/confirm_login.rhtml | 11 | ||||
| -rw-r--r-- | config/general-example | 5 | ||||
| -rw-r--r-- | config/routes.rb | 3 | ||||
| -rw-r--r-- | db/migrate/016_add_reasons_to_post_redirects.rb | 11 | ||||
| -rw-r--r-- | db/migrate/017_add_email_confirmed_to_users.rb | 9 | ||||
| -rw-r--r-- | db/schema.rb | 17 | ||||
| -rw-r--r-- | public/stylesheets/main.css | 36 | ||||
| -rw-r--r-- | todo.txt | 12 |
16 files changed, 190 insertions, 62 deletions
diff --git a/app/controllers/application.rb b/app/controllers/application.rb index fb56497f4..cf5ca0db8 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -6,7 +6,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: application.rb,v 1.22 2007-11-01 14:45:56 francis Exp $ +# $Id: application.rb,v 1.23 2007-11-05 16:46:10 francis Exp $ class ApplicationController < ActionController::Base @@ -19,9 +19,10 @@ class ApplicationController < ActionController::Base private # Check the user is logged in - def authenticated? + def authenticated?(reason_params) unless session[:user] - post_redirect = PostRedirect.new(:uri => request.request_uri, :post_params => params) + post_redirect = PostRedirect.new(:uri => request.request_uri, :post_params => params, + :reason_params => reason_params) post_redirect.save! redirect_to signin_url(:token => post_redirect.token) return false diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index b1c27ecb6..c13ec45eb 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_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: request_controller.rb,v 1.11 2007-11-01 05:44:43 francis Exp $ +# $Id: request_controller.rb,v 1.12 2007-11-05 16:46:10 francis Exp $ class RequestController < ApplicationController @@ -41,7 +41,11 @@ class RequestController < ApplicationController # This automatically saves dependent objects, such as @info_request, in the same transaction if not @info_request.valid? render :action => 'new' - elsif authenticated? + elsif authenticated?( + :web => "To send your FOI request, please log in or make a new account.", + :email => "Then your FOI request to " + @info_request.public_body.name + " will be sent.", + :email_subject => "Confirm that you want to send an FOI request to " + @info_request.public_body.name + ) @info_request.user = authenticated_user @info_request.save @outgoing_message.send_message diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 6c950e7ba..5089c3cf8 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.10 2007-11-01 16:14:43 francis Exp $ +# $Id: user_controller.rb,v 1.11 2007-11-05 16:46:10 francis Exp $ class UserController < ApplicationController # XXX See controllers/application.rb simplify_url_part for reverse of expression in SQL below @@ -16,9 +16,11 @@ class UserController < ApplicationController def signin # The explict signin link uses this to store where it is to go back to if params[:r] - post_redirect = PostRedirect.new(:uri => params[:r], :post_params => {}) - post_redirect.save! - params[:token] = post_redirect.token + @post_redirect = PostRedirect.new(:uri => params[:r], :post_params => {}) + @post_redirect.save! + params[:token] = @post_redirect.token + else + @post_redirect = PostRedirect.find_by_token(params[:token]) end if not params[:user] @@ -29,9 +31,12 @@ class UserController < ApplicationController @user = User.authenticate(params[:user][:email], params[:user][:password]) if @user # Successful login - session[:user] = @user.id - post_redirect = PostRedirect.find_by_token(params[:token]) - do_post_redirect post_redirect.uri, post_redirect.post_params + if @user.email_confirmed + session[:user] = @user.id + do_post_redirect @post_redirect.uri, @post_redirect.post_params + else + send_confirmation_mail + end return else if User.find(:first, :conditions => [ "email ilike ?", params[:user][:email] ]) # using like for case insensitive @@ -54,21 +59,44 @@ class UserController < ApplicationController def signup # Make the user and try to save it @user = User.new(params[:user]) - if not @user.save + if not @user.valid? # First time get to form (e.g. from signin) , don't show errors @first_time = params[:first_time] @user.errors.clear if @first_time # Show the form render :action => 'signup' else - # New user made, redirect back to where we were - session[:user] = @user.id - post_redirect = PostRedirect.find_by_token(params[:token]) - do_post_redirect post_redirect.uri, post_redirect.post_params + # Unconfirmed user + @user.email_confirmed = false + @user.save + + send_confirmation_mail return end end + # Followed link in user account confirmation email + def confirm + post_redirect = PostRedirect.find_by_email_token(params[:email_token]) + + # XXX add message like this if post_redirect not found + # err(sprintf(_("Please check the URL (i.e. the long code of + # letters and numbers) is copied correctly from your email. If + # you can't click on it in the email, you'll have to select and + # copy it from the email. Then paste it into your browser, into + # the place you would type the address of any other webpage. + # Technical details: The token '%s' wasn't found."), $q_t)); + # + + @user = post_redirect.user + @user.email_confirmed = true + @user.save + + session[:user] = @user.id + + do_post_redirect post_redirect.uri, post_redirect.post_params + end + # Logout form def signout session[:user] = nil @@ -82,4 +110,17 @@ class UserController < ApplicationController private + # Ask for email confirmation + def send_confirmation_mail + raise "user #{@user.id} already confirmed" if @user.email_confirmed + + post_redirect = PostRedirect.find_by_token(params[:token]) + post_redirect.user = @user + post_redirect.save! + + url = confirm_url(:email_token => post_redirect.email_token) + UserMailer.deliver_confirm_login(@user, post_redirect.reason_params, url) + render :action => 'confirm' + end + end diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb index b7cf39092..4c4ddcc2f 100644 --- a/app/models/post_redirect.rb +++ b/app/models/post_redirect.rb @@ -5,11 +5,14 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: post_redirect.rb,v 1.2 2007-11-02 10:28:20 francis Exp $ +# $Id: post_redirect.rb,v 1.3 2007-11-05 16:46:10 francis Exp $ require 'openssl' # for random bytes function class PostRedirect < ActiveRecord::Base + # Optional, does login confirm after email. + belongs_to :user + # We store YAML version of POST parameters in the database def post_params=(params) self.post_params_yaml = params.to_yaml @@ -18,6 +21,14 @@ class PostRedirect < ActiveRecord::Base YAML.load(self.post_params_yaml) end + # We store YAML version of textual "reason for redirect" parameters + def reason_params=(reason_params) + self.reason_params_yaml = reason_params.to_yaml + end + def reason_params + YAML.load(self.reason_params_yaml) + end + # Makes a random token, suitable for using in URLs e.g confirmation messages. def self.generate_random_token bits = 12 * 8 diff --git a/app/models/user_mailer.rb b/app/models/user_mailer.rb new file mode 100644 index 000000000..a7e59f36a --- /dev/null +++ b/app/models/user_mailer.rb @@ -0,0 +1,26 @@ +# models/user_mailer.rb: +# Emails relating to user accounts. e.g. Confirming a new account +# +# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. +# Email: francis@mysociety.org; WWW: http://www.mysociety.org/ +# +# $Id: user_mailer.rb,v 1.1 2007-11-05 16:46:10 francis Exp $ + +class UserMailer < ActionMailer::Base + + def confirm_login(user, reasons, url) + @from = MySociety::Config.get("CONTACT_EMAIL") + @recipients = user.email + @subject = reasons[:email_subject] + @body[:reasons] = reasons + @body[:name] = user.name + @body[:url] = url + end + +end + +#'reason_web' => _("To view your pledges, we need to check your email address."), +#'reason_email' => _("Then you will be able to view your pledges."), +#'reason_email_subject' => _('View your pledges at PledgeBank.com') + + diff --git a/app/views/user/confirm.rhtml b/app/views/user/confirm.rhtml new file mode 100644 index 000000000..bc9b259cc --- /dev/null +++ b/app/views/user/confirm.rhtml @@ -0,0 +1,14 @@ +<% @title = h("Now check your email!") %> + +<h1 class="confirmation_heading">Now check your email!</h1> + +<p class="confirmation_message"> +We've sent you an email, and you'll need to click the link in it before you can +continue. +</p> + +<p class="confirmation_message"> +<small>If you use web-based email or have "junk mail" filters, also check your +bulk/spam mail folders. Sometimes, our messages are marked that way.</small> +</p> + diff --git a/app/views/user/signin.rhtml b/app/views/user/signin.rhtml index 137de91b3..239617189 100644 --- a/app/views/user/signin.rhtml +++ b/app/views/user/signin.rhtml @@ -1,6 +1,8 @@ <%= foi_error_messages_for :user %> <% form_tag({:action => "signin"}, {:id => "accountForm"}) do %> + <div class="form_note"><%= @post_redirect.reason_params[:web] %></div> + <p> <label for="user_email" id="signin_email"><strong>Enter your e-mail address:</strong></label> <%= text_field 'user', 'email', { :size => 20 } %> @@ -11,16 +13,14 @@ <%= password_field 'user', 'password', { :size => 15 } %> </p> - <p> - <label> </label> + <p class="form_note"> Don't have a password? Just enter one to register a new account. </p> - <p> - <label for="submit"> </label> + <div class="form_button"> <%= hidden_field_tag 'token', params[:token] %> <%= submit_tag "Sign in" %> - </p> + </div> <% end %> diff --git a/app/views/user/signup.rhtml b/app/views/user/signup.rhtml index dd779a0f3..d4d13277f 100644 --- a/app/views/user/signup.rhtml +++ b/app/views/user/signup.rhtml @@ -39,11 +39,10 @@ </ul> </div> - <p> - <label for="submit"> </label> + <div class="form_button"> <%= hidden_field_tag 'token', params[:token] %> <%= submit_tag "Sign in" %> - </p> + </div> <% end %> diff --git a/app/views/user_mailer/confirm_login.rhtml b/app/views/user_mailer/confirm_login.rhtml new file mode 100644 index 000000000..0c58905a8 --- /dev/null +++ b/app/views/user_mailer/confirm_login.rhtml @@ -0,0 +1,11 @@ +<%= @name %>, + +Please click on the link below to confirm your email address. +<%=@reasons[:email]%> + +<%=@url%> + +We will never give away or sell your email address to anyone else +without your permission. + +-- the GovernmentSpy.com team diff --git a/config/general-example b/config/general-example index ad5bd117b..6e8f3e0bb 100644 --- a/config/general-example +++ b/config/general-example @@ -14,7 +14,7 @@ * Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. * Email: francis@mysociety.org; WWW: http://www.mysociety.org * - * $Id: general-example,v 1.2 2007-10-26 18:00:27 francis Exp $ + * $Id: general-example,v 1.3 2007-11-05 16:46:11 francis Exp $ * */ @@ -26,5 +26,8 @@ define('OPTION_INCOMING_EMAIL_DOMAIN', 'localhost'); // e.g. 'foifa.com' define('OPTION_INCOMING_EMAIL_PREFIX', ''); // e.g. 'foi+' define('OPTION_INCOMING_EMAIL_SECRET', 'xxxx xxxx xxxx xxxx xxxx xxxx xxxx xxxx'); // Used for hash in request email address +// Administration +define('OPTION_CONTACT_EMAIL', 'admin@localhost'); + ?> diff --git a/config/routes.rb b/config/routes.rb index 66740a58a..cb235172f 100644 --- a/config/routes.rb +++ b/config/routes.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: routes.rb,v 1.19 2007-10-30 18:52:27 francis Exp $ +# $Id: routes.rb,v 1.20 2007-11-05 16:46:11 francis Exp $ ActionController::Routing::Routes.draw do |map| # The priority is based upon order of creation: first created -> highest priority. @@ -25,6 +25,7 @@ ActionController::Routing::Routes.draw do |map| user.signin '/signin', :action => 'signin' user.signup '/signup', :action => 'signup' user.signout '/signout', :action => 'signout' + user.confirm '/c/:email_token', :action => 'confirm' user.show_user "/user/:simple_name", :action => 'show' end diff --git a/db/migrate/016_add_reasons_to_post_redirects.rb b/db/migrate/016_add_reasons_to_post_redirects.rb new file mode 100644 index 000000000..622e1173b --- /dev/null +++ b/db/migrate/016_add_reasons_to_post_redirects.rb @@ -0,0 +1,11 @@ +class AddReasonsToPostRedirects < ActiveRecord::Migration + def self.up + add_column :post_redirects, :reason_params_yaml, :text + add_column :post_redirects, :user_id, :integer + end + + def self.down + remove_column :post_redirects, :reason_params_yaml + remove_column :post_redirects, :user_id + end +end diff --git a/db/migrate/017_add_email_confirmed_to_users.rb b/db/migrate/017_add_email_confirmed_to_users.rb new file mode 100644 index 000000000..b552e853e --- /dev/null +++ b/db/migrate/017_add_email_confirmed_to_users.rb @@ -0,0 +1,9 @@ +class AddEmailConfirmedToUsers < ActiveRecord::Migration + def self.up + add_column :users, :email_confirmed, :boolean, :default => false + end + + def self.down + remove_column :users, :email_confirmed + end +end diff --git a/db/schema.rb b/db/schema.rb index dce64b418..9f05fd4b4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,7 +2,7 @@ # migrations feature of ActiveRecord to incrementally modify your database, and # then regenerate this schema definition. -ActiveRecord::Schema.define(:version => 15) do +ActiveRecord::Schema.define(:version => 17) do create_table "incoming_messages", :force => true do |t| t.column "info_request_id", :integer @@ -30,12 +30,14 @@ ActiveRecord::Schema.define(:version => 15) do end create_table "post_redirects", :force => true do |t| - t.column "token", :text - t.column "uri", :text - t.column "post_params_yaml", :text - t.column "created_at", :datetime - t.column "updated_at", :datetime - t.column "email_token", :text + t.column "token", :text + t.column "uri", :text + t.column "post_params_yaml", :text + t.column "created_at", :datetime + t.column "updated_at", :datetime + t.column "email_token", :text + t.column "reason_params_yaml", :text + t.column "user_id", :integer end create_table "public_bodies", :force => true do |t| @@ -79,6 +81,7 @@ ActiveRecord::Schema.define(:version => 15) do t.column "salt", :string t.column "created_at", :datetime t.column "updated_at", :datetime + t.column "email_confirmed", :boolean, :default => false end end diff --git a/public/stylesheets/main.css b/public/stylesheets/main.css index 5f0c63b2f..8cc0c23b4 100644 --- a/public/stylesheets/main.css +++ b/public/stylesheets/main.css @@ -143,14 +143,14 @@ label { float: left; text-align: right; padding-right: 0.5em; - width: 10em; + width: 16em; margin-bottom: 0.5em; } .form_note { - margin-left: 10.5em; + margin-left: 16.5em; } .form_button { - margin-left: 10em; + margin-left: 16em; } /* Flashes */ @@ -242,34 +242,16 @@ table#list_requests .odd { width: 75%; } -#accountForm label { - width: 15em; -/* font-weight: bold; */ -} - -#accountForm input[type=radio] { - float: left; - text-align: right; - padding-right: 0.5em; - margin-left: 14em; -} - -#accountForm .radio_label { - text-align: left; - margin-left: 0.5em; - float: none; -} - -#accountForm .form_note { - display: block; - margin-left: 16em; - clear: both; +.confirmation_message { + font-size: 150%; + font-weight: bold; + text-align: center; } - -#accountForm h2 { +.confirmation_heading { text-align: center; } + /* Speech bubbles * from http://www.cssplay.co.uk/boxes/chunky.html */ @@ -1,3 +1,6 @@ +Check confirmed everywhere in password check etc. + + Next ==== @@ -7,10 +10,19 @@ Send email to requestor telling them new information has come in Make it say "dear" as default letter Work out how to do controller/view integrated specs and add some +Make it validate the HTML + +Make login have wording like on PledgeBank + Do you have a PledgeBank password? + Yes, please enter it: + No, or you’ve forgotten it + — we’ll send you a confirmation email instead. Tidying ======= +Add indices to token / email_token in post_redirects + Prevent double posting of same request If summary is blank, says "title must be filled in" grrrr |