mirror of
https://github.com/bitwarden/server.git
synced 2025-07-25 19:41:19 -05:00
[PM-8107] Remove Duo v2 from server (#4934)
refactor(TwoFactorAuthentication): Remove references to old Duo SDK version 2 code and replace them with the Duo SDK version 4 supported library DuoUniversal code. Increased unit test coverage in the Two Factor Authentication code space. We opted to use DI instead of Inheritance for the Duo and OrganizaitonDuo two factor tokens to increase testability, since creating a testing mock of the Duo.Client was non-trivial. Reviewed-by: @JaredSnider-Bitwarden
This commit is contained in:
@ -4,15 +4,14 @@ using Bit.Api.Auth.Models.Response.TwoFactor;
|
||||
using Bit.Api.Models.Request;
|
||||
using Bit.Api.Models.Response;
|
||||
using Bit.Core.Auth.Enums;
|
||||
using Bit.Core.Auth.Identity.TokenProviders;
|
||||
using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces;
|
||||
using Bit.Core.Auth.Models.Business.Tokenables;
|
||||
using Bit.Core.Auth.Utilities;
|
||||
using Bit.Core.Context;
|
||||
using Bit.Core.Entities;
|
||||
using Bit.Core.Exceptions;
|
||||
using Bit.Core.Repositories;
|
||||
using Bit.Core.Services;
|
||||
using Bit.Core.Settings;
|
||||
using Bit.Core.Tokens;
|
||||
using Bit.Core.Utilities;
|
||||
using Fido2NetLib;
|
||||
@ -29,11 +28,10 @@ public class TwoFactorController : Controller
|
||||
private readonly IUserService _userService;
|
||||
private readonly IOrganizationRepository _organizationRepository;
|
||||
private readonly IOrganizationService _organizationService;
|
||||
private readonly GlobalSettings _globalSettings;
|
||||
private readonly UserManager<User> _userManager;
|
||||
private readonly ICurrentContext _currentContext;
|
||||
private readonly IVerifyAuthRequestCommand _verifyAuthRequestCommand;
|
||||
private readonly IFeatureService _featureService;
|
||||
private readonly IDuoUniversalTokenService _duoUniversalTokenService;
|
||||
private readonly IDataProtectorTokenFactory<TwoFactorAuthenticatorUserVerificationTokenable> _twoFactorAuthenticatorDataProtector;
|
||||
private readonly IDataProtectorTokenFactory<SsoEmail2faSessionTokenable> _ssoEmailTwoFactorSessionDataProtector;
|
||||
|
||||
@ -41,22 +39,20 @@ public class TwoFactorController : Controller
|
||||
IUserService userService,
|
||||
IOrganizationRepository organizationRepository,
|
||||
IOrganizationService organizationService,
|
||||
GlobalSettings globalSettings,
|
||||
UserManager<User> userManager,
|
||||
ICurrentContext currentContext,
|
||||
IVerifyAuthRequestCommand verifyAuthRequestCommand,
|
||||
IFeatureService featureService,
|
||||
IDuoUniversalTokenService duoUniversalConfigService,
|
||||
IDataProtectorTokenFactory<TwoFactorAuthenticatorUserVerificationTokenable> twoFactorAuthenticatorDataProtector,
|
||||
IDataProtectorTokenFactory<SsoEmail2faSessionTokenable> ssoEmailTwoFactorSessionDataProtector)
|
||||
{
|
||||
_userService = userService;
|
||||
_organizationRepository = organizationRepository;
|
||||
_organizationService = organizationService;
|
||||
_globalSettings = globalSettings;
|
||||
_userManager = userManager;
|
||||
_currentContext = currentContext;
|
||||
_verifyAuthRequestCommand = verifyAuthRequestCommand;
|
||||
_featureService = featureService;
|
||||
_duoUniversalTokenService = duoUniversalConfigService;
|
||||
_twoFactorAuthenticatorDataProtector = twoFactorAuthenticatorDataProtector;
|
||||
_ssoEmailTwoFactorSessionDataProtector = ssoEmailTwoFactorSessionDataProtector;
|
||||
}
|
||||
@ -184,21 +180,7 @@ public class TwoFactorController : Controller
|
||||
public async Task<TwoFactorDuoResponseModel> PutDuo([FromBody] UpdateTwoFactorDuoRequestModel model)
|
||||
{
|
||||
var user = await CheckAsync(model, true);
|
||||
try
|
||||
{
|
||||
// for backwards compatibility - will be removed with PM-8107
|
||||
DuoApi duoApi = null;
|
||||
if (model.ClientId != null && model.ClientSecret != null)
|
||||
{
|
||||
duoApi = new DuoApi(model.ClientId, model.ClientSecret, model.Host);
|
||||
}
|
||||
else
|
||||
{
|
||||
duoApi = new DuoApi(model.IntegrationKey, model.SecretKey, model.Host);
|
||||
}
|
||||
await duoApi.JSONApiCall("GET", "/auth/v2/check");
|
||||
}
|
||||
catch (DuoException)
|
||||
if (!await _duoUniversalTokenService.ValidateDuoConfiguration(model.ClientSecret, model.ClientId, model.Host))
|
||||
{
|
||||
throw new BadRequestException(
|
||||
"Duo configuration settings are not valid. Please re-check the Duo Admin panel.");
|
||||
@ -241,21 +223,7 @@ public class TwoFactorController : Controller
|
||||
}
|
||||
|
||||
var organization = await _organizationRepository.GetByIdAsync(orgIdGuid) ?? throw new NotFoundException();
|
||||
try
|
||||
{
|
||||
// for backwards compatibility - will be removed with PM-8107
|
||||
DuoApi duoApi = null;
|
||||
if (model.ClientId != null && model.ClientSecret != null)
|
||||
{
|
||||
duoApi = new DuoApi(model.ClientId, model.ClientSecret, model.Host);
|
||||
}
|
||||
else
|
||||
{
|
||||
duoApi = new DuoApi(model.IntegrationKey, model.SecretKey, model.Host);
|
||||
}
|
||||
await duoApi.JSONApiCall("GET", "/auth/v2/check");
|
||||
}
|
||||
catch (DuoException)
|
||||
if (!await _duoUniversalTokenService.ValidateDuoConfiguration(model.ClientId, model.ClientSecret, model.Host))
|
||||
{
|
||||
throw new BadRequestException(
|
||||
"Duo configuration settings are not valid. Please re-check the Duo Admin panel.");
|
||||
|
@ -2,8 +2,8 @@
|
||||
using Bit.Api.Auth.Models.Request.Accounts;
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.Auth.Enums;
|
||||
using Bit.Core.Auth.Identity.TokenProviders;
|
||||
using Bit.Core.Auth.Models;
|
||||
using Bit.Core.Auth.Utilities;
|
||||
using Bit.Core.Entities;
|
||||
using Fido2NetLib;
|
||||
|
||||
@ -43,21 +43,16 @@ public class UpdateTwoFactorAuthenticatorRequestModel : SecretVerificationReques
|
||||
public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IValidatableObject
|
||||
{
|
||||
/*
|
||||
To support both v2 and v4 we need to remove the required annotation from the properties.
|
||||
todo - the required annotation will be added back in PM-8107.
|
||||
String lengths based on Duo's documentation
|
||||
https://github.com/duosecurity/duo_universal_csharp/blob/main/DuoUniversal/Client.cs
|
||||
*/
|
||||
[StringLength(50)]
|
||||
public string ClientId { get; set; }
|
||||
[StringLength(50)]
|
||||
public string ClientSecret { get; set; }
|
||||
//todo - will remove SKey and IKey with PM-8107
|
||||
[StringLength(50)]
|
||||
public string IntegrationKey { get; set; }
|
||||
//todo - will remove SKey and IKey with PM-8107
|
||||
[StringLength(50)]
|
||||
public string SecretKey { get; set; }
|
||||
[Required]
|
||||
[StringLength(50)]
|
||||
[StringLength(20, MinimumLength = 20, ErrorMessage = "Client Id must be exactly 20 characters.")]
|
||||
public string ClientId { get; set; }
|
||||
[Required]
|
||||
[StringLength(40, MinimumLength = 40, ErrorMessage = "Client Secret must be exactly 40 characters.")]
|
||||
public string ClientSecret { get; set; }
|
||||
[Required]
|
||||
public string Host { get; set; }
|
||||
|
||||
public User ToUser(User existingUser)
|
||||
@ -65,22 +60,17 @@ public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IV
|
||||
var providers = existingUser.GetTwoFactorProviders();
|
||||
if (providers == null)
|
||||
{
|
||||
providers = new Dictionary<TwoFactorProviderType, TwoFactorProvider>();
|
||||
providers = [];
|
||||
}
|
||||
else if (providers.ContainsKey(TwoFactorProviderType.Duo))
|
||||
{
|
||||
providers.Remove(TwoFactorProviderType.Duo);
|
||||
}
|
||||
|
||||
Temporary_SyncDuoParams();
|
||||
|
||||
providers.Add(TwoFactorProviderType.Duo, new TwoFactorProvider
|
||||
{
|
||||
MetaData = new Dictionary<string, object>
|
||||
{
|
||||
//todo - will remove SKey and IKey with PM-8107
|
||||
["SKey"] = SecretKey,
|
||||
["IKey"] = IntegrationKey,
|
||||
["ClientSecret"] = ClientSecret,
|
||||
["ClientId"] = ClientId,
|
||||
["Host"] = Host
|
||||
@ -96,22 +86,17 @@ public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IV
|
||||
var providers = existingOrg.GetTwoFactorProviders();
|
||||
if (providers == null)
|
||||
{
|
||||
providers = new Dictionary<TwoFactorProviderType, TwoFactorProvider>();
|
||||
providers = [];
|
||||
}
|
||||
else if (providers.ContainsKey(TwoFactorProviderType.OrganizationDuo))
|
||||
{
|
||||
providers.Remove(TwoFactorProviderType.OrganizationDuo);
|
||||
}
|
||||
|
||||
Temporary_SyncDuoParams();
|
||||
|
||||
providers.Add(TwoFactorProviderType.OrganizationDuo, new TwoFactorProvider
|
||||
{
|
||||
MetaData = new Dictionary<string, object>
|
||||
{
|
||||
//todo - will remove SKey and IKey with PM-8107
|
||||
["SKey"] = SecretKey,
|
||||
["IKey"] = IntegrationKey,
|
||||
["ClientSecret"] = ClientSecret,
|
||||
["ClientId"] = ClientId,
|
||||
["Host"] = Host
|
||||
@ -124,34 +109,22 @@ public class UpdateTwoFactorDuoRequestModel : SecretVerificationRequestModel, IV
|
||||
|
||||
public override IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
|
||||
{
|
||||
if (!DuoApi.ValidHost(Host))
|
||||
var results = new List<ValidationResult>();
|
||||
if (string.IsNullOrWhiteSpace(ClientId))
|
||||
{
|
||||
yield return new ValidationResult("Host is invalid.", [nameof(Host)]);
|
||||
results.Add(new ValidationResult("ClientId is required.", [nameof(ClientId)]));
|
||||
}
|
||||
if (string.IsNullOrWhiteSpace(ClientSecret) && string.IsNullOrWhiteSpace(ClientId) &&
|
||||
string.IsNullOrWhiteSpace(SecretKey) && string.IsNullOrWhiteSpace(IntegrationKey))
|
||||
{
|
||||
yield return new ValidationResult("Neither v2 or v4 values are valid.", [nameof(IntegrationKey), nameof(SecretKey), nameof(ClientSecret), nameof(ClientId)]);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
use this method to ensure that both v2 params and v4 params are in sync
|
||||
todo will be removed in pm-8107
|
||||
*/
|
||||
private void Temporary_SyncDuoParams()
|
||||
{
|
||||
// Even if IKey and SKey exist prioritize v4 params ClientId and ClientSecret
|
||||
if (!string.IsNullOrWhiteSpace(ClientSecret) && !string.IsNullOrWhiteSpace(ClientId))
|
||||
if (string.IsNullOrWhiteSpace(ClientSecret))
|
||||
{
|
||||
SecretKey = ClientSecret;
|
||||
IntegrationKey = ClientId;
|
||||
results.Add(new ValidationResult("ClientSecret is required.", [nameof(ClientSecret)]));
|
||||
}
|
||||
else if (!string.IsNullOrWhiteSpace(SecretKey) && !string.IsNullOrWhiteSpace(IntegrationKey))
|
||||
|
||||
if (string.IsNullOrWhiteSpace(Host) || !DuoUniversalTokenService.ValidDuoHost(Host))
|
||||
{
|
||||
ClientSecret = SecretKey;
|
||||
ClientId = IntegrationKey;
|
||||
results.Add(new ValidationResult("Host is invalid.", [nameof(Host)]));
|
||||
}
|
||||
return results;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -13,37 +13,26 @@ public class TwoFactorDuoResponseModel : ResponseModel
|
||||
public TwoFactorDuoResponseModel(User user)
|
||||
: base(ResponseObj)
|
||||
{
|
||||
if (user == null)
|
||||
{
|
||||
throw new ArgumentNullException(nameof(user));
|
||||
}
|
||||
ArgumentNullException.ThrowIfNull(user);
|
||||
|
||||
var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo);
|
||||
Build(provider);
|
||||
}
|
||||
|
||||
public TwoFactorDuoResponseModel(Organization org)
|
||||
public TwoFactorDuoResponseModel(Organization organization)
|
||||
: base(ResponseObj)
|
||||
{
|
||||
if (org == null)
|
||||
{
|
||||
throw new ArgumentNullException(nameof(org));
|
||||
}
|
||||
ArgumentNullException.ThrowIfNull(organization);
|
||||
|
||||
var provider = org.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo);
|
||||
var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo);
|
||||
Build(provider);
|
||||
}
|
||||
|
||||
public bool Enabled { get; set; }
|
||||
public string Host { get; set; }
|
||||
//TODO - will remove SecretKey with PM-8107
|
||||
public string SecretKey { get; set; }
|
||||
//TODO - will remove IntegrationKey with PM-8107
|
||||
public string IntegrationKey { get; set; }
|
||||
public string ClientSecret { get; set; }
|
||||
public string ClientId { get; set; }
|
||||
|
||||
// updated build to assist in the EDD migration for the Duo 2FA provider
|
||||
private void Build(TwoFactorProvider provider)
|
||||
{
|
||||
if (provider?.MetaData != null && provider.MetaData.Count > 0)
|
||||
@ -54,36 +43,13 @@ public class TwoFactorDuoResponseModel : ResponseModel
|
||||
{
|
||||
Host = (string)host;
|
||||
}
|
||||
|
||||
//todo - will remove SKey and IKey with PM-8107
|
||||
// check Skey and IKey first if they exist
|
||||
if (provider.MetaData.TryGetValue("SKey", out var sKey))
|
||||
{
|
||||
ClientSecret = MaskKey((string)sKey);
|
||||
SecretKey = MaskKey((string)sKey);
|
||||
}
|
||||
if (provider.MetaData.TryGetValue("IKey", out var iKey))
|
||||
{
|
||||
IntegrationKey = (string)iKey;
|
||||
ClientId = (string)iKey;
|
||||
}
|
||||
|
||||
// Even if IKey and SKey exist prioritize v4 params ClientId and ClientSecret
|
||||
if (provider.MetaData.TryGetValue("ClientSecret", out var clientSecret))
|
||||
{
|
||||
if (!string.IsNullOrWhiteSpace((string)clientSecret))
|
||||
{
|
||||
ClientSecret = MaskKey((string)clientSecret);
|
||||
SecretKey = MaskKey((string)clientSecret);
|
||||
}
|
||||
ClientSecret = MaskSecret((string)clientSecret);
|
||||
}
|
||||
if (provider.MetaData.TryGetValue("ClientId", out var clientId))
|
||||
{
|
||||
if (!string.IsNullOrWhiteSpace((string)clientId))
|
||||
{
|
||||
ClientId = (string)clientId;
|
||||
IntegrationKey = (string)clientId;
|
||||
}
|
||||
ClientId = (string)clientId;
|
||||
}
|
||||
}
|
||||
else
|
||||
@ -92,30 +58,7 @@ public class TwoFactorDuoResponseModel : ResponseModel
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
use this method to ensure that both v2 params and v4 params are in sync
|
||||
todo will be removed in pm-8107
|
||||
*/
|
||||
private void Temporary_SyncDuoParams()
|
||||
{
|
||||
// Even if IKey and SKey exist prioritize v4 params ClientId and ClientSecret
|
||||
if (!string.IsNullOrWhiteSpace(ClientSecret) && !string.IsNullOrWhiteSpace(ClientId))
|
||||
{
|
||||
SecretKey = ClientSecret;
|
||||
IntegrationKey = ClientId;
|
||||
}
|
||||
else if (!string.IsNullOrWhiteSpace(SecretKey) && !string.IsNullOrWhiteSpace(IntegrationKey))
|
||||
{
|
||||
ClientSecret = SecretKey;
|
||||
ClientId = IntegrationKey;
|
||||
}
|
||||
else
|
||||
{
|
||||
throw new InvalidDataException("Invalid Duo parameters.");
|
||||
}
|
||||
}
|
||||
|
||||
private static string MaskKey(string key)
|
||||
private static string MaskSecret(string key)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(key) || key.Length <= 6)
|
||||
{
|
||||
|
Reference in New Issue
Block a user