Merge pull request #378 from basecamp/update-ca-certificates-1.5

Update the trusted CA certificates (1.5.6)

.github/workflows/ci.yml

@@ -4,7 +4,7 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    branches: [ main ]
+    branches: [ main, "release-*" ]
 
 permissions: {}
 

Dockerfile

@@ -9,10 +9,23 @@ WORKDIR /rails
 
 # Install base packages
 RUN apt-get update -qq && \
-    apt-get install --no-install-recommends -y curl libsqlite3-0 libvips libjemalloc2 ffmpeg redis && \
+    apt-get install --no-install-recommends -y ca-certificates curl libsqlite3-0 libvips libjemalloc2 ffmpeg redis && \
     ln -s /usr/lib/$(uname -m)-linux-gnu/libjemalloc.so.2 /usr/local/lib/libjemalloc.so && \
     rm -rf /var/lib/apt/lists /var/cache/apt/archive
 
+# Naming ca-certificates above takes the distribution's current trusted roots rather than the
+# ones frozen into the base image. Fail the build if either of these withdrawn roots is still
+# where Ruby's OpenSSL looks for trusted certificates.
+RUN ruby -ropenssl -e ' \
+    file = ENV[OpenSSL::X509::DEFAULT_CERT_FILE_ENV] || OpenSSL::X509::DEFAULT_CERT_FILE; \
+    dir = ENV[OpenSSL::X509::DEFAULT_CERT_DIR_ENV] || OpenSSL::X509::DEFAULT_CERT_DIR; \
+    pems = [file, *Dir.glob(File.join(dir, "*"))].select { |path| File.file?(path) } \
+      .flat_map { |path| File.read(path).scan(/-----BEGIN CERTIFICATE-----.+?-----END CERTIFICATE-----/m) }; \
+    subjects = pems.map { |pem| OpenSSL::X509::Certificate.new(pem).subject.to_s }.uniq; \
+    abort "Only #{subjects.size} roots in #{file} and #{dir}" if subjects.size < 100; \
+    withdrawn = subjects.grep(%r{/CN=(emSign ECC Root CA - G3|AAA Certificate Services)\z}); \
+    abort "Withdrawn roots still trusted: #{withdrawn}" if withdrawn.any?'
+
 # Set production environment
 ENV RAILS_ENV="production" \
     BUNDLE_DEPLOYMENT="1" \

Gemfile.lock

@@ -146,7 +146,7 @@ GEM
     bcrypt (3.1.22)
     benchmark (0.5.0)
     bigdecimal (4.1.3)
-    brakeman (8.0.6)
+    brakeman (8.1.0)
       racc
     builder (3.3.0)
     bundler-audit (0.9.3)

app/channels/application_cable/connection.rb

@@ -2,19 +2,42 @@ module ApplicationCable
   class Connection < ActionCable::Connection::Base
     include Authentication::SessionLookup
 
+    SESSION_CHECK_INTERVAL = 1.minute
+
     identified_by :current_user
 
     def connect
       self.current_user = find_verified_user
+      @session_checked_at = Time.now
+    end
+
+    # Signing out or changing the password ends sessions and then disconnects their connections, but a connection can
+    # miss that: one that found its session just before isn't listening yet. So on the server's heartbeat, which
+    # reaches every open connection, each one looks its session up again now and then, and closes once it's gone. A
+    # rejected connection has no session, and stays registered until its socket closes.
+    def beat
+      super
+      check_session if @session_checked_at&.before?(SESSION_CHECK_INTERVAL.ago)
     end
 
     private
       def find_verified_user
-        if verified_session = find_session_by_cookie
+        if @verified_session = find_session_by_cookie
-          verified_session.user
+          @verified_session.user
         else
           reject_unauthorized_connection
         end
       end
+
+      def check_session
+        @session_checked_at = Time.now
+        perform_work -> { close_unless_session_remains }, :call
+      end
+
+      def close_unless_session_remains
+        unless Session.exists?(@verified_session.id)
+          close reason: ActionCable::INTERNAL[:disconnect_reasons][:unauthorized]
+        end
+      end
   end
 end

app/controllers/application_controller.rb

@@ -1,4 +1,4 @@
 class ApplicationController < ActionController::Base
-  include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders
+  include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, InlineScriptHashes
   include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName
 end

app/controllers/concerns/authentication.rb

@@ -57,7 +57,7 @@ module Authentication
     end
 
     def start_new_session_for(user)
-      user.sessions.start!(user_agent: request.user_agent, ip_address: request.remote_ip).tap do |session|
+      user.sessions.create!(user_agent: request.user_agent, ip_address: request.remote_ip).tap do |session|
         authenticated_as session
       end
     end

app/controllers/concerns/inline_script_hashes.rb

@@ -0,0 +1,50 @@
+# Allows the scripts the importmap helper emits and nothing else: the inline
+# importmap and its entry-point module by content hash, and the asset
+# directory its URLs point into. Not the whole origin, because uploads are
+# served from this origin too. The hashes change only when the importmap does
+# (a deploy), so cached HTML stays valid, unlike a per-request nonce.
+module InlineScriptHashes
+  extend ActiveSupport::Concern
+
+  ENTRY_POINT_SCRIPT = %(import "application")
+
+  included do
+    # With the policy turned off there's nothing to add to, so leave the
+    # request without one rather than starting an empty policy
+    content_security_policy if: -> { request.content_security_policy } do |policy|
+      if sources = policy.directives["script-src"]
+        importmap = Rails.application.importmap.to_json(resolver: helpers)
+        policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
+        policy.worker_src "#{request.host_with_port}#{service_worker_path(format: :js)}"
+      end
+    end
+  end
+
+  private
+    # The importmap is resolved by Rails' asset helpers, so its URLs already
+    # carry any relative root and asset host, however the host is written:
+    # with or without a scheme, protocol-relative, a %d set, or a proc. Each
+    # is allowed up to its asset directory, the last one in its path, since an
+    # asset host's own path may contain one too; a URL outside it is allowed
+    # exactly.
+    #
+    # This origin's own sources leave out the scheme, so they match the page's
+    # scheme even behind a proxy that terminates TLS without saying so.
+    #
+    # A path only limits the first request: browsers check just the origin
+    # after a redirect. Nothing under the asset directory redirects.
+    def script_locations(importmap)
+      assets = File.join(config.relative_url_root.to_s, Rails.application.config.assets.prefix, "")
+
+      JSON.parse(importmap)["imports"].values.map do |url|
+        url = URI.join(request.base_url, url)
+        origin = url.origin == request.base_url ? request.host_with_port : url.origin
+        directory = url.path[0, url.path.rindex(assets) + assets.length] if url.path.include?(assets)
+        "#{origin}#{directory || url.path}"
+      end.uniq
+    end
+
+    def script_hashes(*scripts)
+      scripts.map { |script| "'sha256-#{Digest::SHA256.base64digest(script)}'" }
+    end
+end

app/controllers/sessions/transfers_controller.rb

@@ -11,5 +11,7 @@ class Sessions::TransfersController < ApplicationController
     else
       head :bad_request
     end
+  rescue ActiveRecord::RecordInvalid
+    head :bad_request
   end
 end

app/controllers/sessions_controller.rb

@@ -14,6 +14,8 @@ class SessionsController < ApplicationController
     else
       render_rejection :unauthorized
     end
+  rescue ActiveRecord::RecordInvalid
+    render_rejection :unauthorized
   end
 
   def destroy

app/controllers/users/profiles_controller.rb

@@ -1,14 +1,26 @@
 class Users::ProfilesController < ApplicationController
   before_action :set_user
 
+  # Limited like signing in, so the current password can't be guessed here instead.
+  rate_limit to: 10, within: 3.minutes, only: :update, if: :answering_password_challenge?,
+    with: -> { redirect_to user_profile_url, alert: "Too many attempts. Try again in a few minutes." }
+
   def show
     @direct_memberships, @shared_memberships =
       Current.user.memberships.with_ordered_room.partition { |m| m.room.direct? }
   end
 
   def update
