Skip to content

Commit

Permalink
Merge upstream changes from v4.0.15 (#1344)
Browse files Browse the repository at this point in the history
This includes the security content of Mastodon 4.0.15/4.2.7.
https://github.com/mastodon/mastodon/releases/tag/v4.2.7

---------

Co-authored-by: Claire <claire.github-309c@sitedethib.com>
Co-authored-by: Matt Jankowski <matt@jankowski.online>
  • Loading branch information
3 people authored Feb 16, 2024
1 parent 3c9599f commit ba20b7d
Show file tree
Hide file tree
Showing 19 changed files with 222 additions and 46 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ 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.15] - 2024-02-16

### Fixed

- Fix OmniAuth tests and edge cases in error handling ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/29201), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/29207))

### Security

- Fix insufficient checking of remote posts ([GHSA-jhrq-qvrm-qr36](https://github.com/mastodon/mastodon/security/advisories/GHSA-jhrq-qvrm-qr36))

## [4.0.14] - 2024-02-14

### Security
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/auth/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ def self.provides_callback_for(provider)
session["devise.#{provider}_data"] = request.env['omniauth.auth']
redirect_to new_user_registration_url
end
rescue ActiveRecord::RecordInvalid
flash[:alert] = I18n.t('devise.failure.omniauth_user_creation_failure') if is_navigational_format?
redirect_to new_user_session_url
end
end

Expand Down
14 changes: 13 additions & 1 deletion app/helpers/jsonld_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,19 @@ def fetch_resource_without_id_validation(uri, on_behalf_of = nil, raise_on_tempo
build_request(uri, on_behalf_of).perform do |response|
raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error

body_to_json(response.body_with_limit) if response.code == 200
body_to_json(response.body_with_limit) if response.code == 200 && valid_activitypub_content_type?(response)
end
end

def valid_activitypub_content_type?(response)
return true if response.mime_type == 'application/activity+json'

# When the mime type is `application/ld+json`, we need to check the profile,
# but `http.rb` does not parse it for us.
return false unless response.mime_type == 'application/ld+json'

response.headers[HTTP::Headers::CONTENT_TYPE]&.split(';')&.map(&:strip)&.any? do |str|
str.start_with?('profile="') && str[9...-1].split.include?('https://www.w3.org/ns/activitystreams')
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/fetch_resource_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def process_response(response, terminal = false)
@response_code = response.code
return nil if response.code != 200

if ['application/activity+json', 'application/ld+json'].include?(response.mime_type)
if valid_activitypub_content_type?(response)
body = response.body_with_limit
json = body_to_json(body)

Expand Down
1 change: 1 addition & 0 deletions config/locales/devise.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ en:
last_attempt: You have one more attempt before your account is locked.
locked: Your account is locked.
not_found_in_database: Invalid %{authentication_keys} or password.
omniauth_user_creation_failure: Error creating an account for this identity.
pending: Your account is still under review.
timeout: Your session expired. Please sign in again to continue.
unauthenticated: You need to sign in or sign up before continuing.
Expand Down
6 changes: 3 additions & 3 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ services:

web:
build: .
image: ghcr.io/mastodon/mastodon:v4.0.14
image: ghcr.io/mastodon/mastodon:v4.0.15
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.14
image: ghcr.io/mastodon/mastodon:v4.0.15
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.14
image: ghcr.io/mastodon/mastodon:v4.0.15
restart: always
env_file: .env.production
command: bundle exec sidekiq
Expand Down
2 changes: 1 addition & 1 deletion lib/mastodon/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def minor
end

def patch
14
15
end

def flags
Expand Down
14 changes: 7 additions & 7 deletions spec/helpers/jsonld_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,36 +56,36 @@
describe '#fetch_resource' do
context 'when the second argument is false' do
it 'returns resource even if the retrieved ID and the given URI does not match' do
stub_request(:get, 'https://bob.test/').to_return body: '{"id": "https://alice.test/"}'
stub_request(:get, 'https://alice.test/').to_return body: '{"id": "https://alice.test/"}'
stub_request(:get, 'https://bob.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })
stub_request(:get, 'https://alice.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })

expect(fetch_resource('https://bob.test/', false)).to eq({ 'id' => 'https://alice.test/' })
end

it 'returns nil if the object identified by the given URI and the object identified by the retrieved ID does not match' do
stub_request(:get, 'https://mallory.test/').to_return body: '{"id": "https://marvin.test/"}'
stub_request(:get, 'https://marvin.test/').to_return body: '{"id": "https://alice.test/"}'
stub_request(:get, 'https://mallory.test/').to_return(body: '{"id": "https://marvin.test/"}', headers: { 'Content-Type': 'application/activity+json' })
stub_request(:get, 'https://marvin.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })

expect(fetch_resource('https://mallory.test/', false)).to eq nil
end
end

context 'when the second argument is true' do
it 'returns nil if the retrieved ID and the given URI does not match' do
stub_request(:get, 'https://mallory.test/').to_return body: '{"id": "https://alice.test/"}'
stub_request(:get, 'https://mallory.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })
expect(fetch_resource('https://mallory.test/', true)).to eq nil
end
end
end

describe '#fetch_resource_without_id_validation' do
it 'returns nil if the status code is not 200' do
stub_request(:get, 'https://host.test/').to_return status: 400, body: '{}'
stub_request(:get, 'https://host.test/').to_return(status: 400, body: '{}', headers: { 'Content-Type': 'application/activity+json' })
expect(fetch_resource_without_id_validation('https://host.test/')).to eq nil
end

it 'returns hash' do
stub_request(:get, 'https://host.test/').to_return status: 200, body: '{}'
stub_request(:get, 'https://host.test/').to_return(status: 200, body: '{}', headers: { 'Content-Type': 'application/activity+json' })
expect(fetch_resource_without_id_validation('https://host.test/')).to eq({})
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/activitypub/activity/announce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
context 'when sender is followed by a local account' do
before do
Fabricate(:account).follow!(sender)
stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json))
stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json), headers: { 'Content-Type': 'application/activity+json' })
subject.perform
end

Expand Down Expand Up @@ -118,7 +118,7 @@
subject { described_class.new(json, sender, relayed_through_actor: relay_account) }

before do
stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json))
stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json), headers: { 'Content-Type': 'application/activity+json' })
end

