Merge pull request from GHSA-vp5r-5pgw-jwqx
* Fix streaming sessions not being closed when revoking access to an app * Add tests for GHSA-7w3c-p9j8-mq3x
This commit is contained in:
		@@ -17,6 +17,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio
 | 
			
		||||
 | 
			
		||||
  def destroy
 | 
			
		||||
    Web::PushSubscription.unsubscribe_for(params[:id], current_resource_owner)
 | 
			
		||||
    Doorkeeper::Application.find_by(id: params[:id])&.close_streaming_sessions(current_resource_owner)
 | 
			
		||||
    super
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -16,7 +16,7 @@ module ApplicationExtension
 | 
			
		||||
    # 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
 | 
			
		||||
    before_destroy :close_streaming_sessions, prepend: true
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def confirmation_redirect_uri
 | 
			
		||||
@@ -29,10 +29,12 @@ module ApplicationExtension
 | 
			
		||||
    redirect_uri.split
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def push_to_streaming_api
 | 
			
		||||
  def close_streaming_sessions(resource_owner = nil)
 | 
			
		||||
    # TODO: #28793 Combine into a single topic
 | 
			
		||||
    payload = Oj.dump(event: :kill)
 | 
			
		||||
    access_tokens.in_batches do |tokens|
 | 
			
		||||
    scope = access_tokens
 | 
			
		||||
    scope = scope.where(resource_owner_id: resource_owner.id) unless resource_owner.nil?
 | 
			
		||||
    scope.in_batches do |tokens|
 | 
			
		||||
      redis.pipelined do |pipeline|
 | 
			
		||||
        tokens.ids.each do |id|
 | 
			
		||||
          pipeline.publish("timeline:access_token:#{id}", payload)
 | 
			
		||||
 
 | 
			
		||||
@@ -50,9 +50,11 @@ describe Oauth::AuthorizedApplicationsController do
 | 
			
		||||
    let!(:application) { Fabricate(:application) }
 | 
			
		||||
    let!(:access_token) { Fabricate(:accessible_access_token, application: application, resource_owner_id: user.id) }
 | 
			
		||||
    let!(:web_push_subscription) { Fabricate(:web_push_subscription, user: user, access_token: access_token) }
 | 
			
		||||
    let(:redis_pipeline_stub) { instance_double(Redis::Namespace, publish: nil) }
 | 
			
		||||
 | 
			
		||||
    before do
 | 
			
		||||
      sign_in user, scope: :user
 | 
			
		||||
      allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub)
 | 
			
		||||
      post :destroy, params: { id: application.id }
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
@@ -67,5 +69,9 @@ describe Oauth::AuthorizedApplicationsController do
 | 
			
		||||
    it 'removes the web_push_subscription' do
 | 
			
		||||
      expect { web_push_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound)
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    it 'sends a session kill payload to the streaming server' do
 | 
			
		||||
      expect(redis_pipeline_stub).to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}')
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
 
 | 
			
		||||
@@ -147,14 +147,22 @@ describe Settings::ApplicationsController do
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  describe 'destroy' do
 | 
			
		||||
    let(:redis_pipeline_stub) { instance_double(Redis::Namespace, publish: nil) }
 | 
			
		||||
    let!(:access_token) { Fabricate(:accessible_access_token, application: app) }
 | 
			
		||||
 | 
			
		||||
    before do
 | 
			
		||||
      allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub)
 | 
			
		||||
      post :destroy, params: { id: app.id }
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    it 'redirects back to applications page and removes the app' do
 | 
			
		||||
    it 'redirects back to applications page removes the app' do
 | 
			
		||||
      expect(response).to redirect_to(settings_applications_path)
 | 
			
		||||
      expect(Doorkeeper::Application.find_by(id: app.id)).to be_nil
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    it 'sends a session kill payload to the streaming server' do
 | 
			
		||||
      expect(redis_pipeline_stub).to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}')
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  describe 'regenerate' do
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user