From 23217dd765082d3351e7f34ae299632241f598ea Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 14:16:54 -0400 Subject: [PATCH 1/6] feat(protocol): a tapped notification's route, peeked and acked appState gains PushTapRoute plus peekPushTapRoute/ackPushTapRoute, and NotifyApp gains pushTapRouteAvailable. Peek does not retire the route: a reply lost on the way to the client would take the tap with it, and only the client knows whether it acted, so the ack is what clears it. The notification carries nothing -- the route rides the peek's reply -- so a tap has exactly one reader whether it happened before a client existed or while one was connected. --- go/protocol/keybase1/appstate.go | 59 ++++++++++++++++++++++++++ go/protocol/keybase1/notify_app.go | 19 +++++++++ protocol/avdl/keybase1/appstate.avdl | 29 +++++++++++++ protocol/avdl/keybase1/notify_app.avdl | 8 ++++ protocol/bin/enabled-calls.json | 3 ++ protocol/json/keybase1/appstate.json | 34 +++++++++++++++ protocol/json/keybase1/notify_app.json | 5 +++ shared/constants/rpc/index.tsx | 1 + shared/constants/rpc/rpc-gen.tsx | 19 ++++++++- 9 files changed, 175 insertions(+), 2 deletions(-) diff --git a/go/protocol/keybase1/appstate.go b/go/protocol/keybase1/appstate.go index 9850958b713e..ebf300c5986f 100644 --- a/go/protocol/keybase1/appstate.go +++ b/go/protocol/keybase1/appstate.go @@ -78,16 +78,39 @@ func (o MobileNetworkState) String() string { return fmt.Sprintf("%v", int(o)) } +type PushTapRoute struct { + Url string `codec:"url" json:"url"` + TargetUID string `codec:"targetUID" json:"targetUID"` + Id int `codec:"id" json:"id"` +} + +func (o PushTapRoute) DeepCopy() PushTapRoute { + return PushTapRoute{ + Url: o.Url, + TargetUID: o.TargetUID, + Id: o.Id, + } +} + type UpdateMobileNetStateArg struct { State string `codec:"state" json:"state"` } +type PeekPushTapRouteArg struct { +} + +type AckPushTapRouteArg struct { + Id int `codec:"id" json:"id"` +} + type PowerMonitorEventArg struct { Event string `codec:"event" json:"event"` } type AppStateInterface interface { UpdateMobileNetState(context.Context, string) error + PeekPushTapRoute(context.Context) (*PushTapRoute, error) + AckPushTapRoute(context.Context, int) error PowerMonitorEvent(context.Context, string) error } @@ -110,6 +133,31 @@ func AppStateProtocol(i AppStateInterface) rpc.Protocol { return }, }, + "peekPushTapRoute": { + MakeArg: func() any { + var ret [1]PeekPushTapRouteArg + return &ret + }, + Handler: func(ctx context.Context, args any) (ret any, err error) { + ret, err = i.PeekPushTapRoute(ctx) + return + }, + }, + "ackPushTapRoute": { + MakeArg: func() any { + var ret [1]AckPushTapRouteArg + return &ret + }, + Handler: func(ctx context.Context, args any) (ret any, err error) { + typedArgs, ok := args.(*[1]AckPushTapRouteArg) + if !ok { + err = rpc.NewTypeError((*[1]AckPushTapRouteArg)(nil), args) + return + } + err = i.AckPushTapRoute(ctx, typedArgs[0].Id) + return + }, + }, "powerMonitorEvent": { MakeArg: func() any { var ret [1]PowerMonitorEventArg @@ -139,6 +187,17 @@ func (c AppStateClient) UpdateMobileNetState(ctx context.Context, state string) return } +func (c AppStateClient) PeekPushTapRoute(ctx context.Context) (res *PushTapRoute, err error) { + err = c.Cli.Call(ctx, "keybase.1.appState.peekPushTapRoute", []any{PeekPushTapRouteArg{}}, &res, 0*time.Millisecond) + return +} + +func (c AppStateClient) AckPushTapRoute(ctx context.Context, id int) (err error) { + __arg := AckPushTapRouteArg{Id: id} + err = c.Cli.Call(ctx, "keybase.1.appState.ackPushTapRoute", []any{__arg}, nil, 0*time.Millisecond) + return +} + func (c AppStateClient) PowerMonitorEvent(ctx context.Context, event string) (err error) { __arg := PowerMonitorEventArg{Event: event} err = c.Cli.Call(ctx, "keybase.1.appState.powerMonitorEvent", []any{__arg}, nil, 0*time.Millisecond) diff --git a/go/protocol/keybase1/notify_app.go b/go/protocol/keybase1/notify_app.go index e2fb1a2b3742..5db46395b403 100644 --- a/go/protocol/keybase1/notify_app.go +++ b/go/protocol/keybase1/notify_app.go @@ -21,10 +21,14 @@ type ClientStateArg struct { State ClientState `codec:"state" json:"state"` } +type PushTapRouteAvailableArg struct { +} + type NotifyAppInterface interface { Exit(context.Context) error MobileAppStateChanged(context.Context, MobileAppState) error ClientState(context.Context, ClientState) error + PushTapRouteAvailable(context.Context) error } func NotifyAppProtocol(i NotifyAppInterface) rpc.Protocol { @@ -71,6 +75,16 @@ func NotifyAppProtocol(i NotifyAppInterface) rpc.Protocol { return }, }, + "pushTapRouteAvailable": { + MakeArg: func() any { + var ret [1]PushTapRouteAvailableArg + return &ret + }, + Handler: func(ctx context.Context, args any) (ret any, err error) { + err = i.PushTapRouteAvailable(ctx) + return + }, + }, }, } } @@ -95,3 +109,8 @@ func (c NotifyAppClient) ClientState(ctx context.Context, state ClientState) (er err = c.Cli.Notify(ctx, "keybase.1.NotifyApp.clientState", []any{__arg}, 0*time.Millisecond) return } + +func (c NotifyAppClient) PushTapRouteAvailable(ctx context.Context) (err error) { + err = c.Cli.Notify(ctx, "keybase.1.NotifyApp.pushTapRouteAvailable", []any{PushTapRouteAvailableArg{}}, 0*time.Millisecond) + return +} diff --git a/protocol/avdl/keybase1/appstate.avdl b/protocol/avdl/keybase1/appstate.avdl index 8cc21333147d..d6062abad1af 100644 --- a/protocol/avdl/keybase1/appstate.avdl +++ b/protocol/avdl/keybase1/appstate.avdl @@ -17,10 +17,39 @@ protocol appState { NOTAVAILABLE_4 } + // Where a tapped notification opens. The service resolves this from the push + // payload native hands it, so no client parses a push payload. + record PushTapRoute { + // A keybase:// URL. + string url; + // The account the notification belongs to, or empty. Only a route resolved + // from a real notification tap can name one, which is what keeps a link + // opened by another app from switching accounts. + string targetUID; + // Identifies this tap, so an ack cannot clear a newer one. Rises with each + // tap and is meaningless across a restart of the service. + int id; + } + // gui -> service // mobile only void updateMobileNetState(string state); + // gui -> service + // mobile only + // Returns the route a tapped notification resolved to, or null when no tap is + // waiting. Reading does NOT clear it: a reply lost on the way to the client + // would take the tap with it, and the client is what knows whether it acted. + // The route stays armed until ackPushTapRoute. + union { null, PushTapRoute } peekPushTapRoute(); + + // gui -> service + // mobile only + // Says the client has acted on the route with this id, which clears it. A + // stale id -- the tap was replaced by a newer one while this was in flight -- + // clears nothing. + void ackPushTapRoute(int id); + // gui -> service // desktop only // https://electronjs.org/docs/api/power-monitor diff --git a/protocol/avdl/keybase1/notify_app.avdl b/protocol/avdl/keybase1/notify_app.avdl index 0c6ebb0c78e5..04668d8fbd63 100644 --- a/protocol/avdl/keybase1/notify_app.avdl +++ b/protocol/avdl/keybase1/notify_app.avdl @@ -20,4 +20,12 @@ protocol NotifyApp { // received is never older than any of them. void clientState(ClientState state) oneway; + // A notification tap resolved to a route and it is waiting to be acted on. A + // nudge, not a delivery: the route rides peekPushTapRoute's reply, so the + // reader is the same one whether the tap happened before this client existed + // or while it was connected. Carries nothing for that reason -- acting on + // this rather than on what the peek reports would be a second delivery path, + // and the two could then act on one tap twice. + void pushTapRouteAvailable() oneway; + } diff --git a/protocol/bin/enabled-calls.json b/protocol/bin/enabled-calls.json index 9c97f458cee7..bf59f5b62c40 100644 --- a/protocol/bin/enabled-calls.json +++ b/protocol/bin/enabled-calls.json @@ -155,6 +155,7 @@ "keybase.1.NotifyApp.clientState": {"incoming":true}, "keybase.1.NotifyApp.exit": {"custom":true}, "keybase.1.NotifyApp.mobileAppStateChanged": {"incoming":true}, + "keybase.1.NotifyApp.pushTapRouteAvailable": {"incoming":true}, "keybase.1.NotifyAudit.boxAuditError": {"incoming":true}, "keybase.1.NotifyAudit.rootAuditError": {"incoming":true}, "keybase.1.NotifyBadges.badgeState": {"incoming":true}, @@ -254,6 +255,8 @@ "keybase.1.apiserver.Post": {"promise":true}, "keybase.1.apiserver.PostJSON": {"promise":true}, "keybase.1.appState.powerMonitorEvent": {"promise":true}, + "keybase.1.appState.ackPushTapRoute": {"promise":true}, + "keybase.1.appState.peekPushTapRoute": {"promise":true}, "keybase.1.appState.updateMobileNetState": {"promise":true}, "keybase.1.config.appendGUILogs": {"promise":true}, "keybase.1.config.generateWebAuthToken": {"promise":true}, diff --git a/protocol/json/keybase1/appstate.json b/protocol/json/keybase1/appstate.json index 4d096179e475..94a212f08962 100644 --- a/protocol/json/keybase1/appstate.json +++ b/protocol/json/keybase1/appstate.json @@ -22,6 +22,24 @@ "UNKNOWN_3", "NOTAVAILABLE_4" ] + }, + { + "type": "record", + "name": "PushTapRoute", + "fields": [ + { + "type": "string", + "name": "url" + }, + { + "type": "string", + "name": "targetUID" + }, + { + "type": "int", + "name": "id" + } + ] } ], "messages": { @@ -34,6 +52,22 @@ ], "response": null }, + "peekPushTapRoute": { + "request": [], + "response": [ + null, + "PushTapRoute" + ] + }, + "ackPushTapRoute": { + "request": [ + { + "name": "id", + "type": "int" + } + ], + "response": null + }, "powerMonitorEvent": { "request": [ { diff --git a/protocol/json/keybase1/notify_app.json b/protocol/json/keybase1/notify_app.json index ec56c94121ff..40184b5e862c 100644 --- a/protocol/json/keybase1/notify_app.json +++ b/protocol/json/keybase1/notify_app.json @@ -40,6 +40,11 @@ ], "response": null, "oneway": true + }, + "pushTapRouteAvailable": { + "request": [], + "response": null, + "oneway": true } }, "namespace": "keybase.1" diff --git a/shared/constants/rpc/index.tsx b/shared/constants/rpc/index.tsx index 03b90b4c3aaa..c9671c4222ea 100644 --- a/shared/constants/rpc/index.tsx +++ b/shared/constants/rpc/index.tsx @@ -73,6 +73,7 @@ type Chat1ResponseActionMap = { type Keybase1IncomingAction = 'keybase.1.NotifyApp.clientState' | 'keybase.1.NotifyApp.mobileAppStateChanged' | + 'keybase.1.NotifyApp.pushTapRouteAvailable' | 'keybase.1.NotifyAudit.boxAuditError' | 'keybase.1.NotifyAudit.rootAuditError' | 'keybase.1.NotifyBadges.badgeState' | diff --git a/shared/constants/rpc/rpc-gen.tsx b/shared/constants/rpc/rpc-gen.tsx index 5e5add3dc6eb..d3297ee69744 100644 --- a/shared/constants/rpc/rpc-gen.tsx +++ b/shared/constants/rpc/rpc-gen.tsx @@ -23,6 +23,10 @@ export type MessageTypes = { inParam: {readonly state: MobileAppState}, outParam: void, }, + 'keybase.1.NotifyApp.pushTapRouteAvailable': { + inParam: undefined, + outParam: void, + }, 'keybase.1.NotifyAudit.boxAuditError': { inParam: {readonly message: string}, outParam: void, @@ -415,6 +419,14 @@ export type MessageTypes = { inParam: {readonly endpoint: string,readonly args?: ReadonlyArray | null,readonly JSONPayload?: ReadonlyArray | null,readonly httpStatus?: ReadonlyArray | null,readonly appStatusCode?: ReadonlyArray | null}, outParam: APIRes, }, + 'keybase.1.appState.ackPushTapRoute': { + inParam: {readonly id: number}, + outParam: void, + }, + 'keybase.1.appState.peekPushTapRoute': { + inParam: undefined, + outParam: PushTapRoute | null, + }, 'keybase.1.appState.powerMonitorEvent': { inParam: {readonly event: string}, outParam: void, @@ -1280,7 +1292,7 @@ export type MessageKey = keyof MessageTypes export type RpcIn = MessageTypes[M]['inParam'] export type RpcOut = MessageTypes[M]['outParam'] export type RpcResponse = {error: IncomingErrorCallback, result: (res: RpcOut) => void} -type PromiseMethod = 'keybase.1.account.cancelReset' | 'keybase.1.account.getLockdownMode' | 'keybase.1.account.guessCurrentLocation' | 'keybase.1.account.hasServerKeys' | 'keybase.1.account.passphraseChange' | 'keybase.1.account.passphraseCheck' | 'keybase.1.account.recoverUsernameWithEmail' | 'keybase.1.account.recoverUsernameWithPhone' | 'keybase.1.account.setLockdownMode' | 'keybase.1.account.userGetContactSettings' | 'keybase.1.account.userSetContactSettings' | 'keybase.1.apiserver.Delete' | 'keybase.1.apiserver.GetWithSession' | 'keybase.1.apiserver.Post' | 'keybase.1.apiserver.PostJSON' | 'keybase.1.appState.powerMonitorEvent' | 'keybase.1.appState.updateMobileNetState' | 'keybase.1.config.appendGUILogs' | 'keybase.1.config.generateWebAuthToken' | 'keybase.1.config.getBootstrapStatus' | 'keybase.1.config.getProxyData' | 'keybase.1.config.getRememberPassphrase' | 'keybase.1.config.getUpdateInfo' | 'keybase.1.config.getUpdateInfo2' | 'keybase.1.config.guiGetValue' | 'keybase.1.config.guiSetValue' | 'keybase.1.config.helloIAm' | 'keybase.1.config.logSend' | 'keybase.1.config.requestFollowingAndUnverifiedFollowers' | 'keybase.1.config.setProxyData' | 'keybase.1.config.setRememberPassphrase' | 'keybase.1.config.startUpdateIfNeeded' | 'keybase.1.config.toggleRuntimeStats' | 'keybase.1.config.updateLastLoggedInAndServerConfig' | 'keybase.1.config.waitForClient' | 'keybase.1.contacts.getContactsForUserRecommendations' | 'keybase.1.contacts.saveContactList' | 'keybase.1.cryptocurrency.registerAddress' | 'keybase.1.ctl.dbNuke' | 'keybase.1.ctl.getOnLoginStartup' | 'keybase.1.ctl.setOnLoginStartup' | 'keybase.1.ctl.stop' | 'keybase.1.delegateUiCtl.registerChatUI' | 'keybase.1.delegateUiCtl.registerGregorFirehoseFiltered' | 'keybase.1.delegateUiCtl.registerHomeUI' | 'keybase.1.delegateUiCtl.registerIdentify3UI' | 'keybase.1.delegateUiCtl.registerLogUI' | 'keybase.1.delegateUiCtl.registerRekeyUI' | 'keybase.1.delegateUiCtl.registerSecretUI' | 'keybase.1.device.checkDeviceNameFormat' | 'keybase.1.device.deviceHistoryList' | 'keybase.1.device.dismissDeviceChangeNotifications' | 'keybase.1.emails.addEmail' | 'keybase.1.emails.deleteEmail' | 'keybase.1.emails.sendVerificationEmail' | 'keybase.1.emails.setPrimaryEmail' | 'keybase.1.emails.setVisibilityEmail' | 'keybase.1.favorite.favoriteIgnore' | 'keybase.1.featuredBot.featuredBots' | 'keybase.1.featuredBot.search' | 'keybase.1.git.createPersonalRepo' | 'keybase.1.git.createTeamRepo' | 'keybase.1.git.deletePersonalRepo' | 'keybase.1.git.deleteTeamRepo' | 'keybase.1.git.getAllGitMetadata' | 'keybase.1.git.getTeamRepoSettings' | 'keybase.1.git.setTeamRepoSettings' | 'keybase.1.gregor.dismissCategory' | 'keybase.1.gregor.getState' | 'keybase.1.gregor.updateCategory' | 'keybase.1.home.homeDismissAnnouncement' | 'keybase.1.home.homeGetScreen' | 'keybase.1.home.homeMarkViewed' | 'keybase.1.home.homeSkipTodoType' | 'keybase.1.identify3.identify3FollowUser' | 'keybase.1.identify3.identify3IgnoreUser' | 'keybase.1.incomingShare.getIncomingShareItems' | 'keybase.1.incomingShare.getPreference' | 'keybase.1.incomingShare.setPreference' | 'keybase.1.install.fuseStatus' | 'keybase.1.install.installFuse' | 'keybase.1.install.installKBFS' | 'keybase.1.install.uninstallKBFS' | 'keybase.1.kbfsMount.GetCurrentMountDir' | 'keybase.1.kbfsMount.GetKBFSPathInfo' | 'keybase.1.kbfsMount.GetPreferredMountDirs' | 'keybase.1.kbfsMount.WaitForMounts' | 'keybase.1.log.perfLogPoint' | 'keybase.1.login.accountDelete' | 'keybase.1.login.deprovision' | 'keybase.1.login.getConfiguredAccounts' | 'keybase.1.login.isOnline' | 'keybase.1.login.logout' | 'keybase.1.login.paperKeySubmit' | 'keybase.1.notifyCtl.setNotifications' | 'keybase.1.pgp.pgpStorageDismiss' | 'keybase.1.phoneNumbers.addPhoneNumber' | 'keybase.1.phoneNumbers.deletePhoneNumber' | 'keybase.1.phoneNumbers.resendVerificationForPhoneNumber' | 'keybase.1.phoneNumbers.setVisibilityPhoneNumber' | 'keybase.1.phoneNumbers.verifyPhoneNumber' | 'keybase.1.pprof.logProcessorProfile' | 'keybase.1.pprof.logTrace' | 'keybase.1.prove.checkProof' | 'keybase.1.reachability.checkReachability' | 'keybase.1.rekey.getRevokeWarning' | 'keybase.1.rekey.rekeyStatusFinish' | 'keybase.1.rekey.showPendingRekeyStatus' | 'keybase.1.revoke.revokeDevice' | 'keybase.1.revoke.revokeKey' | 'keybase.1.revoke.revokeSigs' | 'keybase.1.saltpack.saltpackDecryptFile' | 'keybase.1.saltpack.saltpackDecryptString' | 'keybase.1.saltpack.saltpackEncryptFile' | 'keybase.1.saltpack.saltpackEncryptString' | 'keybase.1.saltpack.saltpackSaveCiphertextToFile' | 'keybase.1.saltpack.saltpackSaveSignedMsgToFile' | 'keybase.1.saltpack.saltpackSignFile' | 'keybase.1.saltpack.saltpackSignString' | 'keybase.1.saltpack.saltpackVerifyFile' | 'keybase.1.saltpack.saltpackVerifyString' | 'keybase.1.signup.checkUsernameAvailable' | 'keybase.1.signup.getInvitationCode' | 'keybase.1.SimpleFS.simpleFSArchiveAllFiles' | 'keybase.1.SimpleFS.simpleFSArchiveAllGitRepos' | 'keybase.1.SimpleFS.simpleFSArchiveCancelOrDismissJob' | 'keybase.1.SimpleFS.simpleFSArchiveStart' | 'keybase.1.SimpleFS.simpleFSCancelDownload' | 'keybase.1.SimpleFS.simpleFSCheckReachability' | 'keybase.1.SimpleFS.simpleFSClearConflictState' | 'keybase.1.SimpleFS.simpleFSConfigureDownload' | 'keybase.1.SimpleFS.simpleFSCopyRecursive' | 'keybase.1.SimpleFS.simpleFSDismissDownload' | 'keybase.1.SimpleFS.simpleFSDismissUpload' | 'keybase.1.SimpleFS.simpleFSFinishResolvingConflict' | 'keybase.1.SimpleFS.simpleFSFolderSyncConfigAndStatus' | 'keybase.1.SimpleFS.simpleFSGetArchiveJobFreshness' | 'keybase.1.SimpleFS.simpleFSGetArchiveStatus' | 'keybase.1.SimpleFS.simpleFSGetDownloadInfo' | 'keybase.1.SimpleFS.simpleFSGetDownloadStatus' | 'keybase.1.SimpleFS.simpleFSGetFilesTabBadge' | 'keybase.1.SimpleFS.simpleFSGetFolder' | 'keybase.1.SimpleFS.simpleFSGetGUIFileContext' | 'keybase.1.SimpleFS.simpleFSGetOnlineStatus' | 'keybase.1.SimpleFS.simpleFSGetUploadStatus' | 'keybase.1.SimpleFS.simpleFSList' | 'keybase.1.SimpleFS.simpleFSListFavorites' | 'keybase.1.SimpleFS.simpleFSListRecursiveToDepth' | 'keybase.1.SimpleFS.simpleFSMakeTempDirForUpload' | 'keybase.1.SimpleFS.simpleFSMove' | 'keybase.1.SimpleFS.simpleFSOpen' | 'keybase.1.SimpleFS.simpleFSReadList' | 'keybase.1.SimpleFS.simpleFSRemove' | 'keybase.1.SimpleFS.simpleFSSetDebugLevel' | 'keybase.1.SimpleFS.simpleFSSetFolderSyncConfig' | 'keybase.1.SimpleFS.simpleFSSetNotificationThreshold' | 'keybase.1.SimpleFS.simpleFSSetSfmiBannerDismissed' | 'keybase.1.SimpleFS.simpleFSSetSyncOnCellular' | 'keybase.1.SimpleFS.simpleFSSettings' | 'keybase.1.SimpleFS.simpleFSStartDownload' | 'keybase.1.SimpleFS.simpleFSStartUpload' | 'keybase.1.SimpleFS.simpleFSStat' | 'keybase.1.SimpleFS.simpleFSSubscribeNonPath' | 'keybase.1.SimpleFS.simpleFSSubscribePath' | 'keybase.1.SimpleFS.simpleFSSyncStatus' | 'keybase.1.SimpleFS.simpleFSUnsubscribe' | 'keybase.1.SimpleFS.simpleFSUserEditHistory' | 'keybase.1.SimpleFS.simpleFSUserIn' | 'keybase.1.SimpleFS.simpleFSUserOut' | 'keybase.1.SimpleFS.simpleFSWait' | 'keybase.1.teams.findAssertionsInTeamNoResolve' | 'keybase.1.teams.getAnnotatedTeam' | 'keybase.1.teams.getInviteLinkDetails' | 'keybase.1.teams.getTeamID' | 'keybase.1.teams.getTeamRoleMap' | 'keybase.1.teams.getUntrustedTeamInfo' | 'keybase.1.teams.loadTeamTreeMembershipsAsync' | 'keybase.1.teams.setTarsDisabled' | 'keybase.1.teams.setTeamMemberShowcase' | 'keybase.1.teams.setTeamShowcase' | 'keybase.1.teams.teamAddEmailsBulk' | 'keybase.1.teams.teamAddMember' | 'keybase.1.teams.teamAddMembersMultiRole' | 'keybase.1.teams.teamCreate' | 'keybase.1.teams.teamCreateFancy' | 'keybase.1.teams.teamCreateSeitanTokenV2' | 'keybase.1.teams.teamEditMembers' | 'keybase.1.teams.teamGetMembersByID' | 'keybase.1.teams.teamIgnoreRequest' | 'keybase.1.teams.teamLeave' | 'keybase.1.teams.teamListMyAccessRequests' | 'keybase.1.teams.teamListUnverified' | 'keybase.1.teams.teamProfileAddList' | 'keybase.1.teams.teamReAddMemberAfterReset' | 'keybase.1.teams.teamRemoveMember' | 'keybase.1.teams.teamRename' | 'keybase.1.teams.teamSetSettings' | 'keybase.1.teams.untrustedTeamExists' | 'keybase.1.teams.uploadTeamAvatar' | 'keybase.1.user.blockUser' | 'keybase.1.user.canLogout' | 'keybase.1.user.dismissBlockButtons' | 'keybase.1.user.getUserBlocks' | 'keybase.1.user.interestingPeople' | 'keybase.1.user.listTrackersUnverified' | 'keybase.1.user.listTracking' | 'keybase.1.user.loadMySettings' | 'keybase.1.user.loadPassphraseState' | 'keybase.1.user.profileEdit' | 'keybase.1.user.proofSuggestions' | 'keybase.1.user.reportUser' | 'keybase.1.user.setUserBlocks' | 'keybase.1.user.unblockUser' | 'keybase.1.user.uploadUserAvatar' | 'keybase.1.user.userCard' | 'keybase.1.userSearch.bulkEmailOrPhoneSearch' | 'keybase.1.userSearch.getNonUserDetails' | 'keybase.1.userSearch.userSearch' +type PromiseMethod = 'keybase.1.account.cancelReset' | 'keybase.1.account.getLockdownMode' | 'keybase.1.account.guessCurrentLocation' | 'keybase.1.account.hasServerKeys' | 'keybase.1.account.passphraseChange' | 'keybase.1.account.passphraseCheck' | 'keybase.1.account.recoverUsernameWithEmail' | 'keybase.1.account.recoverUsernameWithPhone' | 'keybase.1.account.setLockdownMode' | 'keybase.1.account.userGetContactSettings' | 'keybase.1.account.userSetContactSettings' | 'keybase.1.apiserver.Delete' | 'keybase.1.apiserver.GetWithSession' | 'keybase.1.apiserver.Post' | 'keybase.1.apiserver.PostJSON' | 'keybase.1.appState.ackPushTapRoute' | 'keybase.1.appState.peekPushTapRoute' | 'keybase.1.appState.powerMonitorEvent' | 'keybase.1.appState.updateMobileNetState' | 'keybase.1.config.appendGUILogs' | 'keybase.1.config.generateWebAuthToken' | 'keybase.1.config.getBootstrapStatus' | 'keybase.1.config.getProxyData' | 'keybase.1.config.getRememberPassphrase' | 'keybase.1.config.getUpdateInfo' | 'keybase.1.config.getUpdateInfo2' | 'keybase.1.config.guiGetValue' | 'keybase.1.config.guiSetValue' | 'keybase.1.config.helloIAm' | 'keybase.1.config.logSend' | 'keybase.1.config.requestFollowingAndUnverifiedFollowers' | 'keybase.1.config.setProxyData' | 'keybase.1.config.setRememberPassphrase' | 'keybase.1.config.startUpdateIfNeeded' | 'keybase.1.config.toggleRuntimeStats' | 'keybase.1.config.updateLastLoggedInAndServerConfig' | 'keybase.1.config.waitForClient' | 'keybase.1.contacts.getContactsForUserRecommendations' | 'keybase.1.contacts.saveContactList' | 'keybase.1.cryptocurrency.registerAddress' | 'keybase.1.ctl.dbNuke' | 'keybase.1.ctl.getOnLoginStartup' | 'keybase.1.ctl.setOnLoginStartup' | 'keybase.1.ctl.stop' | 'keybase.1.delegateUiCtl.registerChatUI' | 'keybase.1.delegateUiCtl.registerGregorFirehoseFiltered' | 'keybase.1.delegateUiCtl.registerHomeUI' | 'keybase.1.delegateUiCtl.registerIdentify3UI' | 'keybase.1.delegateUiCtl.registerLogUI' | 'keybase.1.delegateUiCtl.registerRekeyUI' | 'keybase.1.delegateUiCtl.registerSecretUI' | 'keybase.1.device.checkDeviceNameFormat' | 'keybase.1.device.deviceHistoryList' | 'keybase.1.device.dismissDeviceChangeNotifications' | 'keybase.1.emails.addEmail' | 'keybase.1.emails.deleteEmail' | 'keybase.1.emails.sendVerificationEmail' | 'keybase.1.emails.setPrimaryEmail' | 'keybase.1.emails.setVisibilityEmail' | 'keybase.1.favorite.favoriteIgnore' | 'keybase.1.featuredBot.featuredBots' | 'keybase.1.featuredBot.search' | 'keybase.1.git.createPersonalRepo' | 'keybase.1.git.createTeamRepo' | 'keybase.1.git.deletePersonalRepo' | 'keybase.1.git.deleteTeamRepo' | 'keybase.1.git.getAllGitMetadata' | 'keybase.1.git.getTeamRepoSettings' | 'keybase.1.git.setTeamRepoSettings' | 'keybase.1.gregor.dismissCategory' | 'keybase.1.gregor.getState' | 'keybase.1.gregor.updateCategory' | 'keybase.1.home.homeDismissAnnouncement' | 'keybase.1.home.homeGetScreen' | 'keybase.1.home.homeMarkViewed' | 'keybase.1.home.homeSkipTodoType' | 'keybase.1.identify3.identify3FollowUser' | 'keybase.1.identify3.identify3IgnoreUser' | 'keybase.1.incomingShare.getIncomingShareItems' | 'keybase.1.incomingShare.getPreference' | 'keybase.1.incomingShare.setPreference' | 'keybase.1.install.fuseStatus' | 'keybase.1.install.installFuse' | 'keybase.1.install.installKBFS' | 'keybase.1.install.uninstallKBFS' | 'keybase.1.kbfsMount.GetCurrentMountDir' | 'keybase.1.kbfsMount.GetKBFSPathInfo' | 'keybase.1.kbfsMount.GetPreferredMountDirs' | 'keybase.1.kbfsMount.WaitForMounts' | 'keybase.1.log.perfLogPoint' | 'keybase.1.login.accountDelete' | 'keybase.1.login.deprovision' | 'keybase.1.login.getConfiguredAccounts' | 'keybase.1.login.isOnline' | 'keybase.1.login.logout' | 'keybase.1.login.paperKeySubmit' | 'keybase.1.notifyCtl.setNotifications' | 'keybase.1.pgp.pgpStorageDismiss' | 'keybase.1.phoneNumbers.addPhoneNumber' | 'keybase.1.phoneNumbers.deletePhoneNumber' | 'keybase.1.phoneNumbers.resendVerificationForPhoneNumber' | 'keybase.1.phoneNumbers.setVisibilityPhoneNumber' | 'keybase.1.phoneNumbers.verifyPhoneNumber' | 'keybase.1.pprof.logProcessorProfile' | 'keybase.1.pprof.logTrace' | 'keybase.1.prove.checkProof' | 'keybase.1.reachability.checkReachability' | 'keybase.1.rekey.getRevokeWarning' | 'keybase.1.rekey.rekeyStatusFinish' | 'keybase.1.rekey.showPendingRekeyStatus' | 'keybase.1.revoke.revokeDevice' | 'keybase.1.revoke.revokeKey' | 'keybase.1.revoke.revokeSigs' | 'keybase.1.saltpack.saltpackDecryptFile' | 'keybase.1.saltpack.saltpackDecryptString' | 'keybase.1.saltpack.saltpackEncryptFile' | 'keybase.1.saltpack.saltpackEncryptString' | 'keybase.1.saltpack.saltpackSaveCiphertextToFile' | 'keybase.1.saltpack.saltpackSaveSignedMsgToFile' | 'keybase.1.saltpack.saltpackSignFile' | 'keybase.1.saltpack.saltpackSignString' | 'keybase.1.saltpack.saltpackVerifyFile' | 'keybase.1.saltpack.saltpackVerifyString' | 'keybase.1.signup.checkUsernameAvailable' | 'keybase.1.signup.getInvitationCode' | 'keybase.1.SimpleFS.simpleFSArchiveAllFiles' | 'keybase.1.SimpleFS.simpleFSArchiveAllGitRepos' | 'keybase.1.SimpleFS.simpleFSArchiveCancelOrDismissJob' | 'keybase.1.SimpleFS.simpleFSArchiveStart' | 'keybase.1.SimpleFS.simpleFSCancelDownload' | 'keybase.1.SimpleFS.simpleFSCheckReachability' | 'keybase.1.SimpleFS.simpleFSClearConflictState' | 'keybase.1.SimpleFS.simpleFSConfigureDownload' | 'keybase.1.SimpleFS.simpleFSCopyRecursive' | 'keybase.1.SimpleFS.simpleFSDismissDownload' | 'keybase.1.SimpleFS.simpleFSDismissUpload' | 'keybase.1.SimpleFS.simpleFSFinishResolvingConflict' | 'keybase.1.SimpleFS.simpleFSFolderSyncConfigAndStatus' | 'keybase.1.SimpleFS.simpleFSGetArchiveJobFreshness' | 'keybase.1.SimpleFS.simpleFSGetArchiveStatus' | 'keybase.1.SimpleFS.simpleFSGetDownloadInfo' | 'keybase.1.SimpleFS.simpleFSGetDownloadStatus' | 'keybase.1.SimpleFS.simpleFSGetFilesTabBadge' | 'keybase.1.SimpleFS.simpleFSGetFolder' | 'keybase.1.SimpleFS.simpleFSGetGUIFileContext' | 'keybase.1.SimpleFS.simpleFSGetOnlineStatus' | 'keybase.1.SimpleFS.simpleFSGetUploadStatus' | 'keybase.1.SimpleFS.simpleFSList' | 'keybase.1.SimpleFS.simpleFSListFavorites' | 'keybase.1.SimpleFS.simpleFSListRecursiveToDepth' | 'keybase.1.SimpleFS.simpleFSMakeTempDirForUpload' | 'keybase.1.SimpleFS.simpleFSMove' | 'keybase.1.SimpleFS.simpleFSOpen' | 'keybase.1.SimpleFS.simpleFSReadList' | 'keybase.1.SimpleFS.simpleFSRemove' | 'keybase.1.SimpleFS.simpleFSSetDebugLevel' | 'keybase.1.SimpleFS.simpleFSSetFolderSyncConfig' | 'keybase.1.SimpleFS.simpleFSSetNotificationThreshold' | 'keybase.1.SimpleFS.simpleFSSetSfmiBannerDismissed' | 'keybase.1.SimpleFS.simpleFSSetSyncOnCellular' | 'keybase.1.SimpleFS.simpleFSSettings' | 'keybase.1.SimpleFS.simpleFSStartDownload' | 'keybase.1.SimpleFS.simpleFSStartUpload' | 'keybase.1.SimpleFS.simpleFSStat' | 'keybase.1.SimpleFS.simpleFSSubscribeNonPath' | 'keybase.1.SimpleFS.simpleFSSubscribePath' | 'keybase.1.SimpleFS.simpleFSSyncStatus' | 'keybase.1.SimpleFS.simpleFSUnsubscribe' | 'keybase.1.SimpleFS.simpleFSUserEditHistory' | 'keybase.1.SimpleFS.simpleFSUserIn' | 'keybase.1.SimpleFS.simpleFSUserOut' | 'keybase.1.SimpleFS.simpleFSWait' | 'keybase.1.teams.findAssertionsInTeamNoResolve' | 'keybase.1.teams.getAnnotatedTeam' | 'keybase.1.teams.getInviteLinkDetails' | 'keybase.1.teams.getTeamID' | 'keybase.1.teams.getTeamRoleMap' | 'keybase.1.teams.getUntrustedTeamInfo' | 'keybase.1.teams.loadTeamTreeMembershipsAsync' | 'keybase.1.teams.setTarsDisabled' | 'keybase.1.teams.setTeamMemberShowcase' | 'keybase.1.teams.setTeamShowcase' | 'keybase.1.teams.teamAddEmailsBulk' | 'keybase.1.teams.teamAddMember' | 'keybase.1.teams.teamAddMembersMultiRole' | 'keybase.1.teams.teamCreate' | 'keybase.1.teams.teamCreateFancy' | 'keybase.1.teams.teamCreateSeitanTokenV2' | 'keybase.1.teams.teamEditMembers' | 'keybase.1.teams.teamGetMembersByID' | 'keybase.1.teams.teamIgnoreRequest' | 'keybase.1.teams.teamLeave' | 'keybase.1.teams.teamListMyAccessRequests' | 'keybase.1.teams.teamListUnverified' | 'keybase.1.teams.teamProfileAddList' | 'keybase.1.teams.teamReAddMemberAfterReset' | 'keybase.1.teams.teamRemoveMember' | 'keybase.1.teams.teamRename' | 'keybase.1.teams.teamSetSettings' | 'keybase.1.teams.untrustedTeamExists' | 'keybase.1.teams.uploadTeamAvatar' | 'keybase.1.user.blockUser' | 'keybase.1.user.canLogout' | 'keybase.1.user.dismissBlockButtons' | 'keybase.1.user.getUserBlocks' | 'keybase.1.user.interestingPeople' | 'keybase.1.user.listTrackersUnverified' | 'keybase.1.user.listTracking' | 'keybase.1.user.loadMySettings' | 'keybase.1.user.loadPassphraseState' | 'keybase.1.user.profileEdit' | 'keybase.1.user.proofSuggestions' | 'keybase.1.user.reportUser' | 'keybase.1.user.setUserBlocks' | 'keybase.1.user.unblockUser' | 'keybase.1.user.uploadUserAvatar' | 'keybase.1.user.userCard' | 'keybase.1.userSearch.bulkEmailOrPhoneSearch' | 'keybase.1.userSearch.getNonUserDetails' | 'keybase.1.userSearch.userSearch' export type RpcFn = [RpcIn] extends [undefined] ? (params?: undefined, waitingKey?: WaitingKey) => Promise> : (params: RpcIn, waitingKey?: WaitingKey) => Promise> @@ -2847,6 +2859,7 @@ export type PublicKeyV2 ={ keyType: KeyType.nacl, nacl: PublicKeyV2NaCl } | { ke export type PublicKeyV2Base = {readonly kid: KID,readonly isSibkey: boolean,readonly isEldest: boolean,readonly cTime: Time,readonly eTime: Time,readonly provisioning: SignatureMetadata,readonly revocation?: SignatureMetadata | null,} export type PublicKeyV2NaCl = {readonly base: PublicKeyV2Base,readonly parent?: KID | null,readonly deviceID: DeviceID,readonly deviceDescription: string,readonly deviceType: DeviceTypeV2,} export type PublicKeyV2PGPSummary = {readonly base: PublicKeyV2Base,readonly fingerprint: PGPFingerprint,readonly identities?: ReadonlyArray | null,} +export type PushTapRoute = {readonly url: string,readonly targetUID: string,readonly id: number,} export type RawPhoneNumber = string export type Reachability = {readonly reachable: Reachable,} export type ReadArgs = {readonly opID: OpID,readonly path: Path,readonly offset: number,readonly size: number,} @@ -3127,7 +3140,7 @@ export type WalletAccountInfo = {readonly accountID: string,readonly numUnread: export type WebProof = {readonly hostname: string,readonly protocols?: ReadonlyArray | null,} export type WriteArgs = {readonly opID: OpID,readonly path: Path,readonly offset: number,} -type IncomingMethod = 'keybase.1.NotifyApp.clientState' | 'keybase.1.NotifyApp.mobileAppStateChanged' | 'keybase.1.NotifyAudit.boxAuditError' | 'keybase.1.NotifyAudit.rootAuditError' | 'keybase.1.NotifyBadges.badgeState' | 'keybase.1.NotifyDeviceHistory.deviceHistoryChanged' | 'keybase.1.NotifyFS.FSActivity' | 'keybase.1.NotifySession.loggedOut' | 'keybase.1.NotifyTracking.trackingChanged' | 'keybase.1.NotifyUsers.userChanged' | 'keybase.1.loginUi.displayPaperKeyPhrase' | 'keybase.1.loginUi.displayPrimaryPaperKey' | 'keybase.1.loginUi.displayResetProgress' | 'keybase.1.loginUi.explainDeviceRecovery' | 'keybase.1.pgpUi.finished' | 'keybase.1.proveUi.displayRecheckWarning' | 'keybase.1.proveUi.outputPrechecks' | 'keybase.1.provisionUi.DisplaySecretExchanged' | 'keybase.1.provisionUi.ProvisioneeSuccess' | 'keybase.1.provisionUi.ProvisionerSuccess' | 'keybase.1.rekeyUI.refresh' | 'keybase.1.rekeyUI.rekeySendEvent' +type IncomingMethod = 'keybase.1.NotifyApp.clientState' | 'keybase.1.NotifyApp.mobileAppStateChanged' | 'keybase.1.NotifyApp.pushTapRouteAvailable' | 'keybase.1.NotifyAudit.boxAuditError' | 'keybase.1.NotifyAudit.rootAuditError' | 'keybase.1.NotifyBadges.badgeState' | 'keybase.1.NotifyDeviceHistory.deviceHistoryChanged' | 'keybase.1.NotifyFS.FSActivity' | 'keybase.1.NotifySession.loggedOut' | 'keybase.1.NotifyTracking.trackingChanged' | 'keybase.1.NotifyUsers.userChanged' | 'keybase.1.loginUi.displayPaperKeyPhrase' | 'keybase.1.loginUi.displayPrimaryPaperKey' | 'keybase.1.loginUi.displayResetProgress' | 'keybase.1.loginUi.explainDeviceRecovery' | 'keybase.1.pgpUi.finished' | 'keybase.1.proveUi.displayRecheckWarning' | 'keybase.1.proveUi.outputPrechecks' | 'keybase.1.provisionUi.DisplaySecretExchanged' | 'keybase.1.provisionUi.ProvisioneeSuccess' | 'keybase.1.provisionUi.ProvisionerSuccess' | 'keybase.1.rekeyUI.refresh' | 'keybase.1.rekeyUI.rekeySendEvent' export type IncomingCallMapType = Partial<{[M in IncomingMethod]: (params: RpcIn) => void}> type CustomIncomingMethod = 'keybase.1.NotifyApp.exit' | 'keybase.1.NotifyEmailAddress.emailAddressVerified' | 'keybase.1.NotifyEmailAddress.emailsChanged' | 'keybase.1.NotifyFS.FSOverallSyncStatusChanged' | 'keybase.1.NotifyFS.FSSubscriptionNotify' | 'keybase.1.NotifyFS.FSSubscriptionNotifyPath' | 'keybase.1.NotifyFeaturedBots.featuredBotsUpdate' | 'keybase.1.NotifyPGP.pgpKeyInSecretStoreFile' | 'keybase.1.NotifyPhoneNumber.phoneNumbersChanged' | 'keybase.1.NotifyRuntimeStats.runtimeStatsUpdate' | 'keybase.1.NotifyService.HTTPSrvInfoUpdate' | 'keybase.1.NotifyService.handleKeybaseLink' | 'keybase.1.NotifyService.shutdown' | 'keybase.1.NotifySession.clientOutOfDate' | 'keybase.1.NotifySession.loggedIn' | 'keybase.1.NotifySimpleFS.simpleFSArchiveStatusChanged' | 'keybase.1.NotifyTeam.avatarUpdated' | 'keybase.1.NotifyTeam.teamChangedByID' | 'keybase.1.NotifyTeam.teamDeleted' | 'keybase.1.NotifyTeam.teamExit' | 'keybase.1.NotifyTeam.teamMetadataUpdate' | 'keybase.1.NotifyTeam.teamRoleMapChanged' | 'keybase.1.NotifyTeam.teamTreeMembershipsDone' | 'keybase.1.NotifyTeam.teamTreeMembershipsPartial' | 'keybase.1.NotifyTracking.notifyUserBlocked' | 'keybase.1.NotifyTracking.trackingInfo' | 'keybase.1.NotifyUsers.identifyUpdate' | 'keybase.1.NotifyUsers.passwordChanged' | 'keybase.1.gpgUi.selectKey' | 'keybase.1.gpgUi.wantToAddGPGKey' | 'keybase.1.gregorUI.pushState' | 'keybase.1.homeUI.homeUIRefresh' | 'keybase.1.identify3Ui.identify3Result' | 'keybase.1.identify3Ui.identify3ShowTracker' | 'keybase.1.identify3Ui.identify3Summary' | 'keybase.1.identify3Ui.identify3UpdateRow' | 'keybase.1.identify3Ui.identify3UpdateUserCard' | 'keybase.1.identify3Ui.identify3UserReset' | 'keybase.1.logUi.log' | 'keybase.1.loginUi.chooseDeviceToRecoverWith' | 'keybase.1.loginUi.displayPaperKeyPhrase' | 'keybase.1.loginUi.displayPrimaryPaperKey' | 'keybase.1.loginUi.displayResetProgress' | 'keybase.1.loginUi.explainDeviceRecovery' | 'keybase.1.loginUi.getEmailOrUsername' | 'keybase.1.loginUi.promptPassphraseRecovery' | 'keybase.1.loginUi.promptResetAccount' | 'keybase.1.loginUi.promptRevokePaperKeys' | 'keybase.1.logsend.prepareLogsend' | 'keybase.1.pgpUi.finished' | 'keybase.1.pgpUi.keyGenerated' | 'keybase.1.pgpUi.shouldPushPrivate' | 'keybase.1.proveUi.checking' | 'keybase.1.proveUi.continueChecking' | 'keybase.1.proveUi.displayRecheckWarning' | 'keybase.1.proveUi.okToCheck' | 'keybase.1.proveUi.outputInstructions' | 'keybase.1.proveUi.outputPrechecks' | 'keybase.1.proveUi.preProofWarning' | 'keybase.1.proveUi.promptOverwrite' | 'keybase.1.proveUi.promptUsername' | 'keybase.1.provisionUi.DisplayAndPromptSecret' | 'keybase.1.provisionUi.DisplaySecretExchanged' | 'keybase.1.provisionUi.PromptNewDeviceName' | 'keybase.1.provisionUi.ProvisioneeSuccess' | 'keybase.1.provisionUi.ProvisionerSuccess' | 'keybase.1.provisionUi.chooseDevice' | 'keybase.1.provisionUi.chooseDeviceType' | 'keybase.1.provisionUi.chooseGPGMethod' | 'keybase.1.provisionUi.switchToGPGSignOK' | 'keybase.1.rekeyUI.delegateRekeyUI' | 'keybase.1.rekeyUI.refresh' | 'keybase.1.rekeyUI.rekeySendEvent' | 'keybase.1.secretUi.getPassphrase' | 'keybase.1.teamsUi.confirmInviteLinkAccept' | 'keybase.1.teamsUi.confirmRootTeamDelete' | 'keybase.1.teamsUi.confirmSubteamDelete' @@ -3195,6 +3208,8 @@ export const apiserverDeleteRpcPromise = createRpc('keybase.1.apiserver.Delete') export const apiserverGetWithSessionRpcPromise = createRpc('keybase.1.apiserver.GetWithSession') export const apiserverPostJSONRpcPromise = createRpc('keybase.1.apiserver.PostJSON') export const apiserverPostRpcPromise = createRpc('keybase.1.apiserver.Post') +export const appStateAckPushTapRouteRpcPromise = createRpc('keybase.1.appState.ackPushTapRoute') +export const appStatePeekPushTapRouteRpcPromise = createRpc('keybase.1.appState.peekPushTapRoute') export const appStatePowerMonitorEventRpcPromise = createRpc('keybase.1.appState.powerMonitorEvent') export const appStateUpdateMobileNetStateRpcPromise = createRpc('keybase.1.appState.updateMobileNetState') export const configAppendGUILogsRpcPromise = createRpc('keybase.1.config.appendGUILogs') From ea4299155773ebc52d80db3f285a28de9fab67fd Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 14:17:00 -0400 Subject: [PATCH 2/6] feat(push): resolve a tapped notification in Go ResolvePushTap turns a push payload into the keybase:// URL it opens, the mapping the client used to carry, and PendingPushTap parks the result until a client acks it. Native delivers a tap through bind.DeliverPushTap, the one door a tap comes through and the only thing that may name an account to switch to, so a URL another app opens cannot switch accounts. Each parked route carries an id, so an ack that crosses a newer tap retires nothing, and a tap not yet acked is replaced rather than queued -- the newest tap is the one the user just made. --- go/bind/keybase.go | 27 +++++- go/bind/keybase_test.go | 4 +- go/libkb/globals.go | 2 + go/libkb/notify_router.go | 22 ++++- go/libkb/pushtap.go | 179 ++++++++++++++++++++++++++++++++++++ go/libkb/pushtap_test.go | 158 +++++++++++++++++++++++++++++++ go/service/appstate.go | 20 ++++ go/service/appstate_test.go | 96 +++++++++++++++++++ 8 files changed, 503 insertions(+), 5 deletions(-) create mode 100644 go/libkb/pushtap.go create mode 100644 go/libkb/pushtap_test.go create mode 100644 go/service/appstate_test.go diff --git a/go/bind/keybase.go b/go/bind/keybase.go index 3c4c16e010d3..d6680cb7629e 100644 --- a/go/bind/keybase.go +++ b/go/bind/keybase.go @@ -749,7 +749,7 @@ func ensureConnection() error { // Reset unconditionally resets the socket connection. Use this only when the // caller genuinely means "tear down whatever connection is current" (e.g. -// iOS invalidate, Android destroy/engineReset) — it will happily close a +// iOS invalidate, Android destroy) — it will happily close a // connection some concurrent failure-driven caller never saw fail. Callers // reacting to a failure on a specific connection should use ResetIfCurrent // instead so a stale complaint can't clobber a connection that has already @@ -919,6 +919,31 @@ func locationUpdate(tracker types.LiveLocationTracker, lat, lon float64, accurac tracker.NativeLocationUpdate(context.Background(), chat1.Coordinate{Lat: lat, Lon: lon, Accuracy: float64(accuracy)}) } +// DeliverPushTap resolves a tapped notification's payload to the route it opens +// and parks it for the client to take. +// +// The one door a tap comes through, and the only thing anywhere that may name +// an account to switch to. Native calls it from its notification-tap handler +// and nowhere else -- on iOS UNUserNotificationCenter's didReceive, on Android +// the unexported PushTapActivity -- so a URL another app, a web page or a +// universal link opens cannot reach it, and cannot switch accounts. A silent or +// background push does not come through here at all: those are +// HandleBackgroundNotification, which never routes. +func DeliverPushTap(payloadJSON string) { + if !isInited() { + log("DeliverPushTap: dropping a tap taken before Init") + return + } + ctx := context.Background() + route, ok := libkb.ResolvePushTap(payloadJSON) + if !ok { + kbCtx.Log.CDebugf(ctx, "DeliverPushTap: a tap with nothing to open") + return + } + kbCtx.Log.CDebugf(ctx, "DeliverPushTap: %s (for another account: %v)", route.Url, route.TargetUID != "") + kbCtx.PendingPushTap.Set(ctx, route) +} + func waitForInit(maxDur time.Duration) error { if isInited() { return nil diff --git a/go/bind/keybase_test.go b/go/bind/keybase_test.go index 19be3b5cd5aa..5d3c0f27d878 100644 --- a/go/bind/keybase_test.go +++ b/go/bind/keybase_test.go @@ -273,7 +273,7 @@ func TestResetIfCurrent_DoubleResetSameEpochIsHarmless(t *testing.T) { } // Test 4: Reset is the unconditional escape hatch used by invalidate/ -// destroy/engineReset. It must close whatever connection is current +// destroy. It must close whatever connection is current // regardless of any epoch bookkeeping. func TestReset_UnconditionallyClosesCurrentConnection(t *testing.T) { resetConnStateForTest(t) @@ -528,7 +528,7 @@ func TestConcurrentReadWriteAndResetsThroughRealEntryPoints(t *testing.T) { }) } - // Unconditional resetters: e.g. concurrent invalidate/engineReset. + // Unconditional resetters: e.g. concurrent invalidate/destroy. for range resetters { wg.Go(func() { for range iterations { diff --git a/go/libkb/globals.go b/go/libkb/globals.go index 77cbb6dd8d2b..28b125767b1e 100644 --- a/go/libkb/globals.go +++ b/go/libkb/globals.go @@ -71,6 +71,7 @@ type GlobalContext struct { MobileNetState *MobileNetState // The kind of network connection for the currently running instance of the app MobileAppState *MobileAppState // The state of focus for the currently running instance of the app MobileLifecycle *lifecycle.Controller // Derives MobileAppState from native UI reports and background-work holds + PendingPushTap *PendingPushTap // Holds the route a tapped notification resolved to until a client takes it DesktopAppState *DesktopAppState // The state of focus for the currently running instance of the app ChatHelper ChatHelper // conveniently send chat messages RPCCanceler *RPCCanceler // register live RPCs so they can be cancelleed en masse @@ -312,6 +313,7 @@ func (g *GlobalContext) Init() *GlobalContext { Flush: g.flushLocalDbs, Debug: func(format string, args ...interface{}) { g.Log.Debug(format, args...) }, }) + g.PendingPushTap = NewPendingPushTap(g) g.DesktopAppState = NewDesktopAppState(g) g.RPCCanceler = NewRPCCanceler() g.IdentifyDispatch = NewIdentifyDispatch() diff --git a/go/libkb/notify_router.go b/go/libkb/notify_router.go index 019b8cb4dbd5..ebc114645405 100644 --- a/go/libkb/notify_router.go +++ b/go/libkb/notify_router.go @@ -337,8 +337,8 @@ func NewNotifyRouter(g *GlobalContext) *NotifyRouter { } // connSender sends one connection's client-state stream: loggedIn, loggedOut, -// HTTPSrvInfoUpdate, mobileAppStateChanged and clientState. Every other -// notification keeps its own goroutine. +// HTTPSrvInfoUpdate, mobileAppStateChanged, clientState and +// pushTapRouteAvailable. Every other notification keeps its own goroutine. // // One goroutine per connection drains an unbounded FIFO, so queueing never // blocks, and the rpc library writes one goroutine's Notify calls in the order @@ -3059,6 +3059,24 @@ func (n *NotifyRouter) HandleMobileAppState(ctx context.Context, state keybase1. }) } +// HandlePushTapRouteAvailable nudges clients that a notification tap resolved +// to a route. It carries nothing: the route rides peekPushTapRoute's reply, so +// the reader is the same one whether the tap happened before a client existed +// or while it was connected, and the route is retired by an ack from whoever +// acted on it rather than by having been read. +func (n *NotifyRouter) HandlePushTapRouteAvailable(ctx context.Context) { + if n == nil { + return + } + n.announce(ctx, "HandlePushTapRouteAvailable", + func(ch keybase1.NotificationChannels) bool { return ch.App }, + func(ctx context.Context, xp rpc.Transporter) { + _ = (keybase1.NotifyAppClient{ + Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), + }).PushTapRouteAvailable(ctx) + }) +} + func (n *NotifyRouter) HandleHandleKeybaseLink(ctx context.Context, link string, deferred bool) { if n == nil { return diff --git a/go/libkb/pushtap.go b/go/libkb/pushtap.go new file mode 100644 index 000000000000..e44993fe9d70 --- /dev/null +++ b/go/libkb/pushtap.go @@ -0,0 +1,179 @@ +package libkb + +import ( + "context" + "encoding/json" + "strings" + "sync" + + "github.com/keybase/client/go/protocol/keybase1" +) + +// PendingPushTap holds the route a tapped notification resolved to until a +// client says it has acted on it. +// +// It is the whole of the exactly-once guarantee for a tap. A tap can arrive +// when no client exists -- on iOS a tap that launches the process, on Android a +// tap that starts PushTapActivity before the RN host -- so it has to wait +// somewhere that outlives the client, which is here. +// +// Reading does not clear, because a reply lost on the way out would take the +// tap with it and nothing would be left to say a tap had ever happened. The +// client is the only party that knows it acted, so the client says so: Peek +// leaves the route armed and Ack retires it. Each route carries an id, so an +// ack that crosses a newer tap retires nothing. +type PendingPushTap struct { + Contextified + sync.Mutex + route *keybase1.PushTapRoute + lastID int +} + +func NewPendingPushTap(g *GlobalContext) *PendingPushTap { + return &PendingPushTap{Contextified: NewContextified(g)} +} + +// Set stores the route a tap resolved to, gives it a fresh id, and nudges +// connected clients. A tap not yet acked is replaced: the newest tap is the one +// the user just made, and queueing them would navigate through a backlog. +// +// Ids count the taps of this process and start at 1, so 0 is never a route a +// client has seen and is safe as a client-side sentinel. +func (p *PendingPushTap) Set(ctx context.Context, route keybase1.PushTapRoute) { + p.Lock() + p.lastID++ + route.Id = p.lastID + p.route = &route + p.Unlock() + p.G().NotifyRouter.HandlePushTapRouteAvailable(ctx) +} + +// Peek returns the waiting route without retiring it, or nil when none is +// waiting. It stays armed for the next reader until it is acked. The result is +// a copy, so the holder's own route is never reachable through a reader. +func (p *PendingPushTap) Peek() *keybase1.PushTapRoute { + p.Lock() + defer p.Unlock() + if p.route == nil { + return nil + } + route := *p.route + return &route +} + +// Ack retires the waiting route if it is still the one with this id, and +// reports whether it did. A stale id means a newer tap arrived while the ack +// was in flight, and that one must survive to be acted on. +func (p *PendingPushTap) Ack(id int) bool { + p.Lock() + defer p.Unlock() + if p.route == nil || p.route.Id != id { + return false + } + p.route = nil + return true +} + +// pushTapNoRouteTypes are the push types a tap never opens anything for: they +// are acted on natively and here, and have no screen of their own. +var pushTapNoRouteTypes = map[string]bool{ + "autoreset": true, + "chat.extension": true, + "chat.failedpending": true, + "chat.newmessageSilent_2": true, + "chat.readmessage": true, +} + +// pushTapContactPrefix is all that is read of a contact-joined message. The +// rest names a person, and only the prefix decides the destination. +const pushTapContactPrefix = "Your contact" + +// ResolvePushTap turns the payload of a tapped notification into the route it +// opens. The second result is false when the tap only opens the app. +// +// payloadJSON is the push as the OS delivered it -- APNs userInfo on iOS, the +// FCM data Bundle on Android -- so every value is whatever the sender put +// there: fields may be missing, and a number is as likely as a string. +func ResolvePushTap(payloadJSON string) (keybase1.PushTapRoute, bool) { + var none keybase1.PushTapRoute + if !json.Valid([]byte(payloadJSON)) { + return none, false + } + dec := json.NewDecoder(strings.NewReader(payloadJSON)) + // Numbers keep their literal text, so a numeric convID reads back as the + // digits that were sent rather than a float rendering of them. + dec.UseNumber() + var parsed any + if err := dec.Decode(&parsed); err != nil { + return none, false + } + fields, isObject := parsed.(map[string]any) + if !isObject { + return none, false + } + get := func(key string) string { + switch value := fields[key].(type) { + case string: + return value + case json.Number: + return value.String() + default: + return "" + } + } + forAccount := func(url, uid string) (keybase1.PushTapRoute, bool) { + return keybase1.PushTapRoute{Url: url, TargetUID: uid}, true + } + + typ := get("type") + switch { + case typ == "chat.newmessage": + if convID := get("convID"); convID != "" { + return forAccount("keybase://convid/"+encodeURIComponent(convID), get("uid")) + } + case typ == "follow": + if username := get("username"); username != "" { + uid := get("uid") + if uid == "" { + uid = get("targetUID") + } + return forAccount("keybase://profile/show/"+encodeURIComponent(username), uid) + } + case typ == "device.new", typ == "device.revoked": + if uid := get("uid"); uid != "" { + return forAccount("keybase://devices", uid) + } + case pushTapNoRouteTypes[typ]: + default: + if strings.HasPrefix(get("message"), pushTapContactPrefix) { + // No account: a contact-joined push is not account-scoped, so a tap + // on it must not switch accounts. + return keybase1.PushTapRoute{Url: "keybase://tabs.peopleTab"}, true + } + } + return none, false +} + +const pushTapUnreservedMarks = "-_.!~*'()" + +// encodeURIComponent escapes a path segment the way JavaScript's function of +// that name does. Go's url escapers each differ from it somewhere -- a space, +// or one of the marks below -- and the result here is compared against URLs +// clients build with the JavaScript one. +func encodeURIComponent(s string) string { + var out strings.Builder + const hex = "0123456789ABCDEF" + for i := 0; i < len(s); i++ { + c := s[i] + switch { + case c >= 'A' && c <= 'Z', c >= 'a' && c <= 'z', c >= '0' && c <= '9', + strings.IndexByte(pushTapUnreservedMarks, c) >= 0: + out.WriteByte(c) + default: + out.WriteByte('%') + out.WriteByte(hex[c>>4]) + out.WriteByte(hex[c&0xf]) + } + } + return out.String() +} diff --git a/go/libkb/pushtap_test.go b/go/libkb/pushtap_test.go new file mode 100644 index 000000000000..857a005c926e --- /dev/null +++ b/go/libkb/pushtap_test.go @@ -0,0 +1,158 @@ +package libkb + +import ( + "context" + "testing" + + "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/require" +) + +// The cases are the table the client used to carry (deep-link-emitter.test.ts), +// kept so the destination a tap opens did not change when the mapping moved +// here. +func TestResolvePushTap(t *testing.T) { + route := func(url, uid string) *keybase1.PushTapRoute { + return &keybase1.PushTapRoute{Url: url, TargetUID: uid} + } + cases := []struct { + name string + payload string + want *keybase1.PushTapRoute + }{ + { + "chat with account", `{"type":"chat.newmessage","convID":"0000ab","uid":"u1"}`, + route("keybase://convid/0000ab", "u1"), + }, + { + "chat without account", `{"type":"chat.newmessage","convID":"0000ab"}`, + route("keybase://convid/0000ab", ""), + }, + {"chat without conversation", `{"type":"chat.newmessage"}`, nil}, + { + "apns chat with numbers and aps", + `{"type":"chat.newmessage","convID":"0000ab","uid":"u1","t":1,"aps":{"alert":{"body":"hi"}}}`, + route("keybase://convid/0000ab", "u1"), + }, + { + "a numeric convID becomes a string", `{"type":"chat.newmessage","convID":1234}`, + route("keybase://convid/1234", ""), + }, + { + "the uid is kept verbatim", `{"type":"chat.newmessage","convID":"0000ab","uid":"u 1&x"}`, + route("keybase://convid/0000ab", "u 1&x"), + }, + { + "follow with uid", `{"type":"follow","username":"testuser","uid":"u1"}`, + route("keybase://profile/show/testuser", "u1"), + }, + { + "follow with targetUID", `{"type":"follow","username":"testuser","targetUID":"u2"}`, + route("keybase://profile/show/testuser", "u2"), + }, + {"follow without username", `{"type":"follow","uid":"u1"}`, nil}, + { + "new device", `{"type":"device.new","uid":"u1","device_id":"d1"}`, + route("keybase://devices", "u1"), + }, + {"revoked device without account", `{"type":"device.revoked","device_id":"d1"}`, nil}, + { + "contacts joined", `{"message":"Your contact testuser joined Keybase"}`, + route("keybase://tabs.peopleTab", ""), + }, + {"read receipt", `{"type":"chat.readmessage","b":0,"message":"Your contact x"}`, nil}, + {"silent chat", `{"type":"chat.newmessageSilent_2","c":"0000ab"}`, nil}, + {"extension", `{"type":"chat.extension","convID":"0000ab"}`, nil}, + {"autoreset", `{"type":"autoreset","uid":"u1"}`, nil}, + {"failed pending", `{"type":"chat.failedpending","convID":"0000ab","uid":""}`, nil}, + {"an unknown type opens nothing", `{"type":"something.new","uid":"u1"}`, nil}, + {"not json", `not json`, nil}, + {"json that is not an object", `"just a string"`, nil}, + {"json with trailing garbage", `{"type":"chat.newmessage","convID":"0000ab"} x`, nil}, + { + "a conversation id is escaped into the URL", + `{"type":"chat.newmessage","convID":"a/b c&d"}`, + route("keybase://convid/a%2Fb%20c%26d", ""), + }, + { + "a username is escaped into the URL", + `{"type":"follow","username":"a b/c"}`, + route("keybase://profile/show/a%20b%2Fc", ""), + }, + {"a non-string message is not a contact push", `{"message":1}`, nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := ResolvePushTap(tc.payload) + if tc.want == nil { + require.False(t, ok) + require.Equal(t, keybase1.PushTapRoute{}, got) + return + } + require.True(t, ok) + require.Equal(t, *tc.want, got) + }) + } +} + +// encodeURIComponent's escape set is what keeps a URL built here identical to +// the one the client used to build, so the marks JavaScript leaves alone are +// pinned rather than assumed. +func TestEncodeURIComponent(t *testing.T) { + require.Equal(t, "-_.!~*'()", encodeURIComponent("-_.!~*'()")) + require.Equal(t, "abcXYZ019", encodeURIComponent("abcXYZ019")) + require.Equal(t, "%20%2B%2F%3F%23%26%3D%25", encodeURIComponent(" +/?#&=%")) + require.Equal(t, "%E2%9C%93", encodeURIComponent("✓")) + require.Empty(t, encodeURIComponent("")) +} + +func TestPendingPushTapPeekIsNotDestructive(t *testing.T) { + tc := SetupTest(t, "pushtap", 1) + defer tc.Cleanup() + g := tc.G + ctx := context.Background() + + require.Nil(t, g.PendingPushTap.Peek()) + + g.PendingPushTap.Set(ctx, keybase1.PushTapRoute{Url: "keybase://convid/0000ab", TargetUID: "u1"}) + first := g.PendingPushTap.Peek() + require.NotNil(t, first) + require.Equal(t, "keybase://convid/0000ab", first.Url) + require.NotZero(t, first.Id, "Set stamps an id") + + // The peek that never reached the client -- or whose reply did not come back -- + // must leave the tap where it was, or the tap is gone with nothing to say so. + again := g.PendingPushTap.Peek() + require.Equal(t, first, again) + + require.True(t, g.PendingPushTap.Ack(first.Id)) + require.Nil(t, g.PendingPushTap.Peek(), "the ack retired it") + require.False(t, g.PendingPushTap.Ack(first.Id), "nothing left to retire") +} + +func TestPendingPushTapAckDoesNotRetireANewerTap(t *testing.T) { + tc := SetupTest(t, "pushtap", 1) + defer tc.Cleanup() + g := tc.G + ctx := context.Background() + + g.PendingPushTap.Set(ctx, keybase1.PushTapRoute{Url: "keybase://convid/0000ab"}) + stale := g.PendingPushTap.Peek() + require.NotNil(t, stale) + + // A tap not yet acked is replaced rather than queued: the newest tap is the + // one the user just made. + g.PendingPushTap.Set(ctx, keybase1.PushTapRoute{Url: "keybase://devices", TargetUID: "u2"}) + newer := g.PendingPushTap.Peek() + require.NotNil(t, newer) + require.Equal(t, "keybase://devices", newer.Url) + require.NotEqual(t, stale.Id, newer.Id) + + // The ack for the tap it replaced was already in flight; it must not take the + // newer one with it. + require.False(t, g.PendingPushTap.Ack(stale.Id)) + require.Equal(t, newer, g.PendingPushTap.Peek()) + + require.True(t, g.PendingPushTap.Ack(newer.Id)) + require.Nil(t, g.PendingPushTap.Peek()) +} diff --git a/go/service/appstate.go b/go/service/appstate.go index 97d05a86a66c..87ff14624d7a 100644 --- a/go/service/appstate.go +++ b/go/service/appstate.go @@ -45,6 +45,26 @@ func (a *appStateHandler) UpdateMobileNetState(ctx context.Context, stateStr str return nil } +// PeekPushTapRoute reports the route a tapped notification resolved to, and +// leaves it armed until the client acks. It is its own call rather than a field +// in setNotifications' snapshot: that reply goes to every subscriber, kbfs +// inside this same process among them, and a tap carried there would be read by +// whichever one subscribed first. +func (a *appStateHandler) PeekPushTapRoute(ctx context.Context) (*keybase1.PushTapRoute, error) { + route := a.G().PendingPushTap.Peek() + a.G().Log.CDebugf(ctx, "PeekPushTapRoute: waiting tap: %v", route != nil) + return route, nil +} + +// AckPushTapRoute retires the tap the client has acted on. Until this call the +// route stays armed, so a peek whose reply never arrived costs a repeat rather +// than the tap. +func (a *appStateHandler) AckPushTapRoute(ctx context.Context, id int) error { + retired := a.G().PendingPushTap.Ack(id) + a.G().Log.CDebugf(ctx, "AckPushTapRoute(%d): retired: %v", id, retired) + return nil +} + func (a *appStateHandler) PowerMonitorEvent(ctx context.Context, event string) (err error) { a.G().Log.CDebugf(ctx, "PowerMonitorEvent(%v)", event) a.G().DesktopAppState.Update(a.MetaContext(ctx), event, a.xp) diff --git a/go/service/appstate_test.go b/go/service/appstate_test.go new file mode 100644 index 000000000000..1d34a47323af --- /dev/null +++ b/go/service/appstate_test.go @@ -0,0 +1,96 @@ +package service + +import ( + "context" + "testing" + + "github.com/keybase/client/go/libkb" + keybase1 "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/require" +) + +// A peek is not a take. The reply can be lost on the way to the client, and the +// client is the only party that knows whether it acted, so the route stays armed +// until the client says so -- a lost reply then costs a repeat, not the tap. +func TestPeekPushTapRouteLeavesTheTapArmed(t *testing.T) { + tc := libkb.SetupTest(t, "appstate", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + + h := newAppStateHandler(nil, g) + ctx := context.Background() + + got, err := h.PeekPushTapRoute(ctx) + require.NoError(t, err) + require.Nil(t, got, "no tap has happened") + + g.PendingPushTap.Set(ctx, keybase1.PushTapRoute{Url: "keybase://convid/0000ab", TargetUID: "u1"}) + + first, err := h.PeekPushTapRoute(ctx) + require.NoError(t, err) + require.NotNil(t, first) + require.Equal(t, "keybase://convid/0000ab", first.Url) + + again, err := h.PeekPushTapRoute(ctx) + require.NoError(t, err) + require.Equal(t, first, again, "still armed for a client that never got the first reply") + + require.NoError(t, h.AckPushTapRoute(ctx, first.Id)) + + got, err = h.PeekPushTapRoute(ctx) + require.NoError(t, err) + require.Nil(t, got, "the client said it acted") +} + +// An ack that crosses a newer tap must retire nothing: the user tapped again, +// and that tap has not been acted on. +func TestAckPushTapRouteIgnoresAStaleID(t *testing.T) { + tc := libkb.SetupTest(t, "appstate", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + + h := newAppStateHandler(nil, g) + ctx := context.Background() + + g.PendingPushTap.Set(ctx, keybase1.PushTapRoute{Url: "keybase://convid/0000ab"}) + stale, err := h.PeekPushTapRoute(ctx) + require.NoError(t, err) + require.NotNil(t, stale) + + g.PendingPushTap.Set(ctx, keybase1.PushTapRoute{Url: "keybase://devices", TargetUID: "u2"}) + + require.NoError(t, h.AckPushTapRoute(ctx, stale.Id)) + + survived, err := h.PeekPushTapRoute(ctx) + require.NoError(t, err) + require.NotNil(t, survived) + require.Equal(t, "keybase://devices", survived.Url) +} + +// A tap must ride its own call and nothing else. Every app subscriber gets a +// clientState, so a tap carried there would be consumed by whichever one got +// it first. +func TestSetNotificationsLeavesTheTapAlone(t *testing.T) { + tc := libkb.SetupTest(t, "appstate", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + + ctx := context.Background() + route := keybase1.PushTapRoute{Url: "keybase://devices", TargetUID: "u1"} + g.PendingPushTap.Set(ctx, route) + + svc := newTestClientStateService(t, g) + svc.settleInitialLoginAttempt(ctx) + rec := libkb.NewNotifyRecorder(g, keybase1.NotificationChannels{}) + defer rec.Close() + require.NoError(t, NewNotifyCtlHandler(nil, rec.ID, g).SetNotifications(ctx, keybase1.NotificationChannels{App: true})) + rec.Flush() + + got, err := newAppStateHandler(nil, g).PeekPushTapRoute(ctx) + require.NoError(t, err) + require.NotNil(t, got, "the subscribe did not consume the tap") + require.Equal(t, route.Url, got.Url) +} From a8f34ec63c461c7239414ae40a09151386783968 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 14:17:07 -0400 Subject: [PATCH 3/6] feat(mobile): native hands a notification tap to the service iOS delivers the tap from UNUserNotificationCenter's didReceive; Android's notification PendingIntent now opens PushTapActivity, which is not exported, so only this app's own notifications can start it. Both pass the push through verbatim -- which fields matter is the service's business -- and neither stores, re-emits or parses it. That retires the whole JS-facing push path: getInitialNotification, onPushNotification, KbSetInitialNotification/KbEmitPushNotification and the become-active re-emit, MainActivity's notification extra and its duplicate-tap hash, and engineReset with them. The Android PendingIntent data is a digest of the payload, because extras are not part of filterEquals and two notifications would otherwise share one PendingIntent, and it is immutable so its holder cannot substitute a payload. --- .../main/java/com/reactnativekb/KbModule.kt | 68 ---------- rnmodules/react-native-kb/ios/Kb.h | 5 - rnmodules/react-native-kb/ios/Kb.mm | 70 ---------- rnmodules/react-native-kb/src/NativeKb.ts | 5 +- rnmodules/react-native-kb/src/index.tsx | 11 -- .../android/app/src/main/AndroidManifest.xml | 9 ++ .../io/keybase/ossifrage/KBPushNotifier.kt | 45 ++++-- .../KeybasePushNotificationListenerService.kt | 58 +------- .../java/io/keybase/ossifrage/MainActivity.kt | 128 +++++++----------- .../io/keybase/ossifrage/PushTapActivity.kt | 53 ++++++++ shared/ios/Keybase/AppDelegate.swift | 85 ++++++------ 11 files changed, 187 insertions(+), 350 deletions(-) create mode 100644 shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt diff --git a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt index 3e4c2fe9caa6..c65813474fc7 100644 --- a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt +++ b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt @@ -7,7 +7,6 @@ import android.content.Context import android.content.Intent import android.net.Uri import android.os.Build -import android.os.Bundle import android.os.Environment import android.provider.Settings import android.text.format.DateFormat @@ -376,37 +375,6 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T // Android manages badge counts automatically via notification channels. } - @ReactMethod - override fun getInitialNotification(promise: Promise) { - // Clear on read so it behaves as a one-shot, matching iOS. - val bundle = KbModule.initialNotificationBundle - KbModule.initialNotificationBundle = null - if (bundle != null) { - try { - @Suppress("UNCHECKED_CAST") - val payload: WritableMap = Arguments.fromBundle(bundle) as WritableMap - promise.resolve(payload) - } catch (e: Exception) { - promise.resolve(null) - } - } else { - promise.resolve(null) - } - } - - private fun emitPushNotificationInternal(notification: Bundle) { - if (reactContext.hasActiveReactInstance() && canEmit()) { - try { - val payload = Arguments.fromBundle(notification) - emitOnPushNotification(payload) - } catch (e: Exception) { - NativeLogger.error("emitPushNotificationInternal failed to emit: " + e.message) - } - } else { - NativeLogger.warn("emitPushNotificationInternal no active react instance") - } - } - internal fun emitShareDataInternal(data: WritableMap) { if (reactContext.hasActiveReactInstance() && canEmit()) { try { @@ -467,18 +435,6 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T // chance of being delivered before committing to it. internal fun canDeliverReset(): Boolean = reactContext.hasActiveReactInstance() && canEmit() - // No current caller (kept for future use). - @ReactMethod - override fun engineReset() { - try { - Keybase.reset() - nativeResetRecv() - relayReset() - } catch (e: Exception) { - NativeLogger.error("Exception in engineReset", e) - } - } - @ReactMethod override fun notifyJSReady() { NativeLogger.info("JS signaled ready, starting ReadFromKBLib loop") @@ -780,35 +736,11 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T // visibility guarantee so the reader never sees a stale instance. @Volatile var instance: KbModule? = null - @JvmStatic - internal var initialNotificationBundle: Bundle? = null - @JvmStatic fun keyPressed(keyName: String) { instance?.sendHardwareKeyEvent(keyName) } - @JvmStatic - fun setInitialNotification(bundle: Bundle?) { - initialNotificationBundle = bundle - } - - @JvmStatic - fun isReactNativeRunning(): Boolean { - return instance != null - } - - @JvmStatic - fun emitPushNotification(notification: Bundle) { - val module = instance - if (module == null) { - // NativeLogger writes to the Go service, which may not be up here. - android.util.Log.w("KbModule", "emitPushNotification called but instance is null (app may not be running)") - return - } - module.emitPushNotificationInternal(notification) - } - @JvmStatic fun emitShareData(data: WritableMap) { val module = instance diff --git a/rnmodules/react-native-kb/ios/Kb.h b/rnmodules/react-native-kb/ios/Kb.h index 929434082d22..aec81655919a 100644 --- a/rnmodules/react-native-kb/ios/Kb.h +++ b/rnmodules/react-native-kb/ios/Kb.h @@ -22,8 +22,3 @@ // Push notification helpers - can be called from AppDelegate FOUNDATION_EXPORT void KbSetDeviceToken(NSString *token); -FOUNDATION_EXPORT void KbSetInitialNotification(NSDictionary *notification); -FOUNDATION_EXPORT void KbEmitPushNotification(NSDictionary *notification); -// Re-emits a stored user-interaction notification once when the app becomes -// active (covers notification taps that arrive before React Native is ready). -FOUNDATION_EXPORT void KbEmitStoredNotificationOnBecomeActive(void); diff --git a/rnmodules/react-native-kb/ios/Kb.mm b/rnmodules/react-native-kb/ios/Kb.mm index 48ad75ca8097..76231a5e19df 100644 --- a/rnmodules/react-native-kb/ios/Kb.mm +++ b/rnmodules/react-native-kb/ios/Kb.mm @@ -49,7 +49,6 @@ + (id)sharedFsPathsHolder { static std::mutex kbSharedInstanceMutex; static BOOL kbPasteImageEnabled = NO; static NSString *kbStoredDeviceToken = nil; -static NSDictionary *kbInitialNotification = nil; // The bridge is created on the JS thread and consumed by the reader thread, // so every access goes through this lock — a plain shared_ptr member would be @@ -496,21 +495,6 @@ - (void)installJSIBindingsWithRuntime:(jsi::Runtime &)runtime RCT_EXPORT_METHOD(shareListenersRegistered) { } -// No current caller (kept for future use). -RCT_EXPORT_METHOD(engineReset) { - NSError *error = nil; - KeybaseReset(&error); - if (auto bridge = kbGetBridge()) { - bridge->resetRecv(); - } - if ([self canEmit]) { - [self emitOnMetaEvent:metaEventEngineReset]; - } - if (error) { - NSLog(@"Error in reset: %@", error); - } -} - RCT_EXPORT_METHOD(notifyJSReady) { // KeybaseNotifyJSReady is a sync.Once on the Go side, so repeat calls after // a reload are free. It must not run on the JS thread — do it on the reader @@ -798,16 +782,6 @@ - (void)installJSIBindingsWithRuntime:(jsi::Runtime &)runtime }); } -RCT_EXPORT_METHOD(getInitialNotification: (RCTPromiseResolveBlock)resolve reject: (RCTPromiseRejectBlock)reject) { - if (kbInitialNotification) { - NSDictionary *notification = kbInitialNotification; - kbInitialNotification = nil; - resolve(notification); - } else { - resolve([NSNull null]); - } -} - RCT_EXPORT_METHOD(removeAllPendingNotificationRequests) { UNUserNotificationCenter *current = UNUserNotificationCenter.currentNotificationCenter; [current removeAllPendingNotificationRequests]; @@ -892,20 +866,6 @@ + (void)setDeviceToken:(NSString *)token { }); } -+ (void)setInitialNotification:(NSDictionary *)notification { - kbInitialNotification = notification; -} - -+ (void)emitPushNotification:(NSDictionary *)notification { - Kb *instance = kbSharedInstance; - if (instance && [instance canEmit]) { - [instance emitOnPushNotification:notification]; - NSLog(@"Kb.emitPushNotification: sent event 'onPushNotification' to JS"); - } else { - NSLog(@"Kb.emitPushNotification: WARNING - module not ready, event not sent"); - } -} - - (void)handleHardwareKeyPressed:(NSNotification *)notification { NSString *keyName = notification.userInfo[@"pressedKey"]; if (keyName && [self canEmit]) { @@ -951,33 +911,3 @@ - (void)kb_paste:(id)sender { void KbSetDeviceToken(NSString *token) { [Kb setDeviceToken:token]; } - -void KbSetInitialNotification(NSDictionary *notification) { - [Kb setInitialNotification:notification]; -} - -void KbEmitPushNotification(NSDictionary *notification) { - [Kb emitPushNotification:notification]; -} - -void KbEmitStoredNotificationOnBecomeActive(void) { - NSDictionary *stored = kbInitialNotification; - kbInitialNotification = nil; - if (!stored) { - NSLog(@"KbEmitStoredNotificationOnBecomeActive: no stored notification"); - return; - } - if (![stored[@"userInteraction"] boolValue]) { - // Not from a user tap; nothing to re-emit. - return; - } - if ([stored[@"reEmittedInBecomeActive"] boolValue]) { - // Already re-emitted once; keep it stored for getInitialNotification. - kbInitialNotification = stored; - return; - } - [Kb emitPushNotification:stored]; - NSMutableDictionary *copy = [stored mutableCopy]; - copy[@"reEmittedInBecomeActive"] = @YES; - kbInitialNotification = copy; -} diff --git a/rnmodules/react-native-kb/src/NativeKb.ts b/rnmodules/react-native-kb/src/NativeKb.ts index 86133a5402e1..dc12a7416c51 100644 --- a/rnmodules/react-native-kb/src/NativeKb.ts +++ b/rnmodules/react-native-kb/src/NativeKb.ts @@ -1,11 +1,10 @@ import {TurboModuleRegistry, type TurboModule} from 'react-native' -import type {EventEmitter, UnsafeObject} from 'react-native/Libraries/Types/CodegenTypes' +import type {EventEmitter} from 'react-native/Libraries/Types/CodegenTypes' export interface Spec extends TurboModule { readonly onMetaEvent: EventEmitter readonly onHardwareKeyPressed: EventEmitter readonly onPasteImage: EventEmitter> - readonly onPushNotification: EventEmitter readonly onPushToken: EventEmitter readonly onShareData: EventEmitter<{text?: string; localPaths?: Array}> getTypedConstants(): { @@ -60,10 +59,8 @@ export interface Spec extends TurboModule { requestPushPermissions(): Promise getRegistrationToken(): Promise setApplicationIconBadgeNumber(n: number): void - getInitialNotification(): Promise removeAllPendingNotificationRequests(): void addNotificationRequest(config: {body: string; id: string}): Promise - engineReset(): void notifyJSReady(): void shareListenersRegistered(): void setEnablePasteImage(enabled: boolean): void diff --git a/rnmodules/react-native-kb/src/index.tsx b/rnmodules/react-native-kb/src/index.tsx index aeccfb245834..c5560f4960d6 100644 --- a/rnmodules/react-native-kb/src/index.tsx +++ b/rnmodules/react-native-kb/src/index.tsx @@ -97,10 +97,6 @@ export const setApplicationIconBadgeNumber = (n: number): void => { Kb.setApplicationIconBadgeNumber(n) } -export const getInitialNotification = (): Promise => { - return Kb.getInitialNotification() -} - export const removeAllPendingNotificationRequests = (): void => { Kb.removeAllPendingNotificationRequests() } @@ -143,9 +139,6 @@ export const onMetaEvent = (callback: (payload: string) => void): EventSubscript } // Push events -export const onPushNotification = (callback: (notification: object) => void): EventSubscription => { - return Kb.onPushNotification(n => callback(n)) -} export const onPushToken = (callback: (token: string) => void): EventSubscription => { return Kb.onPushToken(callback) @@ -158,16 +151,12 @@ export const onShareData = ( return Kb.onShareData(callback) } -export const engineReset = (): void => { - return Kb.engineReset() -} export const notifyJSReady = (): void => { return Kb.notifyJSReady() } export const shareListenersRegistered = (): void => { return Kb.shareListenersRegistered() } - export const clearLocalLogs = (): Promise => { return Kb.clearLocalLogs() } diff --git a/shared/android/app/src/main/AndroidManifest.xml b/shared/android/app/src/main/AndroidManifest.xml index b790337d942f..0e94d6d8ea59 100644 --- a/shared/android/app/src/main/AndroidManifest.xml +++ b/shared/android/app/src/main/AndroidManifest.xml @@ -74,6 +74,15 @@ + + diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt index 1eed346fdc0b..8c32ae7f8ac2 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt @@ -12,8 +12,6 @@ import android.graphics.PorterDuffXfermode import android.graphics.Rect import android.net.Uri import android.os.Bundle -import android.os.Handler -import android.os.Looper import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat import androidx.core.app.Person @@ -22,9 +20,9 @@ import androidx.core.graphics.drawable.IconCompat import keybase.ChatNotification import keybase.PushNotifier import java.io.BufferedInputStream -import java.io.IOException import java.net.HttpURLConnection import java.net.URL +import java.security.MessageDigest class KBPushNotifier internal constructor(private val context: Context, private val bundle: Bundle) : PushNotifier { private var convMsgCache: SmallMsgRingBuffer? = null @@ -38,17 +36,39 @@ class KBPushNotifier internal constructor(private val context: Context, private this.convMsgCache = convMsgCache } - // Controls the Intent that gets built - private fun buildPendingIntent(bundle: Bundle): PendingIntent { - val open_activity_intent = Intent(context, MainActivity::class.java) - open_activity_intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) - open_activity_intent.setPackage(context.packageName) - open_activity_intent.putExtra("notification", bundle) + // A tap goes through PushTapActivity, which hands the push to the service. The payload rides + // in the extras, and the data is a digest of it: PendingIntent.getActivity hands back an + // existing PendingIntent for any Intent that filterEquals the new one, and extras are not part + // of filterEquals, so two notifications with different payloads must differ in the data or the + // second tap would open the first one's target. A digest rather than the payload itself + // because a data URI is printed by `dumpsys activity`, where an extra is not. Immutable, so + // whoever holds this PendingIntent can't substitute another payload. + // + // The whole push goes in rather than a projection of it, since which fields matter is the + // service's business. A push is a few hundred bytes against the ~1MB a Binder transaction + // allows, but it is the sender who decides how big, so a payload that grows without bound is + // the thing that would break this. + private fun tapIntent(bundle: Bundle): Intent = + Intent(context, PushTapActivity::class.java) + .setData(Uri.parse("kbpushtap:" + payloadDigest(bundle))) + .putExtras(bundle) + .setFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - // unique so our intents are deduped, else it'll reuse old ones - return PendingIntent.getActivity(context, (System.currentTimeMillis() / 1000).toInt(), open_activity_intent, PendingIntent.FLAG_MUTABLE) + private fun payloadDigest(bundle: Bundle): String { + val digest = MessageDigest.getInstance("SHA-256") + for (key in bundle.keySet().sorted()) { + @Suppress("DEPRECATION") + val value = bundle.get(key)?.toString() ?: "" + // Length-prefixed so no pair of keys and values can run together into the same digest + // input as a different pair would. + digest.update("${key.length}:$key${value.length}:$value".toByteArray()) + } + return digest.digest().joinToString("") { "%02x".format(it) } } + private fun buildPendingIntent(bundle: Bundle): PendingIntent = + PendingIntent.getActivity(context, 0, tapIntent(bundle), PendingIntent.FLAG_IMMUTABLE) + private fun getKeybaseAvatar(avatarUri: String): IconCompat? { if (avatarUri.isEmpty()) return null @@ -105,7 +125,6 @@ class KBPushNotifier internal constructor(private val context: Context, private private fun displayChatNotification2(chatNotification: ChatNotification) { try { KeybasePushNotificationListenerService.createNotificationChannel(context) - bundle.putBoolean("userInteraction", true) bundle.putString("type", "chat.newmessage") bundle.putString("convID", chatNotification.convID) if (chatNotification.uid.isNotEmpty()) { @@ -179,7 +198,6 @@ class KBPushNotifier internal constructor(private val context: Context, private fun followNotification(username: String, notificationMsg: String?) { val bundle = bundle.clone() as Bundle - bundle.putBoolean("userInteraction", true) bundle.putString("type", "follow") bundle.putString("username", username) val builder = NotificationCompat.Builder(context, KeybasePushNotificationListenerService.FOLLOW_CHANNEL_ID) @@ -203,7 +221,6 @@ class KBPushNotifier internal constructor(private val context: Context, private } fun genericNotification(uniqueTag: String?, notificationTitle: String?, notificationMsg: String?, bundle: Bundle, channelID: String?) { - bundle.putBoolean("userInteraction", true) val builder = NotificationCompat.Builder(context, channelID!!) .setSmallIcon(R.drawable.ic_notif) // Set the intent that will fire when the user taps the notification .setContentIntent(buildPendingIntent(bundle)) diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt index 15d05055970a..88c60b92786f 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt @@ -5,17 +5,13 @@ import android.app.NotificationManager import android.content.Context import android.os.Build import android.os.Bundle -import android.os.Handler -import android.os.Looper import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat -import androidx.core.app.Person import com.google.firebase.messaging.FirebaseMessagingService import com.google.firebase.messaging.RemoteMessage import io.keybase.ossifrage.MainActivity.Companion.setupKBRuntime import io.keybase.ossifrage.modules.NativeLogger import keybase.Keybase -import com.reactnativekb.KbModule import org.json.JSONObject class KeybasePushNotificationListenerService : FirebaseMessagingService() { @@ -33,17 +29,6 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { return "$targetUID|$convID|$messageId" } - private fun buildStyle(convID: String, person: Person): NotificationCompat.Style { - val style = NotificationCompat.MessagingStyle(person) - val buf = msgCache[convID] - if (buf != null) { - for (msg in buf.summary()) { - style.addMessage(msg) - } - } - return style - } - override fun onCreate() { setupKBRuntime(this, false) NativeLogger.info("KeybasePushNotificationListenerService created") @@ -140,23 +125,12 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { n.badgeCount.toLong(), n.unixTime, n.soundName, if (dontNotify) null else notifier, true, targetUID, KBPushNotifier(applicationContext, Bundle())) goProcessingSucceeded = true - if (!dontNotify) { - seenChatNotifications[chatNotificationKey(n.convID, n.messageId, targetUID)] = Unit - } } catch (ex: Exception) { NativeLogger.error("Go couldn't handle background notification: " + ex.message) } } - val isReactNativeRunning = try { - com.reactnativekb.KbModule.isReactNativeRunning() - } catch (e: Exception) { - NativeLogger.info("KeybasePushNotificationListenerService couldn't check if React Native is running: ${e.message}, assuming not") - false - } - NativeLogger.info("KeybasePushNotificationListenerService isReactNativeRunning: $isReactNativeRunning") - val isForeground = try { Keybase.isAppStateForeground() } catch (e: Exception) { @@ -165,14 +139,9 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { } NativeLogger.info("KeybasePushNotificationListenerService isForeground: $isForeground") - // Don't show notifications if app is foreground - user is already looking at the app - if (isForeground) { - - } else if (dontNotify) { - // Silent notifications should never display - they're processed by Go but no notification shown - } else if (!goProcessingSucceeded && type == "chat.newmessage") { - // Only show fallback if Go processing failed AND it's a non-silent notification - // If Go succeeded, it already displayed the notification (via notifier parameter) + // In the foreground the app already has the message. A silent push never + // displays. Otherwise fall back only if Go failed to display it itself. + if (!isForeground && !dontNotify && !goProcessingSucceeded) { NativeLogger.info("KeybasePushNotificationListenerService attempting fallback notification display") try { val chatNotif = keybase.ChatNotification() @@ -198,20 +167,12 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { chatNotif.uid = targetUID notifier.displayChatNotification(chatNotif) - seenChatNotifications[chatNotificationKey(n.convID, n.messageId, targetUID)] = Unit NativeLogger.info("KeybasePushNotificationListenerService fallback notification displayed successfully") } catch (e: Exception) { NativeLogger.error("Failed to display notification fallback: " + e.message) } - } else if (dontNotify) { - } - if (type == "chat.newmessage") { - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) - } } "follow" -> { @@ -219,18 +180,11 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { val m = bundle.getString("message") if (username != null && m != null) { notifier.followNotification(username, m) - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) - } else { } } "device.revoked", "device.new" -> { notifier.deviceNotification() - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) } "chat.readmessage" -> { @@ -247,15 +201,10 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { val notificationManager = NotificationManagerCompat.from(applicationContext) notificationManager.cancelAll() } - val emitBundle = bundle.clone() as Bundle - KbModule.emitPushNotification(emitBundle) } else -> { notifier.generalNotification() - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) } } } catch (ex: Exception) { @@ -390,4 +339,3 @@ internal class NotificationData(type: String, bundle: Bundle) { } } } - diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt index 96e85fae4b72..8509dfeba6d5 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt @@ -16,7 +16,6 @@ import androidx.core.content.IntentCompat import android.webkit.MimeTypeMap import com.facebook.react.ReactActivity import com.facebook.react.ReactActivityDelegate -import com.facebook.react.ReactApplication import com.facebook.react.bridge.Arguments import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint.fabricEnabled import com.facebook.react.defaults.DefaultReactActivityDelegate @@ -155,9 +154,9 @@ class MainActivity : ReactActivity() { (application as MainApplication).lifecycleReporter.onMainActivityDestroy(isFinishing, isChangingConfigurations) } - // A share or notification intent parks here until JS asks for it. Nothing else is parked: - // deep links go through super.onNewIntent -> RCTLinkingManager, so a plain launch leaves - // this null. + // A share intent parks here until JS asks for it. Nothing else is parked: deep links go + // through super.onNewIntent -> RCTLinkingManager, and a notification tap goes to the + // service, so a plain launch leaves this null. private var cachedIntent: Intent? = null private var pendingShareUris: List? = null @@ -168,27 +167,20 @@ class MainActivity : ReactActivity() { // data are tied to the delivered intent, and JS may not be ready to route them until much // later (see shareListenersRegistered). private fun captureIntent(intent: Intent) { - val bundleFromNotification = intent.getBundleExtra("notification") - if (bundleFromNotification != null) { - KbModule.setInitialNotification(bundleFromNotification.clone() as Bundle) - } - val isShare = Intent.ACTION_SEND == intent.action || Intent.ACTION_SEND_MULTIPLE == intent.action - if (!isShare && bundleFromNotification == null) { + if (Intent.ACTION_SEND != intent.action && Intent.ACTION_SEND_MULTIPLE != intent.action) { return } cachedIntent = intent - if (isShare) { - pendingShareUris = extractSharedUris(intent) - pendingShareSubject = intent.getStringExtra(Intent.EXTRA_SUBJECT) - pendingShareText = intent.getStringExtra(Intent.EXTRA_TEXT) - } + pendingShareUris = extractSharedUris(intent) + pendingShareSubject = intent.getStringExtra(Intent.EXTRA_SUBJECT) + pendingShareText = intent.getStringExtra(Intent.EXTRA_TEXT) } override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setIntent(intent) captureIntent(intent) - NativeLogger.info("MainActivity.onNewIntent: action=${intent.action}, uriCount=${pendingShareUris?.size ?: 0}, hasNotification=${intent.getBundleExtra("notification") != null}") + NativeLogger.info("MainActivity.onNewIntent: action=${intent.action}, uriCount=${pendingShareUris?.size ?: 0}") } private var jsIsListening = false @@ -200,8 +192,6 @@ class MainActivity : ReactActivity() { handleIntent() } - private var handledIntentHash: String? = null - private fun extractSharedUris(intent: Intent): List { val action = intent.action if (Intent.ACTION_SEND != action && Intent.ACTION_SEND_MULTIPLE != action) { @@ -238,72 +228,48 @@ class MainActivity : ReactActivity() { if (!jsIsListening) return NativeLogger.info("MainActivity.handleIntent: processing intent action=${intent.action}") - // Here we are just reading from the notification bundle. - // If other sources start the app, we can get their intent data the same way. - val bundleFromNotification = intent.getBundleExtra("notification") - - if (bundleFromNotification != null) { - // Prevent duplicate handling of the same notification - val convID = bundleFromNotification.getString("convID") ?: bundleFromNotification.getString("c") - val messageId = bundleFromNotification.getString("msgID") ?: bundleFromNotification.getString("d") ?: "" - val intentHash = "${convID}_${messageId}" - if (handledIntentHash == intentHash) { - NativeLogger.info("MainActivity.handleIntent skipping duplicate notification: $intentHash") - } else { - handledIntentHash = intentHash - NativeLogger.info("MainActivity.handleIntent processing notification: $intentHash") - - KbModule.emitPushNotification(bundleFromNotification) + val uris = pendingShareUris.orEmpty().also { pendingShareUris = null } + val subject = pendingShareSubject.also { pendingShareSubject = null } + val text = pendingShareText.also { pendingShareText = null } + + // Strip consumed extras so an activity recreation (which redelivers this + // same intent instance) doesn't re-share. + intent.removeExtra(Intent.EXTRA_STREAM) + intent.removeExtra(Intent.EXTRA_SUBJECT) + intent.removeExtra(Intent.EXTRA_TEXT) + intent.setClipData(null) + + val textPayload = listOfNotNull(subject, text).joinToString(" ") + val isTextMime = intent.type?.startsWith("text/") == true + + if (isTextMime && textPayload.isNotEmpty()) { + // Text-type intent (e.g. URL from Chrome): prefer text over any preview images + emitShareText(text ?: textPayload) + } else if (uris.isEmpty()) { + if (textPayload.isNotEmpty()) { + emitShareText(textPayload) } - - intent.removeExtra("notification") - } - - val action = intent.action - if (Intent.ACTION_SEND == action || Intent.ACTION_SEND_MULTIPLE == action) { - val uris = pendingShareUris.orEmpty().also { pendingShareUris = null } - val subject = pendingShareSubject.also { pendingShareSubject = null } - val text = pendingShareText.also { pendingShareText = null } - - // Strip consumed extras so an activity recreation (which redelivers this - // same intent instance) doesn't re-share. - intent.removeExtra(Intent.EXTRA_STREAM) - intent.removeExtra(Intent.EXTRA_SUBJECT) - intent.removeExtra(Intent.EXTRA_TEXT) - intent.setClipData(null) - - val textPayload = listOfNotNull(subject, text).joinToString(" ") - val isTextMime = intent.type?.startsWith("text/") == true - - if (isTextMime && textPayload.isNotEmpty()) { - // Text-type intent (e.g. URL from Chrome): prefer text over any preview images - emitShareText(text ?: textPayload) - } else if (uris.isEmpty()) { - if (textPayload.isNotEmpty()) { + } else { + // Copying out of the content providers can be slow for big files; don't + // block the main thread on it. + val context: Context = this + Thread { + val filePaths = uris.mapNotNull { uri -> + try { + readFileFromUri(context, uri) + } catch (e: SecurityException) { + null + } + } + if (filePaths.isNotEmpty()) { + emitShareFiles(filePaths) + } else if (textPayload.isNotEmpty()) { + // Fallback: non-text MIME but no files resolved, send text emitShareText(textPayload) + } else { + emitShareFiles(emptyList()) } - } else { - // Copying out of the content providers can be slow for big files; don't - // block the main thread on it. - val context: Context = this - Thread { - val filePaths = uris.mapNotNull { uri -> - try { - readFileFromUri(context, uri) - } catch (e: SecurityException) { - null - } - } - if (filePaths.isNotEmpty()) { - emitShareFiles(filePaths) - } else if (textPayload.isNotEmpty()) { - // Fallback: non-text MIME but no files resolved, send text - emitShareText(textPayload) - } else { - emitShareFiles(emptyList()) - } - }.start() - } + }.start() } cachedIntent = null diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt new file mode 100644 index 000000000000..806aef117b72 --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt @@ -0,0 +1,53 @@ +package io.keybase.ossifrage + +import android.app.Activity +import android.content.Intent +import android.os.Bundle +import io.keybase.ossifrage.MainActivity.Companion.setupKBRuntime +import io.keybase.ossifrage.modules.NativeLogger +import keybase.Keybase +import kotlin.concurrent.thread +import org.json.JSONObject + +// Opens the app for a tapped notification. Not exported, so only this app's own notification +// PendingIntents can start it: the payload it hands the service, which may name an account to +// switch to, can't come from another app. MainActivity, which any app can start, never reads it. +class PushTapActivity : Activity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + // Read the Intent here and deliver off the main thread: a tap can be what starts this + // process, and the initOnce below is a known slow path (leveldb, keychain) while this + // activity is Theme.NoDisplay and must finish before onResume. Nothing is racing the app + // coming up: a delivery that lands after the client connected is picked up by the service's + // nudge, one that lands before it by the peek the client does on connect. + val payload = runCatching { payloadJSON(intent.extras) }.getOrElse { + // An empty payload still opens the app, but it opens it nowhere in particular, so the + // tap has to leave a trace rather than vanish. + NativeLogger.error("PushTapActivity: could not read a tap payload", it) + "{}" + } + val context = applicationContext + thread(start = true) { + runCatching { + setupKBRuntime(context, false) + Keybase.deliverPushTap(payload) + }.onFailure { NativeLogger.error("PushTapActivity: failed to deliver a tap", it) } + } + startActivity( + Intent(this, MainActivity::class.java) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) + ) + finish() + } + + // The push as it arrived, as JSON, which is the shape the service parses. Nothing is picked + // out of it here: which fields matter is the service's business. + private fun payloadJSON(extras: Bundle?): String { + val json = JSONObject() + extras?.keySet()?.forEach { key -> + @Suppress("DEPRECATION") + json.put(key, extras.get(key)?.toString() ?: "") + } + return json.toString() + } +} diff --git a/shared/ios/Keybase/AppDelegate.swift b/shared/ios/Keybase/AppDelegate.swift index 08603d0d7a33..2d95db3543ff 100644 --- a/shared/ios/Keybase/AppDelegate.swift +++ b/shared/ios/Keybase/AppDelegate.swift @@ -38,11 +38,6 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi self.didLaunchSetupBefore() - if let remoteNotification = launchOptions?[.remoteNotification] as? [AnyHashable: Any] { - let notificationDict = Dictionary(uniqueKeysWithValues: remoteNotification.map { (String(describing: $0.key), $0.value) }) - KbSetInitialNotification(notificationDict) - } - NotificationCenter.default.addObserver(forName: UIApplication.didReceiveMemoryWarningNotification, object: nil, queue: .main) { [weak self] notification in log.info("Memory warning received - deferring GC during React Native initialization") // see if this helps avoid this crash @@ -126,20 +121,19 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi logQueue.async { [weak self] in guard let self else { return } if self.startupLogFileHandle == nil { - if !FileManager.default.fileExists(atPath: logFilePath) { - FileManager.default.createFile( - atPath: logFilePath, - contents: nil, - attributes: [.protectionKey: FileProtectionType.completeUntilFirstUserAuthentication] - ) - } - if let fileHandle = FileHandle(forWritingAtPath: logFilePath) { - fileHandle.seekToEndOfFile() - self.startupLogFileHandle = fileHandle - } else { - NSLog("Error opening startup timing log file: \(logFilePath)") + // Go's logger opens this same file during KeybaseInit, so share it instead of replacing + // it: createFile swaps in a new file by renaming, which leaves Go logging the whole + // session to an unlinked file, and a non-append handle writes over Go's lines. + let fd = open(logFilePath, O_WRONLY | O_CREAT | O_APPEND, 0o600) + guard fd >= 0 else { + NSLog("Error opening startup timing log file: \(logFilePath) errno=\(errno)") return } + try? FileManager.default.setAttributes( + [.protectionKey: FileProtectionType.completeUntilFirstUserAuthentication], + ofItemAtPath: logFilePath + ) + self.startupLogFileHandle = FileHandle(fileDescriptor: fd, closeOnDealloc: true) } guard let fileHandle = self.startupLogFileHandle else { return } do { @@ -318,11 +312,8 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi } override func application(_ application: UIApplication, didReceiveRemoteNotification notification: [AnyHashable: Any], fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) { - guard let type = notification["type"] as? String else { - completionHandler(.noData) - return - } - if type == "chat.newmessageSilent_2" { + switch notification["type"] as? String { + case "chat.newmessageSilent_2": DispatchQueue.global(qos: .default).async { let convID = notification["c"] as? String let messageID = (notification["d"] as? NSNumber)?.intValue ?? 0 @@ -345,33 +336,46 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi completionHandler(.newData) log.info("Remote notification handle finished...") } - } else { - var notificationDict = Dictionary(uniqueKeysWithValues: notification.map { (String(describing: $0.key), $0.value) }) - notificationDict["userInteraction"] = false - KbEmitPushNotification(notificationDict) + case "chat.readmessage": + Self.clearPendingNotificationsIfAllRead(notification) completionHandler(.newData) + default: + completionHandler(.noData) } } + // A read receipt that leaves this account with nothing unread clears the notification + // requests still waiting to show. + private static func clearPendingNotificationsIfAllRead(_ notification: [AnyHashable: Any]) { + let badge = (notification["b"] as? NSNumber)?.intValue ?? Int(notification["b"] as? String ?? "") ?? -1 + guard badge == 0 else { return } + let target = notification["i"] as? String ?? "" + DispatchQueue.global(qos: .default).async { + guard target.isEmpty || target == Keybasego.KeybaseCurrentUID() else { return } + UNUserNotificationCenter.current().removeAllPendingNotificationRequests() + } + } + + // The only way a tap reaches the service. UIKit calls this only for a notification + // delivered to this app; URLs other apps open go through Linking instead, so only real + // taps can carry an account. The payload goes over unread: the service resolves where it + // opens, and nothing here or in JS parses a push. public func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) { let userInfo = response.notification.request.content.userInfo - var notificationDict = Dictionary(uniqueKeysWithValues: userInfo.map { (String(describing: $0.key), $0.value) }) - notificationDict["userInteraction"] = true - - // Store the notification so it can be processed when app becomes active - // This ensures navigation works even if React Native isn't ready yet - KbSetInitialNotification(notificationDict) - - // Also emit immediately in case React Native is ready - KbEmitPushNotification(notificationDict) + // uniquingKeysWith, not uniqueKeysWithValues: the latter traps on a duplicate key, and + // String(describing:) over [AnyHashable: Any] can in principle produce one. + let payload = Dictionary(userInfo.map { (String(describing: $0.key), $0.value) }, uniquingKeysWith: { first, _ in first }) + if JSONSerialization.isValidJSONObject(payload), + let data = try? JSONSerialization.data(withJSONObject: payload), + let json = String(data: data, encoding: .utf8) { + Keybasego.KeybaseDeliverPushTap(json) + } else { + log.error("Dropped a notification tap: its payload could not be serialized") + } completionHandler() } public func userNotificationCenter(_ center: UNUserNotificationCenter, willPresent notification: UNNotification, withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void) { - let userInfo = notification.request.content.userInfo - var notificationDict = Dictionary(uniqueKeysWithValues: userInfo.map { (String(describing: $0.key), $0.value) }) - notificationDict["userInteraction"] = false - KbEmitPushNotification(notificationDict) completionHandler([]) } @@ -414,9 +418,6 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi log.info("applicationDidBecomeActive: hiding keyz screen.") hideCover() lifecycle.didBecomeActive() - - // Re-emit a notification the user tapped while React Native wasn't ready yet. - KbEmitStoredNotificationOnBecomeActive() } override func applicationWillEnterForeground(_ application: UIApplication) { From c3ecf46a89996eda1bef93a83ba2366201f7c073 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 14:17:13 -0400 Subject: [PATCH 4/6] feat(js): route a tapped notification from the service, not from a payload The client peeks the service's resolved route on connect and on pushTapRouteAvailable, and queues it as a navigation intent. The router owns consumption, so a tap for another account waits for account-link-switch and the intent survives the store reset the switch performs; whatever consumes the intent -- navigating it, the linking config taking it as the startup route, or giving up on it as stale -- is what acks the tap with the service. That makes a tap exactly-once end to end. With no push payload reaching JS any more, the push normalizer, the pending-push-notification machinery and constants/types/push all go, and config.startup.followUser with them: a follow tap is now just a profile URL. --- shared/constants/init/index.tsx | 84 ++-- shared/constants/init/platform.desktop.tsx | 2 +- .../constants/init/push-listener.native.tsx | 410 ++++-------------- shared/constants/init/push-tap.test.ts | 272 ++++++++++++ shared/constants/init/shared.tsx | 30 ++ shared/constants/types/index.tsx | 1 - shared/constants/types/push.tsx | 52 --- shared/router-v2/account-link-switch.test.ts | 165 +++++++ shared/router-v2/account-link-switch.tsx | 66 +++ shared/router-v2/deep-link-emitter.test.ts | 45 +- shared/router-v2/deep-link-emitter.tsx | 36 +- shared/router-v2/intent-consumption.test.ts | 39 +- shared/router-v2/linking-initial-url.test.ts | 47 +- shared/router-v2/linking.test.ts | 17 +- shared/router-v2/linking.tsx | 25 +- shared/stores/config.tsx | 4 - shared/stores/navigation-intents.test.ts | 162 +++++++ shared/stores/navigation-intents.tsx | 103 ++++- shared/stores/push.tsx | 176 +------- shared/stores/tests/config.test.ts | 11 +- shared/stores/tests/push.desktop.test.ts | 1 - 21 files changed, 1090 insertions(+), 658 deletions(-) create mode 100644 shared/constants/init/push-tap.test.ts delete mode 100644 shared/constants/types/push.tsx create mode 100644 shared/router-v2/account-link-switch.test.ts create mode 100644 shared/router-v2/account-link-switch.tsx diff --git a/shared/constants/init/index.tsx b/shared/constants/init/index.tsx index 505e7c24b795..af9c148bef3e 100644 --- a/shared/constants/init/index.tsx +++ b/shared/constants/init/index.tsx @@ -152,7 +152,6 @@ const onChatClearWatch = async () => { const loadStartupDetails = async () => { logger.info('[Startup] loadStartupDetails: starting') const {guiConfig, Linking} = _getNative() - const {getStartupDetailsFromInitialPush} = await import('./push-listener.native') let routeState = '' try { @@ -160,36 +159,29 @@ const loadStartupDetails = async () => { routeState = config?.ui?.routeState2 ?? '' } catch {} - const [initialUrl, push] = await Promise.all([ - neverThrowPromiseFunc(async () => { - const linkingStart = Date.now() - logger.info('[Startup] loadStartupDetails: calling Linking.getInitialURL') - const url = await Linking.getInitialURL() - const elapsed = Date.now() - linkingStart - if (url === null) { - logger.warn(`[Startup] loadStartupDetails: Linking.getInitialURL returned null in ${elapsed}ms`) - } else { - logger.info(`[Startup] loadStartupDetails: Linking.getInitialURL returned in ${elapsed}ms: ${url}`) - } - return url - }), - neverThrowPromiseFunc(getStartupDetailsFromInitialPush), - ] as const) + // A tapped push doesn't pass through here: the service resolves it and constants/init/shared + // takes it, queuing it as a navigation intent. + const initialUrl = await neverThrowPromiseFunc(async () => { + const linkingStart = Date.now() + logger.info('[Startup] loadStartupDetails: calling Linking.getInitialURL') + const url = await Linking.getInitialURL() + const elapsed = Date.now() - linkingStart + if (url === null) { + logger.warn(`[Startup] loadStartupDetails: Linking.getInitialURL returned null in ${elapsed}ms`) + } else { + logger.info(`[Startup] loadStartupDetails: Linking.getInitialURL returned in ${elapsed}ms: ${url}`) + } + return url + }) let conversation: T.Chat.ConversationIDKey | undefined let conversationUid = '' - let followUser = '' let tab = '' - // Top priority, push - if (push) { - logger.info('initialState: push', push.startupConversation, push.startupFollowUser) - conversation = push.startupConversation - followUser = push.startupFollowUser ?? '' - } else if (!initialUrl && routeState) { - // Last priority, saved from last session. The linking config reads the launch URL - // itself; this read only decides whether the saved route may be restored, since a - // launch URL outranks it. + // The linking config reads the launch URL itself; this read only decides whether the + // saved route may be restored, since a launch URL outranks it. + if (!initialUrl && routeState) { + // Last priority, saved from last session try { const item = JSON.parse(routeState) as | undefined @@ -221,7 +213,6 @@ const loadStartupDetails = async () => { useConfigState.getState().dispatch.setStartupDetails({ conversation: conversation ?? noConversationIDKey, conversationUid, - followUser, tab: tab as Tabs.Tab, }) @@ -361,7 +352,11 @@ export const initPlatformListener = () => { } const _initNativePlatformListener = () => { - useShellState.subscribe((s, old) => { + // HMR cleanup: unsubscribe old store subscriptions before re-subscribing + for (const unsub of _platformUnsubs) unsub() + _platformUnsubs.length = 0 + + _platformUnsubs.push(useShellState.subscribe((s, old) => { if (s.mobileAppState === old.mobileAppState) return if (s.mobileAppState === 'background') { persistRoute(false, true, () => useConfigState.getState().startup.loaded) @@ -376,7 +371,7 @@ const _initNativePlatformListener = () => { // only reload on foreground useSettingsContactsState.getState().dispatch.loadContactPermissions() } - }) + })) const configureAndroidCacheDir = () => { const {fsCacheDir, fsDownloadDir} = _getNativeSync() @@ -399,7 +394,7 @@ const _initNativePlatformListener = () => { } } - useConfigState.subscribe((s, old) => { + _platformUnsubs.push(useConfigState.subscribe((s, old) => { if (s.loggedIn === old.loggedIn) return const f = async () => { const {NetInfo} = _getNative() @@ -411,9 +406,9 @@ const _initNativePlatformListener = () => { ) } ignorePromise(f()) - }) + })) - useShellState.subscribe((s, old) => { + _platformUnsubs.push(useShellState.subscribe((s, old) => { if (s.networkStatus === old.networkStatus) return const type = s.networkStatus?.type if (!type) return @@ -425,19 +420,19 @@ const _initNativePlatformListener = () => { } } ignorePromise(f()) - }) + })) if (isAndroid) { - useDarkModeState.subscribe((s, old) => { + _platformUnsubs.push(useDarkModeState.subscribe((s, old) => { if (s.darkModePreference === old.darkModePreference) return const {androidAppColorSchemeChanged} = _getNativeSync() androidAppColorSchemeChanged(s.darkModePreference) - }) + })) } // we call this when we're logged in. let calledShareListenersRegistered = false - useRouterState.subscribe((s, old) => { + _platformUnsubs.push(useRouterState.subscribe((s, old) => { const next = s.navState const prev = old.navState if (next === prev) return @@ -448,13 +443,13 @@ const _initNativePlatformListener = () => { const {shareListenersRegistered} = _getNativeSync() shareListenersRegistered() } - }) + })) // Default to screen capture prevention on Android (matches native default of secure). // Once daemon is ready, sync with the user's saved preference. if (isAndroid) { ignorePromise(ScreenCapture.preventScreenCaptureAsync('screenprotector')) - useDaemonState.subscribe((s, old) => { + _platformUnsubs.push(useDaemonState.subscribe((s, old) => { if (s.handshakeState !== 'done' || old.handshakeState === 'done') return const f = async () => { const {getSecureFlagSetting} = await import('@/constants/platform') @@ -465,20 +460,22 @@ const _initNativePlatformListener = () => { } } ignorePromise(f()) - }) + })) } // Start this immediately instead of waiting so we can do more things in parallel ignorePromise(loadStartupDetails()) - initPushListener() + _platformUnsubs.push(...initPushListener()) ignorePromise(unregisterLegacyIOSLocationTask()) const {NetInfo} = _getNative() - NetInfo.addEventListener(({type}) => { - useShellState.getState().dispatch.osNetworkStatusChanged(type !== NetInfo.NetInfoStateType.none, type) - }) + _platformUnsubs.push( + NetInfo.addEventListener(({type}) => { + useShellState.getState().dispatch.osNetworkStatusChanged(type !== NetInfo.NetInfoStateType.none, type) + }) + ) const {setupAudioMode} = _getNative() ignorePromise(setupAudioMode(false)) @@ -597,7 +594,6 @@ const _initDesktopPlatformListener = () => { if (s.handshakeState !== old.handshakeState && s.handshakeState === 'done') { useConfigState.getState().dispatch.setStartupDetails({ conversation: Chat.noConversationIDKey, - followUser: '', tab: undefined, }) } diff --git a/shared/constants/init/platform.desktop.tsx b/shared/constants/init/platform.desktop.tsx index 9bd3e66f119f..f3787c83d33c 100644 --- a/shared/constants/init/platform.desktop.tsx +++ b/shared/constants/init/platform.desktop.tsx @@ -17,7 +17,7 @@ export const getDesktop = (): DesktopModules => export {maybePauseVideos, setupWindowEventListeners} from './desktop-dom-helpers.desktop' // push notifications are native-only. -export const initPushListener = (): void => {} +export const initPushListener = (): Array<() => void> => [] const notOnDesktop = (name: string): never => { throw new Error(`init/${name} called on desktop`) diff --git a/shared/constants/init/push-listener.native.tsx b/shared/constants/init/push-listener.native.tsx index 5d1d83bd6346..2313646d3ed8 100644 --- a/shared/constants/init/push-listener.native.tsx +++ b/shared/constants/init/push-listener.native.tsx @@ -1,14 +1,13 @@ import * as T from '@/constants/types' -import {ignorePromise, timeoutPromise} from '@/constants/utils' +import {ignorePromise} from '@/constants/utils' import logger from '@/logger' -import {emitDeepLink} from '@/router-v2/linking' +import {emitDeepLink} from '@/router-v2/deep-link-emitter' +import {subscribeIntentAccountSwitch} from '@/router-v2/account-link-switch' import { getRegistrationToken, setApplicationIconBadgeNumber, - onPushNotification, onPushToken, onShareData, - getInitialNotification, removeAllPendingNotificationRequests, } from 'react-native-kb' import {useConfigState} from '@/stores/config' @@ -16,339 +15,98 @@ import {useCurrentUserState} from '@/stores/current-user' import {usePushState} from '@/stores/push' import {useShellState} from '@/stores/shell' -type DataCommon = { - userInteraction: boolean -} -type DataReadMessage = DataCommon & { - type: 'chat.readmessage' - b: string | number - i?: string -} -type DataNewMessage = DataCommon & { - type: 'chat.newmessage' - convID?: string - t: string | number - m: string -} -type DataNewMessageSilent2 = DataCommon & { - type: 'chat.newmessageSilent_2' - t: string | number - c?: string - m: string -} -type DataFollow = DataCommon & { - type: 'follow' - targetUID?: string - username?: string -} -type DataChatExtension = DataCommon & { - type: 'chat.extension' - convID?: string -} -type DataDeviceRevoked = DataCommon & { - type: 'device.revoked' - device_id?: string -} -type DataDeviceNew = DataCommon & { - type: 'device.new' - device_id?: string -} -type DataAutoreset = DataCommon & { - type: 'autoreset' -} -type Data = - | DataReadMessage - | DataNewMessage - | DataNewMessageSilent2 - | DataFollow - | DataChatExtension - | DataDeviceRevoked - | DataDeviceNew - | DataAutoreset - -type PushN = Data & { - message?: string -} - -const anyToConversationMembersType = (a: string | number): T.RPCChat.ConversationMembersType | undefined => { - const membersTypeNumber: T.RPCChat.ConversationMembersType = - typeof a === 'string' ? parseInt(a, 10) : a || -1 - switch (membersTypeNumber) { - case T.RPCChat.ConversationMembersType.kbfs: - return T.RPCChat.ConversationMembersType.kbfs - case T.RPCChat.ConversationMembersType.team: - return T.RPCChat.ConversationMembersType.team - case T.RPCChat.ConversationMembersType.impteamnative: - return T.RPCChat.ConversationMembersType.impteamnative - case T.RPCChat.ConversationMembersType.impteamupgrade: - return T.RPCChat.ConversationMembersType.impteamupgrade - default: - return undefined - } -} -const normalizePush = (_n?: object): T.Push.PushNotification | undefined => { - try { - if (!_n) { - return undefined - } - - const data = _n as PushN - const userInteraction = !!data.userInteraction - const dataUid = data as {uid?: string; targetUID?: string} - const forUid = dataUid.uid - - switch (data.type) { - case 'chat.readmessage': { - const badges = typeof data.b === 'string' ? parseInt(data.b) : data.b - return { - badges, - forUid: data.i, - type: 'chat.readmessage', - } as const - } - case 'chat.newmessage': - return data.convID - ? { - conversationIDKey: T.Chat.stringToConversationIDKey(data.convID), - forUid, - membersType: anyToConversationMembersType(data.t), - type: 'chat.newmessage', - unboxPayload: data.m || '', - userInteraction, - } - : undefined - case 'chat.newmessageSilent_2': - if (data.c) { - const membersType = anyToConversationMembersType(data.t) - if (membersType) { - return { - conversationIDKey: T.Chat.stringToConversationIDKey(data.c), - membersType, - type: 'chat.newmessageSilent_2', - unboxPayload: data.m || '', - } - } - } - return undefined - case 'follow': - return data.username - ? { - forUid: forUid ?? dataUid.targetUID, - type: 'follow', - userInteraction, - username: data.username, - } - : undefined - case 'device.revoked': - return forUid - ? { - forUid, - type: 'device.revoked', - userInteraction, - } - : undefined - case 'device.new': - return forUid - ? { - forUid, - type: 'device.new', - userInteraction, - } - : undefined - case 'autoreset': - return forUid - ? { - forUid, - type: 'autoreset', - userInteraction, - } - : undefined - case 'chat.extension': - return data.convID - ? { - conversationIDKey: T.Chat.stringToConversationIDKey(data.convID), - forUid, - type: 'chat.extension', - } - : undefined - default: - { - const unk = data as any - if (typeof unk.message === 'string' && unk.message.startsWith('Your contact') && userInteraction) { - return { - type: 'settings.contacts', - } - } - } - - return undefined - } - } catch (e) { - logger.error('Error handling push', e) - return undefined - } -} - -const getInitialPush = async () => { - const n = await getInitialNotification() - return n ? normalizePush(n) : undefined -} -const getStartupDetailsFromInitialPush = async () => { - const notification = await Promise.race([getInitialPush(), timeoutPromise(10)]) - if (!notification) { - return - } - - if (notification.type === 'follow') { - if (notification.username) { - return {startupFollowUser: notification.username} - } - } else if (notification.type === 'chat.newmessage' || notification.type === 'chat.newmessageSilent_2') { - if (notification.conversationIDKey) { - // For chat.newmessage with forUid, route through the pending-notification - // subscribers so account-switching logic runs if the notification is for a - // different account. Returning startupConversation here would navigate to a - // conversation in the wrong account before the switch can happen. - if (notification.type === 'chat.newmessage' && notification.forUid) { - usePushState.getState().dispatch.setPendingPushNotification(notification) - return - } - return { - startupConversation: notification.conversationIDKey, - startupPushPayload: notification.unboxPayload, - } - } - } - - return -} - export const initPushListener = () => { + const unsubs: Array<() => void> = [] // Permissions - useShellState.subscribe((s, old) => { - if (s.mobileAppState === old.mobileAppState) return - // Only recheck on foreground, not background - if (s.mobileAppState !== 'active') { - logger.info('[PushCheck] skip on backgrounding') - return - } - logger.debug(`[PushCheck] checking on foreground`) - usePushState - .getState() - .dispatch.checkPermissions() - .then(() => {}) - .catch(() => {}) - }) + unsubs.push( + useShellState.subscribe((s, old) => { + if (s.mobileAppState === old.mobileAppState) return + // Only recheck on foreground, not background + if (s.mobileAppState !== 'active') { + logger.info('[PushCheck] skip on backgrounding') + return + } + logger.debug(`[PushCheck] checking on foreground`) + usePushState + .getState() + .dispatch.checkPermissions() + .then(() => {}) + .catch(() => {}) + }) + ) let lastCount = -1 - useConfigState.subscribe((s, old) => { - if (s.badgeState === old.badgeState) return - if (!s.badgeState) return - const count = s.badgeState.bigTeamBadgeCount + s.badgeState.smallTeamBadgeCount - setApplicationIconBadgeNumber(count) - // Only do this native call if the count actually changed, not over and over if its zero - if (count === 0 && lastCount !== 0) { - removeAllPendingNotificationRequests() - } - lastCount = count - }) - - // Retry token upload when user state becomes available. - // The FCM token often arrives before username/deviceID are loaded, - // so the initial upload silently bails. This retries once user state is ready. - useCurrentUserState.subscribe((s, old) => { - if (s.username === old.username && s.deviceID === old.deviceID) return - const token = usePushState.getState().token - if (token && s.username && s.deviceID) { - usePushState.getState().dispatch.setPushToken(token) - } - }) + unsubs.push( + useConfigState.subscribe((s, old) => { + if (s.badgeState === old.badgeState) return + if (!s.badgeState) return + const count = s.badgeState.bigTeamBadgeCount + s.badgeState.smallTeamBadgeCount + setApplicationIconBadgeNumber(count) + // Only do this native call if the count actually changed, not over and over if its zero + if (count === 0 && lastCount !== 0) { + removeAllPendingNotificationRequests() + } + lastCount = count + }) + ) + + // Not a native-readiness retry: native parks the token and getRegistrationToken reads it + // back, so the token itself is never lost. What the upload waits on is username/deviceID, + // which the token routinely beats, so setPushToken's upload bails. Re-run it once the + // account it has to be filed under exists. + unsubs.push( + useCurrentUserState.subscribe((s, old) => { + if (s.username === old.username && s.deviceID === old.deviceID) return + const token = usePushState.getState().token + if (token && s.username && s.deviceID) { + usePushState.getState().dispatch.setPushToken(token) + } + }) + ) usePushState.getState().dispatch.initialPermissionsCheck() - // When current-user.uid changes, run pending push if it was for this account. - useCurrentUserState.subscribe((s, old) => { - if (s.uid === old.uid) return - const pushState = usePushState.getState() - const pending = pushState.pendingPushNotification - if (!pending || !('forUid' in pending)) return - const forUid = (pending as {forUid?: string}).forUid - if (!forUid || forUid !== s.uid) return - pushState.dispatch.clearPendingPushNotification() - // Replay while switching remains true. The replacement NavigationContainer - // clears it from onReady, so the intent cannot be consumed by the old router. - pushState.dispatch.handlePush(pending) - }) - - useConfigState.subscribe((s, old) => { - if (s.configuredAccounts === old.configuredAccounts || s.userSwitching) return - const pushState = usePushState.getState() - const pending = pushState.pendingPushNotification - if (!pending || !('forUid' in pending)) return - const forUid = (pending as {forUid?: string}).forUid - if (!forUid || forUid === useCurrentUserState.getState().uid) return - const account = s.configuredAccounts.find(acc => acc.uid === forUid) - if (!account?.hasStoredSecret) return - pushState.dispatch.handlePush(pending) - }) + // Taps are taken from the service in constants/init/shared; this only has to be watching the + // intent store by the time one lands, and its own first check covers anything already queued. + unsubs.push(subscribeIntentAccountSwitch()) - useConfigState.subscribe((s, old) => { - if (s.loggedIn === old.loggedIn) return - if (!s.loggedIn && !s.userSwitching) { - usePushState.getState().dispatch.clearPendingPushNotification() - } - }) - - const listenNative = async () => { - // Set up listener immediately, before waiting for token - // This ensures notifications aren't lost if they arrive before token is ready - const onNotification = (n: object) => { - logger.debug('[onNotification]: ', n) - const notification = normalizePush(n) - if (!notification) { - logger.warn('[onNotification]: normalized notification is null/undefined') - return - } - usePushState.getState().dispatch.handlePush(notification) + try { + // Token and share listeners + if (isIOS) { + const tokenSub = onPushToken(token => { + logger.debug('[PushToken] received token via onPushToken event: ', token) + usePushState.getState().dispatch.setPushToken(token) + }) + unsubs.push(() => tokenSub.remove()) } - try { - // Unified push notification handling for both iOS and Android - // Silent notifications (chat.newmessageSilent_2) are handled entirely natively - // Other notification types are handled natively first, then emitted to JS via onPushNotification - onPushNotification(onNotification) - - if (isIOS) { - onPushToken(token => { - logger.debug('[PushToken] received token via onPushToken event: ', token) - usePushState.getState().dispatch.setPushToken(token) - }) - } - - if (isAndroid) { - onShareData(evt => { - const {setAndroidShare} = useConfigState.getState().dispatch + if (isAndroid) { + const shareSub = onShareData(evt => { + const {setAndroidShare} = useConfigState.getState().dispatch - const text = evt.text - const urls = evt.localPaths + const text = evt.text + const urls = evt.localPaths - if (urls) { - setAndroidShare({type: T.RPCGen.IncomingShareType.file, urls}) - } else if (text) { - setAndroidShare({text, type: T.RPCGen.IncomingShareType.text}) - } else { - return - } - emitDeepLink('keybase://incoming-share') - }) - // shareListenersRegistered() is deliberately NOT called here: the init/index.tsx - // router subscriber controls when native flushes pending share intents. - } - } catch (e) { - logger.error('[Push] failed to set up listeners: ', e) + if (urls) { + setAndroidShare({type: T.RPCGen.IncomingShareType.file, urls}) + } else if (text) { + setAndroidShare({text, type: T.RPCGen.IncomingShareType.text}) + } else { + return + } + emitDeepLink('keybase://incoming-share') + }) + unsubs.push(() => shareSub.remove()) + // shareListenersRegistered() is deliberately NOT called here: a parked share intent + // waits for JS to be able to route it, which is the router subscriber in init/index.tsx, + // not merely for this listener to exist. } + } catch (e) { + logger.error('[Push] failed to set up listeners: ', e) + } - // Get token after listener is set up (may fail if not ready yet, but listener is already active) + // Get token after listener is set up (may fail if not ready yet, but listener is already active) + const fetchToken = async () => { try { const pushToken = await getRegistrationToken() logger.debug('[PushToken] received new token: ', pushToken) @@ -358,7 +116,7 @@ export const initPushListener = () => { // Token will be retrieved later when permissions are checked } } - ignorePromise(listenNative()) -} + ignorePromise(fetchToken()) -export {getStartupDetailsFromInitialPush} + return unsubs +} diff --git a/shared/constants/init/push-tap.test.ts b/shared/constants/init/push-tap.test.ts new file mode 100644 index 000000000000..b0be49f5f06e --- /dev/null +++ b/shared/constants/init/push-tap.test.ts @@ -0,0 +1,272 @@ +/// +import * as T from '@/constants/types' +import {resetAllStores} from '@/util/zustand' +import {useConfigState} from '@/stores/config' +import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {onEngineConnected, _onEngineIncoming} from './shared' + +const g = globalThis as unknown as {isMobile: boolean} + +// The intent store remembers a push tap id for the life of the module, so ids must not repeat +// across tests any more than they do across taps. +let nextRouteID = 100 +const chatRoute = (): T.RPCGen.PushTapRoute => ({ + id: ++nextRouteID, + targetUID: 'uid-other', + url: 'keybase://convid/0000ab', +}) + +const nudge = () => + _onEngineIncoming({ + payload: {params: undefined}, + type: 'keybase.1.NotifyApp.pushTapRouteAvailable', + } as never) + +// The service's holder, as far as these tests are concerned: a peek reports what is armed, an ack +// retires it only if it is still the same tap. drainPushTapRoute only peeks and enqueues; the ack +// belongs to whichever consumer -- navigation, or account-link-switch dropping a tap it cannot act +// on -- actually resolves the intent. These tests exercise the service holder only to confirm that. +const serviceHolding = (route?: T.RPCGen.PushTapRoute) => { + let armed = route + // Both answer a microtask late, as a real RPC would: nothing here should depend on a reply + // landing in the same tick as the call. + const peek = jest + .spyOn(T.RPCGen, 'appStatePeekPushTapRouteRpcPromise') + .mockImplementation(async () => { + await Promise.resolve() + return armed ?? null + }) + const ack = jest + .spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise') + .mockImplementation(async (params?: {id: number}) => { + await Promise.resolve() + if (armed && params?.id === armed.id) { + armed = undefined + } + }) + return {ack, arm: (next: T.RPCGen.PushTapRoute) => (armed = next), isArmed: () => !!armed, peek} +} + +const settle = async () => new Promise(resolve => setImmediate(resolve)) + +// Wedges the store the route is queued into, which is the one thing between the peek and the +// enqueue that can throw. +const withEnqueueThrowing = () => { + const original = useNavigationIntentsState.getState().dispatch + useNavigationIntentsState.setState(state => { + state.dispatch = { + ...original, + enqueue: () => { + throw new Error('the store is wedged') + }, + } + }) + return () => + useNavigationIntentsState.setState(state => { + state.dispatch = original + }) +} + +const originalConfigDispatch = useConfigState.getState().dispatch + +// onEngineConnected's other work is not what is under test here; this is the same stubbing +// shared.test.ts does for it. +const stubConnect = () => { + for (const rpc of [ + 'delegateUiCtlRegisterChatUIRpcPromise', + 'delegateUiCtlRegisterLogUIRpcPromise', + 'delegateUiCtlRegisterHomeUIRpcPromise', + 'delegateUiCtlRegisterSecretUIRpcPromise', + 'delegateUiCtlRegisterIdentify3UIRpcPromise', + 'delegateUiCtlRegisterRekeyUIRpcPromise', + ] as const) { + jest.spyOn(T.RPCGen, rpc).mockResolvedValue(undefined) + } + useConfigState.setState(s => { + s.dispatch = {...originalConfigDispatch, onEngineConnected: () => {}} + }) + jest.spyOn(T.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockRejectedValue(new Error('not under test')) + jest.spyOn(T.RPCGen, 'configGetBootstrapStatusRpcPromise').mockResolvedValue({ + loggedIn: true, + } as T.RPCGen.BootstrapStatus) +} + +beforeEach(() => { + g.isMobile = true + resetAllStores() +}) + +afterEach(() => { + g.isMobile = false + useConfigState.setState({dispatch: originalConfigDispatch}) + // Acknowledge any leftover intent while the service mock is still installed, so cleanup's own + // ack (if the intent carries one) hits the mock instead of a real, unmocked RPC call. + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) dispatch.acknowledge(intent.id) + jest.restoreAllMocks() + resetAllStores() +}) + +test('the nudge queues the armed route without acking it', async () => { + const route = chatRoute() + const service = serviceHolding(route) + + nudge() + await settle() + + expect(service.peek).toHaveBeenCalledTimes(1) + expect(service.ack).not.toHaveBeenCalled() + expect(service.isArmed()).toBe(true) + expect(useNavigationIntentsState.getState().intent).toMatchObject({ + targetUid: 'uid-other', + url: 'keybase://convid/0000ab', + }) +}) + +test('a tap waiting from before this connection is taken on connect', async () => { + stubConnect() + const route = chatRoute() + const service = serviceHolding(route) + + onEngineConnected() + await settle() + + expect(service.peek).toHaveBeenCalledTimes(1) + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://convid/0000ab') + expect(service.ack).not.toHaveBeenCalled() +}) + +// The point of never clearing on read: a reply that never arrives must cost a repeat, not the tap. +// Nothing else in the app would say the tap had happened. +test('a peek whose reply is lost leaves the route armed for the next one', async () => { + const route = chatRoute() + const service = serviceHolding(route) + service.peek.mockRejectedValueOnce(new Error('disconnected')) + + nudge() + await settle() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(service.isArmed()).toBe(true) + + // the next connection picks it up + nudge() + await settle() + + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://convid/0000ab') +}) + +// Reading a route never retires it, so a second peek of the same still-armed id reaches enqueue +// again; the intent store, not this layer, turns it away because that id is already queued. +// drainPushTapRoute itself tracks nothing about what it has already seen. +test('a second peek of the same still-armed id does not enqueue a second intent', async () => { + const service = serviceHolding(chatRoute()) + + nudge() + await settle() + const first = useNavigationIntentsState.getState().intent + nudge() + await settle() + + expect(service.peek).toHaveBeenCalledTimes(2) + expect(useNavigationIntentsState.getState().intent).toBe(first) +}) + +// The older tap is replaced outright, not merged (a different URL), so it is given up on for +// good here: the store acks it even though the service already discarded that route itself when +// it armed the newer one. Acking a route the service no longer holds is a harmless no-op there. +test('a newer tap queued while the older one is still pending upgrades nothing away', async () => { + const route = chatRoute() + const service = serviceHolding(route) + + nudge() + await settle() + const devices = {id: ++nextRouteID, targetUID: 'uid-other', url: 'keybase://devices'} + service.arm(devices) + nudge() + await settle() + + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://devices') + expect(service.ack).toHaveBeenCalledWith({id: route.id}) + expect(service.isArmed()).toBe(true) +}) + +// Leaving the route armed is what saves a lost peek, but it means a lost ack shows the same tap +// again. The intent store absorbs that by retrying only the ack, never the navigation, once the +// duplicate window has passed and the router has already consumed the intent. +test('a lost ack retries the ack without navigating again', async () => { + const route = chatRoute() + const service = serviceHolding(route) + service.ack.mockRejectedValueOnce(new Error('disconnected')) + + nudge() + await settle() + const first = useNavigationIntentsState.getState().intent + expect(first?.url).toBe('keybase://convid/0000ab') + expect(service.isArmed()).toBe(true) + + // the router consumes it and navigates, and time moves past the store's duplicate window + useNavigationIntentsState.getState().dispatch.acknowledge(first!.id) + const realNow = Date.now() + jest.spyOn(Date, 'now').mockReturnValue(realNow + 60_000) + + // the route is still armed, so the next peek sees it again + nudge() + await settle() + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(service.ack).toHaveBeenCalledTimes(2) + expect(service.isArmed()).toBe(false) +}) + +test('no waiting tap queues nothing', async () => { + const service = serviceHolding() + + nudge() + await settle() + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(service.ack).not.toHaveBeenCalled() +}) + +test('a route with no account is not a targeted intent', async () => { + serviceHolding({id: ++nextRouteID, targetUID: '', url: 'keybase://tabs.peopleTab'}) + + nudge() + await settle() + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://tabs.peopleTab') + expect(intent?.targetUid).toBeUndefined() +}) + +test('desktop never asks for a tap', async () => { + g.isMobile = false + const service = serviceHolding(chatRoute()) + + nudge() + await settle() + + expect(service.peek).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +// A throw between the peek and the queue must not crash the notification handler, and since +// nothing here ever acks, the route is still armed for the next peek regardless. +test('an enqueue that throws leaves the route armed for the next peek', async () => { + const route = chatRoute() + const service = serviceHolding(route) + const restore = withEnqueueThrowing() + + nudge() + await settle() + + expect(service.ack).not.toHaveBeenCalled() + expect(service.isArmed()).toBe(true) + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + + restore() + nudge() + await settle() + + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://convid/0000ab') +}) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 2690ea2b5fb0..2e6e29eb48ae 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -18,6 +18,7 @@ import {useNotifState} from '@/stores/notifications' import {notifyEngineActionListeners} from '@/engine/action-listener' import {serviceStaticConfigToStaticConfig} from '@/constants/chat/static-config' import {emitDeepLink} from '@/router-v2/linking' +import {enqueuePushTapRoute} from '@/router-v2/deep-link-emitter' import {ignorePromise, timeoutPromise} from '../utils' import {isLinux, isPhone, serverConfigFileName} from '../platform' import {useAvatarState} from '@/common-adapters/avatar/store' @@ -248,6 +249,31 @@ export const applyMobileAppState = (state: T.RPCGen.MobileAppState) => { } } +// Peek and queue. Reading the route does not retire it: the service acks only when navigation (or +// account-link-switch, dropping a tap it cannot act on) has actually consumed the intent this +// enqueues, which is what makes a tap exactly-once end to end. A peek whose reply is lost, or one +// that repeats a tap already queued or consumed this run, is handled by enqueuePushTapRoute/the +// intent store and costs nothing here. +// +// Run on connect, for a tap from before this connection (on iOS a background launch never starts a +// client at all, so a tap can be arbitrarily older than the socket), and on pushTapRouteAvailable +// for a tap during it. Both reach the same armed route, so neither can act on a tap the other +// already did. +const drainPushTapRoute = async () => { + if (!isMobile) { + return + } + try { + const route = await T.RPCGen.appStatePeekPushTapRouteRpcPromise() + if (!route) { + return + } + enqueuePushTapRoute(route) + } catch (error) { + logger.warn('[PushTap] failed to peek a tap route, leaving it armed: ', error) + } +} + // The splash waits for the service to say who is logged in. A clientState with no session means // its startup login attempt has not settled yet -- not known, rather than logged out -- and the // attempt settling sends another that has one. Each connection waits afresh. @@ -414,6 +440,7 @@ export const onEngineConnected = () => { awaitSessionAgain() subscription = subscribe() + ignorePromise(drainPushTapRoute()) useDaemonState.getState().dispatch.startHandshake() } @@ -466,6 +493,9 @@ export const _onEngineIncoming = (action: EngineGen.Actions) => { } switch (action.type) { + case 'keybase.1.NotifyApp.pushTapRouteAvailable': + ignorePromise(drainPushTapRoute()) + break case 'keybase.1.NotifyApp.mobileAppStateChanged': applyMobileAppState(action.payload.params.state) break diff --git a/shared/constants/types/index.tsx b/shared/constants/types/index.tsx index 3a18e25dc3f7..8e73d27e532d 100644 --- a/shared/constants/types/index.tsx +++ b/shared/constants/types/index.tsx @@ -6,7 +6,6 @@ export * as Devices from './devices' export type * as Git from './git' export * as More from './more' export type * as People from './people' -export type * as Push from './push' export * as RPCChat from '@/constants/rpc/rpc-chat-gen' export * as RPCGen from '@/constants/rpc/rpc-gen' export type * as RPCGregor from '@/constants/rpc/rpc-gregor-gen' diff --git a/shared/constants/types/push.tsx b/shared/constants/types/push.tsx deleted file mode 100644 index 244b1556dfbf..000000000000 --- a/shared/constants/types/push.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import type * as ChatTypes from './chat' -import type * as RPCChatTypes from '@/constants/rpc/rpc-chat-gen' - -export type PushNotification = - | { - badges: number - forUid?: string - type: 'chat.readmessage' - } - | { - conversationIDKey: ChatTypes.ConversationIDKey - membersType: RPCChatTypes.ConversationMembersType - type: 'chat.newmessageSilent_2' - unboxPayload: string - } - | { - conversationIDKey: ChatTypes.ConversationIDKey - forUid?: string - membersType?: RPCChatTypes.ConversationMembersType - type: 'chat.newmessage' - unboxPayload: string - userInteraction: boolean - } - | { - forUid?: string - type: 'follow' - userInteraction: boolean - username: string - } - | { - forUid?: string - type: 'device.revoked' - userInteraction: boolean - } - | { - forUid?: string - type: 'device.new' - userInteraction: boolean - } - | { - forUid?: string - type: 'autoreset' - userInteraction: boolean - } - | { - conversationIDKey: ChatTypes.ConversationIDKey - forUid?: string - type: 'chat.extension' - } - | { - type: 'settings.contacts' - } diff --git a/shared/router-v2/account-link-switch.test.ts b/shared/router-v2/account-link-switch.test.ts new file mode 100644 index 000000000000..1cb85b304bf1 --- /dev/null +++ b/shared/router-v2/account-link-switch.test.ts @@ -0,0 +1,165 @@ +/// +import * as T from '@/constants/types' +import RPCError from '@/util/rpcerror' +import {resetAllStores} from '@/util/zustand' +import {subscribeIntentAccountSwitch} from './account-link-switch' +import {enqueuePushTapRoute, emitDeepLink} from './deep-link-emitter' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useDaemonState} from '@/stores/daemon' +import {useNavigationIntentsState} from '@/stores/navigation-intents' + +const currentAccount = {hasStoredSecret: true, uid: 'uid-current', username: 'testuser'} +const otherAccount = {hasStoredSecret: true, uid: 'uid-other', username: 'testuser-mac'} +const noSecretAccount = {hasStoredSecret: false, uid: 'uid-nosecret', username: 'testuser-nosecret'} +const allAccounts = [currentAccount, otherAccount, noSecretAccount] + +// A push tap's id must not repeat across tests any more than it does across taps, so every call +// here gets a fresh one; the ack RPC stays mocked until cleanup has acknowledged a still-pending +// intent, so that acknowledgement makes no real RPC call. +let nextTapID = 9000 +const tapFor = (uid: string) => + enqueuePushTapRoute({id: ++nextTapID, targetUID: uid, url: 'keybase://convid/0000ab'}) + +let login = jest.fn() +let unsub: (() => void) | undefined + +const setAccounts = (configuredAccounts: typeof allAccounts) => { + useConfigState.setState({configuredAccounts}) +} + +// navigation-intents' resetState deliberately keeps account-targeted intents. +const clearIntent = () => { + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) dispatch.acknowledge(intent.id) +} + +beforeEach(() => { + jest.spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise').mockResolvedValue(undefined) + login = jest.fn() + useNavigationIntentsState.setState({lastHandledIntent: undefined}) + useDaemonState.setState({handshakeState: 'done'}) + useCurrentUserState.setState({uid: currentAccount.uid, username: currentAccount.username}) + // config's resetState deliberately keeps userSwitching, so clear it here. + useConfigState.setState({ + configuredAccounts: allAccounts, + dispatch: {...useConfigState.getState().dispatch, login}, + loggedIn: true, + loginError: undefined, + userSwitching: false, + }) + unsub = subscribeIntentAccountSwitch() +}) + +afterEach(() => { + unsub?.() + unsub = undefined + clearIntent() + resetAllStores() + jest.restoreAllMocks() +}) + +test('a tap for the current account does not switch', () => { + tapFor(currentAccount.uid) + + expect(login).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent?.targetUid).toBe(currentAccount.uid) +}) + +test('a tap for a stored account switches to it once', () => { + tapFor(otherAccount.uid) + + expect(login).toHaveBeenCalledTimes(1) + expect(login).toHaveBeenCalledWith(otherAccount.username, '') + expect(useConfigState.getState().userSwitching).toBe(true) + + setAccounts([...allAccounts]) + + expect(login).toHaveBeenCalledTimes(1) +}) + +test('a tap for an account not listed yet waits for the account list', () => { + setAccounts([currentAccount]) + tapFor(otherAccount.uid) + + expect(login).not.toHaveBeenCalled() + + setAccounts(allAccounts) + + expect(login).toHaveBeenCalledTimes(1) +}) + +test('a tap for an account without a stored secret is dropped', () => { + tapFor(noSecretAccount.uid) + + expect(login).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +// Dropped here means no navigation is ever coming for it, so this is where the tap's route must +// be acked -- there is no other consumption point left to do it. +test('a tap dropped for a missing stored secret acks its route', () => { + const ack = T.RPCGen.appStateAckPushTapRouteRpcPromise as jest.Mock + const id = ++nextTapID + + enqueuePushTapRoute({id, targetUID: noSecretAccount.uid, url: 'keybase://convid/0000ab'}) + + expect(ack).toHaveBeenCalledWith({id}) +}) + +test('nothing switches before the handshake is done', () => { + useDaemonState.setState({handshakeState: 'loading'}) + tapFor(otherAccount.uid) + + expect(login).not.toHaveBeenCalled() + + useDaemonState.setState({handshakeState: 'done'}) + + expect(login).toHaveBeenCalledTimes(1) +}) + +test('a login error drops the tap', () => { + tapFor(otherAccount.uid) + expect(login).toHaveBeenCalledTimes(1) + + useConfigState.setState({loginError: new RPCError('bad', 1), userSwitching: false}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('a login error dropping the tap acks its route', () => { + const ack = T.RPCGen.appStateAckPushTapRouteRpcPromise as jest.Mock + const id = ++nextTapID + enqueuePushTapRoute({id, targetUID: otherAccount.uid, url: 'keybase://convid/0000ab'}) + expect(ack).not.toHaveBeenCalled() + + useConfigState.setState({loginError: new RPCError('bad', 1), userSwitching: false}) + + expect(ack).toHaveBeenCalledWith({id}) +}) + +test('logging out drops a tap for another account', () => { + useConfigState.setState({configuredAccounts: [], loggedIn: true}) + tapFor(otherAccount.uid) + + useConfigState.setState({loggedIn: false, userSwitching: false}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('a foreign link naming a stored account never switches', () => { + emitDeepLink(`keybase://profile/show/${otherAccount.username}`) + + expect(login).not.toHaveBeenCalled() + expect(useConfigState.getState().userSwitching).toBe(false) +}) + +test('a switch already under way is not restarted when userSwitching clears early', () => { + tapFor(otherAccount.uid) + expect(login).toHaveBeenCalledTimes(1) + + // the replacement router's onReady clears userSwitching before the new uid lands + useConfigState.setState({userSwitching: false}) + + expect(login).toHaveBeenCalledTimes(1) +}) diff --git a/shared/router-v2/account-link-switch.tsx b/shared/router-v2/account-link-switch.tsx new file mode 100644 index 000000000000..f50289d52989 --- /dev/null +++ b/shared/router-v2/account-link-switch.tsx @@ -0,0 +1,66 @@ +import logger from '@/logger' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useDaemonState} from '@/stores/daemon' +import {useNavigationIntentsState} from '@/stores/navigation-intents' + +type ConfigState = ReturnType + +const tapForOtherAccount = () => { + const {intent} = useNavigationIntentsState.getState() + return intent?.targetUid && intent.targetUid !== useCurrentUserState.getState().uid ? intent : undefined +} + +// A tapped push for another account waits in the intent store until that account is current. This +// switches to it: to a stored account once, never to one without a stored secret, and it drops the +// tap when the switch fails or the user logs out. Only enqueuePushTapRoute sets targetUid, and only +// a route the service resolved from a real notification tap reaches it, so no link another app +// opens can switch accounts. +// +// Both drops below go through dispatch.acknowledge, which also acks the tap's route with the +// service -- there is no navigation coming for it, so this is where it is given up on for good. +export const subscribeIntentAccountSwitch = () => { + // userSwitching already gates a second login, but it is cleared by the replacement router's + // onReady, which can run before the new uid lands; keying on the intent makes the switch + // exactly-once without depending on that ordering. + let switchingFor: number | undefined + const check = () => { + const intent = tapForOtherAccount() + if (!intent || switchingFor === intent.id) return + const {configuredAccounts, dispatch, userSwitching} = useConfigState.getState() + if (userSwitching || useDaemonState.getState().handshakeState !== 'done') return + const account = configuredAccounts.find(a => a.uid === intent.targetUid) + if (!account) return + if (!account.hasStoredSecret) { + logger.info('[AccountLink] target account has no stored secret, dropping the tap') + useNavigationIntentsState.getState().dispatch.acknowledge(intent.id) + return + } + switchingFor = intent.id + logger.info('[AccountLink] switching accounts for a tapped push') + dispatch.setUserSwitching(true) + dispatch.login(account.username, '') + } + const dropOnFailure = (s: ConfigState, old: ConfigState) => { + const loginFailed = !!s.loginError && s.loginError !== old.loginError + const loggedOut = s.loggedIn !== old.loggedIn && !s.loggedIn && !s.userSwitching + if (!loginFailed && !loggedOut) return + const intent = tapForOtherAccount() + if (!intent) return + logger.info('[AccountLink] dropping a tap for another account after a failed switch or logout') + useNavigationIntentsState.getState().dispatch.acknowledge(intent.id) + } + const unsubs = [ + useNavigationIntentsState.subscribe(check), + useConfigState.subscribe((s, old) => { + dropOnFailure(s, old) + check() + }), + useCurrentUserState.subscribe(check), + useDaemonState.subscribe(check), + ] + check() + return () => { + for (const unsub of unsubs) unsub() + } +} diff --git a/shared/router-v2/deep-link-emitter.test.ts b/shared/router-v2/deep-link-emitter.test.ts index c2cec9b60962..8a25a45a35a0 100644 --- a/shared/router-v2/deep-link-emitter.test.ts +++ b/shared/router-v2/deep-link-emitter.test.ts @@ -1,6 +1,11 @@ /// +import * as T from '@/constants/types' import {useNavigationIntentsState} from '@/stores/navigation-intents' -import {emitDeepLink, setInitialURLOnce} from './deep-link-emitter' +import {emitDeepLink, enqueuePushTapRoute, setInitialURLOnce} from './deep-link-emitter' + +// A push tap's id must not repeat across tests any more than it does across taps. +let nextTapID = 8000 +const tapID = () => ++nextTapID const resetNavigationIntents = () => { const {intent, dispatch} = useNavigationIntentsState.getState() @@ -10,8 +15,13 @@ const resetNavigationIntents = () => { dispatch.resetState() } +beforeEach(() => { + jest.spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise').mockResolvedValue(undefined) +}) + afterEach(() => { resetNavigationIntents() + jest.restoreAllMocks() }) test('normalizes and enqueues a deep link until navigation can consume it', () => { @@ -54,3 +64,36 @@ test('removes a queued deep link when the initial URL handles it', () => { expect(useNavigationIntentsState.getState().intent).toBeUndefined() }) + +test('a foreign link never targets an account', () => { + emitDeepLink('keybase://convid/0000ab') + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://convid/0000ab') + expect(intent?.targetUid).toBeUndefined() +}) + +test('a tap targets its account', () => { + enqueuePushTapRoute({id: tapID(), targetUID: 'uid-other', url: 'keybase://convid/0000ab'}) + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://convid/0000ab') + expect(intent?.targetUid).toBe('uid-other') +}) + +test('a tap for a link a foreign open already queued upgrades that intent', () => { + emitDeepLink('keybase://convid/0000ab') + enqueuePushTapRoute({id: tapID(), targetUID: 'uid-other', url: 'keybase://convid/0000ab'}) + + expect(useNavigationIntentsState.getState().intent?.targetUid).toBe('uid-other') +}) + +// The service leaves targetUID empty for a route no account owns, and an empty one must not +// read as a target: an intent with one is what account-link-switch acts on. +test('a tap with no account is not a targeted intent', () => { + enqueuePushTapRoute({id: tapID(), targetUID: '', url: 'keybase://tabs.peopleTab'}) + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://tabs.peopleTab') + expect(intent?.targetUid).toBeUndefined() +}) diff --git a/shared/router-v2/deep-link-emitter.tsx b/shared/router-v2/deep-link-emitter.tsx index 441b1cb77e80..c54e38365def 100644 --- a/shared/router-v2/deep-link-emitter.tsx +++ b/shared/router-v2/deep-link-emitter.tsx @@ -1,11 +1,10 @@ -import { - type NavigationIntentOptions, - useNavigationIntentsState, -} from '@/stores/navigation-intents' +import logger from '@/logger' +import {useNavigationIntentsState} from '@/stores/navigation-intents' -// Deep-link emission + URL normalization. Kept separate from './linking' -// (which imports the config/push/current-user stores) so stores/push can enqueue -// navigation without importing the router's linking config. +// Deep-link emission + URL normalization. Kept separate from './linking' so +// stores/push can enqueue navigation without importing the router's linking config +// (which pulls in the config/push/current-user stores and the route tables). This +// leaf depends on the navigation-intents store and nothing else. // ---- URL normalization ---- @@ -75,8 +74,27 @@ export const setInitialURLOnce = (url: string) => { // Producers only enqueue navigation intent. The active router consumes it once // the intended account is active and its NavigationContainer is ready. -export const emitDeepLink = (url: string, options?: NavigationIntentOptions) => { +// +// A link here can come from any app, web page or typed URL, so it never carries +// a targetUid: only enqueuePushTapRoute may target (and so switch) an account. +export const emitDeepLink = (url: string) => { const normalized = normalizeUrl(url) if (!normalized) return - useNavigationIntentsState.getState().dispatch.enqueue(normalized, options) + useNavigationIntentsState.getState().dispatch.enqueue(normalized) +} + +// ---- Notification taps ---- + +// For routes read from the service's pending-tap holder only (see +// constants/init/shared). The service fills that holder from its push-tap bind +// verb and nothing else, so a targetUID here can only have come from a real +// notification tap, and no link another app opens can switch accounts. +// +// id is the Go route id: carried on the intent so whoever consumes it (or drops it for good) can +// ack it there instead of here, since here the tap isn't queued yet, let alone acted on. +export const enqueuePushTapRoute = (route: {url: string; targetUID: string; id: number}) => { + logger.info('[PushTap] queued a tap link:', route.url) + useNavigationIntentsState + .getState() + .dispatch.enqueue(route.url, {pushTapID: route.id, targetUid: route.targetUID || undefined}) } diff --git a/shared/router-v2/intent-consumption.test.ts b/shared/router-v2/intent-consumption.test.ts index 4223f19e0926..0b6ade323c21 100644 --- a/shared/router-v2/intent-consumption.test.ts +++ b/shared/router-v2/intent-consumption.test.ts @@ -1,9 +1,10 @@ /// +import * as T from '@/constants/types' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' import {resetAllStores} from '@/util/zustand' -import {emitDeepLink} from './deep-link-emitter' +import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import {subscribeNavigationIntents} from './linking' const setCurrentUser = (uid: string) => { @@ -27,6 +28,7 @@ const clearIntent = () => { } beforeEach(() => { + jest.spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise').mockResolvedValue(undefined) useConfigState.getState().dispatch.setLoggedIn(true) useConfigState.getState().dispatch.setUserSwitching(false) setCurrentUser('current-uid') @@ -34,9 +36,40 @@ beforeEach(() => { }) afterEach(() => { - jest.restoreAllMocks() clearIntent() resetAllStores() + jest.restoreAllMocks() +}) + +test('consuming an intent acks the tap route it carries', () => { + const ack = T.RPCGen.appStateAckPushTapRouteRpcPromise as jest.Mock + const listener = jest.fn() + // The store notifies subscribers synchronously, so a ready router consumes (and acks) an + // enqueued intent before enqueuePushTapRoute below returns. + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4242, targetUID: 'current-uid', url: 'keybase://convid/tap-conversation'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/tap-conversation') + expect(ack).toHaveBeenCalledWith({id: 4242}) + unsubscribe() +}) + +test('a stale intent that is dropped without navigating still acks its tap route', () => { + const ack = T.RPCGen.appStateAckPushTapRouteRpcPromise as jest.Mock + const now = jest.spyOn(Date, 'now') + now.mockReturnValue(1_000) + useConfigState.getState().dispatch.setUserSwitching(true) + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4343, targetUID: 'current-uid', url: 'keybase://convid/stale-tap'}) + now.mockReturnValue(1_000 + 5 * 60_000 + 1) + useConfigState.getState().dispatch.setUserSwitching(false) + + expect(listener).not.toHaveBeenCalled() + expect(ack).toHaveBeenCalledWith({id: 4343}) + unsubscribe() }) test('profile links route imperatively so their back stack is built', () => { @@ -139,7 +172,7 @@ test('an account-targeted intent survives the store reset an account switch perf const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) useConfigState.getState().dispatch.setUserSwitching(true) - emitDeepLink('keybase://convid/switch-target-conversation', {targetUid: 'target-uid'}) + enqueuePushTapRoute({id: 4444, targetUID: 'target-uid', url: 'keybase://convid/switch-target-conversation'}) expect(listener).not.toHaveBeenCalled() // the service's loggedOut notification lands mid-switch and resets every store diff --git a/shared/router-v2/linking-initial-url.test.ts b/shared/router-v2/linking-initial-url.test.ts index 250735e6c62c..a25bfe617883 100644 --- a/shared/router-v2/linking-initial-url.test.ts +++ b/shared/router-v2/linking-initial-url.test.ts @@ -7,6 +7,7 @@ import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' import {usePushState} from '@/stores/push' import {createLinkingConfig} from './linking' +import {enqueuePushTapRoute} from './deep-link-emitter' const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ @@ -20,7 +21,6 @@ const setCurrentUser = (uid: string) => { type Startup = { conversation: T.Chat.ConversationIDKey conversationUid?: string - followUser: string tab?: Tabs.Tab } @@ -30,7 +30,6 @@ const setStartup = (st: Partial) => { useConfigState.setState({ startup: { conversation: T.Chat.noConversationIDKey, - followUser: '', loaded: true, ...st, }, @@ -44,13 +43,22 @@ const getInitialURL = async () => { const handleAppLink = jest.fn() +// A push tap's id must not repeat across tests any more than it does across taps. +let nextTapID = 5000 +const tapID = () => ++nextTapID + beforeEach(() => { + jest.spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise').mockResolvedValue(undefined) useConfigState.getState().dispatch.setLoggedIn(true) setCurrentUser('current-uid') }) afterEach(() => { handleAppLink.mockReset() + // resetAllStores deliberately keeps account-targeted intents; drop them here. + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) dispatch.acknowledge(intent.id) + jest.restoreAllMocks() resetAllStores() }) @@ -91,16 +99,32 @@ test('a conversation persisted by this account is kept', async () => { await expect(getInitialURL()).resolves.toBe('keybase://convid/conv-1') }) -test('a follow-user startup opens their profile when there is no conversation', async () => { - setStartup({followUser: 'testuser'}) +test('a cold tap for the current account is the startup route, ahead of saved state', async () => { + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id: tapID(), targetUID: 'current-uid', url: 'keybase://convid/0000ab'}) - await expect(getInitialURL()).resolves.toBe('keybase://profile/show/testuser') + await expect(getInitialURL()).resolves.toBe('keybase://convid/0000ab') + expect(useNavigationIntentsState.getState().intent).toBeUndefined() }) -test('a saved conversation wins over a follow-user startup', async () => { - setStartup({conversation: 'conv-1', followUser: 'testuser'}) +test('getInitialURL taking a cold tap acks its route', async () => { + const ack = T.RPCGen.appStateAckPushTapRouteRpcPromise as jest.Mock + const id = tapID() + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id, targetUID: 'current-uid', url: 'keybase://convid/0000ab'}) + expect(ack).not.toHaveBeenCalled() + + await expect(getInitialURL()).resolves.toBe('keybase://convid/0000ab') + + expect(ack).toHaveBeenCalledWith({id}) +}) + +test('a cold tap for another account opens saved state and waits for the switch', async () => { + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id: tapID(), targetUID: 'other-uid', url: 'keybase://convid/0000ab'}) await expect(getInitialURL()).resolves.toBe('keybase://convid/conv-1') + expect(useNavigationIntentsState.getState().intent?.targetUid).toBe('other-uid') }) test('the push prompt wins when there is nothing saved to restore', async () => { @@ -155,3 +179,12 @@ test('the returned initial url is recorded so the same deep link is not re-enque expect(useNavigationIntentsState.getState().lastHandledIntent?.url).toBe(`keybase://${Tabs.chatTab}`) }) + +test('a queued tap older than the intent lifetime is not the startup route', async () => { + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id: tapID(), targetUID: 'current-uid', url: 'keybase://convid/0000ab'}) + const intent = useNavigationIntentsState.getState().intent + useNavigationIntentsState.setState({intent: {...intent!, createdAt: Date.now() - 6 * 60_000}}) + + await expect(getInitialURL()).resolves.toBe('keybase://convid/conv-1') +}) diff --git a/shared/router-v2/linking.test.ts b/shared/router-v2/linking.test.ts index b985780ecdb4..fa43fdd37981 100644 --- a/shared/router-v2/linking.test.ts +++ b/shared/router-v2/linking.test.ts @@ -1,8 +1,9 @@ /// +import * as T from '@/constants/types' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' -import {emitDeepLink} from './deep-link-emitter' +import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import * as Settings from '@/constants/settings' import * as Tabs from '@/constants/tabs' import {createLinkingConfig, isHandledByLinkingConfig, subscribeNavigationIntents} from './linking' @@ -16,6 +17,10 @@ const setCurrentUser = (uid: string) => { }) } +// A push tap's id must not repeat across tests any more than it does across taps. +let nextTapID = 10_000 +const tapID = () => ++nextTapID + const clearIntent = () => { const {intent, dispatch} = useNavigationIntentsState.getState() if (intent) { @@ -25,6 +30,7 @@ const clearIntent = () => { } beforeEach(() => { + jest.spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise').mockResolvedValue(undefined) useConfigState.getState().dispatch.setLoggedIn(true) useConfigState.getState().dispatch.setUserSwitching(false) setCurrentUser('current-uid') @@ -32,6 +38,7 @@ beforeEach(() => { afterEach(() => { clearIntent() + jest.restoreAllMocks() }) test('waits for navigation readiness before consuming an intent', () => { @@ -66,7 +73,7 @@ test('waits until the intended account is active', () => { const listener = jest.fn() const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) - emitDeepLink('keybase://convid/target-account-conversation', {targetUid: 'target-uid'}) + enqueuePushTapRoute({id: tapID(), targetUID: 'target-uid', url: 'keybase://convid/target-account-conversation'}) expect(listener).not.toHaveBeenCalled() setCurrentUser('target-uid') @@ -86,7 +93,7 @@ test('waits for an account switch to finish', () => { const listener = jest.fn() const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) - emitDeepLink('keybase://convid/account-switch-conversation', {targetUid: 'current-uid'}) + enqueuePushTapRoute({id: tapID(), targetUID: 'current-uid', url: 'keybase://convid/account-switch-conversation'}) expect(listener).not.toHaveBeenCalled() useConfigState.getState().dispatch.setUserSwitching(false) @@ -102,9 +109,7 @@ test('waits for the replacement router after the current account changes', () => const listener = jest.fn() const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) - emitDeepLink('keybase://convid/replacement-router-conversation', { - targetUid: 'target-uid', - }) + enqueuePushTapRoute({id: tapID(), targetUID: 'target-uid', url: 'keybase://convid/replacement-router-conversation'}) setCurrentUser('target-uid') // The bootstrap UID can change before React commits the keyed router remount. diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index c6e86a3a4cf2..9c05142f632e 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -1,5 +1,6 @@ import * as Settings from '@/constants/settings' import * as Tabs from '@/constants/tabs' +import logger from '@/logger' import {isSplit} from '@/constants/chat/layout' import {isValidConversationIDKey, stringToConversationIDKey} from '@/constants/types/chat/common' import {useConfigState} from '@/stores/config' @@ -96,6 +97,8 @@ const navigationIntentLifetimeMs = 5 * 60_000 // The router owns consumption. Producers can enqueue before this subscription // exists, during an account switch, or before NavigationContainer is ready. +// Every dispatch.acknowledge below -- whether the intent is actually navigated or given up on as +// stale -- is also what acks a tap's route with the service, if the intent carries one. export const subscribeNavigationIntents = ( listener: (url: string) => void, handleAppLink: (link: string) => void @@ -288,7 +291,8 @@ const customGetStateFromPath = ( // Known URLs become launch state; the rest open imperatively once the router is up. // setInitialURLOnce also consumes: markInitialURLHandled clears a pending intent with the -// same URL, so subscribeNavigationIntents won't navigate to it a second time. +// same URL, so subscribeNavigationIntents won't navigate to it a second time, and acks the +// intent's tap route with the service if it carried one. const openInitialLink = (link: string, handleAppLink: (link: string) => void) => { if (isHandledByLinkingConfig(link)) return setInitialURLOnce(link) setInitialURLOnce(link) @@ -304,7 +308,7 @@ export const createLinkingConfig = ( const {loggedIn, startup, androidShare} = useConfigState.getState() if (!loggedIn) return null - const {tab: startupTab, followUser: startupFollowUser} = startup + const {tab: startupTab} = startup let startupConversation = startup.conversation if (!isValidConversationIDKey(startupConversation)) { startupConversation = '' @@ -317,6 +321,18 @@ export const createLinkingConfig = ( startupConversation = '' } + // A tapped push picks where the app opens, once its account is current. A tap for + // another account stays queued until account-link-switch has switched to it. The same + // lifetime applies here as in subscribeNavigationIntents. + const {intent} = useNavigationIntentsState.getState() + if ( + intent && + Date.now() - intent.createdAt <= navigationIntentLifetimeMs && + (!intent.targetUid || intent.targetUid === currentUid) + ) { + return openInitialLink(intent.url, handleAppLink) + } + const pushState = usePushState.getState() const showMonster = !pushState.justSignedUp && pushState.showPushPrompt && !pushState.hasPermissions @@ -345,10 +361,6 @@ export const createLinkingConfig = ( return setInitialURLOnce('keybase://incoming-share') } - if (startupFollowUser && !startupConversation) { - return setInitialURLOnce(`keybase://profile/show/${startupFollowUser}`) - } - if (startupConversation) { return setInitialURLOnce(`keybase://convid/${startupConversation}`) } @@ -375,6 +387,7 @@ export const createLinkingConfig = ( let removeLinkingSub: (() => void) | undefined if (isMobile) { const sub = Linking.addEventListener('url', ({url}: {url: string}) => { + logger.info('[DeepLink] url event:', url) emitDeepLink(url) }) removeLinkingSub = () => sub.remove() diff --git a/shared/stores/config.tsx b/shared/stores/config.tsx index 2d8c600bb5a3..d5393e3443be 100644 --- a/shared/stores/config.tsx +++ b/shared/stores/config.tsx @@ -44,7 +44,6 @@ type Store = T.Immutable<{ // uid of the account that persisted `conversation` (from ui.routeState2). // Used to avoid replaying a conversation under a different account. conversationUid?: string - followUser: string tab?: Tab } userSwitching: boolean @@ -80,7 +79,6 @@ const initialStore: Store = { revokedTrigger: 0, startup: { conversation: noConversationIDKey, - followUser: '', loaded: false, }, userSwitching: false, @@ -503,8 +501,6 @@ export const useConfigState = Z.createZustand('config', (set, get) => { }) if (error) { get().dispatch.setUserSwitching(false) - // push store clears its own pendingPushNotification by subscribing to - // loginError (see stores/push) — keeps config from importing push. } }, setOutOfDate: outOfDate => { diff --git a/shared/stores/navigation-intents.test.ts b/shared/stores/navigation-intents.test.ts index 855bd3914ca8..06f179e2b578 100644 --- a/shared/stores/navigation-intents.test.ts +++ b/shared/stores/navigation-intents.test.ts @@ -1,4 +1,5 @@ /// +import * as T from '@/constants/types' import {resetAllStores} from '@/util/zustand' import {useNavigationIntentsState} from './navigation-intents' @@ -10,10 +11,21 @@ const clearIntent = () => { dispatch.resetState() } +let ack: jest.SpyInstance +beforeEach(() => { + ack = jest.spyOn(T.RPCGen, 'appStateAckPushTapRouteRpcPromise').mockResolvedValue(undefined) +}) + afterEach(() => { clearIntent() + jest.restoreAllMocks() }) +// The module remembers a push tap id for the life of the file, the same as the service does for +// the process, so ids must not repeat across tests any more than they do across taps. +let nextPushTapID = 1000 +const pushTapID = () => ++nextPushTapID + test('acknowledges only the intent that was actually handled', () => { const dispatch = useNavigationIntentsState.getState().dispatch dispatch.enqueue('keybase://convid/first') @@ -106,3 +118,153 @@ test('clears duplicate history across the account store reset', () => { 'keybase://convid/new-session' ) }) + +// A tap route is not the same thing as its intent: the intent can be enqueued and even acked +// locally while the service still thinks the route is armed, so acking it is a distinct, explicit +// step -- never implied by enqueuing. +test('enqueuing a tap does not ack its route', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + expect(ack).not.toHaveBeenCalled() +}) + +test('acknowledging a tapped intent acks its route', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + + expect(ack).toHaveBeenCalledWith({id}) +}) + +test('acknowledging a plain deep link never calls the tap ack', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + dispatch.enqueue('keybase://convid/no-tap') + + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + + expect(ack).not.toHaveBeenCalled() +}) + +test('markInitialURLHandled acks the tapped route it clears', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/cold-start-tap', {pushTapID: id}) + + dispatch.markInitialURLHandled('keybase://convid/cold-start-tap') + + expect(ack).toHaveBeenCalledWith({id}) + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +// The route stays armed on a lost peek reply, so drainPushTapRoute's next peek re-delivers the +// same id. Re-enqueuing it must not queue (and so navigate) a second time. +test('re-enqueuing a still-pending tap id does not replace or duplicate the intent', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + const first = useNavigationIntentsState.getState().intent + + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + expect(useNavigationIntentsState.getState().intent).toBe(first) +}) + +// A redelivery after the route has already been consumed -- the ack RPC itself failed, so the +// service never retired it -- must not navigate a second time, however long ago that was, but the +// ack itself is retried: nothing else will ever ask the service to retire that route again. +test('re-enqueuing an already-consumed tap id retries the ack without navigating again', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + expect(ack).toHaveBeenCalledTimes(1) + + const realNow = Date.now() + jest.spyOn(Date, 'now').mockReturnValue(realNow + 60_000) + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(ack).toHaveBeenCalledTimes(2) + expect(ack).toHaveBeenNthCalledWith(2, {id}) +}) + +// Every path that removes or replaces a pushTapID on s.intent must ack it. The four below are the +// ones enqueue and resetState can take that acknowledge/markInitialURLHandled do not cover. + +test('merging a newer tap into the same-URL pending intent adopts its id instead of acking the old one', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const older = pushTapID() + const newer = pushTapID() + dispatch.enqueue('keybase://convid/same-url', {pushTapID: older}) + + // The service replaces an unacked route outright on a new tap, so by the time this lands the + // older route is already gone on that side; acking it here would be a pointless extra call. + dispatch.enqueue('keybase://convid/same-url', {pushTapID: newer}) + + expect(ack).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toMatchObject({pushTapID: newer}) + + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + + expect(ack).toHaveBeenCalledTimes(1) + expect(ack).toHaveBeenCalledWith({id: newer}) +}) + +test('a tap enqueued again inside the duplicate window of its own navigation acks immediately', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const first = pushTapID() + dispatch.enqueue('keybase://convid/duplicate-window', {pushTapID: first}) + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + ack.mockClear() + + // A redelivery of the same URL (not the same tap id -- a fresh one, as a second real tap + // landing on the same conversation would carry) inside the duplicate window: navigation just + // happened, so this one has nothing left to wait for. + const second = pushTapID() + dispatch.enqueue('keybase://convid/duplicate-window', {pushTapID: second}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(ack).toHaveBeenCalledTimes(1) + expect(ack).toHaveBeenCalledWith({id: second}) +}) + +test('a pending tap superseded by an unrelated enqueue acks the route it loses', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/superseded-tap', {pushTapID: id}) + + // A plain deep link (emitDeepLink) for an unrelated URL: the service was never told this tap + // was acted on, so without an explicit ack here the next peek would hand the same route back. + dispatch.enqueue('keybase://convid/unrelated') + + expect(ack).toHaveBeenCalledWith({id}) + expect(useNavigationIntentsState.getState().intent).toMatchObject({url: 'keybase://convid/unrelated'}) +}) + +test('resetState acks the tap route of an unscoped intent it discards', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + // No targetUid: a contact-joined push tap, which never carries an account. + dispatch.enqueue('keybase://tabs.peopleTab', {pushTapID: id}) + + resetAllStores() + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(ack).toHaveBeenCalledWith({id}) +}) + +test('resetState does not ack a targeted intent it keeps', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/kept-across-reset', {pushTapID: id, targetUid: 'target-uid'}) + + resetAllStores() + + expect(useNavigationIntentsState.getState().intent).toMatchObject({pushTapID: id}) + expect(ack).not.toHaveBeenCalled() +}) diff --git a/shared/stores/navigation-intents.tsx b/shared/stores/navigation-intents.tsx index 63a06455fea9..0db269e15a8f 100644 --- a/shared/stores/navigation-intents.tsx +++ b/shared/stores/navigation-intents.tsx @@ -1,12 +1,16 @@ +import * as T from '@/constants/types' import * as Z from '@/util/zustand' +import logger from '@/logger' export type NavigationIntentOptions = { + pushTapID?: number targetUid?: string } type NavigationIntent = { createdAt: number id: number + pushTapID?: number targetUid?: string url: string } @@ -33,14 +37,32 @@ type Store = { const duplicateWindowMs = 1500 -const targetsCouldMatch = (first?: string, second?: string) => - !first || !second || first === second +// A push tap's Go-side route is retired by an explicit ack, not by anything here clearing the +// intent. Once an id has been queued, remembering it for the rest of the process is what keeps a +// lost-ack redelivery (the route stays armed; see constants/init/shared's drainPushTapRoute) from +// enqueuing -- and so navigating -- a second time. Module state, not store state: it must survive +// resetState, which runs on every account switch this process makes. +// +// Structural rule: every pushTapID that leaves s.intent -- consumed, merged away, superseded by a +// different pending intent, or discarded outright -- goes through ackPushTap exactly once. A route +// left dangling here is a route the service will hand back on the next peek, navigating (or +// failing to navigate) on a tap the app has already moved past. +const seenPushTapIDs = new Set() -// Once an unscoped URL has been handled, a later targeted URL carries new -// account-routing information and must not be discarded. The reverse ordering -// is safe: an unscoped event after a targeted one can be the duplicate source. -const handledTargetMatches = (handled?: string, incoming?: string) => - !incoming || handled === incoming +const sendPushTapAck = (pushTapID: number) => { + T.RPCGen.appStateAckPushTapRouteRpcPromise({id: pushTapID}).catch((error: unknown) => { + logger.warn('[PushTap] failed to ack a consumed tap route: ', error) + }) +} + +// Fires the ack once per id, regardless of how many times consumption is reported for it. A +// redelivery of an id already in the set (the route is still armed, so that first ack did not +// land) is retried directly by enqueue, not through here. +const ackPushTap = (pushTapID: number | undefined) => { + if (pushTapID === undefined || seenPushTapIDs.has(pushTapID)) return + seenPushTapIDs.add(pushTapID) + sendPushTapAck(pushTapID) +} export const useNavigationIntentsState = Z.createZustand( 'navigation-intents', @@ -48,9 +70,9 @@ export const useNavigationIntentsState = Z.createZustand( let nextIntentID = 0 const dispatch: Store['dispatch'] = { acknowledge: id => { + const intent = get().intent + if (intent?.id !== id) return set(s => { - const intent = s.intent - if (intent?.id !== id) return s.lastHandledIntent = { handledAt: Date.now(), targetUid: intent.targetUid, @@ -58,42 +80,85 @@ export const useNavigationIntentsState = Z.createZustand( } s.intent = undefined }) + ackPushTap(intent.pushTapID) }, enqueue: (url, options) => { const now = Date.now() - const targetUid = options?.targetUid + const {pushTapID, targetUid} = options ?? {} const {intent: pending, lastHandledIntent} = get() - if (pending?.url === url && targetsCouldMatch(pending.targetUid, targetUid)) { - if (!pending.targetUid && targetUid) { + + if (pushTapID !== undefined) { + if (pending?.pushTapID === pushTapID) { + // Still queued, waiting on the exact thing this call is asking for. + return + } + if (seenPushTapIDs.has(pushTapID)) { + // The route is still armed on the service, so the ack that was supposed to retire + // it did not land. Retry it; nothing here re-enqueues, since this id already left + // the store once and must not navigate a second time. + sendPushTapAck(pushTapID) + return + } + } + + if ( + pending?.url === url && + (!pending.targetUid || !targetUid || pending.targetUid === targetUid) + ) { + const targetUidChanged = !pending.targetUid && !!targetUid + // pushTapID is guaranteed different from pending.pushTapID here (equal is caught + // above), so this always means the service replaced the route this intent already + // carries with a newer one -- adopt its id so the eventual ack retires the route + // that is actually still armed, rather than one already gone. + const pushTapIDChanged = pushTapID !== undefined + if (targetUidChanged || pushTapIDChanged) { set(s => { - if (s.intent?.id === pending.id) { + if (s.intent?.id !== pending.id) return + if (targetUidChanged) { s.intent.targetUid = targetUid } + if (pushTapIDChanged) { + s.intent.pushTapID = pushTapID + } }) } return } + + // Once an unscoped URL has been handled, a later targeted URL carries new + // account-routing information and must not be discarded. The reverse ordering + // is safe: an unscoped event after a targeted one can be the duplicate source. if ( lastHandledIntent?.url === url && now - lastHandledIntent.handledAt < duplicateWindowMs && - handledTargetMatches(lastHandledIntent.targetUid, targetUid) + (!targetUid || lastHandledIntent.targetUid === targetUid) ) { + // Navigation for this URL just happened; a tap riding along has nothing left to wait + // for, so it acks immediately instead of waiting on a consumption that isn't coming. + ackPushTap(pushTapID) return } + + // A different pending intent is replaced outright rather than merged (see above), so + // its own tap -- if it carries one, and whether or not the service has already + // discarded that route for the one replacing it -- is given up on for good here. + ackPushTap(pending?.pushTapID) + const id = ++nextIntentID set(s => { s.intent = { createdAt: now, id, + pushTapID, targetUid, url, } }) }, markInitialURLHandled: url => { + const pending = get().intent + const matchingPending = pending?.url === url ? pending : undefined set(s => { - const pending = s.intent - const matchingPending = pending?.url === url ? pending : undefined if (matchingPending) { s.intent = undefined } @@ -103,10 +168,13 @@ export const useNavigationIntentsState = Z.createZustand( url, } }) + ackPushTap(matchingPending?.pushTapID) }, // Account changes call resetAllStores. Keep account-targeted navigation // across the reset, but discard unscoped work from the previous session. resetState: () => { + const intent = get().intent + const discarding = !intent?.targetUid set(s => { if (!s.intent?.targetUid) { s.intent = undefined @@ -115,6 +183,9 @@ export const useNavigationIntentsState = Z.createZustand( s.navigationReady = false s.navigationReadyForUid = undefined }) + if (discarding) { + ackPushTap(intent?.pushTapID) + } }, setNavigationReady: (ready, uid) => { set(s => { diff --git a/shared/stores/push.tsx b/shared/stores/push.tsx index fc0b539289b6..e71dd6d334fc 100644 --- a/shared/stores/push.tsx +++ b/shared/stores/push.tsx @@ -1,10 +1,8 @@ import * as S from '@/constants/strings' import * as T from '@/constants/types' -import * as Tabs from '@/constants/tabs' import * as Z from '@/util/zustand' import logger from '@/logger' import {ignorePromise, neverThrowPromiseFunc, timeoutPromise} from '@/constants/utils' -import {navUpToScreen, switchTab, getRootState} from '@/constants/router' import {emitDeepLink} from '@/router-v2/deep-link-emitter' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' @@ -14,7 +12,6 @@ import {openAppSettings} from '@/util/storeless-actions' type Store = { hasPermissions: boolean justSignedUp: boolean - pendingPushNotification?: T.Push.PushNotification showPushPrompt: boolean token: string } @@ -22,20 +19,17 @@ type Store = { type State = Store & { dispatch: { checkPermissions: () => Promise - clearPendingPushNotification: () => void deleteTokenForLogout: () => Promise - handlePush: (notification: T.Push.PushNotification) => void initialPermissionsCheck: () => void rejectPermissions: () => void requestPermissions: () => void resetState: () => void - setPendingPushNotification: (notification: T.Push.PushNotification) => void setPushToken: (token: string) => void showPermissionsPrompt: (p: {show?: boolean; persistSkip?: boolean; justSignedUp?: boolean}) => void } } import {isDevApplePushToken} from '@/local-debug' -import {checkPushPermissions, getRegistrationToken, iosGetHasShownPushPrompt, requestPushPermissions, removeAllPendingNotificationRequests} from 'react-native-kb' +import {checkPushPermissions, getRegistrationToken, iosGetHasShownPushPrompt, requestPushPermissions} from 'react-native-kb' export const tokenType = isMobile ? isIOS ? (isDevApplePushToken ? 'appledev' : 'apple') : 'androidplay' @@ -51,7 +45,6 @@ const desktopInitialStore: Store = { const mobileInitialStore: Store = { hasPermissions: true, justSignedUp: false, - pendingPushNotification: undefined, showPushPrompt: false, token: '', } @@ -64,14 +57,11 @@ export const usePushState = Z.createZustand('push', (set, get) => { checkPermissions: async () => { return Promise.resolve(false) }, - clearPendingPushNotification: () => {}, deleteTokenForLogout: async () => {}, - handlePush: () => {}, initialPermissionsCheck: () => {}, rejectPermissions: () => {}, requestPermissions: () => {}, resetState: Z.defaultReset, - setPendingPushNotification: () => {}, setPushToken: () => {}, showPermissionsPrompt: () => {}, } @@ -110,41 +100,6 @@ export const usePushState = Z.createZustand('push', (set, get) => { } } - const handleLoudMessage = async (notification: T.Push.PushNotification) => { - if (notification.type !== 'chat.newmessage') { - return - } - if (!notification.userInteraction) { - logger.warn('[Push] handleLoudMessage: ignore non userInteraction') - return - } - - const {conversationIDKey, unboxPayload, membersType} = notification - - const rootState = getRootState() - const topRoute = rootState?.routes?.at(-1) - const alreadyOnConv = - topRoute?.name === 'chatConversation' && - (topRoute.params as {conversationIDKey?: string} | undefined)?.conversationIDKey === conversationIDKey - if (!alreadyOnConv) { - const targetUid = 'forUid' in notification ? notification.forUid : undefined - emitDeepLink(`keybase://convid/${conversationIDKey}`, { - targetUid, - }) - } - if (unboxPayload && membersType && !isIOS) { - try { - await T.RPCChat.localUnboxMobilePushNotificationRpcPromise({ - convID: conversationIDKey, - membersType, - payload: unboxPayload, - }) - } catch { - logger.info('[Push] failed to unbox message from payload') - } - } - } - const dispatch: State['dispatch'] = { checkPermissions: async () => { const permissions = await checkPermissionsFromNative() @@ -168,11 +123,6 @@ export const usePushState = Z.createZustand('push', (set, get) => { return false } }, - clearPendingPushNotification: () => { - set(s => { - s.pendingPushNotification = undefined - }) - }, deleteTokenForLogout: async () => { try { const deviceID = useCurrentUserState.getState().deviceID @@ -192,98 +142,6 @@ export const usePushState = Z.createZustand('push', (set, get) => { logger.error('[PushToken] delete failed', e) } }, - handlePush: notification => { - const f = async () => { - try { - const forUid = 'forUid' in notification ? notification.forUid : undefined - const navigationIntentOptions = { - targetUid: forUid, - } - - if (forUid) { - const currentUid = useCurrentUserState.getState().uid - if (forUid !== currentUid) { - const userInteraction = 'userInteraction' in notification ? notification.userInteraction : false - if (!userInteraction) { - logger.info('[Push] notification for different account but no userInteraction, skipping') - return - } - const {configuredAccounts, dispatch: configDispatch} = useConfigState.getState() - const account = configuredAccounts.find(acc => acc.uid === forUid) - if (!account) { - logger.info('[Push] notification forUid not in configured accounts yet, waiting to retry') - set(s => { - s.pendingPushNotification = notification - }) - return - } - if (!account.hasStoredSecret) { - logger.info('[Push] account has no stored secret, cannot switch') - return - } - if (useConfigState.getState().userSwitching) { - logger.info('[Push] switch already in progress for this account, skipping duplicate') - return - } - logger.info('[Push] switching to account for notification tap') - configDispatch.setUserSwitching(true) - set(s => { - s.pendingPushNotification = notification - }) - configDispatch.login(account.username, '') - return - } - } - - switch (notification.type) { - case 'chat.readmessage': - if (notification.badges === 0) { - removeAllPendingNotificationRequests() - } - break - case 'chat.newmessageSilent_2': - // entirely handled by go on ios and in onNotification on Android - break - case 'chat.newmessage': - await handleLoudMessage(notification) - break - case 'follow': - // We only care if the user clicked while in session - if (notification.userInteraction) { - const {username} = notification - emitDeepLink(`keybase://profile/show/${username}`, navigationIntentOptions) - } - break - case 'device.revoked': - case 'device.new': - if (notification.userInteraction && useConfigState.getState().loggedIn) { - switchTab(Tabs.settingsTab) - navUpToScreen('devicesRoot') - } - break - case 'autoreset': - break - case 'chat.extension': - { - const {conversationIDKey} = notification - emitDeepLink(`keybase://convid/${conversationIDKey}`, navigationIntentOptions) - } - break - case 'settings.contacts': - if (useConfigState.getState().loggedIn) { - emitDeepLink('keybase://people', navigationIntentOptions) - } - break - } - } catch (e) { - if (__DEV__) { - console.error(e) - } - logger.error('[Push] unhandled', e) - } - } - ignorePromise(f()) - }, initialPermissionsCheck: () => { const f = async () => { const hasPermissions = await get().dispatch.checkPermissions() @@ -356,19 +214,7 @@ export const usePushState = Z.createZustand('push', (set, get) => { ignorePromise(f()) }, resetState: () => { - const pendingPushNotification = useConfigState.getState().userSwitching - ? get().pendingPushNotification - : undefined - set(s => ({ - ...initialStore, - dispatch: s.dispatch, - pendingPushNotification, - })) - }, - setPendingPushNotification: (notification: T.Push.PushNotification) => { - set(s => { - s.pendingPushNotification = notification - }) + set(s => ({...initialStore, dispatch: s.dispatch})) }, setPushToken: (token: string) => { set(s => { @@ -431,21 +277,3 @@ export const usePushState = Z.createZustand('push', (set, get) => { dispatch, } }) - -// A login error used to clear the pending push notification via a direct call -// from config's setLoginError. Subscribing here instead keeps config from -// importing push (breaks the config <-> push require cycle). -// -// Guard against HMR: the config store instance (and its subscribers) survive -// hot reloads via Z.createZustand's registry, but this module re-evaluates, so -// an unguarded subscribe would register a duplicate every reload. -// eslint-disable-next-line -const _g = globalThis as any -if (!__DEV__ || !_g.__pushLoginErrorSubscribed) { - if (__DEV__) _g.__pushLoginErrorSubscribed = true - useConfigState.subscribe((s, p) => { - if (s.loginError && s.loginError !== p.loginError) { - usePushState.getState().dispatch.clearPendingPushNotification() - } - }) -} diff --git a/shared/stores/tests/config.test.ts b/shared/stores/tests/config.test.ts index b131149fa66c..b607e6961daf 100644 --- a/shared/stores/tests/config.test.ts +++ b/shared/stores/tests/config.test.ts @@ -1,5 +1,6 @@ /// import * as T from '../../constants/types' +import * as Tabs from '../../constants/tabs' import {RPCError} from '../../util/errors' import {noConversationIDKey} from '../../constants/types/chat/common' import {useConfigState} from '../config' @@ -18,7 +19,6 @@ const resetConfigState = () => { }, startup: { conversation: noConversationIDKey, - followUser: '', loaded: false, }, userSwitching: false, @@ -39,20 +39,17 @@ test('setStartupDetails only records the first startup payload', () => { dispatch.setStartupDetails({ conversation: 'first-convo' as any, - followUser: 'alice', - tab: undefined, + tab: Tabs.chatTab, }) dispatch.setStartupDetails({ conversation: 'second-convo' as any, - followUser: 'bob', - tab: undefined, + tab: Tabs.peopleTab, }) expect(useConfigState.getState().startup).toEqual({ conversation: 'first-convo', - followUser: 'alice', loaded: true, - tab: undefined, + tab: Tabs.chatTab, }) }) diff --git a/shared/stores/tests/push.desktop.test.ts b/shared/stores/tests/push.desktop.test.ts index 8f640c662f53..682cfca6599a 100644 --- a/shared/stores/tests/push.desktop.test.ts +++ b/shared/stores/tests/push.desktop.test.ts @@ -11,7 +11,6 @@ test('desktop push store reports resettable defaults', async () => { await expect(dispatch.checkPermissions()).resolves.toBe(false) - dispatch.clearPendingPushNotification() await dispatch.deleteTokenForLogout() dispatch.initialPermissionsCheck() dispatch.rejectPermissions() From b2f5d98d1682c96f0632bdb39cde086c7f088917 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 14:40:43 -0400 Subject: [PATCH 5/6] fix(push): a logout drops the pending tap whatever account it names Two holes found reviewing the tap path. dropOnFailure asked tapForOtherAccount, so a tap for the account being logged out of was kept: it is not "for another account" while that uid is still set, and the teardown clearing the uid then makes it one, so check() logged the user straight back into the account they just left. The drop is account-blind now, which is what the pending-push clear it replaces did. The connect-time peek ran alongside the subscribe rather than after it. pushTapRouteAvailable is filtered per connection on the App channel, so a tap landing between the peek's reply and the service applying the subscribe had no reader left at all -- and on Android the tap really is delivered on another thread while the client is coming up. --- shared/constants/init/shared.tsx | 10 +++++++++- shared/router-v2/account-link-switch.test.ts | 13 +++++++++++++ shared/router-v2/account-link-switch.tsx | 17 +++++++++++++---- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 2e6e29eb48ae..a75b05540974 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -440,7 +440,15 @@ export const onEngineConnected = () => { awaitSessionAgain() subscription = subscribe() - ignorePromise(drainPushTapRoute()) + // Peek only once the subscribe has been answered. pushTapRouteAvailable is filtered per + // connection on the App channel, so a tap landing between an earlier peek's reply and the + // service applying this subscribe would have no reader left at all: the nudge is dropped for + // a connection that has not subscribed yet, and the peek has already answered null. + const subscribedThenDrain = async () => { + await subscription + await drainPushTapRoute() + } + ignorePromise(subscribedThenDrain()) useDaemonState.getState().dispatch.startHandshake() } diff --git a/shared/router-v2/account-link-switch.test.ts b/shared/router-v2/account-link-switch.test.ts index 1cb85b304bf1..9b9cd1361667 100644 --- a/shared/router-v2/account-link-switch.test.ts +++ b/shared/router-v2/account-link-switch.test.ts @@ -147,6 +147,19 @@ test('logging out drops a tap for another account', () => { expect(useNavigationIntentsState.getState().intent).toBeUndefined() }) +// The tap is not "for another account" while the uid being logged out of is still set, so an +// account-relative drop keeps it -- and the teardown clearing the uid then makes it one, which +// logs the user straight back into the account they just left. +test('logging out drops a tap for the account being logged out of', () => { + tapFor(currentAccount.uid) + + useConfigState.setState({loggedIn: false, userSwitching: false}) + useCurrentUserState.setState({uid: '', username: ''}) + + expect(login).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + test('a foreign link naming a stored account never switches', () => { emitDeepLink(`keybase://profile/show/${otherAccount.username}`) diff --git a/shared/router-v2/account-link-switch.tsx b/shared/router-v2/account-link-switch.tsx index f50289d52989..0f35c6ae1c32 100644 --- a/shared/router-v2/account-link-switch.tsx +++ b/shared/router-v2/account-link-switch.tsx @@ -6,9 +6,15 @@ import {useNavigationIntentsState} from '@/stores/navigation-intents' type ConfigState = ReturnType -const tapForOtherAccount = () => { +// Every intent carrying a targetUid, which is every tap and only a tap. +const pendingTap = () => { const {intent} = useNavigationIntentsState.getState() - return intent?.targetUid && intent.targetUid !== useCurrentUserState.getState().uid ? intent : undefined + return intent?.targetUid ? intent : undefined +} + +const tapForOtherAccount = () => { + const intent = pendingTap() + return intent && intent.targetUid !== useCurrentUserState.getState().uid ? intent : undefined } // A tapped push for another account waits in the intent store until that account is current. This @@ -45,9 +51,12 @@ export const subscribeIntentAccountSwitch = () => { const loginFailed = !!s.loginError && s.loginError !== old.loginError const loggedOut = s.loggedIn !== old.loggedIn && !s.loggedIn && !s.userSwitching if (!loginFailed && !loggedOut) return - const intent = tapForOtherAccount() + // Account-blind, unlike the switch above: a tap for the account being logged out of is not + // "for another account" while the uid is still set, but it is read as one the moment the + // teardown clears the uid, and check() would then log the user straight back in. + const intent = pendingTap() if (!intent) return - logger.info('[AccountLink] dropping a tap for another account after a failed switch or logout') + logger.info('[AccountLink] dropping a tap after a failed switch or logout') useNavigationIntentsState.getState().dispatch.acknowledge(intent.id) } const unsubs = [ From 520d69aa7b88c947eadb26080ca5c03f73adc08c Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:32:12 -0400 Subject: [PATCH 6/6] refactor(push): drop chat.extension from the no-route push types Nothing sends a chat.extension push anymore, and an unknown type already resolves to no route, so the entry and its test case were dead. --- go/libkb/pushtap.go | 1 - go/libkb/pushtap_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/go/libkb/pushtap.go b/go/libkb/pushtap.go index e44993fe9d70..763d578d170b 100644 --- a/go/libkb/pushtap.go +++ b/go/libkb/pushtap.go @@ -78,7 +78,6 @@ func (p *PendingPushTap) Ack(id int) bool { // are acted on natively and here, and have no screen of their own. var pushTapNoRouteTypes = map[string]bool{ "autoreset": true, - "chat.extension": true, "chat.failedpending": true, "chat.newmessageSilent_2": true, "chat.readmessage": true, diff --git a/go/libkb/pushtap_test.go b/go/libkb/pushtap_test.go index 857a005c926e..5cf361fbae40 100644 --- a/go/libkb/pushtap_test.go +++ b/go/libkb/pushtap_test.go @@ -62,7 +62,6 @@ func TestResolvePushTap(t *testing.T) { }, {"read receipt", `{"type":"chat.readmessage","b":0,"message":"Your contact x"}`, nil}, {"silent chat", `{"type":"chat.newmessageSilent_2","c":"0000ab"}`, nil}, - {"extension", `{"type":"chat.extension","convID":"0000ab"}`, nil}, {"autoreset", `{"type":"autoreset","uid":"u1"}`, nil}, {"failed pending", `{"type":"chat.failedpending","convID":"0000ab","uid":""}`, nil}, {"an unknown type opens nothing", `{"type":"something.new","uid":"u1"}`, nil},