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

Auth/pm 17129/login with 2fa recovery code #5383

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8d92120
feat(2FA): [PM-17129] Login with 2FA Recovery Code - Initial changes,โ€ฆ
Patrick-Pimentel-Bitwarden Feb 6, 2025
f94f20b
Merge remote-tracking branch 'origin' into auth/pm-17129/login-with-2โ€ฆ
Patrick-Pimentel-Bitwarden Feb 6, 2025
2716dca
feat(2FA): [PM-17129] Login with 2FA Recovery Code - Login with Recovโ€ฆ
Patrick-Pimentel-Bitwarden Feb 7, 2025
8454e6f
feat(2FA): [PM-17129] Login with 2FA Recovery Code - Feature flagged โ€ฆ
Patrick-Pimentel-Bitwarden Feb 7, 2025
020ce0f
feat(2FA): [PM-17129] Login with 2FA Recovery Code - Added the featurโ€ฆ
Patrick-Pimentel-Bitwarden Feb 7, 2025
b966da7
feat(2FA): [PM-17129] Login with 2FA Recovery Code - Changed logic toโ€ฆ
Patrick-Pimentel-Bitwarden Feb 10, 2025
e0b95b5
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Code cleanup.
Patrick-Pimentel-Bitwarden Feb 10, 2025
7bddf6d
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Added back in oldโ€ฆ
Patrick-Pimentel-Bitwarden Feb 10, 2025
e9a0894
test(2FA): [PM-17129] Login with 2FA Recovery Code - Tests.
Patrick-Pimentel-Bitwarden Feb 10, 2025
9bca4c4
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Fixed to be propeโ€ฆ
Patrick-Pimentel-Bitwarden Feb 10, 2025
f69c5e9
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Updated some commโ€ฆ
Patrick-Pimentel-Bitwarden Feb 10, 2025
9602ed2
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Updated a comments.
Patrick-Pimentel-Bitwarden Feb 11, 2025
8035127
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Updated function โ€ฆ
Patrick-Pimentel-Bitwarden Feb 11, 2025
22e6469
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Fixed broken publโ€ฆ
Patrick-Pimentel-Bitwarden Feb 11, 2025
831f111
fix(2FA): [PM-17129] Login with 2FA Recovery Code - Added more verbosโ€ฆ
Patrick-Pimentel-Bitwarden Feb 11, 2025
3fc2c62
refactor(2FA): [PM-17129] Login with 2FA Recovery Code - Initial chanโ€ฆ
Patrick-Pimentel-Bitwarden Feb 12, 2025
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
27 changes: 12 additions & 15 deletions src/Api/Auth/Controllers/TwoFactorController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@

if (user != null)
{
// check if 2FA email is from passwordless
// Check if 2FA email is from Passwordless.
if (!string.IsNullOrEmpty(requestModel.AuthRequestAccessCode))
{
if (await _verifyAuthRequestCommand
Expand All @@ -317,17 +317,14 @@
}
else if (!string.IsNullOrEmpty(requestModel.SsoEmail2FaSessionToken))
{
if (this.ValidateSsoEmail2FaToken(requestModel.SsoEmail2FaSessionToken, user))
if (ValidateSsoEmail2FaToken(requestModel.SsoEmail2FaSessionToken, user))
{
await _userService.SendTwoFactorEmailAsync(user);
return;
}
else
{
await this.ThrowDelayedBadRequestExceptionAsync(
"Cannot send two-factor email: a valid, non-expired SSO Email 2FA Session token is required to send 2FA emails.",
2000);
}

await ThrowDelayedBadRequestExceptionAsync(
"Cannot send two-factor email: a valid, non-expired SSO Email 2FA Session token is required to send 2FA emails.");

Check warning on line 327 in src/Api/Auth/Controllers/TwoFactorController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Auth/Controllers/TwoFactorController.cs#L326-L327

Added lines #L326 - L327 were not covered by tests
}
else if (await _userService.VerifySecretAsync(user, requestModel.Secret))
{
Expand All @@ -336,8 +333,7 @@
}
}

await this.ThrowDelayedBadRequestExceptionAsync(
"Cannot send two-factor email.", 2000);
await ThrowDelayedBadRequestExceptionAsync("Cannot send two-factor email.");

Check warning on line 336 in src/Api/Auth/Controllers/TwoFactorController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Auth/Controllers/TwoFactorController.cs#L336

Added line #L336 was not covered by tests
}

