Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pay::Stripe::PaymentMethod.sync attaching to wrong customer in rare (?) conditions #842

Closed
fschwahn opened this issue Aug 28, 2023 · 3 comments

Comments

@fschwahn
Copy link

This is issue is a bit weird, but what happened in our app is this:

--> Payment method is attached to the wrong Pay::Customer


I'm not sure why object.customer was nil for the returned payment method object, this may well have been a temporary Stripe bug, but it still seems dangerous.

Maybe Pay::Stripe::PaymentMethod.sync should return (or raise?) in case object.customer is nil?

@cjilbert504
Copy link
Collaborator

cjilbert504 commented Sep 1, 2023

Hey @fschwahn - The code already does return if the find_by call does not find a Pay::Customer:

pay_customer = Pay::Customer.find_by(processor: :stripe, processor_id: object.customer)
return unless pay_customer

That said, we may need to add an additional check like you mentioned.

Also, I am wondering if it is possible that you actually had a Pay::Customer record in your database that had a processor_id of nil. If so that would be why a seemingly random Pay::Customer record was given a different customers new default payment method.

For example:

irb(main):006:0> pay_customer = Pay::Customer.find_by(processor: :stripe, processor_id: nil)
=> 
#<Pay::Customer:0x000000055555555b8
...
irb(main):007:0> _
=> 
#<Pay::Customer:0x000000055555555b8
 id: 716567020,              
 owner_type: "User",         
 owner_id: 555555555,        
 processor: "stripe",        
 processor_id: nil,          
 default: true,              
 data: nil,                  
 deleted_at: nil,
 created_at: Thu, 17 Aug 2023 00:48:02.303326000 UTC +00:00,
 updated_at: Thu, 17 Aug 2023 00:48:02.303326000 UTC +00:00,
 payment_method_token: nil>
irb(main):008:0> 

@fschwahn
Copy link
Author

fschwahn commented Sep 4, 2023

Also, I am wondering if it is possible that you actually had a Pay::Customer record in your database that had a processor_id of nil

Yes, I forgot to mention that. We have a few of those in our database, because creating the customer in Stripe failed (in our case due to an invalid email address).

I see now that this is quite an edge case, but probably still worth to add the guard?

@excid3
Copy link
Collaborator

excid3 commented Sep 5, 2023

Yeah, this should definitely return if object.customer is nil. Same for other places we do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants