Avoid fast post on webapi c#

Question

I have problem in when user post the data. Some times the post run so fast and this make problem in my website.

The user want to register a form about 100$ and have 120$ balance.

When the post (save) button pressed sometimes two post come to server very fast like:

2018-01-31 19:34:43.660 Register Form 5760$

2018-01-31 19:34:43.663 Register Form 5760$

Therefore my client balance become negative.

I use If in my code to check balance but the code run many fast and I think both if happen together and I missed them.

Therefore I made Lock Controll class to avoid concurrency per user but not work well.

I made global Action Filter to control the users this is my code:

public void OnActionExecuting(ActionExecutingContext context)
{
    try
    {
        var controller = (Controller)context.Controller;
        if (controller.User.Identity.IsAuthenticated)
        {
            bool jobDone = false;
            int delay = 0;
            int counter = 0;
            do
            {
                delay = LockControllers.IsRequested(controller.User.Identity.Name);
                if (delay == 0)
                {
                    LockControllers.AddUser(controller.User.Identity.Name);
                    jobDone = true;
                }
                else
                {
                    counter++;
                    System.Threading.Thread.Sleep(delay);
                }
                if (counter >= 10000)
                {
                    context.HttpContext.Response.StatusCode = 400;
                    jobDone = true;
                    context.Result = new ContentResult()
                    {
                        Content = "Attack Detected"
                    };
                }
            } while (!jobDone);

        }
    }
    catch (System.Exception)
    {
    }
}

public void OnActionExecuted(ActionExecutedContext context)
{
    try
    {
        var controller = (Controller)context.Controller;
        if (controller.User.Identity.IsAuthenticated)
        {
            LockControllers.RemoveUser(controller.User.Identity.Name);
        }
    }
    catch (System.Exception)
    {
    }   
}

I made list static list of user and sleep their thread until previous task happen.

Is there any better way to manage this problem?

Answer 1

So the original question has been edited so this answer is invalid.

so the issue isn't that the code runs too fast. Fast is always good :) The issue is that the account is going into negative funds. If the client decides to post a form twice that is the clients fault. It maybe that you only want the client to pay only once which is an other problem.

So for the first problem, I would recommend a using transactions (https://en.wikipedia.org/wiki/Database_transaction) to lock your table. Which means that the add update/add a change (or set of changes) and you force other calls to that table to wait until those operations have been done. You can always begin your transaction and check that the account has the correct amount of funds.

If the case is that they are only ever meant to pay once then.. then have a separate table that records if the user has payed (again within a transaction), before processing the update/add.

http://www.entityframeworktutorial.net/entityframework6/transaction-in-entity-framework.aspx

(Edit: fixing link)

Answer 2

You have a few options here

1. You implement ETag functionality in your app which you can use for optimistic concurrency. This works well, when you are working with records, i.e. you have a database with a data record, return that to the user and then the user changes it.

2. You could add an required field with a guid to your view model which you pass to your app and add it to in memory cache and check it on each request.

   public class RegisterViewModel
   {
       [Required]
       public Guid Id { get; set; }
   
       /* other properties here */
       ...
   }
   

   and then use IMemoryCache or IDistributedMemoryCache (see ASP.NET Core Docs) to put this Id into the memory cache and validate it on request

   public Task<IActioNResult> Register(RegisterViewModel register)
   {
       if(!ModelState.IsValid)
           return BadRequest(ModelState);
   
       var userId = ...; /* get userId */
       if(_cache.TryGetValue($"Registration-{userId}", register.Id))
       {
           return BadRequest(new { ErrorMessage = "Command already recieved by this user" });
       }
   
       // Set cache options.
       var cacheEntryOptions = new MemoryCacheEntryOptions()
           // Keep in cache for 5 minutes, reset time if accessed.
           .SetSlidingExpiration(TimeSpan.FromMinutes(5));
   
       // when we're here, the command wasn't executed before, so we save the key in the cache
       _cache.Set($"Registration-{userId}", register.Id, cacheEntryOptions );
   
       // call your service here to process it
       registrationService.Register(...);
   }
   

   When the second request arrives, the value will already be in the (distributed) memory cache and the operation will fail.

   If the caller do not sets the Id, validation will fail.

Of course all that Jonathan Hickey listed in his answer below applies to, you should always validate that there is enough balance and use EF-Cores optimistic or pessimistic concurrency