[HttpPut("email")]
Expand Down Expand Up @@ -374,7 +370,7 @@
public async Task<TwoFactorProviderResponseModel> PutOrganizationDisable(string id,
[FromBody] TwoFactorProviderRequestModel model)
{
var user = await CheckAsync(model, false);
await CheckAsync(model, false);

Check warning on line 373 in src/Api/Auth/Controllers/TwoFactorController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Auth/Controllers/TwoFactorController.cs#L373

Added line #L373 was not covered by tests

var orgIdGuid = new Guid(id);
if (!await _currentContext.ManagePolicies(orgIdGuid))
Expand All @@ -401,6 +397,9 @@
return response;
}

/// <summary>
/// To be removed when the feature flag pm-17128-recovery-code-login is removed PM-18175.
/// </summary>
[HttpPost("recover")]
[AllowAnonymous]
public async Task PostRecover([FromBody] TwoFactorRecoveryRequestModel model)
Expand Down Expand Up @@ -463,10 +462,8 @@
await Task.Delay(2000);
throw new BadRequestException(name, $"{name} is invalid.");
}
else
{
await Task.Delay(500);
}

await Task.Delay(500);

Check warning on line 466 in src/Api/Auth/Controllers/TwoFactorController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Auth/Controllers/TwoFactorController.cs#L466

Added line #L466 was not covered by tests
}

private bool ValidateSsoEmail2FaToken(string ssoEmail2FaSessionToken, User user)
Expand Down
1 change: 1 addition & 0 deletions src/Core/Auth/Enums/TwoFactorProviderType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ public enum TwoFactorProviderType : byte
Remember = 5,
OrganizationDuo = 6,
WebAuthn = 7,
RecoveryCode = 8,
}
43 changes: 43 additions & 0 deletions src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ’ญ : I don't think we need this whole Token Provider implementation since we aren't really generating a token. We just use the TwoFactorProviderType to control how we validate the user.

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
๏ปฟusing Bit.Core.Entities;
using Microsoft.AspNetCore.Identity;

namespace Bit.Core.Auth.Identity.TokenProviders;

/// <summary>
/// We are repurposing the TwoFactorTokenProvider workflow to handle Recovery Codes
/// being sent in because it is the easiest way identified to handle allowing a user
/// to successfully go through the login process while providing their Recovery Code.
///
/// Originally, submitting your recovery code would land you on the login screen,
/// but with the approach of using a TwoFactorTokenProvider we don't have to embed
/// logic jankily in other parts of the recovery code process to get them logged in,
/// we can treat Recovery Codes as just another 2FA method. This means that some of
/// the functionality will not be needed in the same way it's needed for other 2FA
/// methods.
/// </summary>
public class RecoveryCodeTokenProvider : IUserTwoFactorTokenProvider<User>
{
/// <summary>
/// Hijack the can generate two factor token to repurpose it to check
/// if the user has a two factor recovery code on their account.
/// </summary>
public virtual Task<bool> CanGenerateTwoFactorTokenAsync(UserManager<User> manager, User user)
{
return Task.FromResult(!string.IsNullOrEmpty(user.TwoFactorRecoveryCode));
}

/// <summary>
/// This function shouldn't get called because we are not using recovery codes
/// the typical 2FA flow.
/// </summary>
public virtual async Task<string> GenerateAsync(string purpose, UserManager<User> manager, User user)

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build MSSQL migrator utility (win-x64)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build MSSQL migrator utility (osx-x64)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build MSSQL migrator utility (linux-x64)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (MsSqlMigratorUtility, ./util, true)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Identity, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Events, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Billing, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Icons, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Api, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Notifications, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Setup, ./util)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Scim, ./bitwarden_license/src, true)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Sso, ./bitwarden_license/src, true)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (EventsProcessor, ./src)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Quality scan

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Admin, ./src, true)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Run tests

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 33 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View workflow job for this annotation

GitHub Actions / Upload

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
throw new Exception("This should not have been called.");

Check warning on line 35 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs#L34-L35

Added lines #L34 - L35 were not covered by tests
}

public Task<bool> ValidateAsync(string purpose, string token, UserManager<User> manager, User user)
{
var processedToken = token.Replace(" ", string.Empty).ToLower();
return Task.FromResult(string.Equals(processedToken, user.TwoFactorRecoveryCode));
}

Check warning on line 42 in src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Auth/Identity/TokenProviders/RecoveryCodeTokenProvider.cs#L39-L42

