From a5c993526a3021a31f1bc1b727acf07b1eb49b71 Mon Sep 17 00:00:00 2001 From: henry0715-dev Date: Thu, 29 Aug 2024 14:35:25 +0900 Subject: [PATCH] Add signInWithNewPassword GraphQL API Close #14 Changed `sign-in` GraphQL API logic. - Returns `Err` if `last_signin_time` of `account` is `None`. --- CHANGELOG.md | 1 + src/graphql/account.rs | 361 ++++++++++++++++++++++++++++++++++------- 2 files changed, 307 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b22a592b..2d573dc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Added a `language` field to the `Account`. Consequently, the `account` and `accountList` API responses now include this field. The `insertAccount` and `updateAccount` GraphQL API endpoints are also updated to support the field. +- Added `signInWithNewPassword` GraphQL API for signing in with a new password. ### Fixed diff --git a/src/graphql/account.rs b/src/graphql/account.rs index c60aad70..874100f8 100644 --- a/src/graphql/account.rs +++ b/src/graphql/account.rs @@ -12,7 +12,7 @@ use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; use review_database::{ self as database, types::{self}, - Direction, Iterable, Store, + Direction, Iterable, Store, Table, }; use serde::Serialize; use tracing::info; @@ -287,7 +287,25 @@ impl AccountMutation { Ok(username) } - /// Authenticates with the given username and password + /// Authenticate with the given username and password. + /// If the `last_signin_time` value of the `account` is `None`, the operation will fail, and + /// it should be guided to call `signInWithNewPassword` GraphQL API. + /// + /// # Guide + /// If the `signIn` GraphQL API response is the error message "a password change is + /// required to proceed", you can call the `signInWithNewPassword` GraphQL API. + /// + /// ```ignore + /// r#"mutation { + /// signInWithNewPassword(username: "user", password: "pw", newPassword: "pw2") { + /// token + /// } + /// }"# + /// ``` + /// + /// # Errors + /// + /// an error message (`String`) if any of the checks fail or an error occurs during the process. async fn sign_in( &self, ctx: &Context<'_>, @@ -299,62 +317,47 @@ impl AccountMutation { let client_ip = get_client_ip(ctx); if let Some(mut account) = account_map.get(&username)? { - if let Some(allow_access_from) = account.allow_access_from.as_ref() { - if let Some(socket) = client_ip { - let ip = socket.ip(); - if !allow_access_from.contains(&ip) { - info!("access denied for {username} from IP: {ip}"); - return Err("access denied from this IP".into()); - } - } else { - info!("unable to retrieve client IP for {username}"); - return Err("unable to retrieve client IP".into()); - } - } + validate_password(&account, &username, &password)?; + validate_last_signin_time(&account, &username)?; + validate_allow_access_from(&account, &client_ip, &username)?; + validate_max_parallel_sessions(&account, &store, &username)?; - if account.verify_password(&password) { - if let Some(max_parallel_sessions) = account.max_parallel_sessions { - let access_token_map = store.access_token_map(); - let count = access_token_map - .iter(Direction::Forward, Some(username.as_bytes())) - .filter_map(|res| { - if let Ok(access_token) = res { - if access_token.username == username { - Some(access_token) - } else { - None - } - } else { - None - } - }) - .count(); - if count >= max_parallel_sessions as usize { - info!("maximum parallel sessions exceeded for {username}"); - return Err("maximum parallel sessions exceeded".into()); - } - } + sign_in_actions(&mut account, &store, &account_map, &client_ip, &username) + } else { + info!("{username} is not a valid username"); + Err("incorrect username or password".into()) + } + } - let (token, expiration_time) = - create_token(account.username.clone(), account.role.to_string())?; - account.update_last_signin_time(); - account_map.put(&account)?; + /// Authenticate with the given username and password, and update the new password. + /// + /// # Errors + /// + /// an error message (`String`) if any of the checks fail or an error occurs during the process. + async fn sign_in_with_new_password( + &self, + ctx: &Context<'_>, + username: String, + password: String, + new_password: String, + ) -> Result { + if password.is_empty() || new_password.is_empty() || username.is_empty() { + return Err("Username and password cannot be empty.".into()); + } - insert_token(&store, &token, &username)?; + let store = crate::graphql::get_store(ctx).await?; + let account_map = store.account_map(); + let client_ip = get_client_ip(ctx); - if let Some(socket) = client_ip { - info!("{username} signed in from IP: {}", socket.ip()); - } else { - info!("{username} signed in"); - } - Ok(AuthPayload { - token, - expiration_time, - }) - } else { - info!("wrong password for {username}"); - Err("incorrect username or password".into()) - } + if let Some(mut account) = account_map.get(&username)? { + validate_password(&account, &username, &password)?; + validate_allow_access_from(&account, &client_ip, &username)?; + validate_max_parallel_sessions(&account, &store, &username)?; + validate_update_new_password(&password, &new_password, &username)?; + + account.update_password(&new_password)?; + + sign_in_actions(&mut account, &store, &account_map, &client_ip, &username) } else { info!("{username} is not a valid username"); Err("incorrect username or password".into()) @@ -420,6 +423,104 @@ impl AccountMutation { } } +fn validate_password(account: &types::Account, username: &str, password: &str) -> Result<()> { + if !account.verify_password(password) { + info!("wrong password for {username}"); + return Err("incorrect username or password".into()); + } + Ok(()) +} + +fn validate_last_signin_time(account: &types::Account, username: &str) -> Result<()> { + if account.last_signin_time().is_none() { + info!("a password change is required to proceed for {username}"); + return Err("a password change is required to proceed".into()); + } + Ok(()) +} + +fn validate_allow_access_from( + account: &types::Account, + client_ip: &Option, + username: &str, +) -> Result<()> { + if let Some(allow_access_from) = account.allow_access_from.as_ref() { + if let Some(socket) = client_ip { + let ip = socket.ip(); + if !allow_access_from.contains(&ip) { + info!("access denied for {username} from IP: {ip}"); + return Err("access denied from this IP".into()); + } + } else { + info!("unable to retrieve client IP for {username}"); + return Err("unable to retrieve client IP".into()); + } + } + Ok(()) +} + +fn validate_max_parallel_sessions( + account: &types::Account, + store: &Store, + username: &str, +) -> Result<()> { + if let Some(max_parallel_sessions) = account.max_parallel_sessions { + let access_token_map = store.access_token_map(); + let count = access_token_map + .iter(Direction::Forward, Some(username.as_bytes())) + .filter_map(|res| { + if let Ok(access_token) = res { + if access_token.username == username { + Some(access_token) + } else { + None + } + } else { + None + } + }) + .count(); + if count >= max_parallel_sessions as usize { + info!("maximum parallel sessions exceeded for {username}"); + return Err("maximum parallel sessions exceeded".into()); + } + } + Ok(()) +} + +fn validate_update_new_password(password: &str, new_password: &str, username: &str) -> Result<()> { + if password.eq(new_password) { + info!("password is the same as the previous one for {username}"); + return Err("password is the same as the previous one".into()); + } + Ok(()) +} + +fn sign_in_actions( + account: &mut types::Account, + store: &Store, + account_map: &Table, + client_ip: &Option, + username: &str, +) -> Result { + let (token, expiration_time) = + create_token(account.username.clone(), account.role.to_string())?; + account.update_last_signin_time(); + account_map.put(account)?; + + insert_token(store, &token, username)?; + + if let Some(socket) = client_ip { + info!("{username} signed in from IP: {}", socket.ip()); + } else { + info!("{username} signed in"); + } + Ok(AuthPayload { + token, + expiration_time, + }) +} + /// Returns the expiration time according to the account policy. /// /// # Errors @@ -631,7 +732,7 @@ pub fn reset_admin_password(store: &Store) -> anyhow::Result<()> { fn initial_credential() -> anyhow::Result { let (username, password) = read_review_admin()?; - let initial_account = review_database::types::Account::new( + let mut initial_account = types::Account::new( &username, &password, database::Role::SystemAdministrator, @@ -641,6 +742,7 @@ fn initial_credential() -> anyhow::Result { None, None, )?; + initial_account.update_last_signin_time(); Ok(initial_account) } @@ -683,6 +785,14 @@ mod tests { BoxedAgentManager, MockAgentManager, RoleGuard, TestSchema, }; + async fn update_account_last_signin_time(schema: &TestSchema, name: &str) { + let store = schema.store().await; + let map = store.account_map(); + let mut account = map.get(name).unwrap().unwrap(); + account.update_last_signin_time(); + let _ = map.put(&account).is_ok(); + } + #[tokio::test] #[serial] async fn pagination() { @@ -1312,6 +1422,8 @@ mod tests { .await; assert_eq!(res.data.to_string(), r#"{insertAccount: "u1"}"#); + update_account_last_signin_time(&schema, "u1").await; + let res = schema .execute( r#"mutation { @@ -1387,6 +1499,8 @@ mod tests { .await; assert_eq!(res.data.to_string(), r#"{insertAccount: "u1"}"#); + update_account_last_signin_time(&schema, "u1").await; + let res = schema .execute( r#"mutation { @@ -1422,6 +1536,8 @@ mod tests { .await; assert_eq!(res.data.to_string(), r#"{insertAccount: "u1"}"#); + update_account_last_signin_time(&schema, "u1").await; + let res = schema .execute( r#"mutation { @@ -1434,4 +1550,139 @@ mod tests { assert!(res.is_err()); } + + #[tokio::test] + async fn password_required_proceed() { + let schema = TestSchema::new().await; + let res = schema + .execute( + r#"mutation { + insertAccount( + username: "u2", + password: "pw2", + role: "SECURITY_ADMINISTRATOR", + name: "User One", + department: "Test", + maxParallelSessions: 2 + ) + }"#, + ) + .await; + assert_eq!(res.data.to_string(), r#"{insertAccount: "u2"}"#); + + let res = schema + .execute( + r#"mutation { + signIn(username: "u2", password: "pw2") { + token + } + }"#, + ) + .await; + + assert_eq!( + res.errors.first().unwrap().message.to_string(), + "a password change is required to proceed".to_string() + ); + } + + #[tokio::test] + async fn sign_in_with_new_password_proceed() { + let schema = TestSchema::new().await; + let res = schema + .execute( + r#"mutation { + insertAccount( + username: "u3", + password: "pw3", + role: "SECURITY_ADMINISTRATOR", + name: "User One", + department: "Test", + maxParallelSessions: 2 + ) + }"#, + ) + .await; + assert_eq!(res.data.to_string(), r#"{insertAccount: "u3"}"#); + + let res = schema + .execute( + r#"mutation { + signIn(username: "u3", password: "pw3") { + token + } + }"#, + ) + .await; + + assert_eq!( + res.errors.first().unwrap().message.to_string(), + "a password change is required to proceed".to_string() + ); + + let res = schema + .execute( + r#"mutation { + signInWithNewPassword(username: "u3", password: "pw3", newPassword: "pw3") { + token + } + }"#, + ) + .await; + assert_eq!( + res.errors.first().unwrap().message.to_string(), + "password is the same as the previous one".to_string() + ); + + let res = schema + .execute( + r#"mutation { + signInWithNewPassword(username: "u3", password: "pw3", newPassword: "pw4") { + token + } + }"#, + ) + .await; + assert!(res.is_ok()); + + let store = schema.store().await; + let map = store.account_map(); + let account = map.get("u3").unwrap().unwrap(); + assert!(account.verify_password("pw4")); + } + + #[tokio::test] + async fn password_validate_proceed() { + let schema = TestSchema::new().await; + let res = schema + .execute( + r#"mutation { + insertAccount( + username: "u2", + password: "pw2", + role: "SECURITY_ADMINISTRATOR", + name: "User One", + department: "Test", + maxParallelSessions: 2 + ) + }"#, + ) + .await; + assert_eq!(res.data.to_string(), r#"{insertAccount: "u2"}"#); + + let res = schema + .execute( + r#"mutation { + signIn(username: "u2", password: "pw3") { + token + } + }"#, + ) + .await; + + assert_eq!( + res.errors.first().unwrap().message.to_string(), + "incorrect username or password".to_string() + ); + } }