From 8e4fea77e311399e4bcfff729aa06fed4e82e57c Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 6 May 2024 14:41:14 +0200 Subject: [PATCH] Fix race condition in `POST /api/v1/push/subscription` (#30166) --- .../api/v1/push/subscriptions_controller.rb | 33 ++++++++++++------- app/lib/access_token_extension.rb | 2 ++ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb index 3634acf956..e1ad89ee3e 100644 --- a/app/controllers/api/v1/push/subscriptions_controller.rb +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -1,9 +1,12 @@ # frozen_string_literal: true class Api::V1::Push::SubscriptionsController < Api::BaseController + include Redisable + include Lockable + before_action -> { doorkeeper_authorize! :push } before_action :require_user! - before_action :set_push_subscription + before_action :set_push_subscription, only: [:show, :update] before_action :check_push_subscription, only: [:show, :update] def show @@ -11,16 +14,18 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController end def create - @push_subscription&.destroy! + with_redis_lock("push_subscription:#{current_user.id}") do + destroy_web_push_subscriptions! - @push_subscription = Web::PushSubscription.create!( - endpoint: subscription_params[:endpoint], - key_p256dh: subscription_params[:keys][:p256dh], - key_auth: subscription_params[:keys][:auth], - data: data_params, - user_id: current_user.id, - access_token_id: doorkeeper_token.id - ) + @push_subscription = Web::PushSubscription.create!( + endpoint: subscription_params[:endpoint], + key_p256dh: subscription_params[:keys][:p256dh], + key_auth: subscription_params[:keys][:auth], + data: data_params, + user_id: current_user.id, + access_token_id: doorkeeper_token.id + ) + end render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end @@ -31,14 +36,18 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController end def destroy - @push_subscription&.destroy! + destroy_web_push_subscriptions! render_empty end private + def destroy_web_push_subscriptions! + doorkeeper_token.web_push_subscriptions.destroy_all + end + def set_push_subscription - @push_subscription = Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) + @push_subscription = doorkeeper_token.web_push_subscriptions.first end def check_push_subscription diff --git a/app/lib/access_token_extension.rb b/app/lib/access_token_extension.rb index f51bde4927..4e9585dd1e 100644 --- a/app/lib/access_token_extension.rb +++ b/app/lib/access_token_extension.rb @@ -6,6 +6,8 @@ module AccessTokenExtension included do include Redisable + has_many :web_push_subscriptions, class_name: 'Web::PushSubscription', inverse_of: :access_token + after_commit :push_to_streaming_api end