Skip to content

Commit

Permalink
Require password complexity (#3205)
Browse files Browse the repository at this point in the history
Require password complexity
  • Loading branch information
skofman1 authored Sep 12, 2016
1 parent a173a57 commit 93fa02b
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 21 deletions.
12 changes: 12 additions & 0 deletions src/NuGetGallery/Configuration/AppConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,18 @@ public class AppConfiguration : IAppConfiguration
/// </summary>
public string EnforcedAuthProviderForAdmin { get; set; }

/// <summary>
/// A regex to validate password format. The default regex requires the password to be atlease 8 characters,
/// include at least one uppercase letter, one lowercase letter and a digit.
/// </summary>
[Required]
[DefaultValue("^(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9]).{8,64}$")]
public string UserPasswordRegex { get; set; }

[Required]
[DefaultValue("Your password must be at least 8 characters, should include at least one uppercase letter, one lowercase letter and a digit.")]
public string UserPasswordHint { get; set; }

/// <summary>
/// Defines the time after which V1 API keys expire.
/// </summary>
Expand Down
10 changes: 10 additions & 0 deletions src/NuGetGallery/Configuration/IAppConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,16 @@ public interface IAppConfiguration
/// </summary>
string EnforcedAuthProviderForAdmin { get; set; }

/// <summary>
/// The required format for a user password.
/// </summary>
string UserPasswordRegex { get; set; }

/// <summary>
/// A message to show the user, to explain password requirements.
/// </summary>
string UserPasswordHint { get; set; }

/// <summary>
/// Defines the time after which V1 API keys expire.
/// </summary>
Expand Down
36 changes: 36 additions & 0 deletions src/NuGetGallery/Infrastructure/PasswordValidationAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.ComponentModel.DataAnnotations;
using System.Web.Mvc;
using NuGetGallery.Configuration;

namespace NuGetGallery.Infrastructure
{
[AttributeUsage(AttributeTargets.Property)]
public sealed class PasswordValidationAttribute : ValidationAttribute
{
private readonly RegularExpressionAttribute _regexAttribute;

public PasswordValidationAttribute()
{
var configuration = DependencyResolver.Current.GetService<IGalleryConfigurationService>().Current;

_regexAttribute = new RegularExpressionAttribute(configuration.UserPasswordRegex)
{
ErrorMessage = configuration.UserPasswordHint
};
}

public override bool IsValid(object value)
{
return _regexAttribute.IsValid(value);
}

public override string FormatErrorMessage(string name)
{
return _regexAttribute.FormatErrorMessage(name);
}
}
}
1 change: 1 addition & 0 deletions src/NuGetGallery/NuGetGallery.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@
<Compile Include="Infrastructure\Authentication\CredentialValidator.cs" />
<Compile Include="Infrastructure\Authentication\ICredentialBuilder.cs" />
<Compile Include="Infrastructure\Authentication\ICredentialValidator.cs" />
<Compile Include="Infrastructure\PasswordValidationAttribute.cs" />
<Compile Include="Migrations\201602181939424_RemovePackageStatistics.cs" />
<Compile Include="Migrations\201602181939424_RemovePackageStatistics.Designer.cs">
<DependentUpon>201602181939424_RemovePackageStatistics.cs</DependentUpon>
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ The {2} Team</value>
</data>
<data name="NuGetPackageReleaseVersionContainsOnlyNumerics" xml:space="preserve">
<value>Package {0} invalid: the release label can not only contain numerics.</value>
</data>
</data>
<data name="CopyExternalPackages_PackageFileBlobAlreadyExists" xml:space="preserve">
<value>SKIPPED! Package file blob {0} already exists</value>
</data>
Expand Down
2 changes: 2 additions & 0 deletions src/NuGetGallery/ViewModels/AccountViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.ComponentModel.DataAnnotations;
using System.Web.Mvc;
using NuGetGallery.Authentication.Providers;
using NuGetGallery.Infrastructure;

namespace NuGetGallery
{
Expand Down Expand Up @@ -53,6 +54,7 @@ public class ChangePasswordViewModel

[Required]
[Display(Name = "New Password")]
[PasswordValidation]
[AllowHtml]
public string NewPassword { get; set; }

Expand Down
22 changes: 7 additions & 15 deletions src/NuGetGallery/ViewModels/LogOnViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Text.RegularExpressions;
using System.Web.Mvc;
using NuGetGallery.Authentication.Providers;
using NuGetGallery.Infrastructure;

namespace NuGetGallery
{
Expand Down Expand Up @@ -74,40 +74,32 @@ public class RegisterViewModel
internal const string EmailValidationRegex = "^" + FirstPart + "@" + SecondPart + "$";

internal const string EmailValidationErrorMessage = "This doesn't appear to be a valid email address.";
public const string EmailHint = "Your email will not be public unless you choose to disclose it. " +
"It is required to verify your registration and for password retrieval, important notifications, etc. ";

internal const string UsernameValidationRegex =
@"[A-Za-z0-9][A-Za-z0-9_\.-]+[A-Za-z0-9]";

/// <summary>
/// Regex that matches INVALID username characters, to make it easy to strip those characters out.
/// </summary>
internal static readonly Regex UsernameNormalizationRegex =
new Regex(@"[^A-Za-z0-9_\.-]");


internal const string UsernameValidationErrorMessage =
"User names must start and end with a letter or number, and may only contain letters, numbers, underscores, periods, and hyphens in between.";

public const string UserNameHint = "Choose something unique so others will know which contributions are yours.";

[Required]
[StringLength(255)]
[Display(Name = "Email")]
//[DataType(DataType.EmailAddress)] - does not work with client side validation
[RegularExpression(EmailValidationRegex, ErrorMessage = EmailValidationErrorMessage)]
[Hint(
"Your email will not be public unless you choose to disclose it. " +
"It is required to verify your registration and for password retrieval, important notifications, etc. ")]
[Subtext("We use <a href=\"http://www.gravatar.com\" target=\"_blank\">Gravatar</a> to get your profile picture", AllowHtml = true)]
public string EmailAddress { get; set; }

[Required]
[StringLength(64)]
[RegularExpression(UsernameValidationRegex, ErrorMessage = UsernameValidationErrorMessage)]
[Hint("Choose something unique so others will know which contributions are yours.")]
public string Username { get; set; }

[Required]
[DataType(DataType.Password)]
[StringLength(64, MinimumLength = 7)]
[Hint("Passwords must be at least 7 characters long.")]
[PasswordValidation]
[AllowHtml]
public string Password { get; set; }
}
Expand Down
4 changes: 2 additions & 2 deletions src/NuGetGallery/ViewModels/PasswordResetViewModel.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.ComponentModel.DataAnnotations;
using NuGetGallery.Infrastructure;

namespace NuGetGallery
{
Expand All @@ -9,8 +10,7 @@ public class PasswordResetViewModel
[Required]
[DataType(DataType.Password)]
[Display(Name = "New password")]
[StringLength(64, MinimumLength = 7)]
[Hint("Passwords must be at least 7 characters long.")]
[PasswordValidation]
public string NewPassword { get; set; }

[Required]
Expand Down
17 changes: 14 additions & 3 deletions src/NuGetGallery/Views/Authentication/_Register.cshtml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@model LogOnViewModel
@using NuGetGallery.Configuration
@model LogOnViewModel

<div id="register-form">
@using (Html.BeginForm("Register", "Authentication"))
Expand All @@ -19,7 +20,8 @@
<div class="form-field form-field-full">
@Html.LabelFor(m => m.Register.Username)
@Html.EditorFor(m => m.Register.Username)
@Html.ValidationMessageFor(m => m.Register.Username)
@Html.ValidationMessageFor(m => m.Register.Username)
<span class="field-hint-message">@RegisterViewModel.UserNameHint</span>
</div>

@if (Model.External == null)
Expand All @@ -28,13 +30,15 @@
@Html.LabelFor(m => m.Register.Password)
@Html.EditorFor(m => m.Register.Password)
@Html.ValidationMessageFor(m => m.Register.Password)
<span class="field-hint-message">@PasswordHint()</span>
</div>
}

<div class="form-field form-field-full">
<label for="EmailAddress">Email (<a href="http://www.gravatar.com">Gravatar</a>, notifications, and password recovery)</label>
@Html.EditorFor(m => m.Register.EmailAddress)
@Html.ValidationMessageFor(m => m.Register.EmailAddress)
<span class="field-hint-message">@RegisterViewModel.EmailHint</span>
</div>

<img src="@Url.Content("~/Content/images/required.png")" alt="Blue border on left means required." />
Expand All @@ -46,5 +50,12 @@

<input class="btn btn-big" type="submit" value="Register" title="Register" />
</fieldset>
}
}

