Skip to content

Commit

Permalink
Merge pull request #339 from himmelblau-idm/stable-0.8.x_sfa_auth_fix
Browse files Browse the repository at this point in the history
Stable 0.8.x Entra Id no longer permits SFA enrollment
  • Loading branch information
dmulder authored Jan 8, 2025
2 parents c689c7b + 65060a5 commit 553c632
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 16 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ members = [
resolver = "2"

[workspace.package]
version = "0.8.1"
version = "0.8.2"
authors = [
"David Mulder <[email protected]>"
]
Expand All @@ -40,7 +40,7 @@ tracing-subscriber = "^0.3.17"
tracing = "^0.1.37"
himmelblau_unix_common = { path = "src/common" }
kanidm_unix_common = { path = "src/glue" }
libhimmelblau = { version = "0.5.0" }
libhimmelblau = { version = "0.5.3" }
clap = { version = "^4.5", features = ["derive", "env"] }
clap_complete = "^4.4.1"
reqwest = { version = "^0.12.2", features = ["json"] }
Expand Down
51 changes: 37 additions & 14 deletions src/common/src/idprovider/himmelblau.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,12 @@ impl IdProvider for HimmelblauProvider {
if service != "ssh" {
opts.push(AuthOption::Fido);
}
// If SFA is enabled, disable the DAG fallback, otherwise SFA users
// will always be prompted for DAG.
let sfa_enabled = self.config.read().await.get_enable_sfa_fallback();
if sfa_enabled {
opts.push(AuthOption::NoDAGFallback);
}
let mresp = self
.client
.write()
Expand All @@ -1083,26 +1089,43 @@ impl IdProvider for HimmelblauProvider {
Ok(resp) => resp,
Err(e) => {
// If SFA is disabled, we need to skip the SFA fallback.
let sfa_enabled = self.config.read().await.get_enable_sfa_fallback();
let mtoken = if sfa_enabled {
// If we got an AADSTSError, then we don't want to
// perform a fallback, since the authentication
// legitimately failed.
if let MsalError::AADSTSError(_) = e {
error!("{:?}", e);
if let MsalError::AADSTSError(ref e) = e {
// If the error is just requesting MFA (since we demanded it
// in the previous call), then continue with the SFA fallback.
// AADSTS50072: UserStrongAuthEnrollmentRequiredInterrupt
// AADSTS50074: UserStrongAuthClientAuthNRequiredInterrupt
// AADSTS50076: UserStrongAuthClientAuthNRequired
if ![50072, 50074, 50076].contains(&e.code) {
error!("Skipping SFA fallback because authentication failed: {:?}", e);
return Err(IdpError::BadRequest);
}
}
// We can only do a password auth for an enrolled device
if self.is_domain_joined(keystore).await {
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(
account_id,
&cred,
vec![],
Some("https://graph.microsoft.com".to_string()),
tpm,
machine_key,
)
.await
} else {
error!("Single factor authentication is only permitted on an enrolled host: {:?}", e);
return Err(IdpError::BadRequest);
}
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
} else {
error!("{:?}", e);
return Err(IdpError::BadRequest);
Expand Down

0 comments on commit 553c632

Please sign in to comment.