mirror of
https://github.com/bitwarden/server
synced 2026-01-13 22:13:43 +00:00
feat(2fa-webauthn) [PM-20109]: Increase 2FA WebAuthn Security Key Limit (#6751)
* feat(global-settings) [PM-20109]: Add WebAuthN global settings. * feat(webauthn) [PM-20109]: Update maximum allowed WebAuthN credentials to use new settings. * test(webauthn) [PM-20109]: Update command tests to use global configs. * feat(global-settings) [PM-20109]: Set defaults for maximum allowed credentials. * feat(two-factor-request-model) [PM-20109]: Remove hard-coded 5 limit on ID validation. * Revert "test(webauthn) [PM-20109]: Update command tests to use global configs." This reverts commitba9f0d5fb6. * Revert "feat(webauthn) [PM-20109]: Update maximum allowed WebAuthN credentials to use new settings." This reverts commitd2faef0c13. * feat(global-settings) [PM-20109]: Add WebAuthNSettings to interface for User Service consumption. * feat(user-service) [PM-20109]: Add boundary and persistence-time validation for maximum allowed WebAuthN 2FA credentials. * test(user-service) [PM-20109]: Update tests for WebAuthN limit scenarios. * refactor(user-service) [PM-20109]: Typo in variable name. * refactor(user-service) [PM-20109]: Remove unnecessary pending check. * refactor(user-service) [PM-20109]: Pending check is necessary. * refactor(webauthn) [PM-20109]: Re-spell WebAuthN => WebAuthn. * refactor(user-service) [PM-20109]: Re-format pending checks for consistency. * refactor(user-service) [PM-20109]: Fix type spelling in comments. * test(user-service) [PM-20109]: Combine premium and non-premium test cases with AutoData. * refactor(user-service) [PM-20109]: Swap HasPremiumAccessQuery in for CanAccessPremium. * refactor(user-service) [PM-20109]: Convert limit check to positive, edit comments.
This commit is contained in:
@@ -273,7 +273,7 @@ public class TwoFactorWebAuthnDeleteRequestModel : SecretVerificationRequestMode
|
||||
yield return validationResult;
|
||||
}
|
||||
|
||||
if (!Id.HasValue || Id < 0 || Id > 5)
|
||||
if (!Id.HasValue)
|
||||
{
|
||||
yield return new ValidationResult("Invalid Key Id", new string[] { nameof(Id) });
|
||||
}
|
||||
|
||||
@@ -344,6 +344,12 @@ public class UserService : UserManager<User>, IUserService
|
||||
await _mailService.SendMasterPasswordHintEmailAsync(email, user.MasterPasswordHint);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Initiates WebAuthn 2FA credential registration and generates a challenge for adding a new security key.
|
||||
/// </summary>
|
||||
/// <param name="user">The current user.</param>
|
||||
/// <returns></returns>
|
||||
/// <exception cref="BadRequestException">Maximum allowed number of credentials already registered.</exception>
|
||||
public async Task<CredentialCreateOptions> StartWebAuthnRegistrationAsync(User user)
|
||||
{
|
||||
var providers = user.GetTwoFactorProviders();
|
||||
@@ -364,6 +370,17 @@ public class UserService : UserManager<User>, IUserService
|
||||
provider.MetaData = new Dictionary<string, object>();
|
||||
}
|
||||
|
||||
// Boundary validation to provide a better UX. There is also second-level enforcement at persistence time.
|
||||
var maximumAllowedCredentialCount = await _hasPremiumAccessQuery.HasPremiumAccessAsync(user.Id)
|
||||
? _globalSettings.WebAuthn.PremiumMaximumAllowedCredentials
|
||||
: _globalSettings.WebAuthn.NonPremiumMaximumAllowedCredentials;
|
||||
// Count only saved credentials ("Key{id}") toward the limit.
|
||||
if (provider.MetaData.Count(k => k.Key.StartsWith("Key")) >=
|
||||
maximumAllowedCredentialCount)
|
||||
{
|
||||
throw new BadRequestException("Maximum allowed WebAuthn credential count exceeded.");
|
||||
}
|
||||
|
||||
var fidoUser = new Fido2User
|
||||
{
|
||||
DisplayName = user.Name,
|
||||
@@ -402,6 +419,17 @@ public class UserService : UserManager<User>, IUserService
|
||||
return false;
|
||||
}
|
||||
|
||||
// Persistence-time validation for comprehensive enforcement. There is also boundary validation for best-possible UX.
|
||||
var maximumAllowedCredentialCount = await _hasPremiumAccessQuery.HasPremiumAccessAsync(user.Id)
|
||||
? _globalSettings.WebAuthn.PremiumMaximumAllowedCredentials
|
||||
: _globalSettings.WebAuthn.NonPremiumMaximumAllowedCredentials;
|
||||
// Count only saved credentials ("Key{id}") toward the limit.
|
||||
if (provider.MetaData.Count(k => k.Key.StartsWith("Key")) >=
|
||||
maximumAllowedCredentialCount)
|
||||
{
|
||||
throw new BadRequestException("Maximum allowed WebAuthn credential count exceeded.");
|
||||
}
|
||||
|
||||
var options = CredentialCreateOptions.FromJson((string)pendingValue);
|
||||
|
||||
// Callback to ensure credential ID is unique. Always return true since we don't care if another
|
||||
|
||||
@@ -66,6 +66,7 @@ public class GlobalSettings : IGlobalSettings
|
||||
public virtual NotificationHubPoolSettings NotificationHubPool { get; set; } = new();
|
||||
public virtual YubicoSettings Yubico { get; set; } = new YubicoSettings();
|
||||
public virtual DuoSettings Duo { get; set; } = new DuoSettings();
|
||||
public virtual WebAuthnSettings WebAuthn { get; set; } = new WebAuthnSettings();
|
||||
public virtual BraintreeSettings Braintree { get; set; } = new BraintreeSettings();
|
||||
public virtual ImportCiphersLimitationSettings ImportCiphersLimitation { get; set; } = new ImportCiphersLimitationSettings();
|
||||
public virtual BitPaySettings BitPay { get; set; } = new BitPaySettings();
|
||||
@@ -613,6 +614,12 @@ public class GlobalSettings : IGlobalSettings
|
||||
public string AKey { get; set; }
|
||||
}
|
||||
|
||||
public class WebAuthnSettings
|
||||
{
|
||||
public int PremiumMaximumAllowedCredentials { get; set; } = 10;
|
||||
public int NonPremiumMaximumAllowedCredentials { get; set; } = 5;
|
||||
}
|
||||
|
||||
public class BraintreeSettings
|
||||
{
|
||||
public bool Production { get; set; }
|
||||
|
||||
@@ -28,4 +28,5 @@ public interface IGlobalSettings
|
||||
string DevelopmentDirectory { get; set; }
|
||||
IWebPushSettings WebPush { get; set; }
|
||||
GlobalSettings.EventLoggingSettings EventLogging { get; set; }
|
||||
GlobalSettings.WebAuthnSettings WebAuthn { get; set; }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user