diff --git a/src/NuGetGallery.Core/Entities/Credential.cs b/src/NuGetGallery.Core/Entities/Credential.cs new file mode 100644 index 0000000000..edeebfb766 --- /dev/null +++ b/src/NuGetGallery.Core/Entities/Credential.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using System.Text; + +namespace NuGetGallery +{ + public class Credential : IEntity + { + public Credential() { } + + public Credential(string type, string value) + { + Type = type; + Value = value; + } + + public int Key { get; set; } + + [Required] + public int UserKey { get; set; } + + [Required] + [StringLength(maximumLength: 64)] + public string Type { get; set; } + + [StringLength(maximumLength: 256)] + public string Identifier { get; set; } + + [Required] + [StringLength(maximumLength: 256)] + public string Value { get; set; } + + public virtual User User { get; set; } + } +} diff --git a/src/NuGetGallery.Core/Entities/EntitiesContext.cs b/src/NuGetGallery.Core/Entities/EntitiesContext.cs index a07a773775..60abfab867 100644 --- a/src/NuGetGallery.Core/Entities/EntitiesContext.cs +++ b/src/NuGetGallery.Core/Entities/EntitiesContext.cs @@ -55,6 +55,12 @@ public void DeleteOnCommit(T entity) where T : class #pragma warning disable 618 // TODO: remove Package.Authors completely once prodution services definitely no longer need it protected override void OnModelCreating(DbModelBuilder modelBuilder) { + modelBuilder.Entity() + .HasKey(c => c.Key) + .HasRequired(c => c.User) + .WithMany(u => u.Credentials) + .HasForeignKey(c => c.UserKey); + modelBuilder.Entity() .HasKey(r => r.Key) .HasMany(r => r.Licenses) diff --git a/src/NuGetGallery.Core/Entities/User.cs b/src/NuGetGallery.Core/Entities/User.cs index a20bb470e9..72ad48fa2a 100644 --- a/src/NuGetGallery.Core/Entities/User.cs +++ b/src/NuGetGallery.Core/Entities/User.cs @@ -17,6 +17,7 @@ public User( { HashedPassword = hashedPassword; Messages = new HashSet(); + Credentials = new List(); Username = username; } @@ -59,6 +60,8 @@ public bool Confirmed public DateTime? CreatedUtc { get; set; } + public virtual ICollection Credentials { get; set; } + public void ConfirmEmailAddress() { if (String.IsNullOrEmpty(UnconfirmedEmailAddress)) diff --git a/src/NuGetGallery.Core/NuGetGallery.Core.csproj b/src/NuGetGallery.Core/NuGetGallery.Core.csproj index 4c87fc786f..3e34de5abb 100644 --- a/src/NuGetGallery.Core/NuGetGallery.Core.csproj +++ b/src/NuGetGallery.Core/NuGetGallery.Core.csproj @@ -58,6 +58,7 @@ Properties\CommonAssemblyInfo.cs + diff --git a/src/NuGetGallery/App_Start/ContainerBindings.cs b/src/NuGetGallery/App_Start/ContainerBindings.cs index 360524c6d5..bddb1947ed 100644 --- a/src/NuGetGallery/App_Start/ContainerBindings.cs +++ b/src/NuGetGallery/App_Start/ContainerBindings.cs @@ -89,6 +89,10 @@ public override void Load() .To>() .InRequestScope(); + Bind>() + .To>() + .InRequestScope(); + Bind() .To() .InRequestScope(); diff --git a/src/NuGetGallery/Constants.cs b/src/NuGetGallery/Constants.cs index 4377d8e20a..b0f53ac447 100644 --- a/src/NuGetGallery/Constants.cs +++ b/src/NuGetGallery/Constants.cs @@ -32,6 +32,9 @@ public static class Constants public static readonly string ReturnUrlViewDataKey = "ReturnUrl"; + public const string UrlValidationRegEx = @"(https?):\/\/[^ ""]+$"; + public const string UrlValidationErrorMessage = "This doesn't appear to be a valid HTTP/HTTPS URL"; + public static class ContentNames { public static readonly string FrontPageAnnouncement = "FrontPage-Announcement"; @@ -43,7 +46,10 @@ public static class ContentNames public static readonly string PrivacyPolicy = "Privacy-Policy"; } - public const string UrlValidationRegEx = @"(https?):\/\/[^ ""]+$"; - public const string UrlValidationErrorMessage = "This doesn't appear to be a valid HTTP/HTTPS URL"; + public static class CredentialTypes + { + public static readonly string PasswordPbkdf2 = "password.pbkdf2"; + public static readonly string ApiKeyV1 = "apikey.v1"; + } } } \ No newline at end of file diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs index c3db3b01c7..1bbde704fd 100644 --- a/src/NuGetGallery/Controllers/ApiController.cs +++ b/src/NuGetGallery/Controllers/ApiController.cs @@ -158,7 +158,7 @@ public virtual ActionResult VerifyPackageKey(string apiKey, string id, string ve HttpStatusCode.BadRequest, String.Format(CultureInfo.CurrentCulture, Strings.InvalidApiKey, apiKey)); } - var user = UserService.FindByApiKey(parsedApiKey); + User user = GetUserByApiKey(apiKey); if (user == null) { return new HttpStatusCodeWithBodyResult( @@ -210,7 +210,7 @@ private async Task CreatePackageInternal(string apiKey) HttpStatusCode.BadRequest, String.Format(CultureInfo.CurrentCulture, Strings.InvalidApiKey, apiKey)); } - var user = UserService.FindByApiKey(parsedApiKey); + User user = GetUserByApiKey(apiKey); if (user == null) { return new HttpStatusCodeWithBodyResult( @@ -260,7 +260,7 @@ private async Task CreatePackageInternal(string apiKey) } } - return new HttpStatusCodeResult(201); + return new HttpStatusCodeResult(HttpStatusCode.Created); } [HttpDelete] @@ -275,7 +275,7 @@ public virtual ActionResult DeletePackage(string apiKey, string id, string versi HttpStatusCode.BadRequest, String.Format(CultureInfo.CurrentCulture, Strings.InvalidApiKey, apiKey)); } - var user = UserService.FindByApiKey(parsedApiKey); + User user = GetUserByApiKey(apiKey); if (user == null) { return new HttpStatusCodeWithBodyResult( @@ -312,7 +312,7 @@ public virtual ActionResult PublishPackage(string apiKey, string id, string vers HttpStatusCode.BadRequest, String.Format(CultureInfo.CurrentCulture, Strings.InvalidApiKey, apiKey)); } - var user = UserService.FindByApiKey(parsedApiKey); + User user = GetUserByApiKey(apiKey); if (user == null) { return new HttpStatusCodeWithBodyResult( @@ -447,6 +447,23 @@ public virtual async Task GetStatsDownloads(int? count) return new HttpStatusCodeResult(HttpStatusCode.NotFound); } + private User GetUserByApiKey(string apiKey) + { + var cred = UserService.AuthenticateCredential(Constants.CredentialTypes.ApiKeyV1, apiKey.ToLowerInvariant()); + User user; + if (cred == null) + { +#pragma warning disable 0618 + user = UserService.FindByApiKey(Guid.Parse(apiKey)); +#pragma warning restore 0618 + } + else + { + user = cred.User; + } + return user; + } + private static void QuietlyLogException(Exception e) { try diff --git a/src/NuGetGallery/Controllers/AuthenticationController.cs b/src/NuGetGallery/Controllers/AuthenticationController.cs index f74c300984..376ae80c0c 100644 --- a/src/NuGetGallery/Controllers/AuthenticationController.cs +++ b/src/NuGetGallery/Controllers/AuthenticationController.cs @@ -55,7 +55,7 @@ public virtual ActionResult LogOn(SignInRequest request, string returnUrl) { ModelState.AddModelError( String.Empty, - Strings.UserNotFound); + Strings.UsernameAndPasswordNotFound); return View(); } diff --git a/src/NuGetGallery/Controllers/UsersController.cs b/src/NuGetGallery/Controllers/UsersController.cs index 7a92cc1971..13dd82cded 100644 --- a/src/NuGetGallery/Controllers/UsersController.cs +++ b/src/NuGetGallery/Controllers/UsersController.cs @@ -39,10 +39,15 @@ public virtual ActionResult Account() { var user = UserService.FindByUsername(CurrentUser.Identity.Name); var curatedFeeds = CuratedFeedService.GetFeedsForManager(user.Key); + var apiCredential = user + .Credentials + .FirstOrDefault(c => c.Type == Constants.CredentialTypes.ApiKeyV1); return View( new AccountViewModel { - ApiKey = user.ApiKey.ToString(), + ApiKey = apiCredential == null ? + user.ApiKey.ToString() : + apiCredential.Value, CuratedFeeds = curatedFeeds.Select(cf => cf.Name) }); } @@ -193,7 +198,17 @@ public virtual ActionResult Packages() [HttpPost] public virtual ActionResult GenerateApiKey() { - UserService.GenerateApiKey(CurrentUser.Identity.Name); + // Get the user + var user = UserService.FindByUsername(CurrentUser.Identity.Name); + + // Generate an API Key + var apiKey = Guid.NewGuid(); + + // Set the existing API Key field + user.ApiKey = apiKey; + + // Add/Replace the API Key credential, and save to the database + UserService.ReplaceCredential(user, CredentialBuilder.CreateV1ApiKey(apiKey)); return RedirectToAction(MVC.Users.Account()); } diff --git a/src/NuGetGallery/Infrastructure/CredentialBuilder.cs b/src/NuGetGallery/Infrastructure/CredentialBuilder.cs new file mode 100644 index 0000000000..d2f735ae95 --- /dev/null +++ b/src/NuGetGallery/Infrastructure/CredentialBuilder.cs @@ -0,0 +1,33 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Web; + +namespace NuGetGallery +{ + /// + /// Provides helper methods to generate credentials. + /// + public static class CredentialBuilder + { + public static Credential CreateV1ApiKey() + { + return CreateV1ApiKey(Guid.NewGuid()); + } + + public static Credential CreateV1ApiKey(Guid apiKey) + { + var value = apiKey + .ToString() + .ToLowerInvariant(); + return new Credential(Constants.CredentialTypes.ApiKeyV1, value); + } + + public static Credential CreatePbkdf2Password(string plaintextPassword) + { + return new Credential( + Constants.CredentialTypes.PasswordPbkdf2, + CryptographyService.GenerateSaltedHash(plaintextPassword, Constants.PBKDF2HashAlgorithmId)); + } + } +} \ No newline at end of file diff --git a/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.Designer.cs b/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.Designer.cs new file mode 100644 index 0000000000..46059acde7 --- /dev/null +++ b/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.Designer.cs @@ -0,0 +1,27 @@ +// +namespace NuGetGallery.Migrations +{ + using System.Data.Entity.Migrations; + using System.Data.Entity.Migrations.Infrastructure; + using System.Resources; + + public sealed partial class CredentialsTable : IMigrationMetadata + { + private readonly ResourceManager Resources = new ResourceManager(typeof(CredentialsTable)); + + string IMigrationMetadata.Id + { + get { return "201309172217450_CredentialsTable"; } + } + + string IMigrationMetadata.Source + { + get { return null; } + } + + string IMigrationMetadata.Target + { + get { return Resources.GetString("Target"); } + } + } +} diff --git a/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.cs b/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.cs new file mode 100644 index 0000000000..bdbb8973b5 --- /dev/null +++ b/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.cs @@ -0,0 +1,39 @@ +namespace NuGetGallery.Migrations +{ + using System; + using System.Data.Entity.Migrations; + + public partial class CredentialsTable : DbMigration + { + public override void Up() + { + CreateTable( + "dbo.Credentials", + c => new + { + Key = c.Int(nullable: false, identity: true), + UserKey = c.Int(nullable: false), + Type = c.String(nullable: false, maxLength: 64), + Identifier = c.String(maxLength: 256), + Value = c.String(nullable: false, maxLength: 256), + }) + .PrimaryKey(t => t.Key) + .ForeignKey("dbo.Users", t => t.UserKey, cascadeDelete: true) + .Index(t => t.UserKey); + + CreateIndex( + "dbo.Credentials", + new[] { "Type", "Value" }, + unique: true, + name: "IX_Credentials_Type_Value"); + } + + public override void Down() + { + DropIndex("dbo.Credentials", new[] { "UserKey" }); + DropIndex("dbo.Credentials", "IX_Credentials_Type_Value"); + DropForeignKey("dbo.Credentials", "UserKey", "dbo.Users"); + DropTable("dbo.Credentials"); + } + } +} diff --git a/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.resx b/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.resx new file mode 100644 index 0000000000..dabe134f2a --- /dev/null +++ b/src/NuGetGallery/Migrations/201309172217450_CredentialsTable.resx @@ -0,0 +1,123 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + H4sIAAAAAAAEAO1d23LjOJJ934j9B4WedidiLLu6pqOnw54Jt+syjilXVZSq+tXBEmGZ2xSpJqkue39tH+aT5heW4BWXTFxIkJRUenFYAJgAEgcJIJGZ+Pf//evy70+bcPYHSdIgjq7mF2fn8xmJVrEfROur+S57+PNP87//7T//4/K1v3ma/VqX+4GWy7+M0qv5Y5Ztf14s0tUj2Xjp2SZYJXEaP2Rnq3iz8Px48eL8/K+Li4sFyUnMc1qz2eWnXZQFG1L8yH/exNGKbLOdF97FPgnTKj3PWRZUZ++9DUm33opczd/v3pLsrReGJHmez67DwMvbsCThw3y2ffnzl5QssySO1sutlwVe+Pl5S/L8By9MSdXin7cvTRt9/oI2euFFUZzl5OKoU6fnTXfyDr3OO54902YVnbqa3+wSLyP+G0J8tmBe9J/kmUvIkz4m8ZYk2fMn8lB9nheazxb8hwvxy+Y79iPahqv5bZT98GI+e78LQ+9rSBpW5bxcZnFC3pKIFA386GUZSfKxv/VJ0QepWqES+reuJR+THFDz2ZvgifjvSLTOHpua7rynOiX/dz77EgU5/vKPsmRH2JaVv4VK33t/BOtibITq77zIW+d4nc8+kbAokD4G2xIqZwzT79uCb5J48ykO+UFp8u+X8S5Z0S7FikKfvWRNMvNmfvRWv+Vf6pvZFoSbWecrm9kUgpp5uWjBqYRsPsmS48Lq9TZg6nm7C3ygGjWJ1xsvCK99PyFpagn7F3/50Rb2QuVfcpEdPQTJhviTtuMfXvpI2Z+m3+LEH736umLajOtwHSdB9rgZXgaJg5FPj8he+P34UlGvBQTDMP5GGt7/EudCwIu60bopQVWIpc/xbySy7BKdrU4G9BNJSbYnLXj9tA2SgiWvcllTt4f+/znYWNO/SQiVWF+ylTUlfOnLpz66plB03rcl2sWEy5BWET7XdpWjNBTNqbKFthSpcEPKLNtW5Lwu1oMcJfBy2+TfF0scu9LyWU3VzSIr5Net7rS+FnOv4vVxrbO/xP7z6PKYjiIdFFUPTegs8z72FavL3df/IatsdB58jnUcgJuPzqWaqeBEYgF835ZspxNYQJrqcCnbWV92vY8sFCc7LCk7TXVK8Lim+MTHPjo2HVcacJj5RajTGLdrw3GNdDeJIs7O/NvRN8plDx8CKhZGPqL86oU72w6ra7UU3KgwtNj9SCoGZHfUbbrsKjgWqorjmjKMLqb3zKkY9ImsgzQrDyS9aV7vspge91ZeGD5Xje275bmNVuHO70/nfZwRW42Gy5WFVdQ6U9ZJ5wiVRs9Sp8giQ9Xiqvg9+JnUfkVpTPWo+qSXqIBafFTy4tZWh3bx4qe+i+Or+FsUxp5/E+/a807f48KHbxG2L4NQURdv0YeXklCnKOpUOw/VA018VTmjxrvR2h/lgjrEIngTb5+TYP04/im9UgaqNIFmU5ikqyTYlhJ85D7kE4V4KRlptXYou0QF+BNFohdWCPuShKP3ps81xsW5g8pHOKeIC94qjqZg9W36Lp9saW/9Xk1nmdGP+lJ756XZl63vRCZQWq/9QEPKhFfvghWJ0mlmROCTqvpPZBsnvcfrnRetd3RVtER639n1cfc1DOhdae9xreTTmyAky+B/CSP0qJ7EllgSU/X0FEP7ify+CxKSVsN7vaIWQV606j2JlrvNxkvGv3b47K3HXwA/B1loDeb+Sq3SLmx0PV6FlcI6bHRWc3JoihnzLt/yOlDPpB8TkpSbtr603oR0/x4R/3qXPcbJ+GPSNOAV2ZIoP0OsggmQcRdEN2GQH2C6zYuXqnlhaoBifdnZW5vk5Dgs6sOMzs6mPalPB0tqzJlmwUp5rr+HikvdAEphZ3qoqK1CoplYinY3ZeTGVlloC+t822bx003FU64gwE0mH+cjW8i2qZzYVjaWK1mjDmg0WA5DMlzYFsnL3ZZ+T/w3Sf77W5z8puxJU0rRC6kM1gO5oG3r0SuomqB4/8Smo6joZBxQfUwPRkoO0gIK5rHZGN+4Mh1V+v/IxUacaCZZWehZ0VyhBNZisVivW/CKCCNMj1ER2Vv5ePuxm+2uG7PV6zVj4zRazR+2pN5X2Om4VHcdRurCaiHJmqni8LLFSA1QnrepwfmUV5tN94fYDSHCRbVx6iNgyi3MSbhAdCa2zTKBmcHmFQOUuLntg6Jmi/l8QhJEx/HFtIWSabklK8u6+yu4Ssg1O9/h6+8nqm0OWah4hk5ifaYUr7c/zSqAjpkziJnWm/y+I4z2vFuDuio3HdwE3sSbjf1m1OVUqwBroSNov9AoCeqC2GEWKd3xdDucksOo+eIp04EQOS7x0WFb1sXOC/fOstWF2ePcSBsmzYo+SGkX66PCykHtRLrpHRGhgiso+8CEauWOCyGuNiNu3DvyvUuaeZtt7w3N5yQgLuy6CjuYJIltXT8O9W5+qqvY6awop7R+nMp+bUKbrElthia0NT0ZLI2uHbS5hUN2EeBNnauLy4I4cnnZ5Clb1t99jr+7O21tIDouogE43Nmc9gWnfcFpX3DaF3zX+4Kh3U/23uHGqSG/S8+RHr4KfTZ79mZMyMYKs3Zyteur6SMbPzZb18T+278qMuqSZBlF7VFt/1iLkZQi/Hq9Tsia0mrvo7tt5uLMCzHnQX76mZB7T54y0E1pIPFpezgovKHpOkU93o4KIkP4A78n3wqG9SZUcTxHgBt6bHjGmxwqk2x18g7p4iBarg01v1Uyl0XwffuBJHzBcpgUhgtbxzrkR9m4F9J36s4IxY36JH5js9Jcp2m8CoqOVB2Fo7zxXHod+TOjkG8lcMXgcTled2EWbMNglTfpav4naRh0FTQXj20F5TrNEz4/O7uQaOcCjyRlXKN8qlF5EkSZLB2DaBVsvdCkGcLHpsKVjkpTj5jTGLGaMNqkAVy8RLkhTX2C/Ndx63LBQEiNLD4uHzbgSMBS9UBfiCy4/BC9yo+LGZldr7IiuP2Nl648XxZY+cTwzdoCgM4O1Z2QB/JjDMSBnTepmIlKOR3OypCAyoEVItGqEaaSUVC0wZZaEQxSTc2ic2I8N6xNaHC3tmFMBEOLzqIxc0ecoZ0mEtLwMaYSMhgmVU87lcAXFVBkKJ9XYHDHhlyzAJ7qYQa72WvBAchVtYpjhTUU/wTiBuiIa84VRWVj8QZyItE0V+lfK/FmpNXexNtFahvrODPI+q/n2BgSTM8bk1awN32TyDPRH0UHBcmzempsSt4yUoMqT6lhwSjwZUwEChw4KNhxLhxaMQQ6zU8NQNjLRGoV42k1sFgEuDSqQAT4cUiYREzANTDQ+T1IeBD05tb7G50BOlah880O7Flh1XrpYmtgbknxAMaVIX2mt7IjI05z5QAe0nSXbfc1UFIY8kswYvwXrCGrCFSihaus5u2wNiu1KWwhi7XYoN9WShR3+myo9jGXzcNSg0CGqppxha1WJcQUjir2qIEj0xyOXIfaPyL8oME5JCneWiSb4EQjtPpA0E4D7FJySdWPDZ+DlGCi9ZVmjFFTLAlEtWG6PY7QqFWHI82QLoyISGSgDkmmcfZ2hpjRSLb+oJx0cwY1YgJMHaSgg+OVakZd85aH1Z3NsGo4TZjVDgeTPkBV8m1ExCq5YiELJTvDKe9ea5EOYQ5Dic2jTtK9bIezrMWLUBPOom5WA8ZdGwPo5gN7qHDnH1rTAA5+UM3M1GAgAa1+D677ZOsDXpBNI8IVZIZJ/eLDilPuKGDbZM06rTFUlmQhZ8Jvv21VmzrvvYWWSS9G3E4oB8+kHZyLwd5gV7RIt8EUap4+KJIxA/eDBDTSmalwjYyoSXMgx5cRUF46MuTfZPkXJKndFmhqQFKaTp5E/6fymyXJ5IUlnc9a1who2yBBlSdVPU0u0SgRqfmYtR+HiPD25RpipcmzTKQ0QNZ83JqoggxhTIV1hLiNjYK7zdZHQxDYpUJUwdOEGWkFOVMSrBkgRostY0a1ieaAkayNvczI8RFkMZqs6Y4ZXfHxDYyyYNZgRVxP1pQg+7gGRpK5yTYjWj01gdEr71jMSDGvQWDkGsWmhiLvNgwRFByLDZvIriCKZvJ7AIE0I91hedg6n82YsoBgRLzUuG2F2k+t6aYkkaX1zNQzjSFZrRDiFoNngAFzeP8pgCkKByuu5bCLla7FChJAp5V87Nr5aoVDeg64/Mht5p1+7PvMu/kw31dt691R0cEH6K3SB4jXRiBeQEy7uYVf0XvM72cAoINeJxAftN4pqGpG8k9hOcJtDlUsUXmkDMAXSN1Xv6kuc0dRGu8S/hHEKXhrpuCYgvzwfAMfO0H5pvVqgTqm8muR+WbEK5U7ikxSscfszrdmG4ozC3SoALsjulR0ZYvoCSHTaZrtDkDcBloBHdTOHx5hyNK/M1wgA32ZGN8VVxzCAoqjrDIxP4e6qTFAl/srnlH0nNSYnKNVDMbO5khqyE3QyEffU9HYxz0vRVMfA6h35yEQrRzln8Y6GuoZbh8t94o9eep5hltED8ovdNfJ5evlELbftBJmQ+80meO5ATxw+1+o8aAFsMyISnOgZwZo8zsoFJjAt2qemEFCNkbty42R4CHFrEO5obSvhLqBWVjKnGHUQnruYEaVg8KFj5in5ZEZaEBTP0fcGQk+yNvZZie2przdoUq87Xd+amsqGBZaBkZQuGJA9ZX2VK/4WKE0MFrdLMylLIetp7JFBU69fQ6qG1GBsZOyRQU+zUB0n8ZIcDp0GhsYjNibjMh4EHTx+mmsNhIZTgyqw+KZsVFpu9DZesE1UzF7hR68reP3NdfeTd7lYrl6JBuvSrhc5EVobOqdF97FPgnTOuPO227pLVD7ZZUyW269FZ1hf17OZ0+bMEqv5o9Ztv15sUgL0unZJlglcRo/ZGereLPw/Hjx4vz8r4uLi8WmpLHgtWPiJX1TU77u0s0bn0tjXvrkTZAUQSe9rx69zLvxN1Ixw0v+ujbgrl8eyPrqqv6I/l+Z7+zekqy6IONMAmTjiOrj/Mi2ps8bFv0ksJSTv86/X6680Eug0Ko3cbjbRAr7D/x7+pcnUKbIFC4XQgckKw6JTwJwRd4bjUw5CToPCbSBNBgL+LOhBuF6G0gk6jRzKsVtWvOEPEuLzzGn+CValXFmiY8TRwuZ10ODr9PVOE2/xYnPkxfzzKnW3wih3VniSBELDuU4iaQp1Kbajl4Yxt+ID41enWNJkQ0U/Dn+jUQAbaCMPZc/kZRkQA1Qfh/qr5+2QbmxLWMPq+sSS5vXzD7Iy9bBpu+NkOQv0jsLS86SzF5oqj8fSnj+EvsCgTLFnAIX9ZYlpAyHi9NbEhpEniVUplhQ2H2lD6gIROpEczpMlFWWkiL46mQQxgwzDKFb2C/aQxb+7LvdbLF2Ht13wa0daIdNsOLjoYYFnCMd5j3lhzDRihRzCuVbBw8BPeyydNh0c2q/euFOaFCVtD+AExQxfY9eiE7d/PSFEhgKfKL7ErfX0Lg24VQBPaBEHStjcW7ZZTHdNuZ54XPVWOEUA5awmBDRKtz5ItU21UL2li+CccK3TNqbyQAqbzvPCAOFtsG0MKIy1Ny4FcfdasSFR31YQkLWviGg/6h3H+nxRnccGcU8OsmJ1jbZ+kwIHgitkMk+R8nhks0wp8c/esgS5HPGmTuoPuKJPujkhdWYFq9DcroIIN9Oh4RoeTprd8qXCEVSVhKsfvyTE2N1ogWd9F0OslQYijbVntIyoy80wfTqPHOq3KODLEkuw44etd6AyNXpFtSYp1A5aky6BSoCX/RS4iAiZ9v0O1rvCvMRvtd1qoV0bZ9u5ARqm2wtqdv3KQER3WZa0GXeieVIMuk2UhB9fZUXiWgxGwVN9SArr6CpEi2OjMUDq9yRsUixoFA+Ic2RKJMsDookSaW1qEm0nmf0RwrOtCrHmmI5kbD5y+TaUE5l6VKl2UjTjwlJykVWlKVsjoUyMqQPH0bEbyzTOY2klNuBMm/WDZJXWX6r6rgLopswyLeXIKTk3OGUNNPu4JX+IHZbeZyU+aZeRWPg7T22pbc9Cn4ELx+ZZDsoXa8lJT2TbE7rw5bUJmYsLSbZ5lhQRUFobEj5s4GYa73Qvt0FfgoutVXOvk0j3GPIbg5VLvCd5w/2/WHMHfp3r68dIHejvkPORCjoPOwqGocx9P00aNUyvdySFbg3LDNsdrvUxo2Jm8BvfIXMfYOn4NHUF6D8GbEzRjVkDgOm3Ywt8Pv333dEOve1qTZHSvDc0enAcRNvNtKeo0ncU7A7g3lvgJ9u6cUhYjwF+w5SK3U7D5OCxFADddjLSenF1nfkiqBBnQcN/vow1gxnVhvBhqSZt9mKSrQm2YJWEhDozoRNt1SCJ0ksGIMwyeOqCUGNVAdFlMsrOddXaa4ubNxeObhWlLu+Ljxu1fu060Trtdp3ragjwnVeLlACpxWj84pxkstG9E5y+SSXjS/oe5ttDGFMMtQlumvTiy6GAxOtkGLo0s4LpBDh1H6B1BEYaoFk476ldMCv1+uErOmYi6pfTVEbp5LMCxUGYlC+hfaFPGUK6xoge2/gCPqF9920cfFxO+/c1FQG3r4NbOHJvaDAY0XxtAJODwpZL66WupD2OHXW2ZJ6tIt7LTHXut2yWySXMf5s4eMVyN7ddcRehQd3XQRy04ZcuGjsBWDzzwf2lRlhNgkUb9cx1Vq1CH3fwaxFlIhli8QwEtYjB8ff1QdGYAorAiBgcZ9w5xkpWm9XZurfE7IaZVX83xER2Hu8oQBEdVxhcw+U5hMDPxNDDOC19EWC6dJkiQhFgw8XF1hQV4s7cOYrk7tu9fUgznd1VNjOIrgg5xAe6siyXZtZ0ZkIJECYVcN7yfYD2+tHnMNo7NSe8uLeIQrQyLFd22jftnxL6gfFc2G36ftdGF7NH7xQtHZWdF0PHilulVik2aFWKc3vJm5VFTOKC2ZV8ISGpip4kVbxq8QgUmWR+Szv/h+BTwNILZ/TjGzOaIGz5e9habHcFsgX8eAh304X8UWu5i/Oz3+az67DwEvLmGT24bGIv1mkqc95vDPnRHkjIcS0ypkuDlo9mMp3wS4X4peXEHzKN1sCyoNi8r0lEal8pTPqsVW7p2d5WQoQ6jvUgGShpE//1hVEf3jJ6tFL/mvjPf03SylLRJd19kSi5BnwTvjhMquOCFVWsYuC/FQXMIEBLMnxwZr4MZjP7ryndyRaZ485wv/yo3o4ZNpoTCi31YixodxSR4JDWaMVYE8TH0rR4h9fdhzSOlJUSfsrfVSqCx0gKpSitT+86MpeNjDUwBWI0aDK6qiGPAvoaNjRZ40TbSgZSy88ntIBS7EyOJODScQFZ2IabEeljMzUfao0QZkc9IgJywT2B2iQMZZkLdkBY2jobQMWhOiAWWaNLACeRTAjtysWG9jI7epdRToyJ9pndqmCBx0waERVaHfsYKq07hThyELdBXkbU6g7jco+YzjBpI3Gc8Bgu1Vv4C9e/GRNUriEdr6oHtt8dz9LGdsxBxukJtQOvt02wwVrg+agXbx5lgOCHYELnOSAmDoO2qc8EvPT9rwL7Z7rNiBcaktBB51vo+70WG2ESDvdKXFWXj3nBRtdp9/ZmDWndIE3OaZOH47V0XRUILPGLWMg13MYJGPAuq/rTlsvxhbViaxDbUX7nKIrM1EXp+jCWtQFodIO2/GxpI584vYcxcfXcdB5ObSOE6Jp3106F1WnOx05jo4LjRQcPccBZTlwjgI+L1/akrdVo9lukLHwM4e/Ve65PWbC1zi6VqgC2Dig9mHbhLBR7e9+sqUrx7ExP/UZrcNcRJvBz+JQaJjvHtdDK2g18Vm+e/7b6VIs9i1lxBe3WyLJU78PeVsIKcKnfPcoMrvgNNvgN8FXujcH3Yv21g40UVgmQN7RYA6S+ZYaXFsWIkZ/B8xEp7IQlQv3Q2725ZgiBzwebgSpi8vX1rm8pyhmY5F0bw8ThGRfVS0uj/Zu7zFc3z+41G471tw61z46vmk5VnWmrdQGQ3ucBHdv+zKHcvskI08y8iQjwanh6srH9u556rtytzeFLq+Su96GGq9bqogbB7xuaaJ0dF+HgKAcGFhM6AFxOAbfo+BBLA54wN3bXHFhL/qo2+QwF30MwcTAFo4WpiawxTBiBo4wYYY33B1bLouEcNDj04FuoanblIStXTQc0cGMidpwDMZ81/PSnb3zcI4cRoESzFhrHN/AIYvdi7vBWW0cdcCM66pIAXJp3GFfz2umph6KPqYBfTnMOIJX1FkXt/vao0vg6uvIn1ERVbnzVq2grtZnZcLdLsyCbRis8iqv5udnZxdSp1oanFMdS4vP4Gn+SSKYjxBJSh+hfGmjUA7kwEkfkyBaBVsvZNsvFDLdqVCuNvTEnMamAuqkSYWcM51ccUNfQKCOC5zvvxoJRTSkqsmimZAlAi6kGBAfolf5oTgjs+sVrTKX9F668uTwWkUghBNybJDDOC1OgpvWV+++v+wYCDmMPyHbAjb5sFGDOUwi1U2MGM5Z717vU9UOJFSWHVEwfxyI8R6IHMyErEGgZvba/ADIU3heIlWah/UaD4xFeDj49Xd5kIsQMMAIl+nfBdzQYDj7AzN9HMER4FULucG31RAShoXAaIubzaBPu7JBkQW1QmV/17TxMTXVKnawy1ctX9hrC9DTo9OoDosrpq1AW9jcIbE2Jr6UT0mjSJseXeAzwPsIqcoZBGhHnXMsUEJfVN5jGOHvC+8jlhjnFqAtbO6xYEr5ZPP+4oqP1AvHBNlDfIG392xzhALHgjLdu8v7CzQ5GrIDkJmd6ySPBJYkk3ksIFE9iIsCBAv8PB5CqO/F4UigwlMEaEWZfixQQl7o3V8xU4DIxV2KVqpMAICxtUbGo78XmqPKC+BwREjttgA0pMk6FkGCP9+6v7KkBtRY+ufJ8DC2XLEBw16IFtZ49742Ud3L23rI2hjAE59/FKDSvDUo16t8YW8ydAmWyyeQHTbITN5bHM9EDX9CcE+Q1Visi/WXiYeNIezBxz1c9zTPOzIjJo0WNFJHhBbjARwVLYpHNEe2A1K+ZLnP5kCNnwfSGkgKHrZBEPZWKVjnXlgEGb9HuifL2fTQGs+KtgOm9uJsZ/r26iFZHUkOVprWDQjAia2SdG/yqhRQe2WlZPri7p5Ivv0D4thHxU7I2wuJaPHIsOub+RGtBgRnSF1LxeJDCkv7W/4hzAtU3qJIM5QPSU8OYI24NAbECbQK0B4SXNUPijvH62v+oegmuITwqLME0Oq5cPkMNJ+1rsrQ8bl8Hvpq7n+Nc6iUHs9MgRS4kuLrKpdjqZIyGaJOc/RkeZ9TiTyfDVXDltBXV2qDpGrKZIh8ofjSkmU9IOVxYDLBYWjyDSoSnJKwQW8KKMa98ajQVQqeYaSawVJQ9UBB4zbg9SrrMqbPWstjNbFlFHWyxcwqr22rsYrrfEWlTRw8sxpZ61usVraMomb+rRCz6oX9H9YCoZiiEVxJ22ZoG2BStXGljLEhVi1TRFFxU8q46tIsCau1zFVUSAsY19VYK2DVNQUUNZZlTIAlRFOTaxULQLXyZYy7yt9xYv3lSyk6zRY0W56Rxa3NwpZpw0VO1FKqdhz4voApVOttu6xASBPQkoYrUalO6STALKUJ/JGpcEPFDbPhhHdZbXiaGVMW2G8hcWwAvVLDnTpB2kFj0UeYL/kM8XDEd8ugy3wcFqCrikAtB9JFMWQI0EllVJGe3ZQ3vcV3bHL/LupjXEC9toyModfoM/0D81VsArfsJauELFfs4qMw4AxSRGvAL2jl9pfpe8KCetCxGcHlu5wOUO+cdwt2ice7aeBCPyj4R2EK5LWtGHmdj3fXDkDfyedJloLiiNidGc0REOcA6Ifsstv8SZb9us5xN/bcuVMx6qjTrMuOywdqlgKb64oBsNcmzggDL0+XDAGP+CwRoYArtsg+hjhLNP6ILtkhnfpZAkymKzZwjnQ4B3B/O5edZ/UO7LdlutMua1Z/xDXMzQ5gnG6KLk54Z5XOUC7HV9D1sJ83Wa67rxlo3G/HzViP2WXY8wTvu4GnihsmQIovlhN8/iDsEF0lDLmi9LA4SObItv2Y6gN3AOjZcVEV2XxVJrrsYnWNr+4idNfPNVhqLNTQUbuosqLWHOeVRtdDnOpFfbBIA8KLK5bg429sHtxXCTYqE6BTPW+naqgKUBi3jqEPwBT5GM3h2YcjSf/RAKvGtAxSW/2Zni2V1oLuz4paivD9DEpfLD4wc7UAtLBkU7BC0e29ZWj9DENjLtXkXS7K+7IqIf+Zb7nzOu5in4RpkXq5+LSL6Es65a9XJA3WLYnLnGZEVpx5VlPmNnqIa0sxoUV1EeGRiTuSeb6XeddJFjx4qyzPXpE0LW6af/XCXXE59JX4t9GHXbbdZXmXyeZryB0SqLWZqv7LhdTmyw/F+4Kpiy7kzQzo40Mfol92Qeg37X4DPI+BkKBmbNXLUnQsqYgi6+eG0vs4MiRUsa+xvvtMNtswJ5Z+iJbeHwRvm56HPMcuXwXeOvE2aUWj/T7/mcPP3zz97f8BkWB7UmuYAQA= + + \ No newline at end of file diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj index 90641ef87c..6fc3a29464 100644 --- a/src/NuGetGallery/NuGetGallery.csproj +++ b/src/NuGetGallery/NuGetGallery.csproj @@ -592,6 +592,7 @@ + @@ -803,6 +804,10 @@ 201309092041546_AddPackageLicenseReportSproc.cs + + + 201309172217450_CredentialsTable.cs + @@ -1260,6 +1265,9 @@ 201309092041546_AddPackageLicenseReportSproc.cs + + 201309172217450_CredentialsTable.cs + PublicResXFileCodeGenerator Strings.Designer.cs diff --git a/src/NuGetGallery/Services/IUserService.cs b/src/NuGetGallery/Services/IUserService.cs index c4105cab24..82384eb438 100644 --- a/src/NuGetGallery/Services/IUserService.cs +++ b/src/NuGetGallery/Services/IUserService.cs @@ -9,6 +9,7 @@ public interface IUserService void UpdateProfile(User user, string emailAddress, bool emailAllowed); + [Obsolete("Use AuthenticateCredential instead")] User FindByApiKey(Guid apiKey); User FindByEmailAddress(string emailAddress); @@ -21,6 +22,7 @@ public interface IUserService User FindByUsernameOrEmailAddressAndPassword(string usernameOrEmail, string password); + [Obsolete("Use ReplaceCredential instead")] string GenerateApiKey(string username); bool ConfirmEmailAddress(User user, string token); @@ -30,5 +32,36 @@ public interface IUserService User GeneratePasswordResetToken(string usernameOrEmail, int tokenExpirationMinutes); bool ResetPasswordWithToken(string username, string token, string newPassword); + + /// + /// Gets an authenticated credential, that is it returns a credential IF AND ONLY IF + /// one exists with exactly the specified type and value. + /// + /// The type of the credential, see + /// The value of the credential (such as an OAuth ID, API Key, etc.) + /// + /// null if there is no credential matching the request, or a + /// object WITH the associated object eagerly loaded if there is + /// a matching credential + /// + Credential AuthenticateCredential(string type, string value); + + /// + /// Creates a new credential for the specified user, overwriting the + /// previous credential of the same type, if any. Immediately saves + /// changes to the database. + /// + /// The name of the user to create a credential for + /// The credential to create + void ReplaceCredential(string userName, Credential credential); + + /// + /// Creates a new credential for the specified user, overwriting the + /// previous credential of the same type, if any. Immediately saves + /// changes to the database. + /// + /// The user object to create a credential for + /// The credential to create + void ReplaceCredential(User user, Credential credential); } } \ No newline at end of file diff --git a/src/NuGetGallery/Services/UserService.cs b/src/NuGetGallery/Services/UserService.cs index 4a0d51db84..018907a500 100644 --- a/src/NuGetGallery/Services/UserService.cs +++ b/src/NuGetGallery/Services/UserService.cs @@ -11,15 +11,19 @@ public class UserService : IUserService { public IAppConfiguration Config { get; protected set; } public IEntityRepository UserRepository { get; protected set; } + public IEntityRepository CredentialRepository { get; protected set; } - protected UserService() {} + protected UserService() { } public UserService( IAppConfiguration config, - IEntityRepository userRepository) : this() + IEntityRepository userRepository, + IEntityRepository credentialRepository) + : this() { Config = config; UserRepository = userRepository; + CredentialRepository = credentialRepository; } public virtual User Create( @@ -44,11 +48,12 @@ public virtual User Create( var hashedPassword = Crypto.GenerateSaltedHash(password, Constants.PBKDF2HashAlgorithmId); + var apiKey = Guid.NewGuid(); var newUser = new User( username, hashedPassword) { - ApiKey = Guid.NewGuid(), + ApiKey = apiKey, EmailAllowed = true, UnconfirmedEmailAddress = emailAddress, EmailConfirmationToken = Crypto.GenerateToken(), @@ -56,6 +61,10 @@ public virtual User Create( CreatedUtc = DateTime.UtcNow }; + // Add a credential for the password and the API Key + newUser.Credentials.Add(CredentialBuilder.CreateV1ApiKey(apiKey)); + newUser.Credentials.Add(new Credential(Constants.CredentialTypes.PasswordPbkdf2, newUser.HashedPassword)); + if (!Config.ConfirmEmailAddresses) { newUser.ConfirmEmailAddress(); @@ -89,6 +98,7 @@ public void UpdateProfile(User user, string emailAddress, bool emailAllowed) UserRepository.CommitChanges(); } + [Obsolete("Use AuthenticateCredential instead")] public User FindByApiKey(Guid apiKey) { return UserRepository.GetAll().SingleOrDefault(u => u.ApiKey == apiKey); @@ -115,54 +125,28 @@ public virtual User FindByUsername(string username) { return UserRepository.GetAll() .Include(u => u.Roles) + .Include(u => u.Credentials) .SingleOrDefault(u => u.Username == username); } public virtual User FindByUsernameAndPassword(string username, string password) { - // TODO: validate input - var user = FindByUsername(username); - if (user == null) - { - return null; - } - - if (!Crypto.ValidateSaltedHash(user.HashedPassword, password, user.PasswordHashAlgorithm)) - { - return null; - } - - return user; + return AuthenticatePassword(password, user); } public virtual User FindByUsernameOrEmailAddressAndPassword(string usernameOrEmail, string password) { - // TODO: validate input - - var user = FindByUsername(usernameOrEmail) - ?? FindByEmailAddress(usernameOrEmail); - - if (user == null) - { - return null; - } - - if (!Crypto.ValidateSaltedHash(user.HashedPassword, password, user.PasswordHashAlgorithm)) - { - return null; - } - else if (!user.PasswordHashAlgorithm.Equals(Constants.PBKDF2HashAlgorithmId, StringComparison.OrdinalIgnoreCase)) - { - // If the user can be authenticated and they are using an older password algorithm, migrate them to the current one. - ChangePasswordInternal(user, password); - UserRepository.CommitChanges(); - } + var user = UserRepository.GetAll() + .Include(u => u.Roles) + .Include(u => u.Credentials) + .SingleOrDefault(u => u.Username == usernameOrEmail || u.EmailAddress == usernameOrEmail); - return user; + return AuthenticatePassword(password, user); } + [Obsolete("Use ReplaceCredential instead")] public string GenerateApiKey(string username) { var user = FindByUsername(username); @@ -276,11 +260,88 @@ public bool ResetPasswordWithToken(string username, string token, string newPass return false; } - private static void ChangePasswordInternal(User user, string newPassword) + public Credential AuthenticateCredential(string type, string value) + { + // Search for the cred + return CredentialRepository + .GetAll() + .Include(c => c.User) + .SingleOrDefault(c => c.Type == type && c.Value == value); + } + + public void ReplaceCredential(string userName, Credential credential) { - var hashedPassword = Crypto.GenerateSaltedHash(newPassword, Constants.PBKDF2HashAlgorithmId); + var user = UserRepository + .GetAll() + .Include(u => u.Credentials) + .SingleOrDefault(u => u.Username == userName); + if (user == null) + { + throw new InvalidOperationException(Strings.UserNotFound); + } + ReplaceCredential(user, credential); + } + + public void ReplaceCredential(User user, Credential credential) + { + ReplaceCredentialInternal(user, credential); + UserRepository.CommitChanges(); + } + + private static User AuthenticatePassword(string password, User user) + { + if (user == null) + { + return null; + } + + // Check for a credential + var cred = user.Credentials + .FirstOrDefault(c => String.Equals( + c.Type, + Constants.CredentialTypes.PasswordPbkdf2, + StringComparison.OrdinalIgnoreCase)); + + bool valid; + if (cred != null) + { + valid = Crypto.ValidateSaltedHash( + cred.Value, + password, + Constants.PBKDF2HashAlgorithmId); + } + else + { + valid = Crypto.ValidateSaltedHash( + user.HashedPassword, + password, + user.PasswordHashAlgorithm); + } + + return valid ? user : null; + } + + private void ChangePasswordInternal(User user, string newPassword) + { + var cred = CredentialBuilder.CreatePbkdf2Password(newPassword); user.PasswordHashAlgorithm = Constants.PBKDF2HashAlgorithmId; - user.HashedPassword = hashedPassword; + user.HashedPassword = cred.Value; + ReplaceCredentialInternal(user, cred); + } + + private void ReplaceCredentialInternal(User user, Credential credential) + { + // Find the credentials we're replacing, if any + var creds = user.Credentials + .Where(cred => cred.Type == credential.Type) + .ToList(); + foreach (var cred in creds) + { + user.Credentials.Remove(cred); + CredentialRepository.DeleteOnCommit(cred); + } + + user.Credentials.Add(credential); } } } diff --git a/src/NuGetGallery/Strings.Designer.cs b/src/NuGetGallery/Strings.Designer.cs index 4dfa7852a1..9981a8eb99 100644 --- a/src/NuGetGallery/Strings.Designer.cs +++ b/src/NuGetGallery/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.18033 +// Runtime Version:4.0.30319.33440 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -213,6 +213,15 @@ public static string UserIsNotYetConfirmed { } } + /// + /// Looks up a localized string similar to A user with the provided user name and password does not exist.. + /// + public static string UsernameAndPasswordNotFound { + get { + return ResourceManager.GetString("UsernameAndPasswordNotFound", resourceCulture); + } + } + /// /// Looks up a localized string similar to The username '{0}' is not available.. /// @@ -223,7 +232,7 @@ public static string UsernameNotAvailable { } /// - /// Looks up a localized string similar to A user with the provided user name and password does not exist.. + /// Looks up a localized string similar to A user with the provided user name does not exist.. /// public static string UserNotFound { get { diff --git a/src/NuGetGallery/Strings.resx b/src/NuGetGallery/Strings.resx index af869c07bb..f2ff44bc46 100644 --- a/src/NuGetGallery/Strings.resx +++ b/src/NuGetGallery/Strings.resx @@ -123,7 +123,7 @@ The username '{0}' is not available. - + A user with the provided user name and password does not exist. @@ -174,4 +174,7 @@ '{0}' cannot be null or an empty string + + A user with the provided user name does not exist. + \ No newline at end of file diff --git a/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj b/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj index c030a9e984..0331756346 100644 --- a/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj +++ b/tests/NuGetGallery.Core.Facts/NuGetGallery.Core.Facts.csproj @@ -94,6 +94,9 @@ Designer + + + diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs index 45bcdf46b1..3c90c41fe1 100644 --- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs @@ -62,14 +62,6 @@ public class ApiControllerFacts private static readonly Uri HttpRequestUrl = new Uri("http://nuget.org/api/v2/something"); private static readonly Uri HttpsRequestUrl = new Uri("https://nuget.org/api/v2/something"); - private static void AssertStatusCodeResult(ActionResult result, int statusCode, string statusDesc) - { - Assert.IsType(result); - var httpStatus = (HttpStatusCodeWithBodyResult)result; - Assert.Equal(statusCode, httpStatus.StatusCode); - Assert.Equal(statusDesc, httpStatus.StatusDescription); - } - public class TheCreatePackageAction { [Fact] @@ -163,21 +155,82 @@ public async Task WillReturnConflictIfAPackageWithTheIdAndSemanticVersionAlready } [Fact] - public void WillFindTheUserThatMatchesTheApiKey() + public async Task WillFindUserUsingFindByApiKey() { var nuGetPackage = new Mock(); nuGetPackage.Setup(x => x.Metadata.Id).Returns("theId"); nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42")); + var user = new User(); + var apiKey = Guid.NewGuid(); + var controller = new TestableApiController(); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(new User()); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(user); controller.SetupPackageFromInputStream(nuGetPackage); + ResultAssert.IsStatusCode( + await controller.CreatePackagePut(apiKey.ToString()), + HttpStatusCode.Created); + + controller.MockPackageService.Verify(p => + p.CreatePackage(nuGetPackage.Object, user, true)); + } + + [Fact] + public async Task WillFindUserUsingAuthenticateCredential() + { + var nuGetPackage = new Mock(); + nuGetPackage.Setup(x => x.Metadata.Id).Returns("theId"); + nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42")); + + var user = new User(); var apiKey = Guid.NewGuid(); - controller.CreatePackagePut(apiKey.ToString()); + var controller = new TestableApiController(); + controller.MockUserService.Setup( + x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(new Credential() { User = user }); + controller.SetupPackageFromInputStream(nuGetPackage); + + ResultAssert.IsStatusCode( + await controller.CreatePackagePut(apiKey.ToString().ToUpperInvariant()), + HttpStatusCode.Created); - controller.MockUserService.Verify(x => x.FindByApiKey(apiKey)); + controller.MockPackageService.Verify(p => + p.CreatePackage(nuGetPackage.Object, user, true)); + } + + [Fact] + public async Task WillUseUserFoundByAuthenticateCredentialOverFindByApiKey() + { + var nuGetPackage = new Mock(); + nuGetPackage.Setup(x => x.Metadata.Id).Returns("theId"); + nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42")); + + var correctUser = new User(); + var incorrectUser = new User(); + var apiKey = Guid.NewGuid(); + + var controller = new TestableApiController(); + controller.MockUserService + .Setup(x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(new Credential() { User = correctUser }); + controller.MockUserService + .Setup(x => x.FindByApiKey(apiKey)) + .Returns(incorrectUser); + + controller.SetupPackageFromInputStream(nuGetPackage); + + ResultAssert.IsStatusCode( + await controller.CreatePackagePut(apiKey.ToString().ToUpperInvariant()), + HttpStatusCode.Created); + + controller.MockPackageService.Verify(p => + p.CreatePackage(nuGetPackage.Object, correctUser, true)); } [Fact] @@ -266,59 +319,46 @@ public class TheDeletePackageAction public void WillThrowIfTheApiKeyIsAnInvalidGuid(string guidValue) { var controller = new TestableApiController(); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns((User)null); - + + // Act var result = controller.DeletePackage(guidValue, "theId", "1.0.42"); - - Assert.IsType(result); - AssertStatusCodeResult(result, 400, String.Format("The API key '{0}' is invalid.", guidValue)); + + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.BadRequest, + String.Format("The API key '{0}' is invalid.", guidValue)); } [Fact] public void WillThrowIfTheApiKeyDoesNotExist() { var controller = new TestableApiController(); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns((User)null); - + var result = controller.DeletePackage(Guid.NewGuid().ToString(), "theId", "1.0.42"); Assert.IsType(result); var statusCodeResult = (HttpStatusCodeWithBodyResult)result; Assert.Equal(403, statusCodeResult.StatusCode); Assert.Equal(String.Format(Strings.ApiKeyNotAuthorized, "delete"), statusCodeResult.StatusDescription); + controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(It.IsAny(), true), Times.Never()); } [Fact] public void WillThrowIfAPackageWithTheIdAndSemanticVersionDoesNotExist() { var controller = new TestableApiController(); + var apiKey = Guid.NewGuid(); controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns((Package)null); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(new User()); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(new User()); - var result = controller.DeletePackage(Guid.NewGuid().ToString(), "theId", "1.0.42"); + var result = controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42"); Assert.IsType(result); var statusCodeResult = (HttpStatusCodeWithBodyResult)result; Assert.Equal(404, statusCodeResult.StatusCode); Assert.Equal(String.Format(Strings.PackageWithIdAndVersionNotFound, "theId", "1.0.42"), statusCodeResult.StatusDescription); - } - - [Fact] - public void WillFindTheUserThatMatchesTheApiKey() - { - var owner = new User { Key = 1, ApiKey = Guid.NewGuid() }; - var package = new Package - { - PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } - }; - - var controller = new TestableApiController(); - controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(owner); - - controller.DeletePackage(owner.ApiKey.ToString(), "theId", "1.0.42"); - - controller.MockUserService.Verify(x => x.FindByApiKey(owner.ApiKey)); + controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(It.IsAny(), true), Times.Never()); } [Fact] @@ -331,37 +371,116 @@ public void WillNotDeleteThePackageIfApiKeyDoesNotBelongToAnOwner() }; var apiKey = Guid.NewGuid(); var controller = new TestableApiController(); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(owner); - controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); - controller.MockPackageService - .Setup(svc => svc.MarkPackageUnlisted(It.IsAny(), true)) - .Throws(new InvalidOperationException("Should not have unlisted the package!")); - + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(owner); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion("theId", "1.0.42", true)).Returns(package); + var result = controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42"); Assert.IsType(result); var statusCodeResult = (HttpStatusCodeWithBodyResult)result; Assert.Equal(String.Format(Strings.ApiKeyNotAuthorized, "delete"), statusCodeResult.StatusDescription); + + controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(package, true), Times.Never()); } [Fact] - public void WillUnlistThePackageIfApiKeyBelongsToAnOwner() + public void WillUnlistThePackageIfApiKeyBelongsToAnOwnerUsingFindByApiKey() { - var owner = new User { Key = 1 }; + var apiKey = Guid.NewGuid(); + var owner = new User { Key = 1, ApiKey = apiKey }; var package = new Package { PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } }; var controller = new TestableApiController(); controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(owner); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(owner); + + ResultAssert.IsEmpty(controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42")); + + controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(package, true)); + controller.MockIndexingService.Verify(i => i.UpdatePackage(package)); + } + + [Fact] + public void WillUnlistThePackageIfApiKeyBelongsToAnOwnerUsingAuthenticateCredential() + { + var apiKey = Guid.NewGuid(); + var owner = new Credential() { User = new User { Key = 1 } }; + var package = new Package + { + PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner.User } } + }; + var controller = new TestableApiController(); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); + controller.MockUserService + .Setup(x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(owner); + + ResultAssert.IsEmpty(controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42")); + + controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(package, true)); + controller.MockIndexingService.Verify(i => i.UpdatePackage(package)); + } + + [Fact] + public void WillUseUserFromAuthenticateCredentialOverFindByApiKey() + { var apiKey = Guid.NewGuid(); + var owner = new Credential() { User = new User { Key = 1 } }; + var nonOwner = new User() { ApiKey = apiKey }; + var package = new Package + { + PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner.User } } + }; + var controller = new TestableApiController(); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(nonOwner); + controller.MockUserService + .Setup(x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(owner); - controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42"); + ResultAssert.IsEmpty(controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42")); controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(package, true)); controller.MockIndexingService.Verify(i => i.UpdatePackage(package)); } + + [Fact] + public void WillFailIfUserFromAuthenticateCredentialIsNotOwner() + { + // Arrange + var apiKey = Guid.NewGuid(); + var nonOwner = new Credential() { User = new User { Key = 1 } }; + var owner = new User() { ApiKey = apiKey }; + var package = new Package + { + PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } + }; + var controller = new TestableApiController(); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(owner); + controller.MockUserService + .Setup(x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(nonOwner); + + // Act + var result = controller.DeletePackage(apiKey.ToString(), "theId", "1.0.42"); + + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Forbidden, + String.Format(Strings.ApiKeyNotAuthorized, "delete")); + + controller.MockPackageService.Verify(x => x.MarkPackageUnlisted(package, true), Times.Never()); + } } public class TheGetPackageAction @@ -528,105 +647,172 @@ public class ThePublishPackageAction [InlineData("this-is-bad-guid")] public void WillThrowIfTheApiKeyIsAnInvalidGuid(string guidValue) { + // Arrange var controller = new TestableApiController(); controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns((User)null); + // Act var result = controller.PublishPackage(guidValue, "theId", "1.0.42"); - - Assert.IsType(result); - AssertStatusCodeResult(result, 400, String.Format("The API key '{0}' is invalid.", guidValue)); + + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.BadRequest, + String.Format("The API key '{0}' is invalid.", guidValue)); + controller.MockPackageService.Verify(x => x.MarkPackageListed(It.IsAny(), It.IsAny()), Times.Never()); } [Fact] public void WillThrowIfTheApiKeyDoesNotExist() { + // Arrange var controller = new TestableApiController(); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns((User)null); + var apiKey = Guid.NewGuid(); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).ReturnsNull(); - var result = controller.PublishPackage(Guid.NewGuid().ToString(), "theId", "1.0.42"); + // Act + var result = controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); - Assert.IsType(result); - var statusCodeResult = (HttpStatusCodeWithBodyResult)result; - Assert.Equal(403, statusCodeResult.StatusCode); - Assert.Equal(String.Format(Strings.ApiKeyNotAuthorized, "publish"), statusCodeResult.StatusDescription); + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Forbidden, + String.Format(Strings.ApiKeyNotAuthorized, "publish")); + controller.MockPackageService.Verify(x => x.MarkPackageListed(It.IsAny(), It.IsAny()), Times.Never()); } [Fact] public void WillThrowIfAPackageWithTheIdAndSemanticVersionDoesNotExist() { + // Arrange var controller = new TestableApiController(); - controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns((Package)null); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(new User()); + var apiKey = Guid.NewGuid(); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion("theId", "1.0.42", true)).Returns((Package)null); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(new User()); - var result = controller.PublishPackage(Guid.NewGuid().ToString(), "theId", "1.0.42"); + // Act + var result = controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); - Assert.IsType(result); - var statusCodeResult = (HttpStatusCodeWithBodyResult)result; - Assert.Equal(404, statusCodeResult.StatusCode); - Assert.Equal(String.Format(Strings.PackageWithIdAndVersionNotFound, "theId", "1.0.42"), statusCodeResult.StatusDescription); + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.NotFound, + String.Format(Strings.PackageWithIdAndVersionNotFound, "theId", "1.0.42")); + controller.MockPackageService.Verify(x => x.MarkPackageListed(It.IsAny(), It.IsAny()), Times.Never()); } [Fact] - public void WillFindTheUserThatMatchesTheApiKey() + public void WillNotListThePackageIfApiKeyDoesNotBelongToAnOwner() { + // Arrange var owner = new User { Key = 1 }; var package = new Package { - PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } + PackageRegistration = new PackageRegistration { Owners = new[] { new User() } } }; var apiKey = Guid.NewGuid(); var controller = new TestableApiController(); - controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(owner); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion("theId", "1.0.42", true)).Returns(package); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(owner); + + // Act + var result = controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); - controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Forbidden, + String.Format(Strings.ApiKeyNotAuthorized, "publish")); - controller.MockUserService.Verify(x => x.FindByApiKey(apiKey)); + controller.MockPackageService.Verify(x => x.MarkPackageListed(package, It.IsAny()), Times.Never()); } [Fact] - public void WillNotListThePackageIfApiKeyDoesNotBelongToAnOwner() + public void WillListThePackageIfApiKeyBelongsToAnOwnerUsingFindByApiKey() { - var owner = new User { Key = 1 }; - var package = new Package - { - PackageRegistration = new PackageRegistration { Owners = new[] { new User() } } - }; + // Arrange var apiKey = Guid.NewGuid(); + var owner = new User { Key = 1, ApiKey = apiKey }; + var package = new Package + { + PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } + }; var controller = new TestableApiController(); controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); - controller.MockPackageService.Setup(svc => svc.MarkPackageListed(It.IsAny(), It.IsAny())) - .Throws(new InvalidOperationException("Should not have listed the package!")); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(owner); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(owner); + // Act var result = controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); - Assert.IsType(result); - var statusCodeResult = (HttpStatusCodeWithBodyResult)result; - Assert.Equal(String.Format(Strings.ApiKeyNotAuthorized, "publish"), statusCodeResult.StatusDescription); + // Assert + ResultAssert.IsEmpty(result); + controller.MockPackageService.Verify(x => x.MarkPackageListed(package, It.IsAny())); + controller.MockIndexingService.Verify(i => i.UpdatePackage(package)); } [Fact] - public void WillListThePackageIfApiKeyBelongsToAnOwner() + public void WillListThePackageIfApiKeyBelongsToAnOwnerUsingAuthorizeCredential() { - var owner = new User { Key = 1 }; - var package = new Package - { - PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } - }; + // Arrange + var apiKey = Guid.NewGuid(); + var owner = new Credential { User = new User { Key = 1 } }; + var package = new Package + { + PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner.User } } + }; var controller = new TestableApiController(); controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); - controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(owner); + controller.MockUserService + .Setup(x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(owner); - controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); + // Act + var result = controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); + // Assert + ResultAssert.IsEmpty(result); controller.MockPackageService.Verify(x => x.MarkPackageListed(package, It.IsAny())); controller.MockIndexingService.Verify(i => i.UpdatePackage(package)); } + + [Fact] + public void WillFailIfUserFromAuthenticateCredentialIsNotOwner() + { + // Arrange + var apiKey = Guid.NewGuid(); + var nonOwner = new Credential { User = new User { Key = 1 } }; + var owner = new User(); + var package = new Package + { + PackageRegistration = new PackageRegistration { Owners = new[] { new User(), owner } } + }; + + var controller = new TestableApiController(); + controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersion(It.IsAny(), It.IsAny(), true)).Returns(package); + controller.MockUserService.Setup(x => x.FindByApiKey(apiKey)).Returns(owner); + controller.MockUserService + .Setup(x => x.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + apiKey.ToString().ToLowerInvariant())) + .Returns(nonOwner); + + // Act + var result = controller.PublishPackage(apiKey.ToString(), "theId", "1.0.42"); + + // Assert + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Forbidden, + String.Format(Strings.ApiKeyNotAuthorized, "publish")); + + controller.MockPackageService.Verify(x => x.MarkPackageListed(package, It.IsAny()), Times.Never()); + } } public class TheVerifyPackageKeyAction @@ -639,39 +825,74 @@ public void VerifyPackageKeyReturns403IfApiKeyIsInvalidGuid() // Act var result = controller.VerifyPackageKey("bad-guid", "foo", "1.0.0"); - + // Assert - AssertStatusCodeResult(result, 400, "The API key 'bad-guid' is invalid."); + ResultAssert.IsStatusCode( + result, + HttpStatusCode.BadRequest, + "The API key 'bad-guid' is invalid."); } [Fact] - public void VerifyPackageKeyReturns403IfUserDoesNotExist() + public void VerifyPackageKeyReturns403IfUserDoesNotExistByFindByApiKeyOrAuthorizeCredential() { // Arrange var guid = Guid.NewGuid(); var controller = new TestableApiController(); controller.MockUserService.Setup(s => s.FindByApiKey(guid)).Returns(null); + controller.MockUserService + .Setup(s => s.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + guid.ToString().ToLowerInvariant())) + .ReturnsNull(); // Act var result = controller.VerifyPackageKey(guid.ToString(), "foo", "1.0.0"); // Assert - AssertStatusCodeResult(result, 403, "The specified API key does not provide the authority to push packages."); + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Forbidden, + "The specified API key does not provide the authority to push packages."); } [Fact] - public void VerifyPackageKeyReturnsEmptyResultIfApiKeyExistsAndIdAndVersionAreEmpty() + public void VerifyPackageKeyReturnsEmptyResultIfApiKeyExistsInUserRecordAndIdAndVersionAreEmpty() { // Arrange var guid = Guid.NewGuid(); var controller = new TestableApiController(); controller.MockUserService.Setup(s => s.FindByApiKey(guid)).Returns(new User()); + controller.MockUserService + .Setup(s => s.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + guid.ToString().ToLowerInvariant())) + .ReturnsNull(); + + // Act + var result = controller.VerifyPackageKey(guid.ToString(), null, null); + + // Assert + ResultAssert.IsEmpty(result); + } + + [Fact] + public void VerifyPackageKeyReturnsEmptyResultIfApiKeyExistsInCredentialsAndIdAndVersionAreEmpty() + { + // Arrange + var guid = Guid.NewGuid(); + var controller = new TestableApiController(); + controller.MockUserService + .Setup(s => s.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + guid.ToString().ToLowerInvariant())) + .Returns(new Credential() { User = new User() }); // Act var result = controller.VerifyPackageKey(guid.ToString(), null, null); // Assert - Assert.IsType(result); + ResultAssert.IsEmpty(result); } [Fact] @@ -685,18 +906,28 @@ public void VerifyPackageKeyReturns404IfPackageDoesNotExist() // Act var result = controller.VerifyPackageKey(guid.ToString(), "foo", "1.0.0"); - + // Assert - AssertStatusCodeResult(result, 404, "A package with id 'foo' and version '1.0.0' does not exist."); + ResultAssert.IsStatusCode( + result, + HttpStatusCode.NotFound, + "A package with id 'foo' and version '1.0.0' does not exist."); } [Fact] - public void VerifyPackageKeyReturns403IfUserIsNotAnOwner() + public void VerifyPackageKeyReturns403IfUserInCredentialsTableIsNotAnOwner() { // Arrange var guid = Guid.NewGuid(); var controller = new TestableApiController(); - controller.MockUserService.Setup(s => s.FindByApiKey(guid)).Returns(new User()); + var owner = new User(); + var nonOwner = new User(); + controller.MockUserService.Setup(s => s.FindByApiKey(guid)).Returns(owner); + controller.MockUserService + .Setup(s => s.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + guid.ToString().ToLowerInvariant())) + .Returns(new Credential() { User = nonOwner }); controller.MockPackageService.Setup(s => s.FindPackageByIdAndVersion("foo", "1.0.0", true)).Returns( new Package { PackageRegistration = new PackageRegistration() }); @@ -704,11 +935,14 @@ public void VerifyPackageKeyReturns403IfUserIsNotAnOwner() var result = controller.VerifyPackageKey(guid.ToString(), "foo", "1.0.0"); // Assert - AssertStatusCodeResult(result, 403, "The specified API key does not provide the authority to push packages."); + ResultAssert.IsStatusCode( + result, + HttpStatusCode.Forbidden, + "The specified API key does not provide the authority to push packages."); } [Fact] - public void VerifyPackageKeyReturns200IfUserIsAnOwner() + public void VerifyPackageKeyReturns200IfUserHasNoCredentialRecordButIsAnOwner() { // Arrange var guid = Guid.NewGuid(); @@ -716,17 +950,50 @@ public void VerifyPackageKeyReturns200IfUserIsAnOwner() var package = new Package { PackageRegistration = new PackageRegistration() }; package.PackageRegistration.Owners.Add(user); var controller = new TestableApiController(); + controller.MockUserService + .Setup(s => s.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + guid.ToString().ToLowerInvariant())) + .ReturnsNull(); controller.MockUserService.Setup(s => s.FindByApiKey(guid)).Returns(user); controller.MockPackageService.Setup(s => s.FindPackageByIdAndVersion("foo", "1.0.0", true)).Returns(package); // Act var result = controller.VerifyPackageKey(guid.ToString(), "foo", "1.0.0"); - Assert.IsType(result); + // Assert + ResultAssert.IsEmpty(result); } [Fact] - public async void VerifyRecentPopularityStatsDownloads() + public void VerifyPackageKeyReturns200IfUserHasCredentialRecordAndIsAnOwner() + { + // Arrange + var guid = Guid.NewGuid(); + var user = new User(); + var package = new Package { PackageRegistration = new PackageRegistration() }; + package.PackageRegistration.Owners.Add(user); + var controller = new TestableApiController(); + controller.MockUserService + .Setup(s => s.AuthenticateCredential( + Constants.CredentialTypes.ApiKeyV1, + guid.ToString().ToLowerInvariant())) + .Returns(new Credential() { User = user }); + controller.MockUserService.Setup(s => s.FindByApiKey(guid)).ReturnsNull(); + controller.MockPackageService.Setup(s => s.FindPackageByIdAndVersion("foo", "1.0.0", true)).Returns(package); + + // Act + var result = controller.VerifyPackageKey(guid.ToString(), "foo", "1.0.0"); + + // Assert + ResultAssert.IsEmpty(result); + } + } + + public class TheGetStatsDownloadsAction + { + [Fact] + public async Task VerifyRecentPopularityStatsDownloads() { JArray report = new JArray { @@ -773,7 +1040,7 @@ public async void VerifyRecentPopularityStatsDownloads() var fakeReportService = new Mock(); fakeReportService.Setup(x => x.Load("RecentPopularityDetail.json")).Returns(Task.FromResult(new StatisticsReport(fakePackageVersionReport, DateTime.UtcNow))); - + var controller = new TestableApiController { StatisticsService = new JsonStatisticsService(fakeReportService.Object), @@ -793,7 +1060,7 @@ public async void VerifyRecentPopularityStatsDownloads() } [Fact] - public async void VerifyStatsDownloadsReturnsNotFoundWhenStatsNotAvailable() + public async Task VerifyStatsDownloadsReturnsNotFoundWhenStatsNotAvailable() { var controller = new TestableApiController(); controller.MockStatisticsService.Setup(x => x.LoadDownloadPackageVersions()).Returns(Task.FromResult(StatisticsReportResult.Failed)); @@ -808,7 +1075,7 @@ public async void VerifyStatsDownloadsReturnsNotFoundWhenStatsNotAvailable() } [Fact] - public async void VerifyRecentPopularityStatsDownloadsCount() + public async Task VerifyRecentPopularityStatsDownloadsCount() { JArray report = new JArray { diff --git a/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs index 5ee17a2573..95166ba9ee 100644 --- a/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs @@ -145,7 +145,7 @@ public void WillInvalidateModelStateAndShowTheViewWithErrorsWhenTheUsernameAndPa Assert.NotNull(result); Assert.Empty(result.ViewName); Assert.False(controller.ModelState.IsValid); - Assert.Equal(Strings.UserNotFound, controller.ModelState[String.Empty].Errors[0].ErrorMessage); + Assert.Equal(Strings.UsernameAndPasswordNotFound, controller.ModelState[String.Empty].Errors[0].ErrorMessage); } [Fact] diff --git a/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs index 9e3d9fc96b..e56698f102 100644 --- a/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Net.Mail; using System.Security.Principal; @@ -24,7 +25,7 @@ public void WillGetTheCurrentUserUsingTheRequestIdentityName() controller.MockUserService .Setup(s => s.FindByUsername(It.IsAny())) .Returns(new User { Key = 42 }); - + //act controller.Account(); @@ -40,7 +41,7 @@ public void WillGetCuratedFeedsManagedByTheCurrentUser() controller.MockUserService .Setup(s => s.FindByUsername(It.IsAny())) .Returns(new User { Key = 42 }); - + // act controller.Account(); @@ -57,7 +58,7 @@ public void WillReturnTheAccountViewModelWithTheUserApiKey() controller.MockUserService .Setup(s => s.FindByUsername(It.IsAny())) .Returns(new User { Key = 42, ApiKey = stubApiKey }); - + // act var model = ((ViewResult)controller.Account()).Model as AccountViewModel; @@ -75,13 +76,60 @@ public void WillReturnTheAccountViewModelWithTheCuratedFeeds() controller.MockCuratedFeedService .Setup(stub => stub.GetFeedsForManager(It.IsAny())) .Returns(new[] { new CuratedFeed { Name = "theCuratedFeed" } }); - + // act var model = ((ViewResult)controller.Account()).Model as AccountViewModel; // verify Assert.Equal("theCuratedFeed", model.CuratedFeeds.First()); } + + [Fact] + public void WillUseApiKeyInColumnIfNoCredentialPresent() + { + var apiKey = Guid.NewGuid(); + var controller = new TestableUsersController(); + controller.MockUserService + .Setup(s => s.FindByUsername(It.IsAny())) + .Returns(new User { Key = 42, ApiKey = apiKey }); + controller.MockCuratedFeedService + .Setup(stub => stub.GetFeedsForManager(It.IsAny())) + .Returns(new[] { new CuratedFeed { Name = "theCuratedFeed" } }); + + // act + var result = controller.Account(); + + // verify + var model = ResultAssert.IsView(result); + Assert.Equal(apiKey.ToString().ToLowerInvariant(), model.ApiKey); + } + + [Fact] + public void WillUseApiKeyInCredentialIfPresent() + { + var apiKey = Guid.NewGuid(); + var controller = new TestableUsersController(); + controller.MockUserService + .Setup(s => s.FindByUsername(It.IsAny())) + .Returns(new User + { + Key = 42, + ApiKey = Guid.NewGuid(), + Credentials = new List() { + CredentialBuilder.CreateV1ApiKey(apiKey) + } + }); + controller.MockCuratedFeedService + .Setup(stub => stub.GetFeedsForManager(It.IsAny())) + .Returns(new[] { new CuratedFeed { Name = "theCuratedFeed" } }); + + // act + var result = controller.Account(); + + // verify + var model = ResultAssert.IsView(result); + Assert.Equal(apiKey.ToString().ToLowerInvariant(), model.ApiKey); + } } public class TheConfirmMethod @@ -111,7 +159,7 @@ public void ReturnsConfirmedWhenTokenMatchesUser() controller.MockUserService .Setup(u => u.ConfirmEmailAddress(user, "the-token")) .Returns(true); - + var model = (controller.Confirm("username", "the-token") as ViewResult).Model as EmailConfirmationModel; Assert.True(model.SuccessfulConfirmation); @@ -133,7 +181,7 @@ public void SendsAccountChangedNoticeWhenConfirmingChangedEmail() controller.MockUserService .Setup(u => u.ConfirmEmailAddress(user, "the-token")) .Returns(true); - + var model = (controller.Confirm("username", "the-token") as ViewResult).Model as EmailConfirmationModel; Assert.True(model.SuccessfulConfirmation); @@ -158,7 +206,7 @@ public void DoesntSendAccountChangedEmailsWhenNoOldConfirmedAddress() controller.MockUserService .Setup(u => u.ConfirmEmailAddress(user, "the-token")) .Returns(true); - + // act: var model = (controller.Confirm("username", "the-token") as ViewResult).Model as EmailConfirmationModel; @@ -187,7 +235,7 @@ public void DoesntSendAccountChangedEmailsIfConfirmationTokenDoesntMatch() controller.MockUserService .Setup(u => u.ConfirmEmailAddress(user, "faketoken")) .Returns(false); - + // act: var model = (controller.Confirm("username", "faketoken") as ViewResult).Model as EmailConfirmationModel; @@ -216,7 +264,7 @@ public void ReturnsFalseWhenTokenDoesNotMatchUser() controller.MockUserService .Setup(u => u.ConfirmEmailAddress(user, "not-the-token")) .Returns(false); - + var model = (controller.Confirm("username", "not-the-token") as ViewResult).Model as EmailConfirmationModel; Assert.False(model.SuccessfulConfirmation); @@ -311,7 +359,7 @@ public void WithInvalidUsernameReturnsFileNotFound() controller.MockUserService .Setup(u => u.FindByUsername(It.IsAny())) .ReturnsNull(); - + var result = controller.Edit(new EditProfileViewModel()) as HttpNotFoundResult; Assert.NotNull(result); @@ -371,9 +419,9 @@ public void ReturnsSameViewIfTokenGenerationFails() controller.MockUserService .Setup(s => s.GeneratePasswordResetToken("user", 1440)) .Returns((User)null); - + var model = new ForgotPasswordViewModel { Email = "user" }; - + var result = controller.ForgotPassword(model) as ViewResult; Assert.NotNull(result); @@ -386,30 +434,59 @@ public class TheGenerateApiKeyMethod [Fact] public void RedirectsToAccountPage() { + var user = new User(); var controller = new TestableUsersController(); controller.MockCurrentIdentity - .Setup(i => i.Name) - .Returns("the-username"); - - var result = controller.GenerateApiKey() as RedirectToRouteResult; + .Setup(i => i.Name) + .Returns("the-username"); + controller.MockUserService + .Setup(u => u.FindByUsername("the-username")) + .Returns(user); - Assert.NotNull(result); - Assert.Equal("Account", result.RouteValues["action"]); - Assert.Equal("Users", result.RouteValues["controller"]); + var result = controller.GenerateApiKey(); + + ResultAssert.IsRedirectToRoute(result, new { action = "Account", controller = "Users" }); } [Fact] - public void GeneratesAnApiKey() + public void PutsNewCredentialInOldField() { var controller = new TestableUsersController(); + var user = new User() { ApiKey = Guid.NewGuid() }; controller.MockCurrentIdentity - .Setup(i => i.Name) - .Returns("the-username"); - + .Setup(i => i.Name) + .Returns("the-username"); + controller.MockUserService + .Setup(u => u.FindByUsername("the-username")) + .Returns(user); + Credential created = null; + controller.MockUserService + .Setup(u => u.ReplaceCredential(user, It.IsAny())) + .Callback((_, c) => created = c); + + controller.GenerateApiKey(); + + Assert.Equal(created.Value, user.ApiKey.ToString().ToLowerInvariant()); + } + + [Fact] + public void ReplacesTheApiKeyCredential() + { + var controller = new TestableUsersController(); + var user = new User(); + controller.MockCurrentIdentity + .Setup(i => i.Name) + .Returns("the-username"); + controller.MockUserService + .Setup(u => u.FindByUsername("the-username")) + .Returns(user); + controller.GenerateApiKey(); controller.MockUserService - .Verify(s => s.GenerateApiKey("the-username")); + .Verify(u => u.ReplaceCredential( + user, + It.Is(c => c.Type == Constants.CredentialTypes.ApiKeyV1))); } } @@ -434,7 +511,7 @@ public void WillCreateTheUser() controller.MockUserService .Setup(x => x.Create(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new User { Username = "theUsername", EmailAddress = "to@example.com" }); - + controller.Register( new RegisterRequest { @@ -454,7 +531,7 @@ public void WillInvalidateModelStateAndShowTheViewWhenAnEntityExceptionIsThrow() controller.MockUserService .Setup(x => x.Create(It.IsAny(), It.IsAny(), It.IsAny())) .Throws(new EntityException("aMessage")); - + var result = controller.Register( new RegisterRequest { @@ -493,7 +570,7 @@ public void WillSendNewUserEmailIfConfirmationRequired() controller.MockConfig .Setup(x => x.ConfirmEmailAddresses) .Returns(true); - + controller.Register( new RegisterRequest { @@ -509,6 +586,87 @@ public void WillSendNewUserEmailIfConfirmationRequired() } } + public class TheChangePasswordMethod + { + [Fact] + public void ReturnsViewIfModelStateInvalid() + { + // Arrange + var controller = new TestableUsersController(); + controller.ModelState.AddModelError("test", "test"); + var inputModel = new PasswordChangeViewModel(); + + // Act + var result = controller.ChangePassword(inputModel); + + // Assert + var outputModel = ResultAssert.IsView(result); + Assert.Same(inputModel, outputModel); + } + + [Fact] + public void AddsModelErrorIfUserServiceFails() + { + // Arrange + var controller = new TestableUsersController(); + controller.MockCurrentIdentity + .Setup(i => i.Name) + .Returns("user"); + controller.MockUserService + .Setup(u => u.ChangePassword("user", "old", "new")) + .Returns(false); + var inputModel = new PasswordChangeViewModel() + { + OldPassword = "old", + NewPassword = "new", + ConfirmPassword = "new" + }; + + // Act + var result = controller.ChangePassword(inputModel); + + // Assert + var outputModel = ResultAssert.IsView(result); + Assert.Same(inputModel, outputModel); + + var errorMessages = controller + .ModelState["OldPassword"] + .Errors + .Select(e => e.ErrorMessage) + .ToArray(); + Assert.Equal(errorMessages, new[] { Strings.CurrentPasswordIncorrect }); + } + + [Fact] + public void RedirectsToPasswordChangedIfUserServiceSucceeds() + { + // Arrange + var controller = new TestableUsersController(); + controller.MockCurrentIdentity + .Setup(i => i.Name) + .Returns("user"); + controller.MockUserService + .Setup(u => u.ChangePassword("user", "old", "new")) + .Returns(true); + var inputModel = new PasswordChangeViewModel() + { + OldPassword = "old", + NewPassword = "new", + ConfirmPassword = "new" + }; + + // Act + var result = controller.ChangePassword(inputModel); + + // Assert + ResultAssert.IsRedirectToRoute(result, new + { + controller = "Users", + action = "PasswordChanged" + }); + } + } + public class TheResetPasswordMethod { [Fact] @@ -561,7 +719,7 @@ public void ShowsDefaultThanksViewWhenConfirmingEmailAddressIsRequired() controller.MockConfig .Setup(x => x.ConfirmEmailAddresses) .Returns(true); - + var result = controller.Thanks() as ViewResult; Assert.Empty(result.ViewName); @@ -575,7 +733,7 @@ public void ShowsConfirmViewWithModelWhenConfirmingEmailAddressIsNotRequired() controller.MockConfig .Setup(x => x.ConfirmEmailAddresses) .Returns(false); - + var result = controller.Thanks() as ViewResult; Assert.Equal("Confirm", result.ViewName); @@ -594,7 +752,7 @@ public class TestableUsersController : UsersController public Mock MockPackageService { get; protected set; } public Mock MockConfig { get; protected set; } public Mock MockUserService { get; protected set; } - + public TestableUsersController() { CuratedFeedService = (MockCuratedFeedService = new Mock()).Object; diff --git a/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj b/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj index acede600b9..2498b30f0b 100644 --- a/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj +++ b/tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj @@ -27,6 +27,7 @@ 4 true false + 0618 pdbonly @@ -251,6 +252,9 @@ NuGetGallery + + + diff --git a/tests/NuGetGallery.Facts/Services/UserServiceFacts.cs b/tests/NuGetGallery.Facts/Services/UserServiceFacts.cs index 29fecb3739..62e639841c 100644 --- a/tests/NuGetGallery.Facts/Services/UserServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/UserServiceFacts.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using Moq; using NuGetGallery.Configuration; @@ -8,6 +9,13 @@ namespace NuGetGallery { public class UserServiceFacts { + public static User CreateAUser( + string username, + string emailAddress) + { + return CreateAUser(username, password: null, emailAddress: emailAddress); + } + public static User CreateAUser( string username, string password, @@ -16,27 +24,40 @@ public static User CreateAUser( return new User { Username = username, - HashedPassword = CryptographyService.GenerateSaltedHash(password, Constants.PBKDF2HashAlgorithmId), - PasswordHashAlgorithm = Constants.PBKDF2HashAlgorithmId, + HashedPassword = String.IsNullOrEmpty(password) ? + null : + CryptographyService.GenerateSaltedHash(password, Constants.PBKDF2HashAlgorithmId), + PasswordHashAlgorithm = String.IsNullOrEmpty(password) ? + null : + Constants.PBKDF2HashAlgorithmId, EmailAddress = emailAddress, }; } - public static bool VerifyPasswordHash(User user, string password) + public static bool VerifyPasswordHash(string hash, string algorithm, string password) { bool canAuthenticate = CryptographyService.ValidateSaltedHash( - user.HashedPassword, + hash, password, - user.PasswordHashAlgorithm); + algorithm); bool sanity = CryptographyService.ValidateSaltedHash( - user.HashedPassword, + hash, "not_the_password", - user.PasswordHashAlgorithm); + algorithm); return canAuthenticate && !sanity; } + public static Credential CreatePasswordCredential(string password) + { + return new Credential( + type: Constants.CredentialTypes.PasswordPbkdf2, + value: CryptographyService.GenerateSaltedHash( + password, + Constants.PBKDF2HashAlgorithmId)); + } + // Now only for things that actually need a MOCK UserService object. private static UserService CreateMockUserService(Action> setup, Mock> userRepo = null, Mock config = null) { @@ -47,10 +68,12 @@ private static UserService CreateMockUserService(Action> setup } userRepo = userRepo ?? new Mock>(); + var credRepo = new Mock>(); var userService = new Mock( config.Object, - userRepo.Object) + userRepo.Object, + credRepo.Object) { CallBase = true }; @@ -137,7 +160,30 @@ public void UpdatesTheHashedPassword() .Setup(r => r.GetAll()).Returns(new[] { user }.AsQueryable()); var changed = service.ChangePassword("user", "oldpwd", "newpwd"); - Assert.True(VerifyPasswordHash(user, "newpwd")); + Assert.True(VerifyPasswordHash(user.HashedPassword, user.PasswordHashAlgorithm, "newpwd")); + service.MockUserRepository.VerifyCommitted(); + } + + [Fact] + public void UpdatesThePasswordCredential() + { + var hash = CryptographyService.GenerateSaltedHash("oldpwd", "PBKDF2"); + var user = new User { + Username = "user", + Credentials = new List() + { + new Credential(Constants.CredentialTypes.PasswordPbkdf2, hash) + } + }; + var service = new TestableUserService(); + service.MockUserRepository + .Setup(r => r.GetAll()).Returns(new[] { user }.AsQueryable()); + + var changed = service.ChangePassword("user", "oldpwd", "newpwd"); + var cred = user.Credentials.Single(); + Assert.Equal(Constants.CredentialTypes.PasswordPbkdf2, cred.Type); + Assert.True(VerifyPasswordHash(cred.Value, Constants.PBKDF2HashAlgorithmId, "newpwd")); + service.MockUserRepository.VerifyCommitted(); } [Fact] @@ -155,8 +201,9 @@ public void MigratesPasswordIfHashAlgorithmIsNotPBKDF2() var changed = service.ChangePassword("user", "oldpwd", "newpwd"); Assert.True(changed); - Assert.True(VerifyPasswordHash(user, "newpwd")); + Assert.True(VerifyPasswordHash(user.HashedPassword, user.PasswordHashAlgorithm, "newpwd")); Assert.Equal("PBKDF2", user.PasswordHashAlgorithm); + service.MockUserRepository.VerifyCommitted(); } } @@ -276,7 +323,7 @@ public void WillHashThePassword() "theEmailAddress"); Assert.Equal("PBKDF2", user.PasswordHashAlgorithm); - Assert.True(VerifyPasswordHash(user, "thePassword")); + Assert.True(VerifyPasswordHash(user.HashedPassword, user.PasswordHashAlgorithm, "thePassword")); } [Fact] @@ -300,6 +347,28 @@ public void WillSaveTheNewUser() .Verify(x => x.CommitChanges()); } + [Fact] + public void WillSaveThePasswordInTheCredentialsTable() + { + var userService = new TestableUserService(); + + var user = userService.Create( + "theUsername", + "thePassword", + "theEmailAddress"); + + Assert.NotNull(user); + var passwordCred = user.Credentials.FirstOrDefault(c => c.Type == Constants.CredentialTypes.PasswordPbkdf2); + Assert.NotNull(passwordCred); + Assert.Equal(Constants.CredentialTypes.PasswordPbkdf2, passwordCred.Type); + Assert.True(VerifyPasswordHash(passwordCred.Value, Constants.PBKDF2HashAlgorithmId, "thePassword")); + + userService.MockUserRepository + .Verify(x => x.InsertOnCommit(user)); + userService.MockUserRepository + .Verify(x => x.CommitChanges()); + } + [Fact] public void WillSaveTheNewUserAsConfirmedWhenConfigured() { @@ -334,7 +403,13 @@ public void SetsAnApiKey() "thePassword", "theEmailAddress"); + userService.MockUserRepository + .Verify(x => x.InsertOnCommit(user)); Assert.NotEqual(Guid.Empty, user.ApiKey); + + var apiKeyCred = user.Credentials.FirstOrDefault(c => c.Type == Constants.CredentialTypes.ApiKeyV1); + Assert.NotNull(apiKeyCred); + Assert.Equal(user.ApiKey.ToString().ToLowerInvariant(), apiKeyCred.Value); } [Fact] @@ -404,8 +479,7 @@ public void FindsUsersByUserName() [Fact] public void WillNotFindsUsersByEmailAddress() { - var hash = CryptographyService.GenerateSaltedHash("thePassword", Constants.PBKDF2HashAlgorithmId); - var user = new User { Username = "theUsername", HashedPassword = hash, EmailAddress = "test@example.com" }; + var user = CreateAUser("theUsername", "thePassword", "test@example.com"); var service = new TestableUserService(); service.MockUserRepository .Setup(r => r.GetAll()) @@ -415,6 +489,50 @@ public void WillNotFindsUsersByEmailAddress() Assert.Null(foundByEmailAddress); } + + [Fact] + public void DoesNotReturnUserIfPasswordIsInvalid() + { + var user = CreateAUser("theUsername", "thePassword", "test@example.com"); + var service = new TestableUserService(); + service.MockUserRepository + .Setup(r => r.GetAll()) + .Returns(new[] { user }.AsQueryable()); + + var foundByUserName = service.FindByUsernameAndPassword("theUsername", "theWrongPassword"); + + Assert.Null(foundByUserName); + } + + [Fact] + public void FindsUserBasedOnPasswordInCredentialsTable() + { + var user = CreateAUser("theUsername", "test@example.com"); + user.Credentials.Add(CreatePasswordCredential("thePassword")); + var service = new TestableUserService(); + service.MockUserRepository.HasData(user); + service.MockCredentialRepository.HasData(user.Credentials); + + var foundByUserName = service.FindByUsernameAndPassword("theUsername", "thePassword"); + + Assert.NotNull(foundByUserName); + Assert.Same(user, foundByUserName); + } + + [Fact] + public void IfSomehowBothPasswordsExistItFindsUserBasedOnPasswordInCredentialsTable() + { + var user = CreateAUser("theUsername", "theWrongPassword", "test@example.com"); + user.Credentials.Add(CreatePasswordCredential("thePassword")); + var service = new TestableUserService(); + service.MockUserRepository.HasData(user); + service.MockCredentialRepository.HasData(user.Credentials); + + var foundByUserName = service.FindByUsernameAndPassword("theUsername", "thePassword"); + + Assert.NotNull(foundByUserName); + Assert.Same(user, foundByUserName); + } } public class TheFindByUsernameOrEmailAddressAndPasswordMethod @@ -462,28 +580,162 @@ public void FindsUsersByEmailAddress() } [Fact] - public void FindsUsersUpdatesPasswordIfUsingLegacyHashAlgorithm() + public void FindsUserBasedOnPasswordInCredentialsTable() { - var user = new User - { - Username = "theUsername", - HashedPassword = CryptographyService.GenerateSaltedHash("thePassword", "SHA1"), - PasswordHashAlgorithm = "SHA1", - EmailAddress = "test@example.com", + var user = CreateAUser("theUsername", "test@example.com"); + user.Credentials.Add(CreatePasswordCredential("thePassword")); + var service = new TestableUserService(); + service.MockUserRepository.HasData(user); + service.MockCredentialRepository.HasData(user.Credentials); + + var foundByUserName = service.FindByUsernameOrEmailAddressAndPassword("test@example.com", "thePassword"); + + Assert.NotNull(foundByUserName); + Assert.Same(user, foundByUserName); + } + + [Fact] + public void IfSomehowBothPasswordsExistItFindsUserBasedOnPasswordInCredentialsTable() + { + var user = CreateAUser("theUsername", "theWrongPassword", "test@example.com"); + user.Credentials.Add(CreatePasswordCredential("thePassword")); + var service = new TestableUserService(); + service.MockUserRepository.HasData(user); + service.MockCredentialRepository.HasData(user.Credentials); + + var foundByUserName = service.FindByUsernameOrEmailAddressAndPassword("test@example.com", "thePassword"); + + Assert.NotNull(foundByUserName); + Assert.Same(user, foundByUserName); + } + } + + public class TheAuthenticateCredentialMethod + { + [Fact] + public void ReturnsNullIfNoCredentialOfSpecifiedTypeExists() + { + // Arrange + var creds = new List() { + new Credential("foo", "bar") }; + var service = new TestableUserService(); + service.MockCredentialRepository.HasData(creds); + + // Act + var result = service.AuthenticateCredential(type: "baz", value: "bar"); + + // Assert + Assert.Null(result); + } + [Fact] + public void ReturnsNullIfNoCredentialOfSpecifiedTypeWithSpecifiedValueExists() + { + // Arrange + var creds = new List() { + new Credential("foo", "bar") + }; var service = new TestableUserService(); - service.MockUserRepository - .Setup(r => r.GetAll()) - .Returns(new[] { user }.AsQueryable()); - service.MockUserRepository - .Setup(r => r.CommitChanges()) - .Verifiable(); + service.MockCredentialRepository.HasData(creds); - service.FindByUsernameOrEmailAddressAndPassword("test@example.com", "thePassword"); - Assert.Equal("PBKDF2", user.PasswordHashAlgorithm); - Assert.True(VerifyPasswordHash(user, "thePassword")); - service.MockUserRepository.Verify(r => r.CommitChanges(), Times.Once()); + // Act + var result = service.AuthenticateCredential(type: "foo", value: "baz"); + + // Assert + Assert.Null(result); + } + + [Fact] + public void ReturnsCredentialIfOneExistsWithSpecifiedTypeAndValue() + { + // Arrange + var creds = new List() { + new Credential("foo", "bar") + }; + var service = new TestableUserService(); + service.MockCredentialRepository.HasData(creds); + + // Act + var result = service.AuthenticateCredential(type: "foo", value: "bar"); + + // Assert + Assert.Same(creds[0], result); + } + } + + public class TheReplaceCredentialMethod + { + [Fact] + public void ThrowsExceptionIfNoUserWithProvidedUserName() + { + // Arrange + var users = new List() { + new User("foo", "baz") + }; + var service = new TestableUserService(); + service.MockUserRepository.HasData(users); + + // Act + var ex = Assert.Throws(() => + service.ReplaceCredential("biz", new Credential())); + + // Assert + Assert.Equal(Strings.UserNotFound, ex.Message); + } + + [Fact] + public void AddsNewCredentialIfNoneWithSameTypeForUser() + { + // Arrange + var existingCred = new Credential("foo", "bar"); + var newCred = new Credential("baz", "boz"); + var users = new List() { + new User("foo", "baz") { + Credentials = new List() { + existingCred + } + } + }; + var service = new TestableUserService(); + service.MockUserRepository.HasData(users); + + // Act + service.ReplaceCredential("foo", newCred); + + // Assert + Assert.Equal(2, users[0].Credentials.Count); + Assert.Equal(new[] { existingCred, newCred }, users[0].Credentials.ToArray()); + service.MockUserRepository.VerifyCommitted(); + } + + [Fact] + public void ReplacesExistingCredentialIfOneWithSameTypeExistsForUser() + { + // Arrange + var frozenCred = new Credential("foo", "bar"); + var existingCred = new Credential("baz", "bar"); + var newCred = new Credential("baz", "boz"); + var users = new List() { + new User("foo", "baz") { + Credentials = new List() { + existingCred, + frozenCred + } + } + }; + var service = new TestableUserService(); + service.MockUserRepository.HasData(users); + + // Act + service.ReplaceCredential("foo", newCred); + + // Assert + Assert.Equal(2, users[0].Credentials.Count); + Assert.Equal(new[] { frozenCred, newCred }, users[0].Credentials.ToArray()); + service.MockCredentialRepository + .Verify(x => x.DeleteOnCommit(existingCred)); + service.MockUserRepository.VerifyCommitted(); } } @@ -654,10 +906,39 @@ public void ResetsPasswordAndPasswordTokenAndPasswordTokenDate() bool result = userService.ResetPasswordWithToken("user", "some-token", "new-password"); Assert.True(result); - Assert.True(VerifyPasswordHash(user, "new-password")); + Assert.True(VerifyPasswordHash(user.HashedPassword, user.PasswordHashAlgorithm, "new-password")); Assert.Null(user.PasswordResetToken); Assert.Null(user.PasswordResetTokenExpirationDate); - userService.MockUserRepository.Verify(u => u.CommitChanges()); + userService.MockUserRepository.VerifyCommitted(); + } + + [Fact] + public void ResetsPasswordCredential() + { + var oldCred = CredentialBuilder.CreatePbkdf2Password("thePassword"); + var user = new User + { + Username = "user", + EmailAddress = "confirmed@example.com", + PasswordResetToken = "some-token", + PasswordResetTokenExpirationDate = DateTime.UtcNow.AddDays(1), + HashedPassword = oldCred.Value, + PasswordHashAlgorithm = Constants.PBKDF2HashAlgorithmId, + Credentials = new List() { oldCred } + }; + + var userService = new TestableUserService(); + userService.MockUserRepository + .Setup(r => r.GetAll()) + .Returns(new[] { user }.AsQueryable()); + + bool result = userService.ResetPasswordWithToken("user", "some-token", "new-password"); + + Assert.True(result); + var newCred = user.Credentials.Single(); + Assert.Equal(Constants.CredentialTypes.PasswordPbkdf2, newCred.Type); + Assert.True(VerifyPasswordHash(newCred.Value, Constants.PBKDF2HashAlgorithmId, "new-password")); + userService.MockUserRepository.VerifyCommitted(); } [Fact] @@ -681,11 +962,10 @@ public void ResetsPasswordMigratesPasswordHash() Assert.True(result); Assert.Equal("PBKDF2", user.PasswordHashAlgorithm); - Assert.True(VerifyPasswordHash(user, "new-password")); + Assert.True(VerifyPasswordHash(user.HashedPassword, user.PasswordHashAlgorithm, "new-password")); Assert.Null(user.PasswordResetToken); Assert.Null(user.PasswordResetTokenExpirationDate); - userService.MockUserRepository - .Verify(u => u.CommitChanges()); + userService.MockUserRepository.VerifyCommitted(); } } @@ -775,11 +1055,13 @@ public class TestableUserService : UserService { public Mock MockConfig { get; protected set; } public Mock> MockUserRepository { get; protected set; } + public Mock> MockCredentialRepository { get; protected set; } public TestableUserService() { Config = (MockConfig = new Mock()).Object; UserRepository = (MockUserRepository = new Mock>()).Object; + CredentialRepository = (MockCredentialRepository = new Mock>()).Object; // Set ConfirmEmailAddress to a default of true MockConfig.Setup(c => c.ConfirmEmailAddresses).Returns(true); diff --git a/tests/NuGetGallery.Facts/TestUtils/MockExtensions.cs b/tests/NuGetGallery.Facts/TestUtils/MockExtensions.cs index 24030b817b..c1a85805c0 100644 --- a/tests/NuGetGallery.Facts/TestUtils/MockExtensions.cs +++ b/tests/NuGetGallery.Facts/TestUtils/MockExtensions.cs @@ -1,4 +1,7 @@ -using Moq.Language.Flow; +using System.Collections.Generic; +using System.Linq; +using Moq; +using Moq.Language.Flow; namespace NuGetGallery { @@ -9,5 +12,28 @@ public static IReturnsResult ReturnsNull(this ISetup> HasData(this Mock> self, params T[] fakeData) + where T : class, IEntity, new() + { + return HasData(self, (IEnumerable)fakeData); + } + + public static IReturnsResult> HasData(this Mock> self, IEnumerable fakeData) + where T : class, IEntity, new() + { + return self.Setup(e => e.GetAll()).Returns(fakeData.AsQueryable()); + } + + public static void VerifyCommitted(this Mock> self) + where T : class, IEntity, new() + { + self.Verify(e => e.CommitChanges()); + } + + public static void VerifyCommitted(this Mock self) + { + self.Verify(e => e.SaveChanges()); + } } } diff --git a/tests/NuGetGallery.Facts/TestUtils/ResultAssert.cs b/tests/NuGetGallery.Facts/TestUtils/ResultAssert.cs index a2ca791a71..c227dc1faf 100644 --- a/tests/NuGetGallery.Facts/TestUtils/ResultAssert.cs +++ b/tests/NuGetGallery.Facts/TestUtils/ResultAssert.cs @@ -26,13 +26,12 @@ public static RedirectToRouteResult IsRedirectToRoute(ActionResult result, objec return redirect; } - public static ViewResult IsView(ActionResult result, string viewName = "", string masterName = "", object model = null, object viewData = null) + public static ViewResult IsView(ActionResult result, string viewName = "", string masterName = "", object viewData = null) { var view = Assert.IsType(result); Assert.Equal(viewName, view.ViewName); Assert.Equal(masterName, view.MasterName); - Assert.Equal(model, view.ViewData.Model); if (viewData != null) { @@ -45,9 +44,17 @@ public static ViewResult IsView(ActionResult result, string viewName = "", strin return view; } - private static void DictionariesMatch(IDictionary expected, IDictionary actual) + public static TModel IsView(ActionResult result, string viewName = "", string masterName = "", object viewData = null) { - var expectedKeys = expected.Keys.Cast().ToList(); + var model = Assert.IsType(IsView(result, viewName, masterName, viewData).Model); + return model; + } + + private static void DictionariesMatch(IDictionary expected, IDictionary actual) + { + var expectedKeys = new HashSet( + expected.Keys, + StringComparer.OrdinalIgnoreCase); foreach (var key in actual.Keys) { @@ -62,13 +69,29 @@ private static void DictionariesMatch(IDictionary expected, IDiction public static void IsStatusCode(ActionResult result, HttpStatusCode code) { - IsStatusCode(result, (int)code); + IsStatusCode(result, (int)code, description: null); } public static void IsStatusCode(ActionResult result, int code) { - var statusCodeResult = Assert.IsType(result); + IsStatusCode(result, code, description: null); + } + + public static void IsStatusCode(ActionResult result, HttpStatusCode code, string description) + { + IsStatusCode(result, (int)code, description); + } + + public static void IsStatusCode(ActionResult result, int code, string description) + { + var statusCodeResult = Assert.IsAssignableFrom(result); Assert.Equal(code, statusCodeResult.StatusCode); + Assert.Equal(description, statusCodeResult.StatusDescription); + } + + public static EmptyResult IsEmpty(ActionResult result) + { + return Assert.IsType(result); } } }