diff --git a/server/core/src/crypto.rs b/server/core/src/crypto.rs index bcd5f492f..2953e618e 100644 --- a/server/core/src/crypto.rs +++ b/server/core/src/crypto.rs @@ -254,7 +254,7 @@ pub(crate) fn gen_private_key( /// build up a CA certificate and key. pub(crate) fn build_ca(ca_config: Option) -> Result { - let ca_config = ca_config.unwrap_or(CAConfig::default()); + let ca_config = ca_config.unwrap_or_default(); let ca_key = gen_private_key(&ca_config.key_type, Some(ca_config.key_bits))?; @@ -419,7 +419,7 @@ pub(crate) fn build_cert( key_type: Option, key_bits: Option, ) -> Result { - let key_type = key_type.unwrap_or(KeyType::default()); + let key_type = key_type.unwrap_or_default(); let int_key = gen_private_key(&key_type, key_bits)?; let mut req_builder = X509ReqBuilder::new()?; diff --git a/server/core/src/https/errors.rs b/server/core/src/https/errors.rs index a99f93bb7..d6c08b253 100644 --- a/server/core/src/https/errors.rs +++ b/server/core/src/https/errors.rs @@ -26,6 +26,7 @@ impl WebError { let mut res = self.into_response(); res.headers_mut().insert( ACCESS_CONTROL_ALLOW_ORIGIN, + #[allow(clippy::expect_used)] HeaderValue::from_str("*").expect("Header generation failed, this is weird."), ); res diff --git a/server/core/src/lib.rs b/server/core/src/lib.rs index 0bbbd5f91..c207c74b7 100644 --- a/server/core/src/lib.rs +++ b/server/core/src/lib.rs @@ -756,7 +756,6 @@ impl Display for TaskName { TaskName::LdapActor => "LDAP Acceptor Actor", TaskName::Replication => "Replication", } - .to_string() ) } } diff --git a/server/lib/src/constants/acp.rs b/server/lib/src/constants/acp.rs index 6a9ea957c..6a9f93727 100644 --- a/server/lib/src/constants/acp.rs +++ b/server/lib/src/constants/acp.rs @@ -1293,7 +1293,6 @@ lazy_static! { create_classes: vec![ EntryClass::AccountPolicy, ], - ..Default::default() }; } diff --git a/server/lib/src/constants/groups.rs b/server/lib/src/constants/groups.rs index 07dd4bd0b..444ef21b8 100644 --- a/server/lib/src/constants/groups.rs +++ b/server/lib/src/constants/groups.rs @@ -438,7 +438,6 @@ lazy_static! { // Enforce this is a system protected object (Attribute::Class, EntryClass::System.to_value()), ], - ..Default::default() }; pub static ref IDM_ALL_ACCOUNTS: BuiltinGroup = BuiltinGroup { @@ -456,7 +455,6 @@ lazy_static! { // Enforce this is a system protected object (Attribute::Class, EntryClass::System.to_value()), ], - ..Default::default() }; diff --git a/server/lib/src/idm/accountpolicy.rs b/server/lib/src/idm/accountpolicy.rs index 010c58b5e..c81a2a17b 100644 --- a/server/lib/src/idm/accountpolicy.rs +++ b/server/lib/src/idm/accountpolicy.rs @@ -35,19 +35,19 @@ pub(crate) struct AccountPolicy { credential_policy: CredentialPolicy, } -impl Into> for &EntrySealedCommitted { - fn into(self) -> Option { - if !self.attribute_equality( +impl From<&EntrySealedCommitted> for Option { + fn from(val: &EntrySealedCommitted) -> Self { + if !val.attribute_equality( Attribute::Class, &EntryClass::AccountPolicy.to_partialvalue(), ) { return None; } - let authsession_expiry = self + let authsession_expiry = val .get_ava_single_uint32(Attribute::AuthSessionExpiry) .unwrap_or(MAXIMUM_AUTH_SESSION_EXPIRY); - let privilege_expiry = self + let privilege_expiry = val .get_ava_single_uint32(Attribute::PrivilegeExpiry) .unwrap_or(MAXIMUM_AUTH_PRIVILEGE_EXPIRY); let credential_policy = CredentialPolicy::default(); diff --git a/server/lib/src/idm/authsession.rs b/server/lib/src/idm/authsession.rs index d700dd1c4..734864f0d 100644 --- a/server/lib/src/idm/authsession.rs +++ b/server/lib/src/idm/authsession.rs @@ -714,6 +714,15 @@ impl AuthSessionState { } } +pub(crate) struct AuthSessionData<'a> { + pub(crate) account: Account, + pub(crate) account_policy: ResolvedAccountPolicy, + pub(crate) issue: AuthIssueSession, + pub(crate) webauthn: &'a Webauthn, + pub(crate) ct: Duration, + pub(crate) source: Source, +} + #[derive(Clone)] /// The current state of an authentication session that is in progress. pub(crate) struct AuthSession { @@ -745,34 +754,26 @@ impl AuthSession { /// Create a new auth session, based on the available credential handlers of the account. /// the session is a whole encapsulated unit of what we need to proceed, so that subsequent /// or interleved write operations do not cause inconsistency in this process. - pub fn new( - account: Account, - account_policy: ResolvedAccountPolicy, - issue: AuthIssueSession, - privileged: bool, - webauthn: &Webauthn, - ct: Duration, - source: Source, - ) -> (Option, AuthState) { + pub fn new(asd: AuthSessionData<'_>, privileged: bool) -> (Option, AuthState) { // During this setup, determine the credential handler that we'll be using // for this session. This is currently based on presentation of an application // id. - let state = if account.is_within_valid_time(ct) { + let state = if asd.account.is_within_valid_time(asd.ct) { // We want the primary handler - this is where we make a decision // based on the anonymous ... in theory this could be cleaner // and interact with the account more? - if account.is_anonymous() { + if asd.account.is_anonymous() { AuthSessionState::Init(nonempty![CredHandler::Anonymous { - cred_id: account.uuid, + cred_id: asd.account.uuid, }]) } else { // What's valid to use in this context? let mut handlers = Vec::new(); - if let Some(cred) = &account.primary { + if let Some(cred) = &asd.account.primary { // TODO: Make it possible to have multiple creds. // Probably means new authsession has to be failable - if let Ok(ch) = CredHandler::try_from((cred, webauthn)) { + if let Ok(ch) = CredHandler::try_from((cred, asd.webauthn)) { handlers.push(ch); } else { security_critical!( @@ -781,7 +782,7 @@ impl AuthSession { } } - if let Ok(ch) = CredHandler::try_from((&account.passkeys, webauthn)) { + if let Ok(ch) = CredHandler::try_from((&asd.account.passkeys, asd.webauthn)) { handlers.push(ch); }; @@ -804,12 +805,12 @@ impl AuthSession { } else { // We can proceed let auth_session = AuthSession { - account, - account_policy, + account: asd.account, + account_policy: asd.account_policy, state, - issue, + issue: asd.issue, intent: AuthIntent::InitialAuth { privileged }, - source, + source: asd.source, }; // Get the set of mechanisms that can proceed. This is tied // to the session so that it can mutate state and have progression @@ -827,15 +828,10 @@ impl AuthSession { /// will be used in this operation based on the credential id that was used in the /// initial authentication. pub(crate) fn new_reauth( - account: Account, - account_policy: ResolvedAccountPolicy, + asd: AuthSessionData<'_>, session_id: Uuid, session: &Session, cred_id: Uuid, - issue: AuthIssueSession, - webauthn: &Webauthn, - ct: Duration, - source: Source, ) -> (Option, AuthState) { /// An inner enum to allow us to more easily define state within this fn enum State { @@ -844,7 +840,7 @@ impl AuthSession { Proceed(CredHandler), } - let state = if account.is_within_valid_time(ct) { + let state = if asd.account.is_within_valid_time(asd.ct) { // Get the credential that matches this cred_id. // // To make this work "cleanly" we can't really nest a bunch of if @@ -856,9 +852,9 @@ impl AuthSession { let mut cred_handler = None; - if let Some(primary) = account.primary.as_ref() { + if let Some(primary) = asd.account.primary.as_ref() { if primary.uuid == cred_id { - if let Ok(ch) = CredHandler::try_from((primary, webauthn)) { + if let Ok(ch) = CredHandler::try_from((primary, asd.webauthn)) { // Update it. debug_assert!(cred_handler.is_none()); cred_handler = Some(ch); @@ -870,8 +866,8 @@ impl AuthSession { } } - if let Some(pk) = account.passkeys.get(&cred_id).map(|(_, pk)| pk) { - if let Ok(ch) = CredHandler::try_from((cred_id, pk, webauthn)) { + if let Some(pk) = asd.account.passkeys.get(&cred_id).map(|(_, pk)| pk) { + if let Ok(ch) = CredHandler::try_from((cred_id, pk, asd.webauthn)) { // Update it. debug_assert!(cred_handler.is_none()); cred_handler = Some(ch); @@ -906,15 +902,15 @@ impl AuthSession { State::Proceed(handler) => { let allow = handler.next_auth_allowed(); let auth_session = AuthSession { - account, - account_policy, + account: asd.account, + account_policy: asd.account_policy, state: AuthSessionState::InProgress(handler), - issue, + issue: asd.issue, intent: AuthIntent::Reauth { session_id, session_expiry, }, - source, + source: asd.source, }; let as_state = AuthState::Continue(allow); @@ -1260,8 +1256,8 @@ mod tests { use crate::idm::accountpolicy::ResolvedAccountPolicy; use crate::idm::audit::AuditEvent; use crate::idm::authsession::{ - AuthSession, BAD_AUTH_TYPE_MSG, BAD_BACKUPCODE_MSG, BAD_PASSWORD_MSG, BAD_TOTP_MSG, - BAD_WEBAUTHN_MSG, PW_BADLIST_MSG, + AuthSession, AuthSessionData, BAD_AUTH_TYPE_MSG, BAD_BACKUPCODE_MSG, BAD_PASSWORD_MSG, + BAD_TOTP_MSG, BAD_WEBAUTHN_MSG, PW_BADLIST_MSG, }; use crate::idm::delayed::DelayedAction; use crate::idm::AuthState; @@ -1296,16 +1292,15 @@ mod tests { let anon_account: Account = BUILTIN_ACCOUNT_ANONYMOUS_V1.clone().into(); - let (session, state) = AuthSession::new( - anon_account, - ResolvedAccountPolicy::default(), - AuthIssueSession::Token, - false, - &webauthn, - duration_from_epoch_now(), - Source::Internal, - ); - + let asd = AuthSessionData { + account: anon_account, + account_policy: ResolvedAccountPolicy::default(), + issue: AuthIssueSession::Token, + webauthn: &webauthn, + ct: duration_from_epoch_now(), + source: Source::Internal, + }; + let (session, state) = AuthSession::new(asd, false); if let AuthState::Choose(auth_mechs) = state { assert!(auth_mechs.iter().any(|x| matches!(x, AuthMech::Anonymous))); } else { @@ -1333,15 +1328,15 @@ mod tests { $webauthn:expr, $privileged:expr ) => {{ - let (session, state) = AuthSession::new( - $account.clone(), - ResolvedAccountPolicy::default(), - AuthIssueSession::Token, - $privileged, - $webauthn, - duration_from_epoch_now(), - Source::Internal, - ); + let asd = AuthSessionData { + account: $account.clone(), + account_policy: ResolvedAccountPolicy::default(), + issue: AuthIssueSession::Token, + webauthn: $webauthn, + ct: duration_from_epoch_now(), + source: Source::Internal, + }; + let (session, state) = AuthSession::new(asd, $privileged); let mut session = session.unwrap(); if let AuthState::Choose(auth_mechs) = state { @@ -1514,15 +1509,15 @@ mod tests { $account:expr, $webauthn:expr ) => {{ - let (session, state) = AuthSession::new( - $account.clone(), - ResolvedAccountPolicy::default(), - AuthIssueSession::Token, - false, - $webauthn, - duration_from_epoch_now(), - Source::Internal, - ); + let asd = AuthSessionData { + account: $account.clone(), + account_policy: ResolvedAccountPolicy::default(), + issue: AuthIssueSession::Token, + webauthn: $webauthn, + ct: duration_from_epoch_now(), + source: Source::Internal, + }; + let (session, state) = AuthSession::new(asd, false); let mut session = session.expect("Session was unable to be created."); if let AuthState::Choose(auth_mechs) = state { @@ -1842,15 +1837,15 @@ mod tests { $account:expr, $webauthn:expr ) => {{ - let (session, state) = AuthSession::new( - $account.clone(), - ResolvedAccountPolicy::default(), - AuthIssueSession::Token, - false, - $webauthn, - duration_from_epoch_now(), - Source::Internal, - ); + let asd = AuthSessionData { + account: $account.clone(), + account_policy: ResolvedAccountPolicy::default(), + issue: AuthIssueSession::Token, + webauthn: $webauthn, + ct: duration_from_epoch_now(), + source: Source::Internal, + }; + let (session, state) = AuthSession::new(asd, false); let mut session = session.unwrap(); if let AuthState::Choose(auth_mechs) = state { diff --git a/server/lib/src/idm/reauth.rs b/server/lib/src/idm/reauth.rs index 47200e128..4a7a3efdf 100644 --- a/server/lib/src/idm/reauth.rs +++ b/server/lib/src/idm/reauth.rs @@ -2,7 +2,7 @@ use crate::prelude::*; use crate::credential::softlock::CredSoftLock; use crate::idm::account::Account; -use crate::idm::authsession::AuthSession; +use crate::idm::authsession::{AuthSession, AuthSessionData}; use crate::idm::event::AuthResult; use crate::idm::server::IdmServerAuthTransaction; use crate::idm::AuthState; @@ -129,17 +129,16 @@ impl<'a> IdmServerAuthTransaction<'a> { } // Create a re-auth session - let (auth_session, state) = AuthSession::new_reauth( + let asd: AuthSessionData = AuthSessionData { account, account_policy, - ident.session_id, - session, - session_cred_id, issue, - self.webauthn, + webauthn: self.webauthn, ct, source, - ); + }; + let (auth_session, state) = + AuthSession::new_reauth(asd, ident.session_id, session, session_cred_id); // Push the re-auth session to the session maps. match auth_session { diff --git a/server/lib/src/idm/server.rs b/server/lib/src/idm/server.rs index 7cd763d1d..e12cd548b 100644 --- a/server/lib/src/idm/server.rs +++ b/server/lib/src/idm/server.rs @@ -30,7 +30,7 @@ use super::ldap::{LdapBoundToken, LdapSession}; use crate::credential::{softlock::CredSoftLock, Credential}; use crate::idm::account::Account; use crate::idm::audit::AuditEvent; -use crate::idm::authsession::AuthSession; +use crate::idm::authsession::{AuthSession, AuthSessionData}; use crate::idm::credupdatesession::CredentialUpdateSessionMutex; use crate::idm::delayed::{ AuthSessionRecord, BackupCodeRemoval, DelayedAction, PasswordUpgrade, UnixPasswordUpgrade, @@ -1022,15 +1022,16 @@ impl<'a> IdmServerAuthTransaction<'a> { slock_ref }); - let (auth_session, state) = AuthSession::new( + let asd: AuthSessionData = AuthSessionData { account, account_policy, - init.issue, - init.privileged, - self.webauthn, + issue: init.issue, + webauthn: self.webauthn, ct, source, - ); + }; + + let (auth_session, state) = AuthSession::new(asd, init.privileged); match auth_session { Some(auth_session) => { diff --git a/server/lib/src/plugins/default_values.rs b/server/lib/src/plugins/default_values.rs index 9f4f451e2..ec129a2ae 100644 --- a/server/lib/src/plugins/default_values.rs +++ b/server/lib/src/plugins/default_values.rs @@ -58,12 +58,9 @@ impl DefaultValues { // We have to do this rather than get_uuid here because at this stage we haven't // scheme validated the entry so it's uuid could be missing in theory. - let e_uuid = match e.get_ava_single_uuid(Attribute::Uuid) { - Some(e_uuid) => e_uuid, - None => { - trace!("entry does not contain a uuid"); - return Ok(()); - } + let Some(e_uuid) = e.get_ava_single_uuid(Attribute::Uuid) else { + trace!("entry does not contain a uuid"); + return Ok(()); }; if e_uuid == UUID_IDM_ALL_ACCOUNTS { diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 251b8d387..b77a4d5dc 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -1184,8 +1184,12 @@ impl QueryServer { .expect("unable to acquire db_ticket for qsr") }; + #[allow(clippy::expect_used)] QueryServerReadTransaction { - be_txn: self.be.read().unwrap(), + be_txn: self + .be + .read() + .expect("unable to create backend read transaction"), schema: self.schema.read(), d_info: self.d_info.read(), system_config: self.system_config.read(), @@ -1223,8 +1227,13 @@ impl QueryServer { .expect("unable to acquire db_ticket for qsw") }; + #[allow(clippy::expect_used)] + let mut be_txn = self + .be + .write() + .expect("unable to create backend write transaction"); + let schema_write = self.schema.write(); - let mut be_txn = self.be.write().unwrap(); let d_info = self.d_info.write(); let system_config = self.system_config.write(); let phase = self.phase.write(); diff --git a/server/lib/src/value.rs b/server/lib/src/value.rs index ca2d03b16..ebd57b962 100644 --- a/server/lib/src/value.rs +++ b/server/lib/src/value.rs @@ -1695,7 +1695,7 @@ impl Value { Value::Iname(s) => s.clone(), Value::Uuid(u) => u.as_hyphenated().to_string(), // We display the tag and fingerprint. - Value::SshKey(tag, key) => format!("{}: {}", tag, key.to_string()), + Value::SshKey(tag, key) => format!("{}: {}", tag, key), Value::Spn(n, r) => format!("{n}@{r}"), _ => unreachable!( "You've specified the wrong type for the attribute, got: {:?}", diff --git a/server/lib/src/valueset/ssh.rs b/server/lib/src/valueset/ssh.rs index cc6759f15..6eb8bb2f2 100644 --- a/server/lib/src/valueset/ssh.rs +++ b/server/lib/src/valueset/ssh.rs @@ -44,7 +44,7 @@ impl ValueSetSshKey { let map = data .iter() .map(|(tag, data)| { - SshPublicKey::from_string(&data) + SshPublicKey::from_string(data) .map_err(|err| { warn!(%tag, ?err, "discarding corrupted ssh public key"); OperationError::VS0001IncomingReplSshPublicKey diff --git a/tools/cli/src/cli/group/account_policy.rs b/tools/cli/src/cli/group/account_policy.rs index acc5deed1..65926686c 100644 --- a/tools/cli/src/cli/group/account_policy.rs +++ b/tools/cli/src/cli/group/account_policy.rs @@ -14,7 +14,7 @@ impl GroupAccountPolicyOpt { match self { GroupAccountPolicyOpt::Enable { name, copt } => { let client = copt.to_client(OpType::Write).await; - if let Err(e) = client.group_account_policy_enable(&name).await { + if let Err(e) = client.group_account_policy_enable(name).await { handle_client_error(e, &copt.output_mode); } else { println!("Group enabled for account policy."); @@ -23,7 +23,7 @@ impl GroupAccountPolicyOpt { GroupAccountPolicyOpt::AuthSessionExpiry { name, expiry, copt } => { let client = copt.to_client(OpType::Write).await; if let Err(e) = client - .group_account_policy_authsession_expiry_set(&name, *expiry) + .group_account_policy_authsession_expiry_set(name, *expiry) .await { handle_client_error(e, &copt.output_mode); @@ -34,7 +34,7 @@ impl GroupAccountPolicyOpt { GroupAccountPolicyOpt::PrivilegedSessionExpiry { name, expiry, copt } => { let client = copt.to_client(OpType::Write).await; if let Err(e) = client - .group_account_policy_privilege_expiry_set(&name, *expiry) + .group_account_policy_privilege_expiry_set(name, *expiry) .await { handle_client_error(e, &copt.output_mode);