Repository navigation
Set-DbaLogin - Set -PasswordHash on logins with CHECK_POLICY on - #10803
Conversation
SQL Server refuses a hashed password for a login with CHECK_POLICY on (error 33020), which is the default of CREATE LOGIN, so -PasswordHash failed for most SQL logins. The policy is now turned off and set back around it in one transaction, so it is never left off. Sync-DbaLoginPassword sets passwords through this and failed the same way. Also fixes two more defects of the command: - -PasswordHash escaped the login name for a string literal inside brackets, so a name with a quote was not found and one with a closing bracket broke the statement. - -NewName checked for an existing name in the logins of -SqlInstance, which is empty for piped logins, so the rename failed with a warning and no output instead of the note. (do Set-DbaLogin) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
potatoqualitee
left a comment
There was a problem hiding this comment.
Reviewed at 316ef6b2f86d06777f74c4762e68d3eed35e7cc7. Requesting changes for one material transaction-ownership defect: a failed hash change on a piped login can roll back the caller's pre-existing transaction and unrelated pending changes.
I independently traced the exact-head shared-connection path and exercised BeginTransaction/BeginTransaction/RollBackTransaction in CaptureSql mode with the exact pinned SMO assembly (file version 18.100.1.19). It generates two BEGIN TRANSACTION statements followed by an unnamed ROLLBACK TRANSACTION, which SQL Server applies to the outer transaction too. The prior policy-off hash path did not issue that rollback.
The complete patch, surrounding source, tests, description, discussions, and exact-head CI were reviewed. The policy restoration, identifier escaping, and piped rename correction otherwise address their stated defects. All 20 executed head checks passed and one Windows check was skipped. The new tests do not exercise caller-owned transactions. The live SQL failure sequence was not executed locally.
| # A failed ALTER LOGIN leaves the transaction open, and so does a stopped pipeline. | ||
| if (-not $committed) { | ||
| try { | ||
| $server.ConnectionContext.RollBackTransaction() |
There was a problem hiding this comment.
[P2] Preserve the caller's transaction on a hash-setting failure
A piped login uses its existing Parent.ConnectionContext. If the caller has already started a transaction on that connection, the new BeginTransaction nests inside it, but this RollBackTransaction does not roll back just the nested operation.
Concrete path: with XACT_ABORT off, obtain a policy-off SQL login, start a transaction through its parent's ConnectionContext, make an unrelated pending change, then pipe that login into Set-DbaLogin -PasswordHash 0x1234. That value passes this command's hexadecimal validation but is rejected as a SQL password hash. The new finally then rolls back the caller's entire transaction, discarding the preceding work. The old policy-off path executed only ALTER LOGIN without this rollback.
Using the exact pinned SMO assembly in CaptureSql mode, I independently captured BEGIN TRANSACTION, BEGIN TRANSACTION, then if (@@trancount > 0) ROLLBACK TRANSACTION. SQL Server documents that an unnamed rollback unwinds all nested transactions and sets @@TRANCOUNT to zero.
Establish transaction ownership before mutation. For an existing caller transaction, use appropriately scoped savepoint recovery where supported, or reject the operation before changing anything. Only roll back the entire transaction when this command owns it. Add a regression asserting that a rejected hash preserves both the caller's transaction and its prior pending work.
There was a problem hiding this comment.
Confirmed, thanks. Fixed in 104972a: the command reads @@TRANCOUNT before it changes anything. Without an open transaction it begins and commits its own, as before. Inside a transaction of the caller it sets a savepoint. After a failure it rolls back to that savepoint, as long as XACT_STATE() is 1, and it leaves the commit to the caller. A transaction that can no longer commit can only be rolled back as a whole, so that is left to the caller as well.
The new context "Sets the password from a hash inside a transaction of the caller": a non-pooled connection begins a transaction and creates a temporary table in it. A login with CHECK_POLICY on is read through that connection and piped into -PasswordHash 0x1234, then into a valid hash. After the refused hash, @@TRANCOUNT is still 1, the table is still there, and the hash and the policy are as before. After the valid hash, the hash is set, the policy is on, and the transaction of the caller is still open.
Copy-DbaLogin -UpdateExisting in #10801 has the same pattern and gets the same fix there.
Lab, InstanceSingle on SQL Server 2019: Set-DbaLogin.Tests.ps1 passes 59 of 59 on Windows PowerShell 5.1 and on PowerShell 7.6, and Sync-DbaLoginPassword.Tests.ps1, which sets passwords through this, passes 9 of 9 on 5.1. With the Set-DbaLogin.ps1 of 316ef6b, two of the new tests fail with "Expected 1, but got 0": after the refused hash, the transaction of the caller and its table are gone, and after the valid hash the transaction is no longer open.
This text was created by Claude and reviewed by Andreas Jordan.
A piped login brings the connection of its server, and the caller can have a transaction open on it. -PasswordHash then began a nested transaction, and after a refused hash its rollback, which SMO sends as an unnamed ROLLBACK TRANSACTION, took back the whole transaction of the caller with the work done in it. Before the policy toggle the hash was a single ALTER LOGIN without a rollback. The command now begins a transaction only when none is open. Inside a transaction of the caller it sets a savepoint, takes its statements back to it after a failure as long as the transaction can still commit, and leaves the commit to the caller. The new context pipes a login of a connection with an open transaction, first with a value SQL Server refuses as a hash and then with a valid one, and checks on that connection that the transaction and its temporary table are still there and the policy is on. (do Set-DbaLogin) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
potatoqualitee
left a comment
There was a problem hiding this comment.
Reviewed at 104972ad3ce22d0a350299b22c8c090ef0c218c6.
The previous caller-transaction blocker is fixed. The command checks transaction ownership, uses a savepoint within an existing transaction, rolls back only to that savepoint after a recoverable failure, and leaves the caller's transaction open. The new integration context checks both @@TRANCOUNT and unrelated caller work after a rejected hash, then verifies a successful hash update in the same transaction.
Reviewed the complete two-file patch, surrounding source, tests, PR description, existing discussions, compatibility, error continuation, and CI. Policy restoration, bracket escaping, and piped rename-conflict handling also address the stated defects. No remaining material defect found.
A focused PowerShell 7.6.7 probe of the exact hash-setting code confirmed owned versus caller-owned transaction behavior on success and failure, plus identifier construction. The exact-head inventory has 20 successful checks and three skipped checks. This probe used a recording connection boundary; live SQL Server integration tests were not rerun locally.
|
🙏🏼 |
Found while working #10772 (#10801). Fixes three defects of
Set-DbaLogin.Problem
-PasswordHashfails for logins withCHECK_POLICYon. SQL Server refusesALTER LOGIN ... WITH PASSWORD = 0x... HASHEDfor such a login with error 33020, "A HASHED password cannot be set for a login that has CHECK_POLICY turned on". This also happens whenCHECK_POLICY = ONis in the same statement.CHECK_POLICYon is the default ofCREATE LOGIN, so this hits most SQL logins.Sync-DbaLoginPasswordsets passwords throughSet-DbaLogin -PasswordHashand fails the same way. The existing tests of both commands create their logins with the policy off, which is why nobody noticed.-PasswordHashescaped the login name for a string literal. The name was put into brackets with'doubled, so a login namedo'brienbecame[o''brien]and was not found, and a name with]broke the statement.-NewNamedid not see existing names for piped logins. The check looked in the logins collected for-SqlInstance, which is empty when the logins come through the pipeline. The rename was attempted, failed with a warning, and the login had no output row instead of the noteNew login name already exists.Change
CHECK_POLICYon, the policy is turned off, the hash set, and the policy and expiration set back to their values. This is done in one transaction throughBeginTransaction/CommitTransactionof the SMO connection, and afinallyrolls back when a statement fails or the pipeline is stopped, so the policy is never left off. When the caller already has a transaction open on that connection, a savepoint takes the place of the own transaction (see "After review" below). A login with the policy off gets the single statement as before. It runs after theAlter()of the command, so a-PasswordPolicyEnforcedin the same call is already in place and is what gets set back. Turning the policy off and on clears a lockout of the login, as-Unlock -Forcedoes; the help says so.]to]]).This is the same pattern as in #10801 for
Copy-DbaLogin -UpdateExisting. It is lab-verified on SQL Server 2019, 2022 and 2025: 33020 on every version, the policy toggle works in a transaction, and it does not changePasswordLastSetTime.Tests
New context "Sets the password from a hash" on
InstanceSingle, reading state fromsys.sql_logins:CHECK_POLICYandCHECK_EXPIRATIONon gets the hash, both stay on, and the login can connect with the password the hash came from.A further test pipes a login into
-NewNamewith an existing name and expects the note and no warning.Lab, Windows PowerShell 5.1,
InstanceSingleSQL Server 2019:Set-DbaLogin.ps1ofdevelopment: the four new tests fail, each with its defect: the rename warning, error 33020, the failed login, and "Incorrect syntax near ..." for the bracket.Sync-DbaLoginPasswordfrom SQL Server 2022 to 2025 with a policy-on login:development: the 33020 warning, no output row, hash not synced.Success, the hash matches, the policy stays on.After review: a transaction of the caller (104972a)
A piped login brings the connection of its server, and the caller can have a transaction open on it. The first version began a nested transaction there, and after a refused hash its rollback, which SMO sends as an unnamed
ROLLBACK TRANSACTION, took back the whole transaction of the caller and the work done in it. The command now reads@@TRANCOUNTbefore it changes anything:XACT_STATE()is 1, and leaves the commit to the caller. A transaction that can no longer commit can only be rolled back as a whole, which is up to the caller.New context "Sets the password from a hash inside a transaction of the caller": a non-pooled connection begins a transaction and creates a temporary table in it. A login with
CHECK_POLICYon is read through that connection and piped into-PasswordHash 0x1234, which SQL Server refuses after the policy was turned off, and then into a valid hash. All state is read on that connection. After the refused hash,@@TRANCOUNTis 1, the table exists, and hash and policy are as before. After the valid hash, the hash is set, the policy is on and the transaction is still open.Lab,
InstanceSingleSQL Server 2019:Set-DbaLogin.Tests.ps159 of 59 on Windows PowerShell 5.1 and on PowerShell 7.6,Sync-DbaLoginPassword.Tests.ps19 of 9 on 5.1. With theSet-DbaLogin.ps1of 316ef6b, two of the new tests fail with "Expected 1, but got 0".Copy-DbaLogin -UpdateExistingin #10801 had the same pattern and gets the same fix.Not changed here
Sync-DbaLoginPasswordemits itsFailedrow afterStop-Function -Continue, so the row is never written. That command is not touched here.🤖 Generated with Claude Code