From 50fa674a3e72ff4a6fa14108a377fdb27d68db3e Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 19 Mar 2020 16:40:02 -0500 Subject: [PATCH] pundit: convert emails to pundit. --- app/controllers/emails_controller.rb | 23 +++-------- app/helpers/users_helper.rb | 4 ++ app/models/email_address.rb | 14 +++++++ app/policies/email_address_policy.rb | 14 +++++++ .../email_change_confirmation.html.erb | 2 +- app/views/user_mailer/welcome_user.html.erb | 2 +- test/functional/emails_controller_test.rb | 41 +++++++++++++++++-- 7 files changed, 76 insertions(+), 24 deletions(-) create mode 100644 app/policies/email_address_policy.rb diff --git a/app/controllers/emails_controller.rb b/app/controllers/emails_controller.rb index 0a328baa5..949b83616 100644 --- a/app/controllers/emails_controller.rb +++ b/app/controllers/emails_controller.rb @@ -1,24 +1,18 @@ class EmailsController < ApplicationController - before_action :member_only respond_to :html, :xml, :json def show - @user = User.find(params[:user_id]) - check_privilege(@user) - - respond_with(@user.email_address) + @email_address = authorize EmailAddress.find_by_user_id!(params[:user_id]) + respond_with(@email_address) end def edit - @user = User.find(params[:user_id]) - check_privilege(@user) - + @user = authorize User.find(params[:user_id]), policy_class: EmailAddressPolicy respond_with(@user) end def update - @user = User.find(params[:user_id]) - check_privilege(@user) + @user = authorize User.find(params[:user_id]), policy_class: EmailAddressPolicy if User.authenticate(@user.name, params[:user][:password]) @user.update(email_address_attributes: { address: params[:user][:email] }) @@ -37,17 +31,10 @@ class EmailsController < ApplicationController end def verify - email_id = Danbooru::MessageVerifier.new(:email_verification_key).verify(params[:email_verification_key]) - @email_address = EmailAddress.find(email_id) + @email_address = authorize EmailAddress.find_by_user_id!(params[:user_id]) @email_address.update!(is_verified: true) flash[:notice] = "Email address verified" redirect_to @email_address.user end - - private - - def check_privilege(user) - raise User::PrivilegeError unless user.id == CurrentUser.id || CurrentUser.is_admin? - end end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 260120d5d..68526187e 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -15,4 +15,8 @@ module UsersHelper verifier = ActiveSupport::MessageVerifier.new(Danbooru.config.email_key, serializer: JSON, digest: "SHA256") verifier.generate(user.id.to_s) end + + def email_verification_url(user) + verify_user_email_url(user, email_verification_key: user.email_address.verification_key) + end end diff --git a/app/models/email_address.rb b/app/models/email_address.rb index fde636524..7812d5131 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -12,4 +12,18 @@ class EmailAddress < ApplicationRecord self.normalized_address = EmailNormalizer.normalize(value) || address super end + + concerning :VerificationMethods do + def verifier + @verifier ||= Danbooru::MessageVerifier.new(:email_verification_key) + end + + def verification_key + verifier.generate(id) + end + + def valid_key?(key) + id == verifier.verified(key) + end + end end diff --git a/app/policies/email_address_policy.rb b/app/policies/email_address_policy.rb new file mode 100644 index 000000000..7122fa1f7 --- /dev/null +++ b/app/policies/email_address_policy.rb @@ -0,0 +1,14 @@ +class EmailAddressPolicy < ApplicationPolicy + def show? + record.user_id == user.id + end + + def update? + # XXX here record is a user, not the email address. + record.id == user.id + end + + def verify? + record.valid_key?(request.params[:email_verification_key]) + end +end diff --git a/app/views/user_mailer/email_change_confirmation.html.erb b/app/views/user_mailer/email_change_confirmation.html.erb index 3fce8ad7e..148d89ef2 100644 --- a/app/views/user_mailer/email_change_confirmation.html.erb +++ b/app/views/user_mailer/email_change_confirmation.html.erb @@ -9,7 +9,7 @@

- <%= link_to "Verify email address", verify_user_email_url(@user, email_verification_key: Danbooru::MessageVerifier.new(:email_verification_key).generate(@user.email_address.id)) %> + <%= link_to "Verify email address", email_verification_url(@user) %>

diff --git a/app/views/user_mailer/welcome_user.html.erb b/app/views/user_mailer/welcome_user.html.erb index aab8ae2b2..bb155bde5 100644 --- a/app/views/user_mailer/welcome_user.html.erb +++ b/app/views/user_mailer/welcome_user.html.erb @@ -8,7 +8,7 @@

- <%= link_to "Verify email address", verify_user_email_url(@user, email_verification_key: Danbooru::MessageVerifier.new(:email_verification_key).generate(@user.email_address.id)) %> + <%= link_to "Verify email address", email_verification_url(@user) %>

diff --git a/test/functional/emails_controller_test.rb b/test/functional/emails_controller_test.rb index ae7b7b297..058025ed8 100644 --- a/test/functional/emails_controller_test.rb +++ b/test/functional/emails_controller_test.rb @@ -1,9 +1,12 @@ require "test_helper" class EmailsControllerTest < ActionDispatch::IntegrationTest + include UsersHelper + context "in all cases" do setup do @user = create(:user, email_address: build(:email_address, { address: "bob@ogres.net", is_verified: false })) + @other_user = create(:user, email_address: build(:email_address, { address: "alice@ogres.net", is_verified: false })) end context "#show" do @@ -11,6 +14,11 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest get_auth user_email_path(@user), @user, as: :json assert_response :success end + + should "not show email addresses to other users" do + get_auth user_email_path(@user), @other_user, as: :json + assert_response 403 + end end context "#edit" do @@ -20,13 +28,29 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest end end - context "#create" do + context "#update" do context "with the correct password" do - should "work" do - put_auth user_email_path(@user), @user, params: { user: { password: "password", email: "abc@ogres.net" }} + 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" }} + end assert_redirected_to(settings_path) assert_equal("abc@ogres.net", @user.reload.email_address.address) + assert_equal(false, @user.email_address.is_verified) + assert_enqueued_email_with UserMailer, :email_change_confirmation, args: [@user] + end + + should "create a new address" do + @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" }} + end + + assert_redirected_to(settings_path) + assert_equal("abc@ogres.net", @user.reload.email_address.address) + assert_equal(false, @user.reload.email_address.is_verified) assert_enqueued_email_with UserMailer, :email_change_confirmation, args: [@user] end end @@ -46,12 +70,21 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest context "with a correct verification key" do should "mark the email address as verified" do assert_equal(false, @user.reload.email_address.is_verified) - get_auth verify_user_email_path(@user), @user, params: { email_verification_key: Danbooru::MessageVerifier.new(:email_verification_key).generate(@user.email_address.id) } + get email_verification_url(@user) assert_redirected_to @user assert_equal(true, @user.reload.email_address.is_verified) end end + + context "with an incorrect verification key" do + should "not mark the email address as verified" do + get verify_user_email_path(@user, email_verification_key: @other_user.email_address.verification_key) + + assert_response 403 + assert_equal(false, @user.reload.email_address.is_verified) + end + end end end end