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 SDK version 2 from server code #4837

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cd3ae73
replace duo token provider
ike-kottlowski Sep 25, 2024
a917097
removed last bits and replaced validation
ike-kottlowski Sep 26, 2024
34b4a12
fixed tests
ike-kottlowski Sep 27, 2024
966fe5f
Testing Duo two factor implementation
ike-kottlowski Sep 30, 2024
55105fe
Formatting
ike-kottlowski Sep 30, 2024
8ed411e
formatting
ike-kottlowski Sep 30, 2024
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
45b5a04
replace duo token provider
ike-kottlowski Sep 25, 2024
caac7c7
removed last bits and replaced validation
ike-kottlowski Sep 26, 2024
668b451
fixed tests
ike-kottlowski Sep 27, 2024
80fc7c1
Testing Duo two factor implementation
ike-kottlowski Sep 30, 2024
f227fa9
Formatting
ike-kottlowski Sep 30, 2024
5452259
formatting
ike-kottlowski Sep 30, 2024
0fadba2
syncing
ike-kottlowski Oct 16, 2024
6d30c3f
Merge branch 'main' into auth/pm-8107/remove-v2-duo
ike-kottlowski Oct 16, 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
82 changes: 22 additions & 60 deletions src/Api/Auth/Controllers/TwoFactorController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
using Bit.Api.Auth.Models.Response.TwoFactor;
using Bit.Api.Models.Request;
using Bit.Api.Models.Response;
using Bit.Core;

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

View workflow job for this annotation

GitHub Actions / Lint

Using directive is unnecessary.
using Bit.Core.AdminConsole.Entities;
using Bit.Core.Auth.Enums;
using Bit.Core.Auth.Identity;
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;
Expand All @@ -29,11 +31,11 @@
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 IDuoTokenProvider _duoTokenProvider;
private readonly IDataProtectorTokenFactory<TwoFactorAuthenticatorUserVerificationTokenable> _twoFactorAuthenticatorDataProtector;
private readonly IDataProtectorTokenFactory<SsoEmail2faSessionTokenable> _ssoEmailTwoFactorSessionDataProtector;

