Page MenuHomePhabricator
Differential D11267

[keyserver] Print warnings if usingIdentityCredentials not set
ClosedPublic
Actions

Authored by inka on Mar 7 2024, 3:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Mon, Dec 16, 2:10 PM
Unknown Object (File)
Sun, Nov 24, 9:53 PM
View All Files
Subscribers
ashoat

Details

Reviewers
tomek
michal
Commits
rCOMMd90a864ea669: [keyserver] Print warnings if usingIdentityCredentials not set
Summary

issue: ENG-6618
We want to warn the dev that they are not using identity credentials for communictaing with the clients

For now using identity credentials is optional. The dev can proceed with their setup. If they want to start using the identity credentials though, they need to restart nix and go through setting up a new keyserver.
If the keyserver is run in Docker, the dev should set usingIdentityCredentials flag in COMM_JSONCONFIG_secrets_user_credentials env variable and create a new keyserver. See doc

We don't need this warning on the prod keyserver - it will not have the flag set, but that's fine, because the dafualt 256 is its id.

Test Plan

Ran the keyserver and checked that if usingIdentityCredentials is set then the warning is not printed, and if it is set, then the warning is printed

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka created this revision.Mar 7 2024, 3:21 AM
Herald added a subscriber: ashoat. · View Herald TranscriptMar 7 2024, 3:21 AM
inka added a parent revision: D11258: [scripts] Hide password.Mar 7 2024, 3:22 AM
Harbormaster completed remote builds in B27376: Diff 37917.Mar 7 2024, 3:37 AM
inka requested review of this revision.Mar 7 2024, 3:37 AM
inka added a child revision: D11268: [keyserver] Instruct to restart nix if login / register fails in the new flow.Mar 7 2024, 3:42 AM
inka updated this revision to Diff 37920.Mar 7 2024, 3:44 AM

Update text

Harbormaster completed remote builds in B27379: Diff 37920.Mar 7 2024, 4:03 AM
ashoat added inline comments.Mar 7 2024, 7:22 PM
keyserver/src/user/login.js
26–29 ↗(On Diff #37920)

Do we need really to block the rest of this function on this await? Can we do it in parallel?

39–46 ↗(On Diff #37920)

Is the behavior of console.warn well-defined to concatenate the list of strings with a single space in between each?

I wonder if it would be better to just console.warn a single, manually concatened string, to avoid any ambiguity

ashoat mentioned this in D11268: [keyserver] Instruct to restart nix if login / register fails in the new flow.Mar 7 2024, 7:23 PM
inka updated this revision to Diff 37964.Mar 11 2024, 2:48 AM

Address review

Harbormaster completed remote builds in B27416: Diff 37964.Mar 11 2024, 3:03 AM
michal accepted this revision.Mar 12 2024, 9:23 AM
michal added inline comments.
keyserver/src/user/login.js
25 ↗(On Diff #37964)

I find this function name weird. It starts with check but doesn't return bool or sth similar. Maybe just getUserCredentials?

This revision is now accepted and ready to land.Mar 12 2024, 9:23 AM
inka updated this revision to Diff 38027.Mar 12 2024, 9:42 AM

Rename to getUserCredentials

Harbormaster completed remote builds in B27478: Diff 38027.Mar 12 2024, 10:01 AM
Closed by commit rCOMMd90a864ea669: [keyserver] Print warnings if usingIdentityCredentials not set (authored by inka). · Explain WhyMar 13 2024, 3:57 AM
This revision was automatically updated to reflect the committed changes.
inka added a commit: rCOMMd90a864ea669: [keyserver] Print warnings if usingIdentityCredentials not set.

Revision Contents
Changeset List

PathSize
keyserver/
src/
user/
46 lines

Diff 38036

View Options

keyserver/src/user/login.js

Loading...