-    @user.update user_params
+    if @user.update user_params
-    redirect_to user_profile_url, notice: update_notice
+      start_new_session_for @user if @user.saved_change_to_password_digest?
+
+      redirect_to user_profile_url, notice: update_notice
+    else
+      redirect_to user_profile_url, alert: update_alert
+    end
+  rescue ActiveRecord::RecordInvalid
+    # The password changed again before this browser's new session could start, and that change signed it out.
+    redirect_to new_session_url
   end
 
   private
@@ -16,11 +28,35 @@ class Users::ProfilesController < ApplicationController
       @user = Current.user
     end
 
+    # Always with the challenge, so the model checks it whenever the save would change the password, however the
+    # request spelled the new one.
     def user_params
       params.require(:user).permit(:name, :avatar, :email_address, :password, :bio).compact
+        .merge(password_challenge: password_challenge)
+    end
+
+    # Only a string is a challenge, so the rate limit counts exactly what the model checks.
+    def password_challenge
+      challenge = params.dig(:user, :password_challenge)
+      challenge.is_a?(String) ? challenge : ""
+    end
+
+    # Guessing the current password means sending one, so this is what the rate limit counts.
+    def answering_password_challenge?
+      !password_challenge.empty?
     end
 
     def update_notice
       params[:user][:avatar] ? "It may take up to 30 minutes to change everywhere." : "✓"
     end
+
+    def update_alert
+      if !@user.errors.include?(:password_challenge)
+        @user.errors.full_messages.to_sentence
+      elsif password_challenge.empty?
+        "Enter your current password to change it."
+      else
+        "That isn't your current password."
+      end
+    end
 end

app/controllers/users/push_subscriptions_controller.rb

@@ -5,17 +5,14 @@ class Users::PushSubscriptionsController < ApplicationController
   end
 
   def create
-    if subscription = @push_subscriptions.find_by(push_subscription_params)
+    subscription = @push_subscriptions.find_or_initialize_by(push_subscription_params)
-      # Existing endpoints must pass current validations
+    subscription.user_agent = request.user_agent if subscription.new_record?
-      if subscription.valid?
+
-        subscription.touch
+    # Existing endpoints must pass current validations too
-        head :ok
+    if subscription.valid?
-      else
+      head register_while_signed_in(subscription) ? :ok : :unauthorized
-        head :unprocessable_entity
-      end
     else
-      subscription = @push_subscriptions.create push_subscription_params.merge(user_agent: request.user_agent)
+      head :unprocessable_entity
-      head subscription.persisted? ? :ok : :unprocessable_entity
     end
   end
 
@@ -29,6 +26,24 @@ class Users::PushSubscriptionsController < ApplicationController
       @push_subscriptions = Current.user.push_subscriptions
     end
 
+    # A password change ends every session and subscription. A request that authenticated just before it mustn't
+    # put one back, so the session is checked under the same write lock as the save, and the endpoint is looked up
+    # again there, past the query cache, so two tabs posting at once register it once. Validation, which resolves
+    # the endpoint's address, runs before taking the lock.
+    def register_while_signed_in(subscription)
+      Push::Subscription.uncached do
+        Push::Subscription.transaction do
+          if Session.exists?(Current.session.id)
+            if existing = @push_subscriptions.find_by(push_subscription_params)
+              existing.touch
+            else
+              subscription.save!(validate: false)
+            end
+          end
+        end
+      end
+    end
+
     def push_subscription_params
       params.require(:push_subscription).permit(:endpoint, :p256dh_key, :auth_key)
     end

app/helpers/content_filters.rb

@@ -5,7 +5,7 @@ module ContentFilters
   #
   #   * SanitizeTags strips disallowed markup from the message body
   #   * Action Text sanitizes the rendered content (lib/rails_ext/action_text_allowed_tags.rb)
-  #   * auto_link re-sanitizes the final html (MessagesHelper#message_presentation)
+  #   * the final html is re-sanitized before its text is linked (MessagesHelper#auto_link_text)
   EDITOR_FORMATTING_TAGS = %w[ s u mark table thead tbody tfoot tr th td ]
   EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ]
 

app/helpers/messages_helper.rb

@@ -1,9 +1,12 @@
 module MessagesHelper
-  # auto_link re-sanitizes with Rails' default safe list, which lacks the
+  # Rendered text is re-sanitized before linking. Rails' default safe list
-  # formatting the rich text editor produces
+  # lacks the formatting the rich text editor produces
   AUTO_LINK_ALLOWED_TAGS = Rails::HTML5::SafeListSanitizer.allowed_tags + ContentFilters::EDITOR_FORMATTING_TAGS
   AUTO_LINK_ALLOWED_ATTRIBUTES = Rails::HTML5::SafeListSanitizer.allowed_attributes + ContentFilters::EDITOR_FORMATTING_ATTRIBUTES
 
+  # Text auto_link could turn into a link: a URL has a scheme or www., an email address an @
+  LINKABLE_TEXT = /:|@|www\./i
+
   def message_area_tag(room, &)
     tag.div id: "message-area", class: "message-area", contents: true, data: {
       controller: "messages presence drop-target",
@@ -63,8 +66,7 @@ module MessagesHelper
     when "sound"
       message_sound_presentation(message)
     else
-      auto_link h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body)),
+      auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body))
-        html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES }
     end
   rescue Exception => e
     Sentry.capture_exception(e, extra: { message: message })
@@ -74,6 +76,24 @@ module MessagesHelper
   end
 
   private
+    # auto_link finds URLs and email addresses with regular expressions, which
+    # can't tell an attribute value from text. Hand it one text node at a time,
+    # escaped as the serializer writes text, so a link can only land between tags.
+    def auto_link_text(html)
+      fragment = Nokogiri::HTML5.fragment(sanitize(html, tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES))
+
+      fragment.xpath(".//text()[not(ancestor::a)]").each do |node|
+        if node.text.match?(LINKABLE_TEXT)
+          text = node.to_html
+          linked = auto_link(text, html: { target: "_blank" })
+          node.replace Nokogiri::HTML5.fragment(linked) unless linked == text
+        end
+      end
+
+      # A <pre> starting with a newline needs an extra one written out to survive the next parse
+      fragment.to_html(preserve_newline: true).html_safe
+    end
+
     def messages_actions
       "turbo:before-stream-render@document->messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage"
     end

app/helpers/translations_helper.rb

