8000 fix(pool): only allow internal accounts to be added to pool by paul-nicolas · Pull Request #428 · formancehq/payments · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(pool): only allow internal accounts to be added to pool #428

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 30, 2025
Merged

fix(pool): only allow internal accounts to be added to pool #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 32 additions & 0 deletions internal/connectors/engine/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"github.com/pkg/errors"
"go.temporal.io/api/enums/v1"
"go.temporal.io/sdk/client"
"golang.org/x/sync/errgroup"
)

//go:generate mockgen -source engine.go -destination engine_generated.go -package engine . Engine
Expand Down Expand Up @@ -823,6 +824,26 @@
ctx, span := otel.Tracer().Start(ctx, "engine.CreatePool")
defer span.End()

eg, groupCtx := errgroup.WithContext(ctx)
for _, accountID := range pool.PoolAccounts {
aID := accountID
eg.Go(func() error {
acc, err := e.storage.AccountsGet(groupCtx, aID)
if err != nil {
return err
}

Check warning on line 834 in internal/connectors/engine/engine.go

View check run for this annotation

Codecov / codecov/patch

internal/connectors/engine/engine.go#L833-L834 

Added lines #L833 - L834 were not covered by tests
if acc.Type != models.ACCOUNT_TYPE_INTERNAL {
return fmt.Errorf("account %s is not an internal account: %w", aID, ErrValidation)
}
return nil
})
}

if err := eg.Wait(); err != nil {
otel.RecordError(span, err)
return err
}

if err := e.storage.PoolsUpsert(ctx, pool); err != nil {
otel.RecordError(span, err)
return err
Expand Down Expand Up @@ -857,6 +878,17 @@
ctx, span := otel.Tracer().Start(ctx, "engine.AddAccountToPool")
defer span.End()

// Check if the account exists and if it's an INTERNAL account
account, err := e.storage.AccountsGet(ctx, accountID)
if err != nil {
otel.RecordError(span, err)
return err
}

if account.Type != models.ACCOUNT_TYPE_INTERNAL {
return fmt.Errorf("account %s is not an internal account: %w", accountID, ErrValidation)
}

if err := e.storage.PoolsAddAccount(ctx, id, accountID); err != nil {
otel.RecordError(span, err)
return err
Expand Down
128 changes: 128 additions & 0 deletions internal/connectors/engine/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,132 @@ var _ = Describe("Engine Tests", func() {
Expect(task.Status).To(Equal(models.TASK_STATUS_PROCESSING))
})
})

Context("create pool", func() {
var (
poolID uuid.UUID
acc1 models.AccountID
acc2 models.AccountID
)
BeforeEach(func() {
poolID = uuid.New()
acc1 = models.AccountID{
Reference: "test",
ConnectorID: models.ConnectorID{},
}
acc2 = models.AccountID{
Reference: "test",
ConnectorID: models.ConnectorID{},
}
})

It("should return error when pool creation fails", func(ctx SpecContext) {
expectedErr := fmt.Errorf("pool creation failed")
store.EXPECT().PoolsUpsert(gomock.Any(), gomock.Any()).Return(expectedErr)
err := eng.CreatePool(ctx, models.Pool{
ID: poolID,
})
Expect(err).ToNot(BeNil())
Expect(err).To(MatchError(expectedErr))
})

It("should return a validation error when one of the accounts is not an INTERNAL one", func(ctx SpecContext) {
store.EXPECT().AccountsGet(gomock.Any(), acc1).Return(&models.Account{
ID: acc1,
Type: models.ACCOUNT_TYPE_INTERNAL,
}, nil)
store.EXPECT().AccountsGet(gomock.Any(), acc2).Return(&models.Account{
ID: acc2,
Type: models.ACCOUNT_TYPE_EXTERNAL,
}, nil)
err := eng.CreatePool(ctx, models.Pool{
ID: poolID,
PoolAccounts: []models.AccountID{acc1, acc2},
})
Expect(err).ToNot(BeNil())
Expect(err).To(MatchError(engine.ErrValidation))
})

It("should work if the pool is created successfully", func(ctx SpecContext) {
store.EXPECT().AccountsGet(gomock.Any(), acc1).Return(&models.Account{
ID: acc1,
Type: models.ACCOUNT_TYPE_INTERNAL,
}, nil)
store.EXPECT().AccountsGet(gomock.Any(), acc2).Return(&models.Account{
ID: acc2,
Type: models.ACCOUNT_TYPE_INTERNAL,
}, nil)
store.EXPECT().PoolsUpsert(gomock.Any(), gomock.Any()).Return(nil)
cl.EXPECT().ExecuteWorkflow(gomock.Any(), WithWorkflowOptions("pools-creation", defaultTaskQueue),
workflow.RunSendEvents,
gomock.AssignableToTypeOf(workflow.SendEvents{}),
).Return(nil, nil)
err := eng.CreatePool(ctx, models.Pool{
ID: poolID,
PoolAccounts: []models.AccountID{acc1, acc2},
})
Expect(err).To(BeNil())
})
})

Context("add account to pool", func() {
var (
poolID uuid.UUID
accountID models.AccountID
)

BeforeEach(func() {
poolID = uuid.New()
accountID = models.AccountID{
Reference: "test",
ConnectorID: models.ConnectorID{
Reference: uuid.New(),
Provider: "test",
},
}
})

It("should return a storage error if account is not found", func(ctx SpecContext) {
store.EXPECT().AccountsGet(gomock.Any(), accountID).Return(nil, storage.ErrNotFound)
err := eng.AddAccountToPool(ctx, poolID, accountID)
Expect(err).ToNot(BeNil())
Expect(err).To(MatchError(storage.ErrNotFound))
})

It("should return a validation error if account is not an internal account", func(ctx SpecContext) {
store.EXPECT().AccountsGet(gomock.Any(), accountID).Return(&models.Account{
ID: accountID,
Type: models.ACCOUNT_TYPE_EXTERNAL,
}, nil)
err := eng.AddAccountToPool(ctx, poolID, accountID)
Expect(err).ToNot(BeNil())
Expect(err).To(MatchError(engine.ErrValidation))
})

It("should return an error if the account is failing to be added to the pool", func(ctx SpecContext) {
store.EXPECT().AccountsGet(gomock.Any(), accountID).Return(&models.Account{
ID: accountID,
Type: models.ACCOUNT_TYPE_INTERNAL,
}, nil)
store.EXPECT().PoolsAddAccount(gomock.Any(), poolID, accountID).Return(fmt.Errorf("failed to add account to pool"))
err := eng.AddAccountToPool(ctx, poolID, accountID)
Expect(err).ToNot(BeNil())
Expect(err).To(MatchError("failed to add account to pool"))
})

It("should work if the account is successfully added to the pool", func(ctx SpecContext) {
store.EXPECT().AccountsGet(gomock.Any(), accountID).Return(&models.Account{
ID: accountID,
Type: models.ACCOUNT_TYPE_INTERNAL,
}, nil)
store.EXPECT().PoolsAddAccount(gomock.Any(), poolID, accountID).Return(nil)
store.EXPECT().PoolsGet(gomock.Any(), poolID).Return(&models.Pool{}, nil)
cl.EXPECT().ExecuteWorkflow(gomock.Any(), WithWorkflowOptions("pools-add-account", defaultTaskQueue),
workflow.RunSendEvents,
gomock.AssignableToTypeOf(workflow.SendEvents{}),
).Return(nil, nil)
err := eng.AddAccountToPool(ctx, poolID, accountID)
Expect(err).To(BeNil())
})
})
})
Loading
0