open-saas icon indicating copy to clipboard operation
open-saas copied to clipboard

Stripe integration and test fails when using default auth

Open vxlm opened this issue 1 year ago • 1 comments

Currently, the call to the function stripePayment checks for the following:

  if (!context.user || !context.user.email) {
    throw new HttpError(401);
  }

However, the check for email is not made clear, and the default authentication method does not let a user add an email. This means that the default authentication will never work with stripe integration as-is.

In my opinion, there are a few ways to resolve this:

  1. Remove this check for email altogether. I'm new to stripe in general, and so I'm unsure if this is appropriate, or if users expect an email with a stripe call. The integration works fine if we remove this check.
  2. Provide an environment variable to modify this behavior - additionally, 403 might be a more accurate response, and we can describe the reason for easier debugging. Update the documentation along with this change.
  3. Generate an email when account is created using default auth method, e.g username "test" would be associated with "[email protected]"

I can open a PR to fix this, but would like confirmation before going ahead and implementing solution 2

vxlm avatar Feb 01 '24 10:02 vxlm

I really like these ideas, @vxlm!

Regarding the usernameAndPassword authentication method, it's written in the docs that it's only to be used to kick start the template. So in that sense, we don't really expect users to be testing Stripe checkout with it.

I would suggest that we don't remove the check for email, since it seems necessary for many parts of the checkout process, but rather we update the error that's being thrown (403 and throws a specific error when there is no email informing the Dev that this auth method is not suitable for stripe checkout), as you mentioned, as well as the docs to let users know what WON'T work with this Auth method.

If you'd like to open a PR that modifies the error to that would be great! Let me know if that works and if you have any other Qs!

vincanger avatar Feb 05 '24 14:02 vincanger

fixed in recent commit

vincanger avatar Feb 15 '24 14:02 vincanger