@@ -2,6 +2,7 @@ module TranslationsHelper
   TRANSLATIONS = {
     email_address:  { "🇺🇸": "Enter your email address", "🇪🇸": "Introduce tu correo electrónico", "🇫🇷": "Entrez votre adresse courriel", "🇮🇳": "अपना ईमेल पता दर्ज करें", "🇩🇪": "Geben Sie Ihre E-Mail-Adresse ein", "🇧🇷": "Insira seu endereço de email", "🇯🇵": "メールアドレスを入力してください" },
     password: { "🇺🇸": "Enter your password", "🇪🇸": "Introduce tu contraseña", "🇫🇷": "Saisissez votre mot de passe", "🇮🇳": "अपना पासवर्ड दर्ज करें", "🇩🇪": "Geben Sie Ihr Passwort ein", "🇧🇷": "Insira sua senha", "🇯🇵": "パスワードを入力してください" },
+    current_password: { "🇺🇸": "Enter your current password", "🇪🇸": "Introduce tu contraseña actual", "🇫🇷": "Saisissez votre mot de passe actuel", "🇮🇳": "अपना वर्तमान पासवर्ड दर्ज करें", "🇩🇪": "Geben Sie Ihr aktuelles Passwort ein", "🇧🇷": "Insira sua senha atual", "🇯🇵": "現在のパスワードを入力してください" },
     update_password: { "🇺🇸": "Change password", "🇪🇸": "Cambiar contraseña", "🇫🇷": "Changer le mot de passe", "🇮🇳": "पासवर्ड बदलें", "🇩🇪": "Passwort ändern", "🇧🇷": "Alterar senha", "🇯🇵": "パスワードを変更" },
     user_name: { "🇺🇸": "Enter your name", "🇪🇸": "Introduce tu nombre", "🇫🇷": "Entrez votre nom", "🇮🇳": "अपना नाम दर्ज करें", "🇩🇪": "Geben Sie Ihren Namen ein", "🇧🇷": "Insira seu nome", "🇯🇵": "お名前を入力してください" },
     account_name: { "🇺🇸": "Name this account", "🇪🇸": "Nombre de esta cuenta", "🇫🇷": "Nommez ce compte", "🇮🇳": "इस खाते का नाम दें", "🇩🇪": "Benennen Sie dieses Konto", "🇧🇷": "Dê um nome a essa conta", "🇯🇵": "アカウントに名前を付ける" },

app/helpers/users/push_subscriptions_helper.rb

@@ -0,0 +1,7 @@
+module Users::PushSubscriptionsHelper
+  # Names the session without revealing its token, so the browser can tell when it's in a new one. The creation
+  # time keeps an id reused by a new database from looking like the same session.
+  def push_subscription_session_marker
+    Digest::SHA256.hexdigest("push-subscription-session-#{Current.session.id}-#{Current.session.created_at.to_f}") if Current.session
+  end
+end

app/javascript/controllers/notifications_controller.js

@@ -106,7 +106,8 @@ export default class extends Controller {
 
   async #syncPushSubscription(subscription) {
     const response = await post(this.subscriptionsUrlValue, { body: this.#extractJsonPayloadAsString(subscription), responseKind: "turbo-stream" })
-    if (!response.ok) subscription.unsubscribe()
+    // Only a refused endpoint is for good. A session signed out mid-request posts it again after signing in.
+    if (response.unprocessableEntity) subscription.unsubscribe()
   }
 
   async #requestPermissionAndSubscribe(registration) {

app/javascript/controllers/push_subscription_sync_controller.js

@@ -0,0 +1,32 @@
+import { Controller } from "@hotwired/stimulus"
+import { post } from "@rails/request.js"
+
+const SYNCED_SESSION_KEY = "push-subscription-synced-session"
+
+// A password change removes the account's push subscriptions, the one this browser holds included. Post it again
+// once per session, so a browser that's still signed in keeps getting notifications. A failure here leaves the
+// browser's own subscription alone: the next session tries again.
+export default class extends Controller {
+  static values = { url: String, session: String }
+
+  async connect() {
+    if (this.#allowed && this.sessionValue && localStorage.getItem(SYNCED_SESSION_KEY) !== this.sessionValue) {
+      const registration = await navigator.serviceWorker.getRegistration(window.location.origin)
+      const subscription = await registration?.pushManager?.getSubscription()
+
+      if (Notification.permission === "granted" && subscription) {
+        await post(this.urlValue, { body: this.#payload(subscription), responseKind: "turbo-stream" })
+        localStorage.setItem(SYNCED_SESSION_KEY, this.sessionValue)
+      }
+    }
+  }
+
+  get #allowed() {
+    return navigator.serviceWorker && window.Notification
+  }
+
+  #payload(subscription) {
+    const { endpoint, keys: { p256dh, auth } } = subscription.toJSON()
+    return JSON.stringify({ push_subscription: { endpoint, p256dh_key: p256dh, auth_key: auth } })
+  }
+}

app/javascript/controllers/reply_controller.js

@@ -28,7 +28,7 @@ export default class extends Controller {
   }
 
   #stripMentionAttachments(node) {
-    node.querySelectorAll(".mention").forEach(mention => mention.outerHTML = mention.textContent.trim())
+    node.querySelectorAll(".mention").forEach(mention => mention.replaceWith(mention.textContent.trim()))
     return node
   }
 

app/models/opengraph/fetch.rb

@@ -23,11 +23,14 @@ class Opengraph::Fetch
   end
 
   private
+    # The nil proxy address ignores http_proxy: a proxy would look the host up again, and the connection
+    # would no longer go to the checked IP.
+    #
     # The timeouts bound each operation. A host can still send its headers or body a byte at a time,
     # in as many reads as it likes: UnfurlLinksController puts the whole unfurl under one deadline.
     def request(url, request_class, ip:)
       MAX_REDIRECTS.times do
-        Net::HTTP.start(url.host, url.port, ipaddr: ip, use_ssl: url.scheme == "https", **timeouts) do |http|
+        Net::HTTP.start(url.host, url.port, nil, ipaddr: ip, use_ssl: url.scheme == "https", **timeouts) do |http|
           http.request request_class.new(url) do |response|
             if response.is_a?(Net::HTTPRedirection)
               url, ip = resolve_redirect(response["location"])

app/models/session.rb

@@ -7,13 +7,22 @@ class Session < ApplicationRecord
 
   before_create { self.last_active_at ||= Time.now }
 
-  def self.start!(user_agent:, ip_address:)
+  validate :user_password_unchanged, on: :create
-    create! user_agent: user_agent, ip_address: ip_address
-  end
 
   def resume(user_agent:, ip_address:)
     if last_active_at.before?(ACTIVITY_REFRESH_RATE.ago)
       update! user_agent: user_agent, ip_address: ip_address, last_active_at: Time.now
     end
   end
+
+  private
+    # A password change ends every session, so one can't start on a password checked before the change. Validated in
+    # the save's transaction, which holds the database's write lock from the start, so a change commits either before
+    # this check, and fails it, or after the insert, and ends this session with the others. Read past the query cache,
+    # and compared here rather than in the query, which logs its values.
+    def user_password_unchanged
+      unless User.uncached { User.where(id: user_id).pick(:password_digest) } == user.password_digest
+        errors.add :user, :invalid
+      end
+    end
 end

app/models/user.rb

@@ -18,8 +18,10 @@ class User < ApplicationRecord
   enum :status, %i[ active deactivated banned ], default: :active
 
   has_secure_password validations: false
+  validate :password_challenge_matches, if: :will_save_change_to_password_digest?, unless: -> { password_challenge.nil? }
 
   after_create_commit :grant_membership_to_open_rooms
+  after_update :sign_out_all_sessions, if: :saved_change_to_password_digest?
 
   scope :ordered, -> { order("LOWER(name)") }
   scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") }
@@ -50,6 +52,31 @@ class User < ApplicationRecord
   end
 
   private
+    # The profile always sets password_challenge, and it's checked whenever the save would change the password; scripts
+    # that reset one don't set it.
+    def password_challenge_matches
+      unless password_digest_was.present? && BCrypt::Password.new(password_digest_was).is_password?(password_challenge)
+        errors.add :password_challenge, :invalid
+      end
+    end
+
+    # A new password is how someone shuts out whoever else got into their account, so it ends every session, copies
+    # of the changing browser's cookie included, and every push subscription; browsers still signed in post theirs
+    # again. Connections close once that commits, registered on the transaction because a later save in it would
+    # replace the saved change an after_commit condition reads.
+    def sign_out_all_sessions
+      sessions.delete_all
+      push_subscriptions.delete_all
+      self.class.current_transaction.after_commit { disconnect_signed_out_sessions }
+    end
+
+    # The password is saved by now, so a realtime service that can't be reached mustn't fail the change.
+    def disconnect_signed_out_sessions
+      reset_remote_connections
+    rescue => error
+      Rails.logger.warn "Could not disconnect remote connections after a password change: #{error.class}"
+    end
+
     # One statement, which SQLite runs under the write lock, so no room is closed between reading the open ones and
     # inserting. A room that Room#destroy_later closes is either closed before, and skipped, or after, and takes this
     # membership away with the others. A room that granted itself to every user when it was created is skipped too,

app/models/user/transferable.rb

@@ -3,13 +3,20 @@ module User::Transferable
 
   TRANSFER_LINK_EXPIRY_DURATION = 4.hours
 
+  included do
+    # Bound to the password, so changing it also ends sign-in links made before.
+    generates_token_for :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION do
+      password_salt&.last(10)
+    end
+  end
+
   class_methods do
     def find_by_transfer_id(id)
-      find_signed(id, purpose: :transfer)
+      find_by_token_for(:transfer, id)
     end
   end
 
   def transfer_id
-    signed_id(purpose: :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION)
+    generate_token_for(:transfer)
   end
 end

app/views/layouts/application.html.erb

@@ -63,6 +63,11 @@
 
     <%= render "layouts/lightbox" %>
 
+    <% if Current.session %>
+      <%= tag.div hidden: true, data: { controller: "push-subscription-sync",
+            push_subscription_sync_url_value: user_push_subscriptions_path, push_subscription_sync_session_value: push_subscription_session_marker } %>
+    <% end %>
+
     <a href="https://once.com" id="app-logo" target="_blank" aria-label="Once software from 37signals home page">
       <%= image_tag "campfire-icon.png", alt: "Campfire logo", width: 256, height: 216 %>
     </a>

app/views/messages/_message.html.erb

@@ -1,7 +1,7 @@
 <%# Be sure to check/update messages/_template.html.erb when changing this file %>
 
 <%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %>
-<% cache [ message, "presentation-v6" ] do %>
+<% cache [ message, "presentation-v7" ] do %>
   <%= message_tag message do %>
     <h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
 

app/views/users/profiles/show.html.erb

@@ -69,6 +69,16 @@
         </label>
       </div>
 
+      <div class="flex align-center gap">
+        <%= translation_button(:current_password) %>
+
+        <label class="flex align-center gap flex-item-grow input input--actor">
+          <%= form.password_field :password_challenge, class: "input txt-large", autocomplete: "current-password",
+                placeholder: "Current password", required: false, maxlength: 72 %>
+          <%= image_tag "password.svg", aria: { hidden: "true" }, size: 24, class: "colorize--black" %>
+        </label>
+      </div>
+
       <div class="flex align-center gap">
         <%= translation_button(:update_password) %>
 

config/initializers/content_security_policy.rb

@@ -1,25 +1,21 @@
-# Be sure to restart your server when you modify this file.
+# Enforcing baseline policy. Application pages narrow script-src from this
-
+# origin to its asset directory, since uploads are served from this origin
-# Define an application-wide content security policy.
+# too, and allow the two inline scripts the importmap helper emits by hash
-# See the Securing Rails Applications Guide for more information:
+# (InlineScriptHashes). There is no per-request nonce, so rendered HTML stays
-# https://guides.rubyonrails.org/security.html#content-security-policy-header
+# cacheable. 'self' remains for responses outside ApplicationController, none
-
+# of which render user content.
-# Rails.application.configure do
-#   config.content_security_policy do |policy|
-#     policy.default_src :self, :https
-#     policy.font_src    :self, :https, :data
-#     policy.img_src     :self, :https, :data
-#     policy.object_src  :none
-#     policy.script_src  :self, :https
-#     policy.style_src   :self, :https
-#     # Specify URI for violation reports
-#     # policy.report_uri "/csp-violation-report-endpoint"
-#   end
 #
-#   # Generate session nonces for permitted importmap and inline scripts
+# Nothing else is restricted: images, media, fonts, styles (including an
-#   config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
+# account's custom styles) and connections behave as before.
-#   config.content_security_policy_nonce_directives = %w(script-src)
 #
-#   # Report violations without enforcing the policy.
+# DISABLE_CONTENT_SECURITY_POLICY=true is the escape hatch for an install whose
-#   # config.content_security_policy_report_only = true
+# proxy or customization injects scripts the policy would block.
-# end
+unless ENV["DISABLE_CONTENT_SECURITY_POLICY"] == "true"
+  Rails.application.configure do
+    config.content_security_policy do |policy|
+      policy.script_src :self
+      policy.object_src :none
+      policy.base_uri   :self
+    end
+  end
+end

config/initializers/uploaded_script_guard.rb

@@ -0,0 +1,34 @@
+# Uploaded files are served from this origin. Application pages only allow
+# scripts from the asset directory (InlineScriptHashes), so an upload can't
+# load as a script whatever type it's served with. This guard is the second
+# layer: browsers accept sixteen spellings of a JavaScript type, and Disk URLs
+# sign the type they were minted with, so instead of refusing a list of types,
+# anything Active Storage serves that isn't plainly media goes out as a
+# download, whatever its URL says. A multi-range response keeps its
+# multipart/byteranges type and boundary, which no browser runs as a script.
+# Only reads are touched: uploading (direct upload JSON, Disk PUT) answers in
+# its own types.
+#
+# A cache that stored an upload under a script type before this shipped can
+# still serve it that way until the entry expires (an hour for Disk URLs, and
+# never for blob proxy URLs, which Rails caches as immutable). The policy's
+# path restriction is what keeps such a copy from running.
+class UploadedScriptGuard
+  SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain|multipart/byteranges)\z}
+
+  def initialize(app)
+    @app = app
+  end
+
+  def call(env)
+    status, headers, body = @app.call(env)
+
+    if env["REQUEST_METHOD"].in?(%w[ GET HEAD ]) && env["PATH_INFO"].start_with?("#{ActiveStorage.routes_prefix}/") && (type = headers["content-type"])
+      headers["content-type"] = ActiveStorage.binary_content_type unless type.split(";").first.strip.downcase.match?(SERVABLE)
+    end
+
+    [ status, headers, body ]
+  end
+end
+
+Rails.application.config.middleware.use UploadedScriptGuard