Expand All @@ -46,17 +48,18 @@
ICurrentContext currentContext,
IVerifyAuthRequestCommand verifyAuthRequestCommand,
IFeatureService featureService,
IDuoTokenProvider duoTokenProvider,
IDataProtectorTokenFactory<TwoFactorAuthenticatorUserVerificationTokenable> twoFactorAuthenticatorDataProtector,
IDataProtectorTokenFactory<SsoEmail2faSessionTokenable> ssoEmailTwoFactorSessionDataProtector)
{
_userService = userService;
_organizationRepository = organizationRepository;
_organizationService = organizationService;
_globalSettings = globalSettings;
_userManager = userManager;
_currentContext = currentContext;
_verifyAuthRequestCommand = verifyAuthRequestCommand;
_featureService = featureService;
_duoTokenProvider = duoTokenProvider;
_twoFactorAuthenticatorDataProtector = twoFactorAuthenticatorDataProtector;
_ssoEmailTwoFactorSessionDataProtector = ssoEmailTwoFactorSessionDataProtector;
}
Expand Down Expand Up @@ -184,21 +187,7 @@
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 _duoTokenProvider.ValidateDuoConfiguration(model.ClientId, model.ClientSecret, model.Host))
{
throw new BadRequestException(
"Duo configuration settings are not valid. Please re-check the Duo Admin panel.");
Expand All @@ -215,14 +204,8 @@
[FromBody] SecretVerificationRequestModel model)
{
await CheckAsync(model, false, true);
var organization = await CheckOrganizationAsync(new Guid(id));

var orgIdGuid = new Guid(id);
if (!await _currentContext.ManagePolicies(orgIdGuid))
{
throw new NotFoundException();
}

var organization = await _organizationRepository.GetByIdAsync(orgIdGuid) ?? throw new NotFoundException();
var response = new TwoFactorDuoResponseModel(organization);
return response;
}
Expand All @@ -233,29 +216,9 @@
[FromBody] UpdateTwoFactorDuoRequestModel model)
{
await CheckAsync(model, false);
var organization = await CheckOrganizationAsync(new Guid(id));

var orgIdGuid = new Guid(id);
if (!await _currentContext.ManagePolicies(orgIdGuid))
{
throw new NotFoundException();
}

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 _duoTokenProvider.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 Expand Up @@ -406,19 +369,8 @@
public async Task<TwoFactorProviderResponseModel> PutOrganizationDisable(string id,
[FromBody] TwoFactorProviderRequestModel model)
{
var user = await CheckAsync(model, false);

var orgIdGuid = new Guid(id);
if (!await _currentContext.ManagePolicies(orgIdGuid))
{
throw new NotFoundException();
}

var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
if (organization == null)
{
throw new NotFoundException();
}
await CheckAsync(model, false);
var organization = await CheckOrganizationAsync(new Guid(id));

await _organizationService.DisableTwoFactorProviderAsync(organization, model.Type.Value);
var response = new TwoFactorProviderResponseModel(model.Type.Value, organization);
Expand Down Expand Up @@ -460,6 +412,16 @@
return Task.FromResult(new DeviceVerificationResponseModel(false, false));
}

private async Task<Organization> CheckOrganizationAsync(Guid organizationId)
{
if (!await _currentContext.ManagePolicies(organizationId))
{
throw new NotFoundException();
}
var organization = await _organizationRepository.GetByIdAsync(organizationId) ?? throw new NotFoundException();
return organization;
}

private async Task<User> CheckAsync(SecretVerificationRequestModel model, bool premium,
bool skipVerification = false)
{
Expand Down
59 changes: 4 additions & 55 deletions src/Api/Auth/Models/Request/TwoFactorRequestModels.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.Auth.Enums;
using Bit.Core.Auth.Models;
using Bit.Core.Auth.Utilities;
using Bit.Core.Entities;
using Fido2NetLib;

Expand Down Expand Up @@ -42,20 +41,12 @@ 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.
*/
[Required]
[StringLength(50)]
public string ClientId { get; set; }
[Required]
[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)]
public string Host { get; set; }
Expand All @@ -65,22 +56,17 @@ 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 +82,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 @@ -121,38 +102,6 @@ public Organization ToOrganization(Organization existingOrg)
existingOrg.SetTwoFactorProviders(providers);
return existingOrg;
}

public override IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
if (!DuoApi.ValidHost(Host))
{
yield return new ValidationResult("Host is invalid.", [nameof(Host)]);
}
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))
{
SecretKey = ClientSecret;
IntegrationKey = ClientId;
}
else if (!string.IsNullOrWhiteSpace(SecretKey) && !string.IsNullOrWhiteSpace(IntegrationKey))
{
ClientSecret = SecretKey;
ClientId = IntegrationKey;
}
}
}

public class UpdateTwoFactorYubicoOtpRequestModel : SecretVerificationRequestModel, IValidatableObject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ 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);
Expand All @@ -25,25 +22,17 @@ public TwoFactorDuoResponseModel(User user)
public TwoFactorDuoResponseModel(Organization org)
: base(ResponseObj)
{
if (org == null)
{
throw new ArgumentNullException(nameof(org));
}
ArgumentNullException.ThrowIfNull(org);

var provider = org.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,37 +58,14 @@ 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 secret)
{
if (string.IsNullOrWhiteSpace(key) || key.Length <= 6)
if (string.IsNullOrWhiteSpace(secret) || secret.Length <= 6)
{
return key;
return secret;
}

// Mask all but the first 6 characters.
return string.Concat(key.AsSpan(0, 6), new string('*', key.Length - 6));
return string.Concat(secret.AsSpan(0, 6), new string('*', secret.Length - 6));
}
}
Loading
Loading