[lib][native] Rename resolveInvalidatedCookie to resolveKeyserverSessionInvalidationUsingNativeCredentials
ClosedPublic
Actions

Authored by ashoat on Jan 8 2024, 9:41 AM.

Details

Reviewers

inka
varun
tomek

Commits

rCOMM9d8b908c73b8: [lib][native] Rename resolveInvalidatedCookie to…

Summary

Similar to the parent diff. As part of ENG-5940, we're going to be reducing our reliance on this function. If the client is usingCommServicesAccessToken, then instead of calling this function we'll just call the Olm auth responder.

As such, it's probably a good idea to rename the function to be more clear about what it does.

Depends on D10576

Test Plan

Flow

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Jan 8 2024, 9:41 AM

ashoat added a child revision: D10578: [lib] Introduce canResolveKeyserverSessionInvalidation.Jan 8 2024, 9:57 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 8 2024, 10:10 AM

Harbormaster failed remote builds in B25610: Diff 35405!

ashoat requested review of this revision.Jan 8 2024, 11:48 AM

inka added a comment.Jan 9 2024, 2:45 AM

This comment was removed by inka.

In D9648 I was asked to create a task to update the call to getOlmSessionInitializationData in resolveInvalidatedCookie. I created ENG-5658. It was later merged with ENG-5940 which this diff is a part of. I'm flagging this so that it doesn't get lost, because it is no longer in the task description, and I don't see any diffs addressing it

getOlmSessionInitializationData was since moved into getInitialNotificationsEncryptedMessage, but the problem is still the same - we want to move away from using callServerEndpoint and useServerCall directly.

This revision is now accepted and ready to land.Jan 9 2024, 3:06 AM

Thanks for flagging that @inka. One question for you about this part:

If on the other hand it turns out that getOlmSessionInitializationData should call the keyserver, we have to refactor it - we want to move away from using CallServerEndpoint anywhere in the codebase. In such case it should be refactored like other actions - to take CallKeyserverEndpoint, and should be called using useKeyserverCall.

I think we might want to keep the old code around for a while so we can safely roll back the identity launch if there are issues. For some time we'll be gating it on usingCommServicesAccessToken.

Do you think we'll need to refactor getOlmSessionInitializationData in the interim then?

In D10577#306133, @ashoat wrote:

Thanks for flagging that @inka. One question for you about this part:

If on the other hand it turns out that getOlmSessionInitializationData should call the keyserver, we have to refactor it - we want to move away from using CallServerEndpoint anywhere in the codebase. In such case it should be refactored like other actions - to take CallKeyserverEndpoint, and should be called using useKeyserverCall.

I think we might want to keep the old code around for a while so we can safely roll back the identity launch if there are issues. For some time we'll be gating it on usingCommServicesAccessToken.

Do you think we'll need to refactor getOlmSessionInitializationData in the interim then?

We discussed this on our 1:1 - this is not a priority, because the current code works, but we want to eventually refactor getOlmSessionInitializationData to use callKeyserverEndpoint. ENG-5658 was reopened to address this.

This revision was landed with ongoing or failed builds.Jan 10 2024, 5:43 AM

Closed by commit rCOMM9d8b908c73b8: [lib][native] Rename resolveInvalidatedCookie to… (authored by ashoat). · Explain Why

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMM9d8b908c73b8: [lib][native] Rename resolveInvalidatedCookie to….