Skip to content

Commit

Permalink
VCST-1628: Make login time the same for registered and unregistered u…
Browse files Browse the repository at this point in the history
…sers (#2839)
  • Loading branch information
artem-dudarev authored Sep 25, 2024
1 parent d9408cc commit 95bb09d
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 29 deletions.
49 changes: 49 additions & 0 deletions src/VirtoCommerce.Platform.Core/Security/DelayedResponse.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Threading.Tasks;

namespace VirtoCommerce.Platform.Core.Security;

// This class allows to measure the duration of a succeeded response and delay subsequent failed responses to prevent timing attacks.
public class DelayedResponse
{
private const int _minDelay = 150; // milliseconds
private static readonly ConcurrentDictionary<string, int> _delaysByName = new();

private readonly string _name;
private readonly Stopwatch _stopwatch;
private readonly Task _failedDelayTask;
private readonly Task _succeededDelayTask;

public static DelayedResponse Create(params string[] nameParts)
{
return new DelayedResponse(string.Join(".", nameParts));
}

public DelayedResponse(string name)
{
_name = name;
_stopwatch = Stopwatch.StartNew();
_delaysByName.TryAdd(name, 0);
var failedDelay = Math.Max(_minDelay, _delaysByName[name]);
_failedDelayTask = Task.Delay(failedDelay);
_succeededDelayTask = Task.Delay(_minDelay);
}

public Task FailAsync()
{
return _failedDelayTask;
}

public Task SucceedAsync()
{
if (_stopwatch.IsRunning)
{
_stopwatch.Stop();
_delaysByName[_name] = (int)_stopwatch.ElapsedMilliseconds;
}

return _succeededDelayTask;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ public async Task<ActionResult> Exchange()

if (openIdConnectRequest.IsPasswordGrantType())
{
// Measure the duration of a succeeded response and delay subsequent failed responses to prevent timing attacks
var delayedResponse = DelayedResponse.Create(nameof(AuthorizationController), nameof(Exchange), "Password");

var user = await _userManager.FindByNameAsync(openIdConnectRequest.Username);

// Allows signin to back office by either username (login) or email if IdentityOptions.User.RequireUniqueEmail is True.
Expand All @@ -127,11 +130,13 @@ public async Task<ActionResult> Exchange()

if (user is null)
{
await delayedResponse.FailAsync();
return BadRequest(SecurityErrorDescriber.LoginFailed());
}

if (!_passwordLoginOptions.Enabled && !user.IsAdministrator)
{
await delayedResponse.FailAsync();
return BadRequest(SecurityErrorDescriber.PasswordLoginDisabled());
}

Expand All @@ -144,6 +149,7 @@ public async Task<ActionResult> Exchange()
var errors = await requestValidator.ValidateAsync(context);
if (errors.Count > 0)
{
await delayedResponse.FailAsync();
return BadRequest(errors.First());
}
}
Expand All @@ -156,6 +162,8 @@ public async Task<ActionResult> Exchange()
await SetLastLoginDate(user);
await _eventPublisher.Publish(new UserLoginEvent(user));

await delayedResponse.SucceedAsync();

return SignIn(ticket.Principal, ticket.Properties, ticket.AuthenticationScheme);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,41 +97,44 @@ public SecurityController(
[AllowAnonymous]
public async Task<ActionResult<SignInResult>> Login([FromBody] LoginRequest request)
{
var userName = request.UserName;
// Measure the duration of a succeeded response and delay subsequent failed responses to prevent timing attacks
var delayedResponse = DelayedResponse.Create(nameof(SecurityController), nameof(Login));

var user = await UserManager.FindByNameAsync(request.UserName);

// Allows signin to back office by either username (login) or email if IdentityOptions.User.RequireUniqueEmail is True.
if (_identityOptions.User.RequireUniqueEmail)
if (user == null && _identityOptions.User.RequireUniqueEmail)
{
var userByName = await UserManager.FindByNameAsync(userName);

if (userByName == null)
{
var userByEmail = await UserManager.FindByEmailAsync(userName);
if (userByEmail != null)
{
userName = userByEmail.UserName;
}
}
user = await UserManager.FindByEmailAsync(request.UserName);
}

var user = await UserManager.FindByNameAsync(userName);
if (user == null)
{
await delayedResponse.FailAsync();
return Ok(SignInResult.Failed);
}

await _eventPublisher.Publish(new BeforeUserLoginEvent(user));

var loginResult = await _signInManager.PasswordSignInAsync(userName, request.Password, request.RememberMe, true);
var loginResult = await _signInManager.PasswordSignInAsync(user, request.Password, request.RememberMe, lockoutOnFailure: true);

if (loginResult.Succeeded)
if (!loginResult.Succeeded)
{
await SetLastLoginDate(user);
await _eventPublisher.Publish(new UserLoginEvent(user));
await delayedResponse.FailAsync();
return Ok(loginResult);
}

//Do not allow login to admin customers and rejected users
if (await UserManager.IsInRoleAsync(user, PlatformConstants.Security.SystemRoles.Customer))
{
return Ok(SignInResult.NotAllowed);
}
await SetLastLoginDate(user);
await _eventPublisher.Publish(new UserLoginEvent(user));

//Do not allow login to admin customers and rejected users
if (await UserManager.IsInRoleAsync(user, PlatformConstants.Security.SystemRoles.Customer))
{
loginResult = SignInResult.NotAllowed;
}

await delayedResponse.SucceedAsync();

return Ok(loginResult);
}

Expand Down Expand Up @@ -611,25 +614,28 @@ public async Task<ActionResult<bool>> ValidatePasswordResetToken(string userId,
[AllowAnonymous]
public async Task<ActionResult> RequestPasswordReset(string loginOrEmail)
{
// Measure the duration of a succeeded response and delay subsequent failed responses to prevent timing attacks
var delayedResponse = DelayedResponse.Create(nameof(SecurityController), nameof(RequestPasswordReset));

var user = await UserManager.FindByNameAsync(loginOrEmail);
if (user == null)

if (user == null && _identityOptions.User.RequireUniqueEmail)
{
user = await UserManager.FindByEmailAsync(loginOrEmail);
}

// Return 200 to prevent potential user name/email harvesting
if (user == null)
{
await delayedResponse.FailAsync();
return Ok();
}

var nextRequestDate = user.LastPasswordChangeRequestDate + _passwordOptions.RepeatedResetPasswordTimeLimit;
if (nextRequestDate != null && nextRequestDate > DateTime.UtcNow)
{
return Ok(new
{
NextRequestAt = nextRequestDate,
});
await delayedResponse.FailAsync();
return Ok(new { NextRequestAt = nextRequestDate });
}

//Do not permit rejected users and customers
Expand All @@ -652,6 +658,8 @@ public async Task<ActionResult> RequestPasswordReset(string loginOrEmail)
await UserManager.UpdateAsync(user);
}

await delayedResponse.SucceedAsync();

return Ok();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public async Task Login_SignInSuccess()
var user = _fixture.Create<ApplicationUser>();
var request = _fixture.Create<LoginRequest>();
_signInManagerMock
.Setup(x => x.PasswordSignInAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>()))
.Setup(x => x.PasswordSignInAsync(It.IsAny<ApplicationUser>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>()))
.ReturnsAsync(SignInResult.Success);
_userManagerMock
.Setup(x => x.FindByNameAsync(It.IsAny<string>()))
Expand All @@ -168,7 +168,7 @@ public async Task Login_UserRoleIsCustomer()
var user = _fixture.Create<ApplicationUser>();
var request = _fixture.Create<LoginRequest>();
_signInManagerMock
.Setup(x => x.PasswordSignInAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>()))
.Setup(x => x.PasswordSignInAsync(It.IsAny<ApplicationUser>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>()))
.ReturnsAsync(SignInResult.Success);
_userManagerMock
.Setup(x => x.FindByNameAsync(It.IsAny<string>()))
Expand Down

0 comments on commit 95bb09d

Please sign in to comment.