Merge branch 'main' into release/20.10

Author: denny

.rubocop.yml

@@ -86,6 +86,10 @@ Rails/OutputSafety:
 Style/ClassAndModuleChildren:
   Enabled: false
 
+# I think this makes code significantly harder to read, personally
+Style/ConditionalAssignment:
+  Enabled: false
+
 # This test is for a feature that parses a list of IP addresses
 Style/IpAddresses:
   Exclude:

app/controllers/email_recipients_controller.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+# ShinyCMS ~ https://shinycms.org
+#
+# Copyright 2009-2020 Denny de la Haye ~ https://denny.me
+#
+# ShinyCMS is free software; you can redistribute it and/or modify it under the terms of the GPL (version 2 or later)
+
+# Controller for main site email-recipient features in ShinyCMS
+class EmailRecipientsController < MainController
+  # Confirm that the person has access to the email account - AKA double opt-in
+  def confirm
+    recipient = EmailRecipient.find_by( token: params[ :token ] )
+
+    if recipient&.confirm
+      flash[ :notice ] = t( '.success' )
+    else
+      flash[ :alert ] = t( confirm_failure_message( recipient ) )
+    end
+
+    redirect_back fallback_location: root_path
+  end
+
+  private
+
+  def confirm_failure_message( recipient = nil )
+    return '.token_not_found' if recipient.blank?
+    return '.token_expired'   if recipient.confirm_expired?
+
+    # '.failure'
+  end
+end

app/mailers/discussion_mailer.rb

@@ -17,7 +17,7 @@ def parent_comment_notification( comment )
 
     @user = notified_user( @parent.notification_email, @parent.author_name_any )
 
-    return if DoNotContact.include? @user.email # TODO: make this happen without explicit call
+    return if @user.do_not_email? # TODO: make this happen without explicit call
 
     mail to: @user.email_to, subject: parent_comment_notification_subject do |format|
       format.html
@@ -30,7 +30,7 @@ def discussion_notification( comment )
 
     @comment, @resource, @user = comment_and_resource_and_user( comment )
 
-    return if DoNotContact.include? @user.email # TODO: make this happen without explicit call
+    return if @user.do_not_email? # TODO: make this happen without explicit call
 
     mail to: @user.email_to, subject: discussion_notification_subject do |format|
       format.html

app/mailers/email_recipient_mailer.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+# ShinyCMS ~ https://shinycms.org
+#
+# Copyright 2009-2020 Denny de la Haye ~ https://denny.me
+#
+# ShinyCMS is free software; you can redistribute it and/or modify it under the terms of the GPL (version 2 or later)
+
+# Mailer for EmailRecipients (non-authenticated site users that we want to send email to)
+class EmailRecipientMailer < ApplicationMailer
+  # Don't store URLs that might have security tokens in them in email stats data
+  track click: false
+
+  # Email a link that the user must click to prove they have access to this email address
+  def confirm( recipient )
+    @user = @recipient = recipient
+    @confirm_token = recipient.confirm_token
+
+    return if DoNotContact.include? recipient.email # TODO: make this happen without explicit call
+
+    mail to: recipient.email_to, subject: t( '.subject', site_name: site_name ) do |format|
+      format.html
+      format.text
+    end
+  end
+end

app/models/concerns/shiny_email.rb

@@ -23,6 +23,10 @@ def generate_canonical_email
       self.canonical_email = EmailAddress.canonical( email )
     end
 
+    def do_not_email?
+      !confirmed? || DoNotContact.include?( email )
+    end
+
     def obfuscated_email
       EmailAddress.munge( email )
     end

app/models/email_recipient.rb

@@ -17,4 +17,46 @@ class EmailRecipient < ApplicationRecord
 
   # Email stats (powered by Ahoy)
   has_many :messages, as: :user, dependent: :nullify, class_name: 'Ahoy::Message'
+
+  # Scopes
+
+  scope :confirmed, -> { where.not( confirmed_at: nil ) }
+
+  # Instance methods
+
+  after_create :send_confirm_email
+
+  def set_confirm_token
+    update!(
+      confirm_token: SecureRandom.uuid,
+      confirm_sent_at: Time.zone.now,
+      confirmed_at: nil
+    )
+  end
+
+  def send_confirm_email
+    set_confirm_token
+    EmailRecipientMailer.confirm( self )
+  end
+
+  def confirm
+    return false if confirm_expired?
+
+    update!( confirmed_at: Time.zone.now )
+  end
+
+  def confirmed?
+    confirmed_at.present?
+  end
+
+  def confirm_expired?
+    confirm_sent_at < self.class.confirm_token_valid_for.ago
+  end
+
+  # Class methods
+
+  def self.confirm_token_valid_for
+    # TODO: make this configurable
+    7.days
+  end
 end

