From 498f49a2d88c9123c0d294bd1b0bd83672e3d1f8 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 19:26:25 -0300 Subject: [PATCH] fix(auth): end earlier sessions when a user's role or account changes A token carries the user's roles and account, so a demoted admin kept admin claims until the token expired. The team member update and the admin user edit now rotate the security stamp and evict the cached value when the role, account or user name changes. Permission overrides are read per request and are not in the token. --- .../UserServiceTests.cs | 43 ++++++++++- .../Implementation/TeamMemberService.cs | 9 +-- .../Implementation/UserService.cs | 22 +++++- .../SessionRevocationTests.cs | 71 ++++++++++++++++++- SeaHavenIndustries.Tests/SessionTestHost.cs | 8 +++ 5 files changed, 144 insertions(+), 9 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/UserServiceTests.cs b/Api.SeaHavenIndustries.Tests/UserServiceTests.cs index 62fa837..3ef307d 100644 --- a/Api.SeaHavenIndustries.Tests/UserServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/UserServiceTests.cs @@ -26,7 +26,8 @@ public class UserServiceTests Mock userData, Mock email, out Mock> store, - Mock? accounts = null) + Mock? accounts = null, + Mock? sessions = null) { var (manager, s, _) = IdentityTestHelpers.CreateUserManager(); store = s; @@ -35,7 +36,7 @@ public class UserServiceTests userData.Object, (accounts ?? new Mock()).Object, email.Object, - Mock.Of()); + (sessions ?? new Mock()).Object); } [Fact] @@ -430,4 +431,42 @@ public class UserServiceTests && password.Any(char.IsDigit) && password.Any(character => !char.IsLetterOrDigit(character)); } + + [Theory] + [InlineData(2, "alice@example.com", "Dispatcher", true)] + [InlineData(1, "alice@example.com", "Scheduler", true)] + [InlineData(1, "alice.new@example.com", "Dispatcher", true)] + [InlineData(1, "ALICE@example.com", "dispatcher", false)] + public async Task EditUser_EndsEarlierSessionsOnlyWhenTheTokenClaimsChange( + int accountId, string email, string role, bool endsSessions) + { + var existing = IdentityTestHelpers.User("u1", userName: "alice@example.com"); + existing.AccountId = 1; + var stampBefore = existing.SecurityStamp; + var userData = new Mock(); + userData.Setup(u => u.GetForEditAsync("u1", It.IsAny())).ReturnsAsync(existing); + var accounts = new Mock(); + accounts.Setup(a => a.ExistsAsync(It.IsAny())).ReturnsAsync(true); + var sessions = new Mock(); + var service = NewService(userData, new Mock(), out var store, accounts, sessions); + store.Setup(s => s.GetRolesAsync(existing, It.IsAny())) + .ReturnsAsync(new List { "Dispatcher" }); + + var outcome = await service.EditUserAsync( + new EditUserRequestDTO { Id = "u1", Name = "Alice", Email = email, Role = role, AccountId = accountId }, + Principal("Admin"), + CancellationToken.None); + + outcome.Success.Should().BeTrue(); + if (endsSessions) + { + existing.SecurityStamp.Should().NotBe(stampBefore); + sessions.Verify(s => s.Forget("u1"), Times.Once); + } + else + { + existing.SecurityStamp.Should().Be(stampBefore); + sessions.Verify(s => s.Forget(It.IsAny()), Times.Never); + } + } } diff --git a/SeaHaven.Services/Implementation/TeamMemberService.cs b/SeaHaven.Services/Implementation/TeamMemberService.cs index 958edd9..6ba22be 100644 --- a/SeaHaven.Services/Implementation/TeamMemberService.cs +++ b/SeaHaven.Services/Implementation/TeamMemberService.cs @@ -234,13 +234,14 @@ public sealed class TeamMemberService : ITeamMemberService return OperationFailure(addRoleResult.Errors.FirstOrDefault()?.Description ?? "Unable to update role."); } - // A new stamp ends the member's earlier sessions, and keeps them ended if the - // member is reactivated later. - if (deactivated) + // A new stamp ends the member's earlier sessions: a deactivated member's stay + // ended if the member is reactivated later, and a member whose role changed + // signs in again to get a token with the new role. + if (deactivated || roleChanged) user.SecurityStamp = Guid.NewGuid().ToString("N"); await _userDataService.UpdateUserAsync(user, cancellationToken); - if (activeChanged || emailChanged) + if (activeChanged || emailChanged || roleChanged) _sessionStamps.Forget(user.Id); await _areaDataService.ReplaceAsync(user.Id, areas!, cancellationToken); if (roleChanged || request.PermissionOverrides is not null) diff --git a/SeaHaven.Services/Implementation/UserService.cs b/SeaHaven.Services/Implementation/UserService.cs index 94a1b4c..6e4644a 100644 --- a/SeaHaven.Services/Implementation/UserService.cs +++ b/SeaHaven.Services/Implementation/UserService.cs @@ -106,6 +106,10 @@ namespace SeaHaven.Services.Implementation return new AddUserOutcomeDTO { Success = false, Error = UserMutationErrors.UserNotFound }; var existingRole = await _userManager.GetRolesAsync(exist1); + var claimsChanged = exist1.AccountId != dto.AccountId + || existingRole == null + || existingRole.Count != 1 + || !string.Equals(existingRole[0], dto.Role, StringComparison.OrdinalIgnoreCase); if (existingRole != null && existingRole.Any() && existingRole.FirstOrDefault() != exist1.PhoneNumber) { @@ -117,11 +121,15 @@ namespace SeaHaven.Services.Implementation exist1.Contact = model.Contact; exist1.PhoneNumber = model.PhoneNumber; exist1.AccountId = dto.AccountId; + if (claimsChanged) + exist1.SecurityStamp = Guid.NewGuid().ToString("N"); await _userDataService.UpdateUserAsync(exist1, cancellationToken); await _userManager.AddToRoleAsync(exist1, dto.Role ?? ""); await _userManager.UpdateAsync(exist1); + if (claimsChanged) + _sessionStamps.Forget(exist1.Id); return new AddUserOutcomeDTO { Success = true }; } } @@ -150,6 +158,15 @@ namespace SeaHaven.Services.Implementation if (string.IsNullOrWhiteSpace(dto.Email)) throw new ArgumentException("Email is required.", nameof(dto)); + // The token carries the user name, roles and account, so a change to any of + // them needs a fresh sign-in. + var existingRole = await _userManager.GetRolesAsync(exist); + var claimsChanged = exist.AccountId != dto.AccountId + || !string.Equals(exist.UserName, dto.Email, StringComparison.OrdinalIgnoreCase) + || existingRole == null + || existingRole.Count != 1 + || !string.Equals(existingRole[0], dto.Role, StringComparison.OrdinalIgnoreCase); + exist.EmailConfirmed = true; exist.UserName = dto.Email; exist.Email = dto.Email; @@ -160,14 +177,17 @@ namespace SeaHaven.Services.Implementation exist.PhoneNumber = dto.Role; exist.AccountId = dto.AccountId; - var existingRole = await _userManager.GetRolesAsync(exist); if (existingRole != null && existingRole.Any()) { await _userManager.RemoveFromRolesAsync(exist, existingRole); } await _userManager.AddToRoleAsync(exist, dto.Role ?? ""); + if (claimsChanged) + exist.SecurityStamp = Guid.NewGuid().ToString("N"); await _userDataService.UpdateUserAsync(exist, cancellationToken); + if (claimsChanged) + _sessionStamps.Forget(exist.Id); return new AddUserOutcomeDTO { Success = true }; } diff --git a/SeaHavenIndustries.Tests/SessionRevocationTests.cs b/SeaHavenIndustries.Tests/SessionRevocationTests.cs index 1cd9bd7..ec25227 100644 --- a/SeaHavenIndustries.Tests/SessionRevocationTests.cs +++ b/SeaHavenIndustries.Tests/SessionRevocationTests.cs @@ -163,12 +163,79 @@ public sealed class SessionRevocationTests Assert.Empty(leaks); } - private static async Task UpdateMemberAsync(SessionTestHost host, string adminToken, string userId, bool isActive) + [Fact] + public async Task A_member_demoted_on_the_team_page_is_refused_and_signs_in_again_with_the_new_role() + { + await using var host = await SessionTestHost.StartAsync(); + await host.AddUserAsync("admin@example.com", "Admin"); + var member = await host.AddUserAsync("sam@example.com", "Admin"); + await host.EnsureRoleAsync("Scheduler"); + var admin = await host.SignInAsync("admin@example.com"); + var before = await host.SignInAsync("sam@example.com"); + Assert.Equal(new[] { "Admin" }, RolesIn(before)); + await AssertAcceptedAsync(host, before); + + await UpdateMemberAsync(host, admin, member.Id, isActive: true, role: "Scheduler"); + + await AssertRefusedAsync(host, before); + var after = await host.SignInAsync("sam@example.com"); + Assert.Equal(new[] { "Scheduler" }, RolesIn(after)); + await AssertAcceptedAsync(host, after); + } + + [Fact] + public async Task A_user_whose_role_an_admin_edits_is_refused_and_signs_in_again_with_the_new_role() + { + await using var host = await SessionTestHost.StartAsync(); + await host.AddUserAsync("admin@example.com", "Admin"); + var member = await host.AddUserAsync("sam@example.com", "Admin"); + await host.EnsureRoleAsync("Dispatcher"); + var admin = await host.SignInAsync("admin@example.com"); + var before = await host.SignInAsync("sam@example.com"); + await AssertAcceptedAsync(host, before); + + var edit = new + { + id = member.Id, + name = "Sam", + email = "sam@example.com", + role = "Dispatcher" + }; + using (var edited = await host.SendAsync(HttpMethod.Put, "api/User/EditUser", admin, edit)) + Assert.Equal(HttpStatusCode.OK, edited.StatusCode); + + await AssertRefusedAsync(host, before); + var after = await host.SignInAsync("sam@example.com"); + Assert.Equal(new[] { "Dispatcher" }, RolesIn(after)); + await AssertAcceptedAsync(host, after); + } + + [Fact] + public async Task Editing_a_member_without_changing_role_status_or_email_keeps_their_session() + { + await using var host = await SessionTestHost.StartAsync(); + await host.AddUserAsync("admin@example.com", "Admin"); + var member = await host.AddUserAsync("sam@example.com", "Scheduler"); + var admin = await host.SignInAsync("admin@example.com"); + var before = await host.SignInAsync("sam@example.com"); + + await UpdateMemberAsync(host, admin, member.Id, isActive: true); + + await AssertAcceptedAsync(host, before); + } + + private static string[] RolesIn(string token) => + new JwtSecurityTokenHandler().ReadJwtToken(token).Claims + .Where(claim => claim.Type == ClaimTypes.Role) + .Select(claim => claim.Value) + .ToArray(); + + private static async Task UpdateMemberAsync(SessionTestHost host, string adminToken, string userId, bool isActive, string role = "Scheduler") { using var response = await host.SendAsync(HttpMethod.Put, $"api/team-members/{userId}", adminToken, new { name = "Sam Lee", - role = "Scheduler", + role, color = "#0D9488", email = "sam@example.com", phone = "555-0100", diff --git a/SeaHavenIndustries.Tests/SessionTestHost.cs b/SeaHavenIndustries.Tests/SessionTestHost.cs index 31bcf79..0dca587 100644 --- a/SeaHavenIndustries.Tests/SessionTestHost.cs +++ b/SeaHavenIndustries.Tests/SessionTestHost.cs @@ -139,6 +139,14 @@ internal sealed class SessionTestHost : IAsyncDisposable return user; } + public async Task EnsureRoleAsync(string role) + { + await using var scope = _app.Services.CreateAsyncScope(); + var roles = scope.ServiceProvider.GetRequiredService>(); + if (!await roles.RoleExistsAsync(role)) + await roles.CreateAsync(new IdentityRole(role)); + } + public async Task SignInAsync(string email, string password = Password) { using var response = await Client.PostAsJsonAsync("api/Authentication/login", new { username = email, password });