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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: expand AD user name filter #737

wants to merge 7 commits into from

Conversation

razvan
Copy link
Member

@razvan razvan commented Jun 25, 2025

Description

Fixes #702

How I tested

  1. Used ad-init to set up an AD VM and a connected Kubernetes cluster.
  2. Created a new AD group called ad-test, a new AD user called razvan in the SBLE.TEST realm and added razvan to the ad-test group.
  3. Deployed the opa-operator with make run-dev
  4. Deployed the following OPA cluster (see below) and port forwarded 9476 of the OPA pod.
  5. Ran the following curl command:
❯ curl -XPOST -H "Content-type: application/json" \
-d '{"username": "[email protected]"}' \
http://localhost:9476/user | jq
{
  "id": "de639e1d-6b75-4a7d-8347-f2c8ade06d51",
  "username": "[email protected]",
  "groups": [
    "CN=Users,CN=Builtin,DC=sble,DC=test",
    "CN=Domain Users,CN=Users,DC=sble,DC=test",
    "CN=ad-test,CN=Users,DC=sble,DC=test"
  ],
  "customAttributes": {}
}

OPA cluster definition:

---
apiVersion: opa.stackable.tech/v1alpha1
kind: OpaCluster
metadata:
  name: opa
spec:
  image:
    productVersion: 1.0.1
  clusterConfig:
    userInfo:
      backend:
        experimentalActiveDirectory:
          ldapServer: sble-addc.sble.test
          baseDistinguishedName: DC=sble,DC=test
          customAttributeMappings:
            country: c
          kerberosSecretClassName: kerberos-ad
          tls:
            verification:
              server:
                caCert:
                  secretClass: tls-ad
      cache: # optional, enabled by default
        entryTimeToLive: 60s # optional, defaults to 60s
  servers:
    roleGroups:
      default:
        podOverrides:
          spec:
            containers:
              - name: bundle-builder
                imagePullPolicy: IfNotPresent
              - name: user-info-fetcher
                imagePullPolicy: IfNotPresent
                env:
                  - name: CONSOLE_LOG
                    value: DEBUG
                  - name: CONSOLE_LOG_LEVEL
                    value: DEBUG

The (debug) logs of the user-info-fetcher container showed:

+ '[' -f /stackable/kerberos/krb5.conf ']'
++ grep -oP 'default_realm = \K.*' /stackable/kerberos/krb5.conf
+ export KERBEROS_REALM=SBLE.TEST
+ KERBEROS_REALM=SBLE.TEST
+ /sbin/stackable-opa-user-info-fetcher
2025-06-26T15:31:23.126407Z  INFO stackable_opa_user_info_fetcher: Starting user-info-fetcher built_info.pkg_version="0.0.0-dev" built_info.target="x86_64-unknown-linux-gnu" built_info.bui
lt_time_utc="Tue, 1 Jan 1980 00:00:00 +0000" built_info.rustc_version="rustc 1.86.0 (05f9846f8 2025-03-31) (built from a source tarball)"
2025-06-26T15:47:19.663489Z DEBUG get_user_info{request=UserInfoRequestByName(UserInfoRequestByName { username: "[email protected]" }) ldap_server="sble-addc.sble.test"}: stackable_opa_user
_info_fetcher::backend::active_directory: requesting user from LDAP user_query_filter="(&(objectClass=user)(|([email protected]@SBLE.TEST)([email protected]
st)([email protected])))" requested_user_attrs=["objectSid", "objectGUID", "userPrincipalName", "primaryGroupID", "c"]
2025-06-26T15:47:19.664185Z DEBUG get_user_info{request=UserInfoRequestByName(UserInfoRequestByName { username: "[email protected]" }) ldap_server="sble-addc.sble.test"}: stackable_opa_user
_info_fetcher::backend::active_directory: got user from LDAP user=SearchEntry { dn: "CN=Razvan Mihai,CN=Users,DC=sble,DC=test", attrs: {"primaryGroupID": ["513"], "userPrincipalName": ["ra
[email protected]"]}, bin_attrs: {"objectGUID": [[29, 158, 99, 222, 117, 107, 125, 74, 131, 71, 242, 200, 173, 224, 109, 81]], "objectSid": [[1, 5, 0, 0, 0, 0, 0, 5, 21, 0, 0, 0, 170, 35, 214
 189, 227, 141, 132, 148, 20, 231, 109, 119, 84, 4, 0, 0[]]} }
2025-06-26T15:47:19.664293Z DEBUG get_user_info{request=UserInfoRequestByName(UserInfoRequestByName { username: "[email protected]" }) ldap_server="sble-addc.sble.test"}:user_attributes:use
r_group_distinguished_names: stackable_opa_user_info_fetcher::backend::active_directory: computed primary group SID for user user_sid=S-1-5-21-3184927658-2491715043-2003691284-1108 primary
_group_sid=S-1-5-21-3184927658-2491715043-2003691284-513 primary_group_relative_id=513
2025-06-26T15:47:19.664323Z DEBUG get_user_info{request=UserInfoRequestByName(UserInfoRequestByName { username: "[email protected]" }) ldap_server="sble-addc.sble.test"}:user_attributes:use
r_group_distinguished_names: stackable_opa_user_info_fetcher::backend::active_directory: requesting user groups from LDAP groups_query_filter="(&(objectClass=group)(|(objectSid=S-1-5-21-31
84927658-2491715043-2003691284-513)(member:1.2.840.113556.1.4.1941:=<SID=S-1-5-21-3184927658-2491715043-2003691284-513>)(member:1.2.840.113556.1.4.1941:=<SID=S-1-5-21-3184927658-2491715043
-2003691284-1108>)))" requested_group_attrs=["dn"]

where the LDAP search filter was:

(&(objectClass=user)(|([email protected]@SBLE.TEST)([email protected]
st)([email protected])))

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

  • Changes are OpenShift compatible
  • CRD changes approved
  • CRD documentation for all fields, following the style guide.
  • Helm chart can be installed and deployed operator works
  • Integration tests passed (for non trivial changes)
  • Changes need to be "offline" compatible
  • Links to generated (nightly) docs added
  • Release note snippet added

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • (Integration-)Test cases added
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added
  • Links to generated (nightly) docs added
  • Release note snippet added
  • Add type/deprecation label & add to the deprecation schedule
  • Add type/experimental label & add to the experimental features tracker

@razvan razvan self-assigned this Jun 25, 2025
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you just want to do a text search, you can do that from Rust (https://docs.rs/grep/latest/grep/ is ripgrep-as-a-library).

But there's no need to grep through krb5.conf at all, because libkrb5 provides the krb5_get_default_realm function! We expose this as krb5::KrbContext::default_realm.

However, krb5 is currently only available in secret-operator's repo.. we might want to split that out into a separate library entirely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I def didn't want to parse the krb5.conf file in code and we already use the grep approach in other places in the SDP.

The krb5_get_default_realm would be my preferred way too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@razvan razvan marked this pull request as ready for review June 27, 2025 07:46
@razvan
Copy link
Member Author

razvan commented Jun 30, 2025

Depends on stackabletech/krb5-rs#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User Info Fetcher: Retrieve user info by sAMAccountName
2 participants