context 'and the relay is enabled' do
Expand Down
143 changes: 143 additions & 0 deletions spec/requests/omniauth_callbacks_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
# frozen_string_literal: true

require 'rails_helper'

describe 'OmniAuth callbacks' do
shared_examples 'omniauth provider callbacks' do |provider|
subject { post send :"user_#{provider}_omniauth_callback_path" }

context 'with full information in response' do
before do
mock_omniauth(provider, {
provider: provider.to_s,
uid: '123',
info: {
verified: 'true',
email: 'user@host.example',
},
})
end

context 'without a matching user' do
it 'creates a user and an identity and redirects to root path' do
expect { subject }
.to change(User, :count)
.by(1)
.and change(Identity, :count)
.by(1)
.and change(LoginActivity, :count)
.by(1)

expect(User.last.email).to eq('user@host.example')
expect(Identity.find_by(user: User.last).uid).to eq('123')
expect(response).to redirect_to(root_path)
end
end

context 'with a matching user and no matching identity' do
before do
Fabricate(:user, email: 'user@host.example')
end

context 'when ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH is set to true' do
around do |example|
ClimateControl.modify ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH: 'true' do
example.run
end
end

it 'matches the existing user, creates an identity, and redirects to root path' do
expect { subject }
.to not_change(User, :count)
.and change(Identity, :count)
.by(1)
.and change(LoginActivity, :count)
.by(1)

expect(Identity.find_by(user: User.last).uid).to eq('123')
expect(response).to redirect_to(root_path)
end
end

