[keyserver] make force login configurable
ClosedPublic
Actions

Authored by varun on Mar 8 2024, 8:25 PM.

Details

Reviewers

ashoat
inka

Commits

rCOMMe7c687014447: [keyserver] make force login configurable

Summary

dedup'd UserCredentials in login.js and checks.js
used forceLogin value from user_credentials.json in login.js

Depends on D11278

Test Plan

successfully logged in to identity from keyserver with forceLogin in user_credentials.json omitted, set to false, and set to true. tested that on the rust side, the correct value (None, Some(false), Some(true) respectively) was present each time

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Mar 8 2024, 8:25 PM

Herald added a subscriber: tomek. · View Herald TranscriptMar 8 2024, 8:25 PM

Harbormaster completed remote builds in B27408: Diff 37956.Mar 8 2024, 8:41 PM

varun requested review of this revision.Mar 8 2024, 8:41 PM

ashoat accepted this revision.Mar 9 2024, 7:23 PM

ashoat added inline comments.

keyserver/src/user/checks.js
7 ↗	(On Diff #37956)	Is this change intentional? cc @inka
keyserver/src/user/login.js
96–100 ↗	(On Diff #37956)	Do we need a warning for this? I'm not super opposed, but I probably wouldn't have added one
98 ↗	(On Diff #37956)	Can you keep all lines to 80 chars?

This revision is now accepted and ready to land.Mar 9 2024, 7:23 PM

varun added inline comments.Mar 10 2024, 2:51 PM

keyserver/src/user/checks.js
7 ↗	(On Diff #37956)	yeah it was intentional. we haven't run a migration yet that requires this field or `forceLogin` below, so they should be optional properties. left a comment here
keyserver/src/user/login.js
96–100 ↗	(On Diff #37956)	i'll just remove the warning altogether. it's not that useful to begin with and we're eventually going to have a prompt during secondary device auth that makes it unnecessary