Skip to content

Commit 598fdc4

Browse files
author
Daniel Jacinto
authored
[2FA] Disable nuget.org password login (#9141)
* email logging feature flag * add emaillogin to accoutn controller * feature flag enabled by default * added sign in check * refresh content service. * Address PR comments. * update loging exception list method. * Test for account pass condition * Block ForgotPassword workflow when flag is enabled. * fix test cases. * updated ForgotPassword_Disabled * Validation changed to auth service, audit added. * updated ui and refactor email method. * Fix tests. * remove forgot password condition.
1 parent a80bdb5 commit 598fdc4

25 files changed

Lines changed: 354 additions & 18 deletions

src/AccountDeleter/EmptyFeatureFlagService.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ public bool IsDisplayVulnerabilitiesEnabled()
8686
throw new NotImplementedException();
8787
}
8888

89+
public bool IsNuGetAccountPasswordLoginEnabled()
90+
{
91+
throw new NotImplementedException();
92+
}
93+
8994
public bool IsForceFlatContainerIconsEnabled()
9095
{
9196
throw new NotImplementedException();

src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,5 +279,10 @@ public bool IsNewAccount2FAEnforcementEnabled()
279279
{
280280
throw new NotImplementedException();
281281
}
282+
283+
public bool IsNuGetAccountPasswordLoginEnabled()
284+
{
285+
throw new NotImplementedException();
286+
}
282287
}
283288
}

src/NuGetGallery.Core/Auditing/AuditedAuthenticatedOperationAction.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ public enum AuditedAuthenticatedOperationAction
2828
/// <summary>
2929
/// Symbol package push was attempted by a non-owner of the package
3030
/// </summary>
31-
SymbolsPackagePushAttemptByNonOwner
31+
SymbolsPackagePushAttemptByNonOwner,
32+
33+
/// <summary>
34+
/// User attempted to login when password login is unsupported
35+
/// </summary>
36+
PasswordLoginUnsupported
3237
}
3338
}

src/NuGetGallery.Services/Authentication/AuthenticationService.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public class AuthenticationService: IAuthenticationService
3333
private readonly IDateTimeProvider _dateTimeProvider;
3434
private readonly IContentObjectService _contentObjectService;
3535
private readonly ITelemetryService _telemetryService;
36+
private readonly IFeatureFlagService _featureFlagService;
3637

