What is a scenario when this could happen?

@varun didn't explain this in the diff or link to the related task here, but basically the goal is to force log-out all web users and to force them to re-auth using the identity service. (For password users on native, we can transparently re-auth in the background because we have the credentials cached locally. Wallet users on native are unclear to me; see comment I just wrote here.)

Why do we need an additional handler - isn't removing the token associated with a logout action?

The idea is that we will migrate by flipping usingCommServicesAccessToken. When the web client loads up again it will see dataLoaded but !hasAccessToken, and so it will trigger log-out.

3. How the callLogOut method could work? It uses an authenticated Identity client, which requires the access token when calling its functions. Before every call to it, we call ensureThatWorkerClientAuthMetadataIsCurrent which would try to create a wrapper without an access token.

Looks like identityClient.logOut is wrapped in a try-catch that ignores all caught exceptions. The Identity service client is not initialized error thrown here should prevent the identity service from being called, but will otherwise be ignored, which is arguably exactly what we want to see here. That said I wouldn't be opposed to adding an || !commServicesAccessToken condition here.

Separately, I'd like to see the test plan extended. @varun, could you try to simulate what would happen to a web client the first time it sees usingCommServicesAccessToken get flipped?

Note for the corresponding native task ENG-6546. We'd ideally try to do the same kind of testing there, but I'm guessing the transparent re-auth won't work there because our "reserved usernames" flow is only set up to work in production. The issue is that your client will start out with a userID that it got from your local keyserver MariaDB, and then will be get reassigned a different userID from the staging keyserver.

When testing that one, you could try and see if the login actions from the identity service and authoritative keyserver are successful. The client will surely be broken after that, but if the login actions look successful then it's probably fine.

That said I wouldn't be opposed to adding an || !commServicesAccessToken condition here.

Did you link the right line?

In D11705#339275, @varun wrote:

That said I wouldn't be opposed to adding an || !commServicesAccessToken condition here.

Did you link the right line?

I misread

Separately, I'd like to see the test plan extended. @varun, could you try to simulate what would happen to a web client the first time it sees usingCommServicesAccessToken get flipped?

discussed this with @ashoat IRL. a full end-to-end test would require setting usingCommServicesAccessToken to false, registering a user with the keyserver, logging in on web, and then flipping usingCommServicesAccessToken to true. however, my local keyserver/mariadb isn't designed to handle switching between false and true.

i tried the aforementioned workflow, and i got logged out as expected, but saw some keyserver errors. however, they were websocket errors and seemed unrelated to my changes. they probably occurred because my dev environment can't support flipping the usingCSAT switch on and off

The keyserver errors:

[NODEM] ServerError: socket_deauthorized
[NODEM]     at WebSocket.<anonymous> (file:///Users/varun/Code/comm/keyserver/dist/socket/socket.js:128:17)
[NODEM]     at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
[NODEM]   payload: undefined,
[NODEM]   platformDetails: undefined,
[NODEM]   sanitizedInput: undefined
[NODEM] }
[NODEM] /Users/varun/Code/comm/node_modules/@commapp/olm/olm.js:19
[NODEM] if(ia)m=l?require("path").dirname(m)+"/":__dirname+"/",na=()=>{ma||(fs=require("fs"),ma=require("path"))},ja=function(b,c){na();b=ma.normalize(b);return fs.readFileSync(b,c?void 0:"utf8")},la=b=>{b=ja(b,!0);b.buffer||(b=new Uint8Array(b));return b},ka=(b,c,d)=>{na();b=ma.normalize(b);fs.readFile(b,function(e,f){e?d(e):c(f.buffer)})},1<process.argv.length&&process.argv[1].replace(/\\/g,"/"),process.argv.slice(2),process.on("uncaughtException",function(b){throw b;}),process.on("unhandledRejection",
[NODEM]                                                                                                                                                                                                                                                                                                                                                                                                                                                                           ^
[NODEM] 
[NODEM] Invariant Violation: should be set
[NODEM]     at invariant (/Users/varun/Code/comm/node_modules/invariant/invariant.js:40:15)
[NODEM]     at WebSocket.<anonymous> (file:///Users/varun/Code/comm/keyserver/dist/socket/socket.js:177:9)
[NODEM]     at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
[NODEM]   framesToPop: 1
[NODEM] }
[NODEM] 
[NODEM] Node.js v20.10.0

What do you mean by that? After you go with https://www.notion.so/commapp/Running-two-keyservers-0d373941f2494d949846da02752b91db instructions, you can change the client's flags however you want - at least that's what I'm doing and it works reliably. So my keyserver is always configured in a multi-keyserver manner.

Hmm, that's not been my experience. I've found that an environment configured with usingCommServicesAccessToken doesn't really work if you flip that constant. As an example, if you were to register an account with usingCommServicesAccessToken, and then flip it to false, you won't be able to log in with that account anymore.

In D11705#339313, @tomek wrote:

logging in on web, and then flipping usingCommServicesAccessToken to true

I don't think there's a chance that a flag will flip while the app is working. Have you tried refreshing the app after the flag is set? (this would emulate receiving a new web code version)

yeah tried this and was logged out. tested this in the subsequent diff as well and the modal was displayed, so we know that logout was triggered by the handler component