[native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler
ClosedPublic
Actions

Authored by ashoat on Jul 6 2024, 2:39 PM.

Details

Reviewers

varun
tomek

Commits

rCOMM1419a4bd0741: [native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler

Summary

We don't need to do it here, since we're already auth'd with the authoritative keyserver.

We just need to perform identity auth.

This mitigates ENG-8725 and ENG-8726, and addresses ENG-8779.

Test Plan

Confirm that I'm able to go from a release build running with usingCSAT=false to one running with usingCSAT=true. Exact instructions here. Make sure that the two "mitigated" issues are no longer present, and that the upgrade goes without issue (except for ENG-8778, which is a separate issue)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Jul 6 2024, 2:39 PM

ashoat added a child revision: D12679: [native] Limit BackgroundIdentityLoginHandler to one try per startup.

Harbormaster completed remote builds in B30174: Diff 42085.Jul 6 2024, 2:56 PM

ashoat requested review of this revision.Jul 6 2024, 2:56 PM

we're already auth'd with the authoritative keyserver

Why is it guaranteed?

This revision is now accepted and ready to land.Jul 8 2024, 1:40 AM

In D12678#359222, @tomek wrote:

we're already auth'd with the authoritative keyserver

Why is it guaranteed?

That's a good question. The dataLoaded condition generally guarantees this, but I'm currently investigating a situation in ENG-8772 where dataLoaded is true despite the user being logged out (currentUserInfo is anonymous).

As part of that investigation, I'll probably conclude that we need to block BackgroundIdentityLoginHandler on both conditions in LogInHandler: loggedIn and navLoggedIn. I've created ENG-8785, and we can track this work there.

Closed by commit rCOMM1419a4bd0741: [native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler (authored by ashoat). · Explain WhyJul 8 2024, 8:10 AM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMM1419a4bd0741: [native] Don't bother with keyserver auth in BackgroundIdentityLoginHandler.

Revision Contents
Changeset List

Path

Size

native/

components/

background-identity-login-handler.react.js

23 lines

Diff 42085

View Options

native/components/background-identity-login-handler.react.js

Show First 20 Lines • Show All 240 Lines • ▼ Show 20 Lines	const messageDatas = userIDs.map(otherUserID => ({
creatorID: viewer.userID,		creatorID: viewer.userID,
creatorFID: viewerFID,		creatorFID: viewerFID,
targetID: otherUserID,		targetID: otherUserID,
targetFID: request.userIDsToFID[otherUserID],		targetFID: request.userIDsToFID[otherUserID],
time: now,		time: now,
operation: 'farcaster_mutual',		operation: 'farcaster_mutual',
}));		}));
await createMessages(viewer, messageDatas, 'broadcast');		await createMessages(viewer, messageDatas, 'broadcast');
		} else if (request.action === relationshipActions.ACKNOWLEDGE) {
		updateIDs.push(...userIDs);

		const insertRows = userIDs.map(userID => {
		const [user1, user2] = sortUserIDs(viewer.userID, userID);
		return { user1, user2, status: undirectedStatus.KNOW_OF };
		});

		const updateDatas = await updateChangedUndirectedRelationships(insertRows);
		inkaAuthorUnsubmitted Done Inline Actions `updateChangedUndirectedRelationships` only updates the relationship if there was none, or the relationship status provided it greater than the one in the db. This will not be the case here, because `KNOW_OF` is `0`-the lowest possible value. So we will not override a more meaningful relationship. inka: `updateChangedUndirectedRelationships` only updates the relationship if there was none, or the…
		tomekUnsubmitted Not Done Inline Actions Makes sense, but is there an easy way of checking if the relationship doesn't need to be updated? tomek: Makes sense, but is there an easy way of checking if the relationship doesn't need to be…
		inkaAuthorUnsubmitted Done Inline Actions The only thing this function does is checking if the relationship needs to be updated, and if so updates it. So I don't think it makes sense to duplicate this logic. We discussed this offline and agreed that this code makes sense and can be left as is. inka: The only thing this function does is checking if the relationship needs to be updated, and if…
		await createUpdates(updateDatas);
} else {		} else {
invariant(		invariant(
false,		false,
`action ${request.action} is invalid or not supported currently`,		`action ${request.action} is invalid or not supported currently`,
);		);
}		}

await createUpdates(		await createUpdates(
▲ Show 20 Lines • Show All 169 Lines • Show Last 20 Lines

[native] Don't bother with keyserver auth in BackgroundIdentityLoginHandlerClosedPublicActions