Added lines #L39 - L42 were not covered by tests
}
1 change: 1 addition & 0 deletions src/Core/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ public static class FeatureFlagKeys
public const string EnablePMAuthenticatorSync = "enable-pm-bwa-sync";
public const string P15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal";
public const string AndroidMutualTls = "mutual-tls";
public const string RecoveryCodeLogin = "pm-17128-recovery-code-login";

public static List<string> GetAllKeys()
{
Expand Down
16 changes: 11 additions & 5 deletions src/Core/Services/IUserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public interface IUserService
Task<IdentityResult> CreateUserAsync(User user, string masterPasswordHash);
Task SendMasterPasswordHintAsync(string email);
Task SendTwoFactorEmailAsync(User user);
Task<bool> VerifyTwoFactorEmailAsync(User user, string token);
Task<CredentialCreateOptions> StartWebAuthnRegistrationAsync(User user);
Task<bool> DeleteWebAuthnKeyAsync(User user, int id);
Task<bool> CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse);
Expand All @@ -41,8 +40,6 @@ Task<IdentityResult> ChangeKdfAsync(User user, string masterPassword, string new
Task<IdentityResult> RefreshSecurityStampAsync(User user, string masterPasswordHash);
Task UpdateTwoFactorProviderAsync(User user, TwoFactorProviderType type, bool setEnabled = true, bool logEvent = true);
Task DisableTwoFactorProviderAsync(User user, TwoFactorProviderType type);
Task<bool> RecoverTwoFactorAsync(string email, string masterPassword, string recoveryCode);
Task<string> GenerateUserTokenAsync(User user, string tokenProvider, string purpose);
Task<IdentityResult> DeleteAsync(User user);
Task<IdentityResult> DeleteAsync(User user, string token);
Task SendDeleteConfirmationAsync(string email);
Expand All @@ -55,9 +52,7 @@ Task<Tuple<bool, string>> SignUpPremiumAsync(User user, string paymentToken,
Task CancelPremiumAsync(User user, bool? endOfPeriod = null);
Task ReinstatePremiumAsync(User user);
Task EnablePremiumAsync(Guid userId, DateTime? expirationDate);
Task EnablePremiumAsync(User user, DateTime? expirationDate);
Task DisablePremiumAsync(Guid userId, DateTime? expirationDate);
Task DisablePremiumAsync(User user, DateTime? expirationDate);
Task UpdatePremiumExpirationAsync(Guid userId, DateTime? expirationDate);
Task<UserLicense> GenerateLicenseAsync(User user, SubscriptionInfo subscriptionInfo = null,
int? version = null);
Expand Down Expand Up @@ -91,6 +86,17 @@ Task<IdentityResult> UpdatePasswordHash(User user, string newPassword,

void SetTwoFactorProvider(User user, TwoFactorProviderType type, bool setEnabled = true);

[Obsolete("To be removed when the feature flag pm-17128-recovery-code-login is removed PM-18175.")]
Task<bool> RecoverTwoFactorAsync(string email, string secret, string recoveryCode);

/// <summary>
/// This method removes the two factor providers on a user and regenerates
/// a new recovery code. Also removes policies on a user for when they lose
/// their 2fa status, they need to comply with their organizations policies.
/// </summary>
/// <param name="user">The user to refresh the 2FA and Recovery Code on.</param>
Task RefreshUser2FaAndRecoveryCodeAsync(User user);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ’ก : I think this might be a good candidate for a command since it spans multiple domains: User modification and Organization modification.


/// <summary>
/// Returns true if the user is a legacy user. Legacy users use their master key as their encryption key.
/// We force these users to the web to migrate their encryption scheme.
Expand Down
19 changes: 16 additions & 3 deletions src/Core/Services/Implementations/UserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@
return;
}

var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultProvider, "DeleteAccount");
var token = await GenerateUserTokenAsync(user, TokenOptions.DefaultProvider, "DeleteAccount");

Check warning on line 318 in src/Core/Services/Implementations/UserService.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Services/Implementations/UserService.cs#L318

Added line #L318 was not covered by tests
await _mailService.SendVerifyDeleteEmailAsync(user.Email, user.Id, token);
}

Expand Down Expand Up @@ -868,6 +868,9 @@
}
}

