diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DeviceTokenServiceTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DeviceTokenServiceTests.cs index 6c1adb49..31659491 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DeviceTokenServiceTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DeviceTokenServiceTests.cs @@ -1,3 +1,4 @@ +using System; using System.Linq; using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; @@ -10,6 +11,8 @@ using NSubstitute; using NUnit.Framework; using TimePlanning.Pn.Services.DeviceTokenService; +using TimePlanning.Pn.Services.PushNotificationService; +using RegisterDeviceTokenModel = TimePlanning.Pn.Infrastructure.Models.DeviceToken.RegisterDeviceTokenModel; namespace TimePlanning.Pn.Test; @@ -313,21 +316,360 @@ public async Task RegisterAsync_Adoption_DoesNotCrossAppId() }); } + // --------------------------------------------------------------------- + // Backward compatibility with clients shipped before the identity model. + // + // proto3 decodes an absent string as "", so every already-installed build + // sends "" for app_id AND installation_id. Rejecting those would leave the + // whole fleet unable to register (new installs get no push at all; a + // rotated FCM token can never re-register) until the app reaches stores - + // and flutter-time reports each rejection to Sentry, so it would also be a + // continuous fleet-wide warning flood. + // --------------------------------------------------------------------- + + private const string LegacyGoldenToken = "legacy-tok-golden"; + + // sha256(LegacyGoldenToken), lowercase hex, under the reserved prefix. + // Hard-coded on purpose - never computed from the production helper: the + // literal pins the algorithm, the casing and the prefix, so a random or + // row-id-derived scheme cannot pass. + private const string LegacyGoldenInstallationId = + "legacy-token:04095630657a3b3ffbc147418b20e2c539cd9d69bde95c081049bad010b7a71e"; + + [Test] + public async Task RegisterAsync_EmptyAppId_DefaultsToTimeAndStores() + { + var result = await _service.RegisterAsync( + 351, "tok-noapp", "android", 7, "", "inst-noapp"); + + Assert.That(result.Success, Is.True); + var stored = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.Multiple(() => + { + Assert.That(stored.AppId, Is.EqualTo(DeviceTokenService.TimePlanningAppId), + "this service backs exactly one app, so an empty app_id is not ambiguous"); + Assert.That(stored.InstallationId, Is.EqualTo("inst-noapp")); + Assert.That(stored.SdkSiteId, Is.EqualTo(351)); + Assert.That(stored.AppBuildNumber, Is.EqualTo(7)); + }); + } + [Test] - public async Task RegisterAsync_EmptyInstallationId_IsRejectedWithoutStoring() + public async Task RegisterAsync_EmptyInstallationId_NoExistingRow_CreatesRowWithDeterministicId() { - var result = await _service.RegisterAsync(350, "tok-x", "android", 0, "time", ""); + var result = await _service.RegisterAsync( + 360, LegacyGoldenToken, "android", 12, "time", ""); + + Assert.That(result.Success, Is.True); + var stored = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.Multiple(() => + { + Assert.That(stored.InstallationId, Is.EqualTo(LegacyGoldenInstallationId), + "the synthetic id must be a pure function of the token, or the same device " + + "would insert a fresh row on every legacy register"); + Assert.That(stored.InstallationId, Has.Length.LessThanOrEqualTo(128), + "InstallationId is varchar(128)"); + Assert.That(stored.SdkSiteId, Is.EqualTo(360)); + Assert.That(stored.FcmToken, Is.EqualTo(LegacyGoldenToken)); + Assert.That(stored.AppId, Is.EqualTo("time")); + Assert.That(stored.Platform, Is.EqualTo("android")); + Assert.That(stored.AppBuildNumber, Is.EqualTo(12)); + Assert.That(stored.WorkflowState, Is.EqualTo(Constants.WorkflowStates.Created)); + }); + } + + [Test] + public async Task RegisterAsync_SyntheticInstallationId_CannotCollideWithAClientUuid() + { + await _service.RegisterAsync(361, "tok-shape", "android", 0, "", ""); + + var stored = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.Multiple(() => + { + Assert.That(stored.InstallationId, Does.StartWith("legacy-token:")); + Assert.That(Guid.TryParse(stored.InstallationId, out _), Is.False, + "real clients send a v4 UUID; the reserved prefix is outside that alphabet"); + }); + } + + [Test] + public async Task RegisterAsync_EmptyInstallationIdTwice_UpdatesTheSameRow() + { + await _service.RegisterAsync(370, "tok-legacy-twice", "android", 1, "", ""); + var first = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + + var result = await _service.RegisterAsync(371, "tok-legacy-twice", "ios", 2, "", ""); + + Assert.That(result.Success, Is.True); + var rows = await TimePlanningPnDbContext.DeviceTokens.AsNoTracking().ToListAsync(); + Assert.That(rows, Has.Count.EqualTo(1), + "a repeated legacy register from one device must not insert a duplicate"); + Assert.Multiple(() => + { + Assert.That(rows[0].Id, Is.EqualTo(first.Id)); + Assert.That(rows[0].SdkSiteId, Is.EqualTo(371)); + Assert.That(rows[0].Platform, Is.EqualTo("ios")); + Assert.That(rows[0].AppBuildNumber, Is.EqualTo(2)); + }); + } + + [Test] + public async Task RegisterAsync_EmptyInstallationId_ExistingRowForToken_UpdatesInPlace() + { + await _service.RegisterAsync(380, "tok-existing", "android", 5, "time", "inst-real-existing"); + var before = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + + var result = await _service.RegisterAsync(381, "tok-existing", "ios", 6, "", ""); + + Assert.That(result.Success, Is.True); + var rows = await TimePlanningPnDbContext.DeviceTokens.AsNoTracking().ToListAsync(); + Assert.That(rows, Has.Count.EqualTo(1)); + Assert.Multiple(() => + { + Assert.That(rows[0].Id, Is.EqualTo(before.Id), + "match-by-token is the pre-change behaviour and must reuse the row"); + Assert.That(rows[0].InstallationId, Is.EqualTo("inst-real-existing"), + "a legacy register must not downgrade a real install id to the synthetic one"); + Assert.That(rows[0].SdkSiteId, Is.EqualTo(381)); + Assert.That(rows[0].Platform, Is.EqualTo("ios")); + Assert.That(rows[0].AppBuildNumber, Is.EqualTo(6)); + }); + } + + // The whole point of a deterministic synthetic id: when the fleet finally + // upgrades, the real installation_id must CLAIM the row the legacy + // registers created. A second row would be live too, carry the same token + // and site, and the sender would select both - doubled pushes forever. + [Test] + public async Task RegisterAsync_LegacyRow_IsAdoptedWhenTheClientUpgrades() + { + await _service.RegisterAsync(390, "tok-upgrade", "android", 0, "", ""); + var legacyRow = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.That(legacyRow.InstallationId, Does.StartWith("legacy-token:")); + + var result = await _service.RegisterAsync( + 390, "tok-upgrade", "android", 43000, "time", "11111111-2222-4333-8444-555555555555"); + + Assert.That(result.Success, Is.True); + var rows = await TimePlanningPnDbContext.DeviceTokens.AsNoTracking().ToListAsync(); + Assert.That(rows, Has.Count.EqualTo(1), + "the upgraded client must claim the legacy row, not add a second one"); + Assert.Multiple(() => + { + Assert.That(rows[0].Id, Is.EqualTo(legacyRow.Id)); + Assert.That(rows[0].InstallationId, Is.EqualTo("11111111-2222-4333-8444-555555555555")); + Assert.That(rows[0].AppBuildNumber, Is.EqualTo(43000)); + }); + } + + [Test] + public async Task RegisterAsync_SoftDeletedLegacyRow_EmptyInstallationId_IsRevived() + { + await _service.RegisterAsync(400, "tok-legacy-pruned", "android", 0, "", ""); + var row = await TimePlanningPnDbContext!.DeviceTokens.SingleAsync(); + await row.Delete(TimePlanningPnDbContext); + + var result = await _service.RegisterAsync(400, "tok-legacy-pruned", "android", 0, "", ""); + + Assert.That(result.Success, Is.True); + var rows = await TimePlanningPnDbContext.DeviceTokens.AsNoTracking().ToListAsync(); + Assert.That(rows, Has.Count.EqualTo(1), + "a pruned row must be revived, never inserted alongside"); + Assert.That(rows[0].WorkflowState, Is.EqualTo(Constants.WorkflowStates.Created)); + } + + [Test] + public async Task RegisterAsync_LegacyRegister_DoesNotTakeAnotherAppsRow() + { + var foreign = new DeviceToken + { + AppId = "adhoc", + InstallationId = "inst-adhoc", + FcmToken = "tok-cross", + SdkSiteId = 410, + Platform = "android", + }; + await foreign.Create(TimePlanningPnDbContext!); + + await _service.RegisterAsync(410, "tok-cross", "android", 0, "", ""); + + var rows = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking() + .OrderBy(r => r.Id).ToListAsync(); + Assert.That(rows, Has.Count.EqualTo(2)); + Assert.Multiple(() => + { + Assert.That(rows[0].AppId, Is.EqualTo("adhoc")); + Assert.That(rows[0].InstallationId, Is.EqualTo("inst-adhoc")); + Assert.That(rows[1].AppId, Is.EqualTo("time")); + Assert.That(rows[1].InstallationId, Does.StartWith("legacy-token:")); + }); + } + + // fcm_token stays hard-required: there is no sensible fallback for an + // absent token, and it is not one of the fields old clients omit. + [Test] + public async Task RegisterAsync_EmptyToken_IsRejectedWithoutStoring() + { + var result = await _service.RegisterAsync(420, "", "android", 0, "time", "inst-notoken"); Assert.That(result.Success, Is.False); Assert.That(await TimePlanningPnDbContext!.DeviceTokens.CountAsync(), Is.EqualTo(0)); } + // Deliberately passes both before and after this change: it is the + // requirement that the modern path stays untouched, not a change detector. + // The modern path's adoption leg is covered separately by + // RegisterAsync_LegacyBackfilledRow_IsAdoptedNotDuplicated. + [Test] + public async Task RegisterAsync_BothFieldsSupplied_StoresExactlyWhatTheClientSent() + { + var result = await _service.RegisterAsync( + 430, "tok-modern", "ios", 44000, "time", "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"); + + Assert.That(result.Success, Is.True); + var stored = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.Multiple(() => + { + Assert.That(stored.AppId, Is.EqualTo("time")); + Assert.That(stored.InstallationId, Is.EqualTo("aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"), + "the modern path must store the client's id verbatim"); + Assert.That(stored.SdkSiteId, Is.EqualTo(430)); + Assert.That(stored.Platform, Is.EqualTo("ios")); + Assert.That(stored.AppBuildNumber, Is.EqualTo(44000)); + }); + } + + // Two legacy registers can be in flight at once on a client's FIRST launch: + // flutter-time's personal-view init and its onTokenRefresh handler both + // register the same token, so both miss the token lookup and both derive + // the SAME synthetic id - the loser hits the unique index. That must not + // surface as a failed register, because the client turns a failure into the + // Sentry warning this whole change exists to stop. + // + // Seeding a row that already holds the synthetic id under a stale token + // reproduces the losing insert deterministically, without threads. + [Test] + public async Task RegisterAsync_LegacyInsertHitsTheUniqueIndex_AdoptsTheExistingRow() + { + var occupier = new DeviceToken + { + AppId = "time", + InstallationId = LegacyGoldenInstallationId, + FcmToken = "stale-token", + SdkSiteId = 500, + Platform = "android", + }; + await occupier.Create(TimePlanningPnDbContext!); + + var result = await _service.RegisterAsync(501, LegacyGoldenToken, "ios", 9, "", ""); + + Assert.That(result.Success, Is.True, + "losing the insert race must not be reported to the client as a failure"); + var rows = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().ToListAsync(); + Assert.That(rows, Has.Count.EqualTo(1)); + Assert.Multiple(() => + { + Assert.That(rows[0].Id, Is.EqualTo(occupier.Id)); + Assert.That(rows[0].FcmToken, Is.EqualTo(LegacyGoldenToken)); + Assert.That(rows[0].SdkSiteId, Is.EqualTo(501)); + Assert.That(rows[0].Platform, Is.EqualTo("ios")); + Assert.That(rows[0].AppBuildNumber, Is.EqualTo(9)); + }); + } + + // A legacy client's token ROTATION cannot land on the old row - nothing + // ties the new token to the old one without a real install id - so it + // leaves a second row behind and the device is pushed to twice until FCM + // reports the dead token. This pins that honestly: it is the + // pre-identity-model behaviour, not something this change introduced, and + // it ends the moment the client upgrades and sends a real installation id. + [Test] + public async Task RegisterAsync_LegacyRegister_AfterTokenRotation_LeavesTheOldRowBehind() + { + await _service.RegisterAsync(460, "tok-rot-1", "android", 0, "", ""); + await _service.RegisterAsync(460, "tok-rot-2", "android", 0, "", ""); + + var rows = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking() + .OrderBy(r => r.Id).ToListAsync(); + Assert.That(rows.Select(r => r.FcmToken), + Is.EqualTo(new[] { "tok-rot-1", "tok-rot-2" }), + "documented limitation: without a real install id the rotated token is a new row"); + } + + // Rejecting an empty token matters most when a row already exists: the + // unguarded code reached the field assignment and blanked a WORKING token, + // taking that device dark. If anyone moves the guard below the identity + // lookup, this fails. [Test] - public async Task RegisterAsync_EmptyAppId_IsRejectedWithoutStoring() + public async Task RegisterAsync_EmptyToken_ExistingRow_DoesNotWipeTheStoredToken() { - var result = await _service.RegisterAsync(351, "tok-y", "android", 0, "", "inst-noapp"); + await _service.RegisterAsync(470, "tok-keep", "android", 3, "time", "inst-keep"); + + var result = await _service.RegisterAsync(470, "", "android", 3, "time", "inst-keep"); Assert.That(result.Success, Is.False); - Assert.That(await TimePlanningPnDbContext!.DeviceTokens.CountAsync(), Is.EqualTo(0)); + var stored = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.That(stored.FcmToken, Is.EqualTo("tok-keep"), + "an empty token must never overwrite a live one"); + } + + // The point of DEFAULTING app_id rather than rejecting it: the row a legacy + // register writes must still be picked up by the send path, whose query is + // keyed on AppId (IX_DeviceTokens_AppId_SdkSiteId_WorkflowState). A row + // stored with a null or empty AppId would be invisible to every push - + // which would defeat the entire change. + [Test] + public async Task RegisterAsync_LegacyRegisteredRow_IsSelectedByTheSendPath() + { + await _service.RegisterAsync(450, "tok-sendpath", "android", 0, "", ""); + + // Firebase is not configured in tests, so this constructor never + // touches the process-wide FirebaseApp registry; only the token- + // selection seam is exercised. + var push = new PushNotificationService( + TimePlanningPnDbContext!, + Substitute.For>()); + + var targeted = await push.ResolveTargetTokensAsync(450, minBuild: 0); + + Assert.That(targeted.Select(t => t.FcmToken), + Is.EquivalentTo(new[] { "tok-sendpath" }), + "a legacy-registered device must still be targeted by an ungated send. " + + "Version-gated sends (minBuild > 0) still skip it, because a client " + + "old enough to omit installation_id also reports AppBuildNumber 0"); + } + + // The REST endpoint is reachable too, and DeviceTokenController.Register + // hands the bound model straight to RegisterAsync. This pins the seam that + // can silently drift - RegisterDeviceTokenModel's defaults, which are what + // a pre-identity-model JSON body binds to - by feeding them through the + // service. It does not exercise the controller itself (that needs the JWT + // site resolution); the controller's pass-through is a two-line method. + [Test] + public async Task RegisterAsync_RestModelDefaults_TakeTheLegacyPath() + { + var model = new RegisterDeviceTokenModel + { + Token = "tok-rest", + Platform = "android", + }; + Assert.Multiple(() => + { + Assert.That(model.AppId, Is.Empty); + Assert.That(model.InstallationId, Is.Empty); + }); + + var result = await _service.RegisterAsync( + 440, model.Token, model.Platform, model.BuildNumber, + model.AppId, model.InstallationId); + + Assert.That(result.Success, Is.True); + var stored = await TimePlanningPnDbContext!.DeviceTokens.AsNoTracking().SingleAsync(); + Assert.Multiple(() => + { + Assert.That(stored.AppId, Is.EqualTo("time")); + Assert.That(stored.InstallationId, Does.StartWith("legacy-token:")); + Assert.That(stored.SdkSiteId, Is.EqualTo(440)); + }); } } diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Controllers/DeviceTokenController.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Controllers/DeviceTokenController.cs index 407a6ca3..5680d7aa 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Controllers/DeviceTokenController.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Controllers/DeviceTokenController.cs @@ -33,6 +33,9 @@ public DeviceTokenController( _baseDbContext = baseDbContext; } + // Reachable public endpoint even though the shipped client uses gRPC: a + // posted body that omits app_id / installation_id binds them to "", which + // the service reads exactly the way it reads the gRPC path's "". [HttpPost] public async Task Register([FromBody] RegisterDeviceTokenModel model) { diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/DeviceToken/RegisterDeviceTokenModel.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/DeviceToken/RegisterDeviceTokenModel.cs index 58c9b0cc..34d7fc4a 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/DeviceToken/RegisterDeviceTokenModel.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/DeviceToken/RegisterDeviceTokenModel.cs @@ -5,11 +5,16 @@ public class RegisterDeviceTokenModel public string Token { get; set; } = string.Empty; public string Platform { get; set; } = string.Empty; - /// Which app minted the token. Always "time" here. + /// + /// Which app minted the token. Always "time" here, so a body that omits it + /// (binding to "") is read as "time" rather than rejected. + /// public string AppId { get; set; } = string.Empty; /// - /// Stable per-install UUID; the identity of the stored row. Required. + /// Stable per-install UUID; the identity of the stored row. A body that + /// omits it binds to "" and falls back to matching on the FCM token - see + /// IDeviceTokenService.RegisterAsync. /// public string InstallationId { get; set; } = string.Empty; diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/DeviceTokenService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/DeviceTokenService.cs index f9f0b1f9..41f04b80 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/DeviceTokenService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/DeviceTokenService.cs @@ -2,6 +2,8 @@ namespace TimePlanning.Pn.Services.DeviceTokenService; using System; using System.Linq; +using System.Security.Cryptography; +using System.Text; using System.Threading.Tasks; using Infrastructure.Helpers; using Microsoft.EntityFrameworkCore; @@ -95,53 +97,105 @@ public async Task RegisterAsync( { try { - if (string.IsNullOrWhiteSpace(appId)) + // The token is the one field with no sensible fallback, and it is + // not what old clients omit - keep it hard-required. Without this + // guard the legacy path below would derive an installation id from + // the empty string and store a junk row. + if (string.IsNullOrWhiteSpace(token)) { _logger.LogWarning( - "Rejecting device-token registration: app_id is required"); - return new OperationResult(false, "app_id is required"); + "Rejecting device-token registration: fcm_token is required"); + return new OperationResult(false, "fcm_token is required"); } - if (string.IsNullOrWhiteSpace(installationId)) + // Clients shipped before the identity model send neither app_id nor + // installation_id, and proto3 decodes an absent string as "". They + // must keep registering until the app reaches stores, so both get a + // backward-compatible reading here rather than a rejection. + // + // app_id: this service backs exactly one app, so "" is unambiguous. + // (The same relaxation would NOT be sound in BackendConfiguration, + // whose table is shared by two apps.) + if (string.IsNullOrWhiteSpace(appId)) { - _logger.LogWarning( - "Rejecting device-token registration: installation_id is required"); - return new OperationResult(false, "installation_id is required"); + appId = TimePlanningAppId; } + var isLegacyRegister = string.IsNullOrWhiteSpace(installationId); + // Upsert on the install identity, including soft-deleted rows: the // unique index has no WorkflowState filter, so Create() over a // Removed row throws instead of inserting. - var existing = await _dbContext.DeviceTokens - .FirstOrDefaultAsync(dt => dt.AppId == appId - && dt.InstallationId == installationId) - ?? await AdoptRowWithSameTokenAsync(appId, token, installationId); + DeviceToken existing; + if (isLegacyRegister) + { + // installation_id: a legacy register has none, so fall back to + // the pre-change identity - the token alone. The row found this + // way is updated in place and KEEPS its InstallationId, so a + // legacy register can never downgrade a real install id to a + // synthetic one. + existing = await FindRowWithSameTokenAsync(appId, token); + } + else + { + existing = await _dbContext.DeviceTokens + .FirstOrDefaultAsync(dt => dt.AppId == appId + && dt.InstallationId == installationId); + existing ??= await AdoptRowWithSameTokenAsync(appId, token, installationId); + } if (existing != null) { - existing.FcmToken = token; - existing.SdkSiteId = sdkSiteId; - existing.Platform = platform; - existing.AppBuildNumber = buildNumber; - // Explicit revive: PnBase.Update() leaves WorkflowState alone, - // so a row pruned after an FCM permanent failure would stay - // invisible to the send path and the device would go dark. - existing.WorkflowState = Constants.WorkflowStates.Created; + ApplyRegistration(existing, token, sdkSiteId, platform, buildNumber); await existing.Update(_dbContext); + return new OperationResult(true); } - else + + var newInstallationId = isLegacyRegister + ? SyntheticInstallationIdFor(token) + : installationId; + + var deviceToken = new DeviceToken + { + AppId = appId, + InstallationId = newInstallationId, + FcmToken = token, + SdkSiteId = sdkSiteId, + Platform = platform, + AppBuildNumber = buildNumber, + }; + + try { - var deviceToken = new DeviceToken - { - AppId = appId, - InstallationId = installationId, - FcmToken = token, - SdkSiteId = sdkSiteId, - Platform = platform, - AppBuildNumber = buildNumber, - }; await deviceToken.Create(_dbContext); } + catch (DbUpdateException) + { + // Lost an insert race on IX_DeviceTokens_AppId_InstallationId. + // Reachable on a legacy client's FIRST launch, which is the + // "new installs never register" case this change exists to fix: + // flutter-time's personal-view init and its onTokenRefresh + // handler both register the same token concurrently, so both + // miss the lookup above and both derive the same synthetic id. + // Letting that reach the catch below would return a failure the + // client turns into a Sentry warning - the exact noise being + // removed here. The winner stored the row we wanted, so take it. + // + // Also covers the rarer case of a row already holding this + // installation id under a stale token; adopting it converges on + // one row per install either way. + _dbContext.Entry(deviceToken).State = EntityState.Detached; + + var winner = await _dbContext.DeviceTokens.FirstOrDefaultAsync( + dt => dt.AppId == appId && dt.InstallationId == newInstallationId); + if (winner == null) + { + throw; + } + + ApplyRegistration(winner, token, sdkSiteId, platform, buildNumber); + await winner.Update(_dbContext); + } return new OperationResult(true); } catch (Exception ex) @@ -151,9 +205,76 @@ public async Task RegisterAsync( } } + /// + /// Writes the mutable half of a register onto an existing row: the token + /// rotates, and a different user on the same install reassigns the owner. + /// + /// The WorkflowState revive is explicit because PnBase.Update() leaves it + /// alone - a row pruned after an FCM permanent failure would otherwise stay + /// invisible to the send path and the device would go dark. + /// + private static void ApplyRegistration( + DeviceToken row, string token, int sdkSiteId, string platform, int buildNumber) + { + row.FcmToken = token; + row.SdkSiteId = sdkSiteId; + row.Platform = platform; + row.AppBuildNumber = buildNumber; + row.WorkflowState = Constants.WorkflowStates.Created; + } + + /// Reserved prefix for synthesised installation ids. + private const string LegacyInstallationIdPrefix = "legacy-token:"; + + /// + /// InstallationId stood in for a client that sent none. Derived from the + /// FCM token so that repeated legacy registers carrying the SAME token + /// resolve to the same row instead of inserting a duplicate on every call - + /// the column is NOT NULL, so an insert has to put something there. + /// + /// It cannot make a legacy client's token ROTATION land on the old row: + /// nothing ties the new token to the old one without a real install id, so + /// that inserts a second row and the device is pushed to twice until FCM + /// reports the dead token. That is exactly the pre-identity-model + /// behaviour, not a regression, and it ends the moment the client upgrades. + /// + /// The reserved prefix is what rules out a collision with a real client id: + /// clients send a canonical v4 UUID - 36 characters over [0-9a-f-] - and + /// ':' is outside that alphabet, so no client we ship can produce one. + /// (Nothing server-side VALIDATES the shape; both transports accept an + /// arbitrary string, so this is a claim about our clients, not an + /// invariant.) It is equally distinct from the migration's + /// 'legacy:<Id>' backfill. The result is 77 characters, well inside + /// InstallationId's varchar(128). + /// + /// Hashing the TOKEN is sound in THIS table because its old unique index + /// was on Token alone: tokens are unique here, so two distinct devices + /// cannot hash to the same id. The identical formula was rejected for the + /// BackendConfiguration migration, whose old key (WorkerId, FcmToken) + /// permits duplicate tokens - that rejection does not carry over. + /// + private static string SyntheticInstallationIdFor(string token) + { + var digest = SHA256.HashData(Encoding.UTF8.GetBytes(token)); + return LegacyInstallationIdPrefix + Convert.ToHexString(digest).ToLowerInvariant(); + } + + /// + /// The pre-identity lookup: one row per FCM token within this app. Ordered + /// by Id so the outcome is deterministic if two rows ever share a token; + /// the oldest wins. + /// + private Task FindRowWithSameTokenAsync(string appId, string token) => + _dbContext.DeviceTokens + .Where(dt => dt.AppId == appId && dt.FcmToken == token) + .OrderBy(dt => dt.Id) + .FirstOrDefaultAsync(); + /// /// Claims a pre-existing row that already carries this FCM token under a /// different InstallationId, rewriting its InstallationId to the real one. + /// Callers must have rejected a blank token first (RegisterAsync does); + /// this does not re-check. /// /// This exists for the DeviceTokenIdentityModel migration in /// eform-timeplanning-base, which backfills every pre-existing row with @@ -164,22 +285,16 @@ public async Task RegisterAsync( /// pre-existing user gets doubled pushes indefinitely. FCM never clears it /// either, because the legacy row's token is perfectly valid. /// - /// Ordered by Id so the outcome is deterministic if two rows ever share a - /// token; the oldest wins. Pre-migration that could not happen at all - - /// this table's old unique index was on Token alone. + /// It also claims a row a LEGACY register created under a synthetic + /// installation id: when the fleet upgrades and starts sending real ids, + /// that row must be rewritten rather than joined by a second live row + /// carrying the same token and site - the sender would select both and the + /// user would get every push twice, forever. /// private async Task AdoptRowWithSameTokenAsync( string appId, string token, string installationId) { - if (string.IsNullOrWhiteSpace(token)) - { - return null; - } - - var adopted = await _dbContext.DeviceTokens - .Where(dt => dt.AppId == appId && dt.FcmToken == token) - .OrderBy(dt => dt.Id) - .FirstOrDefaultAsync(); + var adopted = await FindRowWithSameTokenAsync(appId, token); if (adopted == null) { diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/IDeviceTokenService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/IDeviceTokenService.cs index 28a11f59..02829084 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/IDeviceTokenService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/IDeviceTokenService.cs @@ -12,7 +12,7 @@ public interface IDeviceTokenService /// is the client's app build number /// (0 = old/unknown), stored for push version-gating. /// and are the - /// stored row's identity; see . + /// stored row's identity; either may be empty, see . /// Task RegisterForCallerAsync( string token, string platform, int buildNumber = 0, @@ -23,8 +23,14 @@ Task RegisterForCallerAsync( /// (, ) - the app /// install, not the FCM token. A rotated token updates that install's row /// in place, and a different user on the same install reassigns - /// . Both arguments are required; an empty one - /// is rejected without storing anything. + /// . + /// + /// Both carry a backward-compatible reading for clients shipped before the + /// identity model, which send neither (proto3 decodes an absent string as + /// ""): an empty means "time", the only app this + /// service serves, and an empty falls + /// back to matching on the FCM token, the pre-change identity. Only + /// is hard-required. /// Task RegisterAsync( int sdkSiteId, string token, string platform, int buildNumber = 0, diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/GrpcServices/TimePlanningDeviceTokenGrpcService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/GrpcServices/TimePlanningDeviceTokenGrpcService.cs index 1f6f3616..cbe70878 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/GrpcServices/TimePlanningDeviceTokenGrpcService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/GrpcServices/TimePlanningDeviceTokenGrpcService.cs @@ -25,9 +25,11 @@ public override async Task RegisterDeviceToken( // DeviceTokenService); the client no longer sends one. The client's // reported app build number is persisted for push version-gating, // and (AppId, InstallationId) is the identity of the stored row. - // An empty app_id or installation_id comes back as an unsuccessful - // OperationResponse - this transport reports failures in the - // response body rather than as a gRPC status. + // Clients shipped before that identity model send neither field and + // proto3 decodes both as "", so the service reads them + // backward-compatibly rather than rejecting; only an empty token + // comes back as an unsuccessful OperationResponse - this transport + // reports failures in the response body, not as a gRPC status. var result = await _deviceTokenService.RegisterForCallerAsync( request.Token, request.Platform, request.BuildNumber, request.AppId, request.InstallationId);