Details

Reviewers

ashoat
will

Commits

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

rCOMM Comm

Branch

keyserver-platform-details (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

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

check isPrimaryNode

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

fix logic

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

ashoat requested changes to this revision.Wed, Oct 9, 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.Wed, Oct 9, 5:06 PM

restructure identity auth code in keyserver.js

keyserver/src/keyserver.js
183 ↗	(On Diff #45063)	I think identity auth should be required at this point since we frequently use getInboundKeys and other identity auth APIs on the keyserver

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

ashoat added inline comments.

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