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:
parent
6cd9bd6ae1
commit
df974a912b
4 changed files with 24 additions and 3 deletions
|
@ -17,6 +17,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio
|
||||||
|
|
||||||
def destroy
|
def destroy
|
||||||
Web::PushSubscription.unsubscribe_for(params[:id], current_resource_owner)
|
Web::PushSubscription.unsubscribe_for(params[:id], current_resource_owner)
|
||||||
|
Doorkeeper::Application.find_by(id: params[:id])&.close_streaming_sessions(current_resource_owner)
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -16,17 +16,19 @@ module ApplicationExtension
|
||||||
# dependent: delete_all, which means the ActiveRecord callback in
|
# dependent: delete_all, which means the ActiveRecord callback in
|
||||||
# AccessTokenExtension is not run, so instead we manually announce to
|
# AccessTokenExtension is not run, so instead we manually announce to
|
||||||
# streaming that these tokens are being deleted.
|
# streaming that these tokens are being deleted.
|
||||||
before_destroy :push_to_streaming_api, prepend: true
|
before_destroy :close_streaming_sessions, prepend: true
|
||||||
end
|
end
|
||||||
|
|
||||||
def confirmation_redirect_uri
|
def confirmation_redirect_uri
|
||||||
redirect_uri.lines.first.strip
|
redirect_uri.lines.first.strip
|
||||||
end
|
end
|
||||||
|
|
||||||
def push_to_streaming_api
|
def close_streaming_sessions(resource_owner = nil)
|
||||||
# TODO: #28793 Combine into a single topic
|
# TODO: #28793 Combine into a single topic
|
||||||
payload = Oj.dump(event: :kill)
|
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|
|
redis.pipelined do |pipeline|
|
||||||
tokens.ids.each do |id|
|
tokens.ids.each do |id|
|
||||||
pipeline.publish("timeline:access_token:#{id}", payload)
|
pipeline.publish("timeline:access_token:#{id}", payload)
|
||||||
|
|
|
@ -50,9 +50,11 @@ describe Oauth::AuthorizedApplicationsController do
|
||||||
let!(:application) { Fabricate(:application) }
|
let!(:application) { Fabricate(:application) }
|
||||||
let!(:access_token) { Fabricate(:accessible_access_token, application: application, resource_owner_id: user.id) }
|
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!(: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
|
before do
|
||||||
sign_in user, scope: :user
|
sign_in user, scope: :user
|
||||||
|
allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub)
|
||||||
post :destroy, params: { id: application.id }
|
post :destroy, params: { id: application.id }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -63,5 +65,13 @@ describe Oauth::AuthorizedApplicationsController do
|
||||||
it 'removes subscriptions for the application\'s access tokens' do
|
it 'removes subscriptions for the application\'s access tokens' do
|
||||||
expect(Web::PushSubscription.where(user: user).count).to eq 0
|
expect(Web::PushSubscription.where(user: user).count).to eq 0
|
||||||
end
|
end
|
||||||
|
|
||||||
|
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
|
||||||
end
|
end
|
||||||
|
|
|
@ -166,7 +166,11 @@ describe Settings::ApplicationsController do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'destroy' do
|
describe 'destroy' do
|
||||||
|
let(:redis_pipeline_stub) { instance_double(Redis::Namespace, publish: nil) }
|
||||||
|
let!(:access_token) { Fabricate(:accessible_access_token, application: app) }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub)
|
||||||
post :destroy, params: { id: app.id }
|
post :destroy, params: { id: app.id }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -177,6 +181,10 @@ describe Settings::ApplicationsController do
|
||||||
it 'removes the app' do
|
it 'removes the app' do
|
||||||
expect(Doorkeeper::Application.find_by(id: app.id)).to be_nil
|
expect(Doorkeeper::Application.find_by(id: app.id)).to be_nil
|
||||||
end
|
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
|
||||||
|
|
||||||
describe 'regenerate' do
|
describe 'regenerate' do
|
||||||
|
|
Loading…
Reference in a new issue