Skip to content
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.

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

VCST-1628: Make login time the same for registered and unregistered users #2839

Merged
merged 7 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading