Details

Reviewers

ashoat
tomek

Commits

rCOMM7bd408fc265e: [keyserver] Introduce `siwe_nonces` table

Summary

Context: https://linear.app/comm/issue/ENG-2226/add-nonce-to-cookie-session

This table will hold nonces for the SIWE flow.

Entries will be created on calls to the siwe_nonce endpoint... at which point any "stale" nonces for a given ethereum_address will be clobbered/deleted.

Entries will be checked during calls to siwe_verify (tentative name) to ensure that the ethereum_address and nonce in the signed message match what's in the siwe_nonces table AND that the creation_time is < 30 minutes ago. After the entry is successfully checked it'll be deleted from the siwe_nonces table... we never care about the value of the nonce again.

Entries will also be "swept up" by some sort of "cron" script that runs every so often to clean up expired nonces.

Here's a rough sketch of the design that I'm working off of:

Test Plan

Table created as expected (migration):

Table created as expected (setup db):
TBD

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Dec 16 2022, 8:56 AM

atul edited the summary of this revision. (Show Details)Dec 16 2022, 8:57 AM

atul edited the test plan for this revision. (Show Details)

atul added inline comments.Dec 16 2022, 9:06 AM

keyserver/src/database/migration-config.js
126 ↗	(On Diff #19436)	Why 17 characters? The siwe:generateNonce() function makes a call to stablelib/random: randomStringForEntropy() with `96` as the argument for number of bits of entropy. `randomStringForEntropy` has some math for determining the length of these string. I called the function with `96` bits of entropy and the value of `length` was 17: Maybe we could just make it bigger to account for possibly using different `nonce` approach in the future, but I think this is fine for now