Page MenuHomePhabricator
Differential D8676

[Identity] Refactor existing onetime key logic to use new tables
AbandonedPublicDraft
Actions

Authored by • jon on Jul 31 2023, 7:05 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 25, 4:46 PM
Unknown Object (File)
Sun, Mar 23, 8:32 AM
Unknown Object (File)
Sat, Mar 22, 3:41 PM
Unknown Object (File)
Mon, Mar 17, 12:27 AM
Unknown Object (File)
Sat, Mar 8, 7:08 AM
Unknown Object (File)
Sat, Mar 8, 7:07 AM
Unknown Object (File)
Sat, Mar 8, 6:59 AM
Unknown Object (File)
Thu, Mar 6, 8:20 PM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
bartek
varun
Summary

Move old writes of onetime-keys to the new tables.

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

Depends on D8642

Test Plan
  • Start identity service and localstack
  • Run ./services/terraform/dev/run.sh
cd services/commtest
cargo test --test identity_onetime_key_tests

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jon created this revision.Jul 31 2023, 7:05 AM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 31 2023, 7:05 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 31 2023, 7:35 AM
Harbormaster failed remote builds in B21370: Diff 29274!
• jon edited parent revisions, added: D8675: [Identity] Add DDB table for one-time-keys; removed: D8642: Allow for devices to register as a keyserver.Jul 31 2023, 7:50 AM
• jon added inline comments.Jul 31 2023, 8:07 AM
services/identity/src/client_service.rs
794

This is no longer needed since user_id was previously needed to find the user in identity-users table, but the onetime keys tables only care about device id.

services/identity/src/constants.rs
91–99

these should be in the previous diff

services/identity/src/database.rs
28

chose to combine one-time to onetime because this was the existing standard

298–299

for consistency

328–336

this is a bit awkward, but create_device_info takes the entire object.

344–374

This is a bit awkward, where we can't write the one time keys in the same call. However, refactoring the user write to be included in the batch_write will require us to remove all mentions of the expression-related logic (E.g. set_expression_attribute_names and set_expression_attribute_values).

But, overall, the code is reading as "If I successfully create this user, please add these onetime keys as well"; which I think is fine.

359

This clone probably isn't required

services/identity/src/ddb_utils.rs
1–10

decided to create a new file, database.rs is already massive.

19–21

This is a bit repetitive, but WriteRequest also supports deletes, so seems to be the canonical way to describe "Create a WriteRequest which contains a put action".

• jon abandoned this revision.Aug 1 2023, 10:47 AM

Squashed this into D8675 to avoid some conflicts with refactoring

• jon removed a parent revision: D8675: [Identity] Add DDB table for one-time-keys.Aug 1 2023, 10:48 AM

Revision Contents
Changeset List

PathSize
services/
commtest/
tests/
35 lines
identity/
src/
1 line
4 lines
100 lines
38 lines
1 line

Diff 29274

View Options

services/commtest/tests/identity_onetime_key_tests.rs

Show All 37 Lines
) )
if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker") if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker")
# Inside the docker build contex # Inside the docker build contex
set(_proto_path "grpc") set(_proto_path "grpc")
set(_shared_path "../shared") set(_shared_path "../shared")
else() else()
# Inside repo # Inside repo
set(_proto_path "../../native/cpp/CommonCpp/grpc")
set(_shared_path "../../shared") set(_shared_path "../../shared")
set(_proto_path "../../shared/protos")
endif() endif()
# Shared Comm protos # Shared Comm protos
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path} add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path}
${CMAKE_CURRENT_BINARY_DIR}/protos ${CMAKE_CURRENT_BINARY_DIR}/protos
EXCLUDE_FROM_ALL EXCLUDE_FROM_ALL
) )
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines
View Options

services/identity/src/client_service.rs