3738
/// <summary>
3839
/// This ctor is used for test only.
@@ -48,7 +49,7 @@ public AuthenticationService(
4849
IEntitiesContext entities, IAppConfiguration config, IDiagnosticsService diagnostics,
4950
IAuditingService auditing, IEnumerable<Authenticator> providers, ICredentialBuilder credentialBuilder,
5051
ICredentialValidator credentialValidator, IDateTimeProvider dateTimeProvider, ITelemetryService telemetryService,
51-
IContentObjectService contentObjectService)
52+
IContentObjectService contentObjectService, IFeatureFlagService featureFlagService)
5253
{
5354
InitCredentialFormatters();
5455

@@ -62,6 +63,7 @@ public AuthenticationService(
6263
_dateTimeProvider = dateTimeProvider ?? throw new ArgumentNullException(nameof(dateTimeProvider));
6364
_telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
6465
_contentObjectService = contentObjectService ?? throw new ArgumentNullException(nameof(contentObjectService));
66+
_featureFlagService = featureFlagService ?? throw new ArgumentNullException(nameof(featureFlagService));
6567
}
6668

6769
public IEntitiesContext Entities { get; private set; }
@@ -83,6 +85,18 @@ public virtual async Task<PasswordAuthenticationResult> Authenticate(string user
8385
{
8486
var user = FindByUserNameOrEmail(userNameOrEmail);
8587

88+
if (!_featureFlagService.IsNuGetAccountPasswordLoginEnabled() &&
89+
!_contentObjectService.LoginDiscontinuationConfiguration.IsEmailOnExceptionsList(userNameOrEmail))
90+
{
91+
_trace.Information("Password login unsupported.");
92+
93+
await Auditing.SaveAuditRecordAsync(
94+
new FailedAuthenticatedOperationAuditRecord(
95+
userNameOrEmail, AuditedAuthenticatedOperationAction.PasswordLoginUnsupported));
96+
97+
return new PasswordAuthenticationResult(PasswordAuthenticationResult.AuthenticationResult.PasswordLoginUnsupported);
98+
}
99+
86100
// Check if the user exists
87101
if (user == null)
88102
{

src/NuGetGallery.Services/Authentication/PasswordAuthenticationResult.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public class PasswordAuthenticationResult
88
public enum AuthenticationResult
99
{
1010
AccountLocked, // The account is locked
11+
PasswordLoginUnsupported, // Password login is not supported
1112
BadCredentials, // Bad user name or password provided
1213
Success // All good
1314
}

src/NuGetGallery.Services/Configuration/FeatureFlagService.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public class FeatureFlagService : IFeatureFlagService
5454
private const string ComputeTargetFrameworkFeatureName = GalleryPrefix + "ComputeTargetFramework";
5555
private const string RecentPackagesNoIndexFeatureName = GalleryPrefix + "RecentPackagesNoIndex";
5656
private const string NewAccount2FAEnforcementFeatureName = GalleryPrefix + "NewAccount2FAEnforcement";
57+
private const string NuGetAccountPasswordLoginFeatureName = GalleryPrefix + "NuGetAccountPasswordLogin";
5758

5859
private const string ODataV1GetAllNonHijackedFeatureName = GalleryPrefix + "ODataV1GetAllNonHijacked";
5960
private const string ODataV1GetAllCountNonHijackedFeatureName = GalleryPrefix + "ODataV1GetAllCountNonHijacked";
@@ -368,5 +369,10 @@ public bool IsNewAccount2FAEnforcementEnabled()
368369
{
369370
return _client.IsEnabled(NewAccount2FAEnforcementFeatureName, defaultValue: false);
370371
}
372+
373+
public bool IsNuGetAccountPasswordLoginEnabled()
374+
{
375+
return _client.IsEnabled(NuGetAccountPasswordLoginFeatureName, defaultValue: true);
376+
}
371377
}
372378
}

src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ public interface IFeatureFlagService
177177
/// Whether the user is able to publish the package with an embedded readme file.
178178
/// </summary>
179179
bool AreEmbeddedReadmesEnabled(User user);
180-
180+
181181
/// <summary>
182182
/// Whether the /Packages() endpoint is enabled for the V1 OData API.
183183
/// </summary>
@@ -262,7 +262,7 @@ public interface IFeatureFlagService
262262
/// Whether rendering Markdown content to HTML using Markdig is enabled
263263
/// </summary>
264264
bool IsMarkdigMdRenderingEnabled();
265-
265+
266266
/// Whether or not the user can delete a package through the API.
267267
/// </summary>
268268
bool IsDeletePackageApiEnabled(User user);
@@ -276,7 +276,7 @@ public interface IFeatureFlagService
276276
/// Whether or not display the banner on nuget.org
277277
/// </summary>
278278
bool IsDisplayBannerEnabled();
279-
279+
280280
/// <summary>
281281
/// Whether or not display target framework badges and table on nuget.org
282282
/// </summary>
@@ -296,5 +296,10 @@ public interface IFeatureFlagService
296296
/// Whether or not to enforce 2FA for new external account link or replacement.
297297
/// </summary>
298298
bool IsNewAccount2FAEnforcementEnabled();
299+
300+
/// <summary>
301+
/// Whether or not NuGet.org password login is supported. NuGet.org accounts in the <see cref="LoginDiscontinuationConfiguration.ExceptionsForEmailAddresses"/> will always be supported.
302+
/// </summary>
303+
bool IsNuGetAccountPasswordLoginEnabled();
299304
}
300305
}

src/NuGetGallery.Services/Configuration/ILoginDiscontinuationConfiguration.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@ public interface ILoginDiscontinuationConfiguration
1313
bool IsUserOnWhitelist(User user);
1414
bool ShouldUserTransformIntoOrganization(User user);
1515
bool IsTenantIdPolicySupportedForOrganization(string emailAddress, string tenantId);
16+
bool IsEmailOnExceptionsList(string emailAddress);
1617
}
1718
}

src/NuGetGallery.Services/Configuration/LoginDiscontinuationConfiguration.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,16 @@ public bool IsPasswordLoginDiscontinuedForAll()
104104
{
105105
return IsPasswordDiscontinuedForAll;
106106
}
107+
108+
public bool IsEmailOnExceptionsList(string emailAddress)
109+
{
110+
if (string.IsNullOrEmpty(emailAddress))
111+
{
112+
return false;
113+
}
114+
115+
return ExceptionsForEmailAddresses.Contains(emailAddress);
116+
}
107117
}
108118

109119
public class OrganizationTenantPair : IEquatable<OrganizationTenantPair>

src/NuGetGallery/App_Data/Files/Content/flags.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@
3232
"NuGetGallery.ImageAllowlist": "Enabled",
3333
"NuGetGallery.ShowReportAbuseSafetyChanges": "Enabled",
3434
"NuGetGallery.ComputeTargetFramework": "Enabled",
35-
"NuGetGallery.NewAccount2FAEnforcement": "Disabled"
35+
"NuGetGallery.NewAccount2FAEnforcement": "Disabled",
36+
"NuGetGallery.NuGetAccountPasswordLogin": "Enabled"
3637
},
3738
"Flights": {
3839
"NuGetGallery.TyposquattingFlight": {

0 commit comments

Comments
 (0)