Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
17 changes: 17 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 45 additions & 0 deletions Cargo.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rust/user-info-fetcher/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ publish = false
[dependencies]
stackable-opa-operator = { path = "../operator-binary" }
stackable-operator.workspace = true
krb5 = { path = "../../../krb5-rs/rust/krb5"}

axum.workspace = true
base64.workspace = true
Expand Down
41 changes: 38 additions & 3 deletions rust/user-info-fetcher/src/backend/active_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::{

use byteorder::{BigEndian, LittleEndian, ReadBytesExt};
use hyper::StatusCode;
use krb5::KrbContext;
use ldap3::{Ldap, LdapConnAsync, LdapConnSettings, LdapError, Scope, SearchEntry, ldap_escape};
use snafu::{OptionExt, ResultExt, Snafu};
use stackable_operator::commons::tls_verification::TlsClientDetails;
Expand Down Expand Up @@ -58,6 +59,15 @@ pub enum Error {
source: ParseSecurityIdError,
user_dn: String,
},

#[snafu(display("failed to create Kerberos context"))]
KerberosContext { source: krb5::Error },

#[snafu(display("failed to get Kerberos realm"))]
KerberosRealm { source: krb5::Error },

#[snafu(display("failed to get Kerberos realm name"))]
KerberosRealmName { source: std::str::Utf8Error },
}

impl http_error::Error for Error {
Expand All @@ -75,6 +85,9 @@ 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::KerberosContext { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Error::KerberosRealm { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Error::KerberosRealmName { .. } => StatusCode::INTERNAL_SERVER_ERROR,
}
}
}
Expand All @@ -89,6 +102,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 +147,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 +191,29 @@ 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`.
/// 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(default_realm_name()?);
Ok(format!(
"|({LDAP_FIELD_USER_NAME}={escaped_username}@{realm})({LDAP_FIELD_USER_NAME}={escaped_username})({LDAP_FIELD_SAM_ACCOUNT_NAME}={escaped_username})"
))
}

/// Returns the default Kerberos realm name, which is used to construct user filters.
/// TODO: this could be moved in a backend specific initialization function,
/// but currently there is no trait for backend implementations.
fn default_realm_name() -> Result<String, Error> {
let krb_context = KrbContext::new().context(KerberosContextSnafu)?;
let krb_realm = krb_context.default_realm().context(KerberosRealmSnafu)?;
Ok(krb_realm
.to_str()
.context(KerberosRealmNameSnafu)?
.to_string())
}

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