Paths

Table of Contentst

Differential D13667

[keyserver] sync platform details on keyserver start
ClosedPublic
Actions

Authored by varun on Oct 9 2024, 12:57 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sat, Nov 9, 1:43 AM

	Unknown Object (File)
	Sat, Nov 9, 12:42 AM

	Unknown Object (File)
	Fri, Nov 1, 2:19 PM

	Unknown Object (File)
	Fri, Nov 1, 12:22 PM

	Unknown Object (File)
	Fri, Nov 1, 8:23 AM

	Unknown Object (File)
	Wed, Oct 30, 11:01 PM

	Unknown Object (File)
	Wed, Oct 30, 7:21 PM

	Unknown Object (File)
	Tue, Oct 29, 10:02 PM

View All 52 Files

Subscribers

Details

Reviewers

ashoat
will

Commits

rCOMM856b61cb9083: [keyserver] sync platform details on keyserver start

Summary

if the primary node is able to fetch auth metadata from mariadb when the process starts, we should try syncing the keyserver's platform details.

if the node is unable to fetch auth metadata from mariadb, then the login or registration RPC will handle syncing platform details

Depends on D13666

Test Plan

ran yarn dev and my keyserver code version in ddb was updated to 129.
logged out of keyserver, cleared access token from metadata table, and ran yarn dev again. keyserver logged in to identity and platform details looked right in devices table. syncPlatformDetails function was not called

Diff Detail

Repository

Branch

keyserver-platform-details (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Oct 9 2024, 12:57 PM

Herald added a subscriber: tomek. · View Herald TranscriptOct 9 2024, 12:57 PM

Harbormaster completed remote builds in B32149: Diff 45022.Oct 9 2024, 1:17 PM

varun requested review of this revision.Oct 9 2024, 1:17 PM

check isPrimaryNode

varun planned changes to this revision.Oct 9 2024, 1:25 PM

fix logic

Harbormaster completed remote builds in B32151: Diff 45024.Oct 9 2024, 1:40 PM

varun added a reviewer: will.Oct 9 2024, 1:41 PM

varun added inline comments.

keyserver/src/keyserver.js
194–196 ↗	(On Diff #45025)	I'm a little confused why we were calling `verifyUserLoggedIn` from secondary nodes

Harbormaster completed remote builds in B32152: Diff 45025.Oct 9 2024, 1:51 PM

ashoat requested changes to this revision.Oct 9 2024, 5:06 PM

ashoat added inline comments.

keyserver/src/keyserver.js
194–196 ↗	(On Diff #45025)	Maybe because we wanted execution to halt when it throws?
202 ↗	(On Diff #45025)	It seems risky to call `authAndSaveIdentityInfo` from secondary nodes. We could have a race where multiple nodes save different identity info into the database. Instead, I think we should have the secondary nodes poll in a loop until the identity info is in the database. We should block execution until the identity info is in the database. Check out lines 250-255, which do something similar (make secondary nodes wait for the primary node to set up the database)

This revision now requires changes to proceed.Oct 9 2024, 5:06 PM

restructure identity auth code in keyserver.js

varun added inline comments.Oct 10 2024, 9:47 AM

keyserver/src/keyserver.js
183	I think identity auth should be required at this point since we frequently use getInboundKeys and other identity auth APIs on the keyserver

Harbormaster completed remote builds in B32181: Diff 45063.Oct 10 2024, 10:02 AM

ashoat requested changes to this revision.Oct 10 2024, 10:33 AM

ashoat added inline comments.

keyserver/src/keyserver.js
185	I think this async IIFE needs to be brought back
186	You're causing landing / webapp nodes to skip `cluster.fork` below

This revision now requires changes to proceed.Oct 10 2024, 10:33 AM

varun added inline comments.Oct 10 2024, 11:01 AM

keyserver/src/keyserver.js
186	ah you're right

bring back async iife

Harbormaster completed remote builds in B32183: Diff 45066.Oct 10 2024, 11:20 AM

varun added a child revision: D13694: [keyserver] move async iife to standalone function.Oct 10 2024, 11:31 AM

in the next diff i move the async IIFE to a standalone async function in a separate file

ashoat accepted this revision.Oct 10 2024, 11:52 AM

This revision is now accepted and ready to land.Oct 10 2024, 11:52 AM

Closed by commit rCOMM856b61cb9083: [keyserver] sync platform details on keyserver start (authored by varun). · Explain WhyOct 10 2024, 12:07 PM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMM856b61cb9083: [keyserver] sync platform details on keyserver start.

Revision Contents
Changeset List

Path

Size

keyserver/

src/

67 lines

user/

6 lines

utils/

identity-utils.js

15 lines

Diff 45063

keyserver/src/keyserver.js

Loading...

keyserver/src/user/login.js

Loading...

keyserver/src/utils/identity-utils.js

Loading...