AuthorizeNet / sdk-dotnet

.Net SDK for Authorize.Net API
Other
149 stars 200 forks source link

Concurrency and multi-thread issues #283

Open mexner opened 4 years ago

mexner commented 4 years ago

The MerchantAuthentication approach provided in the documentation is not thread safe and leads to concurrency issues. This has lead to huge consequences and reliability, where we are seeing payments being processed into the wrong merchant account (because the Transaction ID and Key) are being overwritten when multiple threads are accessing this static method at the same time.

I suspect the abstract class with static method approach works for 98% of the public because they are always accessing the same gateway account (Transaction ID and Key). But we have 100's of customers each with their own Transaction ID and Key, processing hundreds of transactions.

I see there is an overload method for each request object which allows us to push the MerchantAuthentication object in. This solves the concurrency issues.

I opened this issue - so that the documentation can be updated to notate, this is NOT THREAD SAFE,

ApiOperationBase<ANetApiRequest, ANetApiResponse>.MerchantAuthentication = new merchantAuthenticationType()
{
  name = ApiLoginID,
  ItemElementName = ItemChoiceType.transactionKey,
  Item = ApiTransactionKey,
};

And that the overload constructor for each request (ie: createTransactionRequest) allows to pass the MerchantAuthentication object in real-time, and fix this issue.

merchantAuthenticationType myMAT = new merchantAuthenticationType {
    name = "API Login",
    ItemElementName = ItemChoiceType.transactionKey,
    Item = "Transaction Key"
};

createTransactionRequest myRequest = new createTransactionRequest { transactionRequest = myTR, merchantAuthentication = myMAT };

I just didn't see any documentation pointing to this issue and want to save other users the headaches this has caused us and our customers. https://community.developer.authorize.net/t5/Integration-and-Testing/Multi-Tennant-concurrency-issues/td-p/72341

hunkydoryrepair commented 2 years ago

Thanks for posting this. We have multi-tenants and I just found we have that exact code. Thankfully most of our tenants are using other gateways, but this would be a serious problem

hunkydoryrepair commented 2 years ago

I supposed RunEnvironment has the same issue. Unfortunately that doesn't seem to be configurable on the individual request, so we can't have some tenants running in sandbox mode.

zeroClearAmerican commented 2 years ago

Thank you for this. Major problem for multi-client systems. Not a great look for a leading payment processor 😬

manson commented 4 days ago

I supposed RunEnvironment has the same issue. Unfortunately that doesn't seem to be configurable on the individual request, so we can't have some tenants running in sandbox mode.

You can use controller.Execute(Environment); I.e. pass environment during execution running.