From 93fa02bf03cd37fd6ca4585e8aa326d174a17dc0 Mon Sep 17 00:00:00 2001 From: Svetlana Kofman Date: Mon, 12 Sep 2016 16:49:22 -0700 Subject: [PATCH] Require password complexity (#3205) Require password complexity --- .../Configuration/AppConfiguration.cs | 12 +++++ .../Configuration/IAppConfiguration.cs | 10 ++++ .../PasswordValidationAttribute.cs | 36 +++++++++++++ src/NuGetGallery/NuGetGallery.csproj | 1 + src/NuGetGallery/Strings.resx | 2 +- .../ViewModels/AccountViewModel.cs | 2 + src/NuGetGallery/ViewModels/LogOnViewModel.cs | 22 +++----- .../ViewModels/PasswordResetViewModel.cs | 4 +- .../Views/Authentication/_Register.cshtml | 17 +++++-- src/NuGetGallery/Web.config | 5 ++ .../NuGetGallery.Facts.csproj | 1 + .../PasswordValidationRegexTests.cs | 51 +++++++++++++++++++ 12 files changed, 142 insertions(+), 21 deletions(-) create mode 100644 src/NuGetGallery/Infrastructure/PasswordValidationAttribute.cs create mode 100644 tests/NuGetGallery.Facts/PasswordValidationRegexTests.cs diff --git a/src/NuGetGallery/Configuration/AppConfiguration.cs b/src/NuGetGallery/Configuration/AppConfiguration.cs index dacc41c9b0..570d232a50 100644 --- a/src/NuGetGallery/Configuration/AppConfiguration.cs +++ b/src/NuGetGallery/Configuration/AppConfiguration.cs @@ -174,6 +174,18 @@ public class AppConfiguration : IAppConfiguration /// public string EnforcedAuthProviderForAdmin { get; set; } + /// + /// 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. + /// + [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; } + /// /// Defines the time after which V1 API keys expire. /// diff --git a/src/NuGetGallery/Configuration/IAppConfiguration.cs b/src/NuGetGallery/Configuration/IAppConfiguration.cs index b9250fb31f..cb05ae8c29 100644 --- a/src/NuGetGallery/Configuration/IAppConfiguration.cs +++ b/src/NuGetGallery/Configuration/IAppConfiguration.cs @@ -160,6 +160,16 @@ public interface IAppConfiguration /// string EnforcedAuthProviderForAdmin { get; set; } + /// + /// The required format for a user password. + /// + string UserPasswordRegex { get; set; } + + /// + /// A message to show the user, to explain password requirements. + /// + string UserPasswordHint { get; set; } + /// /// Defines the time after which V1 API keys expire. /// diff --git a/src/NuGetGallery/Infrastructure/PasswordValidationAttribute.cs b/src/NuGetGallery/Infrastructure/PasswordValidationAttribute.cs new file mode 100644 index 0000000000..23704056ee --- /dev/null +++ b/src/NuGetGallery/Infrastructure/PasswordValidationAttribute.cs @@ -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().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); + } + } +} \ No newline at end of file diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj index 000442bc3c..38a38ce489 100644 --- a/src/NuGetGallery/NuGetGallery.csproj +++ b/src/NuGetGallery/NuGetGallery.csproj @@ -749,6 +749,7 @@ + 201602181939424_RemovePackageStatistics.cs diff --git a/src/NuGetGallery/Strings.resx b/src/NuGetGallery/Strings.resx index c89ab11253..831eaeed8e 100644 --- a/src/NuGetGallery/Strings.resx +++ b/src/NuGetGallery/Strings.resx @@ -316,7 +316,7 @@ The {2} Team Package {0} invalid: the release label can not only contain numerics. - + SKIPPED! Package file blob {0} already exists diff --git a/src/NuGetGallery/ViewModels/AccountViewModel.cs b/src/NuGetGallery/ViewModels/AccountViewModel.cs index 60d176fcd0..1278cfafe3 100644 --- a/src/NuGetGallery/ViewModels/AccountViewModel.cs +++ b/src/NuGetGallery/ViewModels/AccountViewModel.cs @@ -7,6 +7,7 @@ using System.ComponentModel.DataAnnotations; using System.Web.Mvc; using NuGetGallery.Authentication.Providers; +using NuGetGallery.Infrastructure; namespace NuGetGallery { @@ -53,6 +54,7 @@ public class ChangePasswordViewModel [Required] [Display(Name = "New Password")] + [PasswordValidation] [AllowHtml] public string NewPassword { get; set; } diff --git a/src/NuGetGallery/ViewModels/LogOnViewModel.cs b/src/NuGetGallery/ViewModels/LogOnViewModel.cs index f50e5107e9..5dbd2990a6 100644 --- a/src/NuGetGallery/ViewModels/LogOnViewModel.cs +++ b/src/NuGetGallery/ViewModels/LogOnViewModel.cs @@ -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 { @@ -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]"; - - /// - /// Regex that matches INVALID username characters, to make it easy to strip those characters out. - /// - 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 Gravatar 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; } } diff --git a/src/NuGetGallery/ViewModels/PasswordResetViewModel.cs b/src/NuGetGallery/ViewModels/PasswordResetViewModel.cs index 13fcfca9e5..f2e442b103 100644 --- a/src/NuGetGallery/ViewModels/PasswordResetViewModel.cs +++ b/src/NuGetGallery/ViewModels/PasswordResetViewModel.cs @@ -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 { @@ -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] diff --git a/src/NuGetGallery/Views/Authentication/_Register.cshtml b/src/NuGetGallery/Views/Authentication/_Register.cshtml index 8e1828485a..6f7cf21a3c 100644 --- a/src/NuGetGallery/Views/Authentication/_Register.cshtml +++ b/src/NuGetGallery/Views/Authentication/_Register.cshtml @@ -1,4 +1,5 @@ -@model LogOnViewModel +@using NuGetGallery.Configuration +@model LogOnViewModel
@using (Html.BeginForm("Register", "Authentication")) @@ -19,7 +20,8 @@
@Html.LabelFor(m => m.Register.Username) @Html.EditorFor(m => m.Register.Username) - @Html.ValidationMessageFor(m => m.Register.Username) + @Html.ValidationMessageFor(m => m.Register.Username) + @RegisterViewModel.UserNameHint
@if (Model.External == null) @@ -28,6 +30,7 @@ @Html.LabelFor(m => m.Register.Password) @Html.EditorFor(m => m.Register.Password) @Html.ValidationMessageFor(m => m.Register.Password) + @PasswordHint()
} @@ -35,6 +38,7 @@ @Html.EditorFor(m => m.Register.EmailAddress) @Html.ValidationMessageFor(m => m.Register.EmailAddress) + @RegisterViewModel.EmailHint Blue border on left means required. @@ -46,5 +50,12 @@ - } + } + + @helper PasswordHint() { + var config = DependencyResolver.Current.GetService(); + string hint = config.Current.UserPasswordHint; + @hint + } + \ No newline at end of file diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config index 2307c34249..17c7855ba1 100644 --- a/src/NuGetGallery/Web.config +++ b/src/NuGetGallery/Web.config @@ -81,6 +81,11 @@ + + diff --git a/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj b/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj index bb72f38355..fc4bd5c489 100644 --- a/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj +++ b/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj @@ -404,6 +404,7 @@ + diff --git a/tests/NuGetGallery.Facts/PasswordValidationRegexTests.cs b/tests/NuGetGallery.Facts/PasswordValidationRegexTests.cs new file mode 100644 index 0000000000..074e329ff5 --- /dev/null +++ b/tests/NuGetGallery.Facts/PasswordValidationRegexTests.cs @@ -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 +{ + /// + /// The regex checks that the password is at least 8 characters, one uppercase letter, one lowercase letter, and a digit. + /// + public class PasswordValidationRegexTests : TestContainer + { + private readonly string _defaultPasswordRegex; + + public PasswordValidationRegexTests() + { + var configuration = Get(); + _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); + } + } +}