/// <summary>
/// To be removed when the feature flag pm-17128-recovery-code-login is removed PM-18175.
/// </summary>
public async Task<bool> RecoverTwoFactorAsync(string email, string secret, string recoveryCode)
{
var user = await _userRepository.GetByEmailAsync(email);
Expand Down Expand Up @@ -897,6 +900,15 @@
return true;
}

public async Task RefreshUser2FaAndRecoveryCodeAsync(User user)
{
user.TwoFactorProviders = null;
user.TwoFactorRecoveryCode = CoreHelpers.SecureRandomString(32, upper: false, special: false);
await SaveUserAsync(user);
await _eventService.LogUserEventAsync(user.Id, EventType.User_Recovered2fa);
await CheckPoliciesOnTwoFactorRemovalAsync(user);
}

Check warning on line 910 in src/Core/Services/Implementations/UserService.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Services/Implementations/UserService.cs#L904-L910

Added lines #L904 - L910 were not covered by tests

public async Task<Tuple<bool, string>> SignUpPremiumAsync(User user, string paymentToken,
PaymentMethodType paymentMethodType, short additionalStorageGb, UserLicense license,
TaxInfo taxInfo)
Expand Down Expand Up @@ -1081,7 +1093,7 @@
await EnablePremiumAsync(user, expirationDate);
}

public async Task EnablePremiumAsync(User user, DateTime? expirationDate)
private async Task EnablePremiumAsync(User user, DateTime? expirationDate)
{
if (user != null && !user.Premium && user.Gateway.HasValue)
{
Expand All @@ -1098,7 +1110,7 @@
await DisablePremiumAsync(user, expirationDate);
}

public async Task DisablePremiumAsync(User user, DateTime? expirationDate)
private async Task DisablePremiumAsync(User user, DateTime? expirationDate)
{
if (user != null && user.Premium)
{
Expand Down Expand Up @@ -1151,6 +1163,7 @@
{
if (user == null)
{
Logger.LogWarning($"User {user.Id} does not exist when checking password.");

Check warning on line 1166 in src/Core/Services/Implementations/UserService.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Services/Implementations/UserService.cs#L1166

Added line #L1166 was not covered by tests
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public BaseRequestValidator(
protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
CustomValidatorRequestContext validatorContext)
{
// 1. we need to check if the user is a bot and if their master password hash is correct
// 1. We need to check if the user is a bot and if their master password hash is correct.
var isBot = validatorContext.CaptchaResponse?.IsBot ?? false;
var valid = await ValidateContextAsync(context, validatorContext);
var user = validatorContext.User;
Expand All @@ -99,7 +99,7 @@ protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
return;
}

// 2. Does this user belong to an organization that requires SSO
// 2. Decide if this user belong to an organization that requires SSO.
validatorContext.SsoRequired = await RequireSsoLoginAsync(user, request.GrantType);
if (validatorContext.SsoRequired)
{
Expand All @@ -111,17 +111,21 @@ protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
return;
}

// 3. Check if 2FA is required
(validatorContext.TwoFactorRequired, var twoFactorOrganization) = await _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(user, request);
// This flag is used to determine if the user wants a rememberMe token sent when authentication is successful
// 3. Check if 2FA is required.
(validatorContext.TwoFactorRequired, var twoFactorOrganization) =
await _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(user, request);

// This flag is used to determine if the user wants a rememberMe token sent when
// authentication is successful.
var returnRememberMeToken = false;
if (validatorContext.TwoFactorRequired)
{
var twoFactorToken = request.Raw["TwoFactorToken"]?.ToString();
var twoFactorProvider = request.Raw["TwoFactorProvider"]?.ToString();
var twoFactorToken = request.Raw["TwoFactorToken"];
var twoFactorProvider = request.Raw["TwoFactorProvider"];
var validTwoFactorRequest = !string.IsNullOrWhiteSpace(twoFactorToken) &&
!string.IsNullOrWhiteSpace(twoFactorProvider);
// response for 2FA required and not provided state

// 3a. Response for 2FA required and not provided state.
if (!validTwoFactorRequest ||
!Enum.TryParse(twoFactorProvider, out TwoFactorProviderType twoFactorProviderType))
{
Expand All @@ -133,26 +137,27 @@ protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
return;
}

// Include Master Password Policy in 2FA response
resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicy(user));
// Include Master Password Policy in 2FA response.
resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicyAsync(user));
SetTwoFactorResult(context, resultDict);
return;
}

var twoFactorTokenValid = await _twoFactorAuthenticationValidator
.VerifyTwoFactor(user, twoFactorOrganization, twoFactorProviderType, twoFactorToken);
var twoFactorTokenValid =
await _twoFactorAuthenticationValidator
.VerifyTwoFactorAsync(user, twoFactorOrganization, twoFactorProviderType, twoFactorToken);

// response for 2FA required but request is not valid or remember token expired state
// 3b. Response for 2FA required but request is not valid or remember token expired state.
if (!twoFactorTokenValid)
{
// The remember me token has expired
// The remember me token has expired.
if (twoFactorProviderType == TwoFactorProviderType.Remember)
{
var resultDict = await _twoFactorAuthenticationValidator
.BuildTwoFactorResultAsync(user, twoFactorOrganization);

// Include Master Password Policy in 2FA response
resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicy(user));
resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicyAsync(user));
SetTwoFactorResult(context, resultDict);
}
else
Expand All @@ -163,17 +168,30 @@ protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
return;
}

// When the two factor authentication is successful, we can check if the user wants a rememberMe token
var twoFactorRemember = request.Raw["TwoFactorRemember"]?.ToString() == "1";
if (twoFactorRemember // Check if the user wants a rememberMe token
&& twoFactorTokenValid // Make sure two factor authentication was successful
&& twoFactorProviderType != TwoFactorProviderType.Remember) // if the two factor auth was rememberMe do not send another token
// 3c. When the 2FA authentication is successful, we can check if the user wants a
// rememberMe token.
var twoFactorRemember = request.Raw["TwoFactorRemember"] == "1";
if (twoFactorRemember // Check if the user wants a rememberMe token.
&& twoFactorProviderType != TwoFactorProviderType.Remember) // if the 2FA auth was rememberMe do not send another token.
{
returnRememberMeToken = true;
}

// 3d. If the user is logging in using a RecoveryCode type to login
// as their second factor we know we are in the flow where we need
// to disable their other 2FAs just like as if they were using
// their recovery code because they no longer have access to their
// 2FA device.
if (FeatureService.IsEnabled(FeatureFlagKeys.RecoveryCodeLogin))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐ŸŽจ : I think I would like this to live in the TwoFactorAuthenticationValidator. Adding it to the here increases tech debt on the BaseRequestValidator.

I think it should live in the TwoFactorAuthenticationValidator.ValidateTwoFactorAsync() flow.

{
if (twoFactorProviderType == TwoFactorProviderType.RecoveryCode)
{
await _userService.RefreshUser2FaAndRecoveryCodeAsync(user);
}
}
}

