Release v1.3.2 - surface AD secret/config errors instead of silent UNKNOWN_USER - #19
Merged
Conversation
A broken/mis-deployed secrets.psd1 (a DPAPI SecureString generated on another machine or account) was caught by Get-SPSADUser, logged only to the Event Log, and returned as $null - indistinguishable from a genuinely absent user. On a ~100k-user multi-forest farm this silently produced empty-name JSON records for a whole forest and downgraded thousands of real users to UNKNOWN_USER in SPSyncUserProfile, one Event-Log line each. - Get-SPSADConnection / Get-SPSADUser now distinguish a build-time configuration error (missing LdapPath/CredentialKey, undecodable/missing secret) - which throws a terminating SPSADConfigError - from a genuine lookup miss ($null). - A connectivity error (server not operational / referral, e.g. an external RGA-style directory unreachable from the UPA master) stays NON-fatal: the login is logged and left unresolved so the run continues. - Get-SPSADConnectionError (new public) - query-free per-forest secret pre-flight. - SPSyncUserProfile pre-flights every credential-mode forest in the input and Exit 1s with an actionable message when a secret cannot be decoded here, instead of silently skipping users as UNKNOWN_USER. - SPSyncUserInfoList (parallel) fails the run loudly on a config/secret error, naming the affected forest(s), rather than shipping an empty-name forest. - Test-SPSADUser propagates config errors (still true/false for found/not-found). - Docs: troubleshooting entries for the decode failure (now fail-fast) and for the 'server not operational' / 'referral returned' connectivity errors. - Pester: Get-SPSADUser config-vs-connectivity, Test-SPSADUser propagation, Get-SPSADConnectionError pre-flight. Fixes #18
Pester Test Results81 tests 81 ✅ 3s ⏱️ Results for commit 1cbd8ab. ♻️ This comment has been updated with latest results. |
…irectories Some deployments resolve users against a non-Active-Directory LDAP directory (a partner / group-wide identity store) that needs a simple bind and a custom uid filter. Get-SPSADConnection already read an AuthenticationType key but it was undocumented, only applied to Credential-mode domains, and resolved via the static accessor [AuthenticationTypes]::$name - so a typo silently yielded $null (the same silent-config-error family as #16/#18). - Validate AuthenticationType with [Enum]::TryParse (case-insensitive, supports flag combinations); an unknown value throws with the list of valid names instead of silently falling back to $null. Default stays 'Secure'. - Honour AuthenticationType on Default-mode domains too, while keeping the exact original 1-arg DirectoryEntry construction when the key is absent (zero behaviour change for existing AD forests). - Document AuthenticationType in ad-domains.example.psd1 with a non-AD directory example (None simple bind / SecureSocketsLayer, uid filter, host:port/baseDN). - Genericize the non-AD directory references in the module help. - Pester: invalid AuthenticationType throws with valid names; a valid type is not rejected at validation time. Fixes #20
… 5.1 compat) Test-SPSUserSyncReadiness / Get-SPSADConnection crashed on every domain under Windows PowerShell 5.1 with 'Cannot find an overload for TryParse and the argument count: 4'. The non-generic [Enum]::TryParse(Type, string, bool, [ref]) overload only exists on .NET Core / .NET 5+; the .NET Framework that hosts Windows PowerShell 5.1 (which runs the scripts in production, for the SharePoint snap-in) does not have it. The AuthenticationType parse runs for every domain, so it broke all AD connections. - Replace it with [Enum]::Parse(Type, string, ignoreCase) in a try/catch (present since .NET Framework 1.1; same case-insensitive + flag-combination behaviour, and an invalid value still throws the actionable 'invalid AuthenticationType' error). Applied the same fix to the companion Test-LdapConnectivity diagnostic. - Add a Windows PowerShell 5.1 Pester job to CI. The suite previously ran only under pwsh (PowerShell 7), whose newer .NET BCL has the extra overload, so the pwsh-only pass went green and missed this. The existing AuthenticationType tests now also assert no 'overload'/'TryParse' method-resolution error, so this regression is caught under 5.1. Relates to #20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardening release (v1.3.2) that makes a broken or mis-deployed
secrets.psd1fail fast and loud instead of silently degrading the result. Follows a field case (~100k-user multi-forest farm) where asecrets.psd1that could not be decoded on the User Profile master server (DPAPI SecureStrings are bound to the machine + account that created them) silently skipped thousands of real users asUNKNOWN_USER, the only trace being one Event-Log line per user.This PR fixes the following issues:
Root cause
Get-SPSADUsercaught every exception — including an undecodable DPAPI secret — wrote it to the Event Log only, and returned$null, exactly like a genuinely absent user.Test-SPSADUseris($null -ne (Get-SPSADUser ...)), so a secret failure returned$false→SPSyncUserProfile.ps1labelled the userUNKNOWN_USERand skipped it. On the InfoList side the same swallow produced emptyFirstName/LastNamefor a whole forest, filling the Not Added list. Same silent-failure family as #16 (v1.3.1).Changes
Config/secret error = fatal, connectivity error = non-fatal (the key distinction)
Get-SPSADConnection/Get-SPSADUserthrow a terminatingSPSADConfigErroron a build-time misconfiguration (missingLdapPath/CredentialKey, undecodable/missing secret). A genuine lookup miss still returns$null.Fail loud where it matters
Get-SPSADConnectionError(new public) — query-free per-forest secret pre-flight.SPSyncUserProfile.ps1pre-flights every credential-mode forest in the input JSON andExit 1s with an actionable message when a secret cannot be decoded on this server — no more silentUNKNOWN_USERen masse.SPSyncUserInfoList.ps1(parallel) fails the run naming the affected forest(s) on a config/secret error, instead of shipping an empty-name forest.Test-SPSADUserpropagates config errors (still$true/$falsefor found/not-found).Docs
Validation
Test-SPSADUserpropagation,Get-SPSADConnectionErrorpre-flight)../src: clean. Manifest valid (1.3.2, 17 functions).Scope note
The connectivity/RGA semantic (whether the profile step should trust the JSON and create a profile when AD is unreachable, vs skip-and-retry) is intentionally deferred to a later release, pending field confirmation. This PR does not change that path beyond keeping it non-fatal.
Upgrade notes
Drop-in replacement for 1.3.1. Run
Test-SPSUserSyncReadiness.ps1on each server (application farms and the UPA master), signed in as the service account, to confirm every forest's secret decodes and binds before enabling the scheduled tasks.