feat(auth): disable the login when a worker is resigned, and refuse disabled accounts

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/GrpcServices/TimePlanningAuthGrpcServiceTests.cs

@@ -1,3 +1,4 @@
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.Options;
@@ -30,18 +31,22 @@ public class TimePlanningAuthGrpcServiceTests
     public void SetUp()
     {
         _deviceService = Substitute.For<ITimePlanningRegistrationDeviceService>();
-        // RefreshToken-related deps are not exercised by ActivateDevice tests;
-        // null-pass these. UserManager/RoleManager have no usable interface to
-        // substitute against, so we pass null — any test that exercises
-        // RefreshToken would need to construct real instances.
+        // RoleManager is only reached once a login succeeds, which no test here does, so
+        // it stays null. UserManager substitutes fine through its virtual members.
         _userService = Substitute.For<IUserService>();
-        _userManager = null;
+        _userManager = SubstituteUserManager();
         _roleManager = null;
         _tokenOptions = Substitute.For<IOptions<EformTokenOptions>>();
         _grpcService = new TimePlanningAuthGrpcService(
             _deviceService, _userService, _userManager, _roleManager, _tokenOptions);
     }
 
+    [TearDown]
+    public void TearDown()
+    {
+        _userManager?.Dispose();
+    }
+
     [Test]
     public async Task ActivateDevice_Success_ReturnsToken()
     {
@@ -124,4 +129,81 @@ public async Task ActivateDevice_InvalidCustomerNo_DefaultsToZero()
         await _deviceService.Received(1).Activate(
             Arg.Is<TimePlanningRegistrationDeviceActivateModel>(m => m.CustomerNo == 0));
     }
+
+    // This service is a second, parallel login implementation that mints the same JWT as
+    // the JSON path, so a disabled account has to be refused here too - otherwise a
+    // resigned employee's flutter-time app keeps working. Every credential failure answers
+    // with one message, for the same reason the JSON path does.
+    // Spelled out rather than referenced from the production constant: asserting against
+    // the constant would pass however the message changed.
+    private const string ExpectedMessage = "You have entered an invalid username or password";
+
+    private static UserManager<EformUser> SubstituteUserManager() =>
+        Substitute.For<UserManager<EformUser>>(
+            Substitute.For<IUserStore<EformUser>>(), null, null, null, null, null, null, null, null);
+
+    private TimePlanningAuthGrpcService ServiceWith(UserManager<EformUser> userManager) =>
+        new(_deviceService, _userService, userManager, _roleManager, _tokenOptions);
+
+    private static EformUser DisabledUser() => new()
+    {
+        Id = 42,
+        UserName = "someone@example.com",
+        Email = "someone@example.com",
+        EmailConfirmed = true,
+        IsActive = false
+    };
+
+    [Test]
+    public async Task AuthenticateUser_DisabledAccount_IsRefused()
+    {
+        var userManager = SubstituteUserManager();
+        userManager.FindByNameAsync(Arg.Any<string>()).Returns(DisabledUser());
+        userManager.CheckPasswordAsync(Arg.Any<EformUser>(), Arg.Any<string>()).Returns(true);
+        // A role, so the call would otherwise get past every other check and mint a token -
+        // without this the method returns "Role ... not found" and the refusal proves nothing.
+        userManager.GetRolesAsync(Arg.Any<EformUser>()).Returns(new List<string> { "admin" });
+
+        var response = await ServiceWith(userManager).AuthenticateUser(
+            new AuthenticateUserRequest { Username = "someone@example.com", Password = "right" }, TestServerCallContextFactory.Create());
+
+        Assert.That(response.Success, Is.False, "a disabled account must not be able to log in");
+        Assert.That(response.Message, Is.EqualTo(ExpectedMessage));
+        Assert.That(response.Model, Is.Null, "no token may be issued for a disabled account");
+    }
+
+    [Test]
+    public async Task AuthenticateUser_DisabledAccount_ReturnsSameMessageAsUnknownAccount()
+    {
+        var disabledManager = SubstituteUserManager();
+        disabledManager.FindByNameAsync(Arg.Any<string>()).Returns(DisabledUser());
+        disabledManager.CheckPasswordAsync(Arg.Any<EformUser>(), Arg.Any<string>()).Returns(true);
+        var disabled = await ServiceWith(disabledManager).AuthenticateUser(
+            new AuthenticateUserRequest { Username = "someone@example.com", Password = "right" }, TestServerCallContextFactory.Create());
+
+        var unknownManager = SubstituteUserManager();
+        unknownManager.FindByNameAsync(Arg.Any<string>()).Returns((EformUser)null!);
+        unknownManager.FindByEmailAsync(Arg.Any<string>()).Returns((EformUser)null!);
+        var unknown = await ServiceWith(unknownManager).AuthenticateUser(
+            new AuthenticateUserRequest { Username = "someone@example.com", Password = "right" }, TestServerCallContextFactory.Create());
+
+        Assert.That(disabled.Message, Is.EqualTo(unknown.Message),
+            "a disabled account must not be distinguishable from one that does not exist");
+        Assert.That(unknown.Message, Does.Not.Contain("someone@example.com"),
+            "the response must not repeat what was typed");
+    }
+
+    [Test]
+    public async Task RefreshToken_DisabledAccount_IsRefused()
+    {
+        _userService.UserId.Returns(42);
+        _userService.GetByIdAsync(Arg.Any<int>()).Returns(DisabledUser());
+
+        var response = await _grpcService.RefreshToken(new RefreshTokenRequest(), TestServerCallContextFactory.Create());
+
+        Assert.That(response.Success, Is.False,
+            "a disabled account must not be able to roll its session forward");
+        Assert.That(response.Message, Is.EqualTo(ExpectedMessage));
+        Assert.That(response.Model, Is.Null, "no fresh token may be minted for a disabled account");
+    }
 }

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/GrpcServices/TimePlanningAuthGrpcService.cs

@@ -29,6 +29,16 @@ public class TimePlanningAuthGrpcService : TimePlanningAuthService.TimePlanningA
     private readonly RoleManager<EformRole> _roleManager;
     private readonly IOptions<EformTokenOptions> _tokenOptions;
 
+    /// <summary>
+    /// The one answer every credential failure gives: unknown account, wrong password or
+    /// disabled account. Telling them apart turns a login into a list of which addresses
+    /// have accounts. The text matches core's UserNameOrPasswordIncorrect resource, which
+    /// a plugin cannot reach - microting/eform-angular-frontend#8077 tracks closing that
+    /// gap properly.
+    /// </summary>
+    private const string InvalidCredentialsMessage =
+        "You have entered an invalid username or password";
+
     public TimePlanningAuthGrpcService(
         ITimePlanningRegistrationDeviceService registrationDeviceService,
         IUserService userService,
@@ -91,8 +101,10 @@ public override async Task<ActivateDeviceResponse> ActivateDevice(
     /// plugin-accessible. The next REST call rebuilds the cache lazily.</item>
     /// </list>
     /// Failure messages mirror the JSON oracle so the contract diff stays
-    /// shape-clean: "Empty username or password", "User with username X not
-    /// found", "Incorrect password.", "Email X not confirmed".
+    /// shape-clean - including its refusal to say WHICH credential was wrong:
+    /// unknown account, wrong password and disabled account all answer with
+    /// InvalidCredentialsMessage. "Empty username or password" and
+    /// "Email X not confirmed" stay distinct, as they do there.
     /// </remarks>
     public override async Task<AuthenticateUserResponse> AuthenticateUser(
         AuthenticateUserRequest request, ServerCallContext context)
@@ -118,17 +130,21 @@ public override async Task<AuthenticateUserResponse> AuthenticateUser(
                 return new AuthenticateUserResponse
                 {
                     Success = false,
-                    Message = $"User with username {request.Username} not found"
+                    Message = InvalidCredentialsMessage
                 };
             }
 
             var passwordOk = await _userManager.CheckPasswordAsync(user, request.Password);
-            if (!passwordOk)
+
+            // Checked after the password, not before: answering earlier for a disabled
+            // account would answer faster than a wrong password does, which is a timing
+            // oracle. This mirrors the JSON path in core's AuthService.
+            if (!passwordOk || !user.IsActive)
             {
                 return new AuthenticateUserResponse
                 {
                     Success = false,
-                    Message = "Incorrect password."
+                    Message = InvalidCredentialsMessage
                 };
             }
 
@@ -205,12 +221,15 @@ public override async Task<RefreshTokenResponse> RefreshToken(
         try
         {
             var user = await _userService.GetByIdAsync(_userService.UserId);
-            if (user == null)
+
+            // Without this a disabled account rolls its session forward indefinitely:
+            // this endpoint mints a fresh token from any still-valid one.
+            if (user == null || !user.IsActive)
             {
                 return new RefreshTokenResponse
                 {
                     Success = false,
-                    Message = $"User with id {_userService.UserId} not found"
+                    Message = InvalidCredentialsMessage
                 };
             }
 

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs

@@ -966,6 +966,80 @@ planRegistrationForToday is
         }
     }
 
+    /// <summary>
+    /// Resigning here must disable the person's login, as resigning through the
+    /// device-users screen will once its half lands
+    /// (microting/eform-backendconfiguration-plugin#1283). Resigned is only a visibility flag - no authentication
+    /// code reads it - so without this a worker resigned from time-planning keeps an
+    /// account that still signs in, including the flutter apps. Written with ExecuteUpdate
+    /// rather than UserManager, which would run Identity's validators against addresses
+    /// part of this population cannot satisfy (non-ASCII local parts).
+    /// </summary>
+    private async Task SyncLoginStateAsync(int sdkSiteMicrotingUid, bool resigned)
+    {
+        try
+        {
+            var sdkCore = await core.GetCore().ConfigureAwait(false);
+            var sdkDbContext = sdkCore.DbContextHelper.GetDbContext();
+
+            var email = await (
+                from s in sdkDbContext.Sites
+                join sw in sdkDbContext.SiteWorkers on s.Id equals sw.SiteId
+                join w in sdkDbContext.Workers on sw.WorkerId equals w.Id
+                where s.MicrotingUid == sdkSiteMicrotingUid
+                      && s.WorkflowState != Constants.WorkflowStates.Removed
+                      && sw.WorkflowState != Constants.WorkflowStates.Removed
+                      && w.WorkflowState != Constants.WorkflowStates.Removed
+                // Deterministic by the lowest SiteWorker id, for the reason
+                // SiteWorkerResolver documents: a site carrying more than one live
+                // SiteWorker row would otherwise resolve to whichever the database
+                // happened to return, and disable the wrong person's login.
+                orderby sw.Id
+                select w.Email).FirstOrDefaultAsync().ConfigureAwait(false);
+
+            // Matched the same way the avatar lookup above does it - the two must agree,
+            // or that lookup finds a login this one misses.
+            var workerEmail = (email ?? "").Trim().ToLower();
+            if (string.IsNullOrEmpty(workerEmail))
+            {
+                logger.LogWarning(
+                    "No worker email for site {SdkSiteId}; Resigned={Resigned} saved, no login state written",
+                    sdkSiteMicrotingUid, resigned);
+                return;
+            }
+
+            var isActive = !resigned;
+            var affected = await baseDbContext.Users
+                .Where(x => x.Email.ToLower() == workerEmail)
+                .ExecuteUpdateAsync(x => x.SetProperty(u => u.IsActive, isActive))
+                .ConfigureAwait(false);
+
+            if (affected == 0)
+            {
+                // The resignation did not reach a login: the worker's address matches no
+                // account. That is the failure this method exists to report.
+                logger.LogWarning(
+                    "No login row matched the worker of site {SdkSiteId}; IsActive={IsActive} not written",
+                    sdkSiteMicrotingUid, isActive);
+                return;
+            }
+
+            logger.LogInformation(
+                "Set IsActive={IsActive} on {Count} login(s) for site {SdkSiteId}",
+                isActive, affected, sdkSiteMicrotingUid);
+        }
+        catch (Exception ex)
+        {
+            // The settings row is already committed, and a resignation that does not reach
+            // the login is a security gap rather than a broken save - so report it and let
+            // the caller succeed.
+            SentrySdk.CaptureException(ex);
+            logger.LogError(ex,
+                "Could not sync login state for site {SdkSiteId}",
+                sdkSiteMicrotingUid);
+        }
+    }
+
     public async Task<OperationResult> UpdateAssignedSite(Infrastructure.Models.Settings.AssignedSite site)
     {
         var siteId = site.SiteId;
@@ -982,6 +1056,12 @@ public async Task<OperationResult> UpdateAssignedSite(Infrastructure.Models.Sett
         dbAssignedSite.AllowEditOfRegistrations = site.AllowEditOfRegistrations;
         dbAssignedSite.AllowPersonalTimeRegistration = site.AllowPersonalTimeRegistration;
         dbAssignedSite.AllowAcceptOfPlannedHours = site.AllowAcceptOfPlannedHours;
+        // Captured before the assignment below: the login is only touched when the
+        // resignation actually changes. An unrelated settings save must not re-assert
+        // account state - not least because a body that omits "resigned" deserializes
+        // to false, and that would silently re-enable a disabled account.
+        var wasResigned = dbAssignedSite.Resigned;
+
         dbAssignedSite.Resigned = site.Resigned;
         // Record WHEN one-minute intervals took effect, so every later flex
         // recomputation keeps pre-switch days on 5-minute rules instead of
@@ -1092,6 +1172,11 @@ public async Task<OperationResult> UpdateAssignedSite(Infrastructure.Models.Sett
 
         await dbAssignedSite.Update(dbContext);
 
+        if (site.Resigned != wasResigned)
+        {
+            await SyncLoginStateAsync(dbAssignedSite.SiteId, site.Resigned).ConfigureAwait(false);
+        }
+
         // Fire-and-forget: tell the worker's device(s) that their assigned-site
         // settings changed so personal mode can auto-refresh. Sent AFTER the row
         // is committed; a push failure must NEVER fail the settings update.