Report every sign-in and route JWT sign-in through CustomSignInManager

DapperIdentity.Cookies.Server/DapperIdentityCookieServiceCollectionExtensions.cs

@@ -89,6 +89,7 @@ public static IServiceCollection AddDapperIdentityWithCustomCookies(this IServic
     {
 
         services.TryAddDapperIdentityDatabaseStores();
+        services.TryAddSignInReporter(); // CustomSignInManager reports each sign-in.
         services.AddIdentity<IdentityUser, IdentityRole>()
             .AddDefaultTokenProviders()
             .AddSignInManager<CustomSignInManager>()
@@ -151,9 +152,13 @@ public static IServiceCollection AddDapperIdentityWithVanillaUIAndDefaults(this
                                                             bool slidingExpiration = true)
     {
         services.TryAddDapperIdentityDatabaseStores();
+        services.TryAddSignInReporter(); // CustomSignInManager reports each sign-in.
 
+        // CustomSignInManager here too: without it the IsEnabled column is ignored and sign-ins
+        // through the Identity UI pages are not reported.
         services.AddIdentity<IdentityUser, IdentityRole>()
             //.AddDefaultUI()
+            .AddSignInManager<CustomSignInManager>()
             .AddDefaultTokenProviders();
 
         services.Configure<IdentityOptions>(opts =>

DapperIdentity.Jwt.Server/Controllers/JwtAuthController.cs

@@ -1,4 +1,5 @@
-using CPE.DapperIdentity.Stores.Models;
+using CPE.DapperIdentity.Stores;
+using CPE.DapperIdentity.Stores.Models;
 using CPE.DapperIdentity.Abstractions.Models;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Http;
@@ -50,6 +51,8 @@
 
     private readonly IOptions<DataProtectionTokenProviderOptions> _TokenOptions;
 
+    private readonly CustomSignInManager _SignInManager;
+
     /// <summary>Captures the services the endpoints need.</summary>
     /// <param name="userManager">Identity's user manager.</param>
     /// <param name="tokenService">Issues access and refresh tokens.</param>
@@ -64,14 +67,19 @@
     /// The token provider's settings, read for the link lifetime quoted in emails - the value the
     /// server actually enforces, however it was set.
     /// </param>
+    /// <param name="signInManager">
+    /// Checks a sign-in the way the cookie sign-in does (enabled, confirmed, not locked out, then the
+    /// password) and reports it.
+    /// </param>
     public JwtAuthController(UserManager<IdentityUser> userManager,
                              TokenService tokenService,
                              IAuthEmailSender emailSender,
                              ILogger<JwtAuthController> logger,
                              IAppSettings appSettings,
                              AppBaseUrl appBaseUrl,
                              PasswordResetRateLimiter resetRateLimiter,
-                             IOptions<DataProtectionTokenProviderOptions> tokenOptions)//Todo: Add options, IOptions<JWTControllerOptions> options) //ApplicationDbContext context
+                             IOptions<DataProtectionTokenProviderOptions> tokenOptions,
+                             CustomSignInManager signInManager)//Todo: Add options, IOptions<JWTControllerOptions> options) //ApplicationDbContext context
     {
         _TokenOptions = tokenOptions;
         _userManager = userManager;
@@ -82,6 +90,7 @@
         _AppSettings = appSettings;
         _AppBaseUrl = appBaseUrl;
         _ResetRateLimiter = resetRateLimiter;
+        _SignInManager = signInManager;
     }
 
     /// <summary>
@@ -117,8 +126,8 @@
 
         if (result.Succeeded)
         {
             var user = await _userManager.FindByEmailAsync(request.Email);
             request.Id = user.Id;
             request.Password = "";
 
             //add claims
@@ -184,7 +193,7 @@
         var callbackUrl = QueryHelpers.AddQueryString(resetPage.AbsoluteUri, "code", code);
 
         await _EmailSender.SendEmailAsync(
             user.Email,
             subject,
             $"<p>{opening} <a href='{HtmlEncoder.Default.Encode(callbackUrl)}'>clicking here</a>.</p>" +
             $"<p>{closing}</p>");
@@ -232,7 +241,7 @@
             return StatusCode(StatusCodes.Status429TooManyRequests);
         }
 
         var user = await _userManager.FindByEmailAsync(forgotPasswordRequest.Email);
         if (user == null || !(await _userManager.IsEmailConfirmedAsync(user)))
         {
             // Don't reveal that the user does not exist or is not confirmed
@@ -339,18 +348,15 @@
             return BadRequest(ModelState);
         }
 
-        var managedUser = await _userManager.FindByEmailAsync(request.Email!);
+        // Through the sign-in manager, not UserManager.CheckPasswordAsync: that checks only the
+        // password hash, so a disabled, unconfirmed or locked-out user used to get a token. Every
+        // refusal answers alike, so the reply does not say which accounts exist or are disabled.
+        var (_, managedUser) = await _SignInManager.CheckPasswordByEmailAsync(request.Email!, request.Password!);
         if (managedUser == null)
         {
             return BadRequest("Bad credentials");
         }
 
-        var isPasswordValid = await _userManager.CheckPasswordAsync(managedUser, request.Password!);
-        if (!isPasswordValid)
-        {
-            return BadRequest("Bad credentials");
-        }
-
         var userInDb = managedUser; //why search again?//_userManager.Users.FirstOrDefault(u=>u.Email==) //_context.Users.FirstOrDefault(u => u.Email == request.Email);
 
         if (userInDb is null)
@@ -414,6 +420,11 @@
         var username = principal.Identity!.Name; //do we need to null check on Identity?
         var user = await _userManager.FindByNameAsync(username);// EmailAsync(username);
         if (user == null || user.RefreshToken != tokenDto.RefreshToken || user.RefreshTokenExpireTime <= DateTime.Now)
+            return BadRequest("Invalid access token or refresh token");
+
+        // A refresh has no password to check, so without this a user disabled or locked out after
+        // signing in would keep renewing tokens for as long as they kept refreshing.
+        if (!await _SignInManager.AllowsSignInAsync(user))
             return BadRequest("Invalid access token or refresh token");//return BadRequest(new AuthResponseDto { IsAuthSuccessful = false, ErrorMessage = "Invalid client request" });
 
         var roles = await _userManager.GetRolesAsync(user);

DapperIdentity.Jwt.Server/ServiceCollectionExtensions.cs

@@ -88,6 +88,7 @@ public static IServiceCollection AddJwtIdentity(this IServiceCollection services
                 .ValidateOnStart();
 
         services.TryAddDapperIdentityDatabaseStores();
+        services.TryAddSignInReporter(); // JwtAuthController reports each sign-in.
         services.AddScoped<TokenService>();
         // Route JwtAuthController and nothing else from this assembly. Adding the AssemblyPart on
         // its own would hand the consumer every controller this library happens to contain, now

DapperIdentity.Stores/CustomSignInManager.cs

@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
+using CPE.DapperIdentity.Stores.SignIn;
 using IdentityUser = CPE.DapperIdentity.Stores.Models.CustomIdentityUser;
 
 
@@ -17,12 +18,22 @@ namespace CPE.DapperIdentity.Stores
     /// <see cref="SignInManager{TUser}"/> extended with the library's own IsEnabled check.
     /// </summary>
     /// <remarks>
-    /// The only behaviour added is in <see cref="PreSignInCheck"/>: a user whose IsEnabled column
-    /// is false is refused even when the password is correct. Register it in place of the stock
-    /// sign-in manager, or the column has no effect.
+    /// <para>
+    /// <see cref="PreSignInCheck"/> refuses a user whose IsEnabled column is false even when the
+    /// password is correct. Register it in place of the stock sign-in manager, or the column has no effect.
+    /// </para>
+    /// <para>
+    /// It is also the one place sign-ins are reported (<see cref="ISignInReporter"/>): cookie sign-ins
+    /// through <see cref="PasswordSignInAsync(string, string, bool, bool)"/>, token endpoints through
+    /// <see cref="CheckPasswordByEmailAsync"/>. Token endpoints must use that rather than
+    /// <c>UserManager.CheckPasswordAsync</c>, which checks only the password hash and so lets a
+    /// disabled, unconfirmed or locked-out user through.
+    /// </para>
     /// </remarks>
     public class CustomSignInManager : SignInManager<IdentityUser> 
     {
+        private readonly ISignInReporter? _signInReporter;
+
         /// <summary>Passes every dependency through to the base sign-in manager.</summary>
         /// <param name="userManager">The user manager.</param>
         /// <param name="contextAccessor">Accessor for the current HTTP context.</param>
@@ -40,6 +51,102 @@ public CustomSignInManager(UserManager<IdentityUser> userManager,
             IUserConfirmation<IdentityUser> confirmation) : base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation)
         { }
 
+        /// <summary>As the other constructor, and reports every sign-in to <paramref name="signInReporter"/>.</summary>
+        /// <param name="userManager">The user manager.</param>
+        /// <param name="contextAccessor">Accessor for the current HTTP context.</param>
+        /// <param name="claimsFactory">Builds the claims principal for a signed-in user.</param>
+        /// <param name="optionsAccessor">The configured Identity options.</param>
+        /// <param name="logger">Logger for the base sign-in manager.</param>
+        /// <param name="schemes">The registered authentication schemes.</param>
+        /// <param name="confirmation">Decides whether a user counts as confirmed.</param>
+        /// <param name="signInReporter">Told of each sign-in, so a tool like PortGuardian can ban an address that keeps failing.</param>
+        public CustomSignInManager(UserManager<IdentityUser> userManager,
+            IHttpContextAccessor contextAccessor,
+            IUserClaimsPrincipalFactory<IdentityUser> claimsFactory,
+            IOptions<IdentityOptions> optionsAccessor,
+            ILogger<SignInManager<IdentityUser>> logger,
+            IAuthenticationSchemeProvider schemes,
+            IUserConfirmation<IdentityUser> confirmation,
+            ISignInReporter signInReporter) : base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation)
+        {
+            _signInReporter = signInReporter;
+        }
+
+        /// <summary>
+        /// Signs in by user name, as the base does, and reports the attempt. An unknown name is
+        /// reported here because it never reaches the password check.
+        /// </summary>
+        /// <inheritdoc />
+        public override async Task<SignInResult> PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure)
+        {
+            var user = await UserManager.FindByNameAsync(userName);
+            if (user is null)
+            {
+                Report(SignInAttempt.Failure(CurrentContext(), userName, SignInFailure.UnknownUser, "cookie"));
+                return SignInResult.Failed;
+            }
+
+            var result = await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure);
+            Report(SignInAttempt.FromSignInResult(result, CurrentContext(), userName, "cookie"));
+            return result;
+        }
+
+        /// <summary>
+        /// Checks an email and password the way a sign-in does - enabled, confirmed, not locked out,
+        /// then the password - without issuing a cookie, and reports the attempt. For token endpoints,
+        /// so they refuse exactly whom a cookie sign-in refuses.
+        /// </summary>
+        /// <param name="email">The email as typed.</param>
+        /// <param name="password">The password as typed.</param>
+        /// <param name="path">Which endpoint, for the report.</param>
+        /// <returns>The user when the check passed; otherwise no user and the reason in Result.</returns>
+        public async Task<(SignInResult Result, IdentityUser? User)> CheckPasswordByEmailAsync(string email, string password, string path = "jwt")
+        {
+            var user = await UserManager.FindByEmailAsync(email);
+            if (user is null)
+            {
+                Report(SignInAttempt.Failure(CurrentContext(), email, SignInFailure.UnknownUser, path));
+                return (SignInResult.Failed, null);
+            }
+
+            // No lockout counting, as the cookie sign-in (lockoutOnFailure: false); PortGuardian bans
+            // the address instead, which does not let an attacker lock a real user out.
+            var result = await CheckPasswordSignInAsync(user, password, lockoutOnFailure: false);
+            Report(SignInAttempt.FromSignInResult(result, CurrentContext(), email, path));
+            return (result, result.Succeeded ? user : null);
+        }
+
+        /// <summary>
+        /// Whether the user may sign in now - enabled, confirmed, not locked out. For a token refresh,
+        /// which has no password to check but must stop working for a user who was disabled.
+        /// </summary>
+        /// <param name="user">The user whose token is being refreshed.</param>
+        public async Task<bool> AllowsSignInAsync(IdentityUser user) => await PreSignInCheck(user) is null;
+
+        private HttpContext? CurrentContext()
+        {
+            // The base throws when there is no request (a background job signing someone in); a
+            // report without an address is still worth its log line.
+            try { return Context; }
+            catch (InvalidOperationException) { return null; }
+        }
+
+        private void Report(SignInAttempt? attempt)
+        {
+            if (attempt is null || _signInReporter is null)
+                return;
+            try
+            {
+                _signInReporter.Report(attempt);
+            }
+#pragma warning disable CA1031 // A host's own reporter must not be able to break a sign-in either.
+            catch (Exception ex)
+#pragma warning restore CA1031
+            {
+                Logger.LogWarning(ex, "The sign-in reporter failed; the sign-in itself was not affected.");
+            }
+        }
+
 
         /// <summary>
         /// Used to ensure that a user is allowed to sign in.

DapperIdentity.Stores/DapperIdentity.Stores.csproj

@@ -42,6 +42,11 @@
     <PackageReference Include="TheDapperRepository" Version="1.4.1" />
   </ItemGroup>
 
+  <ItemGroup>
+    <!-- The sign-in reporter's event-log write is swapped out in tests through an internal constructor. -->
+    <InternalsVisibleTo Include="DapperIdentity.Tests" />
+  </ItemGroup>
+
   <ItemGroup>
     <!-- ICustomIdentityUser and IAuthEmailSender live in Abstractions: they are contracts a
          consumer implements, so they must be reachable without taking the Dapper stack. -->

DapperIdentity.Stores/DapperIdentityServiceCollectionExtensions.cs

@@ -1,8 +1,10 @@
 using CPE.DapperIdentity.Stores;
+using CPE.DapperIdentity.Stores.SignIn;
 using DapperRepository;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Options;
 
 using IdentityRole = CPE.DapperIdentity.Stores.Models.CustomIdentityRole;
 using IdentityUser = CPE.DapperIdentity.Stores.Models.CustomIdentityUser;
@@ -47,4 +49,21 @@ public static IServiceCollection TryAddDapperIdentityDatabaseStores(this IServic
 
         return services;
     }
+
+    /// <summary>
+    /// Registers the default <see cref="ISignInReporter"/>: a log line on every OS, and on Windows an
+    /// Application-log event PortGuardian reads to ban an address that keeps failing.
+    /// </summary>
+    /// <remarks>
+    /// Called by every sign-in set-up in these packages. TryAdd, so a host that registered its own
+    /// reporter first (to feed another tool, or to report nothing) keeps it.
+    /// </remarks>
+    public static IServiceCollection TryAddSignInReporter(this IServiceCollection services)
+    {
+        // Names are hashed unless DapperIdentity:SignInReporting:UserNames (or a Configure call) says otherwise.
+        services.AddOptions<SignInReportingOptions>();
+        services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<SignInReportingOptions>, SignInReportingOptionsFromConfiguration>());
+        services.TryAddSingleton<ISignInReporter, SignInReporter>();
+        return services;
+    }
 }