Page MenuHomePhabricator

[keyserver] update acknowledgment table in login was performed from proper source
ClosedPublic

Authored by kamil on Oct 6 2022, 4:43 AM.
Tags
None
Referenced Files
F3506409: D5312.diff
Fri, Dec 20, 4:30 PM
Unknown Object (File)
Wed, Dec 18, 10:51 PM
Unknown Object (File)
Wed, Dec 18, 10:41 PM
Unknown Object (File)
Sun, Dec 15, 8:29 PM
Unknown Object (File)
Sun, Dec 15, 8:29 PM
Unknown Object (File)
Sun, Dec 15, 8:29 PM
Unknown Object (File)
Sun, Dec 15, 8:29 PM
Unknown Object (File)
Sun, Dec 15, 8:29 PM

Details

Summary

Code that updates this table when a login is received with the correct source set.

Note about older clients which don't have code for passing source: I think we should allow them to login but not make any changes in this table, they will be able to use the app but we will have only fully truthful data about their acknowledgments.

Test Plan

Check if table was updated for logins from the native form and was not changed for any other login (eg. web, restoring outdated cookie).

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil edited the test plan for this revision. (Show Details)
kamil added a reviewer: atul.
kamil published this revision for review.Oct 6 2022, 4:51 AM
ashoat requested changes to this revision.Oct 6 2022, 6:18 AM
ashoat added inline comments.
keyserver/src/responders/user-responders.js
250 ↗(On Diff #17383)

For old clients, we should do this if input.source is falsey. You can use hasMinCodeVersion to check client version

This revision now requires changes to proceed.Oct 6 2022, 6:18 AM

check for falsey input on older client

setting version for 99999 because sending source was not released yet and not sure which version to use

This revision is now accepted and ready to land.Oct 12 2022, 8:14 AM

rebase, specify polisy type

kamil added inline comments.
keyserver/src/responders/user-responders.js
258 ↗(On Diff #18522)

looks like sending the source was released already so I need to double-check that and update the code version here

kamil requested review of this revision.Nov 22 2022, 3:31 AM
kamil added inline comments.
keyserver/src/responders/user-responders.js
258 ↗(On Diff #18522)

I discussed this with Ashoat on 1:1 and with Tomek and it'll be better to wait with this until entire feature will be ready

This revision is now accepted and ready to land.Nov 22 2022, 3:31 AM

update function name, rebase

tomek added a subscriber: tomek.
tomek added inline comments.
keyserver/src/responders/user-responders.js
260 ↗(On Diff #18721)

We can slightly improve the performance by avoiding awaiting this separately. We can create a promise here and await it e.g. as a part of Promise.all call.