fix: resolve device-token SdkSiteId server-side from the JWT - #1668
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Tokens were stored under the client-sent SdkSiteId, which the flutter-time app never sends (proto3 default 0), so SendToSiteAsync never matched any token. The gRPC layer now ignores request.SdkSiteId and resolves the caller's active site per the established ResolveCallerSdkSiteIdAsync convention; unresolved callers are rejected without storing a dead token. Also updates DeviceTokenServiceTests' constructor call for the new resolver dependencies (IUserService/BaseDbContext/IEFormCoreService), required for the build to compile after the ctor change; behavior of the existing RegisterAsync/UnregisterAsync tests is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes push device-token registration so the server resolves SdkSiteId from the authenticated caller’s JWT (instead of trusting the client-provided sdk_site_id), preventing “dead” tokens that never match during push lookups.
Changes:
- Updated the gRPC registration endpoint to ignore the wire
sdk_site_idand register tokens for the authenticated caller. - Added
RegisterForCallerAsyncto the device-token service contract and implemented server-side caller site resolution. - Added/updated tests to cover rejection when no authenticated user is present and to verify token-keyed upsert repairs legacy
SdkSiteId=0rows.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/GrpcServices/TimePlanningDeviceTokenGrpcService.cs | Switches gRPC registration to server-resolved site id via RegisterForCallerAsync. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/IDeviceTokenService.cs | Introduces RegisterForCallerAsync API with documentation for server-side resolution behavior. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/DeviceTokenService.cs | Implements caller site resolution from JWT and routes registration through existing upsert logic. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Protos/device_token.proto | Documents that sdk_site_id is ignored but kept for wire compatibility. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DeviceTokenServiceTests.cs | Adds test coverage for null-auth rejection and legacy SdkSiteId repair behavior. |
| .gitignore | Ignores additional local dev folders (.worktrees/, .superpowers/). |
Suppressed comments (1)
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/DeviceTokenService/DeviceTokenService.cs:69
- The SDK DbContext returned from
DbContextHelper.GetDbContext()is used as an EF Core context elsewhere withawait using; leaving it undisposed here can leak connections/resources over time. Dispose it withawait using.
var sdkCore = await _coreService.GetCore();
var sdkDbContext = sdkCore.DbContextHelper.GetDbContext();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
+56
| /// <summary> | ||
| /// Resolves the authenticated caller's SDK site id (MicrotingUid) from | ||
| /// the JWT. Returns 0 if the user has no worker/site record. Mirrors | ||
| /// AbsenceRequestService.ResolveCallerSdkSiteIdAsync(). | ||
| /// </summary> |
Comment on lines
+64
to
+66
| var currentUser = _baseDbContext.Users | ||
| .Single(x => x.Id == currentUserAsync.Id); | ||
|
|
Comment on lines
+85
to
+94
| [Test] | ||
| public async Task RegisterForCallerAsync_NoAuthenticatedUser_RejectsWithoutStoring() | ||
| { | ||
| _userService.GetCurrentUserAsync().Returns(Task.FromResult<EformUser?>(null)); | ||
|
|
||
| var result = await _service.RegisterForCallerAsync("dead-token", "android"); | ||
|
|
||
| Assert.That(result.Success, Is.False); | ||
| Assert.That(await TimePlanningPnDbContext!.DeviceTokens.CountAsync(), Is.EqualTo(0)); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tokens were registered under the client-sent sdk_site_id, which the flutter-time app never sends, so every push lookup (SendToSiteAsync) matched nothing. The gRPC layer now ignores the wire field and resolves the caller's active site server-side (ResolveCallerSdkSiteIdAsync convention); unresolved callers are rejected without storing dead tokens. Existing SdkSiteId=0 rows self-repair on next register via the token-keyed upsert. Part of the Firebase push tenant-deployment effort (flutter-adhoc docs/superpowers/specs/2026-07-31-firebase-push-tenant-deployment-design.md).
🤖 Generated with Claude Code