diff --git a/src/IdLE.Core/Private/Copy-IdleRedactedObject.ps1 b/src/IdLE.Core/Private/Copy-IdleRedactedObject.ps1 index 6c5b1e3a..8dabd206 100644 --- a/src/IdLE.Core/Private/Copy-IdleRedactedObject.ps1 +++ b/src/IdLE.Core/Private/Copy-IdleRedactedObject.ps1 @@ -28,7 +28,9 @@ function Copy-IdleRedactedObject { 'accessToken', 'refreshToken', 'credential', - 'privateKey' + 'privateKey', + 'AccountPassword', + 'AccountPasswordAsPlainText' ) $effectiveKeys = if ($null -ne $RedactedKeys -and $RedactedKeys.Count -gt 0) { diff --git a/src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 b/src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 index f3e49383..d01070a4 100644 --- a/src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 +++ b/src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 @@ -9,8 +9,14 @@ function New-IdleADAdapter { .PARAMETER Credential Optional PSCredential for AD operations. If not provided, uses integrated auth. + + .NOTES + PSScriptAnalyzer suppression: This function intentionally uses ConvertTo-SecureString -AsPlainText + as an explicit escape hatch for AccountPasswordAsPlainText. This is a documented design decision + with automatic redaction via Copy-IdleRedactedObject. #> [CmdletBinding()] + [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '', Justification = 'Intentional escape hatch for AccountPasswordAsPlainText with explicit opt-in and automatic redaction')] param( [Parameter()] [AllowNull()] @@ -171,6 +177,69 @@ function New-IdleADAdapter { $params['EmailAddress'] = $Attributes['EmailAddress'] } + # Password handling: support SecureString, ProtectedString, and explicit PlainText + $hasAccountPassword = $Attributes.ContainsKey('AccountPassword') + $hasAccountPasswordAsPlainText = $Attributes.ContainsKey('AccountPasswordAsPlainText') + + if ($hasAccountPassword -and $hasAccountPasswordAsPlainText) { + throw "Ambiguous password configuration: both 'AccountPassword' and 'AccountPasswordAsPlainText' are provided. Use only one." + } + + if ($hasAccountPassword) { + $passwordValue = $Attributes['AccountPassword'] + + if ($null -eq $passwordValue) { + throw "AccountPassword: Value cannot be null. Provide a SecureString or ProtectedString (from ConvertFrom-SecureString)." + } + + if ($passwordValue -is [securestring]) { + # Mode 1: SecureString - use directly + $params['AccountPassword'] = $passwordValue + } + elseif ($passwordValue -is [string]) { + # Mode 2: ProtectedString (from ConvertFrom-SecureString) + try { + $params['AccountPassword'] = ConvertTo-SecureString -String $passwordValue -ErrorAction Stop + } + catch { + $errorMsg = "AccountPassword: Expected a ProtectedString (output from ConvertFrom-SecureString without -Key) but conversion failed. " + $errorMsg += "Only DPAPI-scoped ProtectedStrings are supported (created under the same Windows user and machine). " + $errorMsg += "Key-based protected strings (using -Key or -SecureKey) are not supported. " + if ($null -ne $_.Exception) { + $errorMsg += "Exception type: $($PSItem.Exception.GetType().FullName). " + if (-not [string]::IsNullOrWhiteSpace($_.Exception.Message)) { + $errorMsg += "Message: $($_.Exception.Message)" + } + } + throw $errorMsg + } + } + else { + throw "AccountPassword: Expected a SecureString or ProtectedString (string from ConvertFrom-SecureString), but received type: $($passwordValue.GetType().FullName)" + } + } + + if ($hasAccountPasswordAsPlainText) { + $plainTextPassword = $Attributes['AccountPasswordAsPlainText'] + + if ($null -eq $plainTextPassword) { + throw "AccountPasswordAsPlainText: Value cannot be null. Provide a non-empty plaintext password string." + } + + if ($plainTextPassword -isnot [string]) { + throw "AccountPasswordAsPlainText: Expected a string but received type: $($plainTextPassword.GetType().FullName)" + } + + if ([string]::IsNullOrWhiteSpace($plainTextPassword)) { + throw "AccountPasswordAsPlainText: Password cannot be null or empty." + } + + # Mode 3: Explicit plaintext - convert with -AsPlainText + # This is an intentional escape hatch with explicit opt-in via AccountPasswordAsPlainText. + # The value is redacted from logs/events via Copy-IdleRedactedObject. + $params['AccountPassword'] = ConvertTo-SecureString -String $plainTextPassword -AsPlainText -Force + } + if ($null -ne $this.Credential) { $params['Credential'] = $this.Credential } diff --git a/tests/Core/Copy-IdleRedactedObject.Tests.ps1 b/tests/Core/Copy-IdleRedactedObject.Tests.ps1 index b124a588..fa19ebab 100644 --- a/tests/Core/Copy-IdleRedactedObject.Tests.ps1 +++ b/tests/Core/Copy-IdleRedactedObject.Tests.ps1 @@ -112,5 +112,53 @@ Describe 'Copy-IdleRedactedObject - deterministic redaction utility' { $copy.password | Should -Be '' } + + It 'redacts AccountPassword key' { + $input = @{ + userName = 'testuser' + AccountPassword = 'ProtectedStringValue' + otherField = 'visible' + } + + $copy = Copy-IdleRedactedObject -Value $input + + $copy.userName | Should -Be 'testuser' + $copy.AccountPassword | Should -Be '[REDACTED]' + $copy.otherField | Should -Be 'visible' + } + + It 'redacts AccountPasswordAsPlainText key' { + $input = @{ + userName = 'testuser' + AccountPasswordAsPlainText = 'PlainTextPassword' + otherField = 'visible' + } + + $copy = Copy-IdleRedactedObject -Value $input + + $copy.userName | Should -Be 'testuser' + $copy.AccountPasswordAsPlainText | Should -Be '[REDACTED]' + $copy.otherField | Should -Be 'visible' + } + + It 'redacts both password fields if present in nested structure' { + $input = @{ + userDetails = @{ + name = 'alice' + AccountPassword = 'SecureValue' + } + settings = [pscustomobject]@{ + AccountPasswordAsPlainText = 'PlainTextValue' + enabled = $true + } + } + + $copy = Copy-IdleRedactedObject -Value $input + + $copy.userDetails.name | Should -Be 'alice' + $copy.userDetails.AccountPassword | Should -Be '[REDACTED]' + $copy.settings.AccountPasswordAsPlainText | Should -Be '[REDACTED]' + $copy.settings.enabled | Should -Be $true + } } } diff --git a/tests/Providers/ADIdentityProvider.Tests.ps1 b/tests/Providers/ADIdentityProvider.Tests.ps1 index 749ca0fe..4d48f45b 100644 --- a/tests/Providers/ADIdentityProvider.Tests.ps1 +++ b/tests/Providers/ADIdentityProvider.Tests.ps1 @@ -1,3 +1,8 @@ +# PSScriptAnalyzer suppression: Test file intentionally uses ConvertTo-SecureString -AsPlainText +# to create test data for password handling validation +[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '')] +param() + Set-StrictMode -Version Latest BeforeDiscovery { @@ -46,6 +51,9 @@ Describe 'AD identity provider' { # Auto-creation behavior: The fake adapter auto-creates identities on lookup # to support provider contract tests (which expect this behavior from test providers). # This differs from the real AD adapter which will throw when an identity is not found. + # Note: This fake adapter may auto-create identities for multiple lookup methods + # (e.g., UPN and sAMAccountName), so tests that rely on non-existence should not + # assume that a given lookup will return $null as the real adapter would. $adapter | Add-Member -MemberType ScriptMethod -Name GetUserByUpn -Value { param([string]$Upn) @@ -54,7 +62,27 @@ Describe 'AD identity provider' { return $this.Store[$key] } } - return $null + + # Auto-create for contract test compatibility + $sam = $Upn -replace '@.*$', '' + $guid = [guid]::NewGuid().ToString() + $user = [pscustomobject]@{ + ObjectGuid = [guid]$guid + sAMAccountName = $sam + UserPrincipalName = $Upn + DistinguishedName = "CN=$sam,OU=Users,DC=domain,DC=local" + Enabled = $true + GivenName = $null + Surname = $null + DisplayName = $null + Description = $null + Department = $null + Title = $null + EmailAddress = $null + Groups = @() + } + $this.Store[$guid] = $user + return $user } -Force $adapter | Add-Member -MemberType ScriptMethod -Name GetUserBySam -Value { @@ -97,6 +125,57 @@ Describe 'AD identity provider' { $adapter | Add-Member -MemberType ScriptMethod -Name NewUser -Value { param([string]$Name, [hashtable]$Attributes, [bool]$Enabled) + # Password handling validation (same as real adapter) + $hasAccountPassword = $Attributes.ContainsKey('AccountPassword') + $hasAccountPasswordAsPlainText = $Attributes.ContainsKey('AccountPasswordAsPlainText') + + if ($hasAccountPassword -and $hasAccountPasswordAsPlainText) { + throw "Ambiguous password configuration: both 'AccountPassword' and 'AccountPasswordAsPlainText' are provided. Use only one." + } + + if ($hasAccountPassword) { + $passwordValue = $Attributes['AccountPassword'] + + if ($null -eq $passwordValue) { + throw "AccountPassword: Value cannot be null. Provide a SecureString or ProtectedString (from ConvertFrom-SecureString)." + } + + if ($passwordValue -is [securestring]) { + # Valid: SecureString + } + elseif ($passwordValue -is [string]) { + # Valid: ProtectedString - test conversion + try { + $null = ConvertTo-SecureString -String $passwordValue -ErrorAction Stop + } + catch { + $errorMsg = "AccountPassword: Expected a ProtectedString (output from ConvertFrom-SecureString) but conversion failed. " + $errorMsg += "ProtectedString only works when encryption and decryption occur under the same Windows user and machine (DPAPI scope). " + $errorMsg += "Original error: $_" + throw $errorMsg + } + } + else { + throw "AccountPassword: Expected a SecureString or ProtectedString (string from ConvertFrom-SecureString), but received type: $($passwordValue.GetType().FullName)" + } + } + + if ($hasAccountPasswordAsPlainText) { + $plainTextPassword = $Attributes['AccountPasswordAsPlainText'] + + if ($null -eq $plainTextPassword) { + throw "AccountPasswordAsPlainText: Value cannot be null. Provide a non-empty plaintext password string." + } + + if ($plainTextPassword -isnot [string]) { + throw "AccountPasswordAsPlainText: Expected a string but received type: $($plainTextPassword.GetType().FullName)" + } + + if ([string]::IsNullOrWhiteSpace($plainTextPassword)) { + throw "AccountPasswordAsPlainText: Password cannot be null or empty." + } + } + $guid = [guid]::NewGuid().ToString() $sam = if ($Attributes.ContainsKey('SamAccountName')) { $Attributes['SamAccountName'] } else { $Name } $upn = if ($Attributes.ContainsKey('UserPrincipalName')) { $Attributes['UserPrincipalName'] } else { "$sam@domain.local" } @@ -681,4 +760,152 @@ Describe 'AD identity provider' { { $provider.DeleteIdentity($guid) } | Should -Throw -ExpectedMessage '*AllowDelete*' } } + + Context 'Password handling' { + BeforeEach { + $adapter = New-FakeADAdapter + $provider = New-IdleADIdentityProvider -Adapter $adapter + $script:TestProvider = $provider + $script:TestAdapter = $adapter + } + + It 'CreateIdentity accepts AccountPassword as SecureString' { + # Test setup requires plaintext conversion - this is intentional for test data + $password = ConvertTo-SecureString -String 'TestPass123!' -AsPlainText -Force + $attrs = @{ + SamAccountName = 'pwtest1' + AccountPassword = $password + } + + # Test the adapter's NewUser method directly to verify password handling + # (CreateIdentity would skip NewUser due to fake adapter auto-creation) + $result = $script:TestAdapter.NewUser('pwtest1', $attrs, $true) + $result | Should -Not -BeNullOrEmpty + $result.sAMAccountName | Should -Be 'pwtest1' + } + + It 'CreateIdentity accepts AccountPassword as ProtectedString' { + # Create a ProtectedString using ConvertFrom-SecureString + # Test setup requires plaintext conversion - this is intentional for test data + $securePassword = ConvertTo-SecureString -String 'TestPass456!' -AsPlainText -Force + $protectedString = ConvertFrom-SecureString -SecureString $securePassword + + $attrs = @{ + SamAccountName = 'pwtest2' + AccountPassword = $protectedString + } + + # Test the adapter's NewUser method directly to verify password handling + # (CreateIdentity would skip NewUser due to fake adapter auto-creation) + $result = $script:TestAdapter.NewUser('pwtest2', $attrs, $true) + $result | Should -Not -BeNullOrEmpty + $result.sAMAccountName | Should -Be 'pwtest2' + } + + It 'CreateIdentity accepts AccountPasswordAsPlainText' { + $attrs = @{ + SamAccountName = 'pwtest3' + AccountPasswordAsPlainText = 'PlainTextPass789!' + } + + # Test the adapter's NewUser method directly to verify password handling + # (CreateIdentity would skip NewUser due to fake adapter auto-creation) + $result = $script:TestAdapter.NewUser('pwtest3', $attrs, $true) + $result | Should -Not -BeNullOrEmpty + $result.sAMAccountName | Should -Be 'pwtest3' + } + + It 'Throws when both AccountPassword and AccountPasswordAsPlainText are provided' { + # Test setup requires plaintext conversion - this is intentional for test data + $password = ConvertTo-SecureString -String 'TestPass!' -AsPlainText -Force + $attrs = @{ + SamAccountName = 'pwtest4' + AccountPassword = $password + AccountPasswordAsPlainText = 'PlainText!' + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest4', $attrs, $true) } | + Should -Throw -ExpectedMessage '*Ambiguous password configuration*' + } + + It 'Throws when AccountPassword has invalid type' { + $attrs = @{ + SamAccountName = 'pwtest5' + AccountPassword = 12345 # Invalid: should be string or SecureString + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest5', $attrs, $true) } | + Should -Throw -ExpectedMessage '*Expected a SecureString or ProtectedString*' + } + + It 'Throws when AccountPassword string is not a valid ProtectedString' { + $attrs = @{ + SamAccountName = 'pwtest6' + AccountPassword = 'NotAProtectedString' # Plain string, not from ConvertFrom-SecureString + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest6', $attrs, $true) } | + Should -Throw -ExpectedMessage '*conversion failed*' + } + + It 'Throws when AccountPasswordAsPlainText has invalid type' { + $attrs = @{ + SamAccountName = 'pwtest7' + AccountPasswordAsPlainText = 12345 # Invalid: should be string + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest7', $attrs, $true) } | + Should -Throw -ExpectedMessage '*Expected a string but received type*' + } + + It 'Throws when AccountPasswordAsPlainText is empty' { + $attrs = @{ + SamAccountName = 'pwtest8' + AccountPasswordAsPlainText = '' + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest8', $attrs, $true) } | + Should -Throw -ExpectedMessage '*cannot be null or empty*' + } + + It 'Throws when AccountPassword is null' { + $attrs = @{ + SamAccountName = 'pwtest9' + AccountPassword = $null + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest9', $attrs, $true) } | + Should -Throw -ExpectedMessage '*Value cannot be null*' + } + + It 'Throws when AccountPasswordAsPlainText is null' { + $attrs = @{ + SamAccountName = 'pwtest10' + AccountPasswordAsPlainText = $null + } + + # Test the adapter's NewUser method directly to verify validation + { $script:TestAdapter.NewUser('pwtest10', $attrs, $true) } | + Should -Throw -ExpectedMessage '*Value cannot be null*' + } + + It 'CreateIdentity works without password (existing behavior)' { + $attrs = @{ + SamAccountName = 'pwtest11' + GivenName = 'Test' + Surname = 'User' + } + + # Should not throw - no password is valid + $result = $script:TestProvider.CreateIdentity('pwtest11', $attrs) + $result | Should -Not -BeNullOrEmpty + $result.IdentityKey | Should -Be 'pwtest11' + } + } }