Show All 37 Lines
) )
if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker") if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker")
# Inside the docker build contex # Inside the docker build contex
set(_proto_path "grpc") set(_proto_path "grpc")
set(_shared_path "../shared") set(_shared_path "../shared")
else() else()
# Inside repo # Inside repo
set(_proto_path "../../native/cpp/CommonCpp/grpc")
set(_shared_path "../../shared") set(_shared_path "../../shared")
set(_proto_path "../../shared/protos")
endif() endif()
# Shared Comm protos # Shared Comm protos
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path} add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path}
${CMAKE_CURRENT_BINARY_DIR}/protos ${CMAKE_CURRENT_BINARY_DIR}/protos
EXCLUDE_FROM_ALL EXCLUDE_FROM_ALL
) )
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines
View Options

services/identity/src/constants.rs

Show All 37 Lines
) )
if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker") if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker")
# Inside the docker build contex # Inside the docker build contex
set(_proto_path "grpc") set(_proto_path "grpc")
set(_shared_path "../shared") set(_shared_path "../shared")
else() else()
# Inside repo # Inside repo
set(_proto_path "../../native/cpp/CommonCpp/grpc")
set(_shared_path "../../shared") set(_shared_path "../../shared")
set(_proto_path "../../shared/protos")
endif() endif()
# Shared Comm protos # Shared Comm protos
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path} add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path}
${CMAKE_CURRENT_BINARY_DIR}/protos ${CMAKE_CURRENT_BINARY_DIR}/protos
EXCLUDE_FROM_ALL EXCLUDE_FROM_ALL
) )
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines
View Options

services/identity/src/database.rs

Show All 37 Lines
) )
if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker") if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker")
# Inside the docker build contex # Inside the docker build contex
set(_proto_path "grpc") set(_proto_path "grpc")
set(_shared_path "../shared") set(_shared_path "../shared")
else() else()
# Inside repo # Inside repo
set(_proto_path "../../native/cpp/CommonCpp/grpc")
set(_shared_path "../../shared") set(_shared_path "../../shared")
set(_proto_path "../../shared/protos")
endif() endif()
# Shared Comm protos # Shared Comm protos
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path} add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path}
${CMAKE_CURRENT_BINARY_DIR}/protos ${CMAKE_CURRENT_BINARY_DIR}/protos
EXCLUDE_FROM_ALL EXCLUDE_FROM_ALL
) )
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines
View Options

services/identity/src/ddb_utils.rs

Show All 37 Lines
) )
if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker") if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker")
# Inside the docker build contex # Inside the docker build contex
set(_proto_path "grpc") set(_proto_path "grpc")
set(_shared_path "../shared") set(_shared_path "../shared")
else() else()
# Inside repo # Inside repo
set(_proto_path "../../native/cpp/CommonCpp/grpc")
set(_shared_path "../../shared") set(_shared_path "../../shared")
set(_proto_path "../../shared/protos")
endif() endif()
# Shared Comm protos # Shared Comm protos
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path} add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path}
${CMAKE_CURRENT_BINARY_DIR}/protos ${CMAKE_CURRENT_BINARY_DIR}/protos
EXCLUDE_FROM_ALL EXCLUDE_FROM_ALL
) )
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines
View Options

services/identity/src/main.rs

Show All 37 Lines
) )
if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker") if(${CMAKE_CURRENT_SOURCE_DIR} STREQUAL "/transferred/tunnelbroker")
# Inside the docker build contex # Inside the docker build contex
set(_proto_path "grpc") set(_proto_path "grpc")
set(_shared_path "../shared") set(_shared_path "../shared")
else() else()
# Inside repo # Inside repo
set(_proto_path "../../native/cpp/CommonCpp/grpc")
set(_shared_path "../../shared") set(_shared_path "../../shared")
set(_proto_path "../../shared/protos")
endif() endif()
# Shared Comm protos # Shared Comm protos
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path} add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${_proto_path}
${CMAKE_CURRENT_BINARY_DIR}/protos ${CMAKE_CURRENT_BINARY_DIR}/protos
EXCLUDE_FROM_ALL EXCLUDE_FROM_ALL
) )
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines