[native] Remove background identity login handler
AbandonedPublic
Actions

Authored by tomek on Feb 27 2025, 6:22 AM.

Details

Reviewers

kamil
bartek

Summary

When this handler is present, clearing the state for SIWE users results in forcing a logout, which interferes with sensitive data cleanup - that sometimes results in the data staying in the store.
We probably no longer need this handler because we can assume that all the users are logged in to identity. I might be wrong, though, so this diff is more like a proposal and a place for discussion.

https://linear.app/comm/issue/ENG-10271/generating-backups-for-siwe-users-who-used-v1-fallback-doesnt-work

Depends on D14416

Test Plan

Wipe state and kill the app. Verify that the alert store doesn't contain recent alert info.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Feb 27 2025, 6:22 AM

Herald added a subscriber: ashoat. · View Herald TranscriptFeb 27 2025, 6:22 AM

Harbormaster completed remote builds in B33643: Diff 47292.Feb 27 2025, 6:55 AM

tomek requested review of this revision.Feb 27 2025, 6:55 AM

Should we bump the version in keyserver/src/session/version.js along with this change?

tomek added a child revision: D14425: [native] Schedule backup generation and upload after saving SIWE secrets.Feb 28 2025, 5:57 AM

Bump min code version

tomek added inline comments.Feb 28 2025, 6:18 AM

keyserver/src/session/version.js
13	If we want to bump the version to the one where Identity login is supported, we need to make use Identity service also supports such a version. This means that we should use at least `446` https://github.com/CommE2E/comm/blob/56c9645dd23fce589df4501a964abeba1eeed602/services/identity/src/constants.rs#L268. And we don't have a good reason to use a higher version.

Harbormaster completed remote builds in B33654: Diff 47307.Feb 28 2025, 6:35 AM

This looks reasonable to me, accepting to unblock, but I don't have the full context about BackgroundIdentityLoginHandler purposes, so feel free to add @bartek or @ashoat as blocking, if you think this is needed

This revision is now accepted and ready to land.Feb 28 2025, 7:50 AM

tomek removed a parent revision: D14416: [native] Avoid running the handler multiple times.Feb 28 2025, 8:55 AM

tomek removed a child revision: D14425: [native] Schedule backup generation and upload after saving SIWE secrets.

This change is controversial, and finding a better solution might be complicated - abandoning the diff.