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

Patches 2281 #2314

Merged
merged 2 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions services/skus/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ func deleteItemCreds(svc *Service) handlers.AppHandler {

isSigned := r.URL.Query().Get("isSigned") == "true"

if err := svc.DeleteOrderCreds(ctx, *orderID.UUID(), reqID.UUID(), isSigned); err != nil {
if err := svc.DeleteOrderCreds(ctx, *orderID.UUID(), *reqID.UUID(), isSigned); err != nil {
lg.Error().Err(err).Msg("failed to delete the order credentials")
return handlers.WrapError(err, "Error deleting credentials", http.StatusBadRequest)
}
Expand Down Expand Up @@ -735,7 +735,7 @@ func DeleteOrderCreds(service *Service) handlers.AppHandler {
}

isSigned := r.URL.Query().Get("isSigned") == "true"
if err := service.DeleteOrderCreds(ctx, id, nil, isSigned); err != nil {
if err := service.DeleteOrderCreds(ctx, id, uuid.Nil, isSigned); err != nil {
return handlers.WrapError(err, "Error deleting credentials", http.StatusBadRequest)
}

Expand Down
33 changes: 18 additions & 15 deletions services/skus/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ func (s *SigningOrderResultErrorHandler) Handle(ctx context.Context, message kaf
// - create repos for credentials;
// - move the corresponding methods there;
// - make those methods work on per-item basis.
func (s *Service) DeleteOrderCreds(ctx context.Context, orderID uuid.UUID, reqID *uuid.UUID, isSigned bool) error {
func (s *Service) DeleteOrderCreds(ctx context.Context, orderID uuid.UUID, reqID uuid.UUID, isSigned bool) error {
order, err := s.Datastore.GetOrder(orderID)
if err != nil {
return err
Expand Down Expand Up @@ -720,20 +720,7 @@ func (s *Service) DeleteOrderCreds(ctx context.Context, orderID uuid.UUID, reqID
}

if doTlv2 {
var itemIDs []*uuid.UUID
if reqID == nil {
// legacy, so put all of the orders' items in a list to be hard deleted
for _, v := range order.Items {
itemIDs = append(itemIDs, &v.ID)
}
} else {
// there is a request id, pass it along to the delete order creds as an "item id"
// this will allow for legacy credentials using the request id of the order's item id
// to be deleted, otherwise we do not delete said credentials for multiple device support
itemIDs = append(itemIDs, reqID)
}

if err := s.Datastore.DeleteTimeLimitedV2OrderCredsByOrderTx(ctx, tx, orderID, itemIDs...); err != nil {
if err := s.deleteTLV2(ctx, tx, order, reqID); err != nil {
return fmt.Errorf("error deleting time limited v2 order creds: %w", err)
}
}
Expand All @@ -749,6 +736,22 @@ func (s *Service) DeleteOrderCreds(ctx context.Context, orderID uuid.UUID, reqID
return nil
}

func (s *Service) deleteTLV2(ctx context.Context, dbi sqlx.ExtContext, order *model.Order, reqID uuid.UUID) error {
// Pass the request id as an "item id", which will allow for legacy credentials to be deleted.
// Otherwise, do not delete said credentials for multiple device support.
if !uuid.Equal(reqID, uuid.Nil) {
return s.Datastore.DeleteTimeLimitedV2OrderCredsByOrderTx(ctx, dbi, order.ID, reqID)
}

itemIDs := make([]uuid.UUID, 0, len(order.Items))
// Legacy, delete all items.
for i := range order.Items {
itemIDs = append(itemIDs, order.Items[i].ID)
}

return s.Datastore.DeleteTimeLimitedV2OrderCredsByOrderTx(ctx, dbi, order.ID, itemIDs...)
}

// checkNumBlindedCreds checks the number of submitted blinded credentials.
//
// The number of submitted credentials must not exceed:
Expand Down
100 changes: 100 additions & 0 deletions services/skus/credentials_noint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package skus

import (
"context"
"testing"

"github.com/golang/mock/gomock"
uuid "github.com/satori/go.uuid"
should "github.com/stretchr/testify/assert"
must "github.com/stretchr/testify/require"

"github.com/brave-intl/bat-go/services/skus/model"
)

func TestService_DeleteTLV2(t *testing.T) {
type tcGiven struct {
ord *model.Order
reqID uuid.UUID
}

type tcExpected struct {
args []gomock.Matcher
err error
}

type testCase struct {
name string
given tcGiven
exp tcExpected
}

tests := []testCase{
{
name: "request_id_specified",
given: tcGiven{
ord: &model.Order{ID: uuid.Must(uuid.FromString("a6b72f11-c886-49ee-b4f4-913eaa0984ae"))},
reqID: uuid.Must(uuid.FromString("d10cf2ae-30d8-4ada-965c-11c582968f26")),
},
exp: tcExpected{
args: []gomock.Matcher{
gomock.Eq(context.Background()),
gomock.Nil(), // dbi
gomock.Eq(uuid.Must(uuid.FromString("a6b72f11-c886-49ee-b4f4-913eaa0984ae"))),
gomock.Eq([]uuid.UUID{uuid.Must(uuid.FromString("d10cf2ae-30d8-4ada-965c-11c582968f26"))}),
},
},
},

{
name: "request_id_nil",
given: tcGiven{
ord: &model.Order{
ID: uuid.Must(uuid.FromString("d7b4f524-ebe3-48c1-b60d-d6f548b25aae")),
Items: []model.OrderItem{
{ID: uuid.Must(uuid.FromString("3882ec99-73fb-476b-b176-1f4b40a9b767"))},
{ID: uuid.Must(uuid.FromString("ed437d36-182b-460f-8213-2ce3d4bb5c93"))},
{ID: uuid.Must(uuid.FromString("d3e62075-996f-4bed-bbc7-f6cd324b83e0"))},
},
},
reqID: uuid.Nil,
},
exp: tcExpected{
args: []gomock.Matcher{
gomock.Eq(context.Background()),
gomock.Nil(), // dbi
gomock.Eq(uuid.Must(uuid.FromString("d7b4f524-ebe3-48c1-b60d-d6f548b25aae"))),
gomock.Eq([]uuid.UUID{
uuid.Must(uuid.FromString("3882ec99-73fb-476b-b176-1f4b40a9b767")),
uuid.Must(uuid.FromString("ed437d36-182b-460f-8213-2ce3d4bb5c93")),
uuid.Must(uuid.FromString("d3e62075-996f-4bed-bbc7-f6cd324b83e0")),
}),
},
},
},
}

for i := range tests {
tc := tests[i]

t.Run(tc.name, func(t *testing.T) {
must.Equal(t, 4, len(tc.exp.args))

ctrl := gomock.NewController(t)
defer ctrl.Finish()

ds := NewMockDatastore(ctrl)

svc := &Service{Datastore: ds}

ctx := context.Background()

ds.EXPECT().DeleteTimeLimitedV2OrderCredsByOrderTx(
tc.exp.args[0], tc.exp.args[1], tc.exp.args[2], tc.exp.args[3],
).Return(nil)

actual := svc.deleteTLV2(ctx, nil, tc.given.ord, tc.given.reqID)
should.Equal(t, nil, actual)
})
}
}
17 changes: 9 additions & 8 deletions services/skus/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type Datastore interface {
AreTimeLimitedV2CredsSubmitted(ctx context.Context, blindedCreds ...string) (bool, error)
GetTimeLimitedV2OrderCredsByOrder(orderID uuid.UUID) (*TimeLimitedV2Creds, error)
GetTLV2Creds(ctx context.Context, dbi sqlx.QueryerContext, ordID, itemID, reqID uuid.UUID) (*TimeLimitedV2Creds, error)
DeleteTimeLimitedV2OrderCredsByOrderTx(ctx context.Context, tx *sqlx.Tx, orderID uuid.UUID, itemIDs ...*uuid.UUID) error
DeleteTimeLimitedV2OrderCredsByOrderTx(ctx context.Context, dbi sqlx.ExtContext, orderID uuid.UUID, itemIDs ...uuid.UUID) error
GetTimeLimitedV2OrderCredsByOrderItem(itemID uuid.UUID) (*TimeLimitedV2Creds, error)
InsertTimeLimitedV2OrderCredsTx(ctx context.Context, tx *sqlx.Tx, tlv2 TimeAwareSubIssuedCreds) error
InsertSigningOrderRequestOutbox(ctx context.Context, requestID uuid.UUID, orderID uuid.UUID, itemID uuid.UUID, signingOrderRequest SigningOrderRequest) error
Expand Down Expand Up @@ -796,19 +796,20 @@ func (pg *Postgres) DeleteSingleUseOrderCredsByOrderTx(ctx context.Context, tx *

// DeleteTimeLimitedV2OrderCredsByOrderTx performs a hard delete for all time limited v2 order
// credentials for a given OrderID.
func (pg *Postgres) DeleteTimeLimitedV2OrderCredsByOrderTx(ctx context.Context, tx *sqlx.Tx, orderID uuid.UUID, itemIDs ...*uuid.UUID) error {
// legacy, if the request id matches the item id on the time limited v2 creds, we actually want to delete the record,
// otherwise we will keep these credentials here for multiple device refresh capabilities
query, args, err := sqlx.In("delete from time_limited_v2_order_creds where order_id = ? and item_id in (?)", orderID, itemIDs)
func (pg *Postgres) DeleteTimeLimitedV2OrderCredsByOrderTx(ctx context.Context, dbi sqlx.ExtContext, orderID uuid.UUID, itemIDs ...uuid.UUID) error {
// Legacy, if the request id matches the item id on the tlv2 creds, we actually want to delete the record.
// Otherwise we will keep these credentials here for multiple device refresh capabilities.
const q = "DELETE FROM time_limited_v2_order_creds WHERE order_id = ? AND item_id in (?)"
query, args, err := sqlx.In(q, orderID, itemIDs)
if err != nil {
return fmt.Errorf("error creating delete query for order with item ids: %w", err)
}

query = tx.Rebind(query)
_, err = tx.ExecContext(ctx, query, args...)
if err != nil {
query = dbi.Rebind(query)
if _, err := dbi.ExecContext(ctx, query, args...); err != nil {
return fmt.Errorf("error deleting time limited v2 order creds: %w", err)
}

return nil
}

Expand Down
4 changes: 2 additions & 2 deletions services/skus/instrumented_datastore.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions services/skus/mockdatastore.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion services/skus/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,5 +198,5 @@ func (s *Service) RenewOrder(ctx context.Context, orderID uuid.UUID) error {
return fmt.Errorf("failed to set order status to paid: %w", err)
}

return s.DeleteOrderCreds(ctx, orderID, nil, true)
return s.DeleteOrderCreds(ctx, orderID, uuid.Nil, true)
}