Skip to content

Add Manager attribute support with automatic DN resolution to AD provider#175

Merged
blindzero merged 6 commits intomainfrom
copilot/fix-manager-attribute-handling
Feb 10, 2026
Merged

Add Manager attribute support with automatic DN resolution to AD provider#175
blindzero merged 6 commits intomainfrom
copilot/fix-manager-attribute-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 10, 2026

AD Provider: Manager attribute handling across CreateIdentity and EnsureAttribute

Changes Made

Added Manager Resolution (addressing @blindzero's comment #3779393462):

  • Added ResolveManagerDN method to AD adapter
  • Automatically resolves Manager from multiple formats:
    • Distinguished Name (DN) - passed through with validation
    • GUID (ObjectGuid) - resolved to DN via lookup
    • UPN (UserPrincipalName) - resolved to DN via lookup
    • sAMAccountName - resolved to DN via lookup
  • Resolution logic mirrors IdentityKey resolution behavior
  • Clear error messages when manager cannot be found

Updated Documentation (addressing comment #2788349846):

  • Added note confirming $null works correctly in PSD1 files
  • Documented all supported Manager input formats
  • Added examples for DN, sAMAccountName, and UPN formats
  • Explained automatic resolution behavior

Fixed Test Naming (addressing bot review comment #2788491515):

  • Renamed test from "sAMAccountName does not exist" to "GUID does not exist"
  • Test name now accurately reflects the implementation
  • Comment clarifies why GUID is used (fake adapter auto-creates on sAMAccountName/UPN)

Updated Tests:

  • Added 5 new tests for Manager resolution (GUID, UPN, sAMAccountName)
  • Updated validation tests to reflect resolution behavior
  • All 75 tests passing

Test Results

  • ✅ All 75 AD provider tests passing
  • ✅ Manager resolution from DN (pass-through)
  • ✅ Manager resolution from GUID
  • ✅ Manager resolution from UPN
  • ✅ Manager resolution from sAMAccountName
  • ✅ Clear error when manager cannot be resolved
  • ✅ Clearing Manager with $null works correctly
Original prompt

This section details on the original issue you should resolve

<issue_title>AD Provider: Manager attribute handling across CreateIdentity and EnsureAttribute</issue_title>
<issue_description>## Summary

Provide reliable, explicit handling for the Manager attribute in the AD provider across steps:

  • CreateIdentity (New-ADUser)
  • EnsureAttribute (Set-ADUser)

Today, Manager may not work (or be silently ignored) due to missing/incorrect mapping and because manager is a DN-valued link attribute that often benefits from dedicated parameter handling.

This issue is intentionally separate from the broader “attribute contract + strict validation” work.

Motivation

Manager is commonly required in Joiner/Mover workflows. AD treats it as a link attribute (DN reference). Generic -Replace @{ manager = ... } is error-prone and requires correct DN formatting. Cmdlets also provide dedicated -Manager parameters which are usually the correct path.

We need:

  • A clear expected value format
  • Deterministic behavior
  • Consistency between create and ensure

Proposed Behavior (defaults)

Supported input format (default)

  • Manager value MUST be a Distinguished Name (DN) string.
    • Example: "CN=Jane Doe,OU=Users,DC=contoso,DC=com"

Rationale: avoids hidden resolution logic and keeps provider behavior deterministic and testable.

Optional future enhancement (not required for this issue)

  • An opt-in resolve mode that accepts UPN/sAMAccountName/GUID and resolves to DN.
    • This is out-of-scope unless explicitly required.

Proposed Implementation

CreateIdentity

When With.Attributes.Manager is provided:

  • Pass it via New-ADUser -Manager <DN> (dedicated parameter)
  • Ensure it is not attempted via generic OtherAttributes or ignored.

EnsureAttribute

When EnsureAttribute targets Manager:

  • Use Set-ADUser -Manager <DN> (dedicated parameter), not generic -Replace.

Validation

  • Validate DN format (lightweight validation):
    • must be a non-empty string
    • should contain at least one "=" and "," (basic heuristic)
  • If invalid, throw a clear error:
    • “Manager must be a DistinguishedName (DN).”

(Strict validation of supported attributes is handled in the separate issue; this issue focuses on Manager-specific handling.)

Edge Cases

  • Clearing Manager:
    • Define an explicit way to clear:
      • EnsureAttribute with Value = $null clears manager (implementation: Set-ADUser -Clear 'manager')
    • Must be documented and tested.

Code Locations (current)

  • src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1
    • Add/extend mapping for Manager in NewUser and SetUser
  • src/IdLE.Provider.AD/Public/New-IdleADIdentityProvider.ps1
    • Ensure step/provider methods route Manager correctly

Acceptance Criteria

  1. CreateIdentity supports Manager

    • With.Attributes.Manager sets the manager via New-ADUser -Manager.
  2. EnsureAttribute supports Manager

    • Ensuring Manager uses Set-ADUser -Manager.
    • Clearing Manager is supported and documented.
  3. Validation

    • Invalid Manager values fail fast with an actionable message.
  4. Tests

    • Unit tests verify correct cmdlet parameter usage for create + ensure.
    • Tests cover set + clear behavior.
  5. Docs + Examples

    • AD provider docs include:
      • expected DN format
      • example for CreateIdentity + EnsureAttribute
      • example for clearing Manager
        </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 10, 2026 12:49
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix manager attribute handling in AD provider Add Manager attribute support to AD provider Feb 10, 2026
Copilot AI requested a review from blindzero February 10, 2026 12:54
Copy link
Copy Markdown
Owner

@blindzero blindzero left a comment

Choose a reason for hiding this comment

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

@copilot I want to have some lookup mechanisms to find the manager DN also by other values (e.g. UPN or sAMAccountName) as we also use it for the IdentityKey topic

Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 10, 2026

@copilot I want to have some lookup mechanisms to find the manager DN also by other values (e.g. UPN or sAMAccountName) as we also use it for the IdentityKey topic

Added Manager DN resolution from multiple formats (GUID, UPN, sAMAccountName) in commit f22d998. The resolution logic mirrors IdentityKey behavior - it automatically detects the format and resolves to DN. Examples added to documentation showing all supported formats.

Copilot AI changed the title Add Manager attribute support to AD provider Add Manager attribute support with automatic DN resolution to AD provider Feb 10, 2026
Copilot AI requested a review from blindzero February 10, 2026 14:47
@blindzero blindzero marked this pull request as ready for review February 10, 2026 14:55
Copilot AI review requested due to automatic review settings February 10, 2026 14:55
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f22d99879a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds first-class Active Directory Manager attribute support to IdLE’s AD provider, including automatic resolution of common manager identifiers to a DN so CreateIdentity/EnsureAttribute can use the dedicated -Manager cmdlet parameters reliably.

Changes:

  • Added Manager DN validation + resolution logic to the AD adapter, and wired New-ADUser -Manager / Set-ADUser -Manager + clear via -Clear manager.
  • Expanded AD user retrieval to include Manager so current-value comparisons are possible.
  • Added Pester tests and updated provider docs with supported formats and examples.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 Adds TestManagerDN/ResolveManagerDN, includes Manager in query properties, and uses dedicated AD cmdlet parameters for set/clear.
tests/Providers/ADIdentityProvider.Tests.ps1 Extends fake adapter + adds tests for setting/clearing/resolution scenarios.
docs/reference/providers/provider-ad.md Documents Manager handling, supported formats, and examples including clearing with $null.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
@blindzero blindzero merged commit 5a097c6 into main Feb 10, 2026
8 checks passed
@blindzero blindzero deleted the copilot/fix-manager-attribute-handling branch February 15, 2026 19:01
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.

AD Provider: Manager attribute handling across CreateIdentity and EnsureAttribute

3 participants