From 66717117843e8fc4fff42dc0acc9c57983ce68a9 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sun, 17 Jan 2021 00:24:02 -0600
Subject: [PATCH] dmails, emails: refactor to use Rails signed_id.

Refactor email verification links and Dmail share links to use the new
Rails signed_id mechanism, rather than our own handrolled mechanism.

For Dmail share links, we have to override some Rails internal methods
so that our old links still work. For email verification links, this
will invalidate existing links, but this isn't a huge deal since these
links are short-lived anyway.

https://api.rubyonrails.org/classes/ActiveRecord/SignedId.html
https://api.rubyonrails.org/classes/ActiveRecord/SignedId/ClassMethods.html
---
 app/controllers/application_controller.rb |  2 ++
 app/controllers/dmails_controller.rb      |  6 +++++-
 app/controllers/emails_controller.rb      |  3 +--
 app/models/dmail.rb                       | 18 +++++++++++-------
 app/models/email_address.rb               | 14 ++------------
 app/policies/dmail_policy.rb              |  2 +-
 app/policies/email_address_policy.rb      |  6 +-----
 7 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 13d1cb4f2..d64946ffb 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -99,6 +99,8 @@ class ApplicationController < ActionController::Base
       render_error_page(401, exception, template: "sessions/new")
     when ActionController::InvalidAuthenticityToken, ActionController::UnpermittedParameters, ActionController::InvalidCrossOriginRequest
       render_error_page(403, exception)
+    when ActiveSupport::MessageVerifier::InvalidSignature # raised by `find_signed!`
+      render_error_page(403, exception, template: "static/access_denied", message: "Access denied")
     when User::PrivilegeError, Pundit::NotAuthorizedError
       render_error_page(403, exception, template: "static/access_denied", message: "Access denied")
     when ActiveRecord::RecordNotFound
diff --git a/app/controllers/dmails_controller.rb b/app/controllers/dmails_controller.rb
index 9fe2eb362..73499707b 100644
--- a/app/controllers/dmails_controller.rb
+++ b/app/controllers/dmails_controller.rb
@@ -20,7 +20,11 @@ class DmailsController < ApplicationController
   end
 
   def show
-    @dmail = authorize Dmail.find(params[:id])
+    if params[:key].present?
+      @dmail = Dmail.find_signed!(params[:key], purpose: "dmail_link")
+    else
+      @dmail = authorize Dmail.find(params[:id])
+    end
 
     if request.format.html? && @dmail.owner == CurrentUser.user
       @dmail.update!(is_read: true)
diff --git a/app/controllers/emails_controller.rb b/app/controllers/emails_controller.rb
index 2f14ad9c3..556c86526 100644
--- a/app/controllers/emails_controller.rb
+++ b/app/controllers/emails_controller.rb
@@ -48,8 +48,7 @@ class EmailsController < ApplicationController
 
     if @email_address.blank?
       redirect_to edit_user_email_path(@user)
-    elsif params[:email_verification_key].present?
-      authorize @email_address
+    elsif params[:email_verification_key].present? && @email_address == EmailAddress.find_signed!(params[:email_verification_key], purpose: "verify")
       @email_address.verify!
       flash[:notice] = "Email address verified"
       redirect_to @email_address.user
diff --git a/app/models/dmail.rb b/app/models/dmail.rb
index 63094bff5..1d55f878e 100644
--- a/app/models/dmail.rb
+++ b/app/models/dmail.rb
@@ -109,16 +109,20 @@ class Dmail < ApplicationRecord
   end
 
   concerning :AuthorizationMethods do
-    def verifier
-      @verifier ||= Danbooru::MessageVerifier.new(:dmail_link)
+    class_methods do
+      # XXX hack so that rails' signed_id mechanism works with our pre-existing dmail keys.
+      # https://github.com/rails/rails/blob/main/activerecord/lib/active_record/signed_id.rb
+      def signed_id_verifier_secret
+        Rails.application.key_generator.generate_key("dmail_link")
+      end
+
+      def combine_signed_id_purposes(purpose)
+        purpose
+      end
     end
 
     def key
-      verifier.generate(id)
-    end
-
-    def valid_key?(key)
-      id == verifier.verified(key)
+      signed_id(purpose: "dmail_link")
     end
   end
 
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
index 7d437751d..e4b76a7bc 100644
--- a/app/models/email_address.rb
+++ b/app/models/email_address.rb
@@ -69,17 +69,7 @@ class EmailAddress < ApplicationRecord
     end
   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
+  def verification_key
+    signed_id(purpose: "verify")
   end
 end
diff --git a/app/policies/dmail_policy.rb b/app/policies/dmail_policy.rb
index a3a2b8d18..29585b438 100644
--- a/app/policies/dmail_policy.rb
+++ b/app/policies/dmail_policy.rb
@@ -17,7 +17,7 @@ class DmailPolicy < ApplicationPolicy
 
   def show?
     return true if user.is_owner?
-    !user.is_anonymous? && (record.owner_id == user.id || record.valid_key?(request.params[:key]))
+    !user.is_anonymous? && record.owner_id == user.id
   end
 
   def reportable?
diff --git a/app/policies/email_address_policy.rb b/app/policies/email_address_policy.rb
index f9ddcdfdb..57d8e03ae 100644
--- a/app/policies/email_address_policy.rb
+++ b/app/policies/email_address_policy.rb
@@ -13,11 +13,7 @@ class EmailAddressPolicy < ApplicationPolicy
   end
 
   def verify?
-    if request.params[:email_verification_key].present?
-      record.valid_key?(request.params[:email_verification_key])
-    else
-      record.user_id == user.id
-    end
+    record.user_id == user.id
   end
 
   def send_confirmation?