Merge pull request #133 from Y-zu-don-maintenance-org/features/v4.1.14

Features/v4.1.14

CHANGELOG.md

@@ -3,6 +3,25 @@ Changelog
 
 All notable changes to this project will be documented in this file.
 
+## [4.1.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.1.13] - 2024-02-01
 
 ### Security

Gemfile.lock

@@ -405,7 +405,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.17.0)
     msgpack (1.6.0)
     multi_json (1.15.0)
@@ -424,8 +424,8 @@ GEM
       net-protocol
     net-ssh (7.0.1)
     nio4r (2.5.9)
-    nokogiri (1.14.5)
+    nokogiri (1.16.2)
-      mini_portile2 (~> 2.8.0)
+      mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     nsa (0.2.8)
       activesupport (>= 4.2, < 7)
@@ -496,7 +496,7 @@ GEM
     pundit (2.3.0)
       activesupport (>= 3.0.0)
     raabro (1.4.0)
-    racc (1.6.2)
+    racc (1.7.3)
     rack (2.2.8)
     rack-attack (6.6.1)
       rack (>= 1.0, < 3)
@@ -634,7 +634,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)
@@ -645,7 +645,7 @@ GEM
       rufus-scheduler (~> 3.2)
       sidekiq (>= 4, < 7)
       tilt (>= 1.4.0)
-    sidekiq-unique-jobs (7.1.29)
+    sidekiq-unique-jobs (7.1.33)
       brpoplpush-redis_script (> 0.1.1, <= 2.0.0)
       concurrent-ruby (~> 1.0, >= 1.0.5)
       redis (< 5.0)

app/controllers/auth/omniauth_callbacks_controller.rb

@@ -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(

app/lib/application_extension.rb

@@ -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

app/models/concerns/omniauthable.rb

@@ -19,17 +19,18 @@ module Omniauthable
   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
@@ -39,19 +40,35 @@ module Omniauthable
       user
     end
 
-    def create_for_oauth(auth)
+    private
-      # Check if the user exists with provided email. If no email was provided,
+
+    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'
+
+      email, email_is_verified = email_from_auth(auth)
+      return unless email_is_verified
+
+      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
 
-      strategy          = Devise.omniauth_configs[auth.provider.to_sym].strategy
+      email, email_is_verified = email_from_auth(auth)
-      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
-
-      user = User.find_by(email: email) if email_is_verified
-
-      return user unless user.nil?
 
       user = User.new(user_params_from_auth(email, auth))
 
@@ -68,7 +85,14 @@ module Omniauthable
       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)
       {

app/models/identity.rb

@@ -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

app/models/user.rb

@@ -400,6 +400,16 @@ class User < ApplicationRecord
     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
 

config/initializers/doorkeeper.rb

@@ -19,9 +19,14 @@ Doorkeeper.configure do
     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).

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

docker-compose.yml

@@ -56,7 +56,7 @@ services:
 
   web:
     build: .
-    image: ghcr.io/mastodon/mastodon:v4.1.13
+    image: ghcr.io/mastodon/mastodon:v4.1.14
     restart: always
     env_file: .env.production
     command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000"
@@ -77,7 +77,7 @@ services:
 
   streaming:
     build: .
-    image: ghcr.io/mastodon/mastodon:v4.1.13
+    image: ghcr.io/mastodon/mastodon:v4.1.14
     restart: always
     env_file: .env.production
     command: node ./streaming
@@ -95,7 +95,7 @@ services:
 
   sidekiq:
     build: .
-    image: ghcr.io/mastodon/mastodon:v4.1.13
+    image: ghcr.io/mastodon/mastodon:v4.1.14
     restart: always
     env_file: .env.production
     command: bundle exec sidekiq

lib/mastodon/version.rb

@@ -13,7 +13,7 @@ module Mastodon
     end
 
     def patch
-      13
+      14
     end
 
     def flags

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

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

spec/models/user_spec.rb

@@ -439,7 +439,10 @@ RSpec.describe User, type: :model do
     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
 
@@ -455,6 +458,10 @@ RSpec.describe User, type: :model do
       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

spec/requests/disabled_oauth_endpoints_spec.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'Disabled OAuth routes' do
+  # These routes are disabled via the doorkeeper configuration for
+  # `admin_authenticator`, as these routes should only be accessible by server
+  # administrators. For now, these routes are not properly designed and
+  # integrated into Mastodon, so we're disabling them completely
+  describe 'GET /oauth/applications' do
+    it 'returns 403 forbidden' do
+      get oauth_applications_path
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'POST /oauth/applications' do
+    it 'returns 403 forbidden' do
+      post oauth_applications_path
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'GET /oauth/applications/new' do
+    it 'returns 403 forbidden' do
+      get new_oauth_application_path
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'GET /oauth/applications/:id' do
+    let(:application) { Fabricate(:application, scopes: 'read') }
+
+    it 'returns 403 forbidden' do
+      get oauth_application_path(application)
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'PATCH /oauth/applications/:id' do
+    let(:application) { Fabricate(:application, scopes: 'read') }
+
+    it 'returns 403 forbidden' do
+      patch oauth_application_path(application)
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'PUT /oauth/applications/:id' do
+    let(:application) { Fabricate(:application, scopes: 'read') }
+
+    it 'returns 403 forbidden' do
+      put oauth_application_path(application)
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'DELETE /oauth/applications/:id' do
+    let(:application) { Fabricate(:application, scopes: 'read') }
+
+    it 'returns 403 forbidden' do
+      delete oauth_application_path(application)
+
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'GET /oauth/applications/:id/edit' do
+    let(:application) { Fabricate(:application, scopes: 'read') }
+
+    it 'returns 403 forbidden' do
+      get edit_oauth_application_path(application)
+
+      expect(response).to have_http_status(403)
+    end
+  end
+end