[keyserver] try registerUser if loginUser fails in keyserver logInResponder
ClosedPublic
Actions

Authored by varun on Mar 6 2023, 10:38 PM.

Details

Reviewers

Commits

Summary

Changes are as follows:

Modified Identity service to send back "not found" status to caller (it was previously eating this up and closing the connection)
Modified loginUser function in rust-node-addon to propagate the not found error to js if encountered
Wrapped the loginUser call in logInResponder in a try-catch so that if we get back the "not found" error, we attempt a registerUser call instead

Normally I'd make this 3 diffs, but this is urgent

Test Plan

tried logging in with a user in my local MariaDB instance but not in localstack DDB, saw that a new entry was made in DDB upon successful login

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

varun requested review of this revision.Mar 6 2023, 10:54 PM

Rust looks OK.

keyserver/src/responders/user-responders.js
441 ↗	(On Diff #23473)	Generally, I'm not a fan of recognizing errors by comparing message strings, if anybody ever changes that string, this stops working. Ideally we'd want error messages to have unified format, e.g. starting with some error code, like `ERR_USER_NOT_FOUND: user not found`. Sometimes regex match is sufficient. But as this diff seems urgent, I'm not blocking on this comment - this is just something worth considering in the future.

ashoat added inline comments.

keyserver/src/responders/user-responders.js
441 ↗	(On Diff #23473)	I agree with @bartek, but also don't want to block this diff. @varun, could you create a follow-up task and link it here before landing?