diff options
-rw-r--r-- | app/controllers/user_controller.rb | 60 | ||||
-rw-r--r-- | app/models/user_mailer.rb | 21 | ||||
-rw-r--r-- | app/views/user/show.rhtml | 2 | ||||
-rw-r--r-- | app/views/user/signchangeemail.rhtml | 7 | ||||
-rw-r--r-- | app/views/user/signchangeemail_confirm.rhtml | 14 | ||||
-rw-r--r-- | app/views/user/signchangepassword.rhtml | 6 | ||||
-rw-r--r-- | app/views/user/signchangepassword_send_confirm.rhtml | 4 | ||||
-rw-r--r-- | app/views/user_mailer/changeemail_already_used.rhtml | 9 | ||||
-rw-r--r-- | app/views/user_mailer/changeemail_confirm.rhtml | 12 | ||||
-rw-r--r-- | spec/controllers/user_controller_spec.rb | 53 | ||||
-rw-r--r-- | spec/models/request_mailer_spec.rb | 2 |
11 files changed, 147 insertions, 43 deletions
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 80163db1d..37cc0db99 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -231,37 +231,55 @@ class UserController < ApplicationController # Change your email def signchangeemail if not authenticated?( - :web => "To change your email address", - :email => "Then you can change your email address", - :email_subject => "Change your email address" + :web => "To change your email address used on WhatDoTheyKnow.com", + :email => "Then you can change your email address used on WhatDoTheyKnow.com", + :email_subject => "Change your email address used on WhatDoTheyKnow.com" ) # "authenticated?" has done the redirect to signin page for us return end - work_out_post_redirect + if !params[:submitted_signchangeemail_do] + render :action => 'signchangeemail' + return + end - if params[:submitted_signchangeemail_do] - @signchangeemail = ChangeEmailValidator.new(params[:signchangeemail]) - @signchangeemail.logged_in_user = @user + @signchangeemail = ChangeEmailValidator.new(params[:signchangeemail]) + @signchangeemail.logged_in_user = @user - if @signchangeemail.valid? - user_alreadyexists = User.find_user_by_email(@signchangeemail.new_email) - if user_alreadyexists - already_registered_mail user_alreadyexists - return - end + if !@signchangeemail.valid? + render :action => 'signchangeemail' + return + end - @user.email = @signchangeemail.new_email - @user.email_confirmed = false - @user.save! - self._do_signout - send_confirmation_mail @user - return - end + # if new email already in use, send email there saying what happened + user_alreadyexists = User.find_user_by_email(@signchangeemail.new_email) + if user_alreadyexists + UserMailer.deliver_changeemail_already_used(@user.email, @signchangeemail.new_email) + render :action => 'signchangeemail_confirm' + return + end + + # if not already, send a confirmation link to the new email address which logs + # them into the old email's user account, but with special user_circumstance + if (not session[:user_circumstance]) or (session[:user_circumstance] != "change_email") + post_redirect = PostRedirect.new(:uri => signchangeemail_url(), :post_params => params, + :circumstance => "change_email" # special login that lets you change your email + ) + post_redirect.user = @user + post_redirect.save! + + url = confirm_url(:email_token => post_redirect.email_token) + UserMailer.deliver_changeemail_confirm(@user, @signchangeemail.new_email, url) + render :action => 'signchangeemail_confirm' + return end - render :action => 'signchangeemail' + # circumstance is 'change_email', so can actually change the email + @user.email = @signchangeemail.new_email + @user.save! + flash[:notice] = "You have now changed your email address used on WhatDoTheyKnow.com" + redirect_to user_url(@user) end # Send a message to another user diff --git a/app/models/user_mailer.rb b/app/models/user_mailer.rb index 06a115c5b..3144c06e2 100644 --- a/app/models/user_mailer.rb +++ b/app/models/user_mailer.rb @@ -27,5 +27,26 @@ class UserMailer < ApplicationMailer @body[:url] = url end + def changeemail_confirm(user, new_email, url) + @from = contact_from_name_and_email + headers 'Return-Path' => blackhole_email, 'Reply-To' => @from # we don't care about bounces when people are fiddling with their account + @recipients = new_email + @subject = "Confirm your new email address on WhatDoTheyKnow.com" + @body[:name] = user.name + @body[:url] = url + @body[:old_email] = user.email + @body[:new_email] = new_email + end + + def changeemail_already_used(old_email, new_email) + @from = contact_from_name_and_email + headers 'Return-Path' => blackhole_email, 'Reply-To' => @from # we don't care about bounces when people are fiddling with their account + @recipients = new_email + @subject = "New email address already in use on WhatDoTheyKnow.com" + @body[:old_email] = old_email + @body[:new_email] = new_email + end + + end diff --git a/app/views/user/show.rhtml b/app/views/user/show.rhtml index 166b005d4..eb547d953 100644 --- a/app/views/user/show.rhtml +++ b/app/views/user/show.rhtml @@ -51,7 +51,7 @@ <% if @is_you %> (just to see how it works) <br><%= link_to "Change your password", signchangepassword_url() %> | - <%= link_to "Change your email", signchangeemail_url(:r => request.request_uri) %> + <%= link_to "Change your email", signchangeemail_url() %> <% end %> </p> diff --git a/app/views/user/signchangeemail.rhtml b/app/views/user/signchangeemail.rhtml index 2d9b15895..caed4235f 100644 --- a/app/views/user/signchangeemail.rhtml +++ b/app/views/user/signchangeemail.rhtml @@ -1,4 +1,4 @@ -<% @title = "Change your email address" %> +<% @title = "Change your email address used on WhatDoTheyKnow.com" %> <% raise "internal error" if not @user %> @@ -8,7 +8,7 @@ <%= foi_error_messages_for :signchangeemail %> <div class="form_note"> - <h1>Change your email address</h1> + <h1>Change your email address used on WhatDoTheyKnow.com</h1> </div> <p> @@ -34,8 +34,7 @@ <div class="form_button"> <%= hidden_field_tag 'submitted_signchangeemail_do', 1 %> - <%= hidden_field_tag 'token', params[:token], { :id => 'signin_token' } %> - <%= submit_tag "Change email" %> + <%= submit_tag "Change email on WhatDoTheyKnow.com" %> </div> <% end %> diff --git a/app/views/user/signchangeemail_confirm.rhtml b/app/views/user/signchangeemail_confirm.rhtml new file mode 100644 index 000000000..96acbf424 --- /dev/null +++ b/app/views/user/signchangeemail_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 an email to your new email address. You'll need to click the link in +it before your email address will be changed. +</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/signchangepassword.rhtml b/app/views/user/signchangepassword.rhtml index 11dc987ee..4191344cb 100644 --- a/app/views/user/signchangepassword.rhtml +++ b/app/views/user/signchangepassword.rhtml @@ -1,4 +1,4 @@ -<% @title = "Change your password" %> +<% @title = "Change your password on WhatDoTheyKnow.com" %> <% raise "internal error" if not @user %> @@ -8,7 +8,7 @@ <%= foi_error_messages_for :user %> <div class="form_note"> - <h1>Change your password</h1> + <h1>Change your password on WhatDoTheyKnow.com</h1> </div> <p> @@ -24,7 +24,7 @@ <div class="form_button"> <%= hidden_field_tag 'submitted_signchangepassword_do', 1 %> <%= hidden_field_tag 'pretoken', params[:pretoken] %> - <%= submit_tag "Change password" %> + <%= submit_tag "Change password on WhatDoTheyKnow.com" %> </div> <% end %> diff --git a/app/views/user/signchangepassword_send_confirm.rhtml b/app/views/user/signchangepassword_send_confirm.rhtml index 2ff670c34..8b2e4fa91 100644 --- a/app/views/user/signchangepassword_send_confirm.rhtml +++ b/app/views/user/signchangepassword_send_confirm.rhtml @@ -1,4 +1,4 @@ -<% @title = "Change password" %> +<% @title = "Change your password on WhatDoTheyKnow.com" %> <div id="change_password"> @@ -6,7 +6,7 @@ <%= foi_error_messages_for :signchangepassword %> <div class="form_note"> - <h1>Change your password</h1> + <h1>Change your password on WhatDoTheyKnow.com</h1> </div> <p> diff --git a/app/views/user_mailer/changeemail_already_used.rhtml b/app/views/user_mailer/changeemail_already_used.rhtml new file mode 100644 index 000000000..0f60ad798 --- /dev/null +++ b/app/views/user_mailer/changeemail_already_used.rhtml @@ -0,0 +1,9 @@ +Someone, perhaps you, just tried to change their email address on +WhatDoTheyKnow.com from <%=@old_email%> to <%=@new_email%>. + +This was not possible because there is already an account using +the email address <%=@new_email%>. + +The accounts have been left as they previously were. + +-- the WhatDoTheyKnow team diff --git a/app/views/user_mailer/changeemail_confirm.rhtml b/app/views/user_mailer/changeemail_confirm.rhtml new file mode 100644 index 000000000..9aa288fb0 --- /dev/null +++ b/app/views/user_mailer/changeemail_confirm.rhtml @@ -0,0 +1,12 @@ +<%= @name %>, + +Please click on the link below to confirm that you want to +change the email address that you use for WhatDoTheyKnow +from <%=@old_email%> to <%=@new_email%> + +<%=@url%> + +We will not reveal your email addresses to anybody unless you +or the law tell us to. + +-- the WhatDoTheyKnow team diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index d85706b2d..2bfb35240 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -346,7 +346,7 @@ describe UserController, "when changing email address" do deliveries.size.should == 0 end - it "should change your email if you get all the details right, and send confirmation email" do + it "should send confirmation email if you get all the details right" do @user = users(:bob_smith_user) session[:user_id] = @user.id @@ -356,17 +356,49 @@ describe UserController, "when changing email address" do } @user.reload - @user.email.should == 'newbob@localhost' - @user.email_confirmed.should == false + @user.email.should == 'bob@localhost' + @user.email_confirmed.should == true - response.should render_template('confirm') + response.should render_template('signchangeemail_confirm') deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 mail = deliveries[0] - - mail.body.should include("not reveal your email") + mail.body.should include("confirm that you want to change") mail.to.should == [ 'newbob@localhost' ] + + mail.body =~ /(http:\/\/.*(\/c\/(.*)))/ + mail_url = $1 + mail_path = $2 + mail_token = $3 + + # Check confirmation URL works + session[:user_id] = nil + session[:user_circumstance].should == nil + get :confirm, :email_token => mail_token + session[:user_id].should == users(:bob_smith_user).id + session[:user_circumstance].should == 'change_email' + response.should redirect_to(:controller => 'user', :action => 'signchangeemail', :post_redirect => 1) + + # Would be nice to do a follow_redirect! here, but rspec-rails doesn't + # have one. Instead do an equivalent manually. + post_redirect = PostRedirect.find_by_email_token(mail_token) + post_redirect.circumstance.should == 'change_email' + post_redirect.user.should == users(:bob_smith_user) + post_redirect.post_params.should == {"submitted_signchangeemail_do"=>"1", + "action"=>"signchangeemail", + "signchangeemail"=>{ + "old_email"=>"bob@localhost", + "new_email"=>"newbob@localhost", + "password"=>"jonespassword"}, + "controller"=>"user"} + post :signchangeemail, post_redirect.post_params + + response.should redirect_to(:controller => 'user', :action => 'show', :url_name => 'bob_smith') + flash[:notice].should match(/You have now changed your email address/) + @user.reload + @user.email.should == 'newbob@localhost' + @user.email_confirmed.should == true end it "should send special 'already signed up' mail if you try to change your email to one already used" do @@ -374,7 +406,7 @@ describe UserController, "when changing email address" do session[:user_id] = @user.id post :signchangeemail, { :signchangeemail => { :old_email => 'bob@localhost', - :password => 'jonespassword', :new_email => 'bob@localhost' }, + :password => 'jonespassword', :new_email => 'silly@localhost' }, :submitted_signchangeemail_do => 1 } @@ -382,16 +414,15 @@ describe UserController, "when changing email address" do @user.email.should == 'bob@localhost' @user.email_confirmed.should == true - response.should render_template('confirm') + response.should render_template('signchangeemail_confirm') deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 mail = deliveries[0] - mail.body.should include("have an account") - mail.to.should == [ 'bob@localhost' ] + mail.body.should include("perhaps you, just tried to change their") + mail.to.should == [ 'silly@localhost' ] end - end diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index 541f1da87..6be7b9cea 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -115,7 +115,7 @@ describe RequestMailer, " when receiving incoming mail" do deliveries.clear end - it "should dump messages to a request if marked to do so" do + it "should destroy the messages sent to a request if marked to do so" do ActionMailer::Base.deliveries.clear # mark request as anti-spam ir = info_requests(:fancy_dog_request) |