Skip to content

Sync-DbaLoginPassword - Output the Failed row of a login that could not be synced - #10804

Merged
potatoqualitee merged 1 commit into
developmentfrom
sync-dbaloginpassword-failed-row
Oct 19, 2026
Merged

potatoqualitee merged 1 commit into
developmentfrom
sync-dbaloginpassword-failed-row

Conversation

@andreasjordan

Copy link
Copy Markdown
Collaborator

Found while working #10772 and #10803.

Problem

When Sync-DbaLoginPassword cannot set the password of a login, it writes a warning and no output row. The help promises "one object per login password synchronized, whether successful or failed".

The Failed row was there, but after Stop-Function -Continue. Without -EnableException, -Continue goes on with the next login right away, so the row was never written. A caller that checks Status sees nothing for the login that failed.

A password hash that could not be read from the source had the same result: a warning, continue, and no row.

Reproduced in the lab before #10803: a login with CHECK_POLICY on fails with error 33020 in Set-DbaLogin -PasswordHash, and Sync-DbaLoginPassword returned nothing for it.

Change

  • The Failed row is written before Stop-Function. The warning and the -EnableException behaviour stay as they were.
  • A hash that could not be read is thrown inside the try, so it takes the same path and gets its Failed row as well.

Tests

New context "Reports a login whose password could not be synced" (InstanceMulti1 to InstanceMulti2, like the rest of the file). A server DDL trigger on the destination, limited to the test login, refuses its ALTER LOGIN, so the failure does not depend on #10803 or on the password policy of the login. It is dropped in AfterAll.

  • Without -EnableException: one row with Status Failed, the trigger's error in Notes, and the warning.
  • With -EnableException: the command throws, as before.

Lab, Windows PowerShell 5.1:

  • This branch: 11 of 11 pass.
  • The same tests against the Sync-DbaLoginPassword.ps1 of development: the new row test fails with "Expected 'dbatoolsci_syncpwdfail_...', but got $null".

🤖 Generated with Claude Code

…ot be synced

The Failed row came after Stop-Function -Continue, which goes on with the
next login, so it was never written: a failure only showed as a warning,
although the help promises one row per login whether it succeeded or failed.
The row is now written before Stop-Function.

A password hash that could not be read skipped the login with a warning and
no row as well; it now goes through the same failure path.

(do Sync-DbaLoginPassword)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@andreasjordan
andreasjordan marked this pull request as ready for review October 18, 2026 16:47

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed at b7561c7caf2f2930afe9a75f35a1d06d3da93da8. No material defect found.

The failed result is now emitted before Stop-Function -Continue transfers control to the next login. Reviewed the complete patch, source/hash/flow-control helpers, tests, description, discussions, and CI. Warning mode retains later logins/destinations; EnableException still throws; success, filtering, and ShouldProcess behavior remain coherent.

The new regression uses a real SQL DDL trigger to reject the targeted login change and checks the Failed result, error, warning, and exception behavior. The exact-head MULTI job ran and passed Sync-DbaLoginPassword.Tests.ps1. All 20 executed head checks passed and one Windows check was skipped. SQL integration tests were not rerun locally.

@potatoqualitee
potatoqualitee merged commit e5e8177 into development Oct 19, 2026
22 checks passed
@potatoqualitee
potatoqualitee deleted the sync-dbaloginpassword-failed-row branch October 19, 2026 15:27
@potatoqualitee

Copy link
Copy Markdown
Member

thank you kindly

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.

2 participants