Page MenuHomePhabricator

D12704.id42217.diff
No OneTemporary

D12704.id42217.diff

diff --git a/services/identity/src/client_service.rs b/services/identity/src/client_service.rs
--- a/services/identity/src/client_service.rs
+++ b/services/identity/src/client_service.rs
@@ -15,7 +15,7 @@
use crate::config::CONFIG;
use crate::constants::{error_types, tonic_status_messages};
use crate::database::{
- DBDeviceTypeInt, DatabaseClient, DeviceType, KeyPayload
+ DBDeviceTypeInt, DatabaseClient, DeviceType, KeyPayload, UserInfoAndPasswordFile
};
use crate::device_list::SignedDeviceList;
use crate::error::{DeviceListError, Error as DBError};
@@ -127,7 +127,7 @@
));
}
- if RESERVED_USERNAME_SET.contains(&message.username) {
+ if RESERVED_USERNAME_SET.contains(&message.username.to_lowercase()) {
return Err(tonic::Status::invalid_argument(
tonic_status_messages::USERNAME_RESERVED,
));
@@ -154,7 +154,7 @@
.start(
&CONFIG.server_setup,
&message.opaque_registration_request,
- message.username.as_bytes(),
+ message.username.to_lowercase().as_bytes(),
)
.map_err(protocol_error_to_grpc_status)?;
let session_id = self
@@ -180,23 +180,22 @@
let message = request.into_inner();
self.check_username_taken(&message.username).await?;
- if RESERVED_USERNAME_SET.contains(&message.username) {
+ if RESERVED_USERNAME_SET.contains(&message.username.to_lowercase()) {
return Err(tonic::Status::invalid_argument(
tonic_status_messages::USERNAME_RESERVED,
));
}
- let username_in_reserved_usernames_table = self
+ let Some(original_username) = self
.client
- .get_user_id_from_reserved_usernames_table(&message.username)
+ .get_original_username_from_reserved_usernames_table(&message.username)
.await
.map_err(handle_db_error)?
- .is_some();
- if !username_in_reserved_usernames_table {
+ else {
return Err(tonic::Status::permission_denied(
tonic_status_messages::USERNAME_NOT_RESERVED,
));
- }
+ };
let user_id = validate_account_ownership_message_and_get_user_id(
&message.username,
@@ -207,7 +206,7 @@
let registration_state = construct_user_registration_info(
&message,
Some(user_id),
- message.username.clone(),
+ original_username,
None,
)?;
self
@@ -221,7 +220,7 @@
.start(
&CONFIG.server_setup,
&message.opaque_registration_request,
- message.username.as_bytes(),
+ message.username.to_lowercase().as_bytes(),
)
.map_err(protocol_error_to_grpc_status)?;
@@ -313,36 +312,39 @@
debug!("Attempting to log in user: {:?}", &message.username);
let user_id_and_password_file = self
.client
- .get_user_id_and_password_file_from_username(&message.username)
+ .get_user_info_and_password_file_from_username(&message.username)
.await
.map_err(handle_db_error)?;
- let (user_id, password_file_bytes) =
- if let Some(data) = user_id_and_password_file {
- data
- } else {
- // It's possible that the user attempting login is already registered
- // on Ashoat's keyserver. If they are, we should send back a gRPC status
- // code instructing them to get a signed message from Ashoat's keyserver
- // in order to claim their username and register with the Identity
- // service.
- let username_in_reserved_usernames_table = self
- .client
- .get_user_id_from_reserved_usernames_table(&message.username)
- .await
- .map_err(handle_db_error)?
- .is_some();
-
- if username_in_reserved_usernames_table {
- return Err(tonic::Status::permission_denied(
- tonic_status_messages::NEED_KEYSERVER_MESSAGE_TO_CLAIM_USERNAME,
- ));
- }
+ let UserInfoAndPasswordFile {
+ user_id,
+ original_username: username,
+ password_file: password_file_bytes,
+ } = if let Some(data) = user_id_and_password_file {
+ data
+ } else {
+ // It's possible that the user attempting login is already registered
+ // on Ashoat's keyserver. If they are, we should send back a gRPC status
+ // code instructing them to get a signed message from Ashoat's keyserver
+ // in order to claim their username and register with the Identity
+ // service.
+ let username_in_reserved_usernames_table = self
+ .client
+ .get_user_id_from_reserved_usernames_table(&message.username)
+ .await
+ .map_err(handle_db_error)?
+ .is_some();
- return Err(tonic::Status::not_found(
- tonic_status_messages::USER_NOT_FOUND,
+ if username_in_reserved_usernames_table {
+ return Err(tonic::Status::permission_denied(
+ tonic_status_messages::NEED_KEYSERVER_MESSAGE_TO_CLAIM_USERNAME,
));
- };
+ }
+
+ return Err(tonic::Status::not_found(
+ tonic_status_messages::USER_NOT_FOUND,
+ ));
+ };
let flattened_device_key_upload =
construct_flattened_device_key_upload(&message)?;
@@ -360,18 +362,29 @@
.await?;
let mut server_login = comm_opaque2::server::Login::new();
- let server_response = server_login
- .start(
- &CONFIG.server_setup,
- &password_file_bytes,
- &message.opaque_login_request,
- message.username.as_bytes(),
- )
- .map_err(protocol_error_to_grpc_status)?;
+ let server_response = match server_login.start(
+ &CONFIG.server_setup,
+ &password_file_bytes,
+ &message.opaque_login_request,
+ message.username.to_lowercase().as_bytes(),
+ ) {
+ Ok(response) => response,
+ Err(_) => {
+ // Retry with original username bytes if the first attempt fails
+ server_login
+ .start(
+ &CONFIG.server_setup,
+ &password_file_bytes,
+ &message.opaque_login_request,
+ username.as_bytes(),
+ )
+ .map_err(protocol_error_to_grpc_status)?
+ }
+ };
let login_state = construct_user_login_info(
user_id,
- message.username,
+ username,
server_login,
flattened_device_key_upload,
maybe_device_to_remove,
diff --git a/services/identity/src/constants.rs b/services/identity/src/constants.rs
--- a/services/identity/src/constants.rs
+++ b/services/identity/src/constants.rs
@@ -54,9 +54,11 @@
pub const USERS_TABLE_DEVICELIST_TIMESTAMP_ATTRIBUTE_NAME: &str =
"deviceListTimestamp";
pub const USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME: &str = "farcasterID";
+pub const USERS_TABLE_USERNAME_LOWER_ATTRIBUTE_NAME: &str = "usernameLower";
pub const USERS_TABLE_USERNAME_INDEX: &str = "username-index";
pub const USERS_TABLE_WALLET_ADDRESS_INDEX: &str = "walletAddress-index";
pub const USERS_TABLE_FARCASTER_ID_INDEX: &str = "farcasterID-index";
+pub const USERS_TABLE_USERNAME_LOWER_INDEX: &str = "usernameLower-index";
pub mod token_table {
pub const NAME: &str = "identity-tokens";
@@ -85,6 +87,10 @@
pub const RESERVED_USERNAMES_TABLE: &str = "identity-reserved-usernames";
pub const RESERVED_USERNAMES_TABLE_PARTITION_KEY: &str = "username";
pub const RESERVED_USERNAMES_TABLE_USER_ID_ATTRIBUTE: &str = "userID";
+pub const RESERVED_USERNAMES_TABLE_USERNAME_LOWER_ATTRIBUTE: &str =
+ "usernameLower";
+pub const RESERVED_USERNAMES_TABLE_USERNAME_LOWER_INDEX: &str =
+ "usernameLower-index";
// Users table social proof attribute
pub const SOCIAL_PROOF_MESSAGE_ATTRIBUTE: &str = "siweMessage";
diff --git a/services/identity/src/database.rs b/services/identity/src/database.rs
--- a/services/identity/src/database.rs
+++ b/services/identity/src/database.rs
@@ -20,11 +20,8 @@
pub use crate::database::device_list::DeviceIDAttribute;
pub use crate::database::one_time_keys::OTKRow;
use crate::{
- constants::{error_types, USERS_TABLE_SOCIAL_PROOF_ATTRIBUTE_NAME},
- ddb_utils::EthereumIdentity,
- device_list::SignedDeviceList,
- grpc_services::shared::PlatformMetadata,
- reserved_users::UserDetail,
+ ddb_utils::EthereumIdentity, device_list::SignedDeviceList,
+ grpc_services::shared::PlatformMetadata, reserved_users::UserDetail,
siwe::SocialProof,
};
use crate::{
@@ -39,15 +36,18 @@
use crate::client_service::{FlattenedDeviceKeyUpload, UserRegistrationInfo};
use crate::config::CONFIG;
use crate::constants::{
- NONCE_TABLE, NONCE_TABLE_CREATED_ATTRIBUTE,
+ error_types, NONCE_TABLE, NONCE_TABLE_CREATED_ATTRIBUTE,
NONCE_TABLE_EXPIRATION_TIME_ATTRIBUTE,
NONCE_TABLE_EXPIRATION_TIME_UNIX_ATTRIBUTE, NONCE_TABLE_PARTITION_KEY,
RESERVED_USERNAMES_TABLE, RESERVED_USERNAMES_TABLE_PARTITION_KEY,
+ RESERVED_USERNAMES_TABLE_USERNAME_LOWER_ATTRIBUTE,
+ RESERVED_USERNAMES_TABLE_USERNAME_LOWER_INDEX,
RESERVED_USERNAMES_TABLE_USER_ID_ATTRIBUTE, USERS_TABLE,
USERS_TABLE_DEVICES_MAP_DEVICE_TYPE_ATTRIBUTE_NAME,
USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME, USERS_TABLE_PARTITION_KEY,
- USERS_TABLE_REGISTRATION_ATTRIBUTE, USERS_TABLE_USERNAME_ATTRIBUTE,
- USERS_TABLE_USERNAME_INDEX, USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE,
+ USERS_TABLE_REGISTRATION_ATTRIBUTE, USERS_TABLE_SOCIAL_PROOF_ATTRIBUTE_NAME,
+ USERS_TABLE_USERNAME_ATTRIBUTE, USERS_TABLE_USERNAME_LOWER_ATTRIBUTE_NAME,
+ USERS_TABLE_USERNAME_LOWER_INDEX, USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE,
USERS_TABLE_WALLET_ADDRESS_INDEX,
};
use crate::id::generate_uuid;
@@ -133,6 +133,12 @@
}
}
+pub struct UserInfoAndPasswordFile {
+ pub user_id: String,
+ pub original_username: String,
+ pub password_file: Vec<u8>,
+}
+
#[derive(Clone)]
pub struct DatabaseClient {
client: Arc<DynamoDBClient>,
@@ -290,12 +296,16 @@
{
user.insert(
USERS_TABLE_USERNAME_ATTRIBUTE.to_string(),
- AttributeValue::S(username),
+ AttributeValue::S(username.clone()),
);
user.insert(
USERS_TABLE_REGISTRATION_ATTRIBUTE.to_string(),
AttributeValue::B(password_file),
);
+ user.insert(
+ USERS_TABLE_USERNAME_LOWER_ATTRIBUTE_NAME.to_string(),
+ AttributeValue::S(username.to_lowercase()),
+ );
}
if let Some(eth_identity) = wallet_identity.clone() {
@@ -573,11 +583,38 @@
}
pub async fn username_taken(&self, username: String) -> Result<bool, Error> {
- let result = self
- .get_user_id_from_user_info(username, &AuthType::Password)
- .await?;
+ let username_lower = username.to_lowercase();
- Ok(result.is_some())
+ let request = self
+ .client
+ .query()
+ .table_name(USERS_TABLE)
+ .index_name(USERS_TABLE_USERNAME_LOWER_INDEX)
+ .key_condition_expression("#username_lower = :username_lower")
+ .expression_attribute_names(
+ "#username_lower",
+ USERS_TABLE_USERNAME_LOWER_ATTRIBUTE_NAME,
+ )
+ .expression_attribute_values(
+ ":username_lower",
+ AttributeValue::S(username_lower),
+ );
+
+ let response = request.send().await.map_err(|e| {
+ error!(
+ errorType = error_types::GENERIC_DB_LOG,
+ "Failed to query lowercase usernames by index: {:?}", e
+ );
+ Error::AwsSdk(e.into())
+ })?;
+
+ if let Some(items) = response.items() {
+ if !items.is_empty() {
+ return Ok(true);
+ }
+ }
+
+ Ok(false)
}
pub async fn filter_out_taken_usernames(
@@ -586,11 +623,16 @@
) -> Result<Vec<UserDetail>, Error> {
let db_usernames = self.get_all_usernames().await?;
- let db_usernames_set: HashSet<String> = db_usernames.into_iter().collect();
+ let db_usernames_set: HashSet<String> = db_usernames
+ .into_iter()
+ .map(|username| username.to_lowercase())
+ .collect();
let available_user_details: Vec<UserDetail> = user_details
.into_iter()
- .filter(|user_detail| !db_usernames_set.contains(&user_detail.username))
+ .filter(|user_detail| {
+ !db_usernames_set.contains(&user_detail.username.to_lowercase())
+ })
.collect();
Ok(available_user_details)
@@ -602,13 +644,16 @@
user_info: String,
auth_type: &AuthType,
) -> Result<Option<HashMap<String, AttributeValue>>, Error> {
- let (index, attribute_name) = match auth_type {
- AuthType::Password => {
- (USERS_TABLE_USERNAME_INDEX, USERS_TABLE_USERNAME_ATTRIBUTE)
- }
+ let (index, attribute_name, attribute_value) = match auth_type {
+ AuthType::Password => (
+ USERS_TABLE_USERNAME_LOWER_INDEX,
+ USERS_TABLE_USERNAME_LOWER_ATTRIBUTE_NAME,
+ user_info.to_lowercase(),
+ ),
AuthType::Wallet => (
USERS_TABLE_WALLET_ADDRESS_INDEX,
USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE,
+ user_info.clone(),
),
};
match self
@@ -617,7 +662,7 @@
.table_name(USERS_TABLE)
.index_name(index)
.key_condition_expression(format!("{} = :u", attribute_name))
- .expression_attribute_values(":u", AttributeValue::S(user_info.clone()))
+ .expression_attribute_values(":u", AttributeValue::S(attribute_value))
.send()
.await
{
@@ -729,10 +774,10 @@
}
#[tracing::instrument(skip_all)]
- pub async fn get_user_id_and_password_file_from_username(
+ pub async fn get_user_info_and_password_file_from_username(
&self,
username: &str,
- ) -> Result<Option<(String, Vec<u8>)>, Error> {
+ ) -> Result<Option<UserInfoAndPasswordFile>, Error> {
match self
.get_user_from_user_info(username.to_string(), &AuthType::Password)
.await
@@ -742,8 +787,14 @@
let password_file = parse_registration_data_attribute(
user.remove(USERS_TABLE_REGISTRATION_ATTRIBUTE),
)?;
+ let original_username =
+ user.take_attr(USERS_TABLE_USERNAME_ATTRIBUTE)?;
- Ok(Some((user_id, password_file)))
+ Ok(Some(UserInfoAndPasswordFile {
+ user_id,
+ original_username,
+ password_file,
+ }))
}
Ok(_) => {
info!(
@@ -1063,6 +1114,10 @@
RESERVED_USERNAMES_TABLE_USER_ID_ATTRIBUTE,
AttributeValue::S(user_detail.user_id.to_string()),
)
+ .item(
+ RESERVED_USERNAMES_TABLE_USERNAME_LOWER_ATTRIBUTE,
+ AttributeValue::S(user_detail.username.to_lowercase()),
+ )
.build();
WriteRequest::builder().put_request(put_request).build()
@@ -1122,30 +1177,65 @@
&self,
username: &str,
) -> Result<Option<String>, Error> {
+ self
+ .query_reserved_usernames_table(
+ username,
+ RESERVED_USERNAMES_TABLE_USER_ID_ATTRIBUTE,
+ )
+ .await
+ }
+
+ pub async fn get_original_username_from_reserved_usernames_table(
+ &self,
+ username: &str,
+ ) -> Result<Option<String>, Error> {
+ self
+ .query_reserved_usernames_table(
+ username,
+ RESERVED_USERNAMES_TABLE_PARTITION_KEY,
+ )
+ .await
+ }
+
+ async fn query_reserved_usernames_table(
+ &self,
+ username: &str,
+ attribute: &str,
+ ) -> Result<Option<String>, Error> {
+ let username_lower = username.to_lowercase();
+
let response = self
.client
- .get_item()
+ .query()
.table_name(RESERVED_USERNAMES_TABLE)
- .key(
- RESERVED_USERNAMES_TABLE_PARTITION_KEY.to_string(),
- AttributeValue::S(username.to_string()),
+ .index_name(RESERVED_USERNAMES_TABLE_USERNAME_LOWER_INDEX)
+ .key_condition_expression("#username_lower = :username_lower")
+ .expression_attribute_names(
+ "#username_lower",
+ RESERVED_USERNAMES_TABLE_USERNAME_LOWER_ATTRIBUTE,
+ )
+ .expression_attribute_values(
+ ":username_lower",
+ AttributeValue::S(username_lower),
)
- .consistent_read(true)
.send()
.await
.map_err(|e| Error::AwsSdk(e.into()))?;
- let GetItemOutput {
- item: Some(mut item),
+ let QueryOutput {
+ items: Some(mut results),
..
} = response
else {
return Ok(None);
};
- // We should not return `Ok(None)` if `userID` is missing from the item
- let user_id = item.take_attr(RESERVED_USERNAMES_TABLE_USER_ID_ATTRIBUTE)?;
- Ok(Some(user_id))
+ let result = results
+ .pop()
+ .map(|mut attrs| attrs.take_attr::<String>(attribute))
+ .transpose()?;
+
+ Ok(result)
}
}
diff --git a/services/terraform/modules/shared/dynamodb.tf b/services/terraform/modules/shared/dynamodb.tf
--- a/services/terraform/modules/shared/dynamodb.tf
+++ b/services/terraform/modules/shared/dynamodb.tf
@@ -145,6 +145,11 @@
type = "S"
}
+ attribute {
+ name = "usernameLower"
+ type = "S"
+ }
+
global_secondary_index {
name = "username-index"
hash_key = "username"
@@ -163,6 +168,12 @@
projection_type = "INCLUDE"
non_key_attributes = ["walletAddress", "username"]
}
+
+ global_secondary_index {
+ name = "usernameLower-index"
+ hash_key = "usernameLower"
+ projection_type = "KEYS_ONLY"
+ }
}
resource "aws_dynamodb_table" "identity-devices" {
@@ -270,6 +281,18 @@
name = "username"
type = "S"
}
+
+ attribute {
+ name = "usernameLower"
+ type = "S"
+ }
+
+ global_secondary_index {
+ name = "usernameLower-index"
+ hash_key = "usernameLower"
+ projection_type = "INCLUDE"
+ non_key_attributes = ["userID"]
+ }
}
resource "aws_dynamodb_table" "identity-one-time-keys" {

File Metadata

Mime Type
text/plain
Expires
Sat, Oct 19, 4:30 AM (20 h, 1 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2321300
Default Alt Text
D12704.id42217.diff (17 KB)

Event Timeline