Page MenuHomePhabricator

[keyserver] Introduce `isCookieMissingSignedIdentityKeysBlob`
ClosedPublic

Authored by atul on Mar 12 2023, 4:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 15, 11:36 PM
Unknown Object (File)
Sat, Jun 15, 10:05 AM
Unknown Object (File)
Wed, Jun 12, 1:30 AM
Unknown Object (File)
Thu, Jun 6, 8:22 AM
Unknown Object (File)
Sun, Jun 2, 3:43 PM
Unknown Object (File)
Sun, Jun 2, 3:43 PM
Unknown Object (File)
Thu, May 30, 12:28 PM
Unknown Object (File)
Thu, May 30, 11:36 AM
Subscribers
None

Details

Summary

Checks if there's an existing row in cookies table with id=cookieID with a NULL signed_identity_keys column.

We're going to use this check in handleInitialClientSocketMessage to determine whether or not to send SIGNED_IDENTITY_KEYS_BLOB request to client from keyserver.


Depends on D7049

Test Plan

Called function from handleInitialClientSocketMessage:

if (viewer.cookieID) {
  isCookieMissingSignedIdentityKeysBlob(viewer.cookieID);
}

and set breakpoint within isCookieMissingSignedIdentityKeysBlob to check whether queryResult was as expected when signed_identity_keys was NULL and a valid stringified SignedIdentityKeysBlob:

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Mar 12 2023, 4:16 PM

I think this diff would be easier to review if you included this new function's usage in the same diff

keyserver/src/session/cookies.js
820–826

I'm not a fan of if () return true else return false constructions. This sort of construction can always be rewritten:

return !!(
  queryResult.length === 1 &&
  queryResult[0].signed_identity_keys === null
);

Separately, I might consider either making the comparison > 0 or factoring out the check that there aren't 2 or more results into an invariant

This revision is now accepted and ready to land.Mar 13 2023, 11:58 AM
keyserver/src/session/cookies.js
820–826

Personally prefer being explicit instead of the casting trick... but I'll update this diff.

keyserver/src/session/cookies.js
820–826

Separately, I might consider either making the comparison > 0 or factoring out the check that there aren't 2 or more results into an invariant

It's totally impossible for more than one row to have the same id so I think it's fine to check === 1

return !!(
  queryResult.length === 1 &&
  queryResult[0].signed_identity_keys === null
);

Shouldn't this just be

return (
  queryResult.length === 1 &&
  queryResult[0].signed_identity_keys === null
);
keyserver/src/session/cookies.js
820–826

Yeah

remove if then and just return

This revision was landed with ongoing or failed builds.Mar 14 2023, 12:24 PM
This revision was automatically updated to reflect the committed changes.