Skip to content

Commit

Permalink
Upstream backports (mastodon#1343)
Browse files Browse the repository at this point in the history
This backports the commits from the upstream `stable-4.0` branch,
including the security content of
[4.2.6](https://github.com/mastodon/mastodon/releases/tag/v4.2.6).

---------

Co-authored-by: blah <blah@blah>
Co-authored-by: Emelia Smith <ThisIsMissEm@users.noreply.github.com>
Co-authored-by: Claire <claire.github-309c@sitedethib.com>
  • Loading branch information
4 people committed Feb 14, 2024
1 parent 13fa4f7 commit 3c9599f
Show file tree
Hide file tree
Showing 15 changed files with 222 additions and 38 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,25 @@ All notable changes to this project will be documented in this file.
**The 4.0.x branch has reached its end of life and will not receive any further update.**
This means that no security fix will be made available for this branch after this date, and you will need to update to a more recent version (such as the 4.2.x branch) to receive security fixes.

## [4.0.14] - 2024-02-14

### Security

- Update the `sidekiq-unique-jobs` dependency (see [GHSA-cmh9-rx85-xj38](https://github.com/mhenrixon/sidekiq-unique-jobs/security/advisories/GHSA-cmh9-rx85-xj38))
In addition, we have disabled the web interface for `sidekiq-unique-jobs` out of caution.
If you need it, you can re-enable it by setting `ENABLE_SIDEKIQ_UNIQUE_JOBS_UI=true`.
If you only need to clear all locks, you can now use `bundle exec rake sidekiq_unique_jobs:delete_all_locks`.
- Update the `nokogiri` dependency (see [GHSA-xc9x-jj77-9p9j](https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-xc9x-jj77-9p9j))
- Disable administrative Doorkeeper routes ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/29187))
- Fix ongoing streaming sessions not being invalidated when applications get deleted in some cases ([GHSA-7w3c-p9j8-mq3x](https://github.com/mastodon/mastodon/security/advisories/GHSA-7w3c-p9j8-mq3x))
In some rare cases, the streaming server was not notified of access tokens revocation on application deletion.
- Change external authentication behavior to never reattach a new identity to an existing user by default ([GHSA-vm39-j3vx-pch3](https://github.com/mastodon/mastodon/security/advisories/GHSA-vm39-j3vx-pch3))
Up until now, Mastodon has allowed new identities from external authentication providers to attach to an existing local user based on their verified e-mail address.
This allowed upgrading users from a database-stored password to an external authentication provider, or move from one authentication provider to another.
However, this behavior may be unexpected, and means that when multiple authentication providers are configured, the overall security would be that of the least secure authentication provider.
For these reasons, this behavior is now locked under the `ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH` environment variable.
In addition, regardless of this environment variable, Mastodon will refuse to attach two identities from the same authentication provider to the same account.

## [4.0.13] - 2024-02-01

### Security
Expand Down
13 changes: 7 additions & 6 deletions Gemfile.lock
Expand Up @@ -402,7 +402,7 @@ GEM
mime-types-data (~> 3.2015)
mime-types-data (3.2022.0105)
mini_mime (1.1.5)
mini_portile2 (2.8.4)
mini_portile2 (2.8.5)
minitest (5.20.0)
msgpack (1.5.4)
multi_json (1.15.0)
Expand All @@ -412,7 +412,7 @@ GEM
net-ssh (>= 2.6.5, < 8.0.0)
net-ssh (7.0.1)
nio4r (2.5.9)
nokogiri (1.15.4)
nokogiri (1.16.2)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
nsa (0.2.8)
Expand Down Expand Up @@ -482,7 +482,7 @@ GEM
pundit (2.2.0)
activesupport (>= 3.0.0)
raabro (1.4.0)
racc (1.7.1)
racc (1.7.3)
rack (2.2.8)
rack-attack (6.6.1)
rack (>= 1.0, < 3)
Expand Down Expand Up @@ -609,7 +609,7 @@ GEM
activerecord (>= 4.0.0)
railties (>= 4.0.0)
semantic_range (3.0.0)
sidekiq (6.5.11)
sidekiq (6.5.12)
connection_pool (>= 2.2.5, < 3)
rack (~> 2.0)
redis (>= 4.5.0, < 5)
Expand All @@ -620,10 +620,11 @@ GEM
rufus-scheduler (~> 3.2)
sidekiq (>= 4, < 7)
tilt (>= 1.4.0)
sidekiq-unique-jobs (7.1.27)
sidekiq-unique-jobs (7.1.33)
brpoplpush-redis_script (> 0.1.1, <= 2.0.0)
concurrent-ruby (~> 1.0, >= 1.0.5)
sidekiq (>= 5.0, < 8.0)
redis (< 5.0)
sidekiq (>= 5.0, < 7.0)
thor (>= 0.20, < 3.0)
simple-navigation (4.4.0)
activesupport (>= 2.3.2)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/auth/omniauth_callbacks_controller.rb
Expand Up @@ -5,7 +5,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController

def self.provides_callback_for(provider)
define_method provider do
@user = User.find_for_oauth(request.env['omniauth.auth'], current_user)
@user = User.find_for_omniauth(request.env['omniauth.auth'], current_user)

if @user.persisted?
LoginActivity.create(
Expand Down
20 changes: 20 additions & 0 deletions app/lib/application_extension.rb
Expand Up @@ -4,12 +4,32 @@ module ApplicationExtension
extend ActiveSupport::Concern

included do
include Redisable

validates :name, length: { maximum: 60 }
validates :website, url: true, length: { maximum: 2_000 }, if: :website?
validates :redirect_uri, length: { maximum: 2_000 }

# The relationship used between Applications and AccessTokens is using
# dependent: delete_all, which means the ActiveRecord callback in
# AccessTokenExtension is not run, so instead we manually announce to
# streaming that these tokens are being deleted.
before_destroy :push_to_streaming_api, prepend: true
end

def confirmation_redirect_uri
redirect_uri.lines.first.strip
end

def push_to_streaming_api
# TODO: #28793 Combine into a single topic
payload = Oj.dump(event: :kill)
access_tokens.in_batches do |tokens|
redis.pipelined do |pipeline|
tokens.ids.each do |id|
pipeline.publish("timeline:access_token:#{id}", payload)
end
end
end
end
end
52 changes: 38 additions & 14 deletions app/models/concerns/omniauthable.rb
Expand Up @@ -19,17 +19,18 @@ def email_present?
end

class_methods do
def find_for_oauth(auth, signed_in_resource = nil)
def find_for_omniauth(auth, signed_in_resource = nil)
# EOLE-SSO Patch
auth.uid = (auth.uid[0][:uid] || auth.uid[0][:user]) if auth.uid.is_a? Hashie::Array
identity = Identity.find_for_oauth(auth)
identity = Identity.find_for_omniauth(auth)

# If a signed_in_resource is provided it always overrides the existing user
# to prevent the identity being locked with accidentally created accounts.
# Note that this may leave zombie accounts (with no associated identity) which
# can be cleaned up at a later date.
user = signed_in_resource || identity.user
user ||= create_for_oauth(auth)
user ||= reattach_for_auth(auth)
user ||= create_for_auth(auth)

if identity.user.nil?
identity.user = user
Expand All @@ -39,19 +40,35 @@ def find_for_oauth(auth, signed_in_resource = nil)
user
end

def create_for_oauth(auth)
# Check if the user exists with provided email. If no email was provided,
# we assign a temporary email and ask the user to verify it on
# the next step via Auth::SetupController.show
private

strategy = Devise.omniauth_configs[auth.provider.to_sym].strategy
assume_verified = strategy&.security&.assume_email_is_verified
email_is_verified = auth.info.verified || auth.info.verified_email || auth.info.email_verified || assume_verified
email = auth.info.verified_email || auth.info.email
def reattach_for_auth(auth)
# If allowed, check if a user exists with the provided email address,
# and return it if they does not have an associated identity with the
# current authentication provider.

# This can be used to provide a choice of alternative auth providers
# or provide smooth gradual transition between multiple auth providers,
# but this is discouraged because any insecure provider will put *all*
# local users at risk, regardless of which provider they registered with.

return unless ENV['ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH'] == 'true'

user = User.find_by(email: email) if email_is_verified
email, email_is_verified = email_from_auth(auth)
return unless email_is_verified

return user unless user.nil?
user = User.find_by(email: email)
return if user.nil? || Identity.exists?(provider: auth.provider, user_id: user.id)

user
end

def create_for_auth(auth)
# Create a user for the given auth params. If no email was provided,
# we assign a temporary email and ask the user to verify it on
# the next step via Auth::SetupController.show

email, email_is_verified = email_from_auth(auth)

user = User.new(user_params_from_auth(email, auth))

Expand All @@ -61,7 +78,14 @@ def create_for_oauth(auth)
user
end

private
def email_from_auth(auth)
strategy = Devise.omniauth_configs[auth.provider.to_sym].strategy
assume_verified = strategy&.security&.assume_email_is_verified
email_is_verified = auth.info.verified || auth.info.verified_email || auth.info.email_verified || assume_verified
email = auth.info.verified_email || auth.info.email

[email, email_is_verified]
end

def user_params_from_auth(email, auth)
{
Expand Down
2 changes: 1 addition & 1 deletion app/models/identity.rb
Expand Up @@ -16,7 +16,7 @@ class Identity < ApplicationRecord
validates :uid, presence: true, uniqueness: { scope: :provider }
validates :provider, presence: true

def self.find_for_oauth(auth)
def self.find_for_omniauth(auth)
find_or_create_by(uid: auth.uid, provider: auth.provider)
end
end
26 changes: 20 additions & 6 deletions app/models/user.rb
Expand Up @@ -373,6 +373,25 @@ def reset_password(new_password, new_password_confirmation)
super
end

def revoke_access!
Doorkeeper::AccessGrant.by_resource_owner(self).update_all(revoked_at: Time.now.utc)

Doorkeeper::AccessToken.by_resource_owner(self).in_batches do |batch|
batch.update_all(revoked_at: Time.now.utc)
Web::PushSubscription.where(access_token_id: batch).delete_all

# Revoke each access token for the Streaming API, since `update_all``
# doesn't trigger ActiveRecord Callbacks:
# TODO: #28793 Combine into a single topic
payload = Oj.dump(event: :kill)
redis.pipelined do |pipeline|
batch.ids.each do |id|
pipeline.publish("timeline:access_token:#{id}", payload)
end
end
end
end

def reset_password!
# First, change password to something random and deactivate all sessions
transaction do
Expand All @@ -381,12 +400,7 @@ def reset_password!
end

# Then, remove all authorized applications and connected push subscriptions
Doorkeeper::AccessGrant.by_resource_owner(self).in_batches.update_all(revoked_at: Time.now.utc)

Doorkeeper::AccessToken.by_resource_owner(self).in_batches do |batch|
batch.update_all(revoked_at: Time.now.utc)
Web::PushSubscription.where(access_token_id: batch).delete_all
end
revoke_access!

# Finally, send a reset password prompt to the user
send_reset_password_instructions
Expand Down
9 changes: 7 additions & 2 deletions config/initializers/doorkeeper.rb
Expand Up @@ -19,9 +19,14 @@
user unless user&.otp_required_for_login?
end

# If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
# Doorkeeper provides some administrative interfaces for managing OAuth
# Applications, allowing creation, edit, and deletion of applications from the
# server. At present, these administrative routes are not integrated into
# Mastodon, and as such, we've disabled them by always return a 403 forbidden
# response for them. This does not affect the ability for users to manage
# their own OAuth Applications.
admin_authenticator do
current_user&.admin? || redirect_to(new_user_session_url)
head 403
end

# Authorization Code expiration time (default 10 minutes).
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require 'sidekiq_unique_jobs/web'
require 'sidekiq_unique_jobs/web' if ENV['ENABLE_SIDEKIQ_UNIQUE_JOBS_UI'] == true
require 'sidekiq-scheduler/web'

Rails.application.routes.draw do
Expand Down
6 changes: 3 additions & 3 deletions docker-compose.yml
Expand Up @@ -56,7 +56,7 @@ services:

web:
build: .
image: ghcr.io/mastodon/mastodon:v4.0.13
image: ghcr.io/mastodon/mastodon:v4.0.14
restart: always
env_file: .env.production
command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000"
Expand All @@ -77,7 +77,7 @@ services:

streaming:
build: .
image: ghcr.io/mastodon/mastodon:v4.0.13
image: ghcr.io/mastodon/mastodon:v4.0.14
restart: always
env_file: .env.production
command: node ./streaming
Expand All @@ -95,7 +95,7 @@ services:

sidekiq:
build: .
image: ghcr.io/mastodon/mastodon:v4.0.13
image: ghcr.io/mastodon/mastodon:v4.0.14
restart: always
env_file: .env.production
command: bundle exec sidekiq
Expand Down
2 changes: 1 addition & 1 deletion lib/mastodon/version.rb
Expand Up @@ -13,7 +13,7 @@ def minor
end

def patch
13
14
end

def flags
Expand Down
11 changes: 11 additions & 0 deletions lib/tasks/sidekiq_unique_jobs.rake
@@ -0,0 +1,11 @@
# frozen_string_literal: true

namespace :sidekiq_unique_jobs do
task delete_all_locks: :environment do
digests = SidekiqUniqueJobs::Digests.new
digests.delete_by_pattern('*', count: digests.count)

expiring_digests = SidekiqUniqueJobs::ExpiringDigests.new
expiring_digests.delete_by_pattern('*', count: expiring_digests.count)
end
end
6 changes: 3 additions & 3 deletions spec/models/identity_spec.rb
@@ -1,16 +1,16 @@
require 'rails_helper'

RSpec.describe Identity, type: :model do
describe '.find_for_oauth' do
describe '.find_for_omniauth' do
let(:auth) { Fabricate(:identity, user: Fabricate(:user)) }

it 'calls .find_or_create_by' do
expect(described_class).to receive(:find_or_create_by).with(uid: auth.uid, provider: auth.provider)
described_class.find_for_oauth(auth)
described_class.find_for_omniauth(auth)
end

it 'returns an instance of Identity' do
expect(described_class.find_for_oauth(auth)).to be_instance_of Identity
expect(described_class.find_for_omniauth(auth)).to be_instance_of Identity
end
end
end
7 changes: 7 additions & 0 deletions spec/models/user_spec.rb
Expand Up @@ -313,7 +313,10 @@ def fabricate
let!(:access_token) { Fabricate(:access_token, resource_owner_id: user.id) }
let!(:web_push_subscription) { Fabricate(:web_push_subscription, access_token: access_token) }

let(:redis_pipeline_stub) { instance_double(Redis::Namespace, publish: nil) }

before do
allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub)
user.reset_password!
end

Expand All @@ -329,6 +332,10 @@ def fabricate
expect(Doorkeeper::AccessToken.active_for(user).count).to eq 0
end

it 'revokes streaming access for all access tokens' do
expect(redis_pipeline_stub).to have_received(:publish).with("timeline:access_token:#{access_token.id}", Oj.dump(event: :kill)).once
end

it 'removes push subscriptions' do
expect(Web::PushSubscription.where(user: user).or(Web::PushSubscription.where(access_token: access_token)).count).to eq 0
end
Expand Down

0 comments on commit 3c9599f

Please sign in to comment.