@helper PasswordHint() {
var config = DependencyResolver.Current.GetService<IGalleryConfigurationService>();
string hint = config.Current.UserPasswordHint;
@hint
}

</div>
5 changes: 5 additions & 0 deletions src/NuGetGallery/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
<add key="Gallery.ExpirationInDaysForApiKeyV1" value="365" />
<add key="Gallery.WarnAboutExpirationInDaysForApiKeyV1" value="10" />

<!-- To change password complexity requirements, uncomment those lines, and set your regex and password hint.
<add key="Gallery.UserPasswordRegex" value="^(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9]).{8,64}$" />
<add key="Gallery.UserPasswordHint" value="Your password must be at least 8 characters, should include at least one uppercase letter, one lowercase letter and a digit." />
-->

<!-- *************** -->
<!-- STABLE SETTINGS -->
<!-- *************** -->
Expand Down
1 change: 1 addition & 0 deletions tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@
<Compile Include="Controllers\StatisticsControllerFacts.cs" />
<Compile Include="Controllers\UsersControllerFacts.cs" />
<Compile Include="Framework\TestGalleryConfigurationService.cs" />
<Compile Include="PasswordValidationRegexTests.cs" />
<Compile Include="SearchClient\CorrelatingHttpClientHandlerFacts.cs" />
<Compile Include="SearchClient\RequestInspectingHandler.cs" />
<Compile Include="SearchClient\WebApiCorrelationHandlerFacts.cs" />
Expand Down
51 changes: 51 additions & 0 deletions tests/NuGetGallery.Facts/PasswordValidationRegexTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Text.RegularExpressions;
using NuGetGallery.Configuration;
using NuGetGallery.Framework;
using Xunit;

