Skip to content

Commit

Permalink
Merge pull request #149 from himmelblau-idm/dmulder/disable_sfa_fallb…
Browse files Browse the repository at this point in the history
…ack_default

Disable the SFA fallback by default
  • Loading branch information
dmulder authored May 30, 2024
2 parents b2b4640 + a4a85b0 commit 6371e30
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 18 deletions.
11 changes: 9 additions & 2 deletions src/common/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::constants::{
BROKER_APP_ID, DEFAULT_CACHE_TIMEOUT, DEFAULT_CONFIG_PATH, DEFAULT_CONN_TIMEOUT,
DEFAULT_DB_PATH, DEFAULT_HELLO_ENABLED, DEFAULT_HOME_ALIAS, DEFAULT_HOME_ATTR,
DEFAULT_HOME_PREFIX, DEFAULT_HSM_PIN_PATH, DEFAULT_ID_ATTR_MAP, DEFAULT_ODC_PROVIDER,
DEFAULT_SELINUX, DEFAULT_SHELL, DEFAULT_SOCK_PATH, DEFAULT_TASK_SOCK_PATH,
DEFAULT_USE_ETC_SKEL, SERVER_CONFIG_PATH,
DEFAULT_SELINUX, DEFAULT_SFA_FALLBACK_ENABLED, DEFAULT_SHELL, DEFAULT_SOCK_PATH,
DEFAULT_TASK_SOCK_PATH, DEFAULT_USE_ETC_SKEL, SERVER_CONFIG_PATH,
};
use crate::unix_config::{HomeAttr, HsmType};
use idmap::DEFAULT_IDMAP_RANGE;
Expand Down Expand Up @@ -381,6 +381,13 @@ impl HimmelblauConfig {
None => DEFAULT_ID_ATTR_MAP,
}
}

pub fn get_enable_sfa_fallback(&self) -> bool {
match_bool(
self.config.get("global", "enable_sfa_fallback"),
DEFAULT_SFA_FALLBACK_ENABLED,
)
}
}

impl fmt::Debug for HimmelblauConfig {
Expand Down
1 change: 1 addition & 0 deletions src/common/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub const DEFAULT_CACHE_TIMEOUT: u64 = 15;
pub const DEFAULT_SELINUX: bool = true;
pub const DEFAULT_HSM_PIN_PATH: &str = "/var/lib/himmelblaud/hsm-pin";
pub const DEFAULT_HELLO_ENABLED: bool = true;
pub const DEFAULT_SFA_FALLBACK_ENABLED: bool = false;
pub const DEFAULT_ID_ATTR_MAP: IdAttr = IdAttr::Name;
pub const BROKER_APP_ID: &str = "29d9ed98-a469-4536-ade2-f981bc1d605e";
pub const BROKER_CLIENT_IDENT: &str = "38aa3b87-a06d-4817-b275-7a316988d93b";
57 changes: 41 additions & 16 deletions src/common/src/idprovider/himmelblau.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use msal::auth::{
UserToken as UnixUserToken,
};
use msal::discovery::EnrollAttrs;
use msal::error::{MsalError, AUTH_PENDING, DEVICE_AUTH_FAIL, REQUIRES_MFA};
use msal::error::{ErrorResponse, MsalError, AUTH_PENDING, DEVICE_AUTH_FAIL, REQUIRES_MFA};
use msal::graph::{DirectoryObject, Graph};
use reqwest;
use std::collections::HashMap;
Expand Down Expand Up @@ -814,18 +814,35 @@ impl IdProvider for HimmelblauProvider {
let resp = match mresp {
Ok(resp) => resp,
Err(e) => {
warn!("MFA auth failed, falling back to SFA: {:?}", e);
// Again, we need to wait to handle the response until after
// we've released the write lock on the client, otherwise we
// will deadlock.
let mtoken = self
.client
.write()
.await
.acquire_token_by_username_password_for_device_enrollment(
account_id, &cred,
)
.await;
// If SFA is disabled, we need to skip the SFA fallback.
let sfa_enabled = self.config.read().await.get_enable_sfa_fallback();
let mtoken = match sfa_enabled {
true => {
warn!("MFA auth failed, falling back to SFA: {:?}", e);
// Again, we need to wait to handle the response until after
// we've released the write lock on the client, otherwise we
// will deadlock.
self.client
.write()
.await
.acquire_token_by_username_password_for_device_enrollment(
account_id, &cred,
)
.await
}
// If SFA fallback is disabled, set mtoken to an
// MsalError in order to permit DAG fallback. If
// the DAG produces SFA, it will be rejected also.
false => {
error!("{:?}", e);
Err(MsalError::AcquireTokenFailed(ErrorResponse {
error: "SFA Disabled".to_string(),
error_description: "SFA fallback is disabled by configuration"
.to_string(),
error_codes: vec![REQUIRES_MFA],
}))
}
};
let token = match mtoken {
Ok(token) => token,
Err(e) => {
Expand Down Expand Up @@ -955,13 +972,21 @@ impl IdProvider for HimmelblauProvider {
let token2 = enroll_and_obtain_enrolled_token!(token);
match self.token_validate(account_id, &token2).await {
Ok(AuthResult::Success { token: token3 }) => {
// STOP! If the DAG doesn't hold an MFA amr, then we
// need to bail out here and refuse Hello enrollment
// (we can't enroll in Hello with an SFA token).
let mfa = token2.amr_mfa().map_err(|e| {
error!("{:?}", e);
IdpError::NotFound
})?;
// If the DAG didn't obtain an MFA amr, and SFA fallback
// is disabled, we need to reject the authentication
// attempt here.
let sfa_enabled = self.config.read().await.get_enable_sfa_fallback();
if !mfa && !sfa_enabled {
info!("A DAG produced an SFA token, yet SFA fallback is disabled by configuration");
return Ok((AuthResult::Denied, AuthCacheAction::None));
}
// STOP! If the DAG doesn't hold an MFA amr, then we
// need to bail out here and refuse Hello enrollment
// (we can't enroll in Hello with an SFA token).
// Also skip Hello enrollment if it is disabled by config
let hello_enabled = self.config.read().await.get_enable_hello();
if !mfa || !hello_enabled {
Expand Down
5 changes: 5 additions & 0 deletions src/config/himmelblau.conf.example
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
# when the host is public facing (such as via SSH).
# enable_hello = true
#
# Whether to permit attempting a SFA (password only) authentication when MFA
# methods are unavailable. Sometimes this is possible when MFA has yet to be
# configured. This is disabled by default.
# enable_sfa_fallback = false
#
# authority_host = login.microsoftonline.com
#
# The location of the cache database
Expand Down

0 comments on commit 6371e30

Please sign in to comment.