// 4. Check if the user is logging in from a new device
// 4. Check if the user is logging in from a new device.
var deviceValid = await _deviceValidator.ValidateRequestDeviceAsync(request, validatorContext);
if (!deviceValid)
{
Expand All @@ -182,7 +200,7 @@ protected async Task ValidateAsync(T context, ValidatedTokenRequest request,
return;
}

// 5. Force legacy users to the web for migration
// 5. Force legacy users to the web for migration.
if (UserService.IsLegacyUser(user) && request.ClientId != "web")
{
await FailAuthForLegacyUserAsync(user, context);
Expand Down Expand Up @@ -223,7 +241,7 @@ protected async Task BuildSuccessResultAsync(User user, T context, Device device
customResponse.Add("Key", user.Key);
}

customResponse.Add("MasterPasswordPolicy", await GetMasterPasswordPolicy(user));
customResponse.Add("MasterPasswordPolicy", await GetMasterPasswordPolicyAsync(user));
customResponse.Add("ForcePasswordReset", user.ForcePasswordReset);
customResponse.Add("ResetMasterPassword", string.IsNullOrWhiteSpace(user.MasterPassword));
customResponse.Add("Kdf", (byte)user.Kdf);
Expand Down Expand Up @@ -402,7 +420,7 @@ private bool ValidateFailedAuthEmailConditions(bool unknownDevice, User user)
return unknownDevice && failedLoginCeiling > 0 && failedLoginCount == failedLoginCeiling;
}

private async Task<MasterPasswordPolicyResponseModel> GetMasterPasswordPolicy(User user)
private async Task<MasterPasswordPolicyResponseModel> GetMasterPasswordPolicyAsync(User user)
{
// Check current context/cache to see if user is in any organizations, avoids extra DB call if not
var orgs = (await CurrentContext.OrganizationMembershipAsync(_organizationUserRepository, user.Id))
Expand Down
Loading
Loading