Page MenuHomePhabricator

[keyserver] Do not verify identity logged for webapp and landing
ClosedPublic

Authored by will on Jul 29 2024, 8:39 PM.
Tags
None
Referenced Files
F3366036: D12927.id43155.diff
Mon, Nov 25, 9:14 AM
F3366015: D12927.id42919.diff
Mon, Nov 25, 9:06 AM
F3362681: D12927.diff
Sun, Nov 24, 11:10 PM
Unknown Object (File)
Fri, Nov 22, 4:31 AM
Unknown Object (File)
Fri, Nov 22, 4:06 AM
Unknown Object (File)
Wed, Nov 20, 9:39 PM
Unknown Object (File)
Wed, Nov 20, 9:39 PM
Unknown Object (File)
Wed, Nov 20, 9:39 PM
Subscribers

Details

Summary

To avoid having to provide user credentials in remote terraform services for webapp and landing, we should only run identity login for non-landing and non-webapp nodes which is specified by running this on
only primary nodes (when unspecified, nodes are also primary) and secondary nodes.

Test Plan

terraform apply. On running landing and webapp nodes, I no longer receive errors about missing user credentials

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

make secondary false by default

will requested review of this revision.Jul 29 2024, 9:01 PM
ashoat added inline comments.
keyserver/src/keyserver.js
137–157 ↗(On Diff #42919)

This is getting really deeply nested. What do you think of this instead?

await (async () => {
  // Should not be run by Landing or WebApp nodes
  if (!isPrimaryNode && !isSecondaryNode) {
    return;
  }

  // We await here to ensure that the keyserver has been provisioned a
  // commServicesAccessToken. In the future, this will be necessary for
  // many keyserver operations.
  const identityInfo = await verifyUserLoggedIn();

  if (!isPrimaryNode) {
    return;
  }

  // We don't await here, as Tunnelbroker communication is not needed
  // for normal keyserver behavior yet. In addition, this doesn't
  // return information useful for other keyserver functions.
  ignorePromiseRejections(
    createAndMaintainTunnelbrokerWebsocket(null),
  );

  if (process.env.NODE_ENV !== 'development') {
    return;
  }

  await createAuthoritativeKeyserverConfigFiles(
    identityInfo.userId,
  );
})();
This revision is now accepted and ready to land.Jul 30 2024, 10:55 PM
keyserver/src/keyserver.js
137–157 ↗(On Diff #42919)

Yeah. I like this way better. It's much cleaner

Will include in next rebase