Skip to content

Commit a37723e

Browse files
pavelbrmpavelbrm
authored
Merge pull request #2724 from brave-intl/master
Production 2024-12-10_01
2 parents 35b5916 + e74a0c6 commit a37723e

10 files changed

+285
-38
lines changed

‎services/skus/controllers.go

+20-16
Original file line numberDiff line numberDiff line change
@@ -847,47 +847,51 @@ type VoteRequest struct {
847847
// MakeVote is the handler for making a vote using credentials
848848
func MakeVote(service *Service) handlers.AppHandler {
849849
return handlers.AppHandler(func(w http.ResponseWriter, r *http.Request) *handlers.AppError {
850-
var (
851-
req VoteRequest
852-
ctx = r.Context()
853-
)
854-
err := requestutils.ReadJSON(ctx, r.Body, &req)
855-
if err != nil {
850+
ctx := r.Context()
851+
852+
req := VoteRequest{}
853+
if err := requestutils.ReadJSON(ctx, r.Body, &req); err != nil {
856854
return handlers.WrapError(err, "Error in request body", http.StatusBadRequest)
857855
}
858856

859-
logger := logging.Logger(ctx, "skus.MakeVote")
860-
861-
_, err = govalidator.ValidateStruct(req)
862-
if err != nil {
857+
if _, err := govalidator.ValidateStruct(req); err != nil {
863858
return handlers.WrapValidationError(err)
864859
}
865860

866-
err = service.Vote(ctx, req.Credentials, req.Vote)
867-
if err != nil {
861+
lg := logging.Logger(ctx, "skus").With().Str("func", "MakeVote").Logger()
862+
863+
if err := service.Vote(ctx, req.Credentials, req.Vote); err != nil {
868864
switch err.(type) {
869865
case govalidator.Error:
870-
logger.Warn().Err(err).Msg("failed vote validation")
866+
lg.Warn().Err(err).Msg("failed vote validation")
871867
return handlers.WrapValidationError(err)
868+
872869
case govalidator.Errors:
873-
logger.Warn().Err(err).Msg("failed multiple vote validation")
870+
lg.Warn().Err(err).Msg("failed multiple vote validation")
874871
return handlers.WrapValidationError(err)
872+
875873
default:
876874
// check for custom vote invalidations
877875
if errors.Is(err, ErrInvalidSKUToken) {
878876
verr := handlers.ValidationError("failed to validate sku token", nil)
877+
879878
data := []string{}
880879
if errors.Is(err, ErrInvalidSKUTokenSKU) {
881880
data = append(data, "invalid sku value")
882881
}
882+
883883
if errors.Is(err, ErrInvalidSKUTokenBadMerchant) {
884884
data = append(data, "invalid merchant value")
885885
}
886+
886887
verr.Data = data
887-
logger.Warn().Err(err).Msg("failed sku validations")
888+
lg.Warn().Err(err).Msg("failed sku validations")
889+
888890
return verr
889891
}
890-
logger.Warn().Err(err).Msg("failed to perform vote")
892+
893+
lg.Warn().Err(err).Msg("failed to perform vote")
894+
891895
return handlers.WrapError(err, "Error making vote", http.StatusBadRequest)
892896
}
893897
}

‎services/skus/credentials.go

+4
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,10 @@ var generateCredentialRedemptions = func(ctx context.Context, cb []CredentialBin
520520
}
521521
}
522522

523+
if issuer == nil {
524+
return nil, model.ErrIssuerNotFound
525+
}
526+
523527
requestCredentials[i].Issuer = issuer.Name()
524528
requestCredentials[i].TokenPreimage = cb[i].TokenPreimage
525529
requestCredentials[i].Signature = cb[i].Signature

‎services/skus/model/model.go

+1
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ type OrderItemRequest struct {
413413
// CreateOrderRequestNew includes information needed to create an order.
414414
type CreateOrderRequestNew struct {
415415
Email string `json:"email" validate:"required,email"`
416+
CustomerID string `json:"customer_id"` // Optional.
416417
Currency string `json:"currency" validate:"required,iso4217"`
417418
StripeMetadata *OrderStripeMetadata `json:"stripe_metadata"`
418419
RadomMetadata *OrderRadomMetadata `json:"radom_metadata"`

‎services/skus/service.go

+31-4
Original file line numberDiff line numberDiff line change
@@ -2021,6 +2021,7 @@ func (s *Service) createStripeSession(ctx context.Context, req *model.CreateOrde
20212021
sreq := createStripeSessionRequest{
20222022
orderID: oid,
20232023
email: req.Email,
2024+
customerID: req.CustomerID,
20242025
successURL: surl,
20252026
cancelURL: curl,
20262027
trialDays: order.GetTrialDays(),
@@ -2105,7 +2106,17 @@ func (s *Service) redeemBlindedCred(ctx context.Context, w http.ResponseWriter,
21052106
// FIXME: we shouldn't be using the issuer as the payload, it ideally would be a unique request identifier
21062107
// to allow for more flexible idempotent behavior.
21072108
if err := redeemFn(ctx, cred.Issuer, cred.TokenPreimage, cred.Signature, cred.Issuer); err != nil {
2108-
return handleRedeemFnError(ctx, w, kind, cred, err)
2109+
if !shouldRetryRedeemFn(kind, cred.Issuer, err) {
2110+
return handleRedeemFnError(ctx, w, kind, cred, err)
2111+
}
2112+
2113+
// TODO: remove this as there should be no credentials in Production signed by brave-leo-premium-year.
2114+
//
2115+
// Fix for https://github.com/brave-intl/challenge-bypass-server/pull/371.
2116+
const leoa = "brave.com?sku=brave-leo-premium-year"
2117+
if err := redeemFn(ctx, leoa, cred.TokenPreimage, cred.Signature, cred.Issuer); err != nil {
2118+
return handleRedeemFnError(ctx, w, kind, cred, err)
2119+
}
21092120
}
21102121

21112122
// TODO(clD11): cleanup after quick fix
@@ -2572,6 +2583,7 @@ func (s *Service) recreateStripeSession(ctx context.Context, dbi sqlx.ExecerCont
25722583
req := createStripeSessionRequest{
25732584
orderID: ord.ID.String(),
25742585
email: email,
2586+
customerID: xstripe.CustomerIDFromSession(oldSess),
25752587
successURL: oldSess.SuccessURL,
25762588
cancelURL: oldSess.CancelURL,
25772589
trialDays: ord.GetTrialDays(),
@@ -2798,6 +2810,7 @@ func chooseStripeSessID(ord *model.Order, canBeNewSessID string) (string, bool)
27982810
type createStripeSessionRequest struct {
27992811
orderID string
28002812
email string
2813+
customerID string
28012814
successURL string
28022815
cancelURL string
28032816
trialDays int64
@@ -2815,9 +2828,17 @@ func createStripeSession(ctx context.Context, cl stripeClient, req createStripeS
28152828
LineItems: req.items,
28162829
}
28172830

2818-
// Email might not be given.
2819-
// This could happen while recreating a session, and the email was not extracted from the old one.
2820-
if req.email != "" {
2831+
// Different processes can supply different info about customer:
2832+
// - when customerID is present, it takes precedence;
2833+
// - when email is present:
2834+
// - first, search for customer;
2835+
// - fallback to using the email directly.
2836+
// Based on the rules above, if both are present, customerID wins.
2837+
switch {
2838+
case req.customerID != "":
2839+
params.Customer = &req.customerID
2840+
2841+
case req.customerID == "" && req.email != "":
28212842
if cust, ok := cl.FindCustomer(ctx, req.email); ok && cust.Email != "" {
28222843
params.Customer = &cust.ID
28232844
} else {
@@ -2876,6 +2897,12 @@ func handleRedeemFnError(ctx context.Context, w http.ResponseWriter, kind string
28762897
return handlers.WrapError(err, "Error verifying credentials", http.StatusInternalServerError)
28772898
}
28782899

2900+
func shouldRetryRedeemFn(kind, issuer string, err error) bool {
2901+
const leo = "brave.com?sku=brave-leo-premium"
2902+
2903+
return kind == timeLimitedV2 && issuer == leo && err.Error() == cbr.ErrBadRequest.Error()
2904+
}
2905+
28792906
func newRadomGateway(env string) (*radom.Gateway, error) {
28802907
switch env {
28812908
case "development", "staging":

‎services/skus/service_nonint_test.go

+156-7
Original file line numberDiff line numberDiff line change
@@ -4388,7 +4388,7 @@ func TestService_recreateStripeSession(t *testing.T) {
43884388
},
43894389

43904390
{
4391-
name: "success_email_from_session",
4391+
name: "success_email_cust_from_session",
43924392
given: tcGiven{
43934393
ordRepo: &repository.MockOrder{
43944394
FnAppendMetadata: func(ctx context.Context, dbi sqlx.ExecerContext, id uuid.UUID, key, val string) error {
@@ -4405,7 +4405,7 @@ func TestService_recreateStripeSession(t *testing.T) {
44054405
ID: "cs_test_id_old",
44064406
SuccessURL: "https://example.com/success",
44074407
CancelURL: "https://example.com/cancel",
4408-
Customer: &stripe.Customer{Email: "[email protected]"},
4408+
Customer: &stripe.Customer{ID: "cus_id", Email: "[email protected]"},
44094409
}
44104410

44114411
return result, nil
@@ -4455,6 +4455,79 @@ func TestService_recreateStripeSession(t *testing.T) {
44554455
},
44564456
},
44574457

4458+
{
4459+
name: "success_email_from_request_cust_without_email",
4460+
given: tcGiven{
4461+
ordRepo: &repository.MockOrder{
4462+
FnAppendMetadata: func(ctx context.Context, dbi sqlx.ExecerContext, id uuid.UUID, key, val string) error {
4463+
if key == "stripeCheckoutSessionId" && val == "cs_test_id" {
4464+
return nil
4465+
}
4466+
4467+
return model.Error("unexpected")
4468+
},
4469+
},
4470+
cl: &xstripe.MockClient{
4471+
FnSession: func(ctx context.Context, id string, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) {
4472+
result := &stripe.CheckoutSession{
4473+
ID: "cs_test_id_old",
4474+
SuccessURL: "https://example.com/success",
4475+
CancelURL: "https://example.com/cancel",
4476+
Customer: &stripe.Customer{ID: "cus_id"},
4477+
}
4478+
4479+
return result, nil
4480+
},
4481+
4482+
FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) {
4483+
return nil, false
4484+
},
4485+
4486+
FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) {
4487+
if params.Customer != nil {
4488+
return nil, model.Error("unexpected_customer")
4489+
}
4490+
4491+
if *params.CustomerEmail != "[email protected]" {
4492+
return nil, model.Error("unexpected_customer_email")
4493+
}
4494+
4495+
result := &stripe.CheckoutSession{
4496+
ID: "cs_test_id",
4497+
PaymentMethodTypes: []string{"card"},
4498+
Mode: stripe.CheckoutSessionModeSubscription,
4499+
SuccessURL: *params.SuccessURL,
4500+
CancelURL: *params.CancelURL,
4501+
ClientReferenceID: *params.ClientReferenceID,
4502+
Subscription: &stripe.Subscription{
4503+
ID: "sub_id",
4504+
Metadata: map[string]string{
4505+
"orderID": *params.ClientReferenceID,
4506+
},
4507+
},
4508+
AllowPromotionCodes: true,
4509+
}
4510+
4511+
return result, nil
4512+
},
4513+
},
4514+
ord: &model.Order{
4515+
ID: uuid.Must(uuid.FromString("facade00-0000-4000-a000-000000000000")),
4516+
Items: []model.OrderItem{
4517+
{
4518+
Quantity: 1,
4519+
Metadata: datastore.Metadata{"stripe_item_id": "stripe_item_id"},
4520+
},
4521+
},
4522+
},
4523+
oldSessID: "cs_test_id_old",
4524+
email: "[email protected]",
4525+
},
4526+
exp: tcExpected{
4527+
val: "cs_test_id",
4528+
},
4529+
},
4530+
44584531
{
44594532
name: "success_email_from_request",
44604533
given: tcGiven{
@@ -4473,7 +4546,6 @@ func TestService_recreateStripeSession(t *testing.T) {
44734546
ID: "cs_test_id_old",
44744547
SuccessURL: "https://example.com/success",
44754548
CancelURL: "https://example.com/cancel",
4476-
Customer: &stripe.Customer{Email: "[email protected]"},
44774549
}
44784550

44794551
return result, nil
@@ -4564,7 +4636,84 @@ func TestCreateStripeSession(t *testing.T) {
45644636

45654637
tests := []testCase{
45664638
{
4567-
name: "success_found_customer",
4639+
name: "success_cust_id",
4640+
given: tcGiven{
4641+
cl: &xstripe.MockClient{
4642+
FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) {
4643+
if params.Customer == nil || *params.Customer != "cus_id" {
4644+
return nil, model.Error("unexpected")
4645+
}
4646+
4647+
result := &stripe.CheckoutSession{ID: "cs_test_id"}
4648+
4649+
return result, nil
4650+
},
4651+
4652+
FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) {
4653+
panic("unexpected_find_customer")
4654+
},
4655+
},
4656+
4657+
req: createStripeSessionRequest{
4658+
orderID: "facade00-0000-4000-a000-000000000000",
4659+
customerID: "cus_id",
4660+
successURL: "https://example.com/success",
4661+
cancelURL: "https://example.com/cancel",
4662+
trialDays: 7,
4663+
items: []*stripe.CheckoutSessionLineItemParams{
4664+
{
4665+
Quantity: ptrTo[int64](1),
4666+
Price: ptrTo("stripe_item_id"),
4667+
},
4668+
},
4669+
},
4670+
},
4671+
exp: tcExpected{
4672+
val: "cs_test_id",
4673+
},
4674+
},
4675+
4676+
{
4677+
name: "success_cust_id_email",
4678+
given: tcGiven{
4679+
cl: &xstripe.MockClient{
4680+
FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) {
4681+
if params.Customer == nil || *params.Customer != "cus_id" {
4682+
return nil, model.Error("unexpected")
4683+
}
4684+
4685+
result := &stripe.CheckoutSession{ID: "cs_test_id"}
4686+
4687+
return result, nil
4688+
},
4689+
4690+
FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) {
4691+
panic("unexpected_find_customer")
4692+
},
4693+
},
4694+
4695+
req: createStripeSessionRequest{
4696+
orderID: "facade00-0000-4000-a000-000000000000",
4697+
customerID: "cus_id",
4698+
email: "[email protected]",
4699+
successURL: "https://example.com/success",
4700+
cancelURL: "https://example.com/cancel",
4701+
trialDays: 7,
4702+
items: []*stripe.CheckoutSessionLineItemParams{
4703+
{
4704+
Quantity: ptrTo[int64](1),
4705+
Price: ptrTo("stripe_item_id"),
4706+
},
4707+
},
4708+
},
4709+
},
4710+
exp: tcExpected{
4711+
val: "cs_test_id",
4712+
},
4713+
},
4714+
4715+
{
4716+
name: "success_email_found_customer",
45684717
given: tcGiven{
45694718
cl: &xstripe.MockClient{
45704719
FnCreateSession: func(ctx context.Context, params *stripe.CheckoutSessionParams) (*stripe.CheckoutSession, error) {
@@ -4598,7 +4747,7 @@ func TestCreateStripeSession(t *testing.T) {
45984747
},
45994748

46004749
{
4601-
name: "success_customer_not_found",
4750+
name: "success_email_customer_not_found",
46024751
given: tcGiven{
46034752
cl: &xstripe.MockClient{
46044753
FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) {
@@ -4636,7 +4785,7 @@ func TestCreateStripeSession(t *testing.T) {
46364785
},
46374786

46384787
{
4639-
name: "success_no_customer_email",
4788+
name: "success_email_no_customer_email",
46404789
given: tcGiven{
46414790
cl: &xstripe.MockClient{
46424791
FnFindCustomer: func(ctx context.Context, email string) (*stripe.Customer, bool) {
@@ -4663,7 +4812,7 @@ func TestCreateStripeSession(t *testing.T) {
46634812
},
46644813

46654814
{
4666-
name: "success_no_trial_days",
4815+
name: "success_email_no_trial_days",
46674816
given: tcGiven{
46684817
cl: &xstripe.MockClient{},
46694818

0 commit comments

Comments
 (0)