djaodjin / djaodjin-saas

Django application for software-as-service and subscription businesses
Other
564 stars 124 forks source link

Removing processor values before authorization #135

Closed knivets closed 6 years ago

knivets commented 6 years ago

I'm not sure if this is as simple as this. Let me know if I misunderstood the problem.

knivets commented 6 years ago

Done

smirolo commented 6 years ago

Apologies I did not caught this earlier. The pull request does not solve the issue #135. The pb is that when the following Stripe API is called with the same code, Stripe will disconnect the account and return an error.

resp = requests.post('https://connect.stripe.com/oauth/token', params=data)

When resp.status_code != 200 we raise a stripe.error.AuthenticationError.

  1. In all likelihood, this should be a ProcessorError otherwise we won't catch it in saas.views.BankAuthorizeView.get. Alternatively BankAuthorizeView even though using OAuth seems very Stripe-centric so maybe it should be moved to saas.backends.stripe_processor.views and catch the currently raised stripe.error.AuthenticationError.

  2. As the exception is raised in connect_auth, self.object.save() is not called, hence the "cleared" processor keys are not saved.

smirolo commented 6 years ago

You could put both self.object.save() in a finally statement. There might be subtle issues on the state of the processor keys vs. connected account on Stripe when an unexpected exception is raised. Still PR looks good.