diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 3f7f20b4..c2201eb1 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -16,8 +16,11 @@ * [Passkey Auth](adr/0005-auth-passkey.md) * [Federation IDP](adr/0006-federation-idp.md) * [Federation Mapping](adr/0007-federation-mapping.md) - * [Workload Federation](adr/0008-workload-federation.md) + * [Workload Federation](adr/0008-federation-workload.md) * [Auth token revocation](adr/0009-auth-token-revoke.md) + * [PCI-DSS: Failed Auth Protection](adr/0010-pci-dss-failed-auth-protection.md) + * [PCI-DSS: Inactive Account Deactivation](adr/0011-pci-dss-inactive-account-deactivation.md) + * [PCI-DSS: Account Password Expiration](adr/0012-pci-dss-account-password-expiry.md) * [Policy enforcement](./policy.md) * [Fernet token]() * [Token payloads]() diff --git a/doc/src/adr/0008-workload-federation.md b/doc/src/adr/0008-federation-workload.md similarity index 100% rename from doc/src/adr/0008-workload-federation.md rename to doc/src/adr/0008-federation-workload.md diff --git a/doc/src/adr/0010-pci-dss-failed-auth-protection.md b/doc/src/adr/0010-pci-dss-failed-auth-protection.md new file mode 100644 index 00000000..c7814e93 --- /dev/null +++ b/doc/src/adr/0010-pci-dss-failed-auth-protection.md @@ -0,0 +1,89 @@ +# 10. PCI-DSS requirement: Invalid authentication attempts are limited + +Date: 2025-11-27 + +## Status + +Accepted + +## Context + +PCI-DSS contains the following requirement to the IAM system: + +Invalid authentication attempts are limited by: + +- Locking out the user ID after not more than 10 attempts. +- Setting the lockout duration to a minimum of 30 minutes or until the user's + identity is confirmed. + +Python Keystone implements this requirement with the help of the +`conf.security_compliance.lockout_duration` during the login attempt to identify +whether the user is currently temporarily disabled: + +```python + + def _is_account_locked(self, user_id, user_ref): + """Check if the user account is locked. + + Checks if the user account is locked based on the number of failed + authentication attempts. + + :param user_id: The user ID + :param user_ref: Reference to the user object + :returns Boolean: True if the account is locked; False otherwise + + """ + ignore_option = user_ref.get_resource_option( + options.IGNORE_LOCKOUT_ATTEMPT_OPT.option_id + ) + if ignore_option and ignore_option.option_value is True: + return False + + attempts = user_ref.local_user.failed_auth_count or 0 + max_attempts = CONF.security_compliance.lockout_failure_attempts + lockout_duration = CONF.security_compliance.lockout_duration + if max_attempts and (attempts >= max_attempts): + if not lockout_duration: + return True + else: + delta = datetime.timedelta(seconds=lockout_duration) + last_failure = user_ref.local_user.failed_auth_at + if (last_failure + delta) > timeutils.utcnow(): + return True + else: + self._reset_failed_auth(user_id) + return False +``` + +## Decision + +For compatibility reasons rust implementation must adhere to the requirement. + +During password authentication before validating the password following check +must be applied part of the locked account verification: + +- When `conf.security_compliance.lockout_duration` and + `conf.security_compliance.lockout_failure_attempts` are not set the account is + NOT locked. + +- When `user_options.IGNORE_LOCKOUT_ATTEMPT` is set user account is NOT locked + +- When `user.failed_auth_count >= conf.security_compliance.lockout_failure_attempts` + the account is locked. + +- When `user.failed_auth_at + conf.security_compliance.lockout_duration > +now()` account is locked. When the time is `< now()` - reset the counters + in the database. + +- Otherwise the account is NOT locked. + +After the authentication is success the `user.failed_auth_at` and +`user.failed_auth_count` are being reset. In the case of failed authentication +such attempt sets the mentioned properties correspondingly. + +## Consequences + +- Authentication with methods other than username password are not protected. + +- Reactivating the temporarily locked account can be performed by the admin or + domain admin via resetting the `user.failed_auth_count` attribute. diff --git a/doc/src/adr/0011-pci-dss-inactive-account-deactivation.md b/doc/src/adr/0011-pci-dss-inactive-account-deactivation.md new file mode 100644 index 00000000..8177bc64 --- /dev/null +++ b/doc/src/adr/0011-pci-dss-inactive-account-deactivation.md @@ -0,0 +1,99 @@ +# 11. PCI-DSS requirement: Inactive user accounts are removed/disabled + +Date: 2025-11-27 + +## Status + +Accepted + +## Context + +PCI-DSS contains the following requirement to the IAM system: + +Inactive user accounts are removed or disabled within 90 days of inactivity. + +Python Keystone implements this requirement with the help of the +`conf.security_compliance.disable_user_account_days_inactive` during the login +attempt to identify whether the user is currently active or deactivated: + +```python + + def enabled(self): + """Return whether user is enabled or not.""" + if self._enabled: + max_days = ( + CONF.security_compliance.disable_user_account_days_inactive + ) + inactivity_exempt = getattr( + self.get_resource_option( + iro.IGNORE_USER_INACTIVITY_OPT.option_id + ), + 'option_value', + False, + ) + last_active = self.last_active_at + if not last_active and self.created_at: + last_active = self.created_at.date() + if max_days and last_active: + now = timeutils.utcnow().date() + days_inactive = (now - last_active).days + if days_inactive >= max_days and not inactivity_exempt: + self._enabled = False + return self._enabled + +``` + +In python Keystone there is no periodic process that deactivates inactive +accounts. Instead it is calculated on demand during the login process and +listint/showing user details. With the new application architecture in Rust it +is possible to implement background processes that disable inactive users. This +allows doing less calculations during user authentication and fetching since it +is possible to rely that the background process deactivates accounts when +necessary. + +## Decision + +For compatibility reasons rust implementation must adhere to the requirement. + +After successful authentication when `user.enabled` attribute is not true the +authentication request must be rejected with `http.Unauthorized`. + +Additional background process must be implemented to deactivate inactive +accounts. For this when +`conf.security_compliance.disable_user_account_days_inactive` is set a process +should loop over all user accounts. When the `user.last_active_at + +disable_user_account_days_inactive < now()` presence of the +`user.options.IGNORE_USER_INACTIVITY_OPT` should be checked. When absent the +account must be updated setting `user.enabled` to `false`. + +Since it is technically possible that the background process is not running for +any reason the same logic should be applied also when converting the identity +backend data to the internal account representation and applied when the user +data is reported by the backend as active. On the other hand having a separate +background process helps updating account data in the backend and produce audit +records on time without waiting for the on-demand logic to apply. It also allows +disabling accounts in the remote identity backends that are connected with +read/write mode (i.e. SCIM push). + +After the successful authentication of the user with password or the federated +workflow the `user.last_active_at` should be set to the current date time. + +## Consequences + +- Authentication with methods other than username password are not updating the + `lst_active_at`. Due to that the account that used i.e. application + credentials for the activation for more than X days would become disabled. This + requires account to perform periodic login using the password. + +- It should be considered to update application credentials workflow to update + the `user.last_active_at` attribute after successful authentication. + +- It could happen that the periodic account deactivation process does not work + for certain amount of time (i.e due to bugs in the code or the chosen + frequency) allowing the user to login when it should have been disabled. This + can be only prevented by applying the same logic during the conversion of the + database entry to the internal `User` structure the same way like python + keystone is doing. + +- Administrator account can be deactivated. Separate tooling or documentation + how to unlock the account must be present. diff --git a/doc/src/adr/0012-pci-dss-account-password-expiry.md b/doc/src/adr/0012-pci-dss-account-password-expiry.md new file mode 100644 index 00000000..13006688 --- /dev/null +++ b/doc/src/adr/0012-pci-dss-account-password-expiry.md @@ -0,0 +1,46 @@ +# 12. PCI-DSS requirement: Inactive user accounts are removed/disabled + +Date: 2025-11-27 + +## Status + +Accepted + +## Context + +PCI-DSS contains the following requirement to the IAM system: + +If passwords/passphrases are used as the only authentication factor for user +access (i.e., in any single-factor authentication implementation) then either: +• Passwords/passphrases are changed at least once every 90 days, OR +• The security posture of accounts is dynamically analyzed, and real-time +access to resources is automatically determined accordingly. + +Python Keystone implements this requirement with the help of the +`conf.security_compliance.password_expires_days` and `password.expires_at` +during the login attempt to identify whether the specified used password is +expired. `user.options.IGNORE_PASSWORD_EXPIRY_OPT` option allows bypassing the +expiration check. + +## Decision + +For compatibility reasons rust implementation must adhere to the requirement. + +Password expiration is performed after verification that the password is valid. + +- `password.expires_at_int` (as epoch seconds) or the `password.expires_at` (as + date time specifies the password expiration. When none is set password is + considered as valid. Otherwise it is compared against the current time. + +- During account password update operation when user is not having the + `user.options.IGNORE_PASSWORD_EXPIRY_OPT` option enabled the current date time + plus the `conf.security_compliance.password_expires_days` time is persisted as + the `password.expires_at_int` property. + +- Password expiration MUST NOT be enforced in the password change flow to + prevent a permanent lock out. + +## Consequences + +- Administrator account can be deactivated. Separate tooling or documentation + how to unlock the account must be present. diff --git a/rustfmt.toml b/rustfmt.toml new file mode 100644 index 00000000..4b810940 --- /dev/null +++ b/rustfmt.toml @@ -0,0 +1,3 @@ +# format_code_in_doc_comments = true +# unstable_features = true +# wrap_comments = true diff --git a/src/api/common.rs b/src/api/common.rs index 5ad22635..6cd45a03 100644 --- a/src/api/common.rs +++ b/src/api/common.rs @@ -12,7 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 //! Common API helpers -//! use crate::api::error::KeystoneApiError; use crate::api::types::ProjectScope; use crate::keystone::ServiceState; diff --git a/src/api/error.rs b/src/api/error.rs index ea134c4c..ff973e44 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -162,9 +162,6 @@ pub enum KeystoneApiError { #[error(transparent)] JsonExtractorRejection(#[from] JsonRejection), - #[error("the account is disabled for user: {0}")] - UserDisabled(String), - /// Selected authentication is forbidden. #[error("selected authentication is forbidden")] SelectedAuthenticationForbidden, @@ -186,9 +183,7 @@ impl IntoResponse for KeystoneApiError { KeystoneApiError::Conflict(_) => StatusCode::CONFLICT, KeystoneApiError::NotFound { .. } => StatusCode::NOT_FOUND, KeystoneApiError::BadRequest(..) => StatusCode::BAD_REQUEST, - KeystoneApiError::UserDisabled(..) => StatusCode::UNAUTHORIZED, KeystoneApiError::Unauthorized(..) => StatusCode::UNAUTHORIZED, - // KeystoneApiError::AuthenticationInfo { .. } => StatusCode::UNAUTHORIZED, KeystoneApiError::Forbidden => StatusCode::FORBIDDEN, KeystoneApiError::Policy { .. } => StatusCode::FORBIDDEN, KeystoneApiError::SelectedAuthenticationForbidden @@ -202,7 +197,11 @@ impl IntoResponse for KeystoneApiError { | KeystoneApiError::RevokeProvider { .. } | KeystoneApiError::Other(..) => StatusCode::INTERNAL_SERVER_ERROR, _ => - // KeystoneApiError::SubjectTokenMissing | KeystoneApiError::InvalidHeader | KeystoneApiError::InvalidToken | KeystoneApiError::Token{..} | KeystoneApiError::WebAuthN{..} | KeystoneApiError::Uuid {..} | KeystoneApiError::Serde {..} | KeystoneApiError::DomainIdOrName | KeystoneApiError::ProjectIdOrName | KeystoneApiError::ProjectDomain => + // KeystoneApiError::SubjectTokenMissing | KeystoneApiError::InvalidHeader | + // KeystoneApiError::InvalidToken | KeystoneApiError::Token{..} | + // KeystoneApiError::WebAuthN{..} | KeystoneApiError::Uuid {..} | + // KeystoneApiError::Serde {..} | KeystoneApiError::DomainIdOrName | + // KeystoneApiError::ProjectIdOrName | KeystoneApiError::ProjectDomain => { StatusCode::BAD_REQUEST } @@ -341,7 +340,8 @@ impl IntoResponse for WebauthnError { WebauthnError::UserHasNoCredentials => "User Has No Credentials", }; - // its often easiest to implement `IntoResponse` by calling other implementations + // its often easiest to implement `IntoResponse` by calling other + // implementations (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() } } @@ -352,7 +352,20 @@ impl From for KeystoneApiError { AuthenticationError::AuthenticatedInfoBuilder { source } => { KeystoneApiError::InternalError(source.to_string()) } - AuthenticationError::UserDisabled(data) => KeystoneApiError::UserDisabled(data), + AuthenticationError::UserDisabled(user_id) => KeystoneApiError::Unauthorized(Some( + format!("The account is disabled for the user: {user_id}"), + )), + AuthenticationError::UserLocked(user_id) => KeystoneApiError::Unauthorized(Some( + format!("The account is locked for the user: {user_id}"), + )), + AuthenticationError::UserPasswordExpired(user_id) => { + KeystoneApiError::Unauthorized(Some(format!( + "The password is expired and need to be changed for user: {user_id}" + ))) + } + AuthenticationError::UserNameOrPasswordWrong => { + KeystoneApiError::Unauthorized(Some("Invalid username or password".to_string())) + } AuthenticationError::TokenRenewalForbidden => { KeystoneApiError::SelectedAuthenticationForbidden } @@ -365,9 +378,6 @@ impl From for KeystoneApiError { fn from(value: IdentityProviderError) -> Self { match value { IdentityProviderError::AuthenticationInfo { source } => source.into(), - IdentityProviderError::WrongUsernamePassword => { - Self::Unauthorized(Some("Invalid username or password".to_string())) - } _ => Self::IdentityError { source: value }, } } diff --git a/src/api/mod.rs b/src/api/mod.rs index 8eb5b994..39a0597d 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -12,7 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 //! Keystone API -//! use axum::{ extract::State, http::{HeaderMap, header}, diff --git a/src/api/types.rs b/src/api/types.rs index ccec68d8..424b722c 100644 --- a/src/api/types.rs +++ b/src/api/types.rs @@ -172,12 +172,13 @@ impl From)>> for Catalog { /// The authorization scope, including the system, a project, or a domain. /// -/// If multiple scopes are specified in the same request (e.g. project and domain or domain and -/// system) an HTTP 400 Bad Request will be returned, as a token cannot be simultaneously scoped to -/// multiple authorization targets. An ID is sufficient to uniquely identify a project but if a -/// project is specified by name, then the domain of the project must also be specified in order to -/// uniquely identify the project by name. A domain scope may be specified by either the domain’s -/// ID or name with equivalent results. +/// If multiple scopes are specified in the same request (e.g. project and +/// domain or domain and system) an HTTP 400 Bad Request will be returned, as a +/// token cannot be simultaneously scoped to multiple authorization targets. An +/// ID is sufficient to uniquely identify a project but if a project is +/// specified by name, then the domain of the project must also be specified in +/// order to uniquely identify the project by name. A domain scope may be +/// specified by either the domain’s ID or name with equivalent results. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[serde(rename_all = "lowercase")] pub enum Scope { diff --git a/src/api/v3/auth/token/common.rs b/src/api/v3/auth/token/common.rs index 013d822e..86303020 100644 --- a/src/api/v3/auth/token/common.rs +++ b/src/api/v3/auth/token/common.rs @@ -45,9 +45,10 @@ pub(super) async fn get_project_info_builder( Ok(project_response) } -/// Authenticate the user ignoring any scope information. It is important not to expose any -/// hints that user, project, domain, etc might exist before we have authenticated them by -/// taking different amount of time in case of certain validations. +/// Authenticate the user ignoring any scope information. It is important not to +/// expose any hints that user, project, domain, etc might exist before we have +/// authenticated them by taking different amount of time in case of certain +/// validations. pub(super) async fn authenticate_request( state: &ServiceState, req: &AuthRequest, @@ -61,7 +62,7 @@ pub(super) async fn authenticate_request( state .provider .get_identity_provider() - .authenticate_by_password(state, req) + .authenticate_by_password(state, &req) .await?, ); } diff --git a/src/api/v3/auth/token/delete.rs b/src/api/v3/auth/token/delete.rs index b0351251..5ea9bddb 100644 --- a/src/api/v3/auth/token/delete.rs +++ b/src/api/v3/auth/token/delete.rs @@ -12,7 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 //! Revoke the authentication token. -//! use axum::{ extract::State, @@ -34,9 +33,9 @@ use crate::token::TokenApi; /// /// Revokes a token. /// -/// This call is similar to the HEAD /auth/tokens call except that the `X-Subject-Token` token is -/// immediately not valid, regardless of the expires_at attribute value. An additional -/// `X-Auth-Token` is not required. +/// This call is similar to the HEAD /auth/tokens call except that the +/// `X-Subject-Token` token is immediately not valid, regardless of the +/// expires_at attribute value. An additional `X-Auth-Token` is not required. #[utoipa::path( delete, path = "/", @@ -63,8 +62,8 @@ pub(super) async fn delete( .map_err(|_| KeystoneApiError::InvalidHeader)? .to_string(); - // Default behavior is to return 404 for expired tokens. It makes sense to log internally the - // error before mapping it. + // Default behavior is to return 404 for expired tokens. It makes sense to log + // internally the error before mapping it. let token = state .provider .get_token_provider() diff --git a/src/api/v3/auth/token/mod.rs b/src/api/v3/auth/token/mod.rs index 0d876054..3328c827 100644 --- a/src/api/v3/auth/token/mod.rs +++ b/src/api/v3/auth/token/mod.rs @@ -12,8 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 //! Authentication API. -//! -//! use utoipa_axum::{router::OpenApiRouter, routes}; use crate::keystone::ServiceState; diff --git a/src/api/v3/auth/token/show.rs b/src/api/v3/auth/token/show.rs index 25641db2..ebb04c81 100644 --- a/src/api/v3/auth/token/show.rs +++ b/src/api/v3/auth/token/show.rs @@ -13,7 +13,9 @@ // SPDX-License-Identifier: Apache-2.0 //! Validate token. //! -//! Check the token whether it can be accepted as a valid. Additionally the token is being expanded returning information like the user_id, scope, roles, etc. +//! Check the token whether it can be accepted as a valid. Additionally the +//! token is being expanded returning information like the user_id, scope, +//! roles, etc. //! //! Token validations: //! @@ -41,12 +43,13 @@ use crate::token::TokenApi; /// Validate and show information for token. /// -/// Validates and shows information for a token, including its expiration date and authorization -/// scope. +/// Validates and shows information for a token, including its expiration date +/// and authorization scope. /// /// Pass your own token in the X-Auth-Token request header. /// -/// Pass the token that you want to validate in the X-Subject-Token request header. +/// Pass the token that you want to validate in the X-Subject-Token request +/// header. #[utoipa::path( get, path = "/", @@ -75,8 +78,8 @@ pub(super) async fn show( .map_err(|_| KeystoneApiError::InvalidHeader)? .to_string(); - // Default behavior is to return 404 for expired tokens. It makes sense to log internally the - // error before mapping it. + // Default behavior is to return 404 for expired tokens. It makes sense to log + // internally the error before mapping it. let token = state .provider .get_token_provider() diff --git a/src/api/v3/auth/token/types.rs b/src/api/v3/auth/token/types.rs index 535326e1..4b1804d7 100644 --- a/src/api/v3/auth/token/types.rs +++ b/src/api/v3/auth/token/types.rs @@ -32,23 +32,28 @@ use crate::token::Token as BackendToken; #[derive(Builder, Clone, Debug, Default, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct Token { - /// A list of one or two audit IDs. An audit ID is a unique, randomly generated, URL-safe - /// string that you can use to track a token. The first audit ID is the current audit ID for - /// the token. The second audit ID is present for only re-scoped tokens and is the audit ID - /// from the token before it was re-scoped. A re- scoped token is one that was exchanged for - /// another token of the same or different scope. You can use these audit IDs to track the use - /// of a token or chain of tokens across multiple requests and endpoints without exposing the + /// A list of one or two audit IDs. An audit ID is a unique, randomly + /// generated, URL-safe string that you can use to track a token. The + /// first audit ID is the current audit ID for the token. The second + /// audit ID is present for only re-scoped tokens and is the audit ID + /// from the token before it was re-scoped. A re- scoped token is one that + /// was exchanged for another token of the same or different scope. You + /// can use these audit IDs to track the use of a token or chain of + /// tokens across multiple requests and endpoints without exposing the /// token ID to non-privileged users. pub audit_ids: Vec, - /// The authentication methods, which are commonly password, token, or other methods. Indicates - /// the accumulated set of authentication methods that were used to obtain the token. For - /// example, if the token was obtained by password authentication, it contains password. Later, - /// if the token is exchanged by using the token authentication method one or more times, the - /// subsequently created tokens contain both password and token in their methods attribute. - /// Unlike multi-factor authentication, the methods attribute merely indicates the methods that - /// were used to authenticate the user in exchange for a token. The client is responsible for - /// determining the total number of authentication factors. + /// The authentication methods, which are commonly password, token, or other + /// methods. Indicates the accumulated set of authentication methods + /// that were used to obtain the token. For example, if the token was + /// obtained by password authentication, it contains password. Later, if + /// the token is exchanged by using the token authentication method one or + /// more times, the subsequently created tokens contain both password + /// and token in their methods attribute. Unlike multi-factor + /// authentication, the methods attribute merely indicates the methods that + /// were used to authenticate the user in exchange for a token. The client + /// is responsible for determining the total number of authentication + /// factors. pub methods: Vec, /// The date and time when the token expires. @@ -58,14 +63,16 @@ pub struct Token { //#[builder(default)] pub user: User, - /// A project object including the id, name and domain object representing the project the - /// token is scoped to. This is only included in tokens that are scoped to a project. + /// A project object including the id, name and domain object representing + /// the project the token is scoped to. This is only included in tokens + /// that are scoped to a project. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub project: Option, - /// A domain object including the id and name representing the domain the token is scoped to. - /// This is only included in tokens that are scoped to a domain. + /// A domain object including the id and name representing the domain the + /// token is scoped to. This is only included in tokens that are scoped + /// to a domain. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub domain: Option, @@ -107,13 +114,16 @@ pub struct AuthRequestInner { /// An identity object. pub identity: Identity, - /// The authorization scope, including the system (Since v3.10), a project, or a domain (Since - /// v3.4). If multiple scopes are specified in the same request (e.g. project and domain or - /// domain and system) an HTTP 400 Bad Request will be returned, as a token cannot be - /// simultaneously scoped to multiple authorization targets. An ID is sufficient to uniquely - /// identify a project but if a project is specified by name, then the domain of the project - /// must also be specified in order to uniquely identify the project by name. A domain scope - /// may be specified by either the domain’s ID or name with equivalent results. + /// The authorization scope, including the system (Since v3.10), a project, + /// or a domain (Since v3.4). If multiple scopes are specified in the + /// same request (e.g. project and domain or domain and system) an HTTP + /// 400 Bad Request will be returned, as a token cannot be + /// simultaneously scoped to multiple authorization targets. An ID is + /// sufficient to uniquely identify a project but if a project is + /// specified by name, then the domain of the project must also be + /// specified in order to uniquely identify the project by name. A domain + /// scope may be specified by either the domain’s ID or name with + /// equivalent results. pub scope: Option, } @@ -121,7 +131,8 @@ pub struct AuthRequestInner { #[derive(Builder, Clone, Debug, Default, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(into, strip_option))] pub struct Identity { - /// The authentication method. For password authentication, specify password. + /// The authentication method. For password authentication, specify + /// password. pub methods: Vec, /// The password object, contains the authentication information. @@ -224,16 +235,17 @@ pub struct TokenAuth { #[derive(Clone, Debug, Default, Deserialize, Serialize, IntoParams)] pub struct CreateTokenParameters { - /// The authentication response excludes the service catalog. By default, the response includes - /// the service catalog. + /// The authentication response excludes the service catalog. By default, + /// the response includes the service catalog. pub nocatalog: Option, } #[derive(Clone, Debug, Default, Deserialize, Serialize, IntoParams)] pub struct ValidateTokenParameters { - /// The authentication response excludes the service catalog. By default, the response includes - /// the service catalog. + /// The authentication response excludes the service catalog. By default, + /// the response includes the service catalog. pub nocatalog: Option, - /// Allow fetching a token that has expired. By default expired tokens return a 404 exception. + /// Allow fetching a token that has expired. By default expired tokens + /// return a 404 exception. pub allow_expired: Option, } diff --git a/src/api/v3/role_assignment/types.rs b/src/api/v3/role_assignment/types.rs index 64277aa2..567f88aa 100644 --- a/src/api/v3/role_assignment/types.rs +++ b/src/api/v3/role_assignment/types.rs @@ -177,8 +177,8 @@ pub struct RoleAssignmentListParameters { #[serde(rename = "group.id")] pub group_id: Option, - /// Returns the effective assignments, including any assignments gained by virtue of group - /// membership. + /// Returns the effective assignments, including any assignments gained by + /// virtue of group membership. pub effective: Option, /// Filters the response by a project ID. @@ -193,8 +193,9 @@ pub struct RoleAssignmentListParameters { #[serde(rename = "user.id")] pub user_id: Option, - /// If set to true, then the names of any entities returned will be include as well as their - /// IDs. Any value other than 0 (including no value) will be interpreted as true. + /// If set to true, then the names of any entities returned will be include + /// as well as their IDs. Any value other than 0 (including no value) + /// will be interpreted as true. /// /// New in version 3.6 #[serde(default)] diff --git a/src/api/v3/user/types.rs b/src/api/v3/user/types.rs index d9fb1867..65258c44 100644 --- a/src/api/v3/user/types.rs +++ b/src/api/v3/user/types.rs @@ -32,14 +32,17 @@ pub struct User { pub domain_id: String, /// User name pub name: String, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: bool, - /// The ID of the default project for the user. A user’s default project must not be a domain. - /// Setting this attribute does not grant any actual authorization on the project, and is - /// merely provided for convenience. Therefore, the referenced project does not need to exist - /// within the user domain. (Since v3.1) If the user does not have authorization to their - /// default project, the default project is ignored at token creation. (Since v3.1) - /// Additionally, if your default project is not valid, a token is issued without an explicit + /// The ID of the default project for the user. A user’s default project + /// must not be a domain. Setting this attribute does not grant any + /// actual authorization on the project, and is merely provided for + /// convenience. Therefore, the referenced project does not need to exist + /// within the user domain. (Since v3.1) If the user does not have + /// authorization to their default project, the default project is + /// ignored at token creation. (Since v3.1) Additionally, if your + /// default project is not valid, a token is issued without an explicit /// scope of authorization. #[serde(skip_serializing_if = "Option::is_none")] pub default_project_id: Option, @@ -50,8 +53,9 @@ pub struct User { pub password_expires_at: Option>, /// The resource options for the user. Available resource options are /// ignore_change_password_upon_first_use, ignore_password_expiry, - /// ignore_lockout_failure_attempts, lock_password, multi_factor_auth_enabled, and - /// multi_factor_auth_rules ignore_user_inactivity. + /// ignore_lockout_failure_attempts, lock_password, + /// multi_factor_auth_enabled, and multi_factor_auth_rules + /// ignore_user_inactivity. #[serde(skip_serializing_if = "Option::is_none")] pub options: Option, } @@ -68,22 +72,26 @@ pub struct UserCreate { pub domain_id: String, /// The user name. Must be unique within the owning domain. pub name: String, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: Option, - /// The ID of the default project for the user. A user’s default project must not be a domain. - /// Setting this attribute does not grant any actual authorization on the project, and is - /// merely provided for convenience. Therefore, the referenced project does not need to exist - /// within the user domain. (Since v3.1) If the user does not have authorization to their - /// default project, the default project is ignored at token creation. (Since v3.1) - /// Additionally, if your default project is not valid, a token is issued without an explicit + /// The ID of the default project for the user. A user’s default project + /// must not be a domain. Setting this attribute does not grant any + /// actual authorization on the project, and is merely provided for + /// convenience. Therefore, the referenced project does not need to exist + /// within the user domain. (Since v3.1) If the user does not have + /// authorization to their default project, the default project is + /// ignored at token creation. (Since v3.1) Additionally, if your + /// default project is not valid, a token is issued without an explicit /// scope of authorization. pub default_project_id: Option, /// The password for the user. pub password: Option, /// The resource options for the user. Available resource options are /// ignore_change_password_upon_first_use, ignore_password_expiry, - /// ignore_lockout_failure_attempts, lock_password, multi_factor_auth_enabled, and - /// multi_factor_auth_rules ignore_user_inactivity. + /// ignore_lockout_failure_attempts, lock_password, + /// multi_factor_auth_enabled, and multi_factor_auth_rules + /// ignore_user_inactivity. pub options: Option, /// Additional user properties #[serde(flatten)] @@ -94,22 +102,26 @@ pub struct UserCreate { pub struct UserUpdateRequest { /// The user name. Must be unique within the owning domain. pub name: Option, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: Option, - /// The ID of the default project for the user. A user’s default project must not be a domain. - /// Setting this attribute does not grant any actual authorization on the project, and is - /// merely provided for convenience. Therefore, the referenced project does not need to exist - /// within the user domain. (Since v3.1) If the user does not have authorization to their - /// default project, the default project is ignored at token creation. (Since v3.1) - /// Additionally, if your default project is not valid, a token is issued without an explicit + /// The ID of the default project for the user. A user’s default project + /// must not be a domain. Setting this attribute does not grant any + /// actual authorization on the project, and is merely provided for + /// convenience. Therefore, the referenced project does not need to exist + /// within the user domain. (Since v3.1) If the user does not have + /// authorization to their default project, the default project is + /// ignored at token creation. (Since v3.1) Additionally, if your + /// default project is not valid, a token is issued without an explicit /// scope of authorization. pub default_project_id: Option, /// The password for the user. pub password: Option, /// The resource options for the user. Available resource options are /// ignore_change_password_upon_first_use, ignore_password_expiry, - /// ignore_lockout_failure_attempts, lock_password, multi_factor_auth_enabled, and - /// multi_factor_auth_rules ignore_user_inactivity. + /// ignore_lockout_failure_attempts, lock_password, + /// multi_factor_auth_enabled, and multi_factor_auth_rules + /// ignore_user_inactivity. pub options: Option, /// Additional user properties #[serde(flatten)] diff --git a/src/api/v4/auth/passkey/finish.rs b/src/api/v4/auth/passkey/finish.rs index 98bf1c58..2c72f594 100644 --- a/src/api/v4/auth/passkey/finish.rs +++ b/src/api/v4/auth/passkey/finish.rs @@ -31,8 +31,8 @@ use crate::token::TokenApi; /// Finish user passkey authentication. /// -/// Exchange the challenge signed with one of the users passkeys or security devices for the -/// unscoped Keystone API token. +/// Exchange the challenge signed with one of the users passkeys or security +/// devices for the unscoped Keystone API token. #[utoipa::path( post, path = "/finish", @@ -64,8 +64,8 @@ pub(super) async fn finish( .get_user_webauthn_credential_authentication_state(&state, &user_id) .await? { - // We explicitly try to deserealize the request data directly into the underlying - // webauthn_rs type. + // We explicitly try to deserealize the request data directly into the + // underlying webauthn_rs type. match state .webauthn .finish_passkey_authentication(&req.try_into()?, &s) diff --git a/src/api/v4/auth/passkey/start.rs b/src/api/v4/auth/passkey/start.rs index c75c26cc..fd7fc58c 100644 --- a/src/api/v4/auth/passkey/start.rs +++ b/src/api/v4/auth/passkey/start.rs @@ -24,9 +24,9 @@ use crate::keystone::ServiceState; /// Start passkey authentication for the user. /// -/// Initiate a passkey login for the user. The user must have at least one passkey previously -/// registered. When the user does not exist a fake challenge is being returned to prevent id -/// scanning. +/// Initiate a passkey login for the user. The user must have at least one +/// passkey previously registered. When the user does not exist a fake challenge +/// is being returned to prevent id scanning. #[utoipa::path( post, path = "/start", @@ -46,7 +46,8 @@ pub(super) async fn start( State(state): State, Json(req): Json, ) -> Result { - // TODO: Check user existence and simulate the response when the user does not exist. + // TODO: Check user existence and simulate the response when the user does not + // exist. state .provider .get_identity_provider() diff --git a/src/api/v4/auth/passkey/types.rs b/src/api/v4/auth/passkey/types.rs index 7837733d..9874376d 100644 --- a/src/api/v4/auth/passkey/types.rs +++ b/src/api/v4/auth/passkey/types.rs @@ -33,8 +33,9 @@ pub struct PasskeyUserAuthenticationRequest { /// Passkey Authorization challenge. /// -/// A JSON serializable challenge which is issued to the user’s webbrowser for handling. This is -/// meant to be opaque, that is, you should not need to inspect or alter the content of the struct +/// A JSON serializable challenge which is issued to the user’s webbrowser for +/// handling. This is meant to be opaque, that is, you should not need to +/// inspect or alter the content of the struct /// - you should serialise it and transmit it to the client only. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct PasskeyAuthenticationStartResponse { @@ -70,11 +71,12 @@ pub struct PublicKeyCredentialRequestOptions { pub user_verification: UserVerificationPolicy, } -/// Request in residentkey workflows that conditional mediation should be used in the UI, or not. +/// Request in residentkey workflows that conditional mediation should be used +/// in the UI, or not. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub enum Mediation { - /// Discovered credentials are presented to the user in a dialog. Conditional UI is used. See - /// + /// Discovered credentials are presented to the user in a dialog. + /// Conditional UI is used. See /// Conditional, } @@ -116,52 +118,58 @@ pub enum AuthenticatorTransport { /// , and each variant lists /// it’s effects. /// -/// To be clear, Verification means that the Authenticator perform extra or supplementary -/// interaction with the user to verify who they are. An example of this is Apple Touch Id required -/// a fingerprint to be verified, or a yubico device requiring a pin in addition to a touch event. +/// To be clear, Verification means that the Authenticator perform extra or +/// supplementary interaction with the user to verify who they are. An example +/// of this is Apple Touch Id required a fingerprint to be verified, or a yubico +/// device requiring a pin in addition to a touch event. /// -/// An example of a non-verified interaction is a yubico device with no pin where touch is the only -/// interaction - we only verify a user is present, but we don’t have extra details to the -/// legitimacy of that user. +/// An example of a non-verified interaction is a yubico device with no pin +/// where touch is the only interaction - we only verify a user is present, but +/// we don’t have extra details to the legitimacy of that user. /// -/// As UserVerificationPolicy is only used in credential registration, this stores the verification -/// state of the credential in the persisted credential. These persisted credentials define which -/// UserVerificationPolicy is issued during authentications. +/// As UserVerificationPolicy is only used in credential registration, this +/// stores the verification state of the credential in the persisted credential. +/// These persisted credentials define which UserVerificationPolicy is issued +/// during authentications. /// -/// IMPORTANT - Due to limitations of the webauthn specification, CTAP devices, and browser -/// implementations, the only secure choice as an RP is required. +/// IMPORTANT - Due to limitations of the webauthn specification, CTAP devices, +/// and browser implementations, the only secure choice as an RP is required. /// -/// ⚠️ WARNING - discouraged is marked with a warning, as some authenticators will FORCE -/// verification during registration but NOT during authentication. This makes it impossible -/// for a relying party to consistently enforce user verification, which can confuse users and -/// lead them to distrust user verification is being enforced. +/// ⚠️ WARNING - discouraged is marked with a warning, as some authenticators +/// will FORCE verification during registration but NOT during authentication. +/// This makes it impossible for a relying party to consistently enforce user +/// verification, which can confuse users and lead them to distrust user +/// verification is being enforced. /// -/// ⚠️ WARNING - preferred can lead to authentication errors in some cases due to browser -/// peripheral exchange allowing authentication verification bypass. Webauthn RS is not -/// vulnerable to these bypasses due to our tracking of UV during registration through -/// authentication, however preferred can cause legitimate credentials to not prompt for UV -/// correctly due to browser perhipheral exchange leading Webauthn RS to deny them in what +/// ⚠️ WARNING - preferred can lead to authentication errors in some cases due +/// to browser peripheral exchange allowing authentication verification +/// bypass. Webauthn RS is not vulnerable to these bypasses due to our +/// tracking of UV during registration through authentication, however +/// preferred can cause legitimate credentials to not prompt for UV correctly +/// due to browser perhipheral exchange leading Webauthn RS to deny them in what /// should otherwise be legitimate operations. -/// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub enum UserVerificationPolicy { - /// Require user verification bit to be set, and fail the registration or authentication if - /// false. If the authenticator is not able to perform verification, it will not be usable with - /// this policy. + /// Require user verification bit to be set, and fail the registration or + /// authentication if false. If the authenticator is not able to perform + /// verification, it will not be usable with this policy. /// - /// This policy is the default as it is the only secure and consistent user verification - /// option. + /// This policy is the default as it is the only secure and consistent user + /// verification option. Required, - /// Prefer UV if possible, but ignore if not present. In other webauthn deployments this is - /// bypassable as it implies the library will not check UV is set correctly for this - /// credential. Webauthn-RS is not vulnerable to this as we check the UV state always based on + /// Prefer UV if possible, but ignore if not present. In other webauthn + /// deployments this is bypassable as it implies the library will not + /// check UV is set correctly for this credential. Webauthn-RS is not + /// vulnerable to this as we check the UV state always based on /// it’s presence at registration. /// - /// However, in some cases use of this policy can lead to some credentials failing to verify - /// correctly due to browser peripheral exchange bypasses. + /// However, in some cases use of this policy can lead to some credentials + /// failing to verify correctly due to browser peripheral exchange + /// bypasses. Preferred, - /// Discourage - but do not prevent - user verification from being supplied. Many CTAP devices - /// will attempt UV during registration but not authentication leading to user confusion. + /// Discourage - but do not prevent - user verification from being supplied. + /// Many CTAP devices will attempt UV during registration but not + /// authentication leading to user confusion. DiscouragedDoNotUse, } @@ -212,11 +220,12 @@ pub struct HmacGetSecretInput { pub output2: Option, } -/// A client response to an authentication challenge. This contains all required information to -/// asses and assert trust in a credentials legitimacy, followed by authentication to a user. +/// A client response to an authentication challenge. This contains all required +/// information to asses and assert trust in a credentials legitimacy, followed +/// by authentication to a user. /// -/// You should not need to handle the inner content of this structure - you should provide this to -/// the correctly handling function of Webauthn only. +/// You should not need to handle the inner content of this structure - you +/// should provide this to the correctly handling function of Webauthn only. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct PasskeyAuthenticationFinishRequest { /// The credential Id, likely base64. diff --git a/src/api/v4/auth/token/mod.rs b/src/api/v4/auth/token/mod.rs index d801a808..635b952c 100644 --- a/src/api/v4/auth/token/mod.rs +++ b/src/api/v4/auth/token/mod.rs @@ -12,8 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 //! Authentication API. -//! -//! use utoipa_axum::router::OpenApiRouter; use crate::api::v3::auth::token as v3_token; diff --git a/src/api/v4/auth/token/types.rs b/src/api/v4/auth/token/types.rs index 5023285d..1610e3d9 100644 --- a/src/api/v4/auth/token/types.rs +++ b/src/api/v4/auth/token/types.rs @@ -32,23 +32,28 @@ use crate::token::Token as BackendToken; #[derive(Builder, Clone, Debug, Default, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct Token { - /// A list of one or two audit IDs. An audit ID is a unique, randomly generated, URL-safe - /// string that you can use to track a token. The first audit ID is the current audit ID for - /// the token. The second audit ID is present for only re-scoped tokens and is the audit ID - /// from the token before it was re-scoped. A re- scoped token is one that was exchanged for - /// another token of the same or different scope. You can use these audit IDs to track the use - /// of a token or chain of tokens across multiple requests and endpoints without exposing the + /// A list of one or two audit IDs. An audit ID is a unique, randomly + /// generated, URL-safe string that you can use to track a token. The + /// first audit ID is the current audit ID for the token. The second + /// audit ID is present for only re-scoped tokens and is the audit ID + /// from the token before it was re-scoped. A re- scoped token is one that + /// was exchanged for another token of the same or different scope. You + /// can use these audit IDs to track the use of a token or chain of + /// tokens across multiple requests and endpoints without exposing the /// token ID to non-privileged users. pub audit_ids: Vec, - /// The authentication methods, which are commonly password, token, or other methods. Indicates - /// the accumulated set of authentication methods that were used to obtain the token. For - /// example, if the token was obtained by password authentication, it contains password. Later, - /// if the token is exchanged by using the token authentication method one or more times, the - /// subsequently created tokens contain both password and token in their methods attribute. - /// Unlike multi-factor authentication, the methods attribute merely indicates the methods that - /// were used to authenticate the user in exchange for a token. The client is responsible for - /// determining the total number of authentication factors. + /// The authentication methods, which are commonly password, token, or other + /// methods. Indicates the accumulated set of authentication methods + /// that were used to obtain the token. For example, if the token was + /// obtained by password authentication, it contains password. Later, if + /// the token is exchanged by using the token authentication method one or + /// more times, the subsequently created tokens contain both password + /// and token in their methods attribute. Unlike multi-factor + /// authentication, the methods attribute merely indicates the methods that + /// were used to authenticate the user in exchange for a token. The client + /// is responsible for determining the total number of authentication + /// factors. pub methods: Vec, /// The date and time when the token expires. @@ -58,14 +63,16 @@ pub struct Token { //#[builder(default)] pub user: User, - /// A project object including the id, name and domain object representing the project the - /// token is scoped to. This is only included in tokens that are scoped to a project. + /// A project object including the id, name and domain object representing + /// the project the token is scoped to. This is only included in tokens + /// that are scoped to a project. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub project: Option, - /// A domain object including the id and name representing the domain the token is scoped to. - /// This is only included in tokens that are scoped to a domain. + /// A domain object including the id and name representing the domain the + /// token is scoped to. This is only included in tokens that are scoped + /// to a domain. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub domain: Option, @@ -107,20 +114,24 @@ pub struct AuthRequestInner { /// An identity object. pub identity: Identity, - /// The authorization scope, including the system (Since v3.10), a project, or a domain (Since - /// v3.4). If multiple scopes are specified in the same request (e.g. project and domain or - /// domain and system) an HTTP 400 Bad Request will be returned, as a token cannot be - /// simultaneously scoped to multiple authorization targets. An ID is sufficient to uniquely - /// identify a project but if a project is specified by name, then the domain of the project - /// must also be specified in order to uniquely identify the project by name. A domain scope - /// may be specified by either the domain’s ID or name with equivalent results. + /// The authorization scope, including the system (Since v3.10), a project, + /// or a domain (Since v3.4). If multiple scopes are specified in the + /// same request (e.g. project and domain or domain and system) an HTTP + /// 400 Bad Request will be returned, as a token cannot be + /// simultaneously scoped to multiple authorization targets. An ID is + /// sufficient to uniquely identify a project but if a project is + /// specified by name, then the domain of the project must also be + /// specified in order to uniquely identify the project by name. A domain + /// scope may be specified by either the domain’s ID or name with + /// equivalent results. pub scope: Option, } /// An identity object. #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize, ToSchema)] pub struct Identity { - /// The authentication method. For password authentication, specify password. + /// The authentication method. For password authentication, specify + /// password. pub methods: Vec, /// The password object, contains the authentication information. @@ -217,16 +228,17 @@ pub struct TokenAuth { #[derive(Clone, Debug, Default, Deserialize, Serialize, IntoParams)] pub struct CreateTokenParameters { - /// The authentication response excludes the service catalog. By default, the response includes - /// the service catalog. + /// The authentication response excludes the service catalog. By default, + /// the response includes the service catalog. pub nocatalog: Option, } #[derive(Clone, Debug, Default, Deserialize, Serialize, IntoParams)] pub struct ValidateTokenParameters { - /// The authentication response excludes the service catalog. By default, the response includes - /// the service catalog. + /// The authentication response excludes the service catalog. By default, + /// the response includes the service catalog. pub nocatalog: Option, - /// Allow fetching a token that has expired. By default expired tokens return a 404 exception. + /// Allow fetching a token that has expired. By default expired tokens + /// return a 404 exception. pub allow_expired: Option, } diff --git a/src/api/v4/federation/auth.rs b/src/api/v4/federation/auth.rs index 8ca225c9..44e06833 100644 --- a/src/api/v4/federation/auth.rs +++ b/src/api/v4/federation/auth.rs @@ -41,20 +41,23 @@ pub(super) fn openapi_router() -> OpenApiRouter { /// Authenticate using identity provider. /// -/// Initiate the authentication for the given identity provider. Mapping can be passed, otherwise -/// the one which is set as a default on the identity provider level is used. +/// Initiate the authentication for the given identity provider. Mapping can be +/// passed, otherwise the one which is set as a default on the identity provider +/// level is used. /// -/// The API returns the link to the identity provider which must be open in the web browser. Once -/// user authenticates in the identity provider UI a redirect to the url passed as a callback in -/// the request is being done as a typical oauth2 authorization code callback. The client is -/// responsible for serving this callback server and use received authorization code and state to -/// exchange it for the Keystone token passing it to the `/v4/federation/oidc/callback`. +/// The API returns the link to the identity provider which must be open in the +/// web browser. Once user authenticates in the identity provider UI a redirect +/// to the url passed as a callback in the request is being done as a typical +/// oauth2 authorization code callback. The client is responsible for serving +/// this callback server and use received authorization code and state to +/// exchange it for the Keystone token passing it to the +/// `/v4/federation/oidc/callback`. /// -/// Desired scope (OpenStack) can be also passed to get immediately scoped token after the -/// authentication completes instead of the unscoped token. +/// Desired scope (OpenStack) can be also passed to get immediately scoped token +/// after the authentication completes instead of the unscoped token. /// -/// This is an unauthenticated API call. User, mapping, scope validation will happen when the -/// callback is invoked. +/// This is an unauthenticated API call. User, mapping, scope validation will +/// happen when the callback is invoked. #[utoipa::path( post, path = "/identity_providers/{idp_id}/auth", diff --git a/src/api/v4/federation/error.rs b/src/api/v4/federation/error.rs index a873f2d6..3f318800 100644 --- a/src/api/v4/federation/error.rs +++ b/src/api/v4/federation/error.rs @@ -140,7 +140,8 @@ impl OidcError { } } -/// Convert OIDC error into the [HTTP](KeystoneApiError) with the expected message +/// Convert OIDC error into the [HTTP](KeystoneApiError) with the expected +/// message impl From for KeystoneApiError { #[instrument(level = Level::ERROR)] fn from(value: OidcError) -> Self { diff --git a/src/api/v4/federation/identity_provider/create.rs b/src/api/v4/federation/identity_provider/create.rs index 68ac597b..cdb5ce98 100644 --- a/src/api/v4/federation/identity_provider/create.rs +++ b/src/api/v4/federation/identity_provider/create.rs @@ -28,7 +28,8 @@ use crate::policy::Policy; /// /// Create the identity provider with the specified properties. /// -/// It is expected that only admin user is able to create global identity providers. +/// It is expected that only admin user is able to create global identity +/// providers. #[utoipa::path( post, path = "/", diff --git a/src/api/v4/federation/identity_provider/delete.rs b/src/api/v4/federation/identity_provider/delete.rs index ac14bdb6..8606adc8 100644 --- a/src/api/v4/federation/identity_provider/delete.rs +++ b/src/api/v4/federation/identity_provider/delete.rs @@ -31,7 +31,8 @@ use crate::policy::Policy; /// /// Deletes the existing identity provider. /// -/// It is expected that only admin user is allowed to delete the global identity provider +/// It is expected that only admin user is allowed to delete the global identity +/// provider #[utoipa::path( delete, path = "/{idp_id}", @@ -73,7 +74,8 @@ pub(super) async fn remove( ) .await?; - // TODO: decide what to do with the users provisioned using this IDP, mappings, ... + // TODO: decide what to do with the users provisioned using this IDP, mappings, + // ... if current.is_some() { state diff --git a/src/api/v4/federation/identity_provider/list.rs b/src/api/v4/federation/identity_provider/list.rs index bae5944e..eafc6b88 100644 --- a/src/api/v4/federation/identity_provider/list.rs +++ b/src/api/v4/federation/identity_provider/list.rs @@ -33,11 +33,12 @@ use crate::policy::Policy; /// List identity providers. /// -/// List identity providers. Without any filters only global identity providers are returned. -/// With the `domain_id` identity providers owned by the specified identity provider are returned. +/// List identity providers. Without any filters only global identity providers +/// are returned. With the `domain_id` identity providers owned by the specified +/// identity provider are returned. /// -/// It is expected that only global or owned identity providers can be returned, while an admin -/// user is able to list all providers. +/// It is expected that only global or owned identity providers can be returned, +/// while an admin user is able to list all providers. #[utoipa::path( get, path = "/", @@ -295,8 +296,8 @@ mod tests { #[tokio::test] #[traced_test] async fn test_list_all() { - // Test listing ALL idps when the user does not specify the domain_id and is allowed to see - // IDP of other domains (admin) + // Test listing ALL idps when the user does not specify the domain_id and is + // allowed to see IDP of other domains (admin) let mut federation_mock = MockFederationProvider::default(); federation_mock .expect_list_identity_providers() diff --git a/src/api/v4/federation/jwt.rs b/src/api/v4/federation/jwt.rs index fd1e9325..462eceb9 100644 --- a/src/api/v4/federation/jwt.rs +++ b/src/api/v4/federation/jwt.rs @@ -67,9 +67,10 @@ type FullIdToken = IdToken< /// Authentication using the JWT. /// -/// This operation allows user to exchange the JWT issued by the trusted identity provider for the -/// regular Keystone session token. Request specifies the necessary authentication mapping, which -/// is also used to validate expected claims. +/// This operation allows user to exchange the JWT issued by the trusted +/// identity provider for the regular Keystone session token. Request specifies +/// the necessary authentication mapping, which is also used to validate +/// expected claims. #[utoipa::path( post, //path = "/jwt/login", diff --git a/src/api/v4/federation/mapping/list.rs b/src/api/v4/federation/mapping/list.rs index 2c1a609d..e26c2b83 100644 --- a/src/api/v4/federation/mapping/list.rs +++ b/src/api/v4/federation/mapping/list.rs @@ -34,8 +34,8 @@ use crate::policy::Policy; /// /// Without `domain_id` specified global mappings are returned. /// -/// It is expected that listing mappings belonging to the other domain is only allowed to the admin -/// user. +/// It is expected that listing mappings belonging to the other domain is only +/// allowed to the admin user. #[utoipa::path( get, path = "/", diff --git a/src/api/v4/federation/oidc.rs b/src/api/v4/federation/oidc.rs index c0c08235..04a9ecef 100644 --- a/src/api/v4/federation/oidc.rs +++ b/src/api/v4/federation/oidc.rs @@ -52,10 +52,12 @@ pub(super) fn openapi_router() -> OpenApiRouter { /// Authentication callback. /// -/// This operation allows user to exchange the authorization code retrieved from the identity -/// provider after calling the `/v4/federation/identity_providers/{idp_id}/auth` for the Keystone -/// token. When desired scope was passed in that auth initialization call the scoped token is -/// returned (assuming the user is having roles assigned on that scope). +/// This operation allows user to exchange the authorization code retrieved from +/// the identity provider after calling the +/// `/v4/federation/identity_providers/{idp_id}/auth` for the Keystone +/// token. When desired scope was passed in that auth initialization call the +/// scoped token is returned (assuming the user is having roles assigned on that +/// scope). #[utoipa::path( post, path = "/oidc/callback", @@ -156,7 +158,8 @@ pub async fn callback( return Err(OidcError::ClientWithoutDiscoveryNotSupported)?; }; - // Finish authorization request by exchanging the authorization code for the token. + // Finish authorization request by exchanging the authorization code for the + // token. let token_response = client .exchange_code(AuthorizationCode::new(query.code)) .map_err(OidcError::from)? diff --git a/src/api/v4/federation/types/auth.rs b/src/api/v4/federation/types/auth.rs index 05dd5c88..10633ea0 100644 --- a/src/api/v4/federation/types/auth.rs +++ b/src/api/v4/federation/types/auth.rs @@ -40,7 +40,8 @@ pub struct IdentityProviderAuthResponse { pub auth_url: String, } -/// Authentication callback request the user is sending to complete the authentication request. +/// Authentication callback request the user is sending to complete the +/// authentication request. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct AuthCallbackParameters { /// Authentication state. diff --git a/src/api/v4/federation/types/identity_provider.rs b/src/api/v4/federation/types/identity_provider.rs index 5d2f048a..931fe52a 100644 --- a/src/api/v4/federation/types/identity_provider.rs +++ b/src/api/v4/federation/types/identity_provider.rs @@ -35,8 +35,9 @@ pub struct IdentityProvider { /// The Name of the federated identity provider. pub name: String, - /// The ID of the domain this identity provider belongs to. Empty value identifies that the - /// identity provider can be used by other domains as well. + /// The ID of the domain this identity provider belongs to. Empty value + /// identifies that the identity provider can be used by other domains + /// as well. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub domain_id: Option, @@ -46,8 +47,8 @@ pub struct IdentityProvider { #[builder(default)] pub oidc_discovery_url: Option, - /// The oidc `client_id` to use for the private client. The `client_secret` is never returned - /// and can be only overwritten. + /// The oidc `client_id` to use for the private client. The `client_secret` + /// is never returned and can be only overwritten. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub oidc_client_id: Option, @@ -62,8 +63,9 @@ pub struct IdentityProvider { #[builder(default)] pub oidc_response_types: Option>, - /// URL to fetch JsonWebKeySet. This must be set for "jwt" mapping when the provider does not - /// provide discovery endpoint or when it is not standard compliant. + /// URL to fetch JsonWebKeySet. This must be set for "jwt" mapping when the + /// provider does not provide discovery endpoint or when it is not + /// standard compliant. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub jwks_url: Option, @@ -78,8 +80,9 @@ pub struct IdentityProvider { #[builder(default)] pub bound_issuer: Option, - /// Default attribute mapping name which is automatically used when no mapping is explicitly - /// requested. The referred attribute mapping must exist. + /// Default attribute mapping name which is automatically used when no + /// mapping is explicitly requested. The referred attribute mapping must + /// exist. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub default_mapping_name: Option, @@ -106,8 +109,9 @@ pub struct IdentityProviderCreate { /// Identity provider name. pub name: String, - /// The ID of the domain this identity provider belongs to. Empty value identifies that the - /// identity provider can be used by other domains as well. + /// The ID of the domain this identity provider belongs to. Empty value + /// identifies that the identity provider can be used by other domains + /// as well. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] #[schema(nullable = false)] @@ -125,7 +129,8 @@ pub struct IdentityProviderCreate { #[schema(nullable = false)] pub oidc_client_id: Option, - /// The oidc `client_secret` to use for the private client. It is never returned back. + /// The oidc `client_secret` to use for the private client. It is never + /// returned back. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] #[schema(nullable = false)] @@ -143,8 +148,9 @@ pub struct IdentityProviderCreate { #[schema(nullable = false)] pub oidc_response_types: Option>, - /// Optional URL to fetch JsonWebKeySet. Must be specified for JWT authentication when - /// discovery for the provider is not available or not standard compliant. + /// Optional URL to fetch JsonWebKeySet. Must be specified for JWT + /// authentication when discovery for the provider is not available or + /// not standard compliant. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] #[schema(nullable = false)] @@ -162,8 +168,9 @@ pub struct IdentityProviderCreate { #[schema(nullable = false)] pub bound_issuer: Option, - /// Default attribute mapping name which is automatically used when no mapping is explicitly - /// requested. The referred attribute mapping must exist. + /// Default attribute mapping name which is automatically used when no + /// mapping is explicitly requested. The referred attribute mapping must + /// exist. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] #[schema(nullable = false)] @@ -204,8 +211,9 @@ pub struct IdentityProviderUpdate { #[builder(default)] pub oidc_response_types: Option>>, - /// New URL to fetch JsonWebKeySet. This must be set for "jwt" mapping when the provider does not - /// provide discovery endpoint or when it is not standard compliant. + /// New URL to fetch JsonWebKeySet. This must be set for "jwt" mapping when + /// the provider does not provide discovery endpoint or when it is not + /// standard compliant. #[builder(default)] pub jwks_url: Option>, @@ -217,8 +225,9 @@ pub struct IdentityProviderUpdate { #[builder(default)] pub bound_issuer: Option>, - /// New default attribute mapping name which is automatically used when no mapping is explicitly - /// requested. The referred attribute mapping must exist. + /// New default attribute mapping name which is automatically used when no + /// mapping is explicitly requested. The referred attribute mapping must + /// exist. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub default_mapping_name: Option>, diff --git a/src/api/v4/federation/types/mapping.rs b/src/api/v4/federation/types/mapping.rs index 4ab007bb..20feadd4 100644 --- a/src/api/v4/federation/types/mapping.rs +++ b/src/api/v4/federation/types/mapping.rs @@ -38,15 +38,16 @@ pub struct Mapping { /// `domain_id` owning the attribute mapping. /// - /// Unset `domain_id` means the attribute mapping is shared and can be used by different - /// domains. This requires `domain_id_claim` to be present. Attribute mapping can be only - /// shared when the referred identity provider is also shared (does not set the `domain_id` - /// attribute). + /// Unset `domain_id` means the attribute mapping is shared and can be used + /// by different domains. This requires `domain_id_claim` to be present. + /// Attribute mapping can be only shared when the referred identity + /// provider is also shared (does not set the `domain_id` attribute). #[builder(default)] #[serde(skip_serializing_if = "Option::is_none")] pub domain_id: Option, - /// ID of the federated identity provider for which this attribute mapping can be used. + /// ID of the federated identity provider for which this attribute mapping + /// can be used. pub idp_id: String, /// Attribute mapping type ([oidc, jwt]). @@ -123,16 +124,17 @@ pub struct MappingCreate { /// `domain_id` owning the attribute mapping. /// - /// Unset `domain_id` means the attribute mapping is shared and can be used by different - /// domains. This requires `domain_id_claim` to be present. Attribute mapping can be only - /// shared when the referred identity provider is also shared (does not set the `domain_id` - /// attribute). + /// Unset `domain_id` means the attribute mapping is shared and can be used + /// by different domains. This requires `domain_id_claim` to be present. + /// Attribute mapping can be only shared when the referred identity + /// provider is also shared (does not set the `domain_id` attribute). #[builder(default)] #[serde(skip_serializing_if = "Option::is_none")] #[schema(nullable = false)] pub domain_id: Option, - /// ID of the federated identity provider for which this attribute mapping can be used. + /// ID of the federated identity provider for which this attribute mapping + /// can be used. pub idp_id: String, /// Attribute mapping type ([oidc, jwt]). @@ -211,15 +213,16 @@ pub struct MappingUpdate { /// `domain_id` owning the attribute mapping. /// - /// Unset `domain_id` means the attribute mapping is shared and can be used by different - /// domains. This requires `domain_id_claim` to be present. Attribute mapping can be only - /// shared when the referred identity provider is also shared (does not set the `domain_id` - /// attribute). + /// Unset `domain_id` means the attribute mapping is shared and can be used + /// by different domains. This requires `domain_id_claim` to be present. + /// Attribute mapping can be only shared when the referred identity + /// provider is also shared (does not set the `domain_id` attribute). #[builder(default)] #[serde(skip_serializing_if = "Option::is_none")] pub domain_id: Option>, - /// ID of the federated identity provider for which this attribute mapping can be used. + /// ID of the federated identity provider for which this attribute mapping + /// can be used. #[serde(skip_serializing_if = "Option::is_none")] pub idp_id: Option, diff --git a/src/api/v4/token/restriction/create.rs b/src/api/v4/token/restriction/create.rs index 9bb92ab2..6eae2f72 100644 --- a/src/api/v4/token/restriction/create.rs +++ b/src/api/v4/token/restriction/create.rs @@ -28,7 +28,8 @@ use crate::token::TokenApi; /// /// Create the token restriction with the specified properties. /// -/// It is expected that only admin user is able to create token restriction in other domain. +/// It is expected that only admin user is able to create token restriction in +/// other domain. #[utoipa::path( post, path = "/", diff --git a/src/api/v4/token/restriction/list.rs b/src/api/v4/token/restriction/list.rs index b2e82658..a5dc1bcc 100644 --- a/src/api/v4/token/restriction/list.rs +++ b/src/api/v4/token/restriction/list.rs @@ -32,8 +32,9 @@ use crate::token::{ /// List token restrictions. /// -/// List existing token restrictions. By default only admin user is allowed to leave `domain_id` -/// query parameter empty, what means that token restrictions for all domains should be listed. +/// List existing token restrictions. By default only admin user is allowed to +/// leave `domain_id` query parameter empty, what means that token restrictions +/// for all domains should be listed. #[utoipa::path( get, path = "/", diff --git a/src/api/v4/token/restriction/update.rs b/src/api/v4/token/restriction/update.rs index bdd9fd00..eaf47d61 100644 --- a/src/api/v4/token/restriction/update.rs +++ b/src/api/v4/token/restriction/update.rs @@ -32,7 +32,8 @@ use crate::token::TokenApi; /// /// Updates the existing token restriction. /// -/// It is expected that only admin user is able to update token restriction in other domain. +/// It is expected that only admin user is able to update token restriction in +/// other domain. #[utoipa::path( put, path = "/{id}", diff --git a/src/api/v4/user/passkey/register_start.rs b/src/api/v4/user/passkey/register_start.rs index b37c40a3..0d709f28 100644 --- a/src/api/v4/user/passkey/register_start.rs +++ b/src/api/v4/user/passkey/register_start.rs @@ -40,8 +40,9 @@ use crate::policy::Policy; /// Start passkey registration for the user. /// -/// Generate a challenge that the user must sign with the passkey or security device. Signed -/// challenge must be sent to the `/v4/users/{user_id}/passkey/register_finish` endpoint. +/// Generate a challenge that the user must sign with the passkey or security +/// device. Signed challenge must be sent to the +/// `/v4/users/{user_id}/passkey/register_finish` endpoint. #[utoipa::path( post, path = "/register_start", diff --git a/src/api/v4/user/types.rs b/src/api/v4/user/types.rs index 097379bb..6cec4329 100644 --- a/src/api/v4/user/types.rs +++ b/src/api/v4/user/types.rs @@ -34,14 +34,17 @@ pub struct User { pub domain_id: String, /// User name pub name: String, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: bool, - /// The ID of the default project for the user. A user’s default project must not be a domain. - /// Setting this attribute does not grant any actual authorization on the project, and is - /// merely provided for convenience. Therefore, the referenced project does not need to exist - /// within the user domain. (Since v3.1) If the user does not have authorization to their - /// default project, the default project is ignored at token creation. (Since v3.1) - /// Additionally, if your default project is not valid, a token is issued without an explicit + /// The ID of the default project for the user. A user’s default project + /// must not be a domain. Setting this attribute does not grant any + /// actual authorization on the project, and is merely provided for + /// convenience. Therefore, the referenced project does not need to exist + /// within the user domain. (Since v3.1) If the user does not have + /// authorization to their default project, the default project is + /// ignored at token creation. (Since v3.1) Additionally, if your + /// default project is not valid, a token is issued without an explicit /// scope of authorization. #[serde(skip_serializing_if = "Option::is_none")] pub default_project_id: Option, @@ -52,8 +55,9 @@ pub struct User { pub password_expires_at: Option>, /// The resource options for the user. Available resource options are /// ignore_change_password_upon_first_use, ignore_password_expiry, - /// ignore_lockout_failure_attempts, lock_password, multi_factor_auth_enabled, and - /// multi_factor_auth_rules ignore_user_inactivity. + /// ignore_lockout_failure_attempts, lock_password, + /// multi_factor_auth_enabled, and multi_factor_auth_rules + /// ignore_user_inactivity. #[serde(skip_serializing_if = "Option::is_none")] pub options: Option, } @@ -70,22 +74,26 @@ pub struct UserCreate { pub domain_id: String, /// The user name. Must be unique within the owning domain. pub name: String, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: Option, - /// The ID of the default project for the user. A user’s default project must not be a domain. - /// Setting this attribute does not grant any actual authorization on the project, and is - /// merely provided for convenience. Therefore, the referenced project does not need to exist - /// within the user domain. (Since v3.1) If the user does not have authorization to their - /// default project, the default project is ignored at token creation. (Since v3.1) - /// Additionally, if your default project is not valid, a token is issued without an explicit + /// The ID of the default project for the user. A user’s default project + /// must not be a domain. Setting this attribute does not grant any + /// actual authorization on the project, and is merely provided for + /// convenience. Therefore, the referenced project does not need to exist + /// within the user domain. (Since v3.1) If the user does not have + /// authorization to their default project, the default project is + /// ignored at token creation. (Since v3.1) Additionally, if your + /// default project is not valid, a token is issued without an explicit /// scope of authorization. pub default_project_id: Option, /// The password for the user. pub password: Option, /// The resource options for the user. Available resource options are /// ignore_change_password_upon_first_use, ignore_password_expiry, - /// ignore_lockout_failure_attempts, lock_password, multi_factor_auth_enabled, and - /// multi_factor_auth_rules ignore_user_inactivity. + /// ignore_lockout_failure_attempts, lock_password, + /// multi_factor_auth_enabled, and multi_factor_auth_rules + /// ignore_user_inactivity. pub options: Option, /// Additional user properties #[serde(flatten)] @@ -96,22 +104,26 @@ pub struct UserCreate { pub struct UserUpdateRequest { /// The user name. Must be unique within the owning domain. pub name: Option, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: Option, - /// The ID of the default project for the user. A user’s default project must not be a domain. - /// Setting this attribute does not grant any actual authorization on the project, and is - /// merely provided for convenience. Therefore, the referenced project does not need to exist - /// within the user domain. (Since v3.1) If the user does not have authorization to their - /// default project, the default project is ignored at token creation. (Since v3.1) - /// Additionally, if your default project is not valid, a token is issued without an explicit + /// The ID of the default project for the user. A user’s default project + /// must not be a domain. Setting this attribute does not grant any + /// actual authorization on the project, and is merely provided for + /// convenience. Therefore, the referenced project does not need to exist + /// within the user domain. (Since v3.1) If the user does not have + /// authorization to their default project, the default project is + /// ignored at token creation. (Since v3.1) Additionally, if your + /// default project is not valid, a token is issued without an explicit /// scope of authorization. pub default_project_id: Option, /// The password for the user. pub password: Option, /// The resource options for the user. Available resource options are /// ignore_change_password_upon_first_use, ignore_password_expiry, - /// ignore_lockout_failure_attempts, lock_password, multi_factor_auth_enabled, and - /// multi_factor_auth_rules ignore_user_inactivity. + /// ignore_lockout_failure_attempts, lock_password, + /// multi_factor_auth_enabled, and multi_factor_auth_rules + /// ignore_user_inactivity. pub options: Option, /// Additional user properties #[serde(flatten)] diff --git a/src/api/v4/user/types/passkey.rs b/src/api/v4/user/types/passkey.rs index 19958d72..c27f0763 100644 --- a/src/api/v4/user/types/passkey.rs +++ b/src/api/v4/user/types/passkey.rs @@ -18,7 +18,6 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; /// Passkey registration request. -/// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct UserPasskeyRegistrationStartRequest { /// The description for the passkey (name). @@ -35,7 +34,6 @@ pub struct PasskeyCreate { } /// Passkey. -/// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct PasskeyResponse { /// The description for the passkey (name). @@ -55,7 +53,8 @@ pub struct Passkey { /// Passkey challenge. /// -/// This is the WebauthN challenge that need to be signed by the passkey/security device. +/// This is the WebauthN challenge that need to be signed by the +/// passkey/security device. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct UserPasskeyRegistrationStartResponse { /// The options. @@ -116,15 +115,16 @@ pub struct RelyingParty { #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[schema(as = PasskeyUser)] pub struct User { - /// The user’s id in base64 form. This MUST be a unique id, and must NOT contain personally - /// identifying information, as this value can NEVER be changed. If in doubt, use a UUID. + /// The user’s id in base64 form. This MUST be a unique id, and must NOT + /// contain personally identifying information, as this value can NEVER + /// be changed. If in doubt, use a UUID. #[schema(value_type = String, format = Binary, content_encoding = "base64")] pub id: String, - /// A detailed name for the account, such as an email address. This value can change, so must - /// not be used as a primary key. + /// A detailed name for the account, such as an email address. This value + /// can change, so must not be used as a primary key. pub name: String, - /// The user’s preferred name for display. This value can change, so must not be used as a - /// primary key. + /// The user’s preferred name for display. This value can change, so must + /// not be used as a primary key. pub display_name: String, } @@ -145,18 +145,20 @@ pub struct PublicKeyCredentialDescriptor { /// The credential id. #[schema(value_type = String, format = Binary, content_encoding = "base64")] pub id: String, - /// The allowed transports for this credential. Note this is a hint, and is NOT enforced. + /// The allowed transports for this credential. Note this is a hint, and is + /// NOT enforced. #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub transports: Option>, } /// -/// Request in residentkey workflows that conditional mediation should be used in the UI, or not. +/// Request in residentkey workflows that conditional mediation should be used +/// in the UI, or not. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub enum Mediation { - /// Discovered credentials are presented to the user in a dialog. Conditional UI is used. See - /// + /// Discovered credentials are presented to the user in a dialog. + /// Conditional UI is used. See /// Conditional, } @@ -180,8 +182,8 @@ pub struct AllowCredentials { pub enum AuthenticatorTransport { /// Ble, - /// Hybrid transport, formerly caBLE. Part of the level 3 draft specification. - /// + /// Hybrid transport, formerly caBLE. Part of the level 3 draft + /// specification. Hybrid, /// Internal, @@ -198,30 +200,31 @@ pub enum AuthenticatorTransport { /// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct AuthenticatorSelectionCriteria { - /// How the authenticator should be attached to the client machine. Note this is only a hint. - /// It is not enforced in anyway shape or form. . + /// How the authenticator should be attached to the client machine. Note + /// this is only a hint. It is not enforced in anyway shape or form. . #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub authenticator_attachment: Option, - /// Hint to the credential to create a resident key. Note this value should be a member of - /// ResidentKeyRequirement, but client must ignore unknown values, treating an unknown value as - /// if the member does not exist. - /// . + /// Hint to the credential to create a resident key. Note this value should + /// be a member of ResidentKeyRequirement, but client must ignore + /// unknown values, treating an unknown value as if the member does not + /// exist. . #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub resident_key: Option, - /// Hint to the credential to create a resident key. Note this can not be enforced or - /// validated, so the authenticator may choose to ignore this parameter. - /// . + /// Hint to the credential to create a resident key. Note this can not be + /// enforced or validated, so the authenticator may choose to ignore + /// this parameter. . pub require_resident_key: bool, - /// The user verification level to request during registration. Depending on if this - /// authenticator provides verification may affect future interactions as this is associated to - /// the credential during registration. + /// The user verification level to request during registration. Depending on + /// if this authenticator provides verification may affect future + /// interactions as this is associated to the credential during + /// registration. pub user_verification: UserVerificationPolicy, } -/// The authenticator attachment hint. This is NOT enforced, and is only used to help a user select -/// a relevant authenticator type. +/// The authenticator attachment hint. This is NOT enforced, and is only used to +/// help a user select a relevant authenticator type. /// /// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] @@ -229,8 +232,8 @@ pub enum AuthenticatorAttachment { /// Request a device that is part of the machine aka inseparable. /// . Platform, - /// Request a device that can be separated from the machine aka an external token. - /// . + /// Request a device that can be separated from the machine aka an external + /// token. . CrossPlatform, } @@ -241,8 +244,8 @@ pub enum AuthenticatorAttachment { pub enum ResidentKeyRequirement { /// . Discouraged, - /// ⚠️ In all major browsers preferred is identical in behaviour to required. You should use - /// required instead. . + /// ⚠️ In all major browsers preferred is identical in behaviour to + /// required. You should use required instead. . Preferred, /// . Required, @@ -301,8 +304,9 @@ pub enum AttestationFormat { /// Implements `AuthenticatorExtensionsClientInputs` from the spec. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct RequestRegistrationExtensions { - /// ⚠️ - This extension result is always unsigned, and only indicates if the browser requests a - /// residentKey to be created. It has no bearing on the true rk state of the credential. + /// ⚠️ - This extension result is always unsigned, and only indicates if the + /// browser requests a residentKey to be created. It has no bearing on + /// the true rk state of the credential. #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub cred_props: Option, @@ -310,8 +314,8 @@ pub struct RequestRegistrationExtensions { #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub cred_protect: Option, - /// ⚠️ - Browsers support the creation of the secret, but not the retrieval of it. CTAP2.1 - /// create hmac secret. + /// ⚠️ - Browsers support the creation of the secret, but not the retrieval + /// of it. CTAP2.1 create hmac secret. #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub hmac_create_secret: Option, @@ -332,8 +336,9 @@ pub struct RequestRegistrationExtensions { pub struct CredProtect { /// The credential policy to enact. pub credential_protection_policy: CredentialProtectionPolicy, - /// Whether it is better for the authenticator to fail to create a credential rather than - /// ignore the protection policy If no value is provided, the client treats it as false. + /// Whether it is better for the authenticator to fail to create a + /// credential rather than ignore the protection policy If no value is + /// provided, the client treats it as false. #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub enforce_credential_protection_policy: Option, @@ -343,15 +348,17 @@ pub struct CredProtect { #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[repr(u8)] pub enum CredentialProtectionPolicy { - /// This reflects “FIDO_2_0” semantics. In this configuration, performing some form of user - /// verification is optional with or without credentialID list. This is the default state of - /// the credential if the extension is not specified. + /// This reflects “FIDO_2_0” semantics. In this configuration, performing + /// some form of user verification is optional with or without + /// credentialID list. This is the default state of the credential if + /// the extension is not specified. UserVerificationOptional = 1, - /// In this configuration, credential is discovered only when its credentialID is provided by - /// the platform or when some form of user verification is performed. + /// In this configuration, credential is discovered only when its + /// credentialID is provided by the platform or when some form of user + /// verification is performed. UserVerificationOptionalWithCredentialIDList = 2, - /// This reflects that discovery and usage of the credential MUST be preceded by some form of - /// user verification. + /// This reflects that discovery and usage of the credential MUST be + /// preceded by some form of user verification. UserVerificationRequired = 3, } @@ -359,60 +366,67 @@ pub enum CredentialProtectionPolicy { /// , and each variant lists /// it’s effects. /// -/// To be clear, Verification means that the Authenticator perform extra or supplementary -/// interaction with the user to verify who they are. An example of this is Apple Touch Id required -/// a fingerprint to be verified, or a yubico device requiring a pin in addition to a touch event. +/// To be clear, Verification means that the Authenticator perform extra or +/// supplementary interaction with the user to verify who they are. An example +/// of this is Apple Touch Id required a fingerprint to be verified, or a yubico +/// device requiring a pin in addition to a touch event. /// -/// An example of a non-verified interaction is a yubico device with no pin where touch is the only -/// interaction - we only verify a user is present, but we don’t have extra details to the -/// legitimacy of that user. +/// An example of a non-verified interaction is a yubico device with no pin +/// where touch is the only interaction - we only verify a user is present, but +/// we don’t have extra details to the legitimacy of that user. /// -/// As UserVerificationPolicy is only used in credential registration, this stores the verification -/// state of the credential in the persisted credential. These persisted credentials define which -/// UserVerificationPolicy is issued during authentications. +/// As UserVerificationPolicy is only used in credential registration, this +/// stores the verification state of the credential in the persisted credential. +/// These persisted credentials define which UserVerificationPolicy is issued +/// during authentications. /// -/// IMPORTANT - Due to limitations of the webauthn specification, CTAP devices, and browser -/// implementations, the only secure choice as an RP is required. +/// IMPORTANT - Due to limitations of the webauthn specification, CTAP devices, +/// and browser implementations, the only secure choice as an RP is required. /// -/// ⚠️ WARNING - discouraged is marked with a warning, as some authenticators will FORCE -/// verification during registration but NOT during authentication. This makes it impossible -/// for a relying party to consistently enforce user verification, which can confuse users and -/// lead them to distrust user verification is being enforced. +/// ⚠️ WARNING - discouraged is marked with a warning, as some authenticators +/// will FORCE verification during registration but NOT during authentication. +/// This makes it impossible for a relying party to consistently enforce user +/// verification, which can confuse users and lead them to distrust user +/// verification is being enforced. /// -/// ⚠️ WARNING - preferred can lead to authentication errors in some cases due to browser -/// peripheral exchange allowing authentication verification bypass. Webauthn RS is not -/// vulnerable to these bypasses due to our tracking of UV during registration through -/// authentication, however preferred can cause legitimate credentials to not prompt for UV -/// correctly due to browser perhipheral exchange leading Webauthn RS to deny them in what +/// ⚠️ WARNING - preferred can lead to authentication errors in some cases due +/// to browser peripheral exchange allowing authentication verification +/// bypass. Webauthn RS is not vulnerable to these bypasses due to our +/// tracking of UV during registration through authentication, however +/// preferred can cause legitimate credentials to not prompt for UV correctly +/// due to browser perhipheral exchange leading Webauthn RS to deny them in what /// should otherwise be legitimate operations. -/// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub enum UserVerificationPolicy { - /// Require user verification bit to be set, and fail the registration or authentication if - /// false. If the authenticator is not able to perform verification, it will not be usable with - /// this policy. + /// Require user verification bit to be set, and fail the registration or + /// authentication if false. If the authenticator is not able to perform + /// verification, it will not be usable with this policy. /// - /// This policy is the default as it is the only secure and consistent user verification - /// option. + /// This policy is the default as it is the only secure and consistent user + /// verification option. Required, - /// Prefer UV if possible, but ignore if not present. In other webauthn deployments this is - /// bypassable as it implies the library will not check UV is set correctly for this - /// credential. Webauthn-RS is not vulnerable to this as we check the UV state always based on + /// Prefer UV if possible, but ignore if not present. In other webauthn + /// deployments this is bypassable as it implies the library will not + /// check UV is set correctly for this credential. Webauthn-RS is not + /// vulnerable to this as we check the UV state always based on /// it’s presence at registration. /// - /// However, in some cases use of this policy can lead to some credentials failing to verify - /// correctly due to browser peripheral exchange bypasses. + /// However, in some cases use of this policy can lead to some credentials + /// failing to verify correctly due to browser peripheral exchange + /// bypasses. Preferred, - /// Discourage - but do not prevent - user verification from being supplied. Many CTAP devices - /// will attempt UV during registration but not authentication leading to user confusion. + /// Discourage - but do not prevent - user verification from being supplied. + /// Many CTAP devices will attempt UV during registration but not + /// authentication leading to user confusion. DiscouragedDoNotUse, } -/// A client response to a registration challenge. This contains all required information to assess -/// and assert trust in a credential’s legitimacy, followed by registration to a user. +/// A client response to a registration challenge. This contains all required +/// information to assess and assert trust in a credential’s legitimacy, +/// followed by registration to a user. /// -/// You should not need to handle the inner content of this structure - you should provide this to -/// the correctly handling function of Webauthn only. +/// You should not need to handle the inner content of this structure - you +/// should provide this to the correctly handling function of Webauthn only. /// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct UserPasskeyRegistrationFinishRequest { @@ -422,13 +436,13 @@ pub struct UserPasskeyRegistrationFinishRequest { pub description: Option, /// The id of the PublicKey credential, likely in base64. /// - /// This is NEVER actually used in a real registration, because the true credential ID is taken - /// from the attestation data. + /// This is NEVER actually used in a real registration, because the true + /// credential ID is taken from the attestation data. pub id: String, /// The id of the credential, as binary. /// - /// This is NEVER actually used in a real registration, because the true credential ID is taken - /// from the attestation data. + /// This is NEVER actually used in a real registration, because the true + /// credential ID is taken from the attestation data. #[schema(value_type = String, format = Binary, content_encoding = "base64")] pub raw_id: String, /// . @@ -462,8 +476,9 @@ pub struct RegistrationExtensionsClientOutputs { #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub appid: Option, - /// Indicates if the client believes it created a resident key. This property is managed by the - /// webbrowser, and is NOT SIGNED and CAN NOT be trusted! + /// Indicates if the client believes it created a resident key. This + /// property is managed by the webbrowser, and is NOT SIGNED and CAN NOT + /// be trusted! #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub cred_props: Option, @@ -471,7 +486,8 @@ pub struct RegistrationExtensionsClientOutputs { #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub hmac_secret: Option, - /// Indicates if the client successfully applied a credential protection policy. + /// Indicates if the client successfully applied a credential protection + /// policy. #[schema(nullable = false)] #[serde(skip_serializing_if = "Option::is_none")] pub cred_protect: Option, @@ -484,11 +500,12 @@ pub struct RegistrationExtensionsClientOutputs { /// #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct CredProps { - /// A user agent supplied hint that this credential may have created a resident key. It is - /// returned from the user agent, not the authenticator meaning that this is an unreliable - /// signal. + /// A user agent supplied hint that this credential may have created a + /// resident key. It is returned from the user agent, not the + /// authenticator meaning that this is an unreliable signal. /// - /// Note that this extension is UNSIGNED and may have been altered by page javascript. + /// Note that this extension is UNSIGNED and may have been altered by page + /// javascript. pub rk: bool, } diff --git a/src/assignment/backend.rs b/src/assignment/backend.rs index 7831ef40..17ab171a 100644 --- a/src/assignment/backend.rs +++ b/src/assignment/backend.rs @@ -61,9 +61,10 @@ pub trait AssignmentBackend: DynClone + Send + Sync + std::fmt::Debug { /// List all role assignments for multiple actors on multiple targets /// - /// It is a naive interpretation of the effective role assignments where we check all roles - /// assigned to the user (including groups) on a concrete target (including all higher targets - /// the role can be inherited from) + /// It is a naive interpretation of the effective role assignments where we + /// check all roles assigned to the user (including groups) on a + /// concrete target (including all higher targets the role can be + /// inherited from) async fn list_assignments_for_multiple_actors_and_targets( &self, state: &ServiceState, diff --git a/src/assignment/backend/error.rs b/src/assignment/backend/error.rs index 027537b3..a9dc0707 100644 --- a/src/assignment/backend/error.rs +++ b/src/assignment/backend/error.rs @@ -60,7 +60,8 @@ pub enum AssignmentDatabaseError { }, } -/// Convert the DB error into the [AssignmentDatabaseError] with the context information. +/// Convert the DB error into the [AssignmentDatabaseError] with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> AssignmentDatabaseError { e.sql_err().map_or_else( || AssignmentDatabaseError::Database { diff --git a/src/assignment/backend/sql/assignment/list.rs b/src/assignment/backend/sql/assignment/list.rs index e2e39cbc..5d941fc6 100644 --- a/src/assignment/backend/sql/assignment/list.rs +++ b/src/assignment/backend/sql/assignment/list.rs @@ -130,9 +130,9 @@ pub async fn list( /// Get all role assignments by list of actors on list of targets. /// -/// It is a naive interpretation of the effective role assignments where we check all roles -/// assigned to the user (including groups) on a concrete target (including all higher targets the -/// role can be inherited from) +/// It is a naive interpretation of the effective role assignments where we +/// check all roles assigned to the user (including groups) on a concrete target +/// (including all higher targets the role can be inherited from) pub async fn list_for_multiple_actors_and_targets( _conf: &Config, db: &DatabaseConnection, diff --git a/src/assignment/types/assignment.rs b/src/assignment/types/assignment.rs index 5bf05e1d..2e1c6e1c 100644 --- a/src/assignment/types/assignment.rs +++ b/src/assignment/types/assignment.rs @@ -86,19 +86,21 @@ pub struct RoleAssignmentListParameters { // #[builder(default)] // pub inherited: Option, - /// Query the effective assignments, including any assignments gained by virtue of group - /// membership. + /// Query the effective assignments, including any assignments gained by + /// virtue of group membership. #[builder(default)] pub effective: Option, - /// If set to true, then the names of any entities returned will be include as well as their - /// IDs. Any value other than 0 (including no value) will be interpreted as true. + /// If set to true, then the names of any entities returned will be include + /// as well as their IDs. Any value other than 0 (including no value) + /// will be interpreted as true. #[builder(default)] pub include_names: Option, } -/// Querying effective role assignments for list of actors (typically user with all groups user is -/// member of) on list of targets (exactl project + inherited from uppoer projects/domain). +/// Querying effective role assignments for list of actors (typically user with +/// all groups user is member of) on list of targets (exactl project + inherited +/// from uppoer projects/domain). #[derive(Builder, Clone, Debug, Default, Deserialize, PartialEq, Serialize)] #[builder(setter(strip_option, into))] pub struct RoleAssignmentListForMultipleActorTargetParameters { @@ -115,8 +117,8 @@ pub struct RoleAssignmentListForMultipleActorTargetParameters { pub targets: Vec, } -/// Role assignment target which is either target_id or target_id with explicit inherited -/// parameter. +/// Role assignment target which is either target_id or target_id with explicit +/// inherited parameter. #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] pub struct RoleAssignmentTarget { pub target_id: String, diff --git a/src/auth/mod.rs b/src/auth/mod.rs index 3ccbac66..e65aef56 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -14,15 +14,17 @@ //! Authorization and authentication information. //! -//! Authentication and authorization types with corresponding validation. Authentication specific -//! validation may stay in the corresponding provider (i.e. user password is expired), but general -//! validation rules must be present here to be shared across different authentication methods. The -//! same is valid for the authorization validation (project/domain must exist and be enabled). +//! Authentication and authorization types with corresponding validation. +//! Authentication specific validation may stay in the corresponding provider +//! (i.e. user password is expired), but general validation rules must be +//! present here to be shared across different authentication methods. The +//! same is valid for the authorization validation (project/domain must exist +//! and be enabled). use derive_builder::Builder; use serde::{Deserialize, Serialize}; use thiserror::Error; -use tracing::{error, warn}; +use tracing::warn; use crate::identity::types::{Group, UserResponse}; use crate::resource::types::{Domain, Project}; @@ -44,6 +46,17 @@ pub enum AuthenticationError { #[error("The account is disabled for user: {0}")] UserDisabled(String), + /// User is locked due to the multiple failed attempts. + #[error("The account is temporarily disabled for user: {0}")] + UserLocked(String), + + /// User password is expired. + #[error("The password is expired for user: {0}")] + UserPasswordExpired(String), + + #[error("wrong username or password")] + UserNameOrPasswordWrong, + /// Token renewal is forbidden #[error("Token renewal (getting token from token) is prohibited.")] TokenRenewalForbidden, @@ -100,8 +113,9 @@ impl AuthenticatedInfo { /// - User must be enabled /// - User object id must match user_id pub fn validate(&self) -> Result<(), AuthenticationError> { - // TODO: all validations (disabled user, locked, etc) should be placed here since every - // authentication method goes different way and we risk missing validations + // TODO: all validations (disabled user, locked, etc) should be placed here + // since every authentication method goes different way and we risk + // missing validations if let Some(user) = &self.user { if user.id != self.user_id { warn!( diff --git a/src/bin/keystone.rs b/src/bin/keystone.rs index 065079a4..ffb51670 100644 --- a/src/bin/keystone.rs +++ b/src/bin/keystone.rs @@ -60,8 +60,9 @@ const DEFAULT_BODY_LIMIT: usize = 1024 * 256; /// `OpenStack` Keystone. /// -/// Keystone is an `OpenStack` service that provides API client authentication, service discovery, -/// and distributed multi-tenant authorization by implementing OpenStack’s Identity API. +/// Keystone is an `OpenStack` service that provides API client authentication, +/// service discovery, and distributed multi-tenant authorization by +/// implementing OpenStack’s Identity API. #[derive(Parser, Debug)] #[command(version, about, long_about = None)] struct Args { diff --git a/src/catalog/backends/error.rs b/src/catalog/backends/error.rs index e6f6967d..9ab1f9c4 100644 --- a/src/catalog/backends/error.rs +++ b/src/catalog/backends/error.rs @@ -56,7 +56,8 @@ pub enum CatalogDatabaseError { }, } -/// Convert the DB error into the [CatalogDatabaseError] with the context information. +/// Convert the DB error into the [CatalogDatabaseError] with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> CatalogDatabaseError { e.sql_err().map_or_else( || CatalogDatabaseError::Database { diff --git a/src/catalog/types/endpoint.rs b/src/catalog/types/endpoint.rs index 25546b28..717b8d8e 100644 --- a/src/catalog/types/endpoint.rs +++ b/src/catalog/types/endpoint.rs @@ -21,12 +21,16 @@ use serde_json::Value; pub struct Endpoint { /// The ID of the endpoint. pub id: String, - /// The interface type, which describes the visibility of the endpoint. Value is: - /// - public. Visible by end users on a publicly available network interface. + /// The interface type, which describes the visibility of the endpoint. + /// Value is: + /// - public. Visible by end users on a publicly available network + /// interface. /// - /// - internal. Visible by end users on an unmetered internal network interface. + /// - internal. Visible by end users on an unmetered internal network + /// interface. /// - /// - admin. Visible by administrative users on a secure network interface. + /// - admin. Visible by administrative users on a secure network + /// interface. #[builder(default)] pub interface: String, /// The ID of the region that contains the service endpoint. @@ -36,8 +40,9 @@ pub struct Endpoint { pub service_id: String, /// The endpoint URL. pub url: String, - /// Indicates whether the endpoint appears in the service catalog: - false. The endpoint does - /// not appear in the service catalog. - true. The endpoint appears in the service catalog. + /// Indicates whether the endpoint appears in the service catalog: - false. + /// The endpoint does not appear in the service catalog. - true. The + /// endpoint appears in the service catalog. pub enabled: bool, /// Additional endpoint properties #[builder(default)] diff --git a/src/catalog/types/service.rs b/src/catalog/types/service.rs index 009c9aa8..dc6f9609 100644 --- a/src/catalog/types/service.rs +++ b/src/catalog/types/service.rs @@ -30,9 +30,10 @@ pub struct Service { /// The service name. #[builder(default)] pub name: Option, - /// Defines whether the service and its endpoints appear in the service catalog: - false. The - /// service and its endpoints do not appear in the service catalog. - true. The service and its - /// endpoints appear in the service catalog. + /// Defines whether the service and its endpoints appear in the service + /// catalog: - false. The service and its endpoints do not appear in the + /// service catalog. - true. The service and its endpoints appear in the + /// service catalog. pub enabled: bool, } @@ -41,6 +42,7 @@ pub struct Service { pub struct ServiceListParameters { /// Filters the response by a service name. pub name: Option, - /// Filters the response by a service type. A valid value is compute, ec2, identity, image, network, or volume. + /// Filters the response by a service type. A valid value is compute, ec2, + /// identity, image, network, or volume. pub r#type: Option, } diff --git a/src/config.rs b/src/config.rs index 57d7ffc2..1750dce2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -12,6 +12,7 @@ // // SPDX-License-Identifier: Apache-2.0 +use chrono::TimeDelta; use config::{File, FileFormat}; use eyre::{Report, WrapErr}; use regex::Regex; @@ -219,12 +220,12 @@ impl Default for ResourceSection { /// Revoke provider configuration. #[derive(Debug, Deserialize, Clone)] pub struct RevokeSection { - /// Entry point for the token revocation backend driver in the `keystone.revoke` namespace. - /// Keystone only provides a `sql` driver. + /// Entry point for the token revocation backend driver in the + /// `keystone.revoke` namespace. Keystone only provides a `sql` driver. #[serde(default = "default_sql_driver")] pub driver: String, - /// The number of seconds after a token has expired before a corresponding revocation event may - /// be purged from the backend. + /// The number of seconds after a token has expired before a corresponding + /// revocation event may be purged from the backend. pub expiration_buffer: usize, } @@ -243,12 +244,209 @@ pub enum PasswordHashingAlgo { Bcrypt, } -#[derive(Debug, Default, Deserialize, Clone)] +/// Security compliance configuration. +#[derive(Debug, Deserialize, Clone)] pub struct SecurityComplianceSection { + /// The maximum number of days a user can go without authenticating before + /// being considered "inactive" and automatically disabled (locked). + /// This feature is disabled by default; set any value to enable + /// it. This feature depends on the sql backend for the [identity] driver. + /// When a user exceeds this threshold and is considered "inactive", the + /// user's enabled attribute in the HTTP API may not match the value of + /// the user’s enabled column in the user table. + #[serde(default)] + pub disable_user_account_days_inactive: Option, + /// Enabling this option requires users to change their password when the + /// user is created, or upon administrative reset. Before accessing any + /// services, affected users will have to change their password. To ignore + /// this requirement for specific users, such as service users, set the + /// options attribute ignore_change_password_upon_first_use to True for the + /// desired user via the update user API. This feature is disabled by + /// default. This feature is only applicable with the sql backend for the + /// [identity] driver. + #[serde(default)] + pub change_password_upon_first_use: bool, + /// If report_invalid_password_hash is configured, defines the hash function + /// to be used by HMAC. Possible values are names suitable to hashlib.new() + /// [https://docs.python.org/3/library/hashlib.html#hashlib.new]. + #[serde(default)] + pub invalid_password_hash_function: InvalidPasswordHashMethod, + /// If report_invalid_password_hash is configured, uses provided secret key + /// when generating password hashes to make them unique and distinct from + /// any other Keystone installations out there. Should be some secret static + /// value specific to the current installation (the same value should be + /// used in distributed installations working with the same backend, to make + /// them all generate equal hashes for equal invalid passwords). 16 bytes + /// (128 bits) or more is recommended. + #[serde(default)] + pub invalid_password_hash_key: Option, + /// This option has a sample default set, which means that its actual + /// default value may vary from the one documented above. + /// + /// If report_invalid_password_hash is configured, defines the number of + /// characters of hash of invalid password to be returned. When not + /// specified, returns full hash. Its length depends on implementation and + /// invalid_password_hash_function configuration, but is typically 16+ + /// characters. It’s recommended to use the least reasonable value however - + /// it’s the most effective measure to protect the hashes. + #[serde(default)] + pub invalid_password_hash_max_chars: Option, + + /// The maximum number of times that a user can fail to authenticate before + /// the user account is locked for the number of seconds specified by + /// [security_compliance] lockout_duration. This feature is disabled by + /// default. If this feature is enabled and [security_compliance] + /// lockout_duration is not set, then users may be locked out indefinitely + /// until the user is explicitly enabled via the API. This feature depends + /// on the sql backend for the [identity] driver. + #[serde(default)] + pub lockout_failure_attempts: Option, + /// The number of seconds a user account will be locked when the maximum + /// number of failed authentication attempts (as specified by + /// [security_compliance] lockout_failure_attempts) is exceeded. Setting + /// this option will have no effect unless you also set + /// [security_compliance] lockout_failure_attempts to a non-zero value. This + /// feature depends on the sql backend for the [identity] driver. + //#[serde(default = "AccountLockoutDuration::default")] + #[serde( + deserialize_with = "optional_timedelta_from_seconds", + default = "AccountLockoutDuration::default" + )] + pub lockout_duration: Option, + /// The number of days that a password must be used before the user can + /// change it. This prevents users from changing their passwords immediately + /// in order to wipe out their password history and reuse an old password. + /// This feature does not prevent administrators from manually resetting + /// passwords. It is disabled by default and allows for immediate password + /// changes. This feature depends on the sql backend for the [identity] + /// driver. Note: If [security_compliance] password_expires_days is set, + /// then the value for this option should be less than the + /// password_expires_days. + #[serde(default)] + pub minimum_password_age: u32, + /// The number of days for which a password will be considered valid before + /// requiring it to be changed. This feature is disabled by default. If + /// enabled, new password changes will have an expiration date, + /// however existing passwords would not be impacted. This feature depends + /// on the sql backend for the [identity] driver. + #[serde(default)] pub password_expires_days: Option, - pub disable_user_account_days_inactive: Option, + /// The regular expression used to validate password strength requirements. + /// By default, the regular expression will match any password. The + /// following is an example of a pattern which requires at least 1 letter, 1 + /// digit, and have a minimum length of 7 characters: + /// ^(?=.*\d)(?=.*[a-zA-Z]).{7,}$ This feature depends on the sql backend + /// for the [identity] driver. + #[serde(default)] + pub password_regex: Option, + /// Describe your password regular expression here in language for humans. + /// If a password fails to match the regular expression, the contents of + /// this configuration variable will be returned to users to explain why + /// their requested password was insufficient. + #[serde(default)] + pub password_regex_description: Option, + /// This option has a sample default set, which means that its actual + /// default value may vary from the one documented above. + /// + /// When configured, enriches the corresponding output channel with hash of + /// invalid password, which could be further used to distinguish bruteforce + /// attacks from e.g. external user automations that did not timely update + /// rotated password by analyzing variability of the hash value. Additional + /// configuration parameters are available using other + /// invalid_password_hash_* configuration entries, that only take effect + /// when this option is activated. + #[serde(default = "ReportInvalidPasswordHash::default")] + pub report_invalid_password_hash: Vec, + /// This controls the number of previous user password iterations to keep in + /// history, in order to enforce that newly created passwords are unique. + /// The total number which includes the new password should not be greater + /// or equal to this value. Setting the value to zero (the default) disables + /// this feature. Thus, to enable this feature, values must be greater than + /// 0. This feature depends on the sql backend for the [identity] driver. + #[serde(default)] + pub unique_last_password_count: Option, } +// /// Deserializes an i64 and interprets it as total SECONDS for +// /// chrono::TimeDelta. +// fn timedelta_from_seconds<'de, D>(deserializer: D) -> Result +// where +// D: Deserializer<'de>, +// { +// // Read the input number from JSON as an i64 +// let seconds = i64::deserialize(deserializer)?; +// +// // Convert the number into a TimeDelta representing seconds +// TimeDelta::try_seconds(seconds) +// .ok_or_else(|| serde::de::Error::custom("TimeDelta overflow for seconds")) +// } + +/// Deserializes an Option and interprets Some(i64) as total SECONDS for +/// TimeDelta. +fn optional_timedelta_from_seconds<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + // Deserialize the field content into Option. + // Serde handles 'null' or an absent field by returning None here. + let seconds_opt: Option = Option::deserialize(deserializer)?; + + match seconds_opt { + // If a number was present, convert it to TimeDelta and wrap it in Some. + Some(seconds) => TimeDelta::try_seconds(seconds) + .map(Some) // Map TimeDelta to Some(TimeDelta) + .ok_or_else(|| serde::de::Error::custom("TimeDelta overflow for optional seconds")), + + // If None was present (null or missing field), return Ok(None). + None => Ok(None), + } +} + +impl Default for SecurityComplianceSection { + fn default() -> Self { + Self { + disable_user_account_days_inactive: None, + change_password_upon_first_use: false, + invalid_password_hash_function: InvalidPasswordHashMethod::default(), + invalid_password_hash_key: None, + invalid_password_hash_max_chars: None, + lockout_failure_attempts: None, + lockout_duration: AccountLockoutDuration::default(), + minimum_password_age: 0, + password_expires_days: None, + password_regex: None, + password_regex_description: None, + report_invalid_password_hash: ReportInvalidPasswordHash::default(), + unique_last_password_count: None, + } + } +} + +struct AccountLockoutDuration {} +impl AccountLockoutDuration { + fn default() -> Option { + Some(TimeDelta::seconds(1800)) + } +} + +struct ReportInvalidPasswordHash {} +impl ReportInvalidPasswordHash { + fn default() -> Vec { + vec![InvalidPasswordHashReport::Event] + } +} + +#[derive(Clone, Debug, Default, Deserialize)] +pub enum InvalidPasswordHashReport { + #[default] + Event, +} + +#[derive(Clone, Debug, Default, Deserialize)] +pub enum InvalidPasswordHashMethod { + #[default] + Sha256, +} fn default_sql_driver() -> String { "sql".into() } @@ -269,12 +467,14 @@ fn default_user_options_mapping() -> HashMap { pub struct TokenSection { #[serde(default)] pub provider: TokenProvider, - /// The amount of time that a token should remain valid (in seconds). Drastically reducing this - /// value may break "long-running" operations that involve multiple services to coordinate - /// together, and will force users to authenticate with keystone more frequently. Drastically - /// increasing this value will increase the number of tokens that will be simultaneously valid. - /// Keystone tokens are also bearer tokens, so a shorter duration will also reduce the - /// potential security impact of a compromised token. + /// The amount of time that a token should remain valid (in seconds). + /// Drastically reducing this value may break "long-running" operations + /// that involve multiple services to coordinate together, and will + /// force users to authenticate with keystone more frequently. Drastically + /// increasing this value will increase the number of tokens that will be + /// simultaneously valid. Keystone tokens are also bearer tokens, so a + /// shorter duration will also reduce the potential security impact of a + /// compromised token. #[serde(default)] pub expiration: usize, } diff --git a/src/db/entity.rs b/src/db/entity.rs index b2f7f186..59fb4a94 100644 --- a/src/db/entity.rs +++ b/src/db/entity.rs @@ -100,19 +100,6 @@ impl Default for user::Model { } } -impl Default for local_user::Model { - fn default() -> Self { - Self { - id: 0, - user_id: String::new(), - domain_id: String::new(), - name: String::new(), - failed_auth_at: None, - failed_auth_count: None, - } - } -} - impl Default for service::Model { fn default() -> Self { Self { diff --git a/src/db/entity/password.rs b/src/db/entity/password.rs index a23b2f69..ba7d5209 100644 --- a/src/db/entity/password.rs +++ b/src/db/entity/password.rs @@ -14,19 +14,31 @@ //! `SeaORM` Entity, @generated by sea-orm-codegen 1.1.4 +#[cfg(test)] +use derive_builder::Builder; use sea_orm::entity::prelude::*; #[derive(Clone, Debug, PartialEq, DeriveEntityModel, Eq)] +#[cfg_attr(test, derive(Builder))] +#[cfg_attr(test, builder(setter(strip_option, into)))] #[sea_orm(table_name = "password")] pub struct Model { #[sea_orm(primary_key)] + #[cfg_attr(test, builder(default))] pub id: i32, + #[cfg_attr(test, builder(default))] pub local_user_id: i32, + #[cfg_attr(test, builder(default))] pub self_service: bool, + #[cfg_attr(test, builder(default))] pub created_at: DateTime, + #[cfg_attr(test, builder(default))] pub expires_at: Option, + #[cfg_attr(test, builder(default))] pub password_hash: Option, + #[cfg_attr(test, builder(default))] pub created_at_int: i64, + #[cfg_attr(test, builder(default))] pub expires_at_int: Option, } diff --git a/src/federation/backends/error.rs b/src/federation/backends/error.rs index 70af2f99..0b9b01ea 100644 --- a/src/federation/backends/error.rs +++ b/src/federation/backends/error.rs @@ -67,7 +67,8 @@ pub enum FederationDatabaseError { }, } -/// Convert the DB error into the [FederationDatabaseError] with the context information. +/// Convert the DB error into the [FederationDatabaseError] with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> FederationDatabaseError { e.sql_err().map_or_else( || FederationDatabaseError::Database { diff --git a/src/federation/backends/sql/identity_provider/create.rs b/src/federation/backends/sql/identity_provider/create.rs index 2af0f30f..b0e76bef 100644 --- a/src/federation/backends/sql/identity_provider/create.rs +++ b/src/federation/backends/sql/identity_provider/create.rs @@ -85,8 +85,8 @@ pub async fn create( .await .map_err(|err| db_err(err, "persisting new identity provider"))?; - // For compatibility reasons add entry for the IDP old-style as well as the protocol to keep - // constraints working + // For compatibility reasons add entry for the IDP old-style as well as the + // protocol to keep constraints working db_old_identity_provider::ActiveModel { id: Set(idp.id.clone()), enabled: Set(true), diff --git a/src/federation/types/auth_state.rs b/src/federation/types/auth_state.rs index c0baabfd..3411964e 100644 --- a/src/federation/types/auth_state.rs +++ b/src/federation/types/auth_state.rs @@ -57,13 +57,15 @@ pub struct AuthState { // Domain(String), // System(String), //} -/// The authorization scope, including the system (Since v3.10), a project, or a domain (Since -/// v3.4). If multiple scopes are specified in the same request (e.g. project and domain or domain -/// and system) an HTTP 400 Bad Request will be returned, as a token cannot be simultaneously -/// scoped to multiple authorization targets. An ID is sufficient to uniquely identify a project -/// but if a project is specified by name, then the domain of the project must also be specified in -/// order to uniquely identify the project by name. A domain scope may be specified by either the -/// domain’s ID or name with equivalent results. +/// The authorization scope, including the system (Since v3.10), a project, or a +/// domain (Since v3.4). If multiple scopes are specified in the same request +/// (e.g. project and domain or domain and system) an HTTP 400 Bad Request will +/// be returned, as a token cannot be simultaneously scoped to multiple +/// authorization targets. An ID is sufficient to uniquely identify a project +/// but if a project is specified by name, then the domain of the project must +/// also be specified in order to uniquely identify the project by name. A +/// domain scope may be specified by either the domain’s ID or name with +/// equivalent results. #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] #[serde(rename_all = "lowercase")] pub enum Scope { diff --git a/src/federation/types/identity_provider.rs b/src/federation/types/identity_provider.rs index 20c0f804..47c39c20 100644 --- a/src/federation/types/identity_provider.rs +++ b/src/federation/types/identity_provider.rs @@ -101,7 +101,8 @@ pub struct IdentityProviderUpdate { pub struct IdentityProviderListParameters { /// Filters the response by IDP name. pub name: Option, - /// Filters the response by a domain_id ID. It is an optional list of optional strings to - /// represent fetching of null and non-null values in a single request. + /// Filters the response by a domain_id ID. It is an optional list of + /// optional strings to represent fetching of null and non-null values + /// in a single request. pub domain_ids: Option>>, } diff --git a/src/federation/types/mapping.rs b/src/federation/types/mapping.rs index 9ffddfe1..47b07ad2 100644 --- a/src/federation/types/mapping.rs +++ b/src/federation/types/mapping.rs @@ -74,7 +74,8 @@ pub struct Mapping { //#[builder(default)] //pub claim_mappings: Option, - /// Fixed `project_id` scope of the token to issue for successful authentication. + /// Fixed `project_id` scope of the token to issue for successful + /// authentication. #[builder(default)] pub token_project_id: Option, @@ -134,7 +135,8 @@ pub struct MappingUpdate { #[builder(default)] pub oidc_scopes: Option>>, - /// Fixed `project_id` scope of the token to issue for successful authentication. + /// Fixed `project_id` scope of the token to issue for successful + /// authentication. #[builder(default)] pub token_project_id: Option>, diff --git a/src/identity/backends.rs b/src/identity/backends.rs index e9f433ed..3901c954 100644 --- a/src/identity/backends.rs +++ b/src/identity/backends.rs @@ -35,7 +35,7 @@ pub trait IdentityBackend: DynClone + Send + Sync + std::fmt::Debug { async fn authenticate_by_password( &self, state: &ServiceState, - auth: UserPasswordAuthRequest, + auth: &UserPasswordAuthRequest, ) -> Result; /// List Users. diff --git a/src/identity/backends/error.rs b/src/identity/backends/error.rs index 1c1f6d42..baaffb51 100644 --- a/src/identity/backends/error.rs +++ b/src/identity/backends/error.rs @@ -75,9 +75,29 @@ pub enum IdentityDatabaseError { #[error("either user id or user name with user domain id or name must be given")] UserIdOrNameWithDomain, + + /// No data for local_user and passwords + #[error("no passwords for the user {0}")] + NoPasswordsForUser(String), + + /// Row does not contain password hash. + #[error("no passwords hash on the row id: {0}")] + NoPasswordHash(String), + + /// No entry in the `user` table for the user. + #[error("no entry in the `user` table found for user_id: {0}")] + NoMainUserEntry(String), + + /// Supported authentication error. + #[error(transparent)] + AuthenticationInfo { + #[from] + source: crate::auth::AuthenticationError, + }, } -/// Convert the DB error into the [IdentityDatabaseError] with the context information. +/// Convert the DB error into the [`IdentityDatabaseError`] with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> IdentityDatabaseError { e.sql_err().map_or_else( || IdentityDatabaseError::Database { diff --git a/src/identity/backends/sql.rs b/src/identity/backends/sql.rs index 853375f8..cb0ddd83 100644 --- a/src/identity/backends/sql.rs +++ b/src/identity/backends/sql.rs @@ -53,9 +53,9 @@ impl IdentityBackend for SqlBackend { async fn authenticate_by_password( &self, state: &ServiceState, - auth: UserPasswordAuthRequest, + auth: &UserPasswordAuthRequest, ) -> Result { - authenticate::authenticate_by_password(&self.config, &state.db, auth).await + Ok(authenticate::authenticate_by_password(&self.config, &state.db, auth).await?) } /// Fetch users from the database diff --git a/src/identity/backends/sql/authenticate.rs b/src/identity/backends/sql/authenticate.rs index 6a1be549..222a6b62 100644 --- a/src/identity/backends/sql/authenticate.rs +++ b/src/identity/backends/sql/authenticate.rs @@ -11,62 +11,585 @@ // limitations under the License. // // SPDX-License-Identifier: Apache-2.0 - +//! Authentication implementation. +use chrono::Utc; use sea_orm::DatabaseConnection; +use tracing::info; use super::local_user; use super::user; use super::user_option; use crate::auth::{AuthenticatedInfo, AuthenticationError}; use crate::config::Config; -use crate::db::entity::password as db_password; -use crate::identity::IdentityProviderError; +use crate::db::entity::{local_user as db_local_user, password as db_password}; +use crate::identity::backends::error::IdentityDatabaseError; +use crate::identity::backends::sql::password; use crate::identity::password_hashing; use crate::identity::types::*; -/// Authenticate a user by a password +/// Authenticate a user by a password. +/// +/// Verify whether the passed password matches the one recorded in the database and that the user +/// is allowed to login (i.e. not locked). +/// +/// - Reads local user database entry with passwords sorted by the creation date (desc). +/// - Reads user options if the user has been found. +/// - Checks whether the user is locked due to the amount of failed attempts (PCI-DSS). +/// - Verifies the password matches the most recent created hash. +/// - Verifies the password is not expired. +/// - Reads main user database entry. +/// - Converts all responses into the [`UserResponse`] structure. pub async fn authenticate_by_password( config: &Config, db: &DatabaseConnection, - auth: UserPasswordAuthRequest, -) -> Result { + auth: &UserPasswordAuthRequest, +) -> Result { let user_with_passwords = local_user::load_local_user_with_passwords( db, - auth.id, - auth.name, - auth.domain.and_then(|x| x.id), + auth.id.as_ref(), + auth.name.as_ref(), + auth.domain.as_ref().and_then(|x| x.id.as_ref()), ) .await?; - if let Some((local_user, password)) = user_with_passwords { - let passwords: Vec = password.into_iter().collect(); - if let Some(latest_password) = passwords.first() - && let Some(expected_hash) = &latest_password.password_hash - { - let user_opts = user_option::list_by_user_id(db, local_user.user_id.clone()).await?; - - if password_hashing::verify_password(config, auth.password, expected_hash).await? { - if let Some(user) = user::get_main_entry(db, &local_user.user_id).await? { - // TODO: Check password is expired - // TODO: reset failed login attempt - let user_builder = local_user::get_local_user_builder( - config, - &user, - local_user, - Some(passwords), - user_opts, - ); - let user = user_builder.build()?; - return Ok(AuthenticatedInfo::builder() - .user_id(user.id.clone()) - .user(user) - .methods(vec!["password".into()]) - .build() - .map_err(AuthenticationError::from)?); + + let (local_user, password) = + user_with_passwords.ok_or(AuthenticationError::UserNameOrPasswordWrong)?; + // User has been found. + // Get user options + let user_opts = user_option::list_by_user_id(db, local_user.user_id.clone()).await?; + + // Check for the temporary lock + if !user_opts + .ignore_lockout_failure_attempts + .is_some_and(|val| val) + && is_account_locked(config, db, &local_user).await? + { + return Err(AuthenticationError::UserLocked(local_user.user_id.clone()))?; + } + + let passwords: Vec = password.into_iter().collect(); + let latest_password = passwords + .first() + .ok_or(IdentityDatabaseError::NoPasswordsForUser( + local_user.user_id.clone(), + ))?; + let expected_hash = + latest_password + .password_hash + .as_ref() + .ok_or(IdentityDatabaseError::NoPasswordHash( + latest_password.id.clone().to_string(), + ))?; + + // Verify the password + if !password_hashing::verify_password(config, &auth.password, expected_hash).await? { + return Err(AuthenticationError::UserNameOrPasswordWrong)?; + } + // Check if expired password exempt is on + if !user_opts.ignore_password_expiry.is_some_and(|val| val) { + // otherwise check for expired password + if password::is_password_expired(latest_password)? { + return Err(AuthenticationError::UserPasswordExpired( + local_user.user_id.clone(), + ))?; + } + } + + let user = user::get_main_entry(db, &local_user.user_id).await?.ok_or( + IdentityDatabaseError::NoMainUserEntry(local_user.user_id.clone()), + )?; + let user = + local_user::get_local_user_builder(config, &user, local_user, Some(passwords), user_opts) + .build()?; + Ok(AuthenticatedInfo::builder() + .user_id(user.id.clone()) + .user(user) + .methods(vec!["password".into()]) + .build() + .map_err(AuthenticationError::from)?) +} + +/// Verify whether the account is temporarily locked according to the security +/// compliance requirements. +/// +/// Checks whether the account is locked temporarily due to the failed login +/// attempts as described by +/// [ADR-10](https://openstack-experimental.github.io/keystone/adr/0010-pci-dss-failed-auth-protection.html) +#[tracing::instrument(level = "debug", skip(config, db))] +async fn is_account_locked( + config: &Config, + db: &DatabaseConnection, + local_user: &db_local_user::Model, +) -> Result { + if let Some(lockout_failure_attempts) = config.security_compliance.lockout_failure_attempts + && let Some(attempts) = local_user.failed_auth_count + && attempts >= lockout_failure_attempts.into() + { + if let Some(lockout_duration) = config.security_compliance.lockout_duration { + if let Some(locked_till) = local_user + .failed_auth_at + .and_then(|last_failure| last_failure.checked_add_signed(lockout_duration)) + { + // last_failure is recorded + if locked_till > Utc::now().naive_utc() { + // Lock is still active + return Ok(true); } - } else { - return Err(IdentityProviderError::WrongUsernamePassword); + } + // Either last failed_auth_at is missing or expired - reset. + local_user::reset_failed_auth(db, local_user).await?; + } else if !config + .security_compliance + .lockout_duration + .is_some_and(|val| val.is_zero()) + { + info!( + "[security_compliance].lockout_duration is unset. The user is permanently locked out." + ); + return Ok(true); + } + } + Ok(false) +} + +#[cfg(test)] +mod tests { + use chrono::{DateTime, NaiveDateTime, TimeDelta, Utc}; + use sea_orm::{DatabaseBackend, MockDatabase, Transaction}; + use tracing_test::traced_test; + + use crate::identity::types::user::*; + use crate::{db::entity::local_user as db_local_user, identity::types::UserOptions}; + + use super::*; + + #[tokio::test] + async fn test_is_account_locked_default_config() { + let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection(); + let config = Config::default(); + assert!( + !is_account_locked(&config, &db, &db_local_user::Model::default(),) + .await + .unwrap(), + "Default config does not request any validation and user is not considered locked" + ); + } + + #[tokio::test] + async fn test_is_account_locked_no_failed_auth_count() { + let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection(); + let mut config = Config::default(); + config.security_compliance.lockout_failure_attempts = Some(5); + assert!( + !is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: None, + failed_auth_at: None, + ..Default::default() + }, + ) + .await + .unwrap(), + "User with unset failed_auth props is not considered locked" + ); + assert!( + !is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: None, + failed_auth_at: Some(Utc::now().naive_utc()), + ..Default::default() + }, + ) + .await + .unwrap(), + "User with unset failed_auth_count props is not considered locked" + ); + } + + #[tokio::test] + async fn test_is_account_locked_no_failed_auth_at() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![db_local_user::Model::default()]]) + .into_connection(); + let mut config = Config::default(); + config.security_compliance.lockout_failure_attempts = Some(5); + assert!( + !is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: Some(10), + failed_auth_at: None, + ..Default::default() + }, + ) + .await + .unwrap(), + "User with unset failed_auth_at props is not considered locked and auth reset" + ); + + // Checking transaction log + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"UPDATE "local_user" SET "failed_auth_count" = $1, "failed_auth_at" = $2 WHERE "local_user"."id" = $3 RETURNING "id", "user_id", "domain_id", "name", "failed_auth_count", "failed_auth_at""#, + [ + None::.into(), + None::.into(), + 1i32.into() + ] + ),] + ); + } + + #[tokio::test] + async fn test_is_account_locked_expired() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![db_local_user::Model::default()]]) + .into_connection(); + let mut config = Config::default(); + config.security_compliance.lockout_failure_attempts = Some(5); + config.security_compliance.lockout_duration = Some(TimeDelta::seconds(100)); + assert!( + !is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: Some(10), + failed_auth_at: Some( + Utc::now() + .checked_sub_signed(TimeDelta::seconds(101)) + .unwrap() + .naive_utc() + ), + ..Default::default() + }, + ) + .await + .unwrap(), + "User with unset expired protection is unlocked" + ); + + // Checking transaction log + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"UPDATE "local_user" SET "failed_auth_count" = $1, "failed_auth_at" = $2 WHERE "local_user"."id" = $3 RETURNING "id", "user_id", "domain_id", "name", "failed_auth_count", "failed_auth_at""#, + [ + None::.into(), + None::.into(), + 1i32.into() + ] + ),] + ); + } + + #[tokio::test] + async fn test_is_account_locked_lock() { + let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection(); + let mut config = Config::default(); + config.security_compliance.lockout_failure_attempts = Some(5); + assert!( + is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: Some(10), + ..Default::default() + }, + ) + .await + .unwrap(), + "User with failed_auth_count > lockout_failure_attempts is locked for lockout_duration", + ); + assert!( + is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: Some(5), + ..Default::default() + }, + ) + .await + .unwrap(), + "User with failed_auth_count = lockout_failure_attempts is locked for lockout_duration", + ); + assert!( + !is_account_locked( + &config, + &db, + &db_local_user::Model { + failed_auth_count: Some(4), + ..Default::default() + }, + ) + .await + .unwrap(), + "User with failed_auth_count < lockout_failure_attempts is locked for lockout_duration", + ); + } + + fn get_local_user_with_password_mock( + password_hash: String, + ) -> (db_local_user::Model, db_password::Model) { + ( + db_local_user::Model::default(), + db_password::ModelBuilder::default() + .password_hash(password_hash) + .build() + .unwrap(), + ) + } + + #[tokio::test] + async fn test_authenticate() { + let config = Config::default(); + let password = String::from("pass"); + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![get_local_user_with_password_mock( + password_hashing::hash_password(&config, &password) + .await + .unwrap(), + )]]) + .append_query_results([user_option::tests::get_user_options_mock( + &UserOptions::default(), + )]) + .append_query_results([vec![user::tests::get_user_mock("1")]]) + .into_connection(); + assert!( + authenticate_by_password( + &config, + &db, + &UserPasswordAuthRequest { + id: Some("user_id".into()), + password, + ..Default::default() + }, + ) + .await + .is_ok(), + "unlocked user with correct password should be allowed to login" + ); + } + + #[tokio::test] + async fn test_authenticate_locked_user() { + let mut config = Config::default(); + config.security_compliance.lockout_failure_attempts = Some(5); + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![( + db_local_user::Model { + id: 1, + user_id: "user_id".into(), + domain_id: "foo_domain".into(), + name: "foo_domain".into(), + failed_auth_count: Some(10), + failed_auth_at: Some(Utc::now().naive_utc()), + }, + db_password::ModelBuilder::default() + .local_user_id(1) + .build() + .unwrap(), + )]]) + .append_query_results([user_option::tests::get_user_options_mock( + &UserOptions::default(), + )]) + .into_connection(); + match authenticate_by_password( + &config, + &db, + &UserPasswordAuthRequest { + id: Some("user_id".into()), + password: "password".into(), + ..Default::default() + }, + ) + .await + { + Err(IdentityDatabaseError::AuthenticationInfo { + source: AuthenticationError::UserLocked(user_id), + }) => { + assert_eq!(user_id, "user_id"); + } + other => { + panic!("Locked user should be refused even before checking password: {other:?}",); } } } - Err(IdentityProviderError::WrongUsernamePassword) + + #[tokio::test] + async fn test_authenticate_locked_user_exempt() { + let mut config = Config::default(); + let password = "foo_pass"; + config.security_compliance.lockout_failure_attempts = Some(5); + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![( + db_local_user::Model { + id: 1, + user_id: "user_id".into(), + domain_id: "foo_domain".into(), + name: "foo_domain".into(), + failed_auth_count: Some(10), + failed_auth_at: Some(Utc::now().naive_utc()), + }, + db_password::ModelBuilder::default() + .local_user_id(1) + .password_hash( + password_hashing::hash_password(&config, &password) + .await + .unwrap(), + ) + .build() + .unwrap(), + )]]) + .append_query_results([user_option::tests::get_user_options_mock(&UserOptions { + ignore_lockout_failure_attempts: Some(true), + ..Default::default() + })]) + .append_query_results([vec![user::tests::get_user_mock("1")]]) + .into_connection(); + assert!( + authenticate_by_password( + &config, + &db, + &UserPasswordAuthRequest { + id: Some("user_id".into()), + password: password.into(), + ..Default::default() + }, + ) + .await + .is_ok(), + "User that should be locked is still allowed due to the exempt" + ); + } + + #[tokio::test] + #[traced_test] + async fn test_authenticate_wrong_password() { + let config = Config::default(); + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![( + db_local_user::Model::default(), + db_password::ModelBuilder::default() + .password_hash("wrong_password") + .build() + .unwrap(), + )]]) + .append_query_results([user_option::tests::get_user_options_mock( + &UserOptions::default(), + )]) + .into_connection(); + match authenticate_by_password( + &config, + &db, + &UserPasswordAuthRequest { + id: Some("user_id".into()), + password: "foo_pass".into(), + ..Default::default() + }, + ) + .await + { + Err(IdentityDatabaseError::AuthenticationInfo { + source: AuthenticationError::UserNameOrPasswordWrong, + }) => {} + other => { + panic!("User with wrong password should be refused: {other:?}"); + } + } + assert!(!logs_contain("foo_pass")); + } + + #[tokio::test] + #[traced_test] + async fn test_authenticate_expired_password() { + let config = Config::default(); + let password = String::from("foo_pass"); + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![( + db_local_user::Model::default(), + db_password::ModelBuilder::default() + .password_hash( + password_hashing::hash_password(&config, &password) + .await + .unwrap(), + ) + .expires(DateTime::::MIN_UTC) + .build() + .unwrap(), + )]]) + .append_query_results([user_option::tests::get_user_options_mock( + &UserOptions::default(), + )]) + .into_connection(); + match authenticate_by_password( + &config, + &db, + &UserPasswordAuthRequest { + id: Some("user_id".into()), + password: password.clone(), + ..Default::default() + }, + ) + .await + { + Err(IdentityDatabaseError::AuthenticationInfo { + source: AuthenticationError::UserPasswordExpired(..), + }) => {} + other => { + panic!("User with expired valid password should be refused: {other:?}"); + } + } + + assert!(!logs_contain(&password)); + } + + #[tokio::test] + #[traced_test] + async fn test_authenticate_exempt_expired_password() { + let config = Config::default(); + let password = String::from("foo_pass"); + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![( + db_local_user::Model::default(), + db_password::ModelBuilder::expired() + .password_hash( + password_hashing::hash_password(&config, &password) + .await + .unwrap(), + ) + .build() + .unwrap(), + )]]) + .append_query_results([user_option::tests::get_user_options_mock(&UserOptions { + ignore_password_expiry: Some(true), + ..Default::default() + })]) + .append_query_results([vec![user::tests::get_user_mock("1")]]) + .into_connection(); + assert!( + authenticate_by_password( + &config, + &db, + &UserPasswordAuthRequest { + id: Some("user_id".into()), + password: password.clone(), + ..Default::default() + }, + ) + .await + .is_ok(), + "User with expired password and expiration exempt should be allowed" + ); + + assert!(!logs_contain(&password)); + } } diff --git a/src/identity/backends/sql/federated_user.rs b/src/identity/backends/sql/federated_user.rs index aa1a8bfd..240d2192 100644 --- a/src/identity/backends/sql/federated_user.rs +++ b/src/identity/backends/sql/federated_user.rs @@ -13,9 +13,7 @@ // SPDX-License-Identifier: Apache-2.0 use super::user; -use crate::db::entity::{ - federated_user as db_federated_user, user as db_user, user_option as db_user_option, -}; +use crate::db::entity::{federated_user as db_federated_user, user as db_user}; use crate::identity::types::*; mod create; @@ -24,13 +22,10 @@ mod find; pub use create::create; pub use find::find_by_idp_and_unique_id; -pub fn get_federated_user_builder< - O: IntoIterator, - F: IntoIterator, ->( +pub fn get_federated_user_builder>( user: &db_user::Model, data: F, - opts: O, + opts: UserOptions, ) -> UserResponseBuilder { let mut user_builder: UserResponseBuilder = user::get_user_builder(user, opts); let mut feds: Vec = Vec::new(); diff --git a/src/identity/backends/sql/group.rs b/src/identity/backends/sql/group.rs index 9d96abc0..ac58171d 100644 --- a/src/identity/backends/sql/group.rs +++ b/src/identity/backends/sql/group.rs @@ -41,11 +41,11 @@ impl From for Group { } #[cfg(test)] -mod tests { +pub(super) mod tests { #![allow(clippy::derivable_impls)] use super::*; - pub(super) fn get_group_mock>(id: S) -> group::Model { + pub fn get_group_mock>(id: S) -> group::Model { group::Model { id: id.as_ref().to_string(), domain_id: "foo_domain".into(), diff --git a/src/identity/backends/sql/local_user.rs b/src/identity/backends/sql/local_user.rs index f6ca6bd6..ac865c12 100644 --- a/src/identity/backends/sql/local_user.rs +++ b/src/identity/backends/sql/local_user.rs @@ -16,29 +16,25 @@ use chrono::{DateTime, Days, Utc}; use super::user; use crate::config::Config; -use crate::db::entity::{ - local_user as db_local_user, password as db_password, user as db_user, - user_option as db_user_option, -}; +use crate::db::entity::{local_user as db_local_user, password as db_password, user as db_user}; use crate::identity::types::*; mod create; mod get; mod load; +mod set; pub use create::create; pub use load::load_local_user_with_passwords; pub use load::load_local_users_passwords; +pub use set::reset_failed_auth; -pub fn get_local_user_builder< - O: IntoIterator, - P: IntoIterator, ->( +pub fn get_local_user_builder>( conf: &Config, user: &db_user::Model, data: db_local_user::Model, passwords: Option

, - opts: O, + opts: UserOptions, ) -> UserResponseBuilder { let mut user_builder: UserResponseBuilder = user::get_user_builder(user, opts); user_builder.name(data.name.clone()); @@ -58,3 +54,52 @@ pub fn get_local_user_builder< } user_builder } + +#[cfg(test)] +pub(super) mod tests { + use chrono::{Local, Utc}; + + use crate::db::entity::{local_user as db_local_user, password as db_password}; + + impl Default for db_local_user::Model { + fn default() -> Self { + Self { + id: 1, + user_id: "user_id".into(), + domain_id: "foo_domain".into(), + name: "foo_domain".into(), + failed_auth_count: Some(0), + failed_auth_at: Some(Utc::now().naive_utc()), + } + } + } + + pub fn get_local_user_with_password_mock>( + user_id: U, + cnt_password: usize, + ) -> Vec<(db_local_user::Model, db_password::Model)> { + let lu = db_local_user::Model { + user_id: user_id.as_ref().into(), + domain_id: "foo_domain".into(), + name: "Apple Cake".to_owned(), + ..Default::default() + }; + let mut passwords: Vec = Vec::new(); + for i in 0..cnt_password { + passwords.push(db_password::Model { + id: i as i32, + local_user_id: 1, + expires_at: None, + self_service: false, + password_hash: None, + created_at: Local::now().naive_utc(), + created_at_int: 12345, + expires_at_int: None, + }); + } + passwords + .into_iter() + .map(|x| (lu.clone(), x.clone())) + .collect() + } +} diff --git a/src/identity/backends/sql/local_user/load.rs b/src/identity/backends/sql/local_user/load.rs index 6eb4c6e6..181fcb73 100644 --- a/src/identity/backends/sql/local_user/load.rs +++ b/src/identity/backends/sql/local_user/load.rs @@ -60,8 +60,8 @@ pub async fn load_local_user_with_passwords, S2: AsRef, S3: /// Fetch passwords for list of optional local user ids /// -/// Returns vector of optional vectors with passwords in the same order as requested -/// keeping None in place where local_user was empty. +/// Returns vector of optional vectors with passwords in the same order as +/// requested keeping None in place where local_user was empty. pub async fn load_local_users_passwords>>( db: &DatabaseConnection, user_ids: L, @@ -102,3 +102,92 @@ pub async fn load_local_users_passwords>>( Ok(result) } + +#[cfg(test)] +mod tests { + use sea_orm::{DatabaseBackend, MockDatabase, Transaction}; + + use super::super::tests::*; + use super::*; + + #[tokio::test] + async fn test_load_local_user_with_passwords_user_id() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([get_local_user_with_password_mock("user_id", 5)]) + .into_connection(); + load_local_user_with_passwords(&db, Some("user_id"), None::, None::) + .await + .unwrap(); + + // Checking transaction log + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "local_user"."id" AS "A_id", "local_user"."user_id" AS "A_user_id", "local_user"."domain_id" AS "A_domain_id", "local_user"."name" AS "A_name", "local_user"."failed_auth_count" AS "A_failed_auth_count", "local_user"."failed_auth_at" AS "A_failed_auth_at", "password"."id" AS "B_id", "password"."local_user_id" AS "B_local_user_id", "password"."self_service" AS "B_self_service", "password"."created_at" AS "B_created_at", "password"."expires_at" AS "B_expires_at", "password"."password_hash" AS "B_password_hash", "password"."created_at_int" AS "B_created_at_int", "password"."expires_at_int" AS "B_expires_at_int" FROM "local_user" LEFT JOIN "password" ON "local_user"."id" = "password"."local_user_id" WHERE "local_user"."user_id" = $1 ORDER BY "local_user"."id" ASC, "password"."created_at_int" DESC"#, + ["user_id".into(),] + ),] + ); + } + + #[tokio::test] + async fn test_load_local_user_with_passwords_user_name_domain_ignored_for_user_id() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([get_local_user_with_password_mock("user_id", 5)]) + .into_connection(); + load_local_user_with_passwords(&db, Some("user_id"), Some("foo"), Some("bar")) + .await + .unwrap(); + + // Checking transaction log + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "local_user"."id" AS "A_id", "local_user"."user_id" AS "A_user_id", "local_user"."domain_id" AS "A_domain_id", "local_user"."name" AS "A_name", "local_user"."failed_auth_count" AS "A_failed_auth_count", "local_user"."failed_auth_at" AS "A_failed_auth_at", "password"."id" AS "B_id", "password"."local_user_id" AS "B_local_user_id", "password"."self_service" AS "B_self_service", "password"."created_at" AS "B_created_at", "password"."expires_at" AS "B_expires_at", "password"."password_hash" AS "B_password_hash", "password"."created_at_int" AS "B_created_at_int", "password"."expires_at_int" AS "B_expires_at_int" FROM "local_user" LEFT JOIN "password" ON "local_user"."id" = "password"."local_user_id" WHERE "local_user"."user_id" = $1 ORDER BY "local_user"."id" ASC, "password"."created_at_int" DESC"#, + ["user_id".into(),] + ),] + ); + } + + #[tokio::test] + async fn test_load_local_user_with_passwords_no_user_name_and_domain() { + let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection(); + match load_local_user_with_passwords(&db, None::, Some("user_name"), None::) + .await + { + Err(IdentityDatabaseError::UserIdOrNameWithDomain) => {} + _ => { + panic!("User name without ID should be rejected") + } + }; + match load_local_user_with_passwords(&db, None::, None::, Some("domain_id")) + .await + { + Err(IdentityDatabaseError::UserIdOrNameWithDomain) => {} + _ => { + panic!("Domain ID without user name should be rejected") + } + }; + } + + #[tokio::test] + async fn test_load_local_user_with_passwords_user_name_domain() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([get_local_user_with_password_mock("user_id", 5)]) + .into_connection(); + load_local_user_with_passwords(&db, None::, Some("foo"), Some("bar")) + .await + .unwrap(); + + // Checking transaction log + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "local_user"."id" AS "A_id", "local_user"."user_id" AS "A_user_id", "local_user"."domain_id" AS "A_domain_id", "local_user"."name" AS "A_name", "local_user"."failed_auth_count" AS "A_failed_auth_count", "local_user"."failed_auth_at" AS "A_failed_auth_at", "password"."id" AS "B_id", "password"."local_user_id" AS "B_local_user_id", "password"."self_service" AS "B_self_service", "password"."created_at" AS "B_created_at", "password"."expires_at" AS "B_expires_at", "password"."password_hash" AS "B_password_hash", "password"."created_at_int" AS "B_created_at_int", "password"."expires_at_int" AS "B_expires_at_int" FROM "local_user" LEFT JOIN "password" ON "local_user"."id" = "password"."local_user_id" WHERE "local_user"."name" = $1 AND "local_user"."domain_id" = $2 ORDER BY "local_user"."id" ASC, "password"."created_at_int" DESC"#, + ["foo".into(), "bar".into()] + ),] + ); + } +} diff --git a/src/identity/backends/sql/local_user/set.rs b/src/identity/backends/sql/local_user/set.rs new file mode 100644 index 00000000..514aa69f --- /dev/null +++ b/src/identity/backends/sql/local_user/set.rs @@ -0,0 +1,32 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use sea_orm::DatabaseConnection; +use sea_orm::entity::*; + +use crate::db::entity::local_user as db_local_user; +use crate::identity::backends::sql::{IdentityDatabaseError, db_err}; + +pub async fn reset_failed_auth( + db: &DatabaseConnection, + user: &db_local_user::Model, +) -> Result { + let mut update: db_local_user::ActiveModel = user.clone().into(); + update.failed_auth_count = Set(None); + update.failed_auth_at = Set(None); + update + .update(db) + .await + .map_err(|err| db_err(err, "resetting local user failed auth counters")) +} diff --git a/src/identity/backends/sql/nonlocal_user.rs b/src/identity/backends/sql/nonlocal_user.rs index 3094241a..9c382ec9 100644 --- a/src/identity/backends/sql/nonlocal_user.rs +++ b/src/identity/backends/sql/nonlocal_user.rs @@ -13,15 +13,13 @@ // SPDX-License-Identifier: Apache-2.0 use super::user; -use crate::db::entity::{ - nonlocal_user as db_nonlocal_user, user as db_user, user_option as db_user_option, -}; +use crate::db::entity::{nonlocal_user as db_nonlocal_user, user as db_user}; use crate::identity::types::*; -pub fn get_nonlocal_user_builder>( +pub fn get_nonlocal_user_builder( user: &db_user::Model, data: db_nonlocal_user::Model, - opts: O, + opts: UserOptions, ) -> UserResponseBuilder { let mut user_builder: UserResponseBuilder = user::get_user_builder(user, opts); user_builder.name(data.name.clone()); diff --git a/src/identity/backends/sql/password.rs b/src/identity/backends/sql/password.rs index 688839e4..8a0f979c 100644 --- a/src/identity/backends/sql/password.rs +++ b/src/identity/backends/sql/password.rs @@ -11,6 +11,137 @@ // limitations under the License. // // SPDX-License-Identifier: Apache-2.0 +use crate::db::entity::password as db_password; +use crate::identity::backends::error::IdentityDatabaseError; +use chrono::{DateTime, Utc}; + mod create; pub use create::create; + +/// Verify whether the password is expired or not. +pub(super) fn is_password_expired( + password_entry: &db_password::Model, +) -> Result { + //if let Some(expires_et) + if let Some(expires) = password_entry + .expires_at_int + .and_then(DateTime::from_timestamp_secs) + .or_else(|| password_entry.expires_at.map(|val| val.and_utc())) + { + return Ok(expires <= Utc::now()); + } + Ok(false) +} + +#[cfg(test)] +pub(super) mod tests { + use crate::db::entity::password as db_password; + use chrono::{DateTime, TimeDelta, Utc}; + + use super::*; + + impl db_password::ModelBuilder { + pub fn expires(&mut self, value: DateTime) -> &mut Self { + self.expires_at_int(value.timestamp()) + .expires_at(value.naive_utc()) + } + + pub fn expired() -> Self { + Self::default() + .expires_at(DateTime::::MIN_UTC.naive_utc()) + .expires_at_int(DateTime::::MIN_UTC.timestamp()) + .to_owned() + } + + pub fn not_expired() -> Self { + Self::default() + .expires_at(DateTime::::MAX_UTC.naive_utc()) + .expires_at_int(DateTime::::MAX_UTC.timestamp()) + .to_owned() + } + + pub fn dummy() -> db_password::Model { + db_password::ModelBuilder::default().build().unwrap() + } + } + + #[test] + fn test_is_password_expired_not() { + let now = Utc::now(); + assert!( + !is_password_expired(&db_password::ModelBuilder::default().build().unwrap()).unwrap(), + "password with no expiration is not expired" + ); + assert!( + !is_password_expired( + &db_password::ModelBuilder::default() + .expires_at_int( + now.checked_add_signed(TimeDelta::seconds(5)) + .unwrap() + .timestamp() + ) + .build() + .unwrap() + ) + .unwrap(), + "password with expires_at_int in the future is not expired" + ); + assert!( + !is_password_expired( + &db_password::ModelBuilder::default() + .expires_at( + now.checked_add_signed(TimeDelta::seconds(5)) + .unwrap() + .naive_utc() + ) + .build() + .unwrap() + ) + .unwrap(), + "password with expires_at in the future is not expired" + ); + assert!( + !is_password_expired(&db_password::ModelBuilder::not_expired().build().unwrap()) + .unwrap(), + "password is not expired" + ); + } + + #[test] + fn test_is_password_expired_true() { + let now = Utc::now(); + assert!( + is_password_expired( + &db_password::ModelBuilder::default() + .expires_at_int( + now.checked_sub_signed(TimeDelta::seconds(1)) + .unwrap() + .timestamp() + ) + .build() + .unwrap() + ) + .unwrap(), + "password with expires_at_int in the past is expired" + ); + assert!( + is_password_expired( + &db_password::ModelBuilder::default() + .expires_at( + now.checked_sub_signed(TimeDelta::seconds(5)) + .unwrap() + .naive_utc() + ) + .build() + .unwrap() + ) + .unwrap(), + "password with expires_at in the past is expired" + ); + assert!( + is_password_expired(&db_password::ModelBuilder::expired().build().unwrap()).unwrap(), + "password is expired" + ); + } +} diff --git a/src/identity/backends/sql/user.rs b/src/identity/backends/sql/user.rs index dd4e7b86..ce62d560 100644 --- a/src/identity/backends/sql/user.rs +++ b/src/identity/backends/sql/user.rs @@ -15,7 +15,7 @@ use serde_json::Value; use tracing::error; -use crate::db::entity::{user as db_user, user_option as db_user_option}; +use crate::db::entity::user as db_user; use crate::identity::types::*; mod create; @@ -29,10 +29,7 @@ pub use get::get; pub(super) use get::get_main_entry; pub use list::list; -pub fn get_user_builder>( - user: &db_user::Model, - opts: O, -) -> UserResponseBuilder { +pub fn get_user_builder(user: &db_user::Model, opts: UserOptions) -> UserResponseBuilder { let mut user_builder: UserResponseBuilder = UserResponseBuilder::default(); user_builder.id(user.id.clone()); user_builder.domain_id(user.domain_id.clone()); @@ -46,20 +43,16 @@ pub fn get_user_builder>( ); } - user_builder.options(UserOptions::from_iter(opts)); + user_builder.options(opts); user_builder } #[cfg(test)] -mod tests { - use chrono::Local; +pub(super) mod tests { + use crate::db::entity::user as db_user; - use crate::db::entity::{ - local_user as db_local_user, password as db_password, user as db_user, - }; - - pub(super) fn get_user_mock>(user_id: U) -> db_user::Model { + pub fn get_user_mock>(user_id: U) -> db_user::Model { db_user::Model { id: user_id.as_ref().into(), domain_id: "foo_domain".into(), @@ -67,33 +60,4 @@ mod tests { ..Default::default() } } - - pub(super) fn get_local_user_with_password_mock>( - user_id: U, - cnt_password: usize, - ) -> Vec<(db_local_user::Model, db_password::Model)> { - let lu = db_local_user::Model { - user_id: user_id.as_ref().into(), - domain_id: "foo_domain".into(), - name: "Apple Cake".to_owned(), - ..Default::default() - }; - let mut passwords: Vec = Vec::new(); - for i in 0..cnt_password { - passwords.push(db_password::Model { - id: i as i32, - local_user_id: 1, - expires_at: None, - self_service: false, - password_hash: None, - created_at: Local::now().naive_utc(), - created_at_int: 12345, - expires_at_int: None, - }); - } - passwords - .into_iter() - .map(|x| (lu.clone(), x.clone())) - .collect() - } } diff --git a/src/identity/backends/sql/user/create.rs b/src/identity/backends/sql/user/create.rs index 6215af50..8907d7f5 100644 --- a/src/identity/backends/sql/user/create.rs +++ b/src/identity/backends/sql/user/create.rs @@ -115,8 +115,11 @@ pub async fn create( } } - let builder = - federated_user::get_federated_user_builder(&main_user, federated_entities, Vec::new()); + let builder = federated_user::get_federated_user_builder( + &main_user, + federated_entities, + UserOptions::default(), + ); Ok(builder.build()?) } else { @@ -139,7 +142,7 @@ pub async fn create( &main_user, local_user, Some(passwords), - Vec::new(), + UserOptions::default(), ) .build()?) } @@ -150,5 +153,6 @@ pub async fn create( #[cfg(test)] mod tests { - // use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult, Transaction}; + // use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult, + // Transaction}; } diff --git a/src/identity/backends/sql/user/get.rs b/src/identity/backends/sql/user/get.rs index f2e65219..e362877a 100644 --- a/src/identity/backends/sql/user/get.rs +++ b/src/identity/backends/sql/user/get.rs @@ -65,7 +65,9 @@ pub async fn get( &user, local_user_with_passwords.0, Some(local_user_with_passwords.1), - user_opts.map_err(|err| db_err(err, "fetching user options"))?, + UserOptions::from_iter( + user_opts.map_err(|err| db_err(err, "fetching user options"))?, + ), ), _ => match user .find_related(NonlocalUser) @@ -76,7 +78,9 @@ pub async fn get( Some(nonlocal_user) => nonlocal_user::get_nonlocal_user_builder( &user, nonlocal_user, - user_opts.map_err(|err| db_err(err, "fetching user options"))?, + UserOptions::from_iter( + user_opts.map_err(|err| db_err(err, "fetching user options"))?, + ), ), _ => { let federated_user = user @@ -88,7 +92,9 @@ pub async fn get( federated_user::get_federated_user_builder( &user, federated_user, - user_opts.map_err(|err| db_err(err, "fetching user options"))?, + UserOptions::from_iter( + user_opts.map_err(|err| db_err(err, "fetching user options"))?, + ), ) } else { return Err(IdentityDatabaseError::MalformedUser(user_id.to_string()))?; @@ -153,7 +159,7 @@ mod tests { ]) .append_query_results([ // Third query result - local user with passwords - get_local_user_with_password_mock("1", 1), + local_user::tests::get_local_user_with_password_mock("1", 1), ]) .into_connection(); let config = Config::default(); diff --git a/src/identity/backends/sql/user/list.rs b/src/identity/backends/sql/user/list.rs index 91999420..0bd46f68 100644 --- a/src/identity/backends/sql/user/list.rs +++ b/src/identity/backends/sql/user/list.rs @@ -92,11 +92,17 @@ pub async fn list( continue; } let user_builder: UserResponseBuilder = if let Some(local) = l { - local_user::get_local_user_builder(conf, &u, local, p.map(|x| x.into_iter()), o) + local_user::get_local_user_builder( + conf, + &u, + local, + p.map(|x| x.into_iter()), + UserOptions::from_iter(o), + ) } else if let Some(nonlocal) = n { - nonlocal_user::get_nonlocal_user_builder(&u, nonlocal, o) + nonlocal_user::get_nonlocal_user_builder(&u, nonlocal, UserOptions::from_iter(o)) } else if !f.is_empty() { - federated_user::get_federated_user_builder(&u, f, o) + federated_user::get_federated_user_builder(&u, f, UserOptions::from_iter(o)) } else { return Err(IdentityDatabaseError::MalformedUser(u.id))?; }; diff --git a/src/identity/backends/sql/user_group.rs b/src/identity/backends/sql/user_group.rs index b398385f..065fc26a 100644 --- a/src/identity/backends/sql/user_group.rs +++ b/src/identity/backends/sql/user_group.rs @@ -23,10 +23,10 @@ pub use remove::{remove_user_from_group, remove_user_from_groups}; pub use set::set_user_groups; #[cfg(test)] -mod tests { +pub(super) mod tests { use crate::db::entity::user_group_membership; - pub(super) fn get_mock, G: AsRef>( + pub fn get_user_group_mock, G: AsRef>( user_id: U, group_id: G, ) -> user_group_membership::Model { diff --git a/src/identity/backends/sql/user_group/add.rs b/src/identity/backends/sql/user_group/add.rs index a8ceb8b0..2909da6e 100644 --- a/src/identity/backends/sql/user_group/add.rs +++ b/src/identity/backends/sql/user_group/add.rs @@ -37,7 +37,8 @@ pub async fn add_user_to_group, G: AsRef>( Ok(()) } -/// Add group user relations as specified by the tuples (user_id, group_id) iterator. +/// Add group user relations as specified by the tuples (user_id, group_id) +/// iterator. pub async fn add_users_to_groups( db: &DatabaseConnection, iter: I, @@ -65,14 +66,14 @@ where mod tests { use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult, Transaction}; - use super::super::tests::get_mock; + use super::super::tests::get_user_group_mock; use super::*; #[tokio::test] async fn test_create() { // Create MockDatabase with mock query results let db = MockDatabase::new(DatabaseBackend::Postgres) - .append_query_results([vec![get_mock("u1", "g1")]]) + .append_query_results([vec![get_user_group_mock("u1", "g1")]]) .into_connection(); assert!(add_user_to_group(&db, "u1", "g1").await.is_ok()); diff --git a/src/identity/backends/sql/user_group/set.rs b/src/identity/backends/sql/user_group/set.rs index 575ffa2e..f827b4dd 100644 --- a/src/identity/backends/sql/user_group/set.rs +++ b/src/identity/backends/sql/user_group/set.rs @@ -24,9 +24,9 @@ use super::{add_users_to_groups, remove_user_from_groups}; /// Set user group memberships. /// -/// Add user to the groups it should be in and remove from the groups where the user is currently -/// member of, but should not be. This is only incremental operation and is not deleting group -/// membership where the user should stay. +/// Add user to the groups it should be in and remove from the groups where the +/// user is currently member of, but should not be. This is only incremental +/// operation and is not deleting group membership where the user should stay. pub async fn set_user_groups( db: &DatabaseConnection, user_id: U, @@ -82,17 +82,17 @@ where mod tests { use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult, Transaction}; - use super::super::tests::get_mock; + use super::super::tests::get_user_group_mock; use super::*; #[tokio::test] async fn test_add_and_remove() { let db = MockDatabase::new(DatabaseBackend::Postgres) .append_query_results([vec![ - get_mock("u1", "g1"), - get_mock("u1", "g2"), - get_mock("u1", "g3"), - get_mock("u1", "g4"), + get_user_group_mock("u1", "g1"), + get_user_group_mock("u1", "g2"), + get_user_group_mock("u1", "g3"), + get_user_group_mock("u1", "g4"), ]]) .append_exec_results([MockExecResult { rows_affected: 1, @@ -135,10 +135,10 @@ mod tests { async fn test_only_add() { let db = MockDatabase::new(DatabaseBackend::Postgres) .append_query_results([vec![ - get_mock("u1", "g1"), - get_mock("u1", "g2"), - get_mock("u1", "g3"), - get_mock("u1", "g4"), + get_user_group_mock("u1", "g1"), + get_user_group_mock("u1", "g2"), + get_user_group_mock("u1", "g3"), + get_user_group_mock("u1", "g4"), ]]) .append_exec_results([MockExecResult { rows_affected: 1, @@ -172,10 +172,10 @@ mod tests { async fn test_only_delete() { let db = MockDatabase::new(DatabaseBackend::Postgres) .append_query_results([vec![ - get_mock("u1", "g1"), - get_mock("u1", "g2"), - get_mock("u1", "g3"), - get_mock("u1", "g4"), + get_user_group_mock("u1", "g1"), + get_user_group_mock("u1", "g2"), + get_user_group_mock("u1", "g3"), + get_user_group_mock("u1", "g4"), ]]) .append_exec_results([MockExecResult { rows_affected: 1, @@ -207,10 +207,10 @@ mod tests { async fn test_no_change() { let db = MockDatabase::new(DatabaseBackend::Postgres) .append_query_results([vec![ - get_mock("u1", "g1"), - get_mock("u1", "g2"), - get_mock("u1", "g3"), - get_mock("u1", "g4"), + get_user_group_mock("u1", "g1"), + get_user_group_mock("u1", "g2"), + get_user_group_mock("u1", "g3"), + get_user_group_mock("u1", "g4"), ]]) .into_connection(); diff --git a/src/identity/backends/sql/user_option.rs b/src/identity/backends/sql/user_option.rs index 1abc5eee..95afad21 100644 --- a/src/identity/backends/sql/user_option.rs +++ b/src/identity/backends/sql/user_option.rs @@ -13,6 +13,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::db::entity::user_option; +use crate::identity::error::IdentityProviderError; use crate::identity::types::*; mod list; @@ -48,3 +49,79 @@ impl FromIterator for UserOptions { user_opts } } + +#[allow(unused)] +fn get_user_options_db_entries>( + user_id: U, + options: &UserOptions, +) -> Result, IdentityProviderError> { + let mut res: Vec = Vec::new(); + if let Some(val) = &options.ignore_change_password_upon_first_use { + res.push(user_option::Model { + user_id: user_id.as_ref().to_string(), + option_id: "1000".into(), + option_value: Some(val.to_string()), + }); + } + if let Some(val) = &options.ignore_password_expiry { + res.push(user_option::Model { + user_id: user_id.as_ref().to_string(), + option_id: "1001".into(), + option_value: Some(val.to_string()), + }); + } + if let Some(val) = &options.ignore_lockout_failure_attempts { + res.push(user_option::Model { + user_id: user_id.as_ref().to_string(), + option_id: "1002".into(), + option_value: Some(val.to_string()), + }); + } + if let Some(val) = &options.lock_password { + res.push(user_option::Model { + user_id: user_id.as_ref().to_string(), + option_id: "1003".into(), + option_value: Some(val.to_string()), + }); + } + if let Some(val) = &options.multi_factor_auth_rules { + res.push(user_option::Model { + user_id: user_id.as_ref().to_string(), + option_id: "MFAR".into(), + option_value: Some(serde_json::to_string(val)?), + }); + } + if let Some(val) = &options.multi_factor_auth_enabled { + res.push(user_option::Model { + user_id: user_id.as_ref().to_string(), + option_id: "MFAE".into(), + option_value: Some(val.to_string()), + }); + } + Ok(res) +} + +#[cfg(test)] +pub(super) mod tests { + use crate::db::entity::user_option; + use crate::identity::types::UserOptions; + + use super::*; + + impl Default for user_option::Model { + fn default() -> Self { + Self { + user_id: "1".into(), + option_id: "1000".into(), + option_value: None, + } + } + } + + pub fn get_user_options_mock(options: &UserOptions) -> Vec { + get_user_options_db_entries("1", options) + .unwrap() + .into_iter() + .collect() + } +} diff --git a/src/identity/backends/sql/user_option/list.rs b/src/identity/backends/sql/user_option/list.rs index 493c65e6..d8a304ac 100644 --- a/src/identity/backends/sql/user_option/list.rs +++ b/src/identity/backends/sql/user_option/list.rs @@ -18,14 +18,17 @@ use sea_orm::query::*; use crate::db::entity::{prelude::UserOption as DbUserOptions, user_option}; use crate::identity::backends::sql::{IdentityDatabaseError, db_err}; +use crate::identity::types::UserOptions; pub async fn list_by_user_id>( db: &DatabaseConnection, user_id: S, -) -> Result, IdentityDatabaseError> { - DbUserOptions::find() - .filter(user_option::Column::UserId.eq(user_id.as_ref())) - .all(db) - .await - .map_err(|err| db_err(err, "fetching options of the user")) +) -> Result { + Ok(UserOptions::from_iter( + DbUserOptions::find() + .filter(user_option::Column::UserId.eq(user_id.as_ref())) + .all(db) + .await + .map_err(|err| db_err(err, "fetching options of the user"))?, + )) } diff --git a/src/identity/error.rs b/src/identity/error.rs index af616c8f..a68df520 100644 --- a/src/identity/error.rs +++ b/src/identity/error.rs @@ -90,9 +90,6 @@ pub enum IdentityProviderError { /// Conflict. #[error("conflict: {0}")] Conflict(String), - - #[error("wrong username or password")] - WrongUsernamePassword, } impl From for IdentityProviderError { @@ -102,6 +99,16 @@ impl From for IdentityProviderError { IdentityDatabaseError::UserNotFound(x) => Self::UserNotFound(x), IdentityDatabaseError::GroupNotFound(x) => Self::GroupNotFound(x), IdentityDatabaseError::Serde { source } => Self::Serde { source }, + IdentityDatabaseError::PasswordHash { source } => Self::PasswordHash { source }, + IdentityDatabaseError::NoPasswordHash(..) => Self::AuthenticationInfo { + source: crate::auth::AuthenticationError::UserNameOrPasswordWrong, + }, + IdentityDatabaseError::AuthenticationInfo { source } => { + Self::AuthenticationInfo { source } + } + //IdentityDatabaseError::UserLocked(x) => Self::AuthenticationInfo { + // source: crate::auth::AuthenticationError::UserLocked(x), + //}, _ => Self::IdentityDatabase { source }, } } diff --git a/src/identity/mock.rs b/src/identity/mock.rs index d3f56149..e40c955b 100644 --- a/src/identity/mock.rs +++ b/src/identity/mock.rs @@ -38,7 +38,7 @@ mock! { async fn authenticate_by_password( &self, state: &ServiceState, - auth: UserPasswordAuthRequest, + auth: &UserPasswordAuthRequest, ) -> Result; async fn list_users( diff --git a/src/identity/mod.rs b/src/identity/mod.rs index 6df0187b..3bd607df 100644 --- a/src/identity/mod.rs +++ b/src/identity/mod.rs @@ -76,9 +76,9 @@ impl IdentityApi for IdentityProvider { async fn authenticate_by_password( &self, state: &ServiceState, - auth: UserPasswordAuthRequest, + auth: &UserPasswordAuthRequest, ) -> Result { - let mut auth = auth; + let mut auth = auth.clone(); if auth.id.is_none() { if auth.name.is_none() { return Err(IdentityProviderError::UserIdOrNameWithDomain); @@ -102,7 +102,7 @@ impl IdentityApi for IdentityProvider { } self.backend_driver - .authenticate_by_password(state, auth) + .authenticate_by_password(state, &auth) .await } diff --git a/src/identity/password_hashing.rs b/src/identity/password_hashing.rs index 98feece7..7b495e50 100644 --- a/src/identity/password_hashing.rs +++ b/src/identity/password_hashing.rs @@ -62,11 +62,20 @@ pub async fn verify_password, H: AsRef>( .to_owned(); let password_hash = hash.as_ref().to_string(); // Do not block the main thread with a definitely long running call. - let verify = - task::spawn_blocking(move || bcrypt::verify(password_bytes, &password_hash)) - .await??; - Ok(verify) - //Ok(bcrypt::verify(password_bytes, hash.as_ref())?) + match task::spawn_blocking(move || bcrypt::verify(password_bytes, &password_hash)) + .await? + { + Ok(res) => Ok(res), + Err(bcrypt::BcryptError::InvalidHash(..)) => { + // InvalidHash error contain the hash itself. We do not want to log it. + warn!("Bcrypt hash verification error: bad hash"); + Ok(false) + } + other => { + warn!("Bcrypt hash verification error: {other:?}"); + Ok(false) + } + } } } } @@ -75,6 +84,7 @@ pub async fn verify_password, H: AsRef>( mod tests { use super::*; use rand::distr::{Alphanumeric, SampleString}; + use tracing_test::traced_test; #[test] fn test_verify_length_and_trunc_password() { @@ -94,6 +104,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_hash_bcrypt() { let builder = config::Config::builder() .set_override("auth.methods", "") @@ -101,10 +112,14 @@ mod tests { .set_override("database.connection", "dummy") .unwrap(); let conf: Config = Config::try_from(builder).expect("can build a valid config"); - assert!(hash_password(&conf, "abcdefg").await.is_ok()); + let pass = "abcdefg"; + let hashed = hash_password(&conf, &pass).await.unwrap(); + assert!(!logs_contain(pass)); + assert!(!logs_contain(&hashed)); } #[tokio::test] + #[traced_test] async fn test_roundtrip_bcrypt() { let builder = config::Config::builder() .set_override("auth.methods", "") @@ -112,11 +127,15 @@ mod tests { .set_override("database.connection", "dummy") .unwrap(); let conf: Config = Config::try_from(builder).expect("can build a valid config"); - let hashed = hash_password(&conf, "abcdefg").await.unwrap(); - assert!(verify_password(&conf, "abcdefg", hashed).await.unwrap()); + let pass = "abcdefg"; + let hashed = hash_password(&conf, &pass).await.unwrap(); + assert!(verify_password(&conf, &pass, &hashed).await.unwrap()); + assert!(!logs_contain(pass)); + assert!(!logs_contain(&hashed)); } #[tokio::test] + #[traced_test] async fn test_roundtrip_bcrypt_longer_than_72() { let builder = config::Config::builder() .set_override("auth.methods", "") @@ -125,7 +144,40 @@ mod tests { .unwrap(); let conf: Config = Config::try_from(builder).expect("can build a valid config"); let pass = Alphanumeric.sample_string(&mut rand::rng(), 80); - let hashed = hash_password(&conf, pass.clone()).await.unwrap(); - assert!(verify_password(&conf, pass, hashed).await.unwrap()); + let hashed = hash_password(&conf, &pass).await.unwrap(); + assert!(verify_password(&conf, &pass, &hashed).await.unwrap()); + assert!(!logs_contain(&pass)); + assert!(!logs_contain(&hashed)); + } + + #[tokio::test] + #[traced_test] + async fn test_roundtrip_bcrypt_mismatch() { + let builder = config::Config::builder() + .set_override("auth.methods", "") + .unwrap() + .set_override("database.connection", "dummy") + .unwrap(); + let conf: Config = Config::try_from(builder).expect("can build a valid config"); + let pass = Alphanumeric.sample_string(&mut rand::rng(), 80); + let hashed = hash_password(&conf, "other password").await.unwrap(); + assert!(!verify_password(&conf, &pass, &hashed).await.unwrap()); + assert!(!logs_contain(&pass)); + assert!(!logs_contain(&hashed)); + } + + #[tokio::test] + #[traced_test] + async fn test_roundtrip_bcrypt_bad_hash() { + let builder = config::Config::builder() + .set_override("auth.methods", "") + .unwrap() + .set_override("database.connection", "dummy") + .unwrap(); + let conf: Config = Config::try_from(builder).expect("can build a valid config"); + let pass = Alphanumeric.sample_string(&mut rand::rng(), 80); + assert!(!verify_password(&conf, &pass, "foobar").await.unwrap()); + assert!(!logs_contain("foobar")); + assert!(!logs_contain(&pass)); } } diff --git a/src/identity/types.rs b/src/identity/types.rs index b3f90057..2dd43ed0 100644 --- a/src/identity/types.rs +++ b/src/identity/types.rs @@ -31,7 +31,7 @@ pub trait IdentityApi: Send + Sync + Clone { async fn authenticate_by_password( &self, state: &ServiceState, - auth: UserPasswordAuthRequest, + auth: &UserPasswordAuthRequest, ) -> Result; async fn list_users( diff --git a/src/identity/types/user.rs b/src/identity/types/user.rs index cd91d164..5af23a28 100644 --- a/src/identity/types/user.rs +++ b/src/identity/types/user.rs @@ -30,7 +30,8 @@ pub struct UserResponse { pub name: String, /// The ID of the domain. pub domain_id: String, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: bool, /// The resource description #[builder(default)] @@ -47,7 +48,10 @@ pub struct UserResponse { /// The resource options for the user. #[builder(default)] pub options: UserOptions, - /// List of federated objects associated with a user. Each object in the list contains the idp_id and protocols. protocols is a list of objects, each of which contains protocol_id and unique_id of the protocol and user respectively. + /// List of federated objects associated with a user. Each object in the + /// list contains the idp_id and protocols. protocols is a list of objects, + /// each of which contains protocol_id and unique_id of the protocol and + /// user respectively. #[builder(default)] pub federated: Option>, } @@ -60,7 +64,8 @@ pub struct UserCreate { pub name: String, /// The ID of the domain. pub domain_id: String, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. pub enabled: Option, /// The ID of the default project for the user. #[builder(default)] @@ -74,7 +79,10 @@ pub struct UserCreate { /// The resource options for the user. #[builder(default)] pub options: Option, - /// List of federated objects associated with a user. Each object in the list contains the idp_id and protocols. protocols is a list of objects, each of which contains protocol_id and unique_id of the protocol and user respectively. + /// List of federated objects associated with a user. Each object in the + /// list contains the idp_id and protocols. protocols is a list of objects, + /// each of which contains protocol_id and unique_id of the protocol and + /// user respectively. #[builder(default)] pub federated: Option>, } @@ -85,7 +93,8 @@ pub struct UserUpdate { /// The user name. Must be unique within the owning domain. #[builder(default)] pub name: Option>, - /// If the user is enabled, this value is true. If the user is disabled, this value is false. + /// If the user is enabled, this value is true. If the user is disabled, + /// this value is false. #[builder(default)] pub enabled: Option, /// The resource description @@ -103,7 +112,10 @@ pub struct UserUpdate { /// The resource options for the user. #[builder(default)] pub options: Option, - /// List of federated objects associated with a user. Each object in the list contains the idp_id and protocols. protocols is a list of objects, each of which contains protocol_id and unique_id of the protocol and user respectively. + /// List of federated objects associated with a user. Each object in the + /// list contains the idp_id and protocols. protocols is a list of objects, + /// each of which contains protocol_id and unique_id of the protocol and + /// user respectively. #[builder(default)] pub federated: Option>, } diff --git a/src/keystone.rs b/src/keystone.rs index 5dae5e73..63d4aab4 100644 --- a/src/keystone.rs +++ b/src/keystone.rs @@ -28,8 +28,8 @@ use crate::policy::Policy; use crate::policy::PolicyFactory; use crate::provider::Provider; -// Placing ServiceState behind Arc is necessary to address DatabaseConnection not implementing -// Clone +// Placing ServiceState behind Arc is necessary to address DatabaseConnection +// not implementing Clone. //#[derive(Clone)] #[derive(FromRef)] pub struct Service { diff --git a/src/plugin_manager.rs b/src/plugin_manager.rs index 46f8090b..e03b8716 100644 --- a/src/plugin_manager.rs +++ b/src/plugin_manager.rs @@ -21,8 +21,8 @@ use crate::identity::backends::IdentityBackend; use crate::resource::types::ResourceBackend; use crate::revoke::backend::RevokeBackend; -/// Plugin manager allowing to pass custom backend plugins implementing required trait during the -/// service start. +/// Plugin manager allowing to pass custom backend plugins implementing required +/// trait during the service start. #[derive(Clone, Debug, Default)] pub struct PluginManager { /// Assignments backend plugin. diff --git a/src/resource/backend/error.rs b/src/resource/backend/error.rs index 0d29eea1..34ecc52e 100644 --- a/src/resource/backend/error.rs +++ b/src/resource/backend/error.rs @@ -55,7 +55,8 @@ pub enum ResourceDatabaseError { }, } -/// Convert the DB error into the [ResourceDatabaseError] with the context information. +/// Convert the DB error into the [ResourceDatabaseError] with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> ResourceDatabaseError { e.sql_err().map_or_else( || ResourceDatabaseError::Database { diff --git a/src/resource/backend/sql.rs b/src/resource/backend/sql.rs index f2f66b37..e1422564 100644 --- a/src/resource/backend/sql.rs +++ b/src/resource/backend/sql.rs @@ -175,18 +175,32 @@ impl ResourceBackend for SqlBackend { // [ // Transaction::from_sql_and_values( // DatabaseBackend::Postgres, -// r#"SELECT "user"."id", "user"."extra", "user"."enabled", "user"."default_project_id", "user"."created_at", "user"."last_active_at", "user"."domain_id" FROM "user" WHERE "user"."id" = $1 LIMIT $2"#, -// ["1".into(), 1u64.into()] -// ), +// r#"SELECT "user"."id", "user"."extra", "user"."enabled", +// "user"."default_project_id", "user"."created_at", "user"."last_active_at", +// "user"."domain_id" FROM "user" WHERE "user"."id" = $1 LIMIT $2"#, +// ["1".into(), 1u64.into()] ), // Transaction::from_sql_and_values( // DatabaseBackend::Postgres, -// r#"SELECT "user_option"."user_id", "user_option"."option_id", "user_option"."option_value" FROM "user_option" INNER JOIN "user" ON "user"."id" = "user_option"."user_id" WHERE "user"."id" = $1"#, -// ["1".into()] +// r#"SELECT "user_option"."user_id", +// "user_option"."option_id", "user_option"."option_value" FROM "user_option" +// INNER JOIN "user" ON "user"."id" = "user_option"."user_id" WHERE "user"."id" +// = $1"#, ["1".into()] // ), // Transaction::from_sql_and_values( // DatabaseBackend::Postgres, -// r#"SELECT "local_user"."id" AS "A_id", "local_user"."user_id" AS "A_user_id", "local_user"."domain_id" AS "A_domain_id", "local_user"."name" AS "A_name", "local_user"."failed_auth_count" AS "A_failed_auth_count", "local_user"."failed_auth_at" AS "A_failed_auth_at", "password"."id" AS "B_id", "password"."local_user_id" AS "B_local_user_id", "password"."self_service" AS "B_self_service", "password"."created_at" AS "B_created_at", "password"."expires_at" AS "B_expires_at", "password"."password_hash" AS "B_password_hash", "password"."created_at_int" AS "B_created_at_int", "password"."expires_at_int" AS "B_expires_at_int" FROM "local_user" LEFT JOIN "password" ON "local_user"."id" = "password"."local_user_id" WHERE "local_user"."user_id" = $1 ORDER BY "local_user"."id" ASC"#, -// ["1".into()] +// r#"SELECT "local_user"."id" AS "A_id", +// "local_user"."user_id" AS "A_user_id", "local_user"."domain_id" AS +// "A_domain_id", "local_user"."name" AS "A_name", +// "local_user"."failed_auth_count" AS "A_failed_auth_count", +// "local_user"."failed_auth_at" AS "A_failed_auth_at", "password"."id" AS +// "B_id", "password"."local_user_id" AS "B_local_user_id", +// "password"."self_service" AS "B_self_service", "password"."created_at" AS +// "B_created_at", "password"."expires_at" AS "B_expires_at", +// "password"."password_hash" AS "B_password_hash", "password"."created_at_int" +// AS "B_created_at_int", "password"."expires_at_int" AS "B_expires_at_int" FROM +// "local_user" LEFT JOIN "password" ON "local_user"."id" = +// "password"."local_user_id" WHERE "local_user"."user_id" = $1 ORDER BY +// "local_user"."id" ASC"#, ["1".into()] // ), // ] // ); diff --git a/src/resource/types/project.rs b/src/resource/types/project.rs index e846c968..162d2ca1 100644 --- a/src/resource/types/project.rs +++ b/src/resource/types/project.rs @@ -28,7 +28,8 @@ pub struct Project { pub name: String, /// The project domain_id. pub domain_id: String, - /// If set to true, project is enabled. If set to false, project is disabled. + /// If set to true, project is enabled. If set to false, project is + /// disabled. pub enabled: bool, /// The description of the project. #[builder(default)] diff --git a/src/revoke/backend.rs b/src/revoke/backend.rs index 3a32919e..e9b86c4e 100644 --- a/src/revoke/backend.rs +++ b/src/revoke/backend.rs @@ -34,7 +34,8 @@ pub trait RevokeBackend: DynClone + Send + Sync + std::fmt::Debug { /// Check token revocation. /// - /// Check whether there are existing revocation records that invalidate the token. + /// Check whether there are existing revocation records that invalidate the + /// token. async fn is_token_revoked( &self, state: &ServiceState, @@ -43,7 +44,8 @@ pub trait RevokeBackend: DynClone + Send + Sync + std::fmt::Debug { /// Revoke the token. /// - /// Mark the token as revoked to prohibit from being used even while not expired. + /// Mark the token as revoked to prohibit from being used even while not + /// expired. async fn revoke_token( &self, state: &ServiceState, diff --git a/src/revoke/backend/error.rs b/src/revoke/backend/error.rs index c152deb9..d1ce5dd4 100644 --- a/src/revoke/backend/error.rs +++ b/src/revoke/backend/error.rs @@ -55,7 +55,8 @@ pub enum RevokeDatabaseError { }, } -/// Convert the DB error into the [RevokeDatabaseError] with the context information. +/// Convert the DB error into the [RevokeDatabaseError] with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> RevokeDatabaseError { e.sql_err().map_or_else( || RevokeDatabaseError::Database { diff --git a/src/revoke/backend/sql.rs b/src/revoke/backend/sql.rs index 3cef8714..03f1eb7d 100644 --- a/src/revoke/backend/sql.rs +++ b/src/revoke/backend/sql.rs @@ -64,8 +64,8 @@ impl RevokeBackend for SqlBackend { /// Check the token for being revoked. /// - /// List not expired revocation records that invalidate the token and returns true if there is - /// at least one such record. + /// List not expired revocation records that invalidate the token and + /// returns true if there is at least one such record. async fn is_token_revoked( &self, state: &ServiceState, @@ -81,7 +81,8 @@ impl RevokeBackend for SqlBackend { /// Revoke the token. /// - /// Mark the token as revoked to prohibit from being used even while not expired. + /// Mark the token as revoked to prohibit from being used even while not + /// expired. async fn revoke_token( &self, state: &ServiceState, diff --git a/src/revoke/backend/sql/list.rs b/src/revoke/backend/sql/list.rs index 4fdc4edb..d7ff944a 100644 --- a/src/revoke/backend/sql/list.rs +++ b/src/revoke/backend/sql/list.rs @@ -30,7 +30,8 @@ fn build_query_filters( let mut select = DbRevocationEvent::find(); //if let Some(val) = ¶ms.access_token_id { - // select = select.filter(db_revocation_event::Column::AccessTokenId.eq(val)); + // select = + // select.filter(db_revocation_event::Column::AccessTokenId.eq(val)); //} //if let Some(val) = ¶ms.audit_chain_id { diff --git a/src/revoke/mock.rs b/src/revoke/mock.rs index 7ecee160..a04d8f81 100644 --- a/src/revoke/mock.rs +++ b/src/revoke/mock.rs @@ -12,7 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 //! Token revocation - internal mocking tools. -//! use async_trait::async_trait; #[cfg(test)] use mockall::mock; diff --git a/src/revoke/mod.rs b/src/revoke/mod.rs index 9d4fbf36..a597957a 100644 --- a/src/revoke/mod.rs +++ b/src/revoke/mod.rs @@ -13,16 +13,16 @@ // SPDX-License-Identifier: Apache-2.0 //! Token revocation provider. //! -//! Token revocation may be implemented in different ways, but in most cases would be represented -//! by the presence of the revocation or the invalidation record matching the certain token -//! parameters. +//! Token revocation may be implemented in different ways, but in most cases +//! would be represented by the presence of the revocation or the invalidation +//! record matching the certain token parameters. //! -//! Default backend is the [crate::revoke::backend::sql] and uses the database table -//! [crate::db::entity::revocation_event::Model] for storing the revocation events. They have their -//! own expiration. +//! Default backend is the [crate::revoke::backend::sql] and uses the database +//! table [crate::db::entity::revocation_event::Model] for storing the +//! revocation events. They have their own expiration. //! -//! Tokens are not invalidated by saving the exact value, but rather by saving certain attributes -//! of the token. +//! Tokens are not invalidated by saving the exact value, but rather by saving +//! certain attributes of the token. //! //! Following attributes are used for matching of the regular fernet token: //! @@ -32,9 +32,8 @@ //! - `project_id` //! - `user_id` //! -//! Additionally the `token.issued_at` is compared to be lower than the `issued_before` field of -//! the revocation record. -//! +//! Additionally the `token.issued_at` is compared to be lower than the +//! `issued_before` field of the revocation record. use async_trait::async_trait; @@ -89,8 +88,8 @@ impl RevokeProvider { impl RevokeApi for RevokeProvider { /// Check whether the token has been revoked or not. /// - /// Checks revocation events matching the token parameters and return `false` if their count is - /// more than `0`. + /// Checks revocation events matching the token parameters and return + /// `false` if their count is more than `0`. #[tracing::instrument(level = "info", skip(self, state, token))] async fn is_token_revoked( &self, @@ -103,7 +102,8 @@ impl RevokeApi for RevokeProvider { /// Revoke the token. /// - /// Mark the token as revoked to prohibit from being used even while not expired. + /// Mark the token as revoked to prohibit from being used even while not + /// expired. async fn revoke_token( &self, state: &ServiceState, diff --git a/src/revoke/types.rs b/src/revoke/types.rs index a9f1b9fd..32b06cef 100644 --- a/src/revoke/types.rs +++ b/src/revoke/types.rs @@ -28,8 +28,8 @@ use crate::token::types::Token; pub trait RevokeApi: Send + Sync + Clone { /// Check whether the token has been revoked of not. /// - /// Checks revocation events matching the token parameters and return `false` if their count is - /// more than `0`. + /// Checks revocation events matching the token parameters and return + /// `false` if their count is more than `0`. async fn is_token_revoked( &self, state: &ServiceState, @@ -38,7 +38,8 @@ pub trait RevokeApi: Send + Sync + Clone { /// Revoke the token. /// - /// Mark the token as revoked to prohibit from being used even while not expired. + /// Mark the token as revoked to prohibit from being used even while not + /// expired. async fn revoke_token( &self, state: &ServiceState, @@ -84,7 +85,8 @@ pub struct RevocationEventCreate { /// Revocation list parameters. /// -/// It may be necessary to list revocation events not related to the certain token. +/// It may be necessary to list revocation events not related to the certain +/// token. #[derive(Builder, Clone, Debug, Default, Deserialize, Serialize, PartialEq)] #[builder(setter(strip_option, into))] pub struct RevocationEventListParameters { @@ -94,14 +96,15 @@ pub struct RevocationEventListParameters { #[builder(default)] pub audit_id: Option, //pub consumer_id: Option, - /// List revocation events with an empty `domain_id` or matching any of the given values. + /// List revocation events with an empty `domain_id` or matching any of the + /// given values. #[builder(default)] pub domain_ids: Option>, /// Expires_at parameter to match against. #[builder(default)] pub expires_at: Option>, - /// List revocation events with the `issued_before` value greater or equal the value - /// (revocating tokens issued before the certain time). + /// List revocation events with the `issued_before` value greater or equal + /// the value (revocating tokens issued before the certain time). #[builder(default)] pub issued_before: Option>, /// Project_id to match against. @@ -110,7 +113,8 @@ pub struct RevocationEventListParameters { #[builder(default)] /// Revocation timestamp to match against. Currently not respected. pub revoked_at: Option>, - /// List revocation events with an empty `role_id` or matching any of the given values. + /// List revocation events with an empty `role_id` or matching any of the + /// given values. #[builder(default)] pub role_ids: Option>, //pub trust_id: Option, @@ -121,9 +125,10 @@ pub struct RevocationEventListParameters { /// Convert Token into the revocation events listing parameters following the /// -// TODO: It is necessary to also consider list of the token roles against the role_id of the entry -// TODO: domain_id of the database entry should be compared against the user_domain_id and the -// scope_domain_id. That means, however, that we must resolve the user first. +// TODO: It is necessary to also consider list of the token roles against the +// role_id of the entry TODO: domain_id of the database entry should be compared +// against the user_domain_id and the scope_domain_id. That means, however, that +// we must resolve the user first. impl TryFrom<&Token> for RevocationEventListParameters { type Error = RevokeProviderError; fn try_from(value: &Token) -> Result { diff --git a/src/token/backend/fernet.rs b/src/token/backend/fernet.rs index 4d5d8bd3..2dc6a88a 100644 --- a/src/token/backend/fernet.rs +++ b/src/token/backend/fernet.rs @@ -165,7 +165,8 @@ impl FernetTokenProvider { .iter() .fold(0, |acc, (k, v)| acc + if me.contains(v) { *k } else { 0 }); - // TODO: Improve unit tests to ensure unsupported auth method immediately raises error. + // TODO: Improve unit tests to ensure unsupported auth method immediately raises + // error. if res == 0 { return Err(TokenProviderError::UnsupportedAuthMethods( me.iter().join(","), @@ -312,8 +313,8 @@ impl FernetTokenProvider { /// 1. Decrypt as Fernet /// 2. Unpack MessagePack payload pub fn decrypt(&self, credential: &str) -> Result { - // TODO: Implement fernet keys change watching. Keystone loads them from FS on every - // request and in the best case it costs 15µs. + // TODO: Implement fernet keys change watching. Keystone loads them from FS on + // every request and in the best case it costs 15µs. let fernet = match &self.fernet { Some(f) => f, None => &self.get_fernet()?, @@ -393,7 +394,8 @@ fn get_fernet_timestamp(payload: &str) -> Result, TokenProviderErr }) } -// Conditionally expose the function when the 'bench_internals' feature is enabled +// Conditionally expose the function when the 'bench_internals' feature is +// enabled #[cfg(feature = "bench_internals")] pub fn bench_get_fernet_timestamp(payload: &str) -> Result, TokenProviderError> { get_fernet_timestamp(payload) diff --git a/src/token/backend/fernet/utils.rs b/src/token/backend/fernet/utils.rs index 16989af8..0cc3dd10 100644 --- a/src/token/backend/fernet/utils.rs +++ b/src/token/backend/fernet/utils.rs @@ -146,9 +146,9 @@ pub fn read_str(rd: &mut R) -> Result { } /// Read the UUID from the payload -/// It is represented as an Array[bool, bytes] where first bool indicates whether following bytes -/// are UUID or just bytes that should be treated as a string (for cases where ID is not a valid -/// UUID) +/// It is represented as an Array[bool, bytes] where first bool indicates +/// whether following bytes are UUID or just bytes that should be treated as a +/// string (for cases where ID is not a valid UUID) pub fn read_uuid(rd: &mut &[u8]) -> Result { match read_marker(rd).map_err(ValueReadError::from)? { Marker::FixArray(_) => { @@ -197,9 +197,9 @@ pub fn read_uuid(rd: &mut &[u8]) -> Result { } /// Write the UUID to the payload -/// It is represented as an Array[bool, bytes] where first bool indicates whether following bytes -/// are UUID or just bytes that should be treated as a string (for cases where ID is not a valid -/// UUID) +/// It is represented as an Array[bool, bytes] where first bool indicates +/// whether following bytes are UUID or just bytes that should be treated as a +/// string (for cases where ID is not a valid UUID) pub fn write_uuid(wd: &mut W, uid: &str) -> Result<(), TokenProviderError> { match Uuid::parse_str(uid) { Ok(uuid) => { diff --git a/src/token/error.rs b/src/token/error.rs index a6c411ea..d19f3a95 100644 --- a/src/token/error.rs +++ b/src/token/error.rs @@ -241,7 +241,8 @@ pub enum TokenProviderError { AuditIdWrongFormat, } -/// Convert the DB error into the TokenProviderError with the context information. +/// Convert the DB error into the TokenProviderError with the context +/// information. pub fn db_err(e: sea_orm::DbErr, context: &str) -> TokenProviderError { e.sql_err().map_or_else( || TokenProviderError::Database { diff --git a/src/token/mod.rs b/src/token/mod.rs index 5ea0afe0..6324d418 100644 --- a/src/token/mod.rs +++ b/src/token/mod.rs @@ -413,12 +413,13 @@ impl TokenApi for TokenProvider { authz_info: AuthzInfo, token_restrictions: Option<&TokenRestriction>, ) -> Result { - // This should be executed already, but let's better repeat it as last line of defence. - // It is also necessary to call this before to stop before we start to resolve authz info. + // This should be executed already, but let's better repeat it as last line of + // defence. It is also necessary to call this before to stop before we + // start to resolve authz info. authentication_info.validate()?; - // TODO: Check whether it is allowed to change the scope of the token if AuthenticatedInfo - // already contains scope it was issued for. + // TODO: Check whether it is allowed to change the scope of the token if + // AuthenticatedInfo already contains scope it was issued for. let mut authentication_info = authentication_info; authentication_info.audit_ids.push( URL_SAFE diff --git a/src/token/types.rs b/src/token/types.rs index 8f6658f9..625aaf89 100644 --- a/src/token/types.rs +++ b/src/token/types.rs @@ -95,8 +95,8 @@ impl Token { /// Set the `issued_at` property of the token. /// - /// An internal method (available only within the module) to set the `issued_at` into the token - /// payload. + /// An internal method (available only within the module) to set the + /// `issued_at` into the token payload. pub(super) fn set_issued_at(&mut self, issued_at: DateTime) -> &mut Self { match self { Self::Unscoped(x) => x.issued_at = issued_at, @@ -113,8 +113,8 @@ impl Token { /// Get token `issued_at` timestamp. /// - /// Returns the UTC timestamp when the token was encoded (part of the Fernet payload and not the - /// token payload). + /// Returns the UTC timestamp when the token was encoded (part of the Fernet + /// payload and not the token payload). pub const fn issued_at(&self) -> &DateTime { match self { Self::Unscoped(x) => &x.issued_at, diff --git a/src/token/types/provider_api.rs b/src/token/types/provider_api.rs index a44f2da4..1c7d9d7a 100644 --- a/src/token/types/provider_api.rs +++ b/src/token/types/provider_api.rs @@ -39,11 +39,14 @@ pub trait TokenApi: Send + Sync + Clone { /// /// * `state` - An application state. /// * `credential` - A token as a string. - /// * `allow_expired` - Indicates whether for the expired token the an error should be raised + /// * `allow_expired` - Indicates whether for the expired token the an error + /// should be raised /// or not. - /// * `window_seconds` - An additional token expiration buffer that is added to the + /// * `window_seconds` - An additional token expiration buffer that is added + /// to the /// `token.expires_at() during the expiration calculation. - /// * `expand` - Indicates whether the token information should be expanded or not. Defaults to + /// * `expand` - Indicates whether the token information should be expanded + /// or not. Defaults to /// true. async fn validate_token<'a>( &self, @@ -77,8 +80,8 @@ pub trait TokenApi: Send + Sync + Clone { token: &mut Token, ) -> Result<(), TokenProviderError>; - /// Populate additional information (project, domain, roles, etc) in the token that support - /// that information + /// Populate additional information (project, domain, roles, etc) in the + /// token that support that information async fn expand_token_information( &self, state: &ServiceState, diff --git a/tests/keycloak/keystone_utils.rs b/tests/keycloak/keystone_utils.rs index 13f30ace..c8d1d372 100644 --- a/tests/keycloak/keystone_utils.rs +++ b/tests/keycloak/keystone_utils.rs @@ -210,8 +210,8 @@ pub async fn ensure_user, U: AsRef, D: AsRef>( Ok(user.user) } -/// Information for finishing the authorization request (received as a callback from `/authorize` -/// call) +/// Information for finishing the authorization request (received as a callback +/// from `/authorize` call) #[derive(Clone, Debug, Deserialize, PartialEq)] pub struct FederationAuthCodeCallbackResponse { /// Authorization code