test/channels/application_cable/connection_test.rb

@@ -18,4 +18,53 @@ class ApplicationCable::ConnectionTest < ActionCable::Connection::TestCase
 
     assert_reject_connection { connect }
   end
+
+  test "closes once its session has ended without it hearing" do
+    cookies.signed[:session_token] = sessions(:david_safari).token
+    connect
+    sessions(:david_safari).destroy
+
+    travel 2.minutes
+    connection.beat
+
+    assert socket.closed
+    assert_equal "disconnect", socket.transmissions.last[:type]
+    assert socket.transmissions.last[:reconnect]
+  end
+
+  test "keeps checking its session while open" do
+    cookies.signed[:session_token] = sessions(:david_safari).token
+    connect
+
+    travel 2.minutes
+    connection.beat
+    assert_not socket.closed
+
+    sessions(:david_safari).destroy
+    travel 2.minutes
+    connection.beat
+    assert socket.closed
+  end
+
+  test "checks its session at most once a minute" do
+    cookies.signed[:session_token] = sessions(:david_safari).token
+    connect
+    sessions(:david_safari).destroy
+
+    travel 30.seconds
+    connection.beat
+
+    assert_not socket.closed
+  end
+
+  test "a rejected connection's heartbeat doesn't fail" do
+    request = ActionCable::Connection::TestSocket.build_request(ActionCable.server.config.mount_path || "/cable", cookies: cookies)
+    rejected = ApplicationCable::Connection.new \
+      ActionCable::Connection::TestServer.new(ActionCable.server), ActionCable::Connection::TestSocket.new(request)
+    rejected.handle_open
+
+    travel 2.minutes
+
+    assert_nothing_raised { rejected.beat }
+  end
 end

test/controllers/sessions/transfers_controller_test.rb

@@ -19,4 +19,30 @@ class Sessions::TransfersControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to root_url
     assert parsed_cookies.signed[:session_token]
   end
+
+  test "update rejects a link made before the password changed" do
+    user = users(:david)
+    transfer_id = user.transfer_id
+    user.update!(password: "a new secret123456")
+
+    put session_transfer_url(transfer_id)
+
+    assert_response :bad_request
+    assert_nil parsed_cookies.signed[:session_token]
+  end
+
+  test "update is refused when the password changes after the link was checked" do
+    user = users(:david)
+    transfer_id = user.transfer_id
+    stale = User.find(user.id)
+    User.find(user.id).update!(password: "a new secret123456")
+    User.stubs(:find_by_transfer_id).with(transfer_id).returns(stale)
+
+    assert_no_difference -> { Session.count } do
+      put session_transfer_url(transfer_id)
+    end
+
+    assert_response :bad_request
+    assert_nil parsed_cookies.signed[:session_token]
+  end
 end

test/controllers/sessions_controller_test.rb

@@ -36,6 +36,19 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest
     assert parsed_cookies.signed[:session_token]
   end
 
+  test "create is refused when the password changes after it was checked" do
+    user = User.find(users(:david).id)
+    User.find(user.id).update!(password: "a new secret123456")
+    User.stubs(:authenticate_by).returns(user)
+
+    assert_no_difference -> { Session.count } do
+      post session_url, params: { email_address: "david@37signals.com", password: "secret123456" }
+    end
+
+    assert_response :unauthorized
+    assert_nil parsed_cookies.signed[:session_token]
+  end
+
   test "create with invalid credentials" do
     post session_url, params: { email_address: "david@37signals.com", password: "wrong" }
 

