jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

users: use sudo mode when changing email addresses.

Browse Source 
When a user tries to change their email, redirect them to the confirm
password page (like Github's sudo mode) instead of having them re-enter
their password on the change email page. This is the same thing we do
when a user updates their API keys. This way we have can use the same
confirm password authentication flow for everything that needs a
password.
This commit is contained in:
evazion
2021-05-18 22:50:09 -05:00
parent 12eacbe76f
commit 24ead500f0
5 changed files with 47 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
app/controllers/emails_controller.rb
View File

@@ -1,4 +1,5 @@
class EmailsController < ApplicationController
before_action :requires_reauthentication, only: [:edit, :update]
respond_to :html, :xml, :json
def index
@@ -24,17 +25,10 @@ class EmailsController < ApplicationController
def update
@user = authorize User.find(params[:user_id]), policy_class: EmailAddressPolicy
if @user.authenticate_password(params[:user][:password])
UserEvent.build_from_request(@user, :email_change, request)
@user.update(email_address_attributes: { address: params[:user][:email] })
else
@user.errors.add(:base, "Password was incorrect")
end
@user.change_email(params[:user][:email], request)
if @user.errors.none?
flash[:notice] = "Email updated. Check your email to confirm your new address"
UserMailer.email_change_confirmation(@user).deliver_later
respond_with(@user, location: settings_url)
else
flash[:notice] = @user.errors.full_messages.join("; ")

11
app/models/user.rb
View File

@@ -361,6 +361,17 @@ class User < ApplicationRecord
def can_receive_email?(require_verification: true)
email_address.present? && email_address.is_deliverable? && (email_address.is_verified? || !require_verification)
end
def change_email(new_email, request)
transaction do
update(email_address_attributes: { address: new_email })
if errors.none?
UserEvent.create_from_request!(self, :email_change, request)
UserMailer.email_change_confirmation(self).deliver_later
end
end
end
end
concerning :BlacklistMethods do

16
app/views/emails/edit.html.erb
View File

@@ -1,17 +1,10 @@
<div id="c-emails">
<div id="a-edit" class="fixed-width-container">
<% page_title "Change Email" %>
<h1>Change Email</h1>
<% if @user.email_address.present? %>
<% page_title "Change Email" %>
<h1>Change Email</h1>
<p>Your current email address is <strong><%= @user.email_address.address %></strong>.
You must re-enter your password in order to update your email address.</p>
<% else %>
<% page_title "Add Email" %>
<h1>Add Email</h1>
<p>Add a new email address below. You must re-enter your password in
order to update your email address.</p>
<% end %>
<% if @user.is_restricted? %>
@@ -23,8 +16,7 @@
<% end %>
<%= edit_form_for(@user, url: user_email_path(@user)) do |f| %>
<%= f.input :email, as: :email, label: "New Email", input_html: { value: "" } %>
<%= f.input :password %>
<%= f.input :email, as: :email, input_html: { value: @user&.email_address&.address } %>
<%= f.submit "Save" %>
<% end %>
</div>

1
test/factories/email_address.rb
View File

@@ -2,5 +2,6 @@ FactoryBot.define do
factory(:email_address) do
address { FFaker::Internet.email }
is_verified { true }
user
end
end

45
test/functional/emails_controller_test.rb
View File

@@ -43,6 +43,17 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
end
context "#edit" do
context "for a user who hasn't recently authenticated" do
should "redirect to the confirm password page" do
post session_path, params: { name: @user.name, password: @user.password }
travel_to 2.hours.from_now do
get edit_user_email_path(@user)
end
assert_redirected_to confirm_password_session_path(url: edit_user_email_path(@user.id))
end
end
context "for a user with an email address" do
should "render" do
get_auth edit_user_email_path(@user), @user
@@ -59,7 +70,7 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
assert_equal false, @user.email_address.present?
assert_response :success
assert_select "h1", text: "Add Email"
assert_select "h1", text: "Change Email"
end
end
@@ -79,10 +90,24 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
end
context "#update" do
context "for a user who hasn't recently authenticated" do
should "redirect to the confirm password page" do
post session_path, params: { name: @user.name, password: @user.password }
travel_to 2.hours.from_now do
put user_email_path(@user), params: { user: { email: "abc@ogres.net" }}
end
assert_redirected_to confirm_password_session_path(url: user_email_path(@user.id))
assert_equal("bob@ogres.net", @user.reload.email_address.address)
assert_no_emails
assert_equal(false, @user.user_events.email_change.exists?)
end
end
context "with the correct password" do
should "update an existing address" do
assert_difference("EmailAddress.count", 0) do
put_auth user_email_path(@user), @user, params: { user: { password: "password", email: "abc@ogres.net" }}
put_auth user_email_path(@user), @user, params: { user: { email: "abc@ogres.net" }}
end
assert_redirected_to(settings_path)
@@ -96,7 +121,7 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
@user.email_address.destroy
assert_difference("EmailAddress.count", 1) do
put_auth user_email_path(@user), @user, params: { user: { password: "password", email: "abc@ogres.net" }}
put_auth user_email_path(@user), @user, params: { user: { email: "abc@ogres.net" }}
end
assert_redirected_to(settings_path)
@@ -108,7 +133,7 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
should "not allow banned users to change their email address" do
create(:ban, user: @user, duration: 1.week)
put_auth user_email_path(@user), @user, params: { user: { password: "password", email: "abc@ogres.net" }}
put_auth user_email_path(@user), @user, params: { user: { email: "abc@ogres.net" }}
assert_response 403
assert_equal("bob@ogres.net", @user.reload.email_address.address)
@@ -117,18 +142,6 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
end
end
context "with the incorrect password" do
should "not work" do
put_auth user_email_path(@user), @user, params: { user: { password: "passwordx", email: "abc@ogres.net" }}
assert_response :success
assert_equal("bob@ogres.net", @user.reload.email_address.address)
assert_no_emails
assert_equal(false, @user.user_events.email_change.exists?)
end
end
end
context "#verify" do
context "with a correct verification key" do
should "mark the email address as verified" do