aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--app/controllers/user_controller.rb60
-rw-r--r--app/models/user_mailer.rb21
-rw-r--r--app/views/user/show.rhtml2
-rw-r--r--app/views/user/signchangeemail.rhtml7
-rw-r--r--app/views/user/signchangeemail_confirm.rhtml14
-rw-r--r--app/views/user/signchangepassword.rhtml6
-rw-r--r--app/views/user/signchangepassword_send_confirm.rhtml4
-rw-r--r--app/views/user_mailer/changeemail_already_used.rhtml9
-rw-r--r--app/views/user_mailer/changeemail_confirm.rhtml12
-rw-r--r--spec/controllers/user_controller_spec.rb53
-rw-r--r--spec/models/request_mailer_spec.rb2
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)