test/controllers/users/profiles_controller_test.rb

@@ -27,9 +27,190 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     assert_equal "david@37signals.com", users(:david).email_address
   end
 
+  test "changing the password signs out the account's other devices but not this browser" do
+    other_device = open_session
+    other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
+    other_device.get user_profile_url
+    other_device.assert_response :success
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    assert_redirected_to user_profile_url
+
+    other_device.get user_profile_url
+    other_device.assert_redirected_to new_session_url
+    get user_profile_url
+    assert_response :success
+    assert_equal 1, users(:david).sessions.count
+  end
+
+  test "changing the password signs this browser out when it changes again before its new session starts" do
+    User.stubs(:uncached).returns(BCrypt::Password.create("someone else's secret", cost: BCrypt::Engine::MIN_COST))
+
+    assert_changes -> { users(:david).sessions.count }, to: 0 do
+      put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    end
+
+    assert_redirected_to new_session_url
+  end
+
+  test "changing the password gives this browser a new session, so a copy of its cookie stops working" do
+    copied_cookie = cookies[:session_token]
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    assert_not_equal copied_cookie, cookies[:session_token]
+
+    copy = open_session
+    copy.cookies[:session_token] = copied_cookie
+    copy.get user_profile_url
+    copy.assert_redirected_to new_session_url
+  end
+
+  test "changing the password along with the avatar still signs this browser back in and closes live connections" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456", avatar: fixture_file_upload("moon.jpg", "image/jpeg") } }
+    assert_redirected_to user_profile_url
+    assert users(:david).reload.avatar.attached?
+
+    get user_profile_url
+    assert_response :success
+  end
+
+  test "changing the password closes the account's live connections" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+
+    assert_redirected_to user_profile_url
+  end
+
+  test "changing the password still signs out other sessions when the realtime service is unreachable" do
+    other_session = users(:david).sessions.first
+    ActionCable.server.stubs(:remote_connections).raises(RuntimeError.new("cable down"))
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+
+    assert_redirected_to user_profile_url
+    assert_not Session.exists?(other_session.id)
+  end
+
+  test "updating the profile without a new password leaves other sessions signed in" do
+    ActionCable.server.expects(:remote_connections).never
+
+    assert_no_difference -> { users(:david).sessions.count } do
+      put user_profile_url, params: { user: { name: "David", password: "" } }
+    end
+
+    assert_redirected_to user_profile_url
+  end
+
+  test "changing the password with the current one" do
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+
+    assert_redirected_to user_profile_url
+    assert_equal "✓", flash[:notice]
+    assert users(:david).reload.authenticate("a new secret123456")
+  end
+
+  test "changing the password without the current one changes nothing" do
+    assert_password_change_refused "Enter your current password to change it.",
+      user: { name: "Someone else", password: "a new secret123456" }
+  end
+
+  test "changing the password with a wrong current one changes nothing" do
+    assert_password_change_refused "That isn't your current password.",
+      user: { name: "Someone else", password: "a new secret123456", password_challenge: "not my password" }
+  end
+
+  test "a new password needs the current one however the form spells it" do
+    assert_password_change_refused "Enter your current password to change it.",
+      user: { name: "Someone else", "password(1)" => "a new secret123456" }
+  end
+
+  test "editing the profile with an empty current password doesn't need it" do
+    put user_profile_url, params: { user: { name: "Dave", password: "", password_challenge: "" } }
+
+    assert_redirected_to user_profile_url
+    assert_equal "Dave", users(:david).reload.name
+  end
+
+  test "editing the profile without changing the password doesn't need the current one" do
+    put user_profile_url, params: { user: { name: "Dave", bio: "Acrobat", password: "" } }
+
+    assert_redirected_to user_profile_url
+    assert_equal "Dave", users(:david).reload.name
+    assert users(:david).authenticate("secret123456")
+  end
+
+  test "attempts to change the password are rate limited like signing in" do
+    Users::ProfilesController.cache_store.stubs(:increment).returns(11)
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    assert_redirected_to user_profile_url
+    assert_equal "Too many attempts. Try again in a few minutes.", flash[:alert]
+    assert users(:david).reload.authenticate("secret123456")
+
+    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    assert_equal "Enter your current password to change it.", flash[:alert]
+
+    put user_profile_url, params: { user: { name: "Dave" } }
+    assert_equal "Dave", users(:david).reload.name
+  end
+
+  test "any current password sent counts toward the rate limit, however blank it looks" do
+    Users::ProfilesController.cache_store.stubs(:increment).returns(11)
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: " " } }
+    assert_equal "Too many attempts. Try again in a few minutes.", flash[:alert]
+  end
+
+  test "a current password that isn't text is no current password" do
+    [ false, [], {}, 0 ].each do |challenge|
+      put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: challenge } }, as: :json
+
+      assert_redirected_to user_profile_url
+      assert_equal "Enter your current password to change it.", flash[:alert]
+      assert users(:david).reload.authenticate("secret123456")
+    end
+  end
+
+  test "changing the password removes the other devices' push subscriptions, and this browser can register its own again" do
+    stub_web_push_dns_resolution
+    endpoint = push_subscriptions(:david_chrome).attributes.slice("endpoint", "p256dh_key", "auth_key")
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    assert_empty users(:david).push_subscriptions
+
+    post user_push_subscriptions_url, params: { push_subscription: endpoint }
+    assert_response :ok
+    assert_equal [ endpoint["endpoint"] ], users(:david).push_subscriptions.pluck(:endpoint)
+  end
+
   test "updates are limited to the current user" do
     put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } }
 
     assert_equal "Jason", users(:jason).reload.name
   end
+
+  private
+    def assert_password_change_refused(alert, params)
+      other_device = open_session
+      other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
+      ActionCable.server.expects(:remote_connections).never
+
+      assert_no_changes -> { [ Session.count, Push::Subscription.count ] } do
+        put user_profile_url, params: params
+      end
+
+      assert_redirected_to user_profile_url
+      assert_equal alert, flash[:alert]
+      assert users(:david).reload.authenticate("secret123456")
+      assert_equal "David", users(:david).name
+      other_device.get user_profile_url
+      other_device.assert_response :success
+    end
 end

test/controllers/users/push_subscriptions_controller_test.rb

@@ -74,4 +74,98 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
       assert_redirected_to user_push_subscriptions_url
     end
   end
+
+  test "a signed-out session can't register a subscription" do
+    Session.delete_all
+
+    assert_no_difference -> { Push::Subscription.count } do
+      post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
+    end
+
+    assert_redirected_to new_session_url
+  end
+
+  test "a session signed out while its request is in flight doesn't register a subscription" do
+    Push::Subscription.any_instance.stubs(:valid?).with { Session.delete_all; true }.returns(true)
+
+    assert_no_difference -> { Push::Subscription.count } do
+      post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
+    end
+
+    assert_response :unauthorized
+  end
+
+  test "a session signed out while its request is in flight doesn't keep an existing subscription alive" do
+    subscription = push_subscriptions(:david_chrome)
+    Push::Subscription.any_instance.stubs(:valid?).with { Session.delete_all; true }.returns(true)
+
+    assert_no_changes -> { subscription.reload.updated_at } do
+      post user_push_subscriptions_url, params: { push_subscription: subscription.attributes.slice("endpoint", "p256dh_key", "auth_key") }
+    end
+
+    assert_response :unauthorized
+  end
+
+  test "pages name their session for the browser to post its subscription again in a new one" do
+    get user_profile_url
+    assert_equal user_push_subscriptions_path, css_select("[data-push-subscription-sync-url-value]").first["data-push-subscription-sync-url-value"]
+
+    get room_url(rooms(:watercooler))
+    first_marker = css_select("[data-push-subscription-sync-session-value]").first["data-push-subscription-sync-session-value"]
+
+    assert_match(/\A\h{64}\z/, first_marker)
+    assert_not_includes response.body, users(:david).sessions.order(:id).last.token
+
+    sign_in :david
+    get room_url(rooms(:watercooler))
+    assert_not_equal first_marker, css_select("[data-push-subscription-sync-session-value]").first["data-push-subscription-sync-session-value"]
+  end
+
+  test "two posts of the same new subscription at once register it once" do
+    Push::Subscription.any_instance.stubs(:valid?).with do
+      users(:david).push_subscriptions.build(new_subscription_params).save!(validate: false) unless Push::Subscription.exists?(endpoint: new_subscription_params["endpoint"])
+      true
+    end.returns(true)
+
+    assert_difference -> { Push::Subscription.where(endpoint: new_subscription_params["endpoint"]).count }, +1 do
+      post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
+    end
+
+    assert_response :ok
+  end
+
+  private
+    def new_subscription_params
+      { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
+    end
+end
+
+class Users::PushSubscriptionsControllerForeignWriteTest < ActionDispatch::IntegrationTest
+  self.use_transactional_tests = false
+
+  setup do
+    sign_in :david
+    stub_web_push_dns_resolution
+  end
+
+  test "a subscription another connection registers while this one validates isn't registered twice" do
+    params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/foreign", "p256dh_key" => "123", "auth_key" => "456" }
+    Push::Subscription.any_instance.stubs(:valid?).with { foreign_insert(params); true }.returns(true)
+
+    post user_push_subscriptions_url, params: { push_subscription: params }
+
+    assert_response :ok
+    assert_equal 1, Push::Subscription.where(endpoint: params["endpoint"]).count
+  end
+
+  private
+    def foreign_insert(params)
+      unless @inserted
+        SQLite3::Database.new(ActiveRecord::Base.connection_db_config.database) do |database|
+          database.execute "INSERT INTO push_subscriptions (user_id, endpoint, p256dh_key, auth_key, created_at, updated_at) " \
+            "VALUES (?, ?, ?, ?, datetime('now'), datetime('now'))", [ users(:david).id, *params.values ]
+        end
+        @inserted = true
+      end
+    end
 end

test/helpers/content_filters_test.rb

@@ -172,7 +172,7 @@ class ContentFiltersTest < ActionView::TestCase
 
     filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body).to_html
 
