Compare commits

..
Author SHA1 Message Date
dependabot[bot] 8f2d5c8bac Bump surfguard from 59e278c to 71cac69
Bumps [surfguard](https://github.com/basecamp/surfguard) from `59e278c` to `71cac69`.
- [Release notes](https://github.com/basecamp/surfguard/releases)
- [Commits](https://github.com/basecamp/surfguard/compare/59e278c01a537755f22791429051891949231ead...71cac69a592d9ef63ce27600eb26bdf94803a3eb)

---
updated-dependencies:
- dependency-name: surfguard
  dependency-version: 71cac69a592d9ef63ce27600eb26bdf94803a3eb
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
2026-10-09 19:54:16 +00:00
47 changed files with 79 additions and 1300 deletions

No files matched your search

+6 -6
View File
@@ -21,7 +21,7 @@ jobs:
persist-credentials: false
- name: Set up Ruby
uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0
with:
ruby-version: .ruby-version
bundler-cache: true
@@ -40,7 +40,7 @@ jobs:
persist-credentials: false
- name: Set up Ruby
uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0
with:
ruby-version: .ruby-version
bundler-cache: true
@@ -63,7 +63,7 @@ jobs:
persist-credentials: false
- name: Set up Ruby
uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
uses: ruby/setup-ruby@a0102e0972be65f351c307e2d64b9314a57c8073 # v1.324.0
with:
ruby-version: .ruby-version
bundler-cache: true
@@ -86,7 +86,7 @@ jobs:
uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12
- name: Run zizmor
uses: zizmorcore/zizmor-action@85b2b9fb8e3325e7a23d7d0a9d7ba5ae0f999e3e # v0.7.1
uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4
with:
advanced-security: false
@@ -110,7 +110,7 @@ jobs:
persist-credentials: false
- name: Set up Ruby
uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0
env:
REDIS_URL: redis://localhost:6379/0
with:
@@ -143,7 +143,7 @@ jobs:
persist-credentials: false
- name: Set up Ruby
uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0
env:
REDIS_URL: redis://localhost:6379/0
with:
+1 -1
View File
@@ -1 +1 @@
4.0.9
4.0.7
+1 -1
View File
@@ -1,7 +1,7 @@
# syntax = docker/dockerfile:1
# Make sure it matches the Ruby version in .ruby-version and Gemfile
ARG RUBY_VERSION=4.0.9
ARG RUBY_VERSION=4.0.7
FROM docker.io/library/ruby:$RUBY_VERSION-slim AS base
# Rails app lives here
+1 -1
View File
@@ -1,7 +1,7 @@
# syntax = docker/dockerfile:1
# Make sure it matches the Ruby version in .ruby-version and Gemfile
ARG RUBY_VERSION=4.0.9
ARG RUBY_VERSION=4.0.7
FROM ruby:$RUBY_VERSION-slim as base
# Install the tools we need
+2 -2
View File
@@ -1,6 +1,6 @@
GIT
remote: https://github.com/basecamp/surfguard.git
revision: 59e278c01a537755f22791429051891949231ead
revision: 71cac69a592d9ef63ce27600eb26bdf94803a3eb
specs:
surfguard (0.2.0)
@@ -146,7 +146,7 @@ GEM
bcrypt (3.1.22)
benchmark (0.5.0)
bigdecimal (4.1.3)
brakeman (8.1.0)
brakeman (8.0.6)
racc
builder (3.3.0)
bundler-audit (0.9.3)
+2 -25
View File
@@ -2,42 +2,19 @@ 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
@verified_session.user
if verified_session = find_session_by_cookie
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
+1 -1
View File
@@ -1,4 +1,4 @@
class ApplicationController < ActionController::Base
include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses, InlineScriptHashes
include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses
include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName
end
+1 -1
View File
@@ -57,7 +57,7 @@ module Authentication
end
def start_new_session_for(user)
user.sessions.create!(user_agent: request.user_agent, ip_address: request.remote_ip).tap do |session|
user.sessions.start!(user_agent: request.user_agent, ip_address: request.remote_ip).tap do |session|
authenticated_as session
end
end
@@ -1,69 +0,0 @@
# 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_sources(importmap)
policy.worker_src "#{request.host_with_port}#{service_worker_path(format: :js)}"
end
end
end
mattr_accessor :last_script_sources
private
# Parsing the importmap and resolving each of its URLs costs about a
# millisecond a page, yet the answer changes only with the importmap and
# the host the page is served from. Keep the last one: a single entry, so
# a request with a made-up Host header can't grow it.
def script_sources(importmap)
key = [ importmap, request.base_url, request.host_with_port, config.relative_url_root ]
cached_key, sources = InlineScriptHashes.last_script_sources
if cached_key == key
sources
else
(script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)).freeze.tap do |sources|
InlineScriptHashes.last_script_sources = [ key, sources ].freeze
end
end
end
# 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
@@ -11,7 +11,5 @@ class Sessions::TransfersController < ApplicationController
else
head :bad_request
end
rescue ActiveRecord::RecordInvalid
head :bad_request
end
end
-2
View File
@@ -14,8 +14,6 @@ class SessionsController < ApplicationController
else
render_rejection :unauthorized
end
rescue ActiveRecord::RecordInvalid
render_rejection :unauthorized
end
def destroy
+2 -38
View File
@@ -1,26 +1,14 @@
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
if @user.update user_params
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
@user.update user_params
redirect_to user_profile_url, notice: update_notice
end
private
@@ -28,35 +16,11 @@ 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
@@ -5,14 +5,17 @@ class Users::PushSubscriptionsController < ApplicationController
end
def create
subscription = @push_subscriptions.find_or_initialize_by(push_subscription_params)
subscription.user_agent = request.user_agent if subscription.new_record?
# Existing endpoints must pass current validations too
if subscription.valid?
head register_while_signed_in(subscription) ? :ok : :unauthorized
if subscription = @push_subscriptions.find_by(push_subscription_params)
# Existing endpoints must pass current validations
if subscription.valid?
subscription.touch
head :ok
else
head :unprocessable_entity
end
else
head :unprocessable_entity
subscription = @push_subscriptions.create push_subscription_params.merge(user_agent: request.user_agent)
head subscription.persisted? ? :ok : :unprocessable_entity
end
end
@@ -26,24 +29,6 @@ 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
+1 -1
View File
@@ -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)
# * the final html is re-sanitized before its text is linked (MessagesHelper#auto_link_text)
# * auto_link re-sanitizes the final html (MessagesHelper#message_presentation)
EDITOR_FORMATTING_TAGS = %w[ s u mark table thead tbody tfoot tr th td ]
EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ]
+7 -25
View File
@@ -1,12 +1,9 @@
module MessagesHelper
# Rendered text is re-sanitized before linking. Rails' default safe list
# lacks the formatting the rich text editor produces
# auto_link re-sanitizes with Rails' default safe list, which 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",
@@ -104,36 +101,21 @@ module MessagesHelper
private
def text_message_presentation(body)
render = -> { auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(body)) }
render = -> do
auto_link h(ContentFilters::TextMessagePresentationFilters.apply(body)),
html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES }
end
# Embedded attachments render database-backed metadata and signed URLs.
# Plain text HTML depends only on its content, even after a foreign edit.
html = body.to_html
if controller.perform_caching && !html.include?("<action-text-attachment") && html.bytesize <= ResponseCache::MAX_ENTRY_BYTES
FragmentCache.store.fetch([ "text-presentation-v2", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call }
FragmentCache.store.fetch([ "text-presentation-v1", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call }
else
render.call
end
end
# 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
-1
View File
@@ -2,7 +2,6 @@ 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", "🇯🇵": "アカウントに名前を付ける" },
@@ -1,7 +0,0 @@
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
@@ -106,8 +106,7 @@ export default class extends Controller {
async #syncPushSubscription(subscription) {
const response = await post(this.subscriptionsUrlValue, { body: this.#extractJsonPayloadAsString(subscription), responseKind: "turbo-stream" })
// Only a refused endpoint is for good. A session signed out mid-request posts it again after signing in.
if (response.unprocessableEntity) subscription.unsubscribe()
if (!response.ok) subscription.unsubscribe()
}
async #requestPermissionAndSubscribe(registration) {
@@ -1,32 +0,0 @@
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 } })
}
}
@@ -28,7 +28,7 @@ export default class extends Controller {
}
#stripMentionAttachments(node) {
node.querySelectorAll(".mention").forEach(mention => mention.replaceWith(mention.textContent.trim()))
node.querySelectorAll(".mention").forEach(mention => mention.outerHTML = mention.textContent.trim())
return node
}
+3 -12
View File
@@ -7,22 +7,13 @@ class Session < ApplicationRecord
before_create { self.last_active_at ||= Time.now }
validate :user_password_unchanged, on: :create
def self.start!(user_agent:, ip_address:)
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
-27
View File
@@ -18,10 +18,8 @@ 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}%") }
@@ -52,31 +50,6 @@ 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,
+2 -9
View File
@@ -3,20 +3,13 @@ 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_by_token_for(:transfer, id)
find_signed(id, purpose: :transfer)
end
end
def transfer_id
generate_token_for(:transfer)
signed_id(purpose: :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION)
end
end
-5
View File
@@ -62,11 +62,6 @@
<%= 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>
+1 -1
View File
@@ -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_fragment_cache_key(message), "presentation-v8" ] do %>
<% cache [ message_fragment_cache_key(message), "presentation-v7" ] do %>
<%= message_tag message do %>
<h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
-10
View File
@@ -69,16 +69,6 @@
</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) %>
+23 -19
View File
@@ -1,21 +1,25 @@
# Enforcing baseline policy. Application pages narrow script-src from this
# origin to its asset directory, since uploads are served from this origin
# too, and allow the two inline scripts the importmap helper emits by hash
# (InlineScriptHashes). There is no per-request nonce, so rendered HTML stays
# cacheable. 'self' remains for responses outside ApplicationController, none
# of which render user content.
# Be sure to restart your server when you modify this file.
# Define an application-wide content security policy.
# See the Securing Rails Applications Guide for more information:
# https://guides.rubyonrails.org/security.html#content-security-policy-header
# 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
#
# Nothing else is restricted: images, media, fonts, styles (including an
# account's custom styles) and connections behave as before.
# # Generate session nonces for permitted importmap and inline scripts
# config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
# config.content_security_policy_nonce_directives = %w(script-src)
#
# DISABLE_CONTENT_SECURITY_POLICY=true is the escape hatch for an install whose
# proxy or customization injects scripts the policy would block.
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
# # Report violations without enforcing the policy.
# # config.content_security_policy_report_only = true
# end
@@ -1,34 +0,0 @@
# 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
+1 -1
View File
@@ -29,7 +29,7 @@ module WebPush::PersistentRequest
# itself and re-resolve the endpoint host, so http.ipaddr would no longer
# pin the destination and the DNS-rebinding guarantee would be lost.
# Push delivery to public vendor endpoints goes direct.
http = (@options[:connection] ? WebPush::Connections::HTTP : Net::HTTP).new(uri.host, uri.port, nil)
http = Net::HTTP.new(uri.host, uri.port, nil)
http.ipaddr = endpoint_ip
http.use_ssl = true
http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil?
+3 -8
View File
@@ -26,12 +26,6 @@ class WebPush::Connections
end
end
# The Net::HTTP a pooled delivery is opened with: Stages is part of the class, so a connection carries its stage
# from the start instead of having the module mixed into each one as it's opened.
class HTTP < Net::HTTP
include Stages
end
def initialize(keep_alive_timeout: 30, max_idle: 150)
@keep_alive_timeout = keep_alive_timeout
@max_idle = max_idle
@@ -41,8 +35,8 @@ class WebPush::Connections
end
def request(http, request)
unless http.is_a?(HTTP) && http.ipaddr && !http.proxy? && http.use_ssl? && !http.started?
raise ArgumentError, "Only new, direct TLS connections pinned to an address and opened as WebPush::Connections::HTTP are pooled"
unless http.ipaddr && !http.proxy? && http.use_ssl? && !http.started?
raise ArgumentError, "Only new, direct TLS connections pinned to an address are pooled"
end
address = [ http.address, http.port, http.ipaddr ]
@@ -54,6 +48,7 @@ class WebPush::Connections
end
end
http.extend Stages
http.keep_alive_timeout = @keep_alive_timeout
http.start
send_over(http, request, reused: false).tap { checkin(address, http) }
@@ -18,53 +18,4 @@ 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
-35
View File
@@ -26,18 +26,6 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest
ActionController::Base.perform_caching = @previous_caching
end
test "a reused page carries the same enforcing policy, with no nonce" do
get room_messages_url(@room)
first = response.headers["Content-Security-Policy"]
assert_match %r{script-src [^/ ]+/assets/ 'sha256-}, first
assert_no_match "nonce-", first
MessagesController.any_instance.expects(:find_paged_messages).never
get room_messages_url(@room)
assert_response :success
assert_equal first, response.headers["Content-Security-Policy"]
end
test "all four read actions reuse completed pages" do
urls = [ room_url(@room), room_messages_url(@room), user_sidebar_url(:me), searches_url(q: "hello") ]
urls.each do |url|
@@ -136,29 +124,6 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest
assert_redirected_to new_session_url
end
test "a session signed out by a password change isn't served from a cached snapshot or page" do
other_device = open_session { |session| session.host! "once.campfire.test" }
other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
other_device.get user_sidebar_url(:me)
other_device.assert_response :success
Session.expects(:find_by).never
ResponseCache.instance.expects(:write).never
other_device.get user_sidebar_url(:me)
other_device.assert_response :success
Session.unstub(:find_by)
ResponseCache.instance.unstub(:write)
put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } },
headers: { "Sec-Fetch-Site" => "same-origin", "Origin" => "http://once.campfire.test" }
assert_redirected_to user_profile_url
other_device.get user_sidebar_url(:me)
other_device.assert_redirected_to new_session_url
get user_sidebar_url(:me)
assert_response :success
end
test "conditional requests keep native validators" do
get room_messages_url(@room)
etag = response.headers["ETag"]
@@ -19,30 +19,4 @@ 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
@@ -36,19 +36,6 @@ 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" }
@@ -27,190 +27,9 @@ 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
@@ -74,98 +74,4 @@ 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
-41
View File
@@ -37,47 +37,6 @@ 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))
@@ -1,197 +0,0 @@
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 "each host's pages carry that host's sources, one after another" do
[ "www.example.com", "www.example.com:8443", "www.example.com" ].each do |host|
host! host
get PAGE_PATH
follow_redirect! while response.redirect?
assert_includes response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split, "#{host}/assets/"
end
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
+7 -10
View File
@@ -76,13 +76,11 @@ class WebPush::ConnectionsTest < ActiveSupport::TestCase
end
end
class FailingCheckHTTP < WebPush::Connections::HTTP
private def begin_transport(*) = raise(OpenSSL::SSL::SSLError, "alert right after the handshake")
end
test "a TLS failure on a new connection before the push is written is reported as it is" do
failing_check = Module.new { private def begin_transport(*) = raise(OpenSSL::SSL::SSLError, "alert right after the handshake") }
with_push_service do |server|
assert_raises(OpenSSL::SSL::SSLError) { @connections.request(pinned_connection(server, http_class: FailingCheckHTTP), push_request) }
assert_raises(OpenSSL::SSL::SSLError) { @connections.request(pinned_connection(server).extend(failing_check), push_request) }
assert_empty server.requests
end
end
@@ -99,15 +97,14 @@ class WebPush::ConnectionsTest < ActiveSupport::TestCase
end
end
test "only new, direct TLS connections pinned to an address and opened as WebPush::Connections::HTTP are pooled" do
test "only new, direct TLS connections pinned to an address are pooled" do
with_push_service do |server|
unpinned = WebPush::Connections::HTTP.new(HOST, server.port, nil).tap { it.use_ssl = true }
proxied = WebPush::Connections::HTTP.new(HOST, server.port, "127.0.0.1", 3128).tap { it.ipaddr = IP; it.use_ssl = true }
unpinned = Net::HTTP.new(HOST, server.port, nil).tap { it.use_ssl = true }
proxied = Net::HTTP.new(HOST, server.port, "127.0.0.1", 3128).tap { it.ipaddr = IP; it.use_ssl = true }
plain = pinned_connection(server).tap { it.use_ssl = false }
started = pinned_connection(server).tap(&:start)
untracked = Net::HTTP.new(HOST, server.port, nil).tap { it.ipaddr = IP; it.use_ssl = true }
[ unpinned, proxied, plain, started, untracked ].each do |http|
[ unpinned, proxied, plain, started ].each do |http|
assert_raises(ArgumentError) { @connections.request(http, push_request) }
end
assert_empty server.requests
@@ -1,42 +0,0 @@
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
-20
View File
@@ -1,20 +0,0 @@
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
-40
View File
@@ -1,40 +0,0 @@
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
-72
View File
@@ -6,14 +6,6 @@ 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
@@ -50,70 +42,6 @@ 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")
-21
View File
@@ -1,21 +0,0 @@
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
-14
View File
@@ -174,20 +174,6 @@ 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
@@ -1,74 +0,0 @@
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
@@ -108,8 +108,8 @@ module PushServiceTestHelper
end
# Built as WebPush::PersistentRequest builds it for a delivery.
def pinned_connection(server, ip = IP, http_class: WebPush::Connections::HTTP)
http_class.new(HOST, server.port, nil).tap do |http|
def pinned_connection(server, ip = IP)
Net::HTTP.new(HOST, server.port, nil).tap do |http|
http.ipaddr = ip
http.use_ssl = true
end