app/views/shinycms/email_recipient_mailer/confirm.html.mjml

@@ -0,0 +1,40 @@
+  <mj-body background-color="#f8f8f8">
+    <mj-section background-color="#ffffff" background-repeat="repeat" padding-bottom="0px" padding-left="0px" padding-right="0px" padding-top="0px" padding="20px 0" text-align="center">
+      <mj-column>
+        <mj-divider border-color="#60b4cc" border-style="solid" border-width="7px" padding-bottom="40px" padding-left="0px" padding-right="0px" padding-top="0px" padding="10px 25px" width="100%">
+        </mj-divider>
+        <mj-image align="center" alt="" border="none" href="" padding-bottom="0px" padding-top="0px" padding="10px 25px" src="https://shinycms.org/images/spiral.png" target="_blank" title="" height="auto" width="110px">
+        </mj-image>
+      </mj-column>
+    </mj-section>
+    <mj-section background-color="#ffffff" background-repeat="repeat" background-size="auto" padding-bottom="0px" padding-top="0px" padding="20px 0" text-align="center">
+      <mj-column>
+        <mj-image align="center" alt="" border="none" height="auto" href="" padding-bottom="0px" padding-left="50px" padding-right="50px" padding-top="40px" padding="10px 25px" src="http://9pl9.mjt.lu/tplimg/9pl9/b/yg0q/t65sy.png" target="_blank" title="" width="300px">
+        </mj-image>
+      </mj-column>
+    </mj-section>
+    <mj-section background-color="#ffffff" background-repeat="repeat" background-size="auto" padding-bottom="70px" padding-top="30px" padding="20px 0px 20px 0px" text-align="center">
+      <mj-column>
+        <mj-text align="left" color="#797e82" font-family="Open Sans, Helvetica, Arial, sans-serif" font-size="13px" line-height="22px" padding-bottom="0px" padding-left="50px" padding-right="50px" padding-top="0px" padding="0px 25px 0px 25px">
+          <h2 style="text-align:center; color: #000000; line-height:32px">
+            <%= t( 'user_mailer.hello', name: ( @recipient.name || @recipient.email ) ) %>
+          </h2>
+        </mj-text>
+        <mj-text align="left" color="#797e82" font-family="Open Sans, Helvetica, Arial, sans-serif" font-size="13px" line-height="22px" padding-bottom="0px" padding-left="50px" padding-right="50px" padding-top="0px" padding="0px 25px 0px 25px">
+          <p style="margin: 10px 0; text-align: center;">
+            Before we can send you any emails from <%= site_name %>, we need you to click on the button below:
+          </p>
+        </mj-text>
+        <mj-button href="<%= confirm_email_url( @confirm_token ) %>" align="center" background-color="#60b4cc" border-radius="100px" border="none" color="#ffffff" font-family="Open Sans, Helvetica, Arial, sans-serif" font-size="13px" font-weight="normal" inner-padding="15px 25px 15px 25px" padding-bottom="20px" padding-top="20px" padding="10px 25px" text-decoration="none" text-transform="none" vertical-align="middle">
+          <b style="font-weight:700">Verify my email address</b>
+        </mj-button>
+        <mj-text align="left" color="#797e82" font-family="Open Sans, Helvetica, Arial, sans-serif" font-size="13px" line-height="22px" padding-bottom="0px" padding-left="50px" padding-right="50px" padding-top="0px" padding="0px 25px 0px 25px">
+          <p style="margin: 10px 0; text-align: center;">
+            If the button doesn&rsquo;t work, copy this URL into your browser:
+            <br>
+            <%= confirm_email_url( @confirm_token ) %>
+          </p>
+        </mj-text>
+      </mj-column>
+    </mj-section>
+  </mj-body>

app/views/shinycms/email_recipient_mailer/confirm.text.erb