context 'when ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH is not set to true' do
it 'does not match the existing user or create an identity, and redirects to login page' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and not_change(LoginActivity, :count)

expect(response).to redirect_to(new_user_session_url)
end
end
end

context 'with a matching user and a matching identity' do
before do
user = Fabricate(:user, email: 'user@host.example')
Fabricate(:identity, user: user, uid: '123', provider: provider)
end

it 'matches the existing records and redirects to root path' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and change(LoginActivity, :count)
.by(1)

expect(response).to redirect_to(root_path)
end
end
end

context 'with a response missing email address' do
before do
mock_omniauth(provider, {
provider: provider.to_s,
uid: '123',
info: {
verified: 'true',
},
})
end

it 'redirects to the auth setup page' do
expect { subject }
.to change(User, :count)
.by(1)
.and change(Identity, :count)
.by(1)
.and change(LoginActivity, :count)
.by(1)

expect(response).to redirect_to(auth_setup_path(missing_email: '1'))
end
end

context 'when a user cannot be built' do
before do
allow(User).to receive(:find_for_omniauth).and_return(User.new)
end

it 'redirects to the new user signup page' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and not_change(LoginActivity, :count)

expect(response).to redirect_to(new_user_registration_url)
end
end
end

describe '#openid_connect', if: ENV['OIDC_ENABLED'] == 'true' && ENV['OIDC_SCOPE'].present? do
include_examples 'omniauth provider callbacks', :openid_connect
end

describe '#cas', if: ENV['CAS_ENABLED'] == 'true' do
include_examples 'omniauth provider callbacks', :cas
end

describe '#saml', if: ENV['SAML_ENABLED'] == 'true' do
include_examples 'omniauth provider callbacks', :saml
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@

shared_examples 'sets pinned posts' do
before do
stub_request(:get, 'https://example.com/account/pinned/1').to_return(status: 200, body: Oj.dump(status_json_1))
stub_request(:get, 'https://example.com/account/pinned/2').to_return(status: 200, body: Oj.dump(status_json_2))
stub_request(:get, 'https://example.com/account/pinned/1').to_return(status: 200, body: Oj.dump(status_json_1), headers: { 'Content-Type': 'application/activity+json' })
stub_request(:get, 'https://example.com/account/pinned/2').to_return(status: 200, body: Oj.dump(status_json_2), headers: { 'Content-Type': 'application/activity+json' })
stub_request(:get, 'https://example.com/account/pinned/3').to_return(status: 404)
stub_request(:get, 'https://example.com/account/pinned/4').to_return(status: 200, body: Oj.dump(status_json_4))
stub_request(:get, 'https://example.com/account/pinned/4').to_return(status: 200, body: Oj.dump(status_json_4), headers: { 'Content-Type': 'application/activity+json' })

subject.call(actor, note: true, hashtag: false)
end
Expand All @@ -76,7 +76,7 @@
describe '#call' do
context 'when the endpoint is a Collection' do
before do
stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
end

it_behaves_like 'sets pinned posts'
Expand All @@ -93,7 +93,7 @@
end

before do
stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
end

it_behaves_like 'sets pinned posts'
Expand All @@ -114,7 +114,7 @@
end

before do
stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
end

it_behaves_like 'sets pinned posts'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@
describe '#call' do
context 'when the endpoint is a Collection' do
before do
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
end

it_behaves_like 'sets featured tags'
end

context 'when the account already has featured tags' do
before do
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })

actor.featured_tags.create!(name: 'FoO')
actor.featured_tags.create!(name: 'baz')
Expand All @@ -65,7 +65,7 @@
end

before do
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
end

it_behaves_like 'sets featured tags'
Expand All @@ -86,7 +86,7 @@
end

before do
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload))
stub_request(:get, collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
end

it_behaves_like 'sets featured tags'
Expand Down
Loading

0 comments on commit ba20b7d

Please sign in to comment.