Page MenuHomePhabricator
Differential D14501

[identity] Skip keyserver during primary logout and restore
ClosedPublic
Actions

Authored by bartek on Tue, Mar 25, 7:01 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 31, 7:00 PM
Unknown Object (File)
Mon, Mar 31, 3:02 PM
Unknown Object (File)
Mon, Mar 31, 11:33 AM
Unknown Object (File)
Mon, Mar 31, 7:54 AM
Unknown Object (File)
Sun, Mar 30, 10:26 PM
Unknown Object (File)
Sun, Mar 30, 11:11 AM
Unknown Object (File)
Sun, Mar 30, 7:59 AM
Unknown Object (File)
Sat, Mar 29, 9:20 PM
View All 19 Files
Subscribers
ashoat
tomek

Details

Reviewers
kamil
Commits
rCOMM0a82eb871ff8: [identity] Skip keyserver during primary logout and restore
Summary

Address ENG-10233 and data deletion part of ENG-10239.
Updated primary logout and restore to skip keyserver (if present) during devices removal.

Depends on D14490, D14500

Test Plan

Tested on staging along with the rest of the stack. For keyserver owner:

  • Did primary device logout. Verified that keyserver device wasn't removed from DDB
  • Performed a restore. Keyserver wasn't removed.

For users not owning a keyserver, all old devices data is removed, leaving only singleton device lists.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Tue, Mar 25, 7:01 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptTue, Mar 25, 7:01 AM
Harbormaster completed remote builds in B33825: Diff 47540.Tue, Mar 25, 7:35 AM
bartek published this revision for review.Tue, Mar 25, 7:45 AM
kamil accepted this revision.Tue, Mar 25, 11:56 AM
kamil added inline comments.
services/identity/src/grpc_services/authenticated.rs
584–589 ↗(On Diff #47540)

I prefer this syntax but up to you

This revision is now accepted and ready to land.Tue, Mar 25, 11:56 AM
bartek updated this revision to Diff 47560.Wed, Mar 26, 3:48 AM

Use alternative syntax

Harbormaster completed remote builds in B33842: Diff 47560.Wed, Mar 26, 4:07 AM
Closed by commit rCOMM0a82eb871ff8: [identity] Skip keyserver during primary logout and restore (authored by bartek). · Explain WhyWed, Mar 26, 7:55 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM0a82eb871ff8: [identity] Skip keyserver during primary logout and restore.
bartek mentioned this in D14506: [lib][native] Keep keyserver in device list after primary logout.Thu, Mar 27, 2:24 AM
bartek mentioned this in rCOMMfc01ada1f576: [lib][native] Keep keyserver in device list after primary logout.Thu, Mar 27, 5:23 AM

Revision Contents
Changeset List

PathSize
services/
identity/
src/
22 lines
grpc_services/
22 lines

Diff 47568

View Options

services/identity/src/client_service.rs

use std::collections::HashMap; use std::collections::{HashMap, HashSet};
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use comm_lib::{ use comm_lib::{
aws::ddb::{ aws::ddb::{
operation::{get_item::GetItemOutput, query::builders::QueryFluentBuilder}, operation::{get_item::GetItemOutput, query::builders::QueryFluentBuilder},
types::{ types::{
error::TransactionCanceledException, AttributeValue, Delete, error::TransactionCanceledException, AttributeValue, Delete,
DeleteRequest, Put, TransactWriteItem, Update, WriteRequest, DeleteRequest, Put, TransactWriteItem, Update, WriteRequest,
▲ Show 20 Lines • Show All 1,753 Lines • ▼ Show 20 Lines
/// Deletes all device data for user. Keeps device list rows. /// Deletes all device data for user. Keeps device list rows.
/// Returns list of deleted device IDs /// Returns list of deleted device IDs
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
pub async fn delete_devices_data_for_user( pub async fn delete_devices_data_for_user(
&self, &self,
user_id: impl Into<String>, user_id: impl Into<String>,
) -> Result<Vec<String>, Error> { ) -> Result<Vec<String>, Error> {
self.delete_user_devices_data_excluding(user_id, &[]).await
}
/// Same as [`DatabaseClient::delete_devices_data_for_user`]
/// but allows to exclude certain device IDs.
#[tracing::instrument(skip_all)]
pub async fn delete_user_devices_data_excluding(
&self,
user_id: impl Into<String>,
excluded_device_ids: &[&String],
) -> Result<Vec<String>, Error> {
let user_id: String = user_id.into(); let user_id: String = user_id.into();
let excluded_device_ids: HashSet<&String> =
excluded_device_ids.iter().cloned().collect();
// we project only the primary keys so we can pass these directly to delete requests // we project only the primary keys so we can pass these directly to delete requests
let primary_keys = let primary_keys =
query_rows_with_prefix(self, &user_id, DEVICE_ITEM_KEY_PREFIX) query_rows_with_prefix(self, &user_id, DEVICE_ITEM_KEY_PREFIX)
.projection_expression(format!("{ATTR_USER_ID}, {ATTR_ITEM_ID}")) .projection_expression(format!("{ATTR_USER_ID}, {ATTR_ITEM_ID}"))
.send() .send()
.await .await
.map_err(|e| { .map_err(|e| {
error!( error!(
errorType = error_types::DEVICE_LIST_DB_LOG, errorType = error_types::DEVICE_LIST_DB_LOG,
"Failed to list user's devices' primary keys: {:?}", e "Failed to list user's devices' primary keys: {:?}", e
); );
Error::AwsSdk(e.into()) Error::AwsSdk(e.into())
})? })?
.items .items
.unwrap_or_default(); .unwrap_or_default();
let device_ids = primary_keys let filtered_keys_iter = primary_keys.into_iter().filter_map(|pk_attrs| {
.iter() let item_id = pk_attrs.get(devices_table::ATTR_ITEM_ID).cloned();
.map(|attrs| { let device_id_result =
let attr = attrs.get(devices_table::ATTR_ITEM_ID).cloned(); DeviceIDAttribute::try_from(item_id).map(DeviceIDAttribute::into_inner);
DeviceIDAttribute::try_from(attr).map(DeviceIDAttribute::into_inner)
}) match device_id_result {
.collect::<Result<_, _>>()?; Err(err) => Some(Err(err)),
Ok(device_id) if excluded_device_ids.contains(&device_id) => None,
let delete_requests = primary_keys Ok(device_id) => {
.into_iter()
.map(|item| {
let request = DeleteRequest::builder() let request = DeleteRequest::builder()
.set_key(Some(item)) .set_key(Some(pk_attrs))
.build() .build()
.expect("key not set in DeleteRequest builder"); .expect("key not set in DeleteRequest builder");
WriteRequest::builder().delete_request(request).build() let request = WriteRequest::builder().delete_request(request).build();
})
.collect::<Vec<_>>(); Some(Ok((device_id, request)))
}
}
});
let (device_ids, delete_requests) =
filtered_keys_iter.collect::<Result<(Vec<_>, Vec<_>), _>>()?;
comm_lib::database::batch_operations::batch_write( comm_lib::database::batch_operations::batch_write(
&self.client, &self.client,
devices_table::NAME, devices_table::NAME,
delete_requests, delete_requests,
Default::default(), Default::default(),
) )
.await?; .await?;
▲ Show 20 Lines • Show All 683 Lines • Show Last 20 Lines
View Options

services/identity/src/grpc_services/authenticated.rs

use std::collections::HashMap; use std::collections::{HashMap, HashSet};
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use comm_lib::{ use comm_lib::{
aws::ddb::{ aws::ddb::{
operation::{get_item::GetItemOutput, query::builders::QueryFluentBuilder}, operation::{get_item::GetItemOutput, query::builders::QueryFluentBuilder},
types::{ types::{
error::TransactionCanceledException, AttributeValue, Delete, error::TransactionCanceledException, AttributeValue, Delete,
DeleteRequest, Put, TransactWriteItem, Update, WriteRequest, DeleteRequest, Put, TransactWriteItem, Update, WriteRequest,
▲ Show 20 Lines • Show All 1,753 Lines • ▼ Show 20 Lines
/// Deletes all device data for user. Keeps device list rows. /// Deletes all device data for user. Keeps device list rows.
/// Returns list of deleted device IDs /// Returns list of deleted device IDs
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
pub async fn delete_devices_data_for_user( pub async fn delete_devices_data_for_user(
&self, &self,
user_id: impl Into<String>, user_id: impl Into<String>,
) -> Result<Vec<String>, Error> { ) -> Result<Vec<String>, Error> {
self.delete_user_devices_data_excluding(user_id, &[]).await
}
/// Same as [`DatabaseClient::delete_devices_data_for_user`]
/// but allows to exclude certain device IDs.
#[tracing::instrument(skip_all)]
pub async fn delete_user_devices_data_excluding(
&self,
user_id: impl Into<String>,
excluded_device_ids: &[&String],
) -> Result<Vec<String>, Error> {
let user_id: String = user_id.into(); let user_id: String = user_id.into();
let excluded_device_ids: HashSet<&String> =
excluded_device_ids.iter().cloned().collect();
// we project only the primary keys so we can pass these directly to delete requests // we project only the primary keys so we can pass these directly to delete requests
let primary_keys = let primary_keys =
query_rows_with_prefix(self, &user_id, DEVICE_ITEM_KEY_PREFIX) query_rows_with_prefix(self, &user_id, DEVICE_ITEM_KEY_PREFIX)
.projection_expression(format!("{ATTR_USER_ID}, {ATTR_ITEM_ID}")) .projection_expression(format!("{ATTR_USER_ID}, {ATTR_ITEM_ID}"))
.send() .send()
.await .await
.map_err(|e| { .map_err(|e| {
error!( error!(
errorType = error_types::DEVICE_LIST_DB_LOG, errorType = error_types::DEVICE_LIST_DB_LOG,
"Failed to list user's devices' primary keys: {:?}", e "Failed to list user's devices' primary keys: {:?}", e
); );
Error::AwsSdk(e.into()) Error::AwsSdk(e.into())
})? })?
.items .items
.unwrap_or_default(); .unwrap_or_default();
let device_ids = primary_keys let filtered_keys_iter = primary_keys.into_iter().filter_map(|pk_attrs| {
.iter() let item_id = pk_attrs.get(devices_table::ATTR_ITEM_ID).cloned();
.map(|attrs| { let device_id_result =
let attr = attrs.get(devices_table::ATTR_ITEM_ID).cloned(); DeviceIDAttribute::try_from(item_id).map(DeviceIDAttribute::into_inner);
DeviceIDAttribute::try_from(attr).map(DeviceIDAttribute::into_inner)
}) match device_id_result {
.collect::<Result<_, _>>()?; Err(err) => Some(Err(err)),
Ok(device_id) if excluded_device_ids.contains(&device_id) => None,
let delete_requests = primary_keys Ok(device_id) => {
.into_iter()
.map(|item| {
let request = DeleteRequest::builder() let request = DeleteRequest::builder()
.set_key(Some(item)) .set_key(Some(pk_attrs))
.build() .build()
.expect("key not set in DeleteRequest builder"); .expect("key not set in DeleteRequest builder");
WriteRequest::builder().delete_request(request).build() let request = WriteRequest::builder().delete_request(request).build();
})
.collect::<Vec<_>>(); Some(Ok((device_id, request)))
}
}
});
let (device_ids, delete_requests) =
filtered_keys_iter.collect::<Result<(Vec<_>, Vec<_>), _>>()?;
comm_lib::database::batch_operations::batch_write( comm_lib::database::batch_operations::batch_write(
&self.client, &self.client,
devices_table::NAME, devices_table::NAME,
delete_requests, delete_requests,
Default::default(), Default::default(),
) )
.await?; .await?;
▲ Show 20 Lines • Show All 683 Lines • Show Last 20 Lines