@@ -0,0 +1,7 @@
+Hi <%= @recipient.name || @recipient.email %>,
+
+Before we can send you any emails from <%= @site_name %>, we need you to click on the link below:
+
+    <%= confirm_email_url( @confirm_token ) %>
+
+Thank you!

config/locales/en.yml

@@ -102,6 +102,13 @@ en:
       failure: Failed to add email address to Do Not Contact list; please try again
       duplicate: Your email address is already on our Do Not Contact list
 
+  email_recipients:
+    confirm:
+      success: Thank you for verifying your email address
+      token_not_found: Failed to find email address; please check URL and try again
+      token_expired: Your confirmation link has expired; please request a new email and try again
+      # failure: Failed to confirm email address; please try again, or alert site admins
+
   errors:
     messages:
       slug_not_safe_at_top_level: cannot be used as a top-level slug
@@ -154,6 +161,10 @@ en:
     overview_notification:
       subject: '%{comment_author_name} commented on %{site_name}'
 
+  email_recipient_mailer:
+    confirm:
+      subject: Confirm your email address for %{site_name}
+
   user_mailer:
     welcome: Welcome, %{name}!
     hello: Hello %{name},

config/routes.rb

@@ -23,8 +23,10 @@
     get  'discussion/:id/:number', to: 'discussions#show_thread', as: :comment
     post 'discussion/:id/:number', to: 'discussions#add_reply'
 
-    get  'do-not-contact', to: 'do_not_contact#new'
-    post 'do-not-contact', to: 'do_not_contact#create'
+    get  'email/confirm/:token', to: 'email_recipients#confirm', as: :confirm_email
+
+    get  'email/do-not-contact', to: 'do_not_contact#new', as: :do_not_contact
+    post 'email/do-not-contact', to: 'do_not_contact#create'
 
     get 'site-settings', to: 'site_settings#index'
     put 'site-settings', to: 'site_settings#update'

db/archived-migrations/20201001180612_add_confirm_to_email_recipient.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+# ShinyCMS ~ https://shinycms.org
+#
+# Copyright 2009-2020 Denny de la Haye ~ https://denny.me
+#
+# ShinyCMS is free software; you can redistribute it and/or modify it under the terms of the GPL (version 2 or later)
+
+class AddConfirmToEmailRecipient < ActiveRecord::Migration[6.0]
+  def change
+    add_column :email_recipients, :confirm_token, :uuid
+    add_column :email_recipients, :confirm_sent_at, :timestamp, precision: 6
+    add_column :email_recipients, :confirmed_at, :timestamp, precision: 6
+  end
+end

db/schema.rb

@@ -12,7 +12,7 @@
 # migrations from scratch. Old migrations may fail to apply correctly if those
 # migrations use external dependencies or application code.
 
-ActiveRecord::Schema.define(version: 2020_09_15_225123) do
+ActiveRecord::Schema.define(version: 2020_10_01_180612) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
@@ -217,6 +217,9 @@
     t.string "email", null: false
     t.string "canonical_email", null: false
     t.uuid "token", default: -> { "gen_random_uuid()" }, null: false
+    t.uuid "confirm_token"
+    t.datetime "confirm_sent_at", precision: 6
+    t.datetime "confirmed_at", precision: 6
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
     t.index ["email"], name: "index_email_recipients_on_email", unique: true

docs/Developers/TODO.md

@@ -2,10 +2,7 @@
 
 ## Fixes and refactoring of code already written - to do next/soon
 
-* Double opt-in journey
-  * Email recipients
-    * List subscriptions
-    * Comment notifications
+* Add subscription-management links to list emails
 
 * Move pages, newsletters, and forms test templates into each plugin's spec/fixtures
 
@@ -78,6 +75,8 @@
 * 2FA
   * https://github.com/tinfoil/devise-two-factor
 
+* Allow an EmailRecipient to reset their token (in case they forward an email containing it to somebody else)
+
 * Configurable (per-site and per-user) menu order in admin area
 
 * Better tooling for loading (and ideally, for creating/updating) the demo data

plugins/ShinyBlog/spec/factories/shiny_blog/blog_posts.rb

@@ -14,7 +14,7 @@ module ShinyBlog
       body   { Faker::Lorem.paragraph }
       posted_at { Time.zone.now.iso8601 }
 
-      association :user
+      association :user, factory: :blog_admin
     end
   end
 end

plugins/ShinyLists/app/controllers/shiny_lists/subscriptions_controller.rb

