Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F3001416
D12704.id42217.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
17 KB
Referenced Files
None
Subscribers
None
D12704.id42217.diff
View Options
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
Details
Attached
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)
Attached To
Mode
D12704: [identity] fix issues with capitalized usernames in users and reserved usernames tables
Attached
Detach File
Event Timeline
Log In to Comment