Compare commits

...
Author SHA1 Message Date
dependabot[bot] 6a5ba042df Bump zizmorcore/zizmor-action
Bumps the github-actions group with 1 update in the / directory: [zizmorcore/zizmor-action](https://github.com/zizmorcore/zizmor-action).


Updates `zizmorcore/zizmor-action` from 0.6.4 to 0.7.1
- [Release notes](https://github.com/zizmorcore/zizmor-action/releases)
- [Commits](https://github.com/zizmorcore/zizmor-action/compare/cc914d7f3750a2d13d75c7f184a1060aa0e9d482...85b2b9fb8e3325e7a23d7d0a9d7ba5ae0f999e3e)

---
updated-dependencies:
- dependency-name: zizmorcore/zizmor-action
  dependency-version: 0.7.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <support@github.com>
2026-10-23 22:37:35 +00:00
Mike Dalessio eccb2d6c91 Upgrade runtime to Ruby 4.0.9 (#381)
* Upgrade runtime to Ruby 4.0.9

Ruby 4.0.9 fixes the Regexp compilation bug behind GitLab's
CVE-2026-110264. Move `.ruby-version`, `Dockerfile` and
`Dockerfile-export` to it.

ref: https://www.ruby-lang.org/en/news/2026/10/23/ruby-4-0-9-released/

* Bump setup-ruby to v1.334.0 for Ruby 4.0.9

The pinned v1.327.0 and v1.324.0 did not know Ruby 4.0.9, so every CI
job failed at Ruby setup. v1.334.0 is the first release that does.
2026-10-23 18:34:43 -04:00
Sam RubyandClaude Opus 5.5 c8f74a0b0e Compute a page's script sources once per importmap and host
Every page parsed the importmap and joined each of its URLs against the
request's base URL, about a millisecond of CPU a page, though the answer
changes only with the importmap and the host. Keep the last answer, keyed
by both. One entry, so a request with a made-up Host header can't grow it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 21:06:01 +09:00
Sam RubyandClaude Opus 5.5 8d4a5264cf Raise the TLS alert from a subclass instead of extending one connection
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 21:04:16 +09:00
Jeremy Daer ce7b5d4ea0 Check a cable connection's session while it's open (#376)
* Check a cable connection's session while it's open

Signing out or changing the password ends sessions and then disconnects
their connections. A connection that found its session just before isn't
subscribed to its disconnects yet, so it missed that and stayed open.

Each connection now looks its session up every minute while it's open, and
closes once the session is gone, asking the browser to reconnect so one
holding a current session comes back. That also covers any other way a
connection misses the disconnect, such as a subscription that takes long to
register, and a lookup that fails is simply tried again.

* Check the session on the server's heartbeat rather than a timer of its own

A connection's own timer could outlive it: closing can run before opening finishes, or a channel's teardown can fail before the connection's. The heartbeat only reaches connections that are open, so nothing is left to stop.

* Skip the session check on a rejected connection

A rejected connection stays registered until its socket closes, and has no session to check, so its heartbeat mustn't fail and stop the heartbeat reaching the connections after it.
2026-10-20 20:42:54 -07:00
Jeremy Daer a10211fb8a Merge pull request #375 from basecamp/refuse-session-for-changed-password
Refuse a session for a password that changed after it was checked
2026-10-20 10:17:29 -07:00
Jeremy Daer a476cbfa24 Sign the browser out when its new session is refused after a password change
If the password changes again between this browser's change and its new session, that session is refused, and the browser is sent to sign in instead of an error.
2026-10-20 08:57:40 -07:00
Jeremy Daer c3faccb0f4 Read the password digest past the query cache
A digest cached earlier in the request would let the check pass on a password that has since changed.
2026-10-20 08:47:47 -07:00
Jeremy Daer 19319a1cff Refuse a session for a password that changed after it was checked
A password change ends every session, but a sign-in or sign-in link checked
against the old password just before the change could still save its session
just after, and that session outlived the change.

The session now checks, as it's saved, that the user's password is still the
one the request loaded. The save's transaction holds SQLite's write lock from
the start, so a change commits either before the check, which fails it, or
after the insert, and ends this session with the others. Sessions start
through the user's association so the check sees the user that was
authenticated, and a refused sign-in is answered like a wrong password.
2026-10-20 08:37:07 -07:00
Jeremy Daer ac073010c2 Merge pull request #372 from basecamp/password-check-on-digest-change
Check the current password whenever the saved password changes
2026-10-19 16:04:53 -07:00
Jeremy Daer 4a580302a5 Check the current password whenever the saved password changes
The profile asked for the current password when the request's password
field was filled in. The check now follows the save itself: the model
checks the challenge whenever the password digest is about to change,
and the profile always hands it the challenge. Editing a name, avatar
or email without a new password still needs none, and scripts that
reset a password, which set no challenge, are unaffected.

Only a string counts as a challenge, and the rate limit counts every
non-empty one, so what's limited is exactly what gets checked.
2026-10-19 15:05:19 -07:00
Jeremy Daer af4dd9fd59 Merge pull request #371 from basecamp/sign-out-elsewhere-on-password-change
Sign out the account's other sessions when its password changes
2026-10-19 14:08:53 -07:00
Jeremy Daer aa93269ceb Look the endpoint up past the query cache, and keep a subscription a race refused
The lookup under the write lock repeated the one made before
validation, so the query cache could answer it with that earlier miss
even after another connection had registered the endpoint. It runs
uncached now.

Enabling notifications unsubscribed the browser on any refusal, so a
session signed out between authenticating and saving lost the browser's
subscription too, and the next session had nothing to post. Only a
refused endpoint unsubscribes it now.
2026-10-19 13:34:34 -07:00
Jeremy Daer 3786ee5244 Post the browser's push subscription from every page, and register it once
Only room pages posted the subscription again, so a browser that
changed its password and closed the profile page went without
notifications until it next opened a room. A small controller in the
layout does it now, still once per session, and the bell is back to how
it was.

Two tabs posting at once after a password change could both miss the
row and insert two, so the endpoint is looked up again under the same
write lock as the session check.
2026-10-19 13:15:10 -07:00
Jeremy Daer 1af62a5bff Remove the fake push subscription after its system test
The browser is shared across system tests, and a script added to every
new document stays until it's removed, so later tests would have seen
the granted permission and the fake subscription too.
2026-10-19 12:55:14 -07:00
Jeremy Daer 6cab924097 Remove push subscriptions on a password change, and post them again
A push subscription belongs to the user, not a session, so a device
signed out by a password change kept getting notifications. Changing
the password now removes every subscription along with the sessions,
in the same transaction.

A browser still signed in keeps its own subscription in its service
worker, so the notifications controller posts it once per session: the
bell names the session by a digest of its id, never its token, and the
browser remembers the last one it posted for. That post never
unsubscribes the browser when the server refuses it.

A request that authenticated just before the change could put a
subscription back after it. The create action now checks the session
still exists and saves in one immediate transaction, after validating
the endpoint outside it.
2026-10-19 12:39:03 -07:00
Jeremy Daer 44a92ff6c6 Ask for the current password to change it
Changing the password now ends every other session, so whoever holds
one could shut the owner out of their own account with a new password.
The profile asks for the current password whenever the password
changes; without it, or with a wrong one, nothing is saved and the page
says why. Other profile edits don't need it, and the admin
reset-password script still works without it.

Attempts are rate limited like signing in, 10 in 3 minutes per address,
so the current password can't be guessed here instead.
2026-10-19 12:17:14 -07:00
Jeremy Daer 160ba295b5 End sign-in links made before a password change
A transfer link signed only the user id, so one copied before the
password changed still started a session for up to four hours after.
Bind it to the password salt with generates_token_for, so a new
password ends those links too. Links made before this upgrade stop
working; they last four hours at most.
2026-10-19 11:36:16 -07:00
Jeremy Daer e158388f3d Close live connections on the transaction that changed the password
An after_commit condition reads the record's last save, so another save
of the user in the same transaction skipped the disconnect even though
the sessions were gone. The callback that deletes them now registers
the disconnect on the transaction itself, which also leaves a rolled
back change alone. A password changed along with the avatar is covered
too: the attachment's touch doesn't hide the change.
2026-10-19 10:57:14 -07:00
Jeremy Daer b01bb7a0a7 Give the changing browser a new session rather than keeping its own
Keeping the session the password was changed from also kept any copy of
its cookie working, and a copied cookie is one of the ways someone else
gets into an account. Every session ends now, and the profile page
signs the browser that changed the password back in with a new one.
2026-10-19 10:45:34 -07:00
Jeremy Daer c6e0105491 Close live connections from the model on any password change
The sessions end wherever the password changes, but only the profile
page closed the live connections, so a change from a console left a
signed-out device receiving messages until it dropped. Close them after
commit from User, logging rather than failing if the realtime service
can't be reached, since the password is saved by then.
2026-10-19 10:33:46 -07:00
Jeremy Daer c78c3fd7ef Sign out the account's other sessions when its password changes
Changing the password is how someone shuts out whoever else is signed
in to their account, but every session opened with the old password
stayed valid. Now the others end and their live connections close, as
on sign-out. The session the password was changed from stays signed in.

Changing the password outside a request, as from a console, ends every
session.
2026-10-19 10:28:06 -07:00
Jeremy Daer 11d3fb9ce0 Merge pull request #365 from basecamp/enforce-script-policy
Enforce a script Content-Security-Policy
2026-10-15 19:20:35 -07:00
Jeremy Daer 2ba940c683 Allow an asset host only up to its own asset directory
An asset host's path can contain the asset directory too, as in
https://cdn.example.com/assets, and cutting each URL at the first one would
allow everything the host serves under it. Cut at the last one instead.
2026-10-15 19:13:26 -07:00
Jeremy Daer 0a494a10cb Leave the scheme off this origin's script sources
Behind a proxy that terminates TLS without X-Forwarded-Proto, Rails sees
http while the page is https, and an http source on a non-default port
doesn't match. A source without a scheme takes the page's own.
2026-10-15 19:01:12 -07:00
Jeremy Daer 2be2227f40 Allow scripts only from the asset directory, not the whole origin
Uploads are served from this origin too, so script-src 'self' would run one
that a cache still holds under a script type from before the upload guard.
Application pages now allow each importmap URL up to its asset directory
(relative root and asset host included), the inline importmap by hash, and
the service worker by worker-src.
2026-10-15 19:01:12 -07:00
Jeremy Daer 2a449401ec Leave a request without a policy when the policy is turned off
With DISABLE_CONTENT_SECURITY_POLICY=true there's no application policy,
and the hash callback started an empty one, so pages sent an empty
Content-Security-Policy header. Skip the callback then.
2026-10-15 19:01:12 -07:00
Jeremy Daer 9c37f41ff6 Keep a multi-range upload response's multipart type
A request for several byte ranges gets a 206 multipart/byteranges whose
boundary the client needs to split the body. Rewriting it to
octet-stream lost the boundary. No browser runs that type as a script,
so it passes through.
2026-10-15 19:01:12 -07:00
Jeremy Daer d713f2631d Say how long a pre-upgrade cached upload can last
Disk URLs are cached for an hour, but blob proxy URLs are cached
publicly and as immutable for a hundred years, so a copy taken that way
before the upgrade doesn't expire.
2026-10-15 19:01:12 -07:00
Jeremy Daer 562888251e Enforce a script policy that allows the importmap by hash
script-src 'self', object-src 'none' and base-uri 'self', enforced. The
two inline scripts the importmap helper emits are allowed by their
SHA-256, and the origins its URLs point to (a configured asset host, in
any form Rails accepts) are allowed by name. There is no per-request
nonce, so cached pages stay valid.

DISABLE_CONTENT_SECURITY_POLICY=true leaves the policy off, for an
install whose proxy or customization injects scripts.
2026-10-15 19:01:12 -07:00
Jeremy Daer cad2d7211d Serve uploads that aren't media as downloads
Uploaded files come from this origin, so one served with a JavaScript
type could run as one of the app's own scripts. Browsers accept sixteen
spellings of that type, and Disk URLs sign the type they were minted
with, so refusing a list of types at upload can't cover links already
out there. Instead, a middleware rewrites the type of anything Active
Storage reads out that isn't plainly media, a PDF or plain text to
application/octet-stream, whatever the URL says.
2026-10-15 19:01:12 -07:00
Jeremy Daer 821ff92dda Merge pull request #364 from basecamp/autolink-text-only
Link URLs only in a message's text
2026-10-15 18:59:57 -07:00
Jeremy Daer 9b271d0e19 Hand auto_link text escaped the way the serializer writes it
That's exactly what it was given before, with quotes and apostrophes
left as they are, rather than relying on its own sanitize pass to undo
h's &#39;. Covers an email address with an apostrophe.
2026-10-15 18:42:06 -07:00
Jeremy Daer 4e29c6e32b Keep a code block's leading newline when linking a message's text
A <pre> whose text starts with a newline needs one more written out to
survive being parsed again, as the html5 serializer does with
preserve_newline. Without it, linking text dropped one more of a code
block's leading blank lines than rendering did before.
2026-10-15 18:42:06 -07:00
Jeremy Daer 6052b123d5 Quote a mention as text when replying
Replying swaps each mention in the quoted copy for its text. Assigning
that text to outerHTML parsed it as html; replace the mention with a
text node instead.
2026-10-15 18:42:06 -07:00
Jeremy Daer 7ea05e18f8 Link URLs only in a message's text
auto_link finds URLs and email addresses with regular expressions over
the rendered html, which can't tell an attribute value from text, so it
could put a link inside an attribute. Hand it each text node on its own,
escaped, after the final sanitize, so links only ever land between tags.

Bump the presentation cache versions so nothing rendered the old way is
served again.
2026-10-15 18:42:06 -07:00
Jeremy Daer 1ac6dfde84 Merge pull request #366 from basecamp/bump-brakeman
Update Brakeman to 8.1.0
2026-10-15 18:41:55 -07:00
Jeremy Daer b2140cf2aa Update Brakeman to 8.1.0 2026-10-15 18:33:17 -07:00
Sam RubyandClaude Opus 5.5 edbc779f4d Test the pooled-connection class check on its own, and name the class
The rejection cases built plain Net::HTTP connections that were also
unpinned or proxied, so they failed the guard either way. They're built
as WebPush::Connections::HTTP now, so each fails only for its stated
reason, and a pinned, direct, TLS plain Net::HTTP is refused for being
one. The error says which class is required.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 23:41:45 +02:00
Sam RubyandClaude Opus 5.5 2daa01e26b Open pooled push connections as a Net::HTTP subclass that includes Stages
WebPush::Connections mixed Stages into each new connection with
`http.extend Stages` just before starting it. The connection is built in
one place, WebPush::PersistentRequest, so it can be built from a class
that already includes Stages: the same method lookup, without giving
every pooled connection its own singleton class. Connections#request
now also refuses a connection that isn't one, so a caller that builds a
plain Net::HTTP gets the ArgumentError rather than a connection whose
failures can't be told apart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 23:41:45 +02:00
47 changed files with 1299 additions and 78 deletions

No files matched your search

+6 -6
View File
@@ -21,7 +21,7 @@ jobs:
persist-credentials: false persist-credentials: false
- name: Set up Ruby - name: Set up Ruby
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
with: with:
ruby-version: .ruby-version ruby-version: .ruby-version
bundler-cache: true bundler-cache: true
@@ -40,7 +40,7 @@ jobs:
persist-credentials: false persist-credentials: false
- name: Set up Ruby - name: Set up Ruby
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
with: with:
ruby-version: .ruby-version ruby-version: .ruby-version
bundler-cache: true bundler-cache: true
@@ -63,7 +63,7 @@ jobs:
persist-credentials: false persist-credentials: false
- name: Set up Ruby - name: Set up Ruby
uses: ruby/setup-ruby@a0102e0972be65f351c307e2d64b9314a57c8073 # v1.324.0 uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
with: with:
ruby-version: .ruby-version ruby-version: .ruby-version
bundler-cache: true bundler-cache: true
@@ -86,7 +86,7 @@ jobs:
uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12 uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12
- name: Run zizmor - name: Run zizmor
uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 uses: zizmorcore/zizmor-action@85b2b9fb8e3325e7a23d7d0a9d7ba5ae0f999e3e # v0.7.1
with: with:
advanced-security: false advanced-security: false
@@ -110,7 +110,7 @@ jobs:
persist-credentials: false persist-credentials: false
- name: Set up Ruby - name: Set up Ruby
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
env: env:
REDIS_URL: redis://localhost:6379/0 REDIS_URL: redis://localhost:6379/0
with: with:
@@ -143,7 +143,7 @@ jobs:
persist-credentials: false persist-credentials: false
- name: Set up Ruby - name: Set up Ruby
uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1.334.0
env: env:
REDIS_URL: redis://localhost:6379/0 REDIS_URL: redis://localhost:6379/0
with: with:
+1 -1
View File
@@ -1 +1 @@
4.0.7 4.0.9
+1 -1
View File
@@ -1,7 +1,7 @@
# syntax = docker/dockerfile:1 # syntax = docker/dockerfile:1
# Make sure it matches the Ruby version in .ruby-version and Gemfile # Make sure it matches the Ruby version in .ruby-version and Gemfile
ARG RUBY_VERSION=4.0.7 ARG RUBY_VERSION=4.0.9
FROM docker.io/library/ruby:$RUBY_VERSION-slim AS base FROM docker.io/library/ruby:$RUBY_VERSION-slim AS base
# Rails app lives here # Rails app lives here
+1 -1
View File
@@ -1,7 +1,7 @@
# syntax = docker/dockerfile:1 # syntax = docker/dockerfile:1
# Make sure it matches the Ruby version in .ruby-version and Gemfile # Make sure it matches the Ruby version in .ruby-version and Gemfile
ARG RUBY_VERSION=4.0.7 ARG RUBY_VERSION=4.0.9
FROM ruby:$RUBY_VERSION-slim as base FROM ruby:$RUBY_VERSION-slim as base
# Install the tools we need # Install the tools we need
+1 -1
View File
@@ -146,7 +146,7 @@ GEM
bcrypt (3.1.22) bcrypt (3.1.22)
benchmark (0.5.0) benchmark (0.5.0)
bigdecimal (4.1.3) bigdecimal (4.1.3)
brakeman (8.0.6) brakeman (8.1.0)
racc racc
builder (3.3.0) builder (3.3.0)
bundler-audit (0.9.3) bundler-audit (0.9.3)
+25 -2
View File
@@ -2,19 +2,42 @@ module ApplicationCable
class Connection < ActionCable::Connection::Base class Connection < ActionCable::Connection::Base
include Authentication::SessionLookup include Authentication::SessionLookup
SESSION_CHECK_INTERVAL = 1.minute
identified_by :current_user identified_by :current_user
def connect def connect
self.current_user = find_verified_user 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 end
private private
def find_verified_user 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 else
reject_unauthorized_connection reject_unauthorized_connection
end end
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
end end
+1 -1
View File
@@ -1,4 +1,4 @@
class ApplicationController < ActionController::Base class ApplicationController < ActionController::Base
include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, CachedResponses, InlineScriptHashes
include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName
end end
+1 -1
View File
@@ -57,7 +57,7 @@ module Authentication
end end
def start_new_session_for(user) 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 authenticated_as session
end end
end end
@@ -0,0 +1,69 @@
# 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,5 +11,7 @@ class Sessions::TransfersController < ApplicationController
else else
head :bad_request head :bad_request
end end
rescue ActiveRecord::RecordInvalid
head :bad_request
end end
end end
+2
View File
@@ -14,6 +14,8 @@ class SessionsController < ApplicationController
else else
render_rejection :unauthorized render_rejection :unauthorized
end end
rescue ActiveRecord::RecordInvalid
render_rejection :unauthorized
end end
def destroy def destroy
+38 -2
View File
@@ -1,14 +1,26 @@
class Users::ProfilesController < ApplicationController class Users::ProfilesController < ApplicationController
before_action :set_user 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 def show
@direct_memberships, @shared_memberships = @direct_memberships, @shared_memberships =
Current.user.memberships.with_ordered_room.partition { |m| m.room.direct? } Current.user.memberships.with_ordered_room.partition { |m| m.room.direct? }
end end
def update 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 end
private private
@@ -16,11 +28,35 @@ class Users::ProfilesController < ApplicationController
@user = Current.user @user = Current.user
end 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 def user_params
params.require(:user).permit(:name, :avatar, :email_address, :password, :bio).compact 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 end
def update_notice def update_notice
params[:user][:avatar] ? "It may take up to 30 minutes to change everywhere." : "✓" params[:user][:avatar] ? "It may take up to 30 minutes to change everywhere." : "✓"
end 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 end
@@ -5,17 +5,14 @@ class Users::PushSubscriptionsController < ApplicationController
end end
def create 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 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
end end
@@ -29,6 +26,24 @@ class Users::PushSubscriptionsController < ApplicationController
@push_subscriptions = Current.user.push_subscriptions @push_subscriptions = Current.user.push_subscriptions
end 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 def push_subscription_params
params.require(:push_subscription).permit(:endpoint, :p256dh_key, :auth_key) params.require(:push_subscription).permit(:endpoint, :p256dh_key, :auth_key)
end end
+1 -1
View File
@@ -5,7 +5,7 @@ module ContentFilters
# #
# * SanitizeTags strips disallowed markup from the message body # * SanitizeTags strips disallowed markup from the message body
# * Action Text sanitizes the rendered content (lib/rails_ext/action_text_allowed_tags.rb) # * 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_TAGS = %w[ s u mark table thead tbody tfoot tr th td ]
EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ] EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ]
+25 -7
View File
@@ -1,9 +1,12 @@
module MessagesHelper 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_TAGS = Rails::HTML5::SafeListSanitizer.allowed_tags + ContentFilters::EDITOR_FORMATTING_TAGS
AUTO_LINK_ALLOWED_ATTRIBUTES = Rails::HTML5::SafeListSanitizer.allowed_attributes + ContentFilters::EDITOR_FORMATTING_ATTRIBUTES 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, &) def message_area_tag(room, &)
tag.div id: "message-area", class: "message-area", contents: true, data: { tag.div id: "message-area", class: "message-area", contents: true, data: {
controller: "messages presence drop-target", controller: "messages presence drop-target",
@@ -101,21 +104,36 @@ module MessagesHelper
private private
def text_message_presentation(body) def text_message_presentation(body)
render = -> do render = -> { auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(body)) }
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. # Embedded attachments render database-backed metadata and signed URLs.
# Plain text HTML depends only on its content, even after a foreign edit. # Plain text HTML depends only on its content, even after a foreign edit.
html = body.to_html html = body.to_html
if controller.perform_caching && !html.include?("<action-text-attachment") && html.bytesize <= ResponseCache::MAX_ENTRY_BYTES if controller.perform_caching && !html.include?("<action-text-attachment") && html.bytesize <= ResponseCache::MAX_ENTRY_BYTES
FragmentCache.store.fetch([ "text-presentation-v1", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call } FragmentCache.store.fetch([ "text-presentation-v2", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call }
else else
render.call render.call
end end
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 def messages_actions
"turbo:before-stream-render@document->messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage" "turbo:before-stream-render@document->messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage"
end end
+1
View File
@@ -2,6 +2,7 @@ module TranslationsHelper
TRANSLATIONS = { 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", "🇯🇵": "メールアドレスを入力してください" }, 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", "🇯🇵": "パスワードを入力してください" }, 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", "🇯🇵": "パスワードを変更" }, 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", "🇯🇵": "お名前を入力してください" }, 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", "🇯🇵": "アカウントに名前を付ける" }, account_name: { "🇺🇸": "Name this account", "🇪🇸": "Nombre de esta cuenta", "🇫🇷": "Nommez ce compte", "🇮🇳": "इस खाते का नाम दें", "🇩🇪": "Benennen Sie dieses Konto", "🇧🇷": "Dê um nome a essa conta", "🇯🇵": "アカウントに名前を付ける" },
@@ -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
@@ -106,7 +106,8 @@ export default class extends Controller {
async #syncPushSubscription(subscription) { async #syncPushSubscription(subscription) {
const response = await post(this.subscriptionsUrlValue, { body: this.#extractJsonPayloadAsString(subscription), responseKind: "turbo-stream" }) 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) { async #requestPermissionAndSubscribe(registration) {
@@ -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 } })
}
}
@@ -28,7 +28,7 @@ export default class extends Controller {
} }
#stripMentionAttachments(node) { #stripMentionAttachments(node) {
node.querySelectorAll(".mention").forEach(mention => mention.outerHTML = mention.textContent.trim()) node.querySelectorAll(".mention").forEach(mention => mention.replaceWith(mention.textContent.trim()))
return node return node
} }
+12 -3
View File
@@ -7,13 +7,22 @@ class Session < ApplicationRecord
before_create { self.last_active_at ||= Time.now } 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:) def resume(user_agent:, ip_address:)
if last_active_at.before?(ACTIVITY_REFRESH_RATE.ago) if last_active_at.before?(ACTIVITY_REFRESH_RATE.ago)
update! user_agent: user_agent, ip_address: ip_address, last_active_at: Time.now update! user_agent: user_agent, ip_address: ip_address, last_active_at: Time.now
end end
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 end
+27
View File
@@ -18,8 +18,10 @@ class User < ApplicationRecord
enum :status, %i[ active deactivated banned ], default: :active enum :status, %i[ active deactivated banned ], default: :active
has_secure_password validations: false 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_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 :ordered, -> { order("LOWER(name)") }
scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") } scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") }
@@ -50,6 +52,31 @@ class User < ApplicationRecord
end end
private 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 # 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 # 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, # membership away with the others. A room that granted itself to every user when it was created is skipped too,
+9 -2
View File
@@ -3,13 +3,20 @@ module User::Transferable
TRANSFER_LINK_EXPIRY_DURATION = 4.hours 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 class_methods do
def find_by_transfer_id(id) def find_by_transfer_id(id)
find_signed(id, purpose: :transfer) find_by_token_for(:transfer, id)
end end
end end
def transfer_id def transfer_id
signed_id(purpose: :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION) generate_token_for(:transfer)
end end
end end
+5
View File
@@ -62,6 +62,11 @@
<%= render "layouts/lightbox" %> <%= 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"> <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 %> <%= image_tag "campfire-icon.png", alt: "Campfire logo", width: 256, height: 216 %>
</a> </a>
+1 -1
View File
@@ -1,7 +1,7 @@
<%# Be sure to check/update messages/_template.html.erb when changing this file %> <%# 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). %> <%# 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-v7" ] do %> <% cache [ message_fragment_cache_key(message), "presentation-v8" ] do %>
<%= message_tag message do %> <%= message_tag message do %>
<h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2> <h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
+10
View File
@@ -69,6 +69,16 @@
</label> </label>
</div> </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"> <div class="flex align-center gap">
<%= translation_button(:update_password) %> <%= translation_button(:update_password) %>
+19 -23
View File
@@ -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
@@ -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
+1 -1
View File
@@ -29,7 +29,7 @@ module WebPush::PersistentRequest
# itself and re-resolve the endpoint host, so http.ipaddr would no longer # itself and re-resolve the endpoint host, so http.ipaddr would no longer
# pin the destination and the DNS-rebinding guarantee would be lost. # pin the destination and the DNS-rebinding guarantee would be lost.
# Push delivery to public vendor endpoints goes direct. # Push delivery to public vendor endpoints goes direct.
http = Net::HTTP.new(uri.host, uri.port, nil) http = (@options[:connection] ? WebPush::Connections::HTTP : Net::HTTP).new(uri.host, uri.port, nil)
http.ipaddr = endpoint_ip http.ipaddr = endpoint_ip
http.use_ssl = true http.use_ssl = true
http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil? http.ssl_timeout = @options[:ssl_timeout] unless @options[:ssl_timeout].nil?
+8 -3
View File
@@ -26,6 +26,12 @@ class WebPush::Connections
end end
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) def initialize(keep_alive_timeout: 30, max_idle: 150)
@keep_alive_timeout = keep_alive_timeout @keep_alive_timeout = keep_alive_timeout
@max_idle = max_idle @max_idle = max_idle
@@ -35,8 +41,8 @@ class WebPush::Connections
end end
def request(http, request) def request(http, request)
unless http.ipaddr && !http.proxy? && http.use_ssl? && !http.started? 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 are pooled" raise ArgumentError, "Only new, direct TLS connections pinned to an address and opened as WebPush::Connections::HTTP are pooled"
end end
address = [ http.address, http.port, http.ipaddr ] address = [ http.address, http.port, http.ipaddr ]
@@ -48,7 +54,6 @@ class WebPush::Connections
end end
end end
http.extend Stages
http.keep_alive_timeout = @keep_alive_timeout http.keep_alive_timeout = @keep_alive_timeout
http.start http.start
send_over(http, request, reused: false).tap { checkin(address, http) } send_over(http, request, reused: false).tap { checkin(address, http) }
@@ -18,4 +18,53 @@ class ApplicationCable::ConnectionTest < ActionCable::Connection::TestCase
assert_reject_connection { connect } assert_reject_connection { connect }
end 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 end
+35
View File
@@ -26,6 +26,18 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest
ActionController::Base.perform_caching = @previous_caching ActionController::Base.perform_caching = @previous_caching
end 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 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 = [ room_url(@room), room_messages_url(@room), user_sidebar_url(:me), searches_url(q: "hello") ]
urls.each do |url| urls.each do |url|
@@ -124,6 +136,29 @@ class CachedResponsesTest < ActionDispatch::IntegrationTest
assert_redirected_to new_session_url assert_redirected_to new_session_url
end 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 test "conditional requests keep native validators" do
get room_messages_url(@room) get room_messages_url(@room)
etag = response.headers["ETag"] etag = response.headers["ETag"]
@@ -19,4 +19,30 @@ class Sessions::TransfersControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to root_url assert_redirected_to root_url
assert parsed_cookies.signed[:session_token] assert parsed_cookies.signed[:session_token]
end 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 end
@@ -36,6 +36,19 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest
assert parsed_cookies.signed[:session_token] assert parsed_cookies.signed[:session_token]
end 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 test "create with invalid credentials" do
post session_url, params: { email_address: "david@37signals.com", password: "wrong" } post session_url, params: { email_address: "david@37signals.com", password: "wrong" }
@@ -27,9 +27,190 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
assert_equal "david@37signals.com", users(:david).email_address assert_equal "david@37signals.com", users(:david).email_address
end 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 test "updates are limited to the current user" do
put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } } put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } }
assert_equal "Jason", users(:jason).reload.name assert_equal "Jason", users(:jason).reload.name
end 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 end
@@ -74,4 +74,98 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to user_push_subscriptions_url assert_redirected_to user_push_subscriptions_url
end end
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 end
+41
View File
@@ -37,6 +37,47 @@ class MessagesHelperTest < ActionView::TestCase
assert_match /<strong>bold<\/strong>/, presentation assert_match /<strong>bold<\/strong>/, presentation
end 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 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)) presentation = view.message_presentation(attachment_message("moon.jpg", "image/jpeg", processed: true))
@@ -0,0 +1,197 @@
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
+10 -7
View File
@@ -76,11 +76,13 @@ class WebPush::ConnectionsTest < ActiveSupport::TestCase
end end
end end
test "a TLS failure on a new connection before the push is written is reported as it is" do class FailingCheckHTTP < WebPush::Connections::HTTP
failing_check = Module.new { private def begin_transport(*) = raise(OpenSSL::SSL::SSLError, "alert right after the handshake") } 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
with_push_service do |server| with_push_service do |server|
assert_raises(OpenSSL::SSL::SSLError) { @connections.request(pinned_connection(server).extend(failing_check), push_request) } assert_raises(OpenSSL::SSL::SSLError) { @connections.request(pinned_connection(server, http_class: FailingCheckHTTP), push_request) }
assert_empty server.requests assert_empty server.requests
end end
end end
@@ -97,14 +99,15 @@ class WebPush::ConnectionsTest < ActiveSupport::TestCase
end end
end end
test "only new, direct TLS connections pinned to an address are pooled" do test "only new, direct TLS connections pinned to an address and opened as WebPush::Connections::HTTP are pooled" do
with_push_service do |server| with_push_service do |server|
unpinned = Net::HTTP.new(HOST, server.port, nil).tap { it.use_ssl = true } unpinned = WebPush::Connections::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 } proxied = WebPush::Connections::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 } plain = pinned_connection(server).tap { it.use_ssl = false }
started = pinned_connection(server).tap(&:start) 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 ].each do |http| [ unpinned, proxied, plain, started, untracked ].each do |http|
assert_raises(ArgumentError) { @connections.request(http, push_request) } assert_raises(ArgumentError) { @connections.request(http, push_request) }
end end
assert_empty server.requests assert_empty server.requests
@@ -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
+20
View File
@@ -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
+40
View File
@@ -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
+72
View File
@@ -6,6 +6,14 @@ class UserTest < ActiveSupport::TestCase
assert users(:david).valid? assert users(:david).valid?
end 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 test "creating users grants membership to the open rooms" do
assert_difference -> { Membership.count }, +Rooms::Open.count do assert_difference -> { Membership.count }, +Rooms::Open.count do
create_new_user create_new_user
@@ -42,6 +50,70 @@ class UserTest < ActiveSupport::TestCase
end end
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 private
def create_new_user def create_new_user
User.create!(name: "User", email_address: "user@example.com", password: "secret123456") User.create!(name: "User", email_address: "user@example.com", password: "secret123456")
+21
View File
@@ -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
+14
View File
@@ -174,6 +174,20 @@ class ComposerTest < ApplicationSystemTestCase
assert_selector last_message_selector("cite"), text: "JZ" assert_selector last_message_selector("cite"), text: "JZ"
end 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 test "arrow up edits my last message when the composer is empty" do
composer_editor.click composer_editor.click
press_in_composer :up press_in_composer :up
@@ -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
@@ -108,8 +108,8 @@ module PushServiceTestHelper
end end
# Built as WebPush::PersistentRequest builds it for a delivery. # Built as WebPush::PersistentRequest builds it for a delivery.
def pinned_connection(server, ip = IP) def pinned_connection(server, ip = IP, http_class: WebPush::Connections::HTTP)
Net::HTTP.new(HOST, server.port, nil).tap do |http| http_class.new(HOST, server.port, nil).tap do |http|
http.ipaddr = ip http.ipaddr = ip
http.use_ssl = true http.use_ssl = true
end end