diff --git a/starsky/build/helpers/SonarQube.cs b/starsky/build/helpers/SonarQube.cs index da736677a9..dcab72424e 100644 --- a/starsky/build/helpers/SonarQube.cs +++ b/starsky/build/helpers/SonarQube.cs @@ -28,7 +28,7 @@ public static class SonarQube /// /// @see: https://www.nuget.org/packages/dotnet-sonarscanner /// - public const string SonarQubePackageVersion = "6.2.0"; + public const string SonarQubePackageVersion = "8.0.0"; public const string SonarQubeDotnetSonarScannerApi = "https://api.nuget.org/v3-flatcontainer/dotnet-sonarscanner/index.json"; diff --git a/starsky/starsky.feature.webhtmlpublish/starsky.feature.webhtmlpublish.csproj b/starsky/starsky.feature.webhtmlpublish/starsky.feature.webhtmlpublish.csproj index 88ed54dd50..d4ad7e6209 100644 --- a/starsky/starsky.feature.webhtmlpublish/starsky.feature.webhtmlpublish.csproj +++ b/starsky/starsky.feature.webhtmlpublish/starsky.feature.webhtmlpublish.csproj @@ -21,7 +21,7 @@ - + diff --git a/starsky/starsky.foundation.accountmanagement/Helpers/Pbkdf2Hasher.cs b/starsky/starsky.foundation.accountmanagement/Helpers/Pbkdf2Hasher.cs index be57fb95f3..21592ebd8a 100644 --- a/starsky/starsky.foundation.accountmanagement/Helpers/Pbkdf2Hasher.cs +++ b/starsky/starsky.foundation.accountmanagement/Helpers/Pbkdf2Hasher.cs @@ -22,7 +22,7 @@ public static string ComputeHash(string password, byte[] salt) password: password, salt: salt, prf: KeyDerivationPrf.HMACSHA1, - iterationCount: 10000, + iterationCount: 100_000, numBytesRequested: 256 / 8 ) ); diff --git a/starsky/starsky.foundation.platform/starsky.foundation.platform.csproj b/starsky/starsky.foundation.platform/starsky.foundation.platform.csproj index 87f13f5aac..2b15166f5c 100644 --- a/starsky/starsky.foundation.platform/starsky.foundation.platform.csproj +++ b/starsky/starsky.foundation.platform/starsky.foundation.platform.csproj @@ -25,7 +25,7 @@ - + diff --git a/starsky/starsky.foundation.thumbnailgeneration/starsky.foundation.thumbnailgeneration.csproj b/starsky/starsky.foundation.thumbnailgeneration/starsky.foundation.thumbnailgeneration.csproj index 77d71d8964..57284ab390 100644 --- a/starsky/starsky.foundation.thumbnailgeneration/starsky.foundation.thumbnailgeneration.csproj +++ b/starsky/starsky.foundation.thumbnailgeneration/starsky.foundation.thumbnailgeneration.csproj @@ -19,7 +19,7 @@ - + diff --git a/starsky/starsky.foundation.webtelemetry/starsky.foundation.webtelemetry.csproj b/starsky/starsky.foundation.webtelemetry/starsky.foundation.webtelemetry.csproj index 82643709a8..2099e778fc 100644 --- a/starsky/starsky.foundation.webtelemetry/starsky.foundation.webtelemetry.csproj +++ b/starsky/starsky.foundation.webtelemetry/starsky.foundation.webtelemetry.csproj @@ -25,13 +25,13 @@ - - - - - + + + + + - + diff --git a/starsky/starsky/Controllers/AccountController.cs b/starsky/starsky/Controllers/AccountController.cs index b723387ccb..0063165bd8 100644 --- a/starsky/starsky/Controllers/AccountController.cs +++ b/starsky/starsky/Controllers/AccountController.cs @@ -7,7 +7,6 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using starsky.foundation.accountmanagement.Interfaces; -using starsky.foundation.accountmanagement.Models; using starsky.foundation.accountmanagement.Models.Account; using starsky.foundation.platform.Models; using starsky.foundation.storage.Interfaces; @@ -23,6 +22,8 @@ public sealed class AccountController : Controller private readonly IAntiforgery _antiForgery; private readonly IStorage _storageHostFullPathFilesystem; + private const string ModelError = "Model is not correct"; + public AccountController(IUserManager userManager, AppSettings appSettings, IAntiforgery antiForgery, ISelectorStorage selectorStorage) { @@ -34,7 +35,7 @@ public AccountController(IUserManager userManager, AppSettings appSettings, } /// - /// Check the account status of the current logged in user + /// Check the account status of the current logged-in user /// /// Logged in /// When not logged in @@ -113,6 +114,8 @@ public async Task Status() [AllowAnonymous] public IActionResult LoginGet(string? returnUrl = null, bool? fromLogout = null) { + if ( !ModelState.IsValid ) return BadRequest(ModelError); + new AntiForgeryCookie(_antiForgery).SetAntiForgeryCookie(HttpContext); var clientApp = Path.Combine(_appSettings.BaseDirectoryProject, "clientapp", "build", "index.html"); @@ -143,7 +146,9 @@ public IActionResult LoginGet(string? returnUrl = null, bool? fromLogout = null) [AllowAnonymous] public async Task LoginPost(LoginViewModel model) { - ValidateResult validateResult = + if ( !ModelState.IsValid ) return BadRequest(ModelError); + + var validateResult = await _userManager.ValidateAsync("Email", model.Email, model.Password); if ( !validateResult.Success ) @@ -192,6 +197,8 @@ public IActionResult LogoutJson() [AllowAnonymous] public IActionResult Logout(string? returnUrl = null) { + if ( !ModelState.IsValid ) return BadRequest(ModelError); + _userManager.SignOut(HttpContext); // fromLogout is used in middleware return RedirectToAction(nameof(LoginGet), @@ -205,7 +212,7 @@ public IActionResult Logout(string? returnUrl = null) /// Change secret status /// successful login /// Model is not correct - /// please login first or your current password is not correct + /// please log in first or your current password is not correct [HttpPost("/api/account/change-secret")] [ProducesResponseType(typeof(string), 200)] [ProducesResponseType(typeof(string), 400)] @@ -222,7 +229,7 @@ public async Task ChangeSecret(ChangePasswordViewModel model) if ( !ModelState.IsValid || model.ChangedPassword != model.ChangedConfirmPassword ) { - return BadRequest("Model is not correct"); + return BadRequest(ModelError); } var currentUserId = _userManager.GetCurrentUser(HttpContext)?.Id; @@ -280,7 +287,7 @@ public async Task Register(RegisterViewModel model) // If we got this far, something failed, redisplay form Response.StatusCode = 400; - return Json("Model is not correct"); + return Json(ModelError); } /// diff --git a/starsky/starsky/starsky.csproj b/starsky/starsky/starsky.csproj index 1f1ae798fa..cfd57caa40 100644 --- a/starsky/starsky/starsky.csproj +++ b/starsky/starsky/starsky.csproj @@ -61,7 +61,7 @@ - + diff --git a/starsky/starskytest/Controllers/AccountControllerTest.cs b/starsky/starskytest/Controllers/AccountControllerTest.cs index 538050472c..8e4528dd97 100644 --- a/starsky/starskytest/Controllers/AccountControllerTest.cs +++ b/starsky/starskytest/Controllers/AccountControllerTest.cs @@ -116,7 +116,7 @@ private static ClaimsPrincipal SetTestClaimsSet(string name, string id) public async Task AccountController_NoLogin_Login_And_newAccount_Test() { // Arrange - var userId = "TestUserA"; + const string userId = "TestUserA"; var claims = new List { new Claim(ClaimTypes.NameIdentifier, userId) }; var httpContext = _serviceProvider.GetRequiredService() @@ -168,8 +168,8 @@ public async Task AccountController_NoLogin_Login_And_newAccount_Test() // The logout is mocked so this will not actual log it out and controller.Logout() not crashing is good enough controller.Logout(); - // And clean afterwards - var itemWithId = _dbContext.Users.FirstOrDefault(p => p.Name == newAccount.Name); + // And clean afterward + var itemWithId = await _dbContext.Users.FirstOrDefaultAsync(p => p.Name == newAccount.Name); Assert.IsNotNull(itemWithId); _dbContext.Users.Remove(itemWithId); await _dbContext.SaveChangesAsync(); @@ -467,7 +467,7 @@ public async Task AccountController_LoginContext_GetRegisterPage_AccountCreated( Assert.IsNotNull(actionResult); Assert.AreEqual("Account Created", actionResult.Value); - var getUser = _dbContext.Users.FirstOrDefault(p => p.Name == user.Name); + var getUser = await _dbContext.Users.FirstOrDefaultAsync(p => p.Name == user.Name); Assert.IsNotNull(getUser); _dbContext.Users.Remove(getUser); await _dbContext.SaveChangesAsync(); @@ -515,8 +515,8 @@ public async Task AccountController_IndexGetLoginSuccessful() await controller.Status(); Assert.AreEqual(200, controller.Response.StatusCode); - // And clean afterwards - var getUser = _dbContext.Users.FirstOrDefault(p => p.Name == user.Name); + // And clean afterward + var getUser = await _dbContext.Users.FirstOrDefaultAsync(p => p.Name == user.Name); Assert.IsNotNull(getUser); _dbContext.Users.Remove(getUser); await _dbContext.SaveChangesAsync(); @@ -629,7 +629,6 @@ public async Task AccountController_newAccount_TryToOverwrite_ButItKeepsTheSameP // controller.Logout() not crashing is good enough controller.LogoutJson(); - var newAccountDuplicate = new RegisterViewModel { Password = "test11234567890", // DIFFERENT @@ -650,7 +649,7 @@ public async Task AccountController_newAccount_TryToOverwrite_ButItKeepsTheSameP controller.LogoutJson(); // Clean afterwards - var user = _dbContext.Users.FirstOrDefault(p => p.Name == userId); + var user = await _dbContext.Users.FirstOrDefaultAsync(p => p.Name == userId); Assert.IsNotNull(user); _dbContext.Users.Remove(user); await _dbContext.SaveChangesAsync(); @@ -821,5 +820,95 @@ public void Permissions() Assert.IsNotNull(expectedPermission); Assert.AreEqual(expectedPermission, list.FirstOrDefault()); } + + [TestMethod] + public void Logout_ModelStateIsInvalid_ReturnsBadRequest() + { + // Arrange + var controller = + new AccountController(_userManager, _appSettings, _antiForgery, _selectorStorage) + { + ControllerContext = + { + HttpContext = new DefaultHttpContext() + } + }; + controller.ModelState.AddModelError("Key", "ErrorMessage"); + + // Act + var result = controller.Logout(); + + // Assert + Assert.IsInstanceOfType(result); + } + + [TestMethod] + public void LoginGet_ModelStateIsInvalid_ReturnsBadRequest() + { + // Arrange + var controller = + new AccountController(_userManager, _appSettings, _antiForgery, _selectorStorage) + { + ControllerContext = + { + HttpContext = new DefaultHttpContext() + } + }; + controller.ModelState.AddModelError("Key", "ErrorMessage"); + + // Act + var result = controller.LoginGet(); + + // Assert + Assert.IsInstanceOfType(result); + } + + [TestMethod] + public async Task ChangeSecret_ModelStateIsInvalid_ReturnsBadRequest() + { + // Arrange + var controller = + new AccountController(_userManager, _appSettings, _antiForgery, _selectorStorage) + { + ControllerContext = + { + HttpContext = new DefaultHttpContext + { + User = SetTestClaimsSet("test", "1") + } + } + }; + controller.ModelState.AddModelError("Key", "ErrorMessage"); + + // Act + var result = await controller.ChangeSecret(new ChangePasswordViewModel()); + + // Assert + Assert.IsInstanceOfType(result); + } + + [TestMethod] + public async Task LoginPost_ModelStateIsInvalid_ReturnsBadRequest() + { + // Arrange + var controller = + new AccountController(_userManager, _appSettings, _antiForgery, _selectorStorage) + { + ControllerContext = + { + HttpContext = new DefaultHttpContext + { + User = SetTestClaimsSet("test", "1") + } + } + }; + controller.ModelState.AddModelError("Key", "ErrorMessage"); + + // Act + var result = await controller.LoginPost(new LoginViewModel()); + + // Assert + Assert.IsInstanceOfType(result); + } } } diff --git a/starsky/starskytest/starskytest.csproj b/starsky/starskytest/starskytest.csproj index 20627a1bc6..7b1c6da3b2 100644 --- a/starsky/starskytest/starskytest.csproj +++ b/starsky/starskytest/starskytest.csproj @@ -25,9 +25,9 @@ - - - + + + all runtime; build; native; contentfiles; analyzers; buildtransitive