AD Provider: Support initial password inputs#161
Conversation
- Updated New-IdleADAdapter.ps1 NewUser method to support three password input modes: * SecureString input (direct use) * ProtectedString input (from ConvertFrom-SecureString) * Explicit plaintext via AccountPasswordAsPlainText - Added validation to reject ambiguous input (both password fields) - Added clear error messages for invalid password formats - Updated Copy-IdleRedactedObject.ps1 to redact AccountPassword and AccountPasswordAsPlainText keys - Updated fake adapter in tests to mirror password validation - Added comprehensive password handling tests - Added redaction tests for new password keys Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds support for providing an initial password during AD user creation in IdLE.Provider.AD, while ensuring password inputs are redacted from exported/logged objects via IdLE.Core.
Changes:
- Added password input handling to the AD adapter (
SecureString, ProtectedString fromConvertFrom-SecureString, and explicit plaintext escape hatch). - Extended default redaction keys in
Copy-IdleRedactedObjectto includeAccountPasswordandAccountPasswordAsPlainText. - Added/updated Pester tests covering password input modes and redaction behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/Providers/ADIdentityProvider.Tests.ps1 | Updates fake AD adapter behavior and adds password-handling tests for CreateIdentity/NewUser scenarios. |
| tests/Core/Copy-IdleRedactedObject.Tests.ps1 | Adds regression tests ensuring new password-related keys are redacted (including nested structures). |
| src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 | Implements password input mode validation and conversion before calling New-ADUser. |
| src/IdLE.Core/Private/Copy-IdleRedactedObject.ps1 | Adds AccountPassword and AccountPasswordAsPlainText to the default redaction key list. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd743be36
ℹ️ 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".
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Added explicit null checks before GetType() calls in both real and fake adapters - Prevents unhelpful null-valued expression errors - Added tests for AccountPassword and AccountPasswordAsPlainText null values - Clear error messages guide users to provide valid values Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
- Explained why Changed=true is not asserted in valid password tests - Noted that fake adapter auto-creates during ResolveIdentity lookup - Clarified that password validation is tested via error-throwing tests that call NewUser directly Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
- Updated ProtectedString error message to clarify only DPAPI-scoped strings are supported - Explicitly state that key-based protected strings are not supported - Fixed wording from "Inner exception type" to "Exception type" - Include exception message for better troubleshooting - Changed valid password tests to call NewUser directly instead of CreateIdentity - This ensures password handling code is actually exercised in tests Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
|
@copilot fix the PSSA error |
- Added function-level SuppressMessageAttribute for PSAvoidUsingConvertToSecureStringWithPlainText - This is an intentional, documented escape hatch for AccountPasswordAsPlainText - Password values are automatically redacted via Copy-IdleRedactedObject - All tests passing, no PSSA errors Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Fixed in commit 68b9ea6. Added function-level SuppressMessageAttribute for PSAvoidUsingConvertToSecureStringWithPlainText. This is an intentional, documented escape hatch for the AccountPasswordAsPlainText feature with automatic redaction. All tests pass and PSScriptAnalyzer shows 0 errors. |
|
@copilot |
- Added file-level SuppressMessageAttribute to ADIdentityProvider.Tests.ps1 - Test file intentionally uses ConvertTo-SecureString -AsPlainText to create test data - This suppression is appropriate for test files where plaintext passwords are needed for validation - All tests pass, no PSSA errors in test file Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Fixed in commit 37ee5dc. Added file-level SuppressMessageAttribute to ADIdentityProvider.Tests.ps1 for PSAvoidUsingConvertToSecureStringWithPlainText. Test files intentionally use ConvertTo-SecureString -AsPlainText to create test data for password validation. This is the appropriate pattern for test files where plaintext conversions are necessary and expected. PSScriptAnalyzer now shows 0 errors for both the adapter and test file. |
AD Provider: Support initial password inputs
Implementation Summary
Successfully implemented support for initial password inputs in the AD Provider with three input modes:
ConvertFrom-SecureString(recommended for templating)AccountPasswordAsPlainText(escape hatch with strong redaction)Changes Made
New-IdleADAdapter.ps1NewUsermethod to support password inputs:AccountPasswordas SecureString (direct use)AccountPasswordas ProtectedString (ConvertTo-SecureString)AccountPasswordAsPlainTextas plaintext (ConvertTo-SecureString -AsPlainText)Copy-IdleRedactedObject.ps1:Code Review Feedback Addressed
Test Results
All tests passing:
Key Features
Original prompt
This section details on the original issue you should resolve
<issue_title>AD Provider: Support initial password inputs</issue_title>
<issue_description>## Problem Statement
Creating AD users via
IdLE.Step.CreateIdentity+IdLE.Provider.ADcurrently fails in many real-world domains because AD password policy requires an initial password when the account is enabled. Users also need a safe and ergonomic way to provide an initial password to IdLE.Constraints:
.psd1data files and should not embed secrets directly.SecureStringcannot be embedded directly.Goal: Allow providing an initial password to the AD Provider in a controlled, explicit, and well-documented way.
Proposed Solution
Extend the AD Provider/Adapter password input handling for
CreateIdentity(...)by supporting three input modes:1) SecureString input (power users/tests)
If
With.AccountPasswordis aSecureString, use it directly forNew-ADUser -AccountPassword.2) ProtectedString input (recommended for templating)
If
With.AccountPasswordis astring, treat it as a ProtectedString produced byConvertFrom-SecureString(no-AsPlainText).ConvertTo-SecureString(without-AsPlainText).ConvertFrom-SecureString) and the DPAPI scope limitation.3) Explicit PlainText input via
With.AccountPasswordAsPlainText(escape hatch)Allow plaintext password input only via a dedicated, explicit property:
With.AccountPasswordAsPlainText = '<plaintext>'Rules:
ConvertTo-SecureString -AsPlainText -Force.With.AccountPassword(to avoid accidental use).Precedence / validation
With.AccountPasswordandWith.AccountPasswordAsPlainTextare provided: fail fast (ambiguous).Documentation (mandatory)
Add a dedicated AD Provider documentation section:
Request.VariablesWith.AccountPasswordWith.AccountPasswordAsPlainTextAlternatives Considered
Do not support plaintext at all.
Require a secret store capability first.
Impact
Does this affect existing workflows?
.psd1.Any backward compatibility concerns?
With.*properties and conversion behavior forWith.AccountPasswordstrings.Additional Context
Acceptance Criteria
With.AccountPasswordsupports:With.AccountPasswordAsPlainTextsupports plaintext input explicitly.Examples
ProtectedString via Request.Variables
Workflow:
Explicit plaintext (escape hatch)
</issue_description>
<agent_instructions>although breaking changes, full ahead, no backward compatibility checks / aliases or documentation / references</agent_instructions>
Comments on the Issue (you are @copilot in this section)
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.