Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main'
Browse files Browse the repository at this point in the history
  • Loading branch information
TheEssem committed Feb 14, 2024
2 parents 76f1b63 + 486e4bc commit c16136b
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 31 deletions.
2 changes: 1 addition & 1 deletion app/controllers/auth/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
def self.provides_callback_for(provider)
define_method provider do
@provider = provider
@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?
record_login_activity
Expand Down
20 changes: 20 additions & 0 deletions app/lib/application_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,34 @@ module ApplicationExtension
extend ActiveSupport::Concern

included do
include Redisable

has_many :created_users, class_name: 'User', foreign_key: 'created_by_application_id', inverse_of: :created_by_application

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/user/omniauthable.rb
Original file line number Diff line number Diff line change
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 @@ -66,7 +83,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
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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
10 changes: 10 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,16 @@ def revoke_access!
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

Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
@@ -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'

class RedirectWithVary < ActionDispatch::Routing::PathRedirect
Expand Down
2 changes: 1 addition & 1 deletion lib/mastodon/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def patch
end

def default_prerelease
'alpha.1'
'alpha.2'
end

def prerelease
Expand Down
11 changes: 11 additions & 0 deletions lib/tasks/sidekiq_unique_jobs.rake
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@
require 'rails_helper'

RSpec.describe Identity 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
allow(described_class).to receive(:find_or_create_by)

described_class.find_for_oauth(auth)
described_class.find_for_omniauth(auth)

expect(described_class).to have_received(:find_or_create_by).with(uid: auth.uid, provider: auth.provider)
end

it 'returns an instance of Identity' do
expect(described_class.find_for_oauth(auth)).to be_instance_of described_class
expect(described_class.find_for_omniauth(auth)).to be_instance_of described_class
end
end
end
7 changes: 7 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,10 @@
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 @@ -455,6 +458,10 @@
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
expect { web_push_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound)
Expand Down
37 changes: 27 additions & 10 deletions spec/requests/omniauth_callbacks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,33 @@
Fabricate(:user, email: '[email protected]')
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)
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

expect(Identity.find_by(user: User.last).uid).to eq('123')
expect(response).to redirect_to(root_path)
context 'when ALLOW_UNSAFE_AUTH_PROVIDER_REATTACH is not set to true' do
it 'does not match the existing user or create an identity' do
expect { subject }
.to not_change(User, :count)
.and not_change(Identity, :count)
.and not_change(LoginActivity, :count)
end
end
end

Expand Down Expand Up @@ -96,7 +113,7 @@

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

it 'redirects to the new user signup page' do
Expand Down

0 comments on commit c16136b

Please sign in to comment.