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

[PM_8107] Remove Duo v2 from server #4934

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ed66e44
initial device removal
ike-kottlowski Oct 2, 2024
5b027ed
Unit Testing
ike-kottlowski Oct 3, 2024
07264af
Finalized tests
ike-kottlowski Oct 4, 2024
7430802
initial commit refactoring two factor
ike-kottlowski Oct 11, 2024
b994eea
initial tests
ike-kottlowski Oct 11, 2024
4c0871a
Unit Tests
ike-kottlowski Oct 15, 2024
db2e3fd
initial device removal
ike-kottlowski Oct 2, 2024
d127e6f
Unit Testing
ike-kottlowski Oct 3, 2024
7ef6c0b
Finalized tests
ike-kottlowski Oct 4, 2024
3b5ec21
initial commit refactoring two factor
ike-kottlowski Oct 11, 2024
784cfbd
initial tests
ike-kottlowski Oct 11, 2024
e6b0a80
Unit Tests
ike-kottlowski Oct 15, 2024
baa9125
Merge branch 'auth/pm-6666/twofactorvalidator-refactor' of https://giโ€ฆ
ike-kottlowski Oct 15, 2024
c702ec4
Fixing some tests
ike-kottlowski Oct 15, 2024
2d71376
renaming and reorganizing
ike-kottlowski Oct 16, 2024
398ce1c
refactored two factor flows
ike-kottlowski Oct 21, 2024
acc6796
fixed a possible issue with object mapping.
ike-kottlowski Oct 21, 2024
c3ca34e
Update TwoFactorAuthenticationValidator.cs
ike-kottlowski Oct 22, 2024
2f85154
Initial commit
ike-kottlowski Oct 22, 2024
d2539a4
Updating tests
ike-kottlowski Oct 23, 2024
1081985
formatting and data validation
ike-kottlowski Oct 23, 2024
b29787d
fixing tests
ike-kottlowski Oct 24, 2024
a3eb115
Merge branch 'main' into auth/pm-8107/remove-duo-metadata-v2
ike-kottlowski Oct 24, 2024
4e560d4
formatting
ike-kottlowski Oct 24, 2024
f6e5261
added tests; formatting
ike-kottlowski Oct 25, 2024
7ed5aaf
Merge branch 'main' into auth/pm-8107/remove-duo-metadata-v2
ike-kottlowski Oct 25, 2024
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
44 changes: 6 additions & 38 deletions src/Api/Auth/Controllers/TwoFactorController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
using Bit.Core.Auth.Enums;
using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Auth.Utilities;
using Bit.Core.Auth.Services;
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;
Expand All @@ -29,34 +28,31 @@ 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 IDuoUniversalConfigService _duoUniversalConfigService;
private readonly IDataProtectorTokenFactory<TwoFactorAuthenticatorUserVerificationTokenable> _twoFactorAuthenticatorDataProtector;
private readonly IDataProtectorTokenFactory<SsoEmail2faSessionTokenable> _ssoEmailTwoFactorSessionDataProtector;

public TwoFactorController(
IUserService userService,
IOrganizationRepository organizationRepository,
IOrganizationService organizationService,
GlobalSettings globalSettings,
UserManager<User> userManager,
ICurrentContext currentContext,
IVerifyAuthRequestCommand verifyAuthRequestCommand,
IFeatureService featureService,
IDuoUniversalConfigService duoUniversalConfigService,
IDataProtectorTokenFactory<TwoFactorAuthenticatorUserVerificationTokenable> twoFactorAuthenticatorDataProtector,
IDataProtectorTokenFactory<SsoEmail2faSessionTokenable> ssoEmailTwoFactorSessionDataProtector)
{
_userService = userService;
_organizationRepository = organizationRepository;
_organizationService = organizationService;
_globalSettings = globalSettings;
_userManager = userManager;
_currentContext = currentContext;
_verifyAuthRequestCommand = verifyAuthRequestCommand;
_featureService = featureService;
_duoUniversalConfigService = duoUniversalConfigService;
_twoFactorAuthenticatorDataProtector = twoFactorAuthenticatorDataProtector;
_ssoEmailTwoFactorSessionDataProtector = ssoEmailTwoFactorSessionDataProtector;
}
Expand Down Expand Up @@ -184,21 +180,7 @@ public async Task<TwoFactorDuoResponseModel> GetDuo([FromBody] SecretVerificatio
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 _duoUniversalConfigService.ValidateDuoConfiguration(model.ClientSecret, model.ClientId, model.Host))
{
throw new BadRequestException(
"Duo configuration settings are not valid. Please re-check the Duo Admin panel.");
Expand Down Expand Up @@ -241,21 +223,7 @@ public async Task<TwoFactorDuoResponseModel> PutOrganizationDuo(string id,
}

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 _duoUniversalConfigService.ValidateDuoConfiguration(model.ClientId, model.ClientSecret, model.Host))
{
throw new BadRequestException(
"Duo configuration settings are not valid. Please re-check the Duo Admin panel.");
Expand Down
61 changes: 17 additions & 44 deletions src/Api/Auth/Models/Request/TwoFactorRequestModels.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,44 +43,34 @@ public User ToUser(User existingUser)
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)]
[Required]
[StringLength(20, MinimumLength = 20, ErrorMessage = "Client Id must be exactly 20 characters.")]
public string ClientId { get; set; }
[StringLength(50)]
[Required]
[StringLength(40, MinimumLength = 40, ErrorMessage = "Client Secret must be exactly 40 characters.")]
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)]
public string Host { get; set; }

public User ToUser(User existingUser)
{
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
Expand All @@ -96,22 +86,17 @@ public Organization ToOrganization(Organization existingOrg)
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
Expand All @@ -124,34 +109,22 @@ public Organization ToOrganization(Organization existingOrg)

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) || !DuoUtilities.ValidDuoHost(Host))
{
ClientSecret = SecretKey;
ClientId = IntegrationKey;
results.Add(new ValidationResult("Host is invalid.", [nameof(Host)]));
}
return results;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -54,36 +43,13 @@ private void Build(TwoFactorProvider provider)
{
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
Expand All @@ -92,30 +58,7 @@ private void Build(TwoFactorProvider provider)
}
}

/*
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)
{
Expand Down
3 changes: 1 addition & 2 deletions src/Api/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
using Bit.SharedWeb.Utilities;
using Microsoft.AspNetCore.Diagnostics.HealthChecks;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Bit.Core.Auth.Identity;
using Bit.Core.Auth.UserFeatures;
using Bit.Core.Entities;
using Bit.Core.Billing.Extensions;
Expand All @@ -32,7 +31,7 @@
using Bit.Core.Vault.Entities;
using Bit.Api.Auth.Models.Request.WebAuthn;
using Bit.Core.Auth.Models.Data;

using Bit.Core.Auth.Identity.TokenProviders;

#if !OSS
using Bit.Commercial.Core.SecretsManager;
Expand Down
Loading
Loading