Skip to content

fix: expand AD user name filter #737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 3, 2025
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ All notable changes to this project will be documented in this file.
- The `runAsUser` and `runAsGroup` fields will not be set anymore by the operator
- The defaults from the docker images itself will now apply, which will be different from 1000/0 going forward
- This is marked as breaking because tools and policies might exist, which require these fields to be set
- user-info-fetcher: the AD backend now uses the Kerberos realm to expand the user search filter ([#737])

### Fixed

Expand All @@ -60,6 +61,7 @@ All notable changes to this project will be documented in this file.
[#723]: https://github.com/stackabletech/opa-operator/pull/723
[#727]: https://github.com/stackabletech/opa-operator/pull/727
[#732]: https://github.com/stackabletech/opa-operator/pull/732
[#737]: https://github.com/stackabletech/opa-operator/pull/737

## [25.3.0] - 2025-03-21

Expand Down
21 changes: 20 additions & 1 deletion rust/operator-binary/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,14 @@ fn build_server_rolegroup_daemonset(
cb_user_info_fetcher
.image_from_product_image(resolved_product_image) // inherit the pull policy and pull secrets, and then...
.image(user_info_fetcher_image) // ...override the image
.command(vec!["stackable-opa-user-info-fetcher".to_string()])
.command(vec![
"/bin/bash".to_string(),
"-x".to_string(),
"-euo".to_string(),
"pipefail".to_string(),
"-c".to_string(),
])
.args(vec![build_user_info_fetcher_start_command()])
.add_env_var("CONFIG", format!("{CONFIG_DIR}/user-info-fetcher.json"))
.add_env_var("CREDENTIALS_DIR", USER_INFO_FETCHER_CREDENTIALS_DIR)
.add_volume_mount(CONFIG_VOLUME_NAME, CONFIG_DIR)
Expand Down Expand Up @@ -1340,3 +1347,15 @@ pub fn build_recommended_labels<'a, T>(
role_group,
}
}

/// Builds the command to start the user info fetcher.
/// When using the Active Directory backend, the KERBEROS_REALM is derived from the krb5.conf file.
/// This is later used for the LDAP user search filter.
fn build_user_info_fetcher_start_command() -> String {
formatdoc! {"
if [ -f {USER_INFO_FETCHER_KERBEROS_DIR}/krb5.conf ]; then
export KERBEROS_REALM=$(grep -oP 'default_realm = \\K.*' {USER_INFO_FETCHER_KERBEROS_DIR}/krb5.conf)
fi
/sbin/stackable-opa-user-info-fetcher
"}
}
22 changes: 19 additions & 3 deletions rust/user-info-fetcher/src/backend/active_directory.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{
collections::{BTreeMap, HashMap},
env,
fmt::{Display, Write},
io::{Cursor, Read},
num::ParseIntError,
Expand Down Expand Up @@ -58,6 +59,9 @@ pub enum Error {
source: ParseSecurityIdError,
user_dn: String,
},

#[snafu(display("environment variable KERBEROS_REALM is not set"))]
KerberosRealmEnvVar { source: env::VarError },
}

impl http_error::Error for Error {
Expand All @@ -75,6 +79,7 @@ impl http_error::Error for Error {
Error::InvalidPrimaryGroupRelativeId { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Error::UserSidHasNoSubauthorities { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Error::ParseUserSid { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Error::KerberosRealmEnvVar { .. } => StatusCode::INTERNAL_SERVER_ERROR,
}
}
}
Expand All @@ -89,6 +94,7 @@ const LDAP_FIELD_OBJECT_DISTINGUISHED_NAME: &str = "dn";
const LDAP_FIELD_USER_NAME: &str = "userPrincipalName";
const LDAP_FIELD_USER_PRIMARY_GROUP_RID: &str = "primaryGroupID";
const LDAP_FIELD_GROUP_MEMBER: &str = "member";
const LDAP_FIELD_SAM_ACCOUNT_NAME: &str = "sAMAccountName";

#[tracing::instrument(skip(
tls,
Expand Down Expand Up @@ -133,9 +139,7 @@ pub(crate) async fn get_user_info(
)
)
}
UserInfoRequest::UserInfoRequestByName(username) => {
format!("{LDAP_FIELD_USER_NAME}={}", ldap_escape(&username.username))
}
UserInfoRequest::UserInfoRequestByName(username) => user_name_filter(&username.username)?,
};
let requested_user_attrs = [
LDAP_FIELD_OBJECT_SECURITY_ID,
Expand Down Expand Up @@ -179,6 +183,18 @@ pub(crate) async fn get_user_info(
.await
}

/// Constructs a user filter that searches both the UPN as well as the sAMAccountName attributes.
/// It also searches for `username@realm` in addition to just `username`.
/// The realm is expected to be set in the `KERBEROS_REALM` environment variable.
/// See this issue for details: <https://github.com/stackabletech/opa-operator/issues/702>
fn user_name_filter(username: &str) -> Result<String, Error> {
let escaped_username = ldap_escape(username);
let realm = ldap_escape(env::var("KERBEROS_REALM").context(KerberosRealmEnvVarSnafu)?);
Ok(format!(
"|({LDAP_FIELD_USER_NAME}={escaped_username}@{realm})({LDAP_FIELD_USER_NAME}={escaped_username})({LDAP_FIELD_SAM_ACCOUNT_NAME}={escaped_username})"
))
}

#[tracing::instrument(
skip(
ldap,
Expand Down