From 472373eebc8357ad6e307b557c8c37ccb428122e Mon Sep 17 00:00:00 2001 From: HardcoreMagazine Date: Mon, 4 Sep 2023 18:40:15 +0600 Subject: [PATCH] Error on login if credentials are bad or if email was not confirmed; Check, create (if not presnet) IdentityUserExtra entry in DB on login; Add warning, usernameChangeTokens in Admin Panel EditUser --- Areas/Identity/Pages/Account/Login.cshtml.cs | 68 ++++++++-- Areas/Identity/Pages/Account/Logout.cshtml.cs | 6 +- Controllers/AdminController.cs | 116 ++++++++++++++---- Program.cs | 7 +- README.md | 6 +- Views/Admin/{User.cshtml => EditUser.cshtml} | 29 +++-- Views/Admin/EditUser.cshtml.cs | 18 +++ Views/Admin/Users.cshtml | 2 +- 8 files changed, 199 insertions(+), 53 deletions(-) rename Views/Admin/{User.cshtml => EditUser.cshtml} (71%) create mode 100644 Views/Admin/EditUser.cshtml.cs diff --git a/Areas/Identity/Pages/Account/Login.cshtml.cs b/Areas/Identity/Pages/Account/Login.cshtml.cs index 89c4846..573e008 100644 --- a/Areas/Identity/Pages/Account/Login.cshtml.cs +++ b/Areas/Identity/Pages/Account/Login.cshtml.cs @@ -7,18 +7,25 @@ using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.EntityFrameworkCore; using SelenicSparkApp.CustomClasses; +using SelenicSparkApp.Data; namespace SelenicSparkApp.Areas.Identity.Pages.Account { public class LoginModel : PageModel { private readonly EmailSignInManager _signInManager; + private readonly UserManager _userManager; + private readonly ApplicationDbContext _context; private readonly ILogger _logger; - public LoginModel(EmailSignInManager signInManager, ILogger logger) + public LoginModel(EmailSignInManager signInManager, UserManager userManager, + ApplicationDbContext context, ILogger logger) { _signInManager = signInManager; + _userManager = userManager; + _context = context; _logger = logger; } @@ -78,6 +85,33 @@ public class InputModel public bool RememberMe { get; set; } } + /// + /// Checks if user has entry in IdentityUserExpander database, add if not. + /// ONLY CALL THIS FUNCTION WHEN YOU'RE ABSOLUTELY SURE ABOUT USER EXISTANCE!!! + /// + private async Task CheckOrAddExtraUserData() + { + var user = await _userManager.FindByEmailAsync(Input.Email); + if (user == null) + { + return; + } + // Create IdentityUserExpander entry if not exists - this would allow to exclude manual entry creation in the future + var userExtra = await _context.IdentityUserExpander.FirstOrDefaultAsync(u => u.UID == user.Id); + if (userExtra == null) + { + userExtra = new Models.IdentityUserExpander + { + UID = user.Id, + User = user, + UsernameChangeTokens = 1, + UserWarningsCount = 0 + }; + await _context.IdentityUserExpander.AddAsync(userExtra); + await _context.SaveChangesAsync(); + } + } + public async Task OnGetAsync(string returnUrl = null) { if (!string.IsNullOrEmpty(ErrorMessage)) @@ -105,17 +139,17 @@ public async Task OnPostAsync(string returnUrl = null) if (ModelState.IsValid) { - // This doesn't count login failures towards account lockout - // To enable password failures to trigger account lockout, set lockoutOnFailure: true - //var r = await _signInManager.signin - var result = await _signInManager.PasswordSignInAsync(Input.Email, Input.Password, Input.RememberMe, lockoutOnFailure: false); + // To disable password failures to trigger account lockout, set lockoutOnFailure: false + var result = await _signInManager.PasswordSignInAsync(Input.Email, Input.Password, Input.RememberMe, lockoutOnFailure: true); if (result.Succeeded) { _logger.LogInformation($"User \"{Input.Email}\"logged in."); + await CheckOrAddExtraUserData(); return LocalRedirect(returnUrl); } if (result.RequiresTwoFactor) { + await CheckOrAddExtraUserData(); // We most likely don't need that - users can't enable 2FA without logging in first return RedirectToPage("./LoginWith2fa", new { ReturnUrl = returnUrl, RememberMe = Input.RememberMe }); } if (result.IsLockedOut) @@ -125,8 +159,28 @@ public async Task OnPostAsync(string returnUrl = null) } else { - _logger.LogWarning($"Failed login attempt by {Input.Email}"); - ModelState.AddModelError(string.Empty, "Invalid login attempt."); + /* NOTE: THIS IS WHERE USERS WITHOUT CONFIRMED EMAIL END UP */ +#nullable enable + var user = await _userManager.FindByEmailAsync(Input.Email); + string errMsg; + if (user != null) + { + if (!user.EmailConfirmed) + { + errMsg = "Confirm your email to log in"; + } + else + { + errMsg = "Invalid login attempt."; + } + } + else + { + errMsg = "Invalid login attempt."; + } +#nullable disable + _logger.LogWarning($"Failed login attempt by {Input.Email}; Status: \"{errMsg}\""); + ModelState.AddModelError(string.Empty, errMsg); return Page(); } } diff --git a/Areas/Identity/Pages/Account/Logout.cshtml.cs b/Areas/Identity/Pages/Account/Logout.cshtml.cs index 8295001..a035c6e 100644 --- a/Areas/Identity/Pages/Account/Logout.cshtml.cs +++ b/Areas/Identity/Pages/Account/Logout.cshtml.cs @@ -2,13 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. #nullable disable -using System; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; -using Microsoft.Extensions.Logging; namespace SelenicSparkApp.Areas.Identity.Pages.Account { @@ -25,8 +21,8 @@ public LogoutModel(SignInManager signInManager, ILogger OnPost(string returnUrl = null) { + _logger.LogInformation($"User logged out."); await _signInManager.SignOutAsync(); - _logger.LogInformation("User logged out."); if (returnUrl != null) { return LocalRedirect(returnUrl); diff --git a/Controllers/AdminController.cs b/Controllers/AdminController.cs index 1501dac..481f0b4 100644 --- a/Controllers/AdminController.cs +++ b/Controllers/AdminController.cs @@ -1,6 +1,9 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; +using SelenicSparkApp.Data; +using SelenicSparkApp.Views.Admin; namespace SelenicSparkApp.Controllers { @@ -9,10 +12,12 @@ public class AdminController : Controller { private readonly UserManager _userManager; private readonly ILogger _logger; + private readonly ApplicationDbContext _context; - public AdminController(UserManager userManager, ILogger logger) + public AdminController(UserManager userManager, ILogger logger, ApplicationDbContext context) { _userManager = userManager; + _context = context; _logger = logger; } @@ -29,9 +34,9 @@ public ActionResult Users() } // GET: /Admin/User -- edit user page - public async Task User(string? id) + public async Task EditUser(string? id) { - if (id == null || !_userManager.Users.Any()) + if (string.IsNullOrWhiteSpace(id) || !_userManager.Users.Any()) { return NotFound(); } @@ -41,50 +46,119 @@ public async Task User(string? id) { return NotFound(); } - return View(user); + + var userExtra = await _context.IdentityUserExpander.FirstOrDefaultAsync(u => u.UID == id); + // Create entry if none found + if (userExtra == null) + { + // No data available - create & insert new default values + var defaultUserExtra = new Models.IdentityUserExpander + { + UID = id, + User = user, + UsernameChangeTokens = 1, + UserWarningsCount = 0 + }; + await _context.IdentityUserExpander.AddAsync(defaultUserExtra); + await _context.SaveChangesAsync(); + + var viewModel = new EditUserModel + { + Id = id, + UserName = user.UserName!, // No UserName nor Email can be null here, we did check at start + Email = user.Email!, + EmailConfirmed = user.EmailConfirmed, + LockoutEnd = user.LockoutEnd, + AccessFailedCount = user.AccessFailedCount, + UsernameChangeTokens = 1, + UserWarningsCount = 0 + }; + return View(viewModel); + } + else + { + var viewModel = new EditUserModel + { + Id = id, + UserName = user.UserName!, // No UserName nor Email can be null here, we did check at start + Email = user.Email!, + EmailConfirmed = user.EmailConfirmed, + LockoutEnd = user.LockoutEnd, + AccessFailedCount = user.AccessFailedCount, + UsernameChangeTokens = userExtra.UsernameChangeTokens, + UserWarningsCount = userExtra.UserWarningsCount + }; + return View(viewModel); + } } // POST: /Admin/User -- edit user page [HttpPost] [ValidateAntiForgeryToken] - public async Task User(string id, [Bind("Id,UserName,Email,EmailConfirmed,LockoutEnd,AccessFailedCount")] IdentityUser user) + public async Task EditUser(string id, [Bind("Id,UserName,Email,EmailConfirmed,LockoutEnd," + + "AccessFailedCount,UsernameChangeTokens,UserWarningsCount")] EditUserModel expandedUser) { - if (id != user.Id) // Block cross-editing + if (id != expandedUser.Id) // Block cross-editing { return NotFound(); } if (ModelState.IsValid) { - var selectedUser = await _userManager.FindByIdAsync(user.Id); + var selectedUser = await _userManager.FindByIdAsync(expandedUser.Id); + var selectedUserExtras = await _context.IdentityUserExpander.FirstOrDefaultAsync(u => u.UID == id); + if (selectedUserExtras == null) // Failsafe + { + selectedUserExtras = new Models.IdentityUserExpander + { + UID = selectedUser!.Id, + User = selectedUser!, + UsernameChangeTokens = 1, + UserWarningsCount = 0 + }; + } + if (selectedUser != null) { - if (selectedUser.UserName != user.UserName & !string.IsNullOrWhiteSpace(user.UserName)) + if (selectedUser.UserName != expandedUser.UserName & !string.IsNullOrWhiteSpace(expandedUser.UserName)) { // User will see new nickname (Username) only on re-login - selectedUser.UserName = user.UserName; + selectedUser.UserName = expandedUser.UserName; // All 'Normalized' fields must be updated accordingly, // they DO NOT follow their 'Not normal' values on update - selectedUser.NormalizedUserName = user.UserName!.ToUpper(); + selectedUser.NormalizedUserName = expandedUser.UserName!.ToUpper(); + } + if (selectedUser.Email != expandedUser.Email & !string.IsNullOrWhiteSpace(expandedUser.Email)) + { + selectedUser.Email = expandedUser.Email; + selectedUser.NormalizedEmail = expandedUser.Email!.ToUpper(); + } + if (selectedUser.EmailConfirmed != expandedUser.EmailConfirmed) + { + selectedUser.EmailConfirmed = expandedUser.EmailConfirmed; } - if (selectedUser.Email != user.Email & !string.IsNullOrWhiteSpace(user.Email)) + if (selectedUser.LockoutEnd != expandedUser.LockoutEnd) // LockoutEnd can be NULL { - selectedUser.Email = user.Email; - selectedUser.NormalizedEmail = user.Email!.ToUpper(); + selectedUser.LockoutEnd = expandedUser.LockoutEnd; } - if (selectedUser.EmailConfirmed != user.EmailConfirmed) + if (selectedUser.AccessFailedCount != expandedUser.AccessFailedCount & expandedUser.AccessFailedCount >= 0) { - selectedUser.EmailConfirmed = user.EmailConfirmed; + selectedUser.AccessFailedCount = expandedUser.AccessFailedCount; } - if (selectedUser.LockoutEnd != user.LockoutEnd) // LockoutEnd can be NULL + if (selectedUserExtras.UsernameChangeTokens != expandedUser.UsernameChangeTokens & expandedUser.UsernameChangeTokens >= 0) { - selectedUser.LockoutEnd = user.LockoutEnd; + selectedUserExtras.UsernameChangeTokens = expandedUser.UsernameChangeTokens; } - if (selectedUser.AccessFailedCount != user.AccessFailedCount & user.AccessFailedCount >= 0) + if (selectedUserExtras.UserWarningsCount != expandedUser.UserWarningsCount & expandedUser.UserWarningsCount >= 0) { - selectedUser.AccessFailedCount = user.AccessFailedCount; + selectedUserExtras.UserWarningsCount = expandedUser.UserWarningsCount; } + // Update extra data fields first + _context.IdentityUserExpander.Update(selectedUserExtras); + await _context.SaveChangesAsync(); + + // Update main fields var result = await _userManager.UpdateAsync(selectedUser); if (result.Succeeded) { @@ -98,7 +172,7 @@ public async Task User(string id, [Bind("Id,UserName,Email,EmailC { errorLog += $"{err.Description}; "; } - _logger.LogWarning($"Failed to update {user.Id}, ErrLog: {errorLog}"); + _logger.LogWarning($"Failed to update {expandedUser.Id}, ErrLog: {errorLog}"); return BadRequest(); } } @@ -107,7 +181,7 @@ public async Task User(string id, [Bind("Id,UserName,Email,EmailC return NotFound(); } } - return View(user); + return View(expandedUser); } // GET: /Admin/DeleteUser diff --git a/Program.cs b/Program.cs index 0a28fb5..960ba24 100644 --- a/Program.cs +++ b/Program.cs @@ -11,7 +11,12 @@ options.UseSqlServer(connectionString)); builder.Services.AddDatabaseDeveloperPageExceptionFilter(); -builder.Services.AddDefaultIdentity(options => options.SignIn.RequireConfirmedAccount = true) +builder.Services.AddDefaultIdentity(options => +{ + options.SignIn.RequireConfirmedAccount = true; + options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromMinutes(15); + options.Lockout.MaxFailedAccessAttempts = 5; +}) .AddRoles() .AddSignInManager() // Custom sign-in manager .AddEntityFrameworkStores(); diff --git a/README.md b/README.md index 3ecc871..370a4b3 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,10 @@ Write cool stuff in here :P TODO: - ~~Fix "change username" for users~~ - ~~Add "usernameChangeTokens" to migration & DB, integrate it inside user settings~~ -- Add warning-usernameChangeTokens in Admin panel user management +- ~~Add warning-usernameChangeTokens in Admin panel user management~~ - Add role manger in Admin panel + protection to admin, moderator, user roles. -- Create/check for "extra" IdentityUser fields on login -- Fix/show msg for users w/o confirmed email instead of "Invalid login" on login page +- ~~Create/check for "extra" IdentityUser fields on login~~ +- ~~Fix/show msg for users w/o confirmed email instead of "Invalid login" on login page~~ - Build feed and post view ground-up - Add moderator tools for user's posts diff --git a/Views/Admin/User.cshtml b/Views/Admin/EditUser.cshtml similarity index 71% rename from Views/Admin/User.cshtml rename to Views/Admin/EditUser.cshtml index 4f8d632..f5b4160 100644 --- a/Views/Admin/User.cshtml +++ b/Views/Admin/EditUser.cshtml @@ -1,10 +1,7 @@ @using Microsoft.AspNetCore.Identity -@using SelenicSparkApp.Data @* Normally I would advise against using that *@ -@model IdentityUser +@model SelenicSparkApp.Views.Admin.EditUserModel @inject SignInManager SignInManager -@inject ApplicationDbContext AppDbContext // Normally I would advise against using that *@ - @{ ViewData["Title"] = "Edit user"; } @@ -14,18 +11,15 @@ @if (User.IsInRole("Admin")) // Triple-checking never hurt anyone {
-
-
@* <- do we even need that? *@ +
- - +
- - +
@@ -34,17 +28,22 @@ Yes No -
- - +
- - + +
+
+ + +
+
+ +
diff --git a/Views/Admin/EditUser.cshtml.cs b/Views/Admin/EditUser.cshtml.cs new file mode 100644 index 0000000..84ef902 --- /dev/null +++ b/Views/Admin/EditUser.cshtml.cs @@ -0,0 +1,18 @@ +using System.ComponentModel.DataAnnotations; + +namespace SelenicSparkApp.Views.Admin +{ + public class EditUserModel + { + // IdentityUser + public required string Id { get; set; } + public required string UserName { get; set; } + public required string Email { get; set; } + public required bool EmailConfirmed { get; set; } + public DateTimeOffset? LockoutEnd { get; set; } + public required int AccessFailedCount { get; set; } + // IdentityUserExpander + public required int UsernameChangeTokens { get; set; } + public required int UserWarningsCount { get; set; } + } +} diff --git a/Views/Admin/Users.cshtml b/Views/Admin/Users.cshtml index 2e8b8f5..84cd979 100644 --- a/Views/Admin/Users.cshtml +++ b/Views/Admin/Users.cshtml @@ -83,7 +83,7 @@ @user.EmailConfirmed @user.LockoutEnd - EDIT + EDIT DEL