namespace NuGetGallery
{
/// <summary>
/// The regex checks that the password is at least 8 characters, one uppercase letter, one lowercase letter, and a digit.
/// </summary>
public class PasswordValidationRegexTests : TestContainer
{
private readonly string _defaultPasswordRegex;

public PasswordValidationRegexTests()
{
var configuration = Get<ConfigurationService>();
_defaultPasswordRegex = configuration.Current.UserPasswordRegex;
}

[Theory]
[InlineData("aA1aaaaa")]
[InlineData("abcdefg$0B")]
[InlineData("****1bB***")]
public void Accepts(string password)
{

var match = new Regex(_defaultPasswordRegex).IsMatch(password);
Assert.True(match);
}

[Theory]
[InlineData("v")] // Single letter
[InlineData("V")] // Single upper case letter
[InlineData("8")] // Single number
[InlineData("89984214214")] // Just numbers
[InlineData("%*`~&*()%#@$!@<>?\"")] // Special characters
[InlineData("aaAAaaAAaaAA")] // No digit
[InlineData("12345678a")] // No upperscase letter
[InlineData("12345678A")] // No lowercase letter
[InlineData("1aA")] // Too short
public void DoesNotAccept(string password)
{
var match = new Regex(_defaultPasswordRegex).IsMatch(password);
Assert.False(match);
}
}
}

0 comments on commit 93fa02b

Please sign in to comment.