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
Merged

fix: expand AD user name filter #737

merged 14 commits into from
Jul 3, 2025

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": "razvan@sble.test"}' \
http://localhost:9476/user | jq
{
  "id": "de639e1d-6b75-4a7d-8347-f2c8ade06d51",
  "username": "razvan@sble.test",
  "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: "razvan@sble.test" }) ldap_server="sble-addc.sble.test"}: stackable_opa_user
_info_fetcher::backend::active_directory: requesting user from LDAP user_query_filter="(&(objectClass=user)(|(userPrincipalName=razvan@sble.test@SBLE.TEST)(userPrincipalName=razvan@sble.te
st)(sAMAccountName=razvan@sble.test)))" requested_user_attrs=["objectSid", "objectGUID", "userPrincipalName", "primaryGroupID", "c"]
2025-06-26T15:47:19.664185Z DEBUG get_user_info{request=UserInfoRequestByName(UserInfoRequestByName { username: "razvan@sble.test" }) 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
zvan@sble.test"]}, 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: "razvan@sble.test" }) 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: "razvan@sble.test" }) 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)(|(userPrincipalName=razvan@sble.test@SBLE.TEST)(userPrincipalName=razvan@sble.te
st)(sAMAccountName=razvan@sble.test)))

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
@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

Copy link
Contributor

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

Functionality-wise this seems to work fine for me. Do we want to add cases for this to the ad-user-info test?

@razvan
Copy link
Member Author

razvan commented Jul 2, 2025

Functionality-wise this seems to work fine for me. Do we want to add cases for this to the ad-user-info test?

Indeed. Successfully tested with the help of stackabletech/ad-init#8

@razvan razvan requested a review from nightkr July 2, 2025 13:38
@razvan razvan requested a review from nightkr July 3, 2025 10:54
Copy link
Contributor

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

LGTM if tests pass

@razvan
Copy link
Member Author

razvan commented Jul 3, 2025

❯ ./scripts/run-tests --skip-release --skip-delete --test ad-user-info
...
--- PASS: kuttl (17.51s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/ad-user-info_opa-latest-1.4.2_openshift-false (17.21s)
PASS

@razvan razvan added this pull request to the merge queue Jul 3, 2025
Merged via the queue into main with commit 399bd76 Jul 3, 2025
17 checks passed
@razvan razvan deleted the fix/issue-702 branch July 3, 2025 12:26
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