Fix race condition in POST /api/v1/push/subscription (#30166)
				
					
				
			This commit is contained in:
		@@ -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
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user