-    assert_equal body, filtered
+    assert_dom_equal body, filtered
     assert_match %r{<table>.*<th><p>Name</p></th>.*<td><p>Jason</p></td>}m, message_presentation(message)
   end
 

test/helpers/messages_helper_test.rb

@@ -25,6 +25,47 @@ class MessagesHelperTest < ActionView::TestCase
     assert_match /<strong>bold<\/strong>/, presentation
   end
 
+  test "message_presentation links URLs and email addresses in text" do
+    message = Message.create! room: rooms(:pets), body: "<div>See https://example.com/a?b=1&amp;c=2, or write to me@example.com.</div>", client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ "https://example.com/a?b=1&c=2", "mailto:me@example.com" ], links.map { it["href"] }
+    assert_equal [ "_blank", "_blank" ], links.map { it["target"] }
+    assert_equal [ "https://example.com/a?b=1&c=2", "me@example.com" ], links.map(&:text)
+  end
+
+  test "message_presentation links an email address with an apostrophe whole" do
+    message = Message.create! room: rooms(:pets), body: "<div>Write to o'brien@example.com</div>", client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ [ "mailto:o%27brien@example.com", "o'brien@example.com" ] ], links.map { [ it["href"], it.text ] }
+  end
+
+  test "message_presentation leaves URLs and email addresses in attribute values as they are" do
+    [ "class", "title" ].product([ "https://example.com", "www.example.com", "me@example.com" ]).each do |attribute, linkable|
+      value = "a> #{linkable} <em>b</em> "
+      message = Message.create! room: rooms(:pets), body: %(<div><span #{attribute}="#{value}">c</span> https://example.org</div>), client_message_id: "0015", creator: users(:jason)
+
+      presentation = Nokogiri::HTML5.fragment(view.message_presentation(message))
+      assert_equal [ value ], presentation.css("span[#{attribute}]").map { it[attribute] }, "#{attribute} with #{linkable}"
+      assert_empty presentation.css("em"), "#{attribute} with #{linkable}"
+      assert_equal [ "https://example.org" ], presentation.css("a").map { it["href"] }, "#{attribute} with #{linkable}"
+    end
+  end
+
+  test "linking text doesn't eat a code block's leading blank line" do
+    linked = view.send(:auto_link_text, "<pre>\n\n\nfirst https://example.com</pre>".html_safe)
+
+    assert_equal "\nfirst https://example.com", Nokogiri::HTML5.fragment(linked).at_css("pre").text
+  end
+
+  test "message_presentation leaves an existing link's text alone" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="https://example.com/">https://example.org/</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ [ "https://example.com/", "https://example.org/" ] ], links.map { [ it["href"], it.text ] }
+  end
+
   test "message_presentation shows an image's thumbnail made when it was posted" do
     presentation = view.message_presentation(attachment_message("moon.jpg", "image/jpeg", processed: true))
 

test/integration/content_security_policy_test.rb

@@ -0,0 +1,187 @@
+require "test_helper"
+
+class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
+  PAGE_PATH = "/"
+
+  setup { sign_in :david }
+
+  test "enforcing policy allows exactly the inline scripts the page carries" do
+    get root_url
+    follow_redirect! while response.redirect?
+
+    policy = response.headers["Content-Security-Policy"]
+    assert policy, "expected an enforcing Content-Security-Policy header"
+    assert_nil response.headers["Content-Security-Policy-Report-Only"]
+    assert_match "object-src 'none'", policy
+    assert_match "base-uri 'self'", policy
+    assert_no_match "unsafe-inline", policy
+    assert_no_match "nonce-", policy
+    assert_equal "www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1]
+
+    script_src = policy[/script-src ([^;]+)/, 1].split
+    assert_not_includes script_src, "'self'"
+    assert_includes script_src, "www.example.com/assets/"
+    inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" }
+    assert_operator inline.size, :>=, 2
+    inline.each do |script|
+      assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(script.text)}'", "inline #{script["type"]} script not allowed by hash"
+    end
+  end
+
+  test "scripts may load from a configured asset host, however it's written" do
+    {
+      "https://cdn.example.test" => %r{\Ahttps://cdn\.example\.test\z},
+      "cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z},
+      "//cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z},
+      "https://assets%d.example.test" => %r{\Ahttps://assets[0-3]\.example\.test\z},
+      ->(source) { "https://one.example.test" } => %r{\Ahttps://one\.example\.test\z},
+      ->(source, request) { "https://#{request.host}.example.test" } => %r{\Ahttps://www\.example\.com\.example\.test\z}
+    }.each do |asset_host, expected_origin|
+      with_asset_host(asset_host) do
+        get PAGE_PATH
+        follow_redirect! while response.redirect?
+        assert_response :success
+
+        script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+        page = Nokogiri::HTML5(response.body)
+        urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values +
+          page.css("link[rel=modulepreload]").map { it["href"] } + page.css("script[src]").map { it["src"] }
+        origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ]
+
+        assert_not_empty origins, asset_host.inspect
+        origins.each { assert_match expected_origin, it, asset_host.inspect }
+        assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }, asset_host.inspect
+        assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(page.at_css("script[type=importmap]").text)}'", asset_host.inspect
+      end
+    end
+  end
+
+  test "an asset host whose path contains the asset directory is limited to the directory it serves" do
+    with_asset_host("https://cdn.example.test/assets") do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+
+      script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+      assert_includes script_src, "https://cdn.example.test/assets/assets/"
+      assert_not allowed_by_path?(script_src, "https://cdn.example.test/assets/rails/active_storage/blobs/proxy/x/upload.js")
+    end
+  end
+
+  test "the sign-in page is limited to the asset directory too" do
+    cookies.delete(:session_token)
+    get new_session_url
+    assert_response :success
+
+    script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+    assert_not_includes script_src, "'self'"
+    assert_includes script_src, "www.example.com/assets/"
+  end
+
+  test "scripts are limited to the asset directory under a relative root" do
+    with_relative_url_root("/chat") do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+      assert_response :success
+
+      script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+      page = Nokogiri::HTML5(response.body)
+      urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + page.css("link[rel=modulepreload]").map { it["href"] }
+
+      assert urls.all? { it.start_with?("/chat/assets/") }, urls.inspect
+      assert_includes script_src, "www.example.com/chat/assets/"
+      assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }
+    end
+  end
+
+  test "this origin's sources keep a non-default port and take the page's scheme" do
+    host! "www.example.com:8443"
+    get PAGE_PATH
+    follow_redirect! while response.redirect?
+
+    policy = response.headers["Content-Security-Policy"]
+    assert_includes policy[/script-src ([^;]+)/, 1].split, "www.example.com:8443/assets/"
+    assert_equal "www.example.com:8443/service-worker.js", policy[/worker-src ([^;]+)/, 1]
+  end
+
+  test "uploads can't load as scripts" do
+    get PAGE_PATH
+    follow_redirect! while response.redirect?
+
+    script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+    blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("alert(1)"), filename: "upload.js", content_type: "text/javascript")
+
+    disk_url = ActiveStorage::Current.set(url_options: { protocol: "http", host: "www.example.com" }) { blob.url }
+
+    [ rails_blob_url(blob), rails_blob_url(blob, disposition: :inline), rails_storage_proxy_url(blob), disk_url ].each do |url|
+      assert_not allowed_by_path?(script_src, url), "#{url} allowed by #{script_src.inspect}"
+    end
+  end
+
+  test "with the policy turned off, pages send none" do
+    without_policy do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+
+      assert_response :success
+      assert_nil response.headers["Content-Security-Policy"]
+    end
+  end
+
+  test "the escape hatch boots without a policy" do
+    output = IO.popen({ "DISABLE_CONTENT_SECURITY_POLICY" => "true" },
+      [ "bin/rails", "runner", "print Rails.application.config.content_security_policy.inspect" ], chdir: Rails.root, err: File::NULL, &:read)
+
+    assert_equal "nil", output
+  end
+
+  test "uploaded files are guarded on their way out" do
+    assert_includes Rails.application.middleware.map(&:klass), UploadedScriptGuard
+  end
+
+  private
+    # What DISABLE_CONTENT_SECURITY_POLICY=true leaves at boot: no application policy
+    def without_policy
+      previous = Rails.application.env_config.delete("action_dispatch.content_security_policy")
+      yield
+    ensure
+      Rails.application.env_config["action_dispatch.content_security_policy"] = previous
+    end
+
+    # Every page script is allowed by a source with a path, never by a bare origin
+    def assert_scripts_allowed_by_path(script_src, urls, message = nil)
+      assert_empty script_src.grep(%r{\A([a-z]+://)?[^/']+/?\z}), message
+      urls.each { assert allowed_by_path?(script_src, it), "#{it} not allowed by #{script_src.inspect} #{message}" }
+    end
+
+    # CSP source matching on a first request: 'self' and a bare origin match any
+    # path, a path ending in / matches as a prefix, any other path exactly. A
+    # source without a scheme takes the page's.
+    def allowed_by_path?(script_src, url)
+      url = URI(url)
+      script_src.any? do |source|
+        unless source.start_with?("'") && source != "'self'"
+          source = URI(source == "'self'" ? request.base_url : (source.include?("://") ? source : "#{request.scheme}://#{source}"))
+        end
+        source.is_a?(URI) && source.origin == url.origin &&
+          (source.path.empty? || (source.path.end_with?("/") ? url.path.start_with?(source.path) : url.path == source.path))
+      end
+    end
+
+    def with_relative_url_root(root)
+      previous, ApplicationController.config.relative_url_root = ApplicationController.config.relative_url_root, root
+      Rails.application.importmap.send(:clear_cache)
+      yield
+    ensure
+      ApplicationController.config.relative_url_root = previous
+      Rails.application.importmap.send(:clear_cache)
+    end
+
+    def with_asset_host(asset_host)
+      previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host
+      Rails.application.importmap.send(:clear_cache)
+      yield
+    ensure
+      ApplicationController.asset_host = previous
+      Rails.application.importmap.send(:clear_cache)
+    end
+end

test/middleware/uploaded_script_guard_test.rb

@@ -0,0 +1,42 @@
+require "test_helper"
+
+class UploadedScriptGuardTest < ActiveSupport::TestCase
+  # Every JavaScript MIME type essence in the MIME Sniffing Standard, plus
+  # parameters and casing a signed Disk URL might carry.
+  SCRIPT_TYPES = %w[
+    application/ecmascript application/javascript application/x-ecmascript application/x-javascript
+    text/ecmascript text/javascript text/javascript1.0 text/javascript1.1 text/javascript1.2 text/javascript1.3
+    text/javascript1.4 text/javascript1.5 text/jscript text/livescript text/x-ecmascript text/x-javascript
+  ] + [ "Text/JavaScript; charset=utf-8" ]
+
+  test "uploads never leave with a script type, whatever the URL says" do
+    SCRIPT_TYPES.each do |type|
+      assert_equal ActiveStorage.binary_content_type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/probe.js"), type
+    end
+  end
+
+  test "media and downloads keep their type" do
+    %w[ image/png video/mp4 audio/mpeg application/pdf text/plain ].each do |type|
+      assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/file"), type
+    end
+  end
+
+  test "multi-range responses keep their boundary" do
+    type = "multipart/byteranges; boundary=AaB03x"
+    assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/blobs/proxy/key/movie.mp4")
+  end
+
+  test "direct upload responses keep their JSON type" do
+    assert_equal "application/json", served_type("application/json", "#{ActiveStorage.routes_prefix}/direct_uploads", method: "POST")
+  end
+
+  test "application responses outside uploads are untouched" do
+    assert_equal "text/javascript", served_type("text/javascript", "/assets/application-abc123.js")
+  end
+
+  private
+    def served_type(type, path, method: "GET")
+      app = ->(env) { [ 200, { "content-type" => type }, [ "" ] ] }
+      UploadedScriptGuard.new(app).call(Rack::MockRequest.env_for(path, method: method))[1]["content-type"]
+    end
+end

test/models/opengraph/fetch_test.rb

@@ -74,6 +74,24 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
     end
   end
 
+  test "#fetch_document connects to the resolved IP even when a proxy is configured" do
+    url = URI.parse("http://www.example.com/")
+    saved = ENV.slice("http_proxy", "HTTP_PROXY", "no_proxy", "NO_PROXY")
+    %w[ no_proxy NO_PROXY ].each { |k| ENV.delete(k) }
+    %w[ http_proxy HTTP_PROXY ].each { |k| ENV[k] = "http://proxy.internal:3128" }
+
+    WebMock.disable_net_connect! allow: [ url.host ]
+    TCPSocket.expects(:open).with { |*args, **| args.first == "proxy.internal" }.never
+    TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 80 }.throws(:not_proxied)
+
+    assert_throws :not_proxied do
+      @fetch.fetch_document(url, ip: "1.2.3.4")
+    end
+  ensure
+    %w[ http_proxy HTTP_PROXY no_proxy NO_PROXY ].each { |k| ENV.delete(k) }
+    saved.each { |k, v| ENV[k] = v }
+  end
+
   test "#fetch_document is empty following redirects that never finish" do
     WebMock.stub_request(:get, "https://www.example.com/")
       .to_return(status: 302, headers: { location: "https://www.example.com/" })

test/models/session_test.rb

@@ -0,0 +1,20 @@
+require "test_helper"
+
+class SessionTest < ActiveSupport::TestCase
+  test "start a session" do
+    assert_difference -> { users(:david).sessions.count }, +1 do
+      users(:david).sessions.create!(user_agent: "This device", ip_address: "127.0.0.1")
+    end
+  end
+
+  test "a session doesn't start on a password checked before it changed" do
+    user = User.find(users(:david).id)
+    User.find(user.id).update!(password: "a new secret123456")
+
+    assert_no_difference -> { Session.count } do
+      assert_raises(ActiveRecord::RecordInvalid) do
+        user.sessions.create!(user_agent: "This device", ip_address: "127.0.0.1")
+      end
+    end
+  end
+end

test/models/user/transferable_test.rb

@@ -0,0 +1,40 @@
+require "test_helper"
+
+class User::TransferableTest < ActiveSupport::TestCase
+  test "a transfer id finds its user" do
+    assert_equal users(:david), User.find_by_transfer_id(users(:david).transfer_id)
+  end
+
+  test "a transfer id expires" do
+    transfer_id = users(:david).transfer_id
+
+    travel User::TRANSFER_LINK_EXPIRY_DURATION + 1.second
+
+    assert_nil User.find_by_transfer_id(transfer_id)
+  end
+
+  test "a transfer id made before a password change stops working" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).update!(password: "a new secret123456")
+
+    assert_nil User.find_by_transfer_id(transfer_id)
+    assert_equal users(:david), User.find_by_transfer_id(users(:david).transfer_id)
+  end
+
+  test "other changes keep a transfer id working" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
+
+    assert_equal users(:david), User.find_by_transfer_id(transfer_id)
+  end
+
+  test "a transfer id finds only users in scope" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).deactivate
+
+    assert_nil User.active.find_by_transfer_id(transfer_id)
+  end
+end