@@ -14,7 +14,8 @@ def index
 
       if subscriber
         @subscriptions = subscriber.subscriptions
-        @subscriber    = subscriber
+
+        flash.now[:alert] = t( '.email_not_confirmed' ) unless subscriber.confirmed?
       else
         flash.now[:alert] = t( '.subscriber_not_found' )
       end

plugins/ShinyLists/app/views/shiny_lists/subscriptions/index.html.erb

@@ -9,7 +9,7 @@
 
   <% if @subscriptions.present? %>
   <p>
-    You are managing subscriptions for <strong><%= @subscriber.obfuscated_email %></strong>.
+    You are managing subscriptions for <strong><%= @subscriptions.first.subscriber.obfuscated_email %></strong>.
     If this is not you, please <% if user_signed_in? %>log out<% else %>check the link you used<% end %>!
   </p>
 

plugins/ShinyLists/config/locales/en.yml

@@ -30,6 +30,7 @@ en:
         title: Your mailing list subscriptions
         unsubscribe: Unsubscribe
         subscriber_not_found: Subscriber not found - please check link and try again
+        email_not_confirmed: You will not receive any email until your email address has been confirmed
       subscribe:
         success: You have been subscribed
         already_subscribed: You are already subscribed to this mailing list

plugins/ShinyLists/spec/factories/shiny_lists/subscriptions.rb

@@ -11,7 +11,7 @@ module ShinyLists
   FactoryBot.define do
     factory :mailing_list_subscription, class: 'ShinyLists::Subscription' do
       association :list,       factory: :mailing_list
-      association :subscriber, factory: :email_recipient
+      association :subscriber, factory: %i[ email_recipient confirmed ]
 
       association :consent_version
     end

plugins/ShinyLists/spec/requests/subscriptions_spec.rb

@@ -39,7 +39,7 @@
 
   describe 'GET /subscriptions/:token' do
     it "displays a token-identified user's subscriptions" do
-      subscriber1 = create :email_recipient
+      subscriber1 = create :email_recipient, :confirmed
 
       get shiny_lists.token_list_subscriptions_path( subscriber1.token )
 
@@ -225,7 +225,7 @@
   describe 'PUT /list/:slug/unsubscribe/:token' do
     it 'unsubscribes a token-authenticated user' do
       list1 = create :mailing_list
-      subscriber1 = create :email_recipient
+      subscriber1 = create :email_recipient, :confirmed
       create :mailing_list_subscription, list: list1, subscriber: subscriber1
 
       put shiny_lists.token_list_unsubscribe_path( list1.slug, subscriber1.token )

plugins/ShinyNewsletters/app/controllers/shiny_newsletters/newsletters_controller.rb

@@ -29,7 +29,7 @@ def show
     private
 
     def subscriber
-      current_user || EmailRecipient.find_by( token: params[:token] )
+      current_user || EmailRecipient.confirmed.find_by( token: params[:token] )
     end
 
     def newsletters_sent_to_subscribed_lists

plugins/ShinyNewsletters/app/jobs/shiny_newsletters/send_to_list_job.rb

@@ -17,6 +17,8 @@ def perform( send )
       send.update!( started_sending_at: Time.zone.now )
 
       send.list.subscriptions.each do |subscription|
+        next if subscription.subscriber.do_not_email?
+
         SendToSubscriberJob.perform_later( send, subscription.subscriber )
       end
 

plugins/ShinyNewsletters/app/jobs/shiny_newsletters/send_to_subscriber_job.rb

@@ -10,6 +10,7 @@ module ShinyNewsletters
   # Send an edition of a newsletter to an individual subscriber on a list
   class SendToSubscriberJob < ApplicationJob
     def perform( send, subscriber )
+      return if subscriber.do_not_email?
       return if send.sent?
 
       return unless send.list.subscribed? subscriber.email

plugins/ShinyNewsletters/app/mailers/shiny_newsletters/newsletter_mailer.rb

@@ -15,7 +15,7 @@ def send_email( edition, recipient )
       stash_content( edition )
       stash_user( recipient )
 
-      return if DoNotContact.include? recipient.email # TODO: make this happen without explicit call
+      return if @user.do_not_email? # TODO: make this happen without explicit call
 
       mail to: @user.email_to, subject: @edition.subject, template_name: @edition.template.filename do |format|
         format.html