From 395f17ca17b27f9e722425a4d76ef1b02bcebea8 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Thu, 4 Jul 2024 16:11:28 +0200
Subject: [PATCH] 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
---
 .../oauth/authorized_applications_controller.rb        |  1 +
 app/lib/application_extension.rb                       |  8 +++++---
 .../oauth/authorized_applications_controller_spec.rb   |  6 ++++++
 .../settings/applications_controller_spec.rb           | 10 +++++++++-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb
index 8440df6b7e..7bb22453ca 100644
--- a/app/controllers/oauth/authorized_applications_controller.rb
+++ b/app/controllers/oauth/authorized_applications_controller.rb
@@ -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
 
diff --git a/app/lib/application_extension.rb b/app/lib/application_extension.rb
index 2fea1057cb..d7aaeba5bd 100644
--- a/app/lib/application_extension.rb
+++ b/app/lib/application_extension.rb
@@ -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)
diff --git a/spec/controllers/oauth/authorized_applications_controller_spec.rb b/spec/controllers/oauth/authorized_applications_controller_spec.rb
index b46b944d0e..3fd9f9499f 100644
--- a/spec/controllers/oauth/authorized_applications_controller_spec.rb
+++ b/spec/controllers/oauth/authorized_applications_controller_spec.rb
@@ -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
diff --git a/spec/controllers/settings/applications_controller_spec.rb b/spec/controllers/settings/applications_controller_spec.rb
index ccbb634911..ce2e0749a7 100644
--- a/spec/controllers/settings/applications_controller_spec.rb
+++ b/spec/controllers/settings/applications_controller_spec.rb
@@ -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