test/models/user_test.rb

@@ -6,6 +6,14 @@ class UserTest < ActiveSupport::TestCase
     assert users(:david).valid?
   end
 
+  test "a password challenge is checked whenever the password would change" do
+    assert_not User.find(users(:david).id).update(password: "a new secret123456", password_challenge: "")
+    assert_not User.find(users(:david).id).update(password: "a new secret123456", password_challenge: "not my password")
+    assert User.find(users(:david).id).update(name: "Dave", password_challenge: "")
+    assert User.find(users(:david).id).update(password: "a new secret123456", password_challenge: "secret123456")
+    assert User.find(users(:david).id).update(password: "another secret123456")
+  end
+
   test "creating users grants membership to the open rooms" do
     assert_difference -> { Membership.count }, +Rooms::Open.count do
       create_new_user
@@ -42,6 +50,70 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  test "changing the password signs out every session, the current one included" do
+    Current.session = users(:david).sessions.create!(user_agent: "This device", ip_address: "127.0.0.1")
+
+    assert_changes -> { users(:david).sessions.count }, to: 0 do
+      users(:david).update!(password: "a new secret123456")
+    end
+  end
+
+  test "changing the password closes the account's live connections" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    users(:david).update!(password: "a new secret123456")
+  end
+
+  test "changing the password closes live connections even when the user is saved again before commit" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    User.transaction do
+      users(:david).update!(password: "a new secret123456")
+      users(:david).update!(name: "Dave")
+    end
+  end
+
+  test "a password change that rolls back keeps every session and connection" do
+    ActionCable.server.expects(:remote_connections).never
+
+    assert_no_changes -> { users(:david).sessions.count } do
+      User.transaction do
+        users(:david).update!(password: "a new secret123456")
+        raise ActiveRecord::Rollback
+      end
+    end
+  end
+
+  test "changing the password removes every push subscription of the user" do
+    assert users(:david).push_subscriptions.any?
+
+    users(:david).update!(password: "a new secret123456")
+
+    assert_empty users(:david).push_subscriptions.reload
+    assert users(:jason).push_subscriptions.any?
+  end
+
+  test "a password change that rolls back keeps push subscriptions" do
+    assert_no_changes -> { users(:david).push_subscriptions.count } do
+      User.transaction do
+        users(:david).update!(password: "a new secret123456")
+        raise ActiveRecord::Rollback
+      end
+    end
+  end
+
+  test "changing other details keeps every session" do
+    ActionCable.server.expects(:remote_connections).never
+
+    assert_no_changes -> { [ users(:david).sessions.count, users(:david).push_subscriptions.count ] } do
+      users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
+    end
+  end
+
   private
     def create_new_user
       User.create!(name: "User", email_address: "user@example.com", password: "secret123456")

test/script/admin/reset_password_test.rb

@@ -0,0 +1,21 @@
+require "test_helper"
+
+class ResetPasswordScriptTest < ActiveSupport::TestCase
+  test "resets a password without the current one and signs out every session" do
+    assert_output "Password has been reset\n" do
+      run_script "david@37signals.com", "a new secret123456"
+    end
+
+    assert users(:david).reload.authenticate("a new secret123456")
+    assert_empty users(:david).sessions
+  end
+
+  private
+    def run_script(*arguments)
+      original_arguments = ARGV.dup
+      ARGV.replace(arguments)
+      load Rails.root.join("script/admin/reset-password").to_s
+    ensure
+      ARGV.replace(original_arguments)
+    end
+end

test/system/composer_test.rb

@@ -174,6 +174,20 @@ class ComposerTest < ApplicationSystemTestCase
     assert_selector last_message_selector("cite"), text: "JZ"
   end
 
+  test "replying quotes a mention's text as text" do
+    message = rooms(:designers).messages.create! creator: users(:kevin), client_message_id: "reply-mention",
+      body: %(<div><span class="mention">&lt;em&gt;Kevin&lt;/em&gt;</span> says hi</div>)
+    join_room rooms(:designers)
+
+    within_message message do
+      reveal_message_actions
+      find("[aria-label='Reply']").click
+    end
+
+    assert_composer_text "<em>Kevin</em> says hi"
+    assert_no_selector "#composer lexxy-editor em"
+  end
+
   test "arrow up edits my last message when the composer is empty" do
     composer_editor.click
     press_in_composer :up

test/system/push_subscription_resync_test.rb

@@ -0,0 +1,74 @@
+require "application_system_test_case"
+
+class PushSubscriptionResyncTest < ApplicationSystemTestCase
+  ENDPOINT = "https://fcm.googleapis.com/fcm/send/system-test"
+
+  setup do
+    stub_web_push_dns_resolution
+    fake_a_browser_push_subscription
+  end
+
+  teardown do
+    page.driver.browser.execute_cdp "Page.removeScriptToEvaluateOnNewDocument", identifier: @fake_subscription_script
+    page.execute_script("localStorage.clear()") if page.current_url.start_with?("http")
+  end
+
+  test "a browser posts its subscription again once per session, so it survives a password change" do
+    sign_in "david@37signals.com"
+    assert_subscription_registered
+    first_sync = subscription.updated_at
+
+    join_room rooms(:hq)
+    assert_equal first_sync, subscription.reload.updated_at, "posted again in the same session"
+
+    first_id = subscription.id
+    change_password
+    assert_subscription_registered
+    assert_not_equal first_id, subscription.id, "not removed and posted again by the profile page"
+    assert_not page.evaluate_script("localStorage.getItem('unsubscribed-for-test')")
+  end
+
+  test "a subscription the server refuses stays in the browser" do
+    stub_dns_resolution("169.254.169.254")
+
+    sign_in "david@37signals.com"
+    Timeout.timeout(5) { sleep 0.1 until page.evaluate_script("localStorage.getItem('push-subscription-synced-session')") }
+    join_room rooms(:hq)
+
+    assert_not Push::Subscription.exists?(endpoint: ENDPOINT)
+    assert_not page.evaluate_script("localStorage.getItem('unsubscribed-for-test')")
+  end
+
+  private
+    # Headless Chrome has no push service, so stand in for the subscription the service worker would hold. The
+    # browser is shared with other system tests, so teardown removes this again.
+    def fake_a_browser_push_subscription
+      @fake_subscription_script = page.driver.browser.execute_cdp("Page.addScriptToEvaluateOnNewDocument", source: <<~JS)["identifier"]
+        (() => {
+          Object.defineProperty(Notification, "permission", { get: () => "granted" })
+          const subscription = {
+            toJSON: () => ({ endpoint: "#{ENDPOINT}", keys: { p256dh: "p256dh-key", auth: "auth-key" } }),
+            unsubscribe: async () => { localStorage.setItem("unsubscribed-for-test", "true") }
+          }
+          navigator.serviceWorker.getRegistration = async () => ({ pushManager: { getSubscription: async () => subscription } })
+        })()
+      JS
+    end
+
+    def change_password
+      visit user_profile_url
+      find("input[name='user[password_challenge]']").fill_in with: "secret123456"
+      find("input[name='user[password]']").fill_in with: "a new secret123456"
+      find("input[name='user[password]']").send_keys :enter
+      assert_selector ".flash", text: "✓"
+    end
+
+    def assert_subscription_registered
+      assert_selector "[data-controller~=push-subscription-sync]", visible: :all
+      Timeout.timeout(5) { sleep 0.1 until Push::Subscription.exists?(endpoint: ENDPOINT, user: users(:david)) }
+    end
+
+    def subscription
+      Push::Subscription.find_by!(endpoint: ENDPOINT)
+    end
+end