diff --git a/Core/Resgrid.Model/UserProfile.cs b/Core/Resgrid.Model/UserProfile.cs index 6c4f1249..56662fc6 100644 --- a/Core/Resgrid.Model/UserProfile.cs +++ b/Core/Resgrid.Model/UserProfile.cs @@ -44,9 +44,17 @@ public class UserProfile: IEntity [ProtoMember(8)] public string HomeNumber { get; set; } + /// + /// Legacy migration source for the department-scoped home address. Current application + /// writes exclude this property through IgnoredProperties while Dapper reads still hydrate it. + /// [ProtoMember(9)] public int? HomeAddressId { get; set; } + /// + /// Legacy migration source for the department-scoped mailing address. Current application + /// writes exclude this property through IgnoredProperties while Dapper reads still hydrate it. + /// [ProtoMember(10)] public int? MailingAddressId { get; set; } @@ -81,13 +89,16 @@ public class UserProfile: IEntity public bool DoNotRecieveNewsletters { get; set; } /// - /// GONE FROM THE SCHEMA (M0141). A profile is global to a person across every department - /// they belong to, so this could never be encrypted under one department's key; the value - /// lives on DepartmentMemberSensitiveData per department and is cataloged there. The - /// property is kept only so the ProtoMember numbering stays stable for older app builds - /// that still deserialize it, and is NotMapped/ignored so no SQL ever names the column. + /// LEGACY MIGRATION SOURCE. A profile is global to a person across every department they + /// belong to, so new values live on DepartmentMemberSensitiveData per department and are + /// cataloged there. Until the later contract migration removes the column, Dapper SELECT * + /// queries deliberately still hydrate this property for MemberProfileRelocationService. + /// Generic inserts/updates ignore it, so current application paths cannot add or change + /// legacy values while the relocation is active. JsonIgnore also keeps the temporary + /// plaintext source out of profile JSON and audit payloads. /// [NotMapped] + [JsonIgnore] [ProtoMember(18)] public string IdentificationNumber { get; set; } @@ -251,7 +262,7 @@ public object IdValue public int IdType => 0; [NotMapped] - public IEnumerable IgnoredProperties => new string[] { "IdValue", "IdType", "TableName", "IdName", "User", "MembershipEmail", "IdentificationNumber" }; + public IEnumerable IgnoredProperties => new string[] { "IdValue", "IdType", "TableName", "IdName", "User", "MembershipEmail", "IdentificationNumber", "HomeAddressId", "MailingAddressId" }; [NotMapped] public FullNameFormat FullName diff --git a/Providers/Resgrid.Providers.Migrations/Migrations/M0141_ContractLegacyMemberProfileData.cs b/Providers/Resgrid.Providers.Migrations/Migrations/M0141_ContractLegacyMemberProfileData.cs index 0ab9b145..8ea6bec2 100644 --- a/Providers/Resgrid.Providers.Migrations/Migrations/M0141_ContractLegacyMemberProfileData.cs +++ b/Providers/Resgrid.Providers.Migrations/Migrations/M0141_ContractLegacyMemberProfileData.cs @@ -38,41 +38,45 @@ public class M0141_ContractLegacyMemberProfileData : Migration { public override void Up() { - Execute.Sql(@" -IF EXISTS ( - SELECT 1 - FROM [UserProfiles] up - INNER JOIN [DepartmentMembers] dm ON dm.[UserId] = up.[UserId] AND dm.[IsDeleted] = 0 - LEFT JOIN [DepartmentMemberSensitiveData] s - ON s.[DepartmentId] = dm.[DepartmentId] AND s.[UserId] = up.[UserId] - WHERE ( - (up.[IdentificationNumber] IS NOT NULL AND LTRIM(RTRIM(up.[IdentificationNumber])) <> '') - OR up.[HomeAddressId] IS NOT NULL - OR up.[MailingAddressId] IS NOT NULL - ) - AND (s.[DepartmentMemberSensitiveDataId] IS NULL OR s.[LegacyProfileRelocatedOn] IS NULL)) - THROW 51000, 'M0141 refused: members still hold legacy profile data that relocation has not stamped as moved. Run the member profile relocation to completion first — this migration destroys the originals.', 1;"); + // Note: This migration is in order by the migration code has not run against production, + // so this migration would have been applied and destryed the data. What this migration + // does will need to be recreated here in a little bit. - // Only addresses nothing else references. A shared row (a contact's, a station's) is - // left exactly as it is. - Execute.Sql(@" -DELETE a -FROM [Addresses] a -WHERE EXISTS (SELECT 1 FROM [UserProfiles] up - WHERE up.[HomeAddressId] = a.[AddressId] OR up.[MailingAddressId] = a.[AddressId]) - AND NOT EXISTS (SELECT 1 FROM [Contacts] c - WHERE c.[PhysicalAddressId] = a.[AddressId] OR c.[MailingAddressId] = a.[AddressId]) - AND NOT EXISTS (SELECT 1 FROM [Departments] d WHERE d.[AddressId] = a.[AddressId]) - AND NOT EXISTS (SELECT 1 FROM [DepartmentGroups] g WHERE g.[AddressId] = a.[AddressId]) - AND NOT EXISTS (SELECT 1 FROM [DepartmentProfiles] p WHERE p.[AddressId] = a.[AddressId]);"); - - Execute.Sql(@" -UPDATE [UserProfiles] -SET [HomeAddressId] = NULL, [MailingAddressId] = NULL -WHERE [HomeAddressId] IS NOT NULL OR [MailingAddressId] IS NOT NULL;"); - - if (Schema.Table("UserProfiles").Column("IdentificationNumber").Exists()) - Delete.Column("IdentificationNumber").FromTable("UserProfiles"); +// Execute.Sql(@" +// IF EXISTS ( +// SELECT 1 +// FROM [UserProfiles] up +// INNER JOIN [DepartmentMembers] dm ON dm.[UserId] = up.[UserId] AND dm.[IsDeleted] = 0 +// LEFT JOIN [DepartmentMemberSensitiveData] s +// ON s.[DepartmentId] = dm.[DepartmentId] AND s.[UserId] = up.[UserId] +// WHERE ( +// (up.[IdentificationNumber] IS NOT NULL AND LTRIM(RTRIM(up.[IdentificationNumber])) <> '') +// OR up.[HomeAddressId] IS NOT NULL +// OR up.[MailingAddressId] IS NOT NULL +// ) +// AND (s.[DepartmentMemberSensitiveDataId] IS NULL OR s.[LegacyProfileRelocatedOn] IS NULL)) +// THROW 51000, 'M0141 refused: members still hold legacy profile data that relocation has not stamped as moved. Run the member profile relocation to completion first — this migration destroys the originals.', 1;"); +// +// // Only addresses nothing else references. A shared row (a contact's, a station's) is +// // left exactly as it is. +// Execute.Sql(@" +// DELETE a +// FROM [Addresses] a +// WHERE EXISTS (SELECT 1 FROM [UserProfiles] up +// WHERE up.[HomeAddressId] = a.[AddressId] OR up.[MailingAddressId] = a.[AddressId]) +// AND NOT EXISTS (SELECT 1 FROM [Contacts] c +// WHERE c.[PhysicalAddressId] = a.[AddressId] OR c.[MailingAddressId] = a.[AddressId]) +// AND NOT EXISTS (SELECT 1 FROM [Departments] d WHERE d.[AddressId] = a.[AddressId]) +// AND NOT EXISTS (SELECT 1 FROM [DepartmentGroups] g WHERE g.[AddressId] = a.[AddressId]) +// AND NOT EXISTS (SELECT 1 FROM [DepartmentProfiles] p WHERE p.[AddressId] = a.[AddressId]);"); +// +// Execute.Sql(@" +// UPDATE [UserProfiles] +// SET [HomeAddressId] = NULL, [MailingAddressId] = NULL +// WHERE [HomeAddressId] IS NOT NULL OR [MailingAddressId] IS NOT NULL;"); +// +// if (Schema.Table("UserProfiles").Column("IdentificationNumber").Exists()) +// Delete.Column("IdentificationNumber").FromTable("UserProfiles"); } public override void Down() diff --git a/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0141_ContractLegacyMemberProfileDataPg.cs b/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0141_ContractLegacyMemberProfileDataPg.cs index 7883233f..1c6c550e 100644 --- a/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0141_ContractLegacyMemberProfileDataPg.cs +++ b/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0141_ContractLegacyMemberProfileDataPg.cs @@ -38,44 +38,48 @@ public class M0141_ContractLegacyMemberProfileDataPg : Migration { public override void Up() { - Execute.Sql(@" -DO $$ -BEGIN - IF EXISTS ( - SELECT 1 - FROM userprofiles up - INNER JOIN departmentmembers dm ON dm.userid = up.userid AND dm.isdeleted = false - LEFT JOIN departmentmembersensitivedata s - ON s.departmentid = dm.departmentid AND s.userid = up.userid - WHERE ( - (up.identificationnumber IS NOT NULL AND btrim(up.identificationnumber::text) <> '') - OR up.homeaddressid IS NOT NULL - OR up.mailingaddressid IS NOT NULL - ) - AND (s.departmentmembersensitivedataid IS NULL OR s.legacyprofilerelocatedon IS NULL)) THEN - RAISE EXCEPTION 'M0141 refused: members still hold legacy profile data that relocation has not stamped as moved. Run the member profile relocation to completion first - this migration destroys the originals.'; - END IF; -END $$;"); - - // Only addresses nothing else references. A shared row (a contact's, a station's) is - // left exactly as it is. - Execute.Sql(@" -DELETE FROM addresses a -WHERE EXISTS (SELECT 1 FROM userprofiles up - WHERE up.homeaddressid = a.addressid OR up.mailingaddressid = a.addressid) - AND NOT EXISTS (SELECT 1 FROM contacts c - WHERE c.physicaladdressid = a.addressid OR c.mailingaddressid = a.addressid) - AND NOT EXISTS (SELECT 1 FROM departments d WHERE d.addressid = a.addressid) - AND NOT EXISTS (SELECT 1 FROM departmentgroups g WHERE g.addressid = a.addressid) - AND NOT EXISTS (SELECT 1 FROM departmentprofiles p WHERE p.addressid = a.addressid);"); - - Execute.Sql(@" -UPDATE userprofiles -SET homeaddressid = NULL, mailingaddressid = NULL -WHERE homeaddressid IS NOT NULL OR mailingaddressid IS NOT NULL;"); - - if (Schema.Table("userprofiles").Column("identificationnumber").Exists()) - Delete.Column("identificationnumber").FromTable("userprofiles"); + // Note: This migration is in order by the migration code has not run against production, + // so this migration would have been applied and destryed the data. What this migration + // does will need to be recreated here in a little bit. + +// Execute.Sql(@" +// DO $$ +// BEGIN +// IF EXISTS ( +// SELECT 1 +// FROM userprofiles up +// INNER JOIN departmentmembers dm ON dm.userid = up.userid AND dm.isdeleted = false +// LEFT JOIN departmentmembersensitivedata s +// ON s.departmentid = dm.departmentid AND s.userid = up.userid +// WHERE ( +// (up.identificationnumber IS NOT NULL AND btrim(up.identificationnumber::text) <> '') +// OR up.homeaddressid IS NOT NULL +// OR up.mailingaddressid IS NOT NULL +// ) +// AND (s.departmentmembersensitivedataid IS NULL OR s.legacyprofilerelocatedon IS NULL)) THEN +// RAISE EXCEPTION 'M0141 refused: members still hold legacy profile data that relocation has not stamped as moved. Run the member profile relocation to completion first - this migration destroys the originals.'; +// END IF; +// END $$;"); +// +// // Only addresses nothing else references. A shared row (a contact's, a station's) is +// // left exactly as it is. +// Execute.Sql(@" +// DELETE FROM addresses a +// WHERE EXISTS (SELECT 1 FROM userprofiles up +// WHERE up.homeaddressid = a.addressid OR up.mailingaddressid = a.addressid) +// AND NOT EXISTS (SELECT 1 FROM contacts c +// WHERE c.physicaladdressid = a.addressid OR c.mailingaddressid = a.addressid) +// AND NOT EXISTS (SELECT 1 FROM departments d WHERE d.addressid = a.addressid) +// AND NOT EXISTS (SELECT 1 FROM departmentgroups g WHERE g.addressid = a.addressid) +// AND NOT EXISTS (SELECT 1 FROM departmentprofiles p WHERE p.addressid = a.addressid);"); +// +// Execute.Sql(@" +// UPDATE userprofiles +// SET homeaddressid = NULL, mailingaddressid = NULL +// WHERE homeaddressid IS NOT NULL OR mailingaddressid IS NOT NULL;"); +// +// if (Schema.Table("userprofiles").Column("identificationnumber").Exists()) +// Delete.Column("identificationnumber").FromTable("userprofiles"); } public override void Down() diff --git a/Tests/Resgrid.Tests/Services/MemberProfileRelocationTests.cs b/Tests/Resgrid.Tests/Services/MemberProfileRelocationTests.cs index e9ded5e3..60dde361 100644 --- a/Tests/Resgrid.Tests/Services/MemberProfileRelocationTests.cs +++ b/Tests/Resgrid.Tests/Services/MemberProfileRelocationTests.cs @@ -1,14 +1,19 @@ using System; using System.Collections.Generic; +using System.Data; using System.Linq; using System.Threading; using System.Threading.Tasks; +using Dapper; using FluentAssertions; using Moq; +using Newtonsoft.Json; using NUnit.Framework; using Resgrid.Model; using Resgrid.Model.Repositories; using Resgrid.Model.Services; +using Resgrid.Repositories.DataRepository.Extensions; +using Resgrid.Repositories.DataRepository.Servers.SqlServer; using Resgrid.Services; namespace Resgrid.Tests.Services @@ -108,6 +113,42 @@ public async Task Legacy_identification_number_and_addresses_move_onto_the_depar row.LegacyProfileRelocatedOn.Should().NotBeNull(); } + [Test] + public void Legacy_profile_fields_are_readable_for_relocation_but_not_generically_written() + { + // Dapper maps SELECT * independently of EF's NotMapped attribute and the custom write + // exclusions, so it still hydrates the legacy sources while the generic Resgrid + // insert/update builder keeps current application writes away from the old columns. + var table = new DataTable(); + table.Columns.Add(nameof(UserProfile.UserId), typeof(string)); + table.Columns.Add(nameof(UserProfile.IdentificationNumber), typeof(string)); + table.Columns.Add(nameof(UserProfile.HomeAddressId), typeof(int)); + table.Columns.Add(nameof(UserProfile.MailingAddressId), typeof(int)); + table.Rows.Add("user-1", "BADGE-7", 101, 202); + + using var reader = table.CreateDataReader(); + reader.Read().Should().BeTrue(); + var profile = reader.GetRowParser()(reader); + + profile.IdentificationNumber.Should().Be("BADGE-7", + "MemberProfileRelocationService must still be able to read the retained source column"); + profile.HomeAddressId.Should().Be(101); + profile.MailingAddressId.Should().Be(202); + profile.IgnoredProperties.Should().Contain(new[] + { + nameof(UserProfile.IdentificationNumber), nameof(UserProfile.HomeAddressId), + nameof(UserProfile.MailingAddressId) + }, "new profile inserts and edits must never write the legacy global values"); + var writeColumns = profile.GetColumns(new SqlServerConfiguration(), + ignoreProperties: profile.IgnoredProperties).ToList(); + writeColumns.Should().NotContain(column => + column.Contains(nameof(UserProfile.IdentificationNumber), StringComparison.OrdinalIgnoreCase) || + column.Contains(nameof(UserProfile.HomeAddressId), StringComparison.OrdinalIgnoreCase) || + column.Contains(nameof(UserProfile.MailingAddressId), StringComparison.OrdinalIgnoreCase)); + JsonConvert.SerializeObject(profile).Should().NotContain($"\"{nameof(UserProfile.IdentificationNumber)}\":", + "the temporary plaintext source must not bypass the department-scoped read path"); + } + [Test] public async Task A_department_specific_value_is_never_overwritten() { diff --git a/Tests/Resgrid.Tests/Web/User/MemberProfileMigrationWritePathTests.cs b/Tests/Resgrid.Tests/Web/User/MemberProfileMigrationWritePathTests.cs new file mode 100644 index 00000000..66b3b479 --- /dev/null +++ b/Tests/Resgrid.Tests/Web/User/MemberProfileMigrationWritePathTests.cs @@ -0,0 +1,127 @@ +using System; +using System.IO; +using System.Text.RegularExpressions; +using FluentAssertions; +using NUnit.Framework; + +namespace Resgrid.Tests.Web.User +{ + /// + /// Migration 141's contract work is intentionally deferred. During the expand/relocate window, + /// every application write for the fields it will eventually remove must land on + /// DepartmentMemberSensitiveData, while the UserProfiles values remain read-only migration + /// sources. These structural checks cover both forms that accept an identification number and + /// the only form that accepts the moved addresses. + /// + [TestFixture] + public class MemberProfileMigrationWritePathTests + { + private static DirectoryInfo RepositoryRoot() + { + 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"); + return directory!; + } + + private static string ControllerSource(string controller) + { + var path = Path.Combine(RepositoryRoot().FullName, "Web", "Resgrid.Web", "Areas", "User", + "Controllers", controller); + File.Exists(path).Should().BeTrue($"expected the controller at {path}"); + return File.ReadAllText(path); + } + + private static string MethodBody(string source, string signature) + { + var start = source.IndexOf(signature, StringComparison.Ordinal); + start.Should().BeGreaterThan(0, $"expected method signature {signature}"); + + var next = Regex.Match(source.Substring(start + 1), + @"(?:public|private|protected|internal)\s+(?:static\s+)?(?:async\s+)?Task<[^>]+>\s+\w+\s*\("); + + return next.Success ? source.Substring(start, next.Index + 1) : source.Substring(start); + } + + [Test] + public void Add_person_writes_moved_fields_to_the_department_row_only() + { + var body = MethodBody(ControllerSource("PersonnelController.cs"), + "public async Task AddPerson(AddPersonModel model"); + + var clearLegacy = body.IndexOf("model.Profile.IdentificationNumber = null;", StringComparison.Ordinal); + var saveProfile = body.IndexOf("_userProfileService.SaveProfileAsync", StringComparison.Ordinal); + var addMembership = body.IndexOf("_departmentsService.AddUserToDepartmentAsync", StringComparison.Ordinal); + var saveDepartmentValue = body.IndexOf("_memberSensitiveDataService.SaveAsync", StringComparison.Ordinal); + + clearLegacy.Should().BeGreaterThan(0, "the posted value must not reach the global profile writer"); + clearLegacy.Should().BeLessThan(saveProfile); + body.Should().Contain("model.Profile.HomeAddressId = null;", + "an overposted legacy home-address link must not reach a new global profile row"); + body.Should().Contain("model.Profile.MailingAddressId = null;", + "an overposted legacy mailing-address link must not reach a new global profile row"); + saveDepartmentValue.Should().BeGreaterThan(addMembership, + "the department-scoped row should be created only after its membership exists"); + body.Should().Contain("IdentificationNumber = identificationNumber"); + body.Should().Contain("LegacyProfileRelocatedOn = DateTime.UtcNow", + "a brand-new profile has no legacy source for the relocation worker to revisit"); + body.Should().Contain("catch (InvalidOperationException ex)", + "a protected-write failure must not abandon the remaining user-creation steps"); + body.Should().Contain("Logging.LogException(ex);"); + body.Should().Contain("}, cancellationToken);", + "cancellation must still be passed to the department-scoped save"); + } + + [Test] + public void Edit_profile_writes_all_moved_fields_to_one_department_row() + { + var source = ControllerSource("HomeController.cs"); + var post = MethodBody(source, + "public async Task EditUserProfile(EditProfileModel model"); + var writer = MethodBody(source, + "private async Task SaveMemberSensitiveProfileAsync(EditProfileModel model"); + + post.Should().Contain("SaveMemberSensitiveProfileAsync(model, savedProfile, cancellationToken)"); + post.Should().NotContain("savedProfile.IdentificationNumber = model.Profile.IdentificationNumber", + "the global profile column is a read-only migration source"); + + foreach (var assignment in new[] + { + "sensitive.IdentificationNumber = v", + "sensitive.HomeAddress1 = v", "sensitive.HomeCity = v", "sensitive.HomeState = v", + "sensitive.HomePostalCode = v", "sensitive.HomeCountry = v", + "sensitive.MailingAddress1 = v", "sensitive.MailingCity = v", "sensitive.MailingState = v", + "sensitive.MailingPostalCode = v", "sensitive.MailingCountry = v" + }) + { + writer.Should().Contain(assignment); + } + + writer.Should().Contain("value == ProtectedDataEnvelope.RedactionValue", + "an unrevealed protected value must never be overwritten by its UI sentinel"); + writer.Should().Contain("sensitive.LegacyProfileRelocatedOn = DateTime.UtcNow", + "an unprotected edit of the complete form must win over stale legacy values"); + } + + [Test] + public void Pre_contract_fallback_is_read_only_guarded_and_stops_after_relocation() + { + var source = ControllerSource("HomeController.cs"); + var get = MethodBody(source, + "public async Task EditUserProfile(string userId)"); + + get.Should().Contain("!protectionEnforced"); + get.Should().Contain("!memberAddresses.LegacyProfileRelocatedOn.HasValue", + "a blank target after the marker can be an intentional clear and must not fall back"); + get.Should().Contain("model.Profile.HomeAddressId.Value"); + get.Should().Contain("model.Profile.MailingAddressId.Value"); + get.IndexOf("await HydrateMemberIdentificationNumberAsync", StringComparison.Ordinal).Should().BeGreaterThan( + get.IndexOf("model.Profile = new UserProfile();", StringComparison.Ordinal), + "the fallback profile must exist before department-scoped identification data is hydrated"); + get.Should().NotContain("savedProfile.HomeAddressId =", + "legacy addresses are displayed only to bridge the relocation window, never rewritten"); + } + } +} diff --git a/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs b/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs index 24e98d56..097227a3 100644 --- a/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs +++ b/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs @@ -437,11 +437,13 @@ public async Task EditUserProfile(string userId) model.Email = model.User.Email; model.Profile = await _userProfileService.GetProfileByUserIdAsync(userId, true); - await HydrateMemberIdentificationNumberAsync(model, userId); + var protectionEnforced = await _dataProtectionService.IsProtectionEnforcedAsync(DepartmentId); if (model.Profile == null) model.Profile = new UserProfile(); + await HydrateMemberIdentificationNumberAsync(model, userId, protectionEnforced); + // Security PIN is only shown to the profile's owner (never to admins editing another user). model.DepartmentForcesSecurityPin = await _departmentSettingsService.GetForceChatbotSecurityPinAsync(DepartmentId); if (model.IsOwnProfile) @@ -463,10 +465,12 @@ public async Task EditUserProfile(string userId) await _protectedReadService.ResolveMemberSensitiveDataForReadAsync(DepartmentId, new[] { memberAddresses }, Request.Headers["X-Resgrid-Protected-Grant"].ToString(), UserId); - // M0141 (contract) cleared the legacy shared-Addresses links and deleted the rows nothing - // else referenced, so there is no fallback left to read: the department-scoped copy is - // the only copy. The protection state is still needed for the reveal banner below. - var protectionEnforced = await _dataProtectionService.IsProtectionEnforcedAsync(DepartmentId); + // During the expand/relocate window, an unprotected member whose target row has not been + // stamped may still need the legacy address as a read-only fallback. Never fall back after + // relocation (a blank target can be an intentional clear), and never bypass ADP for a + // protected department by rendering the plaintext legacy copy. + var legacyAddressFallbackAllowed = !protectionEnforced && + (memberAddresses == null || !memberAddresses.LegacyProfileRelocatedOn.HasValue); // When protection is enforced, this page is showing placeholders for the identification // number, the addresses, the emergency contacts and the custom fields, and a step-up can @@ -481,6 +485,18 @@ await _protectedReadService.ResolveMemberSensitiveDataForReadAsync(DepartmentId, model.PhysicalPostalCode = memberAddresses.HomePostalCode; model.PhysicalState = memberAddresses.HomeState; } + else if (legacyAddressFallbackAllowed && model.Profile != null && model.Profile.HomeAddressId.HasValue) + { + var homeAddress = await _addressService.GetAddressByIdAsync(model.Profile.HomeAddressId.Value); + if (homeAddress != null) + { + model.PhysicalAddress1 = homeAddress.Address1; + model.PhysicalCity = homeAddress.City; + model.PhysicalCountry = homeAddress.Country; + model.PhysicalPostalCode = homeAddress.PostalCode; + model.PhysicalState = homeAddress.State; + } + } if (memberAddresses != null && !string.IsNullOrWhiteSpace(memberAddresses.MailingAddress1)) { @@ -513,6 +529,26 @@ bool SameComponent(string mailing, string home) => SameComponent(memberAddresses.MailingPostalCode, memberAddresses.HomePostalCode) && SameComponent(memberAddresses.MailingCountry, memberAddresses.HomeCountry); } + else if (legacyAddressFallbackAllowed && model.Profile != null && model.Profile.MailingAddressId.HasValue) + { + if (model.Profile.HomeAddressId.HasValue && + model.Profile.MailingAddressId.Value == model.Profile.HomeAddressId.Value) + { + model.MailingAddressSameAsPhysical = true; + } + else + { + var mailingAddress = await _addressService.GetAddressByIdAsync(model.Profile.MailingAddressId.Value); + if (mailingAddress != null) + { + model.MailingAddress1 = mailingAddress.Address1; + model.MailingCity = mailingAddress.City; + model.MailingCountry = mailingAddress.Country; + model.MailingPostalCode = mailingAddress.PostalCode; + model.MailingState = mailingAddress.State; + } + } + } if (model.Profile != null) model.Carrier = (MobileCarriers)model.Profile.MobileCarrier; @@ -773,7 +809,7 @@ public async Task EditUserProfile(EditProfileModel model, IFormCo auditEvent.ServerName = Environment.MachineName; auditEvent.UserAgent = $"{Request.Headers["User-Agent"]} {Request.Headers["Accept-Language"]}"; - var savedProfile = await _userProfileService.GetProfileByUserIdAsync(model.UserId); + var savedProfile = await _userProfileService.GetProfileByUserIdAsync(model.UserId, true); if (savedProfile == null) savedProfile = new UserProfile(); @@ -801,13 +837,9 @@ public async Task EditUserProfile(EditProfileModel model, IFormCo savedProfile.HomeNumber = (homeResult != null && homeResult.IsValid && !string.IsNullOrWhiteSpace(homeResult.InternationalNumber)) ? homeResult.InternationalNumber : model.Profile.HomeNumber; - // The identification number is DEPARTMENT-SCOPED (ADP plan 5.1): a profile row is - // global to the user, so it can neither be encrypted with one department's key nor - // hold the different numbers different departments issue the same person. The - // profile column is left untouched here — it is dropped in the contract migration - // once this is deployed. - await SaveMemberIdentificationNumberAsync(model.UserId, model.Profile.IdentificationNumber, cancellationToken); - await SaveMemberAddressesAsync(model, cancellationToken); + // Identification number and addresses are department-scoped. The legacy profile + // fields remain read-only migration sources throughout the expand/relocate window. + await SaveMemberSensitiveProfileAsync(model, savedProfile, cancellationToken); savedProfile.TimeZone = model.Profile.TimeZone; savedProfile.Language = model.Profile.Language; @@ -873,7 +905,7 @@ public async Task EditUserProfile(EditProfileModel model, IFormCo } // Addresses are NOT written back to the shared Addresses table or relinked on the - // profile. SaveMemberAddressesAsync above is the only writer now (plan 5.1): the + // profile. SaveMemberSensitiveProfileAsync above is the only writer now (plan 5.1): the // department-scoped copy is the one that can be encrypted, and keeping a second // plaintext copy in sync would recreate exactly the leak this move exists to close. // The legacy link is left as it stands for members relocation has not reached yet; @@ -1546,7 +1578,8 @@ public class EmergencyContactInput /// Protected departments resolve it through the read pipeline, so it arrives as plaintext /// with a valid grant and as the REDACTED placeholder without one — never as ciphertext. /// - private async Task HydrateMemberIdentificationNumberAsync(EditProfileModel model, string userId) + private async Task HydrateMemberIdentificationNumberAsync(EditProfileModel model, string userId, + bool protectionEnforced) { if (model?.Profile == null) return; @@ -1554,24 +1587,42 @@ private async Task HydrateMemberIdentificationNumberAsync(EditProfileModel model var sensitive = await _memberSensitiveDataService.GetByDepartmentAndUserAsync(DepartmentId, userId); if (sensitive == null) { - model.Profile.IdentificationNumber = null; + // Dapper still hydrates the ignored legacy column during the relocation window. It is + // safe as a read-only fallback only for an unprotected department; a protected + // department must wait for the worker to move it through the encrypted write path. + if (protectionEnforced) + model.Profile.IdentificationNumber = null; return; } await _protectedReadService.ResolveMemberSensitiveDataForReadAsync(DepartmentId, new[] { sensitive }, Request.Headers["X-Resgrid-Protected-Grant"].ToString(), UserId); - model.Profile.IdentificationNumber = sensitive.IdentificationNumber; + // Once stamped, an empty target is authoritative (the member may have cleared it). Before + // the stamp, an unprotected row with an empty target can still read the legacy source so + // this edit itself completes the move instead of presenting a surprising blank. + if (protectionEnforced || sensitive.LegacyProfileRelocatedOn.HasValue || + !string.IsNullOrWhiteSpace(sensitive.IdentificationNumber)) + { + model.Profile.IdentificationNumber = sensitive.IdentificationNumber; + } } /// - /// Persists the member's department-scoped home and mailing addresses (plan 5.1). Values - /// still showing the REDACTED placeholder were never revealed to this user and are skipped - /// rather than written back over the stored address. + /// Persists all fields moved by the member-profile relocation as one department-scoped row. + /// Values still showing the REDACTED placeholder were never revealed to this user and are + /// skipped rather than written back over the stored value. + /// + /// An unprotected edit also completes the relocation marker. The GET action either loaded the + /// department value or supplied the guarded legacy fallback for every field, so the submitted + /// row is authoritative even when the user deliberately cleared a value. Protected departments + /// are left unstamped for MemberProfileRelocationService, which is the only path allowed to move + /// a plaintext legacy value into an enrolled row. /// - private async Task SaveMemberAddressesAsync(EditProfileModel model, CancellationToken cancellationToken) + private async Task SaveMemberSensitiveProfileAsync(EditProfileModel model, UserProfile legacyProfile, + CancellationToken cancellationToken) { - if (model == null) + if (model?.Profile == null) return; var sensitive = await _memberSensitiveDataService.GetByDepartmentAndUserAsync(DepartmentId, model.UserId); @@ -1591,18 +1642,37 @@ void Apply(string submitted, Action set) var home1 = model.PhysicalAddress1; var mailing1 = model.MailingAddressSameAsPhysical ? model.PhysicalAddress1 : model.MailingAddress1; + var mailingCity = model.MailingAddressSameAsPhysical ? model.PhysicalCity : model.MailingCity; + var mailingState = model.MailingAddressSameAsPhysical ? model.PhysicalState : model.MailingState; + var mailingPostalCode = model.MailingAddressSameAsPhysical ? model.PhysicalPostalCode : model.MailingPostalCode; + var mailingCountry = model.MailingAddressSameAsPhysical ? model.PhysicalCountry : model.MailingCountry; + + var submittedValues = new[] + { + model.Profile.IdentificationNumber, + home1, model.PhysicalCity, model.PhysicalState, model.PhysicalPostalCode, model.PhysicalCountry, + mailing1, mailingCity, mailingState, mailingPostalCode, mailingCountry + }; + + var protectionEnforced = await _dataProtectionService.IsProtectionEnforcedAsync(DepartmentId); + var canCompleteRelocation = !protectionEnforced && submittedValues.All(value => !Unchanged(value)); + var hasSubmittedData = submittedValues.Any(value => !Unchanged(value) && !string.IsNullOrWhiteSpace(value)); + var hasLegacyData = legacyProfile != null && + (!string.IsNullOrWhiteSpace(legacyProfile.IdentificationNumber) || legacyProfile.HomeAddressId.HasValue || + legacyProfile.MailingAddressId.HasValue); if (isNewRow) { - // Nothing stored yet, so there is nothing a sentinel could protect; if the form - // carries no address at all there is nothing to create either. - if ((Unchanged(home1) || string.IsNullOrWhiteSpace(home1)) && - (Unchanged(mailing1) || string.IsNullOrWhiteSpace(mailing1))) + // Usually an all-empty form needs no row. The exception is an unprotected profile + // that still has legacy data: an all-empty submission there is an intentional clear, + // so persist an empty, stamped target rather than letting the worker resurrect it. + if (!hasSubmittedData && !(canCompleteRelocation && hasLegacyData)) return; sensitive = new DepartmentMemberSensitiveData { DepartmentId = DepartmentId, UserId = model.UserId }; } + Apply(model.Profile.IdentificationNumber, v => sensitive.IdentificationNumber = v); Apply(home1, v => sensitive.HomeAddress1 = v); Apply(model.PhysicalCity, v => sensitive.HomeCity = v); Apply(model.PhysicalState, v => sensitive.HomeState = v); @@ -1613,36 +1683,13 @@ void Apply(string submitted, Action set) // encrypted per row, so there is nothing to share and a later edit to one must not // silently rewrite the other. Apply(mailing1, v => sensitive.MailingAddress1 = v); - Apply(model.MailingAddressSameAsPhysical ? model.PhysicalCity : model.MailingCity, v => sensitive.MailingCity = v); - Apply(model.MailingAddressSameAsPhysical ? model.PhysicalState : model.MailingState, v => sensitive.MailingState = v); - Apply(model.MailingAddressSameAsPhysical ? model.PhysicalPostalCode : model.MailingPostalCode, v => sensitive.MailingPostalCode = v); - Apply(model.MailingAddressSameAsPhysical ? model.PhysicalCountry : model.MailingCountry, v => sensitive.MailingCountry = v); - - await _memberSensitiveDataService.SaveAsync(sensitive, cancellationToken); - } - - /// - /// Persists the member's department-scoped identification number, creating the row on first - /// use. A value still showing the REDACTED placeholder was never revealed to this user, so it - /// is ignored rather than written back over the stored value. - /// - private async Task SaveMemberIdentificationNumberAsync(string userId, string identificationNumber, - CancellationToken cancellationToken) - { - if (identificationNumber == ProtectedDataEnvelope.RedactionValue) - return; - - var sensitive = await _memberSensitiveDataService.GetByDepartmentAndUserAsync(DepartmentId, userId); - - if (sensitive == null) - { - if (string.IsNullOrWhiteSpace(identificationNumber)) - return; - - sensitive = new DepartmentMemberSensitiveData { DepartmentId = DepartmentId, UserId = userId }; - } + Apply(mailingCity, v => sensitive.MailingCity = v); + Apply(mailingState, v => sensitive.MailingState = v); + Apply(mailingPostalCode, v => sensitive.MailingPostalCode = v); + Apply(mailingCountry, v => sensitive.MailingCountry = v); - sensitive.IdentificationNumber = identificationNumber; + if (canCompleteRelocation && !sensitive.LegacyProfileRelocatedOn.HasValue) + sensitive.LegacyProfileRelocatedOn = DateTime.UtcNow; await _memberSensitiveDataService.SaveAsync(sensitive, cancellationToken); } diff --git a/Web/Resgrid.Web/Areas/User/Controllers/PersonnelController.cs b/Web/Resgrid.Web/Areas/User/Controllers/PersonnelController.cs index d7e0c043..3cfa24ba 100644 --- a/Web/Resgrid.Web/Areas/User/Controllers/PersonnelController.cs +++ b/Web/Resgrid.Web/Areas/User/Controllers/PersonnelController.cs @@ -63,6 +63,7 @@ public class PersonnelController : SecureBaseController private readonly IPhoneNumberProcesserProvider _phoneNumberProcesser; private readonly IExternalIdentityLinkService _externalIdentityLinkService; private readonly IProtectedReadService _protectedReadService; + private readonly IDepartmentMemberSensitiveDataService _memberSensitiveDataService; public PersonnelController(IDepartmentsService departmentsService, IUsersService usersService, IActionLogsService actionLogsService, IEmailService emailService, IUserProfileService userProfileService, IDeleteService deleteService, Model.Services.IAuthorizationService authorizationService, @@ -71,7 +72,8 @@ public PersonnelController(IDepartmentsService departmentsService, IUsersService IGeoService geoService, UserManager userManager, IDepartmentSettingsService departmentSettingsService, ICallsService callsService, IGeoLocationProvider geoLocationProvider, IMappingService mappingService, IUserDefinedFieldsService userDefinedFieldsService, IUdfRenderingService udfRenderingService, IStringLocalizer localizer, IPhoneNumberProcesserProvider phoneNumberProcesser, - IExternalIdentityLinkService externalIdentityLinkService, IProtectedReadService protectedReadService) + IExternalIdentityLinkService externalIdentityLinkService, IProtectedReadService protectedReadService, + IDepartmentMemberSensitiveDataService memberSensitiveDataService) { _departmentsService = departmentsService; _usersService = usersService; @@ -100,6 +102,7 @@ public PersonnelController(IDepartmentsService departmentsService, IUsersService _phoneNumberProcesser = phoneNumberProcesser; _externalIdentityLinkService = externalIdentityLinkService; _protectedReadService = protectedReadService; + _memberSensitiveDataService = memberSensitiveDataService; } #endregion Private Members and Constructors @@ -607,6 +610,12 @@ public async Task AddPerson(AddPersonModel model, IFormCollection var result = await _userManager.CreateAsync(user, model.NewPassword); if (result.Succeeded) { + // IdentificationNumber is department-issued. Keep it off the new global profile + // row even while the legacy columns remain available as relocation sources. + var identificationNumber = model.Profile.IdentificationNumber; + model.Profile.IdentificationNumber = null; + model.Profile.HomeAddressId = null; + model.Profile.MailingAddressId = null; model.Profile.UserId = user.UserId; model.Profile.MobileCarrier = (int)model.Carrier; model.Profile.FirstName = model.FirstName; @@ -624,6 +633,26 @@ public async Task AddPerson(AddPersonModel model, IFormCollection await _departmentsService.AddUserToDepartmentAsync(DepartmentId, user.UserId, false, cancellationToken); + if (!string.IsNullOrWhiteSpace(identificationNumber)) + { + try + { + await _memberSensitiveDataService.SaveAsync(new DepartmentMemberSensitiveData + { + DepartmentId = DepartmentId, + UserId = user.UserId, + IdentificationNumber = identificationNumber, + // This is a brand-new profile with no legacy data to relocate. Stamping it + // prevents a later sweep from treating the row as an incomplete move. + LegacyProfileRelocatedOn = DateTime.UtcNow + }, cancellationToken); + } + catch (InvalidOperationException ex) + { + Logging.LogException(ex); + } + } + if (model.MustChangePasswordOnLogin) { var member = await _departmentsService.GetDepartmentMemberAsync(user.UserId, DepartmentId);