1. If I'm right it's not thread-safe. I suppose two browsers with the same username could login at the same time which means that the AuthenticateUser method runs parallel on two threads.

Suppose the following:

• the user already has an invalid login attempt,
• two new threads enter AddInvalidLoginToCache the same time,
• invalidLoginThresholdFromConfig = 2.

A possible execution order is the following:

    // InvalidLoginTimestamp.Count = 1, we already have an invalid login attempt
    thread1: enters AddInvalidLoginToCache
    thread2: enters AddInvalidLoginToCache
    thread1: runs invalidLogin.InvalidLoginTimestamp.Add(DateTime.Now); // InvalidLoginTimestamp.Count = 2
    thread2: runs invalidLogin.InvalidLoginTimestamp.Add(DateTime.Now); // InvalidLoginTimestamp.Count = 3
    thread1: runs if (invalidLogin.InvalidLoginTimestamp.Count == 2)) // false 
    thread2: runs if (invalidLogin.InvalidLoginTimestamp.Count == 2)) // false

The result is that a lucky attacker will never be locked out.

The second problem is visibility. Since InvalidLoginTimestamp and invalidLogin are shared between threads you need some synchronization to ensure other threads will see modifications (not just stale data or invalid state).

• C# multi-threading: Acquire read lock necessary?
• How to synchronize the access to a List used in ASP.NET?

private readonly ObjectCache cacheInvalidLogins = MemoryCache.Default; //Cache to hold Invalid Login Attempts: {UserId, obj UserLogin}

I'd put the comment before the line to avoid horizontal scrolling:

    //Cache to hold Invalid Login Attempts: {UserId, obj UserLogin}
    private readonly ObjectCache cacheInvalidLogins = MemoryCache.Default; 

Furthermore, I'd rename it to invalidLoginAttemptsCache. The cacheInvalidLogins name looks like a method name since it starts with a verb (cache...).

1.

bool validCredentials = false;
... // other statements
try
{
    validCredentials = securityService.Authenticate(userName, password);

The boolean could be declared right before the first use with a smaller scope:

    ...  // other statements
    bool validCredentials = false;
    try
    {
        validCredentials = securityService.Authenticate(userName, password);

Should I declare variables as close as possible to the scope where they will be used?

1.

private void AddInvalidLoginToCache(string userName)
{
    ...
    cacheInvalidLogins.Set(userName.ToUpper(), invalidLogin, LockOutCachePolicyFromConfig());

The code calls this method with the uppercased username. I'd rename the parameter to upperCasedUserName and remove the .ToUpper() call.

1. This variable is never user, I guess you could remove it:

string clientId = userName.Split('\\')[0];

1. Username seems to be one word, I'd not capitalize the n.