From ac793c711a651b199b92ad0c2027a548d11524bf Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Tue, 24 Dec 2024 16:24:11 +0100 Subject: [PATCH] Fix MFA codes (#881) --- .github/workflows/build-docker.yml | 69 +++++++++++---------- .github/workflows/current.yml | 8 +-- Cargo.toml | 2 + proto | 2 +- src/db/models/user.rs | 38 ++++++++---- src/enterprise/directory_sync/google.rs | 26 ++++---- src/enterprise/directory_sync/mod.rs | 79 ++++++++++++------------- src/enterprise/license.rs | 53 ++++++++--------- src/enterprise/mod.rs | 21 +++---- src/grpc/desktop_client_mfa.rs | 10 +++- src/grpc/enrollment.rs | 16 +++-- src/grpc/interceptor.rs | 3 +- src/grpc/mod.rs | 16 +++-- src/grpc/password_reset.rs | 8 +-- src/handlers/network_devices.rs | 44 ++++++++------ src/handlers/openid_flow.rs | 2 +- src/handlers/ssh_authorized_keys.rs | 39 ++++++------ 17 files changed, 222 insertions(+), 214 deletions(-) diff --git a/.github/workflows/build-docker.yml b/.github/workflows/build-docker.yml index 02800e8cb..172e2821f 100644 --- a/.github/workflows/build-docker.yml +++ b/.github/workflows/build-docker.yml @@ -23,18 +23,17 @@ jobs: - ${{ matrix.runner }} strategy: matrix: - # cpu: [arm64, amd64, arm/v7] - cpu: [amd64] + cpu: [arm64, amd64, arm/v7] include: - # - cpu: arm64 - # runner: ARM64 - # tag: arm64 + - cpu: arm64 + runner: ARM64 + tag: arm64 - cpu: amd64 runner: X64 tag: amd64 - # - cpu: arm/v7 - # runner: ARM - # tag: armv7 + - cpu: arm/v7 + runner: ARM + tag: armv7 steps: - name: Checkout uses: actions/checkout@v4 @@ -63,30 +62,30 @@ jobs: cache-from: type=gha cache-to: type=gha,mode=max - # docker-manifest: - # runs-on: [self-hosted, Linux] - # needs: [build-docker] - # steps: - # - name: Docker meta - # id: meta - # uses: docker/metadata-action@v5 - # with: - # images: | - # ${{ env.GHCR_REPO }} - # flavor: ${{ inputs.flavor }} - # tags: ${{ inputs.tags }} - # - name: Login to GitHub container registry - # uses: docker/login-action@v3 - # with: - # registry: ghcr.io - # username: ${{ github.actor }} - # password: ${{ secrets.GITHUB_TOKEN }} - # - name: Create and push manifests - # run: | - # tags='${{ env.GHCR_REPO }}:${{ github.sha }} ${{ steps.meta.outputs.tags }}' - # for tag in ${tags} - # do - # docker manifest rm ${tag} || true - # docker manifest create ${tag} ${{ env.GHCR_REPO }}:${{ github.sha }}-amd64 ${{ env.GHCR_REPO }}:${{ github.sha }}-arm64 ${{ env.GHCR_REPO }}:${{ github.sha }}-armv7 - # docker manifest push ${tag} - # done + docker-manifest: + runs-on: [self-hosted, Linux] + needs: [build-docker] + steps: + - name: Docker meta + id: meta + uses: docker/metadata-action@v5 + with: + images: | + ${{ env.GHCR_REPO }} + flavor: ${{ inputs.flavor }} + tags: ${{ inputs.tags }} + - name: Login to GitHub container registry + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Create and push manifests + run: | + tags='${{ env.GHCR_REPO }}:${{ github.sha }} ${{ steps.meta.outputs.tags }}' + for tag in ${tags} + do + docker manifest rm ${tag} || true + docker manifest create ${tag} ${{ env.GHCR_REPO }}:${{ github.sha }}-amd64 ${{ env.GHCR_REPO }}:${{ github.sha }}-arm64 ${{ env.GHCR_REPO }}:${{ github.sha }}-armv7 + docker manifest push ${tag} + done diff --git a/.github/workflows/current.yml b/.github/workflows/current.yml index 9506db558..8fe01d886 100644 --- a/.github/workflows/current.yml +++ b/.github/workflows/current.yml @@ -20,7 +20,7 @@ jobs: type=ref,event=branch type=sha - # trigger-e2e: - # needs: build-current - # uses: ./.github/workflows/e2e.yml - # secrets: inherit + trigger-e2e: + needs: build-current + uses: ./.github/workflows/e2e.yml + secrets: inherit diff --git a/Cargo.toml b/Cargo.toml index 64160ad6e..6126e2fa1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,5 +135,7 @@ worker = [] wireguard = [] [profile.release] +codegen-units = 1 +panic = "abort" lto = "thin" strip = "symbols" diff --git a/proto b/proto index b9adb0bc8..41fcdbd92 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit b9adb0bc87228c88c42f144caa47c8b69a3fb98c +Subproject commit 41fcdbd92704f1770a7f4d43d34222cd00da21c6 diff --git a/src/db/models/user.rs b/src/db/models/user.rs index b490b952f..3677031ea 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -137,15 +137,12 @@ impl User { } pub(crate) fn verify_password(&self, password: &str) -> Result<(), HashError> { - match &self.password_hash { - Some(hash) => { - let parsed_hash = PasswordHash::new(hash)?; - Argon2::default().verify_password(password.as_bytes(), &parsed_hash) - } - None => { - error!("Password not set for user {}", self.username); - Err(HashError::Password) - } + if let Some(hash) = &self.password_hash { + let parsed_hash = PasswordHash::new(hash)?; + Argon2::default().verify_password(password.as_bytes(), &parsed_hash) + } else { + error!("Password not set for user {}", self.username); + Err(HashError::Password) } } @@ -570,6 +567,12 @@ impl User { if code == expected_code { return true; } + debug!( + "Email MFA verification TOTP code for user {} doesn't fit current time + frame, checking the previous one. + Expected: {expected_code}, got: {code}", + self.username + ); let previous_code = totp_custom::( timeout, @@ -577,8 +580,23 @@ impl User { email_mfa_secret, timestamp.as_secs() - timeout, ); - return code == previous_code; + + if code == previous_code { + return true; + } + debug!( + "Email MFA verification TOTP code for user {} doesn't fit previous time frame, + expected: {previous_code}, got: {code}", + self.username + ); + return false; } + debug!( + "Couldn't calculate current timestamp when verifying email MFA code for user {}", + self.username + ); + } else { + debug!("Email MFA secret not configured for user {}", self.username); } false } diff --git a/src/enterprise/directory_sync/google.rs b/src/enterprise/directory_sync/google.rs index 58cba5f39..6a393c886 100644 --- a/src/enterprise/directory_sync/google.rs +++ b/src/enterprise/directory_sync/google.rs @@ -3,7 +3,7 @@ use std::{str::FromStr, time::Duration}; use super::{DirectoryGroup, DirectorySync, DirectorySyncError, DirectoryUser}; use chrono::Utc; use jsonwebtoken::{encode, Algorithm, EncodingKey, Header}; -use reqwest::Url; +use reqwest::{header::AUTHORIZATION, Url}; const SCOPES: &str = "openid email profile https://www.googleapis.com/auth/admin.directory.customer.readonly https://www.googleapis.com/auth/admin.directory.group.readonly https://www.googleapis.com/auth/admin.directory.user.readonly"; const ACCESS_TOKEN_URL: &str = "https://oauth2.googleapis.com/token"; @@ -24,6 +24,7 @@ struct Claims { } impl Claims { + #[must_use] fn new(iss: &str, sub: &str) -> Self { let now = chrono::Utc::now(); let now_timestamp = now.timestamp(); @@ -45,7 +46,6 @@ pub struct ServiceAccountConfig { client_email: String, } -#[derive(Debug)] pub struct GoogleDirectorySync { service_account_config: ServiceAccountConfig, access_token: Option, @@ -122,6 +122,7 @@ where } impl GoogleDirectorySync { + #[must_use] pub fn new(private_key: &str, client_email: &str, admin_email: &str) -> Self { Self { service_account_config: ServiceAccountConfig { @@ -144,10 +145,8 @@ impl GoogleDirectorySync { pub fn is_token_expired(&self) -> bool { debug!("Checking if Google directory sync token is expired"); - self.token_expiry - .map(|expiry| expiry < Utc::now()) - // No token = expired token - .unwrap_or(true) + // No token = expired token + self.token_expiry.map_or(true, |expiry| expiry < Utc::now()) } #[cfg(not(test))] @@ -169,10 +168,7 @@ impl GoogleDirectorySync { let client = reqwest::Client::new(); let response = client .get(url) - .header( - reqwest::header::AUTHORIZATION, - format!("Bearer {}", access_token), - ) + .header(AUTHORIZATION, format!("Bearer {access_token}")) .timeout(REQUEST_TIMEOUT) .send() .await?; @@ -198,7 +194,7 @@ impl GoogleDirectorySync { let client = reqwest::Client::builder().build()?; let response = client .get(url) - .header("Authorization", format!("Bearer {}", access_token)) + .header(AUTHORIZATION, format!("Bearer {access_token}")) .timeout(REQUEST_TIMEOUT) .send() .await?; @@ -229,7 +225,7 @@ impl GoogleDirectorySync { let client = reqwest::Client::builder().build()?; let response = client .get(url) - .header("Authorization", format!("Bearer {}", access_token)) + .header(AUTHORIZATION, format!("Bearer {access_token}")) .timeout(REQUEST_TIMEOUT) .send() .await?; @@ -276,7 +272,7 @@ impl GoogleDirectorySync { let client = reqwest::Client::builder().build()?; let response = client .get(url) - .header("Authorization", format!("Bearer {}", access_token)) + .header(AUTHORIZATION, format!("Bearer {access_token}")) .timeout(REQUEST_TIMEOUT) .send() .await?; @@ -296,7 +292,7 @@ impl GoogleDirectorySync { let client = reqwest::Client::builder().build()?; let result = client .get(url) - .header("Authorization", format!("Bearer {}", access_token)) + .header(AUTHORIZATION, format!("Bearer {access_token}")) .timeout(REQUEST_TIMEOUT) .send() .await?; @@ -359,7 +355,7 @@ impl DirectorySync for GoogleDirectorySync { debug!("Getting all users"); let response = self.query_all_users().await?; debug!("Got all users response"); - Ok(response.users.into_iter().map(|u| u.into()).collect()) + Ok(response.users.into_iter().map(Into::into).collect()) } async fn test_connection(&self) -> Result<(), DirectorySyncError> { diff --git a/src/enterprise/directory_sync/mod.rs b/src/enterprise/directory_sync/mod.rs index c55d5c6ba..39b03f27a 100644 --- a/src/enterprise/directory_sync/mod.rs +++ b/src/enterprise/directory_sync/mod.rs @@ -175,7 +175,7 @@ pub(crate) async fn sync_user_groups_if_configured( } let provider = OpenIdProvider::get_current(pool).await?; - if !is_directory_sync_enabled(provider.as_ref()).await? { + if !is_directory_sync_enabled(provider.as_ref()) { debug!("Directory sync is disabled, skipping syncing user groups"); return Ok(()); } @@ -294,17 +294,17 @@ async fn sync_all_users_groups( user.email, current_group.name ); continue; - } else { - debug!( + } + debug!( "Removing user {} from group {} as they are not a member of it in the directory", user.email, current_group.name ); - user.remove_from_group(&mut *transaction, current_group) - .await?; - admin_count -= 1; - } + user.remove_from_group(&mut *transaction, current_group) + .await?; + admin_count -= 1; } else { - debug!("Removing user {} from group {} as they are not a member of it in the directory", user.email, current_group.name); + debug!("Removing user {} from group {} as they are not a member of it in the directory", + user.email, current_group.name); user.remove_from_group(&mut *transaction, current_group) .await?; } @@ -352,19 +352,17 @@ async fn get_directory_sync_client( } } -async fn is_directory_sync_enabled( - provider: Option<&OpenIdProvider>, -) -> Result { +fn is_directory_sync_enabled(provider: Option<&OpenIdProvider>) -> bool { debug!("Checking if directory sync is enabled"); if let Some(provider_settings) = provider { debug!( "Directory sync enabled state: {}", provider_settings.directory_sync_enabled ); - Ok(provider_settings.directory_sync_enabled) + provider_settings.directory_sync_enabled } else { debug!("No openid provider found, directory sync is disabled"); - Ok(false) + false } } @@ -436,8 +434,8 @@ async fn sync_all_users_state( ); let mut admin_count = User::find_admins(&mut *transaction).await?.len(); for mut user in missing_users { - match user.is_admin(&mut *transaction).await? { - true => match admin_behavior { + if user.is_admin(&mut *transaction).await? { + match admin_behavior { DirectorySyncUserBehavior::Keep => { debug!( "Keeping admin {} despite not being present in the directory", @@ -448,19 +446,19 @@ async fn sync_all_users_state( if user.is_active { if admin_count == 1 { error!( - "Admin {} is the last admin in the system, can't disable them", + "Admin {} is the last admin in the system; can't disable", user.email ); continue; - } else { - info!( - "Disabling admin {} because they are not present in the directory and the admin behavior setting is set to disable", + } + info!( + "Disabling admin {} because it is not present in the directory and + the admin behavior setting is set to disable", user.email ); - user.is_active = false; - user.save(&mut *transaction).await?; - admin_count -= 1; - } + user.is_active = false; + user.save(&mut *transaction).await?; + admin_count -= 1; } else { debug!( "Admin {} is already disabled in Defguard, skipping", @@ -475,17 +473,17 @@ async fn sync_all_users_state( user.email ); continue; - } else { - info!( - "Deleting admin {} because they are not present in the directory", - user.email - ); - user.delete(&mut *transaction).await?; - admin_count -= 1; } + info!( + "Deleting admin {} because they are not present in the directory", + user.email + ); + user.delete(&mut *transaction).await?; + admin_count -= 1; } - }, - false => match user_behavior { + } + } else { + match user_behavior { DirectorySyncUserBehavior::Keep => { debug!( "Keeping user {} despite not being present in the directory", @@ -514,7 +512,7 @@ async fn sync_all_users_state( ); user.delete(&mut *transaction).await?; } - }, + } } } debug!("Done processing missing users"); @@ -527,14 +525,13 @@ async fn sync_all_users_state( if user.is_active { debug!("User {} is already enabled, skipping", user.email); continue; - } else { - debug!( - "Enabling user {} because they are enabled in the directory and disabled in Defguard", - user.email - ); - user.is_active = true; - user.save(&mut *transaction).await?; } + debug!( + "Enabling user {} because it is enabled in the directory and disabled in Defguard", + user.email + ); + user.is_active = true; + user.save(&mut *transaction).await?; } debug!("Done processing enabled users"); transaction.commit().await?; @@ -566,7 +563,7 @@ pub(crate) async fn do_directory_sync(pool: &PgPool) -> Result<(), DirectorySync // TODO: The settings are retrieved many times let provider = OpenIdProvider::get_current(pool).await?; - if !is_directory_sync_enabled(provider.as_ref()).await? { + if !is_directory_sync_enabled(provider.as_ref()) { debug!("Directory sync is disabled, skipping performing directory sync"); return Ok(()); } diff --git a/src/enterprise/license.rs b/src/enterprise/license.rs index 077ef2e59..14accee02 100644 --- a/src/enterprise/license.rs +++ b/src/enterprise/license.rs @@ -421,11 +421,11 @@ impl License { /// Checks if the license has reached its maximum overdue time. #[must_use] pub fn is_max_overdue(&self) -> bool { - if !self.subscription { + if self.subscription { + self.time_overdue() > MAX_OVERDUE_TIME + } else { // Non-subscription licenses are considered expired immediately, no grace period is required self.is_expired() - } else { - self.time_overdue() > MAX_OVERDUE_TIME } } } @@ -561,36 +561,31 @@ pub async fn run_periodic_license_check(pool: &PgPool) -> Result<(), LicenseErro let license = get_cached_license(); debug!("Checking if the license {license:?} requires a renewal..."); - match &*license { - Some(license) => { - if license.requires_renewal() { - // check if we are pass the maximum expiration date, after which we don't - // want to try to renew the license anymore - if license.is_max_overdue() { - check_period = *config.check_period; - warn!("Your license has expired and reached its maximum overdue date, please contact sales at salesdefguard.net"); - debug!("Changing check period to {}", format_duration(check_period)); - false - } else { - debug!("License requires renewal, as it is about to expire and is not past the maximum overdue time"); - true - } - } else { - // This if is only for logging purposes, to provide more detailed information - if license.subscription { - debug!( - "License doesn't need to be renewed yet, skipping renewal check" - ); - } else { - debug!("License is not a subscription, skipping renewal check"); - } + if let Some(license) = &*license { + if license.requires_renewal() { + // check if we are pass the maximum expiration date, after which we don't + // want to try to renew the license anymore + if license.is_max_overdue() { + check_period = *config.check_period; + warn!("Your license has expired and reached its maximum overdue date, please contact sales at salesdefguard.net"); + debug!("Changing check period to {}", format_duration(check_period)); false + } else { + debug!("License requires renewal, as it is about to expire and is not past the maximum overdue time"); + true + } + } else { + // This if is only for logging purposes, to provide more detailed information + if license.subscription { + debug!("License doesn't need to be renewed yet, skipping renewal check"); + } else { + debug!("License is not a subscription, skipping renewal check"); } - } - None => { - debug!("No license found, skipping license check"); false } + } else { + debug!("No license found, skipping license check"); + false } }; diff --git a/src/enterprise/mod.rs b/src/enterprise/mod.rs index 1a546395b..025019e66 100644 --- a/src/enterprise/mod.rs +++ b/src/enterprise/mod.rs @@ -13,17 +13,14 @@ pub(crate) fn needs_enterprise_license() -> bool { pub(crate) fn is_enterprise_enabled() -> bool { debug!("Checking if enterprise is enabled"); - match needs_enterprise_license() { - true => { - debug!("User is over limit, checking his license"); - let license = get_cached_license(); - let validation_result = validate_license(license.as_ref()); - debug!("License validation result: {:?}", validation_result); - validation_result.is_ok() - } - false => { - debug!("User is not over limit, allowing enterprise features"); - true - } + if needs_enterprise_license() { + debug!("User is over limit, checking his license"); + let license = get_cached_license(); + let validation_result = validate_license(license.as_ref()); + debug!("License validation result: {:?}", validation_result); + validation_result.is_ok() + } else { + debug!("User is not over limit, allowing enterprise features"); + true } } diff --git a/src/grpc/desktop_client_mfa.rs b/src/grpc/desktop_client_mfa.rs index 0dfcb2a56..434f01bc0 100644 --- a/src/grpc/desktop_client_mfa.rs +++ b/src/grpc/desktop_client_mfa.rs @@ -92,7 +92,7 @@ impl ClientMfaServer { }; // fetch user - let Ok(Some(user)) = User::find_by_id(&self.pool, device.user_id).await else { + let Ok(Some(mut user)) = User::find_by_id(&self.pool, device.user_id).await else { error!("Failed to find user with ID {}", device.user_id); return Err(Status::invalid_argument("user not found")); }; @@ -128,6 +128,14 @@ impl ClientMfaServer { } } + user.verify_mfa_state(&self.pool).await.map_err(|err| { + error!( + "Failed to verify MFA state for user {}: {err:?}", + user.username + ); + Status::internal("unexpected error") + })?; + // check if selected method is enabled let method = MfaMethod::try_from(request.method).map_err(|err| { error!("Invalid MFA method selected ({}): {err}", request.method); diff --git a/src/grpc/enrollment.rs b/src/grpc/enrollment.rs index cd259aa2f..3ab0796ba 100644 --- a/src/grpc/enrollment.rs +++ b/src/grpc/enrollment.rs @@ -55,7 +55,7 @@ impl EnrollmentServer { } /// Checks if token provided with request corresponds to a valid enrollment session - async fn validate_session(&self, token: &Option) -> Result { + async fn validate_session(&self, token: Option<&String>) -> Result { info!("Validating enrollment session. Token: {token:?}"); let Some(token) = token else { error!("Missing authorization header in request"); @@ -63,10 +63,10 @@ impl EnrollmentServer { }; let enrollment = Token::find_by_id(&self.pool, token).await?; debug!("Found matching token, verifying validity: {enrollment:?}."); - if !enrollment + if enrollment .token_type .as_ref() - .is_some_and(|token_type| token_type == ENROLLMENT_TOKEN_TYPE) + .is_none_or(|token_type| token_type != ENROLLMENT_TOKEN_TYPE) { error!( "Invalid token type used in enrollment process: {:?}", @@ -244,7 +244,7 @@ impl EnrollmentServer { req_device_info: Option, ) -> Result<(), Status> { debug!("Activating user account: {request:?}"); - let enrollment = self.validate_session(&request.token).await?; + let enrollment = self.validate_session(request.token.as_ref()).await?; let ip_address; let device_info; @@ -365,7 +365,7 @@ impl EnrollmentServer { req_device_info: Option, ) -> Result { debug!("Adding new user device: {request:?}"); - let enrollment_token = self.validate_session(&request.token).await?; + let enrollment_token = self.validate_session(request.token.as_ref()).await?; // fetch related users let user = enrollment_token.fetch_user(&self.pool).await?; @@ -503,9 +503,7 @@ impl EnrollmentServer { Status::internal("unexpected error") })?; - let network = if let Some(network) = networks.pop() { - network - } else { + let Some(network) = networks.pop() else { error!( "Network device {} added by user {}({:?}) is not assigned to any networks. Aborting partial device configuration process.", device.name, user.username, user.id @@ -690,7 +688,7 @@ impl EnrollmentServer { request: ExistingDevice, ) -> Result { debug!("Getting network info for device: {:?}", request.pubkey); - let _token = self.validate_session(&request.token).await?; + let _token = self.validate_session(request.token.as_ref()).await?; Device::validate_pubkey(&request.pubkey).map_err(|_| { error!("Invalid pubkey {}", &request.pubkey); diff --git a/src/grpc/interceptor.rs b/src/grpc/interceptor.rs index 4b8bceef5..644cc1147 100644 --- a/src/grpc/interceptor.rs +++ b/src/grpc/interceptor.rs @@ -22,8 +22,7 @@ impl Interceptor for JwtInterceptor { let hostname = req .metadata() .get("hostname") - .map(|h| h.to_str().unwrap_or("UNKNOWN")) - .unwrap_or("UNKNOWN"); + .map_or("UNKNOWN", |h| h.to_str().unwrap_or("UNKNOWN")); let token = match req.metadata().get("authorization") { Some(token) => token.to_str().map_err(|err| { diff --git a/src/grpc/mod.rs b/src/grpc/mod.rs index 1cc66eb91..1684837c6 100644 --- a/src/grpc/mod.rs +++ b/src/grpc/mod.rs @@ -527,17 +527,15 @@ pub async fn run_grpc_bidi_stream( Some(core_response::Payload::InstanceInfo(response_payload)) } Err(err) => { - match err.code() { + if Code::FailedPrecondition == err.code() { // Ignore the case when we are not enterprise but the client is trying to fetch the instance config, // to avoid spamming the logs with misleading errors. - Code::FailedPrecondition => { - debug!("A client tried to fetch the instance config, but we are not enterprise."); - Some(core_response::Payload::CoreError(err.into())) - } - _ => { - error!("Instance info error {err}"); - Some(core_response::Payload::CoreError(err.into())) - } + + debug!("A client tried to fetch the instance config, but we are not enterprise."); + Some(core_response::Payload::CoreError(err.into())) + } else { + error!("Instance info error {err}"); + Some(core_response::Payload::CoreError(err.into())) } } } diff --git a/src/grpc/password_reset.rs b/src/grpc/password_reset.rs index b61a51790..4882f5d5d 100644 --- a/src/grpc/password_reset.rs +++ b/src/grpc/password_reset.rs @@ -39,7 +39,7 @@ impl PasswordResetServer { } /// Checks if token provided with request corresponds to a valid password reset session - async fn validate_session(&self, token: &Option) -> Result { + async fn validate_session(&self, token: Option<&String>) -> Result { info!("Validating password reset session. Token: {token:?}"); let Some(token) = token else { error!("Missing authorization header in request"); @@ -47,10 +47,10 @@ impl PasswordResetServer { }; let enrollment = Token::find_by_id(&self.pool, token).await?; debug!("Found matching token, verifying validity: {enrollment:?}."); - if !enrollment + if enrollment .token_type .as_ref() - .is_some_and(|token_type| token_type == PASSWORD_RESET_TOKEN_TYPE) + .is_none_or(|token_type| token_type != PASSWORD_RESET_TOKEN_TYPE) { error!( "Invalid token type used in password reset process: {:?}", @@ -211,7 +211,7 @@ impl PasswordResetServer { req_device_info: Option, ) -> Result<(), Status> { debug!("Starting password reset: {request:?}"); - let enrollment = self.validate_session(&request.token).await?; + let enrollment = self.validate_session(request.token.as_ref()).await?; let ip_address; let user_agent; diff --git a/src/handlers/network_devices.rs b/src/handlers/network_devices.rs index 5ccf00ae5..49af8d2d6 100644 --- a/src/handlers/network_devices.rs +++ b/src/handlers/network_devices.rs @@ -159,10 +159,11 @@ pub(crate) async fn list_network_devices( Ok(device_info) => { devices_response.push(device_info); } - Err(e) => { + Err(err) => { error!( - "Failed to get network information for network device. This device will not be displayed. Error details: {e}" - ) + "Failed to get network information for network device. This device will not be + displayed. Error details: {err}" + ); } } } @@ -488,15 +489,17 @@ pub(crate) async fn start_network_device_setup_for_device( .await? .ok_or_else(|| { WebError::BadRequest(format!( - "Failed to start network device setup for device with ID {}, device not found", - device_id + "Failed to start network device setup for device with ID {device_id}, + device not found" )) })?; if device.device_type != DeviceType::Network { - return Err(WebError::BadRequest( - format!("Failed to start network device setup for a choosen device {}, device is not a network device, device type: {:?}", device.name, device.device_type) - )); + return Err(WebError::BadRequest(format!( + "Failed to start network device setup for a choosen device {}, + device is not a network device, device type: {:?}", + device.name, device.device_type + ))); } let mut transaction = appstate.pool.begin().await?; @@ -504,8 +507,8 @@ pub(crate) async fn start_network_device_setup_for_device( .await? .ok_or_else(|| { WebError::BadRequest(format!( - "Failed to start network device setup for device with ID {}, user which added the device not found", - device_id + "Failed to start network device setup for device with ID {device_id}, + user which added the device not found" )) })?; let config = server_config(); @@ -524,11 +527,15 @@ pub(crate) async fn start_network_device_setup_for_device( transaction.commit().await?; debug!( - "Generated a new device CLI configuration token for already existing network device {} with ID {}: {configuration_token}", + "Generated a new device CLI configuration token for already existing network + device {} with ID {}: {configuration_token}", device.name, device.id ); Ok(ApiResponse { - json: json!({"enrollment_token": configuration_token, "enrollment_url": config.enrollment_url.to_string()}), + json: json!({ + "enrollment_token": configuration_token, + "enrollment_url": config.enrollment_url.to_string() + }), status: StatusCode::CREATED, }) } @@ -717,7 +724,7 @@ fn split_ip(ip: &IpAddr, network: &IpNetwork) -> (String, String, String) { let network_prefix = network.prefix(); let ip_segments = match ip { - IpAddr::V4(ip) => ip.octets().iter().map(|x| *x as u16).collect(), + IpAddr::V4(ip) => ip.octets().iter().map(|x| u16::from(*x)).collect(), IpAddr::V6(ip) => ip.segments().to_vec(), }; @@ -725,7 +732,7 @@ fn split_ip(ip: &IpAddr, network: &IpNetwork) -> (String, String, String) { IpNetwork::V4(net) => { let last_ip = u32::from(net.ip()) | (!u32::from(net.mask())); let last_ip: Ipv4Addr = last_ip.into(); - last_ip.octets().iter().map(|x| *x as u16).collect() + last_ip.octets().iter().map(|x| u16::from(*x)).collect() } IpNetwork::V6(net) => { let last_ip = u128::from(net.ip()) | (!u128::from(net.mask())); @@ -735,7 +742,7 @@ fn split_ip(ip: &IpAddr, network: &IpNetwork) -> (String, String, String) { }; let network_segments = match network_addr { - IpAddr::V4(ip) => ip.octets().iter().map(|x| *x as u16).collect(), + IpAddr::V4(ip) => ip.octets().iter().map(|x| u16::from(*x)).collect(), IpAddr::V6(ip) => ip.segments().to_vec(), }; @@ -746,7 +753,7 @@ fn split_ip(ip: &IpAddr, network: &IpNetwork) -> (String, String, String) { if ip.is_ipv4() { x.to_string() } else { - format!("{:04x}", x) + format!("{x:04x}") } }; @@ -765,10 +772,9 @@ fn split_ip(ip: &IpAddr, network: &IpNetwork) -> (String, String, String) { .join(delimiter); modifiable_part.push_str(&joined); break; - } else { - let formatted = formatter(ip_segment); - network_part.push_str(&format!("{formatted}{delimiter}")); } + let formatted = formatter(ip_segment); + network_part.push_str(&format!("{formatted}{delimiter}")); } (network_part, modifiable_part, network_prefix.to_string()) diff --git a/src/handlers/openid_flow.rs b/src/handlers/openid_flow.rs index e87df21d6..366a2d7cd 100644 --- a/src/handlers/openid_flow.rs +++ b/src/handlers/openid_flow.rs @@ -160,7 +160,7 @@ impl Serialize for FieldResponseTypes { struct FieldResponseTypesVisitor; -impl<'de> Visitor<'de> for FieldResponseTypesVisitor { +impl Visitor<'_> for FieldResponseTypesVisitor { type Value = FieldResponseTypes; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { diff --git a/src/handlers/ssh_authorized_keys.rs b/src/handlers/ssh_authorized_keys.rs index 9db9cfabe..ef8075965 100644 --- a/src/handlers/ssh_authorized_keys.rs +++ b/src/handlers/ssh_authorized_keys.rs @@ -102,31 +102,26 @@ pub async fn get_authorized_keys( // fetch group if let Some(group) = Group::find_by_name(&appstate.pool, group_name).await? { // check if user filter was specified - match ¶ms.username { - Some(username) => { - debug!("Fetching SSH keys for user {username} in group {group_name}"); - // fetch user - if let Some(user) = User::find_by_username(&appstate.pool, username).await? - { - // check if user belongs to specified group - let members = group.member_usernames(&appstate.pool).await?; - if members.contains(&user.username) { - add_user_ssh_keys_to_list(&appstate.pool, &user, &mut ssh_keys) - .await; - } else { - debug!("User {username} is not a member of group {group_name}",); - } + if let Some(username) = ¶ms.username { + debug!("Fetching SSH keys for user {username} in group {group_name}"); + // fetch user + if let Some(user) = User::find_by_username(&appstate.pool, username).await? { + // check if user belongs to specified group + let members = group.member_usernames(&appstate.pool).await?; + if members.contains(&user.username) { + add_user_ssh_keys_to_list(&appstate.pool, &user, &mut ssh_keys).await; } else { - debug!("Specified user does not exist"); + debug!("User {username} is not a member of group {group_name}",); } + } else { + debug!("Specified user does not exist"); } - None => { - debug!("Fetching SSH keys for all users in group {group_name}"); - // fetch all users in group - let users = group.members(&appstate.pool).await?; - for user in users { - add_user_ssh_keys_to_list(&appstate.pool, &user, &mut ssh_keys).await; - } + } else { + debug!("Fetching SSH keys for all users in group {group_name}"); + // fetch all users in group + let users = group.members(&appstate.pool).await?; + for user in users { + add_user_ssh_keys_to_list(&appstate.pool, &user, &mut ssh_keys).await; } } } else {