Page MenuHomePhabricator

[Identity] Implement RefreshUserPreKey
ClosedPublic

Authored by jon on Jul 10 2023, 6:30 AM.
Tags
None
Referenced Files
F3514909: D8464.id28965.diff
Sun, Dec 22, 6:55 AM
Unknown Object (File)
Mon, Dec 16, 10:27 AM
Unknown Object (File)
Mon, Dec 16, 10:27 AM
Unknown Object (File)
Mon, Dec 16, 10:27 AM
Unknown Object (File)
Mon, Dec 16, 10:27 AM
Unknown Object (File)
Mon, Dec 16, 10:27 AM
Unknown Object (File)
Sat, Dec 14, 9:01 PM
Unknown Object (File)
Thu, Nov 28, 1:29 PM
Subscribers

Details

Summary

Implement authenticated RefreshUserPreKey.

https://linear.app/comm/issue/ENG-3942

Depends on D8463

Test Plan

Start docker, localstack, and run terraform

cd serivces/commtest
cargo test --test identity_prekey_tests

# assert prekey was changed
awslocal dynamodb scan --table-name identity-users

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek requested changes to this revision.Jul 11 2023, 10:34 AM

One suggestion, feel free to re-request if I'm wrong or you don't think it's necessary.

The DynamoDB UpdateItem creates the item if it doesn't already exist (source). So if we call db_client.set_prekey() for non-existing user_id, in theory it will create a new db record.

I know that this is called from the RefreshUserPreKeys endpoint which is authenticated so user ID exisrs, but it's good to make the database function bulletproof too - e.g. if it was ever called in any other context.

services/identity/src/database.rs
245–259

I'd add an expression to make sure this only updates existing records and fails if none exist

This revision now requires changes to proceed.Jul 11 2023, 10:34 AM
services/identity/src/database.rs
245–259

Actually I meant attribute_exists() 🤦‍♂️ Suggested the exact opposite

jon marked 2 inline comments as done.

Address feedback

services/identity/src/database.rs
245–259

Makes a lot of sense, thanks!

This revision is now accepted and ready to land.Jul 19 2023, 4:34 AM
This revision was automatically updated to reflect the committed changes.