Page MenuHomePhabricator
Differential D7538 Diff 25463 services/identity/src/database.rs

Changeset View

Standalone View

View Options

services/identity/src/database.rs

Show First 20 Lines • Show All 99 Lines • ▼ Show 20 Lines let client = match &CONFIG.localstack_endpoint {
None => Client::new(aws_config), None => Client::new(aws_config),
}; };
DatabaseClient { DatabaseClient {
client: Arc::new(client), client: Arc::new(client),
} }
} }
pub async fn get_pake_registration(
&self,
user_id: String,
) -> Result<Option<ServerRegistration<Cipher>>, Error> {
match self.get_item_from_users_table(&user_id).await {
Ok(GetItemOutput {
item: Some(mut item),
..
}) => parse_registration_data_attribute(
item.remove(USERS_TABLE_REGISTRATION_ATTRIBUTE),
)
.map(Some)
.map_err(Error::Attribute),
Ok(_) => {
info!(
"No item found for user {} in PAKE registration table",
user_id
);
Ok(None)
}
Err(e) => {
error!(
"DynamoDB client failed to get registration data for user {}: {}",
user_id, e
);
Err(e)
}
}
}
pub async fn add_user_to_users_table( pub async fn add_user_to_users_table(
&self, &self,
registration_state: RegistrationState, registration_state: RegistrationState,
password_file: Vec<u8>, password_file: Vec<u8>,
) -> Result<String, Error> { ) -> Result<String, Error> {
let user_id = generate_uuid(); let user_id = generate_uuid();
let device_info = HashMap::from([ let device_info = HashMap::from([
( (
▲ Show 20 Lines • Show All 246 Lines • ▼ Show 20 Linesimpl DatabaseClient {
pub async fn username_taken(&self, username: String) -> Result<bool, Error> { pub async fn username_taken(&self, username: String) -> Result<bool, Error> {
let result = self let result = self
.get_user_id_from_user_info(username, AuthType::Password) .get_user_id_from_user_info(username, AuthType::Password)
.await?; .await?;
Ok(result.is_some()) Ok(result.is_some())
} }
pub async fn get_user_id_from_user_info( async fn get_user_from_user_info(
&self, &self,
user_info: String, user_info: String,
auth_type: AuthType, auth_type: AuthType,
) -> Result<Option<String>, Error> { ) -> Result<Option<HashMap<String, AttributeValue>>, Error> {
let (index, attribute_name) = match auth_type { let (index, attribute_name) = match auth_type {
AuthType::Password => { AuthType::Password => {
(USERS_TABLE_USERNAME_INDEX, USERS_TABLE_USERNAME_ATTRIBUTE) (USERS_TABLE_USERNAME_INDEX, USERS_TABLE_USERNAME_ATTRIBUTE)
} }
AuthType::Wallet => ( AuthType::Wallet => (
USERS_TABLE_WALLET_ADDRESS_INDEX, USERS_TABLE_WALLET_ADDRESS_INDEX,
USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE, USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE,
), ),
}; };
match self match self
.client .client
.query() .query()
.table_name(USERS_TABLE) .table_name(USERS_TABLE)
.index_name(index) .index_name(index)
.key_condition_expression(format!("{} = :u", attribute_name)) .key_condition_expression(format!("{} = :u", attribute_name))
.expression_attribute_values(":u", AttributeValue::S(user_info.clone())) .expression_attribute_values(":u", AttributeValue::S(user_info.clone()))
.send() .send()
.await .await
{ {
Ok(QueryOutput { Ok(QueryOutput {
items: Some(mut items), items: Some(items), ..
..
}) => { }) => {
let num_items = items.len(); let num_items = items.len();
if num_items == 0 { if num_items == 0 {
return Ok(None); return Ok(None);
} }
if num_items > 1 { if num_items > 1 {
warn!( warn!(
"{} user IDs associated with {} {}: {:?}", "{} user IDs associated with {} {}: {:?}",
num_items, attribute_name, user_info, items num_items, attribute_name, user_info, items
); );
} }
parse_string_attribute( let first_item = items[0].clone();
USERS_TABLE_PARTITION_KEY, Ok(Some(first_item))
items[0].remove(USERS_TABLE_PARTITION_KEY),
)
.map(Some)
.map_err(Error::Attribute)
} }
Ok(_) => { Ok(_) => {
info!( info!(
"No item found for {} {} in users table", "No item found for {} {} in users table",
attribute_name, user_info attribute_name, user_info
); );
Ok(None) Ok(None)
} }
Err(e) => { Err(e) => {
error!( error!(
"DynamoDB client failed to get user ID from {} {}: {}", "DynamoDB client failed to get user from {} {}: {}",
attribute_name, user_info, e attribute_name, user_info, e
); );
Err(Error::AwsSdk(e.into())) Err(Error::AwsSdk(e.into()))
} }
} }
} }
pub async fn get_user_id_from_user_info(
&self,
user_info: String,
auth_type: AuthType,
) -> Result<Option<String>, Error> {
match self
.get_user_from_user_info(user_info.clone(), auth_type)
.await
{
Ok(Some(mut user)) => parse_string_attribute(
USERS_TABLE_PARTITION_KEY,
user.remove(USERS_TABLE_PARTITION_KEY),
)
.map(Some)
.map_err(Error::Attribute),
Ok(_) => Ok(None),
Err(e) => Err(e),
}
}
jonUnsubmitted
Not Done
Inline Actions
auth_type: AuthType,
- ) -> Result<Option<String>, Error> {
+ ) -> Result<String, Error> {
match self
.get_user_from_user_info(user_info.clone(), auth_type)
.await
{
Ok(Some(mut user)) => parse_string_attribute(
USERS_TABLE_PARTITION_KEY,
user.remove(USERS_TABLE_PARTITION_KEY),
)
- .map(Some)
.map_err(Error::Attribute),
- Ok(_) => Ok(None),
+ Ok(_) => Ok(Error::NotFound),
Err(e) => Err(e),
}
}
pub async fn get_password_file_from_username(

All users should have a user id, can we just make this Result<String, Error>?

Error::NotFound relating to whatever would fit best

jon: All users should have a user id, can we just make this `Result<String, Error>`? `Error…
varunAuthorUnsubmitted
Done
Inline Actions

I don't think the db client should decide whether not getting back a user id is an error or not. that's up to the caller

varun: I don't think the db client should decide whether not getting back a user id is an error or not.
jonUnsubmitted
Not Done
Inline Actions

I'm saying that a user entry will always have a user_id. If it's not there, then we can assume that it's an error.

jon: I'm saying that a user entry will always have a user_id. If it's not there, then we can assume…
varunAuthorUnsubmitted
Done
Inline Actions

ok, imagine we're doing some cleanup workflow and we want to make sure a user with username foo was deleted by calling this function. in this scenario, Ok(None) is the most appropriate return value, hence the Option within the Result.

varun: ok, imagine we're doing some cleanup workflow and we want to make sure a user with username…
varunAuthorUnsubmitted
Done
Inline Actions

maybe i'm not fully understanding your point... we can discuss over Comm if that's the case

varun: maybe i'm not fully understanding your point... we can discuss over Comm if that's the case
bartekUnsubmitted
Not Done
Inline Actions

Maybe you already discussed this elsewhere, just wanted to put my point here

My reasoning of such cases is:

  • If there's a legit possibility of having table entry without user_id (e.g. the user was deleted or sth), returning None is OK - the caller will decide what to do
  • If all entries always must have the user_id, lack of this field means database inconsistency/corruption, then an error should be thrown.
bartek: Maybe you already discussed this elsewhere, just wanted to put my point here My reasoning of…
jonUnsubmitted
Not Done
Inline Actions

We could just use the username_available method in that case, and we don't have to deal with ternary logic.

Seems like are trying to find a problem for a potential solution.

jon: We could just use the `username_available` method in that case, and we don't have to deal with…
pub async fn get_password_file_from_username(
&self,
username: &str,
) -> Result<Option<Vec<u8>>, Error> {
jonUnsubmitted
Not Done
Inline Actions

Similar to above, collapse Result<Option<T>, E> to Result<T,E>

jon: Similar to above, collapse `Result<Option<T>, E>` to `Result<T,E>`
varunAuthorUnsubmitted
Done
Inline Actions

same reasoning as above

varun: same reasoning as above
match self
.get_user_from_user_info(username.to_string(), AuthType::Password)
.await
{
Ok(Some(mut user)) => parse_registration_data_attribute(
user.remove(USERS_TABLE_REGISTRATION_ATTRIBUTE),
)
.map(Some)
.map_err(Error::Attribute),
Ok(_) => {
info!(
"No item found for user {} in PAKE registration table",
username
);
Ok(None)
}
Err(e) => {
error!(
"DynamoDB client failed to get registration data for user {}: {}",
username, e
);
Err(e)
}
}
}
pub async fn get_item_from_users_table( pub async fn get_item_from_users_table(
&self, &self,
user_id: &str, user_id: &str,
) -> Result<GetItemOutput, Error> { ) -> Result<GetItemOutput, Error> {
let primary_key = create_simple_primary_key(( let primary_key = create_simple_primary_key((
USERS_TABLE_PARTITION_KEY.to_string(), USERS_TABLE_PARTITION_KEY.to_string(),
user_id.to_string(), user_id.to_string(),
)); ));
▲ Show 20 Lines • Show All 198 Lines • ▼ Show 20 Lines None => Err(DBItemError::new(
attribute, attribute,
DBItemAttributeError::Missing, DBItemAttributeError::Missing,
)), )),
} }
} }
fn parse_registration_data_attribute( fn parse_registration_data_attribute(
attribute: Option<AttributeValue>, attribute: Option<AttributeValue>,
) -> Result<ServerRegistration<Cipher>, DBItemError> { ) -> Result<Vec<u8>, DBItemError> {
match &attribute { match attribute {
Some(AttributeValue::B(server_registration_bytes)) => { Some(AttributeValue::B(server_registration_bytes)) => {
match ServerRegistration::<Cipher>::deserialize( Ok(server_registration_bytes.into_inner())
server_registration_bytes.as_ref(),
) {
Ok(server_registration) => Ok(server_registration),
Err(e) => Err(DBItemError::new(
USERS_TABLE_REGISTRATION_ATTRIBUTE,
attribute,
DBItemAttributeError::Pake(e),
)),
}
} }
Some(_) => Err(DBItemError::new( Some(_) => Err(DBItemError::new(
USERS_TABLE_REGISTRATION_ATTRIBUTE, USERS_TABLE_REGISTRATION_ATTRIBUTE,
attribute, attribute,
DBItemAttributeError::IncorrectType, DBItemAttributeError::IncorrectType,
)), )),
None => Err(DBItemError::new( None => Err(DBItemError::new(
USERS_TABLE_REGISTRATION_ATTRIBUTE, USERS_TABLE_REGISTRATION_ATTRIBUTE,
▲ Show 20 Lines • Show All 97 Lines • Show Last 20 Lines