From fe046e2921f933f869320eefc35fd966f3eb19a5 Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Wed, 2 Sep 2026 21:42:39 -0700 Subject: [PATCH 1/2] RG-T89 Fixes --- .../DeleteRepository.cs | 5 +- ...DepartmentMemberSensitiveDataRepository.cs | 32 +++++++-- .../Services/DeleteRepositorySchemaTests.cs | 49 +++++++++++++ ...emberSensitiveDataRepositorySchemaTests.cs | 68 +++++++++++++++++++ ...orkflowRepositoryDeleteConcurrencyTests.cs | 14 ++-- 5 files changed, 154 insertions(+), 14 deletions(-) create mode 100644 Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs create mode 100644 Tests/Resgrid.Tests/Services/DepartmentMemberSensitiveDataRepositorySchemaTests.cs diff --git a/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs b/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs index 0bdd64fbb..f1d8488d6 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs @@ -83,9 +83,9 @@ DELETE FROM [dbo].[MessageRecipients] WHERE MessageId IN (SELECT MessageId FROM DELETE FROM [dbo].[UnitStateRoles] WHERE UserId = @UserId --AND DepartmentId = @DepartmentId DELETE FROM [dbo].[CallDispatches] WHERE UserId = @UserId --AND DepartmentId = @DepartmentId - IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @UserId) = 1 + IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @UserId) = 0 BEGIN - -- This user is only a member of one department so clear their account out as well + -- The deleted membership was the user's last, so clear their account out as well DELETE FROM [dbo].[ChatbotUserIdentities] WHERE UserId = @UserId DELETE FROM [dbo].[ChatbotLinkingCodes] WHERE UserId = @UserId DELETE FROM [dbo].[UserProfiles] WHERE UserId = @UserId @@ -235,7 +235,6 @@ DELETE FROM [dbo].[UdfFields] WHERE UdfDefinitionId IN (SELECT UdfDefinitionId F DELETE FROM [dbo].[NotificationAlerts] WHERE DepartmentId = @DepartmentId DELETE FROM [dbo].[Permissions] WHERE DepartmentId = @DepartmentId DELETE FROM [dbo].[Ranks] WHERE DepartmentId = @DepartmentId - DELETE FROM [dbo].[ActiveDepartments] WHERE ActiveDepartmentId = @DepartmentId -- Catch-alls for rows the per-user cursor missed (users removed from the -- department before deletion, or rows with no surviving member) diff --git a/Repositories/Resgrid.Repositories.DataRepository/DepartmentMemberSensitiveDataRepository.cs b/Repositories/Resgrid.Repositories.DataRepository/DepartmentMemberSensitiveDataRepository.cs index 1df6d3681..4fa3e90ce 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/DepartmentMemberSensitiveDataRepository.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/DepartmentMemberSensitiveDataRepository.cs @@ -61,25 +61,45 @@ public Task> GetDepartmentIdsWithOutstandingLegacyProfileDataAs // "Outstanding" is the ABSENCE of the relocation marker, not an empty target column: a // member who cleared their department identification number has an empty target and must // not be swept forever. A member with no row at all is outstanding by definition. - var sql = _isPostgres + return WithConnectionAsync(async connection => + { + // QA may already have run the original M0141 contract while production deliberately + // retains this column during the relocation window. Detect the deployed shape before + // composing the query; merely guarding a missing column inside SQL still fails when the + // database parses that column reference. + var columnExistsSql = _isPostgres + ? @"SELECT COUNT(*) FROM information_schema.columns +WHERE table_schema = @SchemaName AND table_name = 'userprofiles' AND column_name = 'identificationnumber'" + : @"SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS +WHERE TABLE_SCHEMA = @SchemaName AND TABLE_NAME = 'UserProfiles' AND COLUMN_NAME = 'IdentificationNumber'"; + + var legacyIdentificationNumberExists = await connection.ExecuteScalarAsync(columnExistsSql, + new { SchemaName = _schema.Trim('[', ']') }, _unitOfWork?.Transaction) > 0; + + var legacyIdentificationNumberPredicate = legacyIdentificationNumberExists + ? _isPostgres + ? " OR (up.identificationnumber IS NOT NULL AND btrim(up.identificationnumber) <> '')" + : " OR (up.[IdentificationNumber] IS NOT NULL AND LTRIM(RTRIM(up.[IdentificationNumber])) <> '')" + : string.Empty; + + var sql = _isPostgres ? $@"SELECT DISTINCT dm.departmentid FROM {_schema}.departmentmembers dm INNER JOIN {_schema}.userprofiles up ON up.userid = dm.userid LEFT JOIN {_table} s ON s.departmentid = dm.departmentid AND s.userid = dm.userid WHERE dm.isdeleted = false AND s.legacyprofilerelocatedon IS NULL - AND (up.homeaddressid IS NOT NULL OR up.mailingaddressid IS NOT NULL - OR (up.identificationnumber IS NOT NULL AND btrim(up.identificationnumber) <> ''))" + AND (up.homeaddressid IS NOT NULL OR up.mailingaddressid IS NOT NULL{legacyIdentificationNumberPredicate})" : $@"SELECT DISTINCT dm.[DepartmentId] FROM {_schema}.[DepartmentMembers] dm INNER JOIN {_schema}.[UserProfiles] up ON up.[UserId] = dm.[UserId] LEFT JOIN {_table} s ON s.[DepartmentId] = dm.[DepartmentId] AND s.[UserId] = dm.[UserId] WHERE dm.[IsDeleted] = 0 AND s.[LegacyProfileRelocatedOn] IS NULL - AND (up.[HomeAddressId] IS NOT NULL OR up.[MailingAddressId] IS NOT NULL - OR (up.[IdentificationNumber] IS NOT NULL AND LTRIM(RTRIM(up.[IdentificationNumber])) <> ''))"; + AND (up.[HomeAddressId] IS NOT NULL OR up.[MailingAddressId] IS NOT NULL{legacyIdentificationNumberPredicate})"; - return WithConnectionAsync(connection => connection.QueryAsync(sql, null, _unitOfWork?.Transaction)); + return await connection.QueryAsync(sql, null, _unitOfWork?.Transaction); + }); } private async Task WithConnectionAsync(Func> operation) diff --git a/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs b/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs new file mode 100644 index 000000000..e4ddbb665 --- /dev/null +++ b/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs @@ -0,0 +1,49 @@ +using System; +using System.IO; +using FluentAssertions; +using NUnit.Framework; + +namespace Resgrid.Tests.Services +{ + [TestFixture] + public class DeleteRepositorySchemaTests + { + private static string RepositorySource() + { + var directory = new DirectoryInfo(TestContext.CurrentContext.TestDirectory); + while (directory != null && !File.Exists(Path.Combine(directory.FullName, "Resgrid.sln"))) + directory = directory.Parent; + + directory.Should().NotBeNull("the tests must be able to find the repository root"); + var path = Path.Combine(directory!.FullName, "Repositories", + "Resgrid.Repositories.DataRepository", "DeleteRepository.cs"); + return File.ReadAllText(path); + } + + [Test] + public void Department_delete_uses_the_current_active_department_storage() + { + RepositorySource().Should().NotContain("[dbo].[ActiveDepartments]", + "active-department state is stored on DepartmentMembers.IsActive"); + } + + [Test] + public void Global_user_data_is_deleted_only_after_the_last_membership_is_removed() + { + var source = RepositorySource(); + const string deleteMembership = + "DELETE FROM [dbo].[DepartmentMembers] WHERE UserId = @UserId AND DepartmentId = @DepartmentId"; + const string noMembershipsRemain = + "IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @UserId) = 0"; + + var deleteMembershipIndex = source.IndexOf(deleteMembership, StringComparison.Ordinal); + var remainingMembershipCheckIndex = source.IndexOf(noMembershipsRemain, StringComparison.Ordinal); + + deleteMembershipIndex.Should().BeGreaterThan(0); + remainingMembershipCheckIndex.Should().BeGreaterThan(deleteMembershipIndex, + "the current department membership must be removed before checking for another membership"); + source.Should().NotContain( + "IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @UserId) = 1"); + } + } +} diff --git a/Tests/Resgrid.Tests/Services/DepartmentMemberSensitiveDataRepositorySchemaTests.cs b/Tests/Resgrid.Tests/Services/DepartmentMemberSensitiveDataRepositorySchemaTests.cs new file mode 100644 index 000000000..25017c72c --- /dev/null +++ b/Tests/Resgrid.Tests/Services/DepartmentMemberSensitiveDataRepositorySchemaTests.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using Resgrid.Config; +using Resgrid.Model.Repositories.Connection; +using Resgrid.Model.Repositories.Queries; +using Resgrid.Repositories.DataRepository; +using Resgrid.Repositories.DataRepository.Configs; +using Resgrid.Repositories.DataRepository.Servers.SqlServer; + +namespace Resgrid.Tests.Services +{ + [TestFixture] + public class DepartmentMemberSensitiveDataRepositorySchemaTests + { + [Test] + [TestCase(DatabaseTypes.SqlServer, true)] + [TestCase(DatabaseTypes.SqlServer, false)] + [TestCase(DatabaseTypes.Postgres, true)] + [TestCase(DatabaseTypes.Postgres, false)] + public async Task Outstanding_legacy_profile_query_matches_the_deployed_schema( + DatabaseTypes databaseType, bool legacyIdentificationNumberExists) + { + var originalDatabaseType = DataConfig.DatabaseType; + DataConfig.DatabaseType = databaseType; + + try + { + var executedSql = new List(); + var connection = new CapturingConnection(executedSql, + scalarResult: legacyIdentificationNumberExists ? 1L : 0L); + var unitOfWork = new Mock(); + unitOfWork.Setup(x => x.Connection).Returns(connection); + unitOfWork.Setup(x => x.CreateOrGetConnection()).Returns(connection); + + SqlConfiguration configuration = databaseType == DatabaseTypes.Postgres + ? new PostgreSqlConfiguration() + : new SqlServerConfiguration(); + + var repository = new DepartmentMemberSensitiveDataRepository( + Mock.Of(), configuration, unitOfWork.Object, Mock.Of()); + + await repository.GetDepartmentIdsWithOutstandingLegacyProfileDataAsync(); + + executedSql.Should().HaveCount(2); + executedSql[0].Should().ContainEquivalentOf("information_schema.columns"); + executedSql[1].Should().ContainEquivalentOf("homeaddressid"); + executedSql[1].Should().ContainEquivalentOf("mailingaddressid"); + + var identificationNumberReference = databaseType == DatabaseTypes.Postgres + ? "up.identificationnumber" + : "up.[IdentificationNumber]"; + + if (legacyIdentificationNumberExists) + executedSql[1].Should().ContainEquivalentOf(identificationNumberReference); + else + executedSql[1].Should().NotContainEquivalentOf(identificationNumberReference); + } + finally + { + DataConfig.DatabaseType = originalDatabaseType; + } + } + } +} diff --git a/Tests/Resgrid.Tests/Services/WorkflowRepositoryDeleteConcurrencyTests.cs b/Tests/Resgrid.Tests/Services/WorkflowRepositoryDeleteConcurrencyTests.cs index 6151c2b1e..8aa35cd5e 100644 --- a/Tests/Resgrid.Tests/Services/WorkflowRepositoryDeleteConcurrencyTests.cs +++ b/Tests/Resgrid.Tests/Services/WorkflowRepositoryDeleteConcurrencyTests.cs @@ -388,6 +388,7 @@ internal sealed class CapturingConnection : DbConnection private readonly List _log; private readonly Action _onLockSelected; private readonly Action _onAllDeleted; + private readonly object _scalarResult; // Counts how many DELETE statements have been executed (we expect 4). private int _deleteCount; @@ -395,11 +396,13 @@ internal sealed class CapturingConnection : DbConnection public CapturingConnection( List log, Action onLockSelected = null, - Action onAllDeleted = null) + Action onAllDeleted = null, + object scalarResult = null) { _log = log; _onLockSelected = onLockSelected; _onAllDeleted = onAllDeleted; + _scalarResult = scalarResult; } // ── DbConnection overrides ─────────────────────────────────────────────────── @@ -417,7 +420,7 @@ protected override DbTransaction BeginDbTransaction(IsolationLevel isolationLeve public override void ChangeDatabase(string databaseName) { } public override void Close() { } - protected override DbCommand CreateDbCommand() => new CapturingCommand(this, OnExecute); + protected override DbCommand CreateDbCommand() => new CapturingCommand(this, OnExecute, _scalarResult); // ── internal callback ──────────────────────────────────────────────────────── @@ -475,11 +478,13 @@ internal sealed class CapturingCommand : DbCommand { private readonly Action _onExecute; private readonly CapturingConnection _conn; + private readonly object _scalarResult; - public CapturingCommand(CapturingConnection conn, Action onExecute) + public CapturingCommand(CapturingConnection conn, Action onExecute, object scalarResult) { _conn = conn; _onExecute = onExecute; + _scalarResult = scalarResult; } public override string CommandText { get; set; } = string.Empty; @@ -500,7 +505,7 @@ public override int ExecuteNonQuery() public override object ExecuteScalar() { _onExecute(CommandText); - return null; + return _scalarResult; } protected override DbDataReader ExecuteDbDataReader(CommandBehavior behavior) { @@ -604,4 +609,3 @@ internal sealed class CapturingParameterCollection : DbParameterCollection - From cf85490b34cd58dab2c175833e9497b0d8e190b5 Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Wed, 2 Sep 2026 22:00:23 -0700 Subject: [PATCH 2/2] RG-T89 PR #493 fixes --- .../DeleteRepository.cs | 63 ++++++++++--------- .../Services/DeleteRepositorySchemaTests.cs | 24 +++++++ 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs b/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs index f1d8488d6..901917e06 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/DeleteRepository.cs @@ -276,35 +276,40 @@ DELETE FROM [dbo].[DepartmentGroupMembers] WHERE DepartmentGroupId IN (SELECT De DELETE FROM [dbo].[DepartmentCallPruning] WHERE DepartmentId = @DepartmentId DELETE FROM [dbo].[Departments] WHERE DepartmentId = @DepartmentId - -- Delete the managing member's user - DELETE FROM [dbo].[ScheduledTasks] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[UserStates] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[Logs] WHERE LoggedByUserId = @ManagingUserId - DELETE FROM [dbo].[MessageRecipients] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[MessageRecipients] WHERE MessageId IN (SELECT MessageId FROM [dbo].[Messages] WHERE ReceivingUserId = @ManagingUserId) - DELETE FROM [dbo].[MessageRecipients] WHERE MessageId IN (SELECT MessageId FROM [dbo].[Messages] WHERE SendingUserId = @ManagingUserId) - DELETE FROM [dbo].[Messages] WHERE ReceivingUserId = @ManagingUserId - DELETE FROM [dbo].[Messages] WHERE SendingUserId = @ManagingUserId - DELETE FROM [dbo].[PersonnelCertifications] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[PersonnelRoleUsers] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[PushUris] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[UserProfiles] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[UserStates] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[ActionLogs] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[DepartmentMembers] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[DepartmentGroupMembers] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[DistributionListMembers] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[PersonnelRoleUsers] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[PushUris] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[UnitStateRoles] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[CallDispatches] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[ChatbotUserIdentities] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[ChatbotLinkingCodes] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[AspNetUserClaims] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[AspNetUserLogins] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[AspNetUserRoles] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[AspNetUsersExt] WHERE UserId = @ManagingUserId - DELETE FROM [dbo].[AspNetUsers] WHERE Id = @ManagingUserId + -- Remove only this department's managing membership. The same user may + -- manage or belong to another department and must retain that account. + DELETE FROM [dbo].[DepartmentMembers] WHERE UserId = @ManagingUserId AND DepartmentId = @DepartmentId + + IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @ManagingUserId) = 0 + BEGIN + DELETE FROM [dbo].[ScheduledTasks] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[UserStates] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[Logs] WHERE LoggedByUserId = @ManagingUserId + DELETE FROM [dbo].[MessageRecipients] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[MessageRecipients] WHERE MessageId IN (SELECT MessageId FROM [dbo].[Messages] WHERE ReceivingUserId = @ManagingUserId) + DELETE FROM [dbo].[MessageRecipients] WHERE MessageId IN (SELECT MessageId FROM [dbo].[Messages] WHERE SendingUserId = @ManagingUserId) + DELETE FROM [dbo].[Messages] WHERE ReceivingUserId = @ManagingUserId + DELETE FROM [dbo].[Messages] WHERE SendingUserId = @ManagingUserId + DELETE FROM [dbo].[PersonnelCertifications] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[PersonnelRoleUsers] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[PushUris] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[UserProfiles] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[UserStates] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[ActionLogs] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[DepartmentGroupMembers] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[DistributionListMembers] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[PersonnelRoleUsers] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[PushUris] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[UnitStateRoles] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[CallDispatches] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[ChatbotUserIdentities] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[ChatbotLinkingCodes] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[AspNetUserClaims] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[AspNetUserLogins] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[AspNetUserRoles] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[AspNetUsersExt] WHERE UserId = @ManagingUserId + DELETE FROM [dbo].[AspNetUsers] WHERE Id = @ManagingUserId + END ", new { DepartmentId = departmentId }, transaction); diff --git a/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs b/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs index e4ddbb665..fbb5a14c9 100644 --- a/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs +++ b/Tests/Resgrid.Tests/Services/DeleteRepositorySchemaTests.cs @@ -45,5 +45,29 @@ public void Global_user_data_is_deleted_only_after_the_last_membership_is_remove source.Should().NotContain( "IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @UserId) = 1"); } + + [Test] + public void Managing_user_keeps_other_department_memberships_and_account() + { + var source = RepositorySource(); + const string deleteTargetMembership = + "DELETE FROM [dbo].[DepartmentMembers] WHERE UserId = @ManagingUserId AND DepartmentId = @DepartmentId"; + const string noMembershipsRemain = + "IF (SELECT COUNT(*) FROM DepartmentMembers WHERE UserId = @ManagingUserId) = 0"; + const string deleteAccount = + "DELETE FROM [dbo].[AspNetUsers] WHERE Id = @ManagingUserId"; + + var deleteTargetMembershipIndex = source.IndexOf(deleteTargetMembership, StringComparison.Ordinal); + var remainingMembershipCheckIndex = source.IndexOf(noMembershipsRemain, StringComparison.Ordinal); + var deleteAccountIndex = source.IndexOf(deleteAccount, StringComparison.Ordinal); + + deleteTargetMembershipIndex.Should().BeGreaterThan(0); + remainingMembershipCheckIndex.Should().BeGreaterThan(deleteTargetMembershipIndex); + deleteAccountIndex.Should().BeGreaterThan(remainingMembershipCheckIndex, + "the managing user's account must be deleted only when no memberships remain"); + source.Should().NotMatchRegex( + @"DELETE FROM \[dbo\]\.\[DepartmentMembers\] WHERE UserId = @ManagingUserId\s*(?:\r?\n|$)", + "memberships in other departments must be preserved"); + } } }