[lib] Introduce `getPublicKeyFromSIWEStatement(...)`
ClosedPublic
Actions

Authored by atul on Dec 31 2022, 3:14 AM.

Details

Reviewers

ashoat
tomek

Commits

rCOMMc9ef956951bf: [lib] Introduce `getPublicKeyFromSIWEStatement(...)`

Summary

Introduce getPublicKeyFromSIWEStatement(...) to pull public key out of SIWE statement which includes a public key. Will be used on the keyserver to retrieve the public key from the signed SIWE message.

Depends on D6130

Test Plan

included a unit test (will include more later)

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D6131 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Dec 31 2022, 3:14 AM

add isValidSIWEStatementWithPublicKey(...) precondition

atul published this revision for review.Dec 31 2022, 3:17 AM

Harbormaster completed remote builds in B15003: Diff 20480.Dec 31 2022, 3:27 AM

Harbormaster completed remote builds in B15004: Diff 20481.Dec 31 2022, 3:30 AM

atul added a child revision: D6132: [landing] Include `Device IdPubKey` in SIWE message statement.Dec 31 2022, 3:42 AM

Accepting to unblock, but in a less urgent context I would request changes. If you prefer, feel free to create (and link) a follow-up task to address before landing, but I feel strongly that we should just have one RegEx literal with a capture group instead of two separate RegEx literals

lib/utils/siwe-utils.js
64 ↗	(On Diff #20481)	Not sure why we're doing this twice. Can't we put a capture group in `siweStatementWithPublicKeyRegex`? Doesn't this allow the statement to have other parts match `IdPubKey`? I know right now there's no other part of the text that can match, but hypothetically that could change in the future. I think it'd better to use a capture group so that we know we're capturing the right part of the statement

This revision is now accepted and ready to land.Jan 1 2023, 8:02 PM

In D6131#183954, @ashoat wrote:

Accepting to unblock, but in a less urgent context I would request changes. If you prefer, feel free to create (and link) a follow-up task to address before landing, but I feel strongly that we should just have one RegEx literal with a capture group instead of two separate RegEx literals

Created a follow-up task: https://linear.app/comm/issue/ENG-2624/create-one-siwestatement-regex-literal-w-capture-group-instead-of-two since I'm confident this works for now. I'll read up on capture groups, I haven't used them before.

atul added inline comments.Jan 3 2023, 2:06 PM

lib/utils/siwe-utils.js
64 ↗	(On Diff #20481)	Created https://linear.app/comm/issue/ENG-2624/create-one-siwestatement-regex-literal-w-capture-group-instead-of-two Yeah, if we had a 43 character base64 encoded substring that preceded the `IdPubKey`. Will use capture group instead as part of https://linear.app/comm/issue/ENG-2624/create-one-siwestatement-regex-literal-w-capture-group-instead-of-two