Skip to content

Copy-DbaLogin - Add -UpdateExisting to update changed logins in place - #10801

Open
andreasjordan wants to merge 11 commits into
developmentfrom
copy-dbalogin-update-existing
Open

andreasjordan wants to merge 11 commits into
developmentfrom
copy-dbalogin-update-existing

Conversation

@andreasjordan

@andreasjordan andreasjordan commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #10772. The Sync-DbaAvailabilityGroup pass-through follows in #10802, stacked on this one.

Problem

Copy-DbaLogin has two ways to handle a login that already exists on the destination:

  • Without -Force it skips the login, so a password changed on the source never reaches the destination.
  • With -Force it gives the databases and Agent jobs the login owns to sa, then drops and recreates the login. A login with open sessions can only be dropped with -KillActiveConnection.

Sync-DbaAvailabilityGroup calls Copy-DbaLogin -Force:$Force, so an AG sync gets one of the two: stale passwords on the secondaries, or every login dropped on every run.

Change

New switch -UpdateExisting

An existing login is compared with the source and updated in place when it differs:

Login type Compared
SQL login password hash (Get-LoginPasswordHash), CHECK_POLICY, CHECK_EXPIRATION
SQL, Windows and external logins default database, default language
all disabled state

The differences are applied with ALTER LOGIN in one transaction, through BeginTransaction/CommitTransaction of the SMO connection, so the caller's connection is not left with SET XACT_ABORT ON. If one statement fails, or the pipeline is stopped, a finally rolls back the earlier statements and the login stays as it was. When the caller already has a transaction open on the destination connection, a savepoint takes the place of the own transaction (see "After review" below). SID, owned databases and jobs, and open sessions are kept.

SQL Server refuses PASSWORD = 0x... HASHED for a login with CHECK_POLICY on (error 33020, on 2022 and 2025). CHECK_POLICY on is the default of CREATE LOGIN, so the password path turns the policy off, sets the hash and sets the policy back to the source's values, all in the same transaction. This is the same off-and-on that Set-DbaLogin -Unlock uses, so it also clears the lockout state of the destination login. The toggle does not change PasswordLastSetTime.

The rest of the behaviour:

  • Different SID: an in-place update cannot change it. The login is skipped with a warning, because database users that come from the source, for example in an availability group database, are not mapped to it. With -Force as well, only these logins are dropped and recreated. -UpdateExisting takes precedence over -Force the same way -UseLastModified does in Copy-DbaAgentJob. -NewSid turns the SID check off.
  • Permissions: Update-SqlPermission runs for every existing login, whether it differed or not (unless -ExcludePermissionSync), which is what -Force does for every login it recreates.
  • Default database missing on the destination: the destination's default database is left as it is.
  • Skipped as with -Force: the login that performs the migration, the destination service account, BUILTIN\Administrators on Linux, and a high-privilege Windows group that the current login may depend on (Copy-DbaLogin may lead to unaccessable SQL instance #8572). An update can disable a group, or the permission sync can take it out of a server role, just like a drop can lock the caller out.
  • Output: an updated login reports Successful and lists what changed in Notes, for example Updated in place: password, default database. An identical login reports Skipped with Login is identical on source and destination. Both go on to -SyncSaName, like a created login.
  • Without the new switch nothing changes.

One effect worth knowing, documented in the help: SQL Server 2025 stores password hashes in a new format (0x0300 instead of 0x0200) and converts an older hash at the first successful login. A login that connected to a 2025 destination while the source still has the older format therefore has a different hash for the same password, and the next run sets that same password again. The two formats cannot be compared without the clear-text password.

Logins that -Force dropped and could not recreate

A SQL login can only be created from a password hash in a format the destination knows. Two cases where the destination does not:

  1. 2012+ source, older destination. The existing check called Stop-Function without -Continue or return, and has done so since 2017. Without -EnableException the copy went on after the "not supported" warning, and -Force dropped the destination login before the create failed.
  2. SQL Server 2025 hashes. SQL Server 2019 refuses a 0x0300 hash with error 15021, and older versions cannot know the format either. There was no check at all.

Both are now one check per SQL login, done before anything is dropped. A 0x0200 hash is skipped for destinations older than 2012, and a 0x0300 hash for destinations older than 2022, each with a warning and a Skipped row. Windows logins are still copied to an older destination, as they were without -EnableException. With -EnableException the old check threw at the first login; now the affected SQL logins are skipped. SQL Server 2022 accepted and verified a 0x0300 hash on 16.0.4255.1, the build in the lab; older 2022 builds were not tested. The check of the whole destination for SQL Server 7 stays and now stops as well (-Continue).

Reproduced with -Force from SQL Server 2025 to 2019: on development the destination login is dropped and the copy reports Failed; with this branch it is skipped and stays.

Tests

tests/Copy-DbaLogin.Tests.ps1, from InstanceCopy1 to InstanceCopy2. Login state is read from sys.sql_logins, not from cached SMO objects. Two new contexts:

Existing logins are updated in place:

  • A login with the same SID and CHECK_POLICY on, but a different password, expiration setting, default database, language and disabled state, is updated in place. Its principal_id stays the same, and its hash and properties now match the source.
  • A repeated run reports the login as identical and leaves PasswordLastSetTime unchanged.
  • A login with a different SID is skipped with the warning and keeps its SID. With -Force added, it is recreated with the source SID.
  • The updated login can connect to the destination with the source password. This test runs last, because on a 2025 destination the connection converts the hash.

A failing statement rolls the whole update back:

  • A server DDL trigger, limited to the test login, fails the last statement of the update (DISABLE) with RAISERROR and XACT_ABORT OFF. Like an error of the statement itself, that leaves the transaction open, so only the command's rollback can restore the login. (A THROW would let the server roll the transaction back on its own and prove nothing.) The update reports Failed with the trigger's error. The hash, the policy (still on) and the enabled state are unchanged, although the earlier statements had already run.
  • The command runs on a non-pooled connection the test owns, and @@TRANCOUNT on it is 0 afterwards. With the rollback call removed, this test fails with "Expected 0, but got 1" (checked in the lab). Before this assertion existed, the same mutation made the next read wait on the uncommitted update's locks indefinitely.
  • After the trigger is dropped, the next run applies all of it. This also covers source policy off, destination policy on, different password.

UpdateExisting was added to the parameter validation test.

The file passes in the lab under Windows PowerShell 5.1 (Copy1 SQL Server 2022, Copy2 SQL Server 2025): 33 tests, 31 passed, 2 skipped (the Linux context). The hash format check cannot get a regression test: no CI or lab instance runs SQL Server 2008 R2, and no Copy instance pair has a 2025 source with a destination older than 2022. The 2025 to 2019 case was reproduced by hand in the lab, as described above.

After review

Server roles with a rename (d493f97). Update-SqlPermission checked whether the destination login is in a server role the source login is not in, and then dropped the name of the source login from that role. Without a rename both names are the same. With -LoginRenameHashtable they differ, and -UpdateExisting reaches this with the memberships the existing login already has: the renamed login stayed in the role, and a different login that happens to have the name of the source login lost it. It now drops the renamed login. The ShouldProcess message of that block said "Adding" for the removal, and both messages named the undefined $destRoleName; both are fixed. New context: a source login in no server role is renamed to a destination login with its SID that is a sysadmin, next to another sysadmin with the name of the source login. Afterwards the renamed login is no longer a sysadmin and the other one still is.

A transaction of the caller (000bc66). The review of #10803 found that the rollback of the update, which SMO sends as an unnamed ROLLBACK TRANSACTION, takes back a transaction the caller has open on the same connection, with the work done in it. A destination server object of the caller has that pattern here too. The update now reads @@TRANCOUNT before it changes anything: without an open transaction it uses its own as before; inside a transaction of the caller it sets a savepoint, rolls back to it after a failure as long as XACT_STATE() is 1, and leaves the commit to the caller. New context: the trigger of the rollback context fails the last statement of an update inside a transaction of the test that has created a temporary table. On that connection, the transaction and the table are still there afterwards, and the login is as it was.

A bare continue in Update-SqlPermission (34a8ddc). For a destination older than SQL Server 2005, the helper ended its database mapping part with continue in a function without a loop of its own. That went on with the next iteration of the loop of the caller: Sync-DbaLoginPermission, which accepts SQL Server 2000 destinations, lost the rest of the iteration, so every login came without a result row, neither Successful nor Failed. Copy-DbaLogin calls the helper inside a loop of its own and was not affected. It is a return now. No lab or CI instance runs SQL Server 2000, so there is no regression test; the binding was checked with a probe of the same shape on Windows PowerShell 5.1 and PowerShell 7.6. The other 40 bare continue/break without a local target in the module are in #10806.

Server roles that exist on the destination only (89979b5). Update-SqlPermission walked the server roles of the source only. A user-defined server role on the destination only was never visited, so a login updated in place kept its membership there, and with it a permission like CONTROL SERVER of that role, while a login that -Force drops and recreates loses it. The helper now also walks the destination roles the source does not have and takes the login out of them; the role and its other members stay. Sync-DbaLoginPermission, the other caller, removes such a membership now as well. New context: on the destination only, a role with CONTROL SERVER has the login and a second login as members; afterwards only the second one is.

Direct server permissions when the source login has none (b3c7709). Update-SqlPermission compared the direct server permissions of the destination login with those of the source inside the loop over the source permissions. For a source login without any, one that gets CONNECT SQL through a server role for example, that loop never ran, so a login updated in place kept every direct permission it had on the destination, CONTROL SERVER included. The comparison runs once after the loop now. The other paths through the helper change the same way: Sync-DbaLoginPermission, and a login that -Force recreates, lose a direct CONNECT SQL that the source login does not have, as they already did whenever the source login had any other direct permission. New context: the source login connects through a user-defined server role with CONNECT SQL, its own CONNECT SQL revoked; the destination login has CONNECT SQL and CONTROL SERVER granted directly. Afterwards neither is granted, the login is in the role, and it connects with the source password.

A password hash that cannot be read (0177c40). The check before the drop (see "Logins that -Force dropped and could not recreate" above) read the password hash only for a source of SQL Server 2012 or later and a destination older than SQL Server 2022, and it took a hash it could not read for a compatible one. LOGINPROPERTY returns no hash to a source connection without CONTROL SERVER, even when that connection can see the logins. So -Force, and -UpdateExisting -Force for a login with a different SID, dropped the destination login, and New-DbaLogin then failed with "Invalid cast from 'System.DBNull' to 'System.Byte[]'" and left the login missing. development does the same with -Force. The hash is now read for every SQL login before anything is dropped, and a hash that could not be read gives a Failed row ("Could not read the password hash") with a warning. The format check runs for the same versions as before. New context: the source connection belongs to a login with VIEW ANY DEFINITION only, which sees the logins but gets no hash; after -Force and after -UpdateExisting -Force, the destination login with its own SID is still there with that SID.

SQL Server 2000 and 2005 (2c3bfc3). Since #10813, Connect-DbaInstance refuses every instance older than SQL Server 2008, so the skip of -UpdateExisting for a destination older than SQL Server 2005 is gone, as the version guide now asks. The branch of 34a8ddc above is unreachable since then as well; it stays, because removing it would mean taking apart the 2005 check around the server permissions of the helper too.

The grant option and DENY of a server permission (5f8f777, f69a33f). The review of 1e9a458ee found that a login updated in place with a server permission WITH GRANT OPTION on the destination and without it on the source lost the permission: the GRANT leaves the grant option in place, and the sync then revoked the permission itself with CASCADE and never granted it again, CONNECT SQL included. Only the grant option is revoked now (REVOKE GRANT OPTION FOR ... CASCADE). Found on the way: a DENY of a permission that the destination login has with grant option failed with "To revoke or deny grantable privileges, specify the CASCADE option.", and the sync then revoked the permission, so the login had neither. The DENY runs with CASCADE now. Sync-DbaLoginPermission had both defects. New context: the destination login has CONNECT SQL, VIEW SERVER STATE and ALTER TRACE with grant option, the source login the first two without it and a DENY of the third; afterwards they are GRANT, GRANT and DENY, without a warning, and the login connects.

Permissions on logins and server roles (19a92e9). The same review found that only the permissions on the server (class 100) were synced, so a login updated in place kept IMPERSONATE ON LOGIN::sa or CONTROL ON SERVER ROLE that only the destination login had, while -Force drops them with the login. Update-SqlPermission now compares the permissions on logins and server roles (Server.EnumObjectPermissions, class 101) by securable and permission and revokes those the source login does not have, or only their grant option. Nothing is granted, as for a created login. Permissions on endpoints and availability groups stay, because they belong to the replica: a secondary grants CONNECT on its endpoint to the service account of the primary, which has no such permission on the primary. The help of -UpdateExisting and of Sync-DbaLoginPermission says so, and Sync-DbaLoginPermission revokes these permissions now as well. New context: IMPERSONATE on another login is revoked, VIEW DEFINITION that the source login has too stays, CONTROL on a server role loses its grant option, CONNECT on an endpoint stays, and another login keeps its IMPERSONATE on the same login.

The SHAs in the paragraphs above these two are from before Chrissy rebased the branch onto development on November 11; the commits kept their titles.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 57 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6, no warnings; Sync-DbaLoginPermission.Tests.ps1 10 of 10 and dbatools.Tests.ps1 87 passed and 6 skipped on both. With the Update-SqlPermission.ps1 of 1e9a458ee, 6 of the 9 new tests fail: the login of the grant option context ends with no server permission at all and cannot connect, the DENY warns "specify the CASCADE option", IMPERSONATE is still granted, and CONTROL keeps its grant option. The round before: Copy-DbaLogin.Tests.ps1 47 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6, no warnings; Start-DbaMigration.Tests.ps1, which calls Copy-DbaLogin, 15 of 15 and dbatools.Tests.ps1 81 passed and 6 skipped on Windows PowerShell 5.1. With the Copy-DbaLogin.ps1 of b3c7709, both new tests fail with the note "Invalid cast from 'System.DBNull' to 'System.Byte[]'", and the destination login is gone. Earlier rounds: Copy-DbaLogin.Tests.ps1 44 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6; Sync-DbaLoginPermission.Tests.ps1, the other caller of Update-SqlPermission, 10 of 10 on both; dbatools.Tests.ps1 81 passed and 6 skipped on both. With the Copy-DbaLogin.ps1 and Update-SqlPermission.ps1 of 41af274, three of the new tests fail: the renamed login stays a sysadmin, the other login loses the role, and the transaction of the caller is gone. With the Update-SqlPermission.ps1 of 34a8ddc, the destination-only role test fails. With the one of 89979b5, the direct permission test fails: CONNECT SQL and CONTROL SERVER are still granted.

Found on the way, not changed here

  • Set-DbaLogin -PasswordHash runs ALTER LOGIN ... HASHED without turning the policy off, so it fails with error 33020 for every login with CHECK_POLICY on. Sync-DbaLoginPassword sets the password through it and fails the same way. Their tests create logins with the policy off, which is why this went unnoticed. They are separate commands, so a separate PR.
  • New-DbaLogin -SecurePassword hashes the password on the client in the 0x0200 format, so a login it creates on SQL Server 2025 does not get the new format until its first login.
  • Copy-DbaLogin copies no permissions on logins, server roles, endpoints or availability groups, neither to a created login nor with -Force or -UpdateExisting. A feature of its own, not a defect of this PR.

🤖 Generated with Claude Code

@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 41af2743919c222a4f2a370383c14aa1d1bf45fb. Requesting changes for one material permission-sync defect: an in-place update with a login rename mapping can remove a server-role membership from the wrong destination login and leave the intended target overprivileged.

The new path reaches the existing helper's wrong-name removal with pre-existing memberships still present. I independently executed the exact-head Update-SqlPermission function under PowerShell 7.6.7 with in-memory role objects: it removed source_app while dest_app remained in sysadmin. This validates principal selection; it was not a live SQL Server integration test.

The complete patch, surrounding source, tests, intended behavior, discussions, and CI were reviewed. All 20 executed head checks passed and one generic Windows check was skipped. The new tests do not cover permission removal with renamed existing logins.

Comment thread public/Copy-DbaLogin.ps1
ObjectLevel = $ObjectLevel
ExcludeDatabaseMapping = $ExcludeDatabaseMapping
}
Update-SqlPermission @splatPermission

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.

[P1] Remove server-role membership from the mapped destination login

This new path passes the differently named source and destination logins to Update-SqlPermission while retaining the destination's existing memberships. That helper checks $destRoleMembers -contains $newLoginName but calls $destRole.DropMember($loginName) (private/functions/Update-SqlPermission.ps1:98-101).

Concrete path: source source_app is not sysadmin; destination dest_app has the same SID as source source_app and is sysadmin; a separate destination login named source_app is also sysadmin. Run Copy-DbaLogin with -Login source_app -LoginRenameHashtable @{ source_app = "dest_app" } -UpdateExisting. The SID check passes, but permission sync removes destination source_app from sysadmin and leaves dest_app in sysadmin. The intended target keeps excess privileges and an unrelated principal loses access. The old skip/drop-and-recreate paths did not retain these existing target memberships.

I independently ran the exact helper in PowerShell 7.6.7 with those role memberships: removed [source_app], remaining [sa, dest_app]. Correct the helper to remove $newLoginName, and add a real SQL regression combining UpdateExisting and LoginRenameHashtable that checks both destination principals.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, thanks. Fixed in d493f97: Update-SqlPermission now drops $newLoginName from the destination role. The same block also said "Adding" in its ShouldProcess message for the removal and named the undefined $destRoleName in both messages; both are fixed.

The new context "The permissions of a renamed login are synced to the login it is renamed to" uses your setup: the source login is in no server role and is renamed to a destination login with its SID that is a sysadmin, next to another sysadmin that has the name of the source login. After Copy-DbaLogin -LoginRenameHashtable ... -UpdateExisting, the renamed login is no longer a sysadmin and the other one still is.

Your review of #10803 found the rollback of a caller's transaction there. -UpdateExisting has the same pattern when the destination is a server object of the caller with a transaction open, so 000bc66 applies the same fix here: inside a transaction of the caller the update sets a savepoint and takes its statements back to it, otherwise it uses its own transaction as before. A second new context fails the last statement of an update inside a transaction of the test and checks on that connection that the transaction and the temporary table created in it are still there and the login is as it was.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 37 passed and 2 skipped (the Linux context) on Windows PowerShell 5.1 and on PowerShell 7.6, Sync-DbaLoginPermission.Tests.ps1, the other caller of Update-SqlPermission, 10 of 10 on 5.1. With the Copy-DbaLogin.ps1 and Update-SqlPermission.ps1 of 41af274, three of the new tests fail: the renamed login stays a sysadmin, the other login loses the role, and the transaction of the caller is gone after the failed update.


This text was created by Claude and reviewed by Andreas Jordan.

@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 34a8ddc78f669205f62b6abaa4606c62d94c938d. The previous wrong-principal removal is fixed, and the caller-owned transaction savepoint handling is sound.

Requesting changes for one material permission-sync defect in the new in-place path: a login can retain membership in a destination-only custom server role, including a role granted CONTROL SERVER, despite the documented synchronization of permissions. Adding -UpdateExisting to -Force now preserves that membership instead of removing it through replacement.

I independently executed the exact-head Update-SqlPermission implementation under PowerShell 7.6.7 with recording role objects. It correctly removed the mapped destination login from the shared sysadmin role, but never enumerated the destination-only role and left its membership unchanged. The inline finding gives the realistic SQL setup and required correction. This verifies the principal-selection/control-flow failure; it was not a live SQL Server integration run.

The complete patch, surrounding source, tests, PR description, existing discussions, transaction/cleanup paths, and CI were reviewed. All 20 executed exact-head checks passed; one Windows check was skipped. The current role-removal regression uses a built-in role present on both instances and does not cover this case.

Comment thread public/Copy-DbaLogin.ps1
ObjectLevel = $ObjectLevel
ExcludeDatabaseMapping = $ExcludeDatabaseMapping
}
Update-SqlPermission @splatPermission

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.

[P1] Reconcile memberships in destination-only server roles

This new in-place path calls Update-SqlPermission on the existing principal, but that helper only iterates SourceServer.Roles (private/functions/Update-SqlPermission.ps1:64). A custom role present only on the destination is never visited.

Concrete case: app_login exists on both servers with the same SID. On the destination only, create destination_admin, grant it CONTROL SERVER, and add app_login:

CREATE SERVER ROLE destination_admin AUTHORIZATION sa;
GRANT CONTROL SERVER TO destination_admin;
ALTER SERVER ROLE destination_admin ADD MEMBER app_login;

Now run Copy-DbaLogin -Source source -Destination destination -Login app_login -UpdateExisting -Force. Matching SIDs select this branch, so app_login keeps the destination-only membership and its elevated permissions. Without -UpdateExisting, the existing -Force drop/recreate path clears the old principal's membership. The new help promises permissions synchronized as after a copy, with -ExcludePermissionSync as the opt-out.

I independently ran this exact helper with recording role objects: shared-role removal worked, but the destination-only role had zero enumerations/removals and retained the target login. Microsoft documents user-defined server roles as grantees of server permissions, including CONTROL SERVER.

Reconcile destination memberships as well as source roles, removing the mapped target from roles it does not belong to on the source, including roles absent there. Preserve the role and unrelated members. Add a SQL regression for a destination-only role; the new sysadmin regression cannot detect this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, thanks. Fixed in 89979b5: Update-SqlPermission walked the server roles of the source only, so a role that exists on the destination only was never visited. It now also walks the destination roles the source does not have and takes the login out of them; the role and its other members stay. Sync-DbaLoginPermission uses the same helper and removes such a membership now as well, which is what its description of a sync promises.

The new context "A server role that exists on the destination only loses the login updated in place" uses your setup: on the destination only, a user-defined server role with CONTROL SERVER has the login (same SID as on the source) and a second login as members. After Copy-DbaLogin -UpdateExisting, the login is no longer a member, and the role and the other member are still there.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 40 passed and 2 skipped (the Linux context) on Windows PowerShell 5.1 and on PowerShell 7.6, Sync-DbaLoginPermission.Tests.ps1 10 of 10 on 5.1. With the Update-SqlPermission.ps1 of 34a8ddc, the new test fails: the login is still a member of the role.

#10802, stacked on this one, has the commit as well (62d22b9).


This text was created by Claude and reviewed by Andreas Jordan.

@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 89979b5.

The previous renamed-principal, destination-only-role, and caller-transaction findings are addressed.

One material permission-sync defect remains in the new in-place path: when the source login has no direct server permissions, Update-SqlPermission never enumerates or revokes destination-only direct grants. A login that inherits CONNECT SQL through a server role can therefore retain a stale direct CONTROL SERVER grant on the destination after an UpdateExisting sync.

I independently executed the exact helper under PowerShell 7.6.6 with recording objects: the source permission list was read once, the destination list was not read, and no revoke occurred. Microsoft's SMO source confirms that the source enumeration contains direct grants rather than inherited role permissions. The inline finding provides the concrete configuration and required correction. This was not a live SQL Server integration run.

Reviewed the complete patch, surrounding source, tests, description, existing discussions, transaction/error/cleanup paths, and CI. All 20 executed exact-head checks passed; the generic Windows job and two Claude jobs were skipped. The COPY job actually ran and passed Copy-DbaLogin.Tests.ps1; the current tests do not cover an empty source server-permission list.

Comment thread public/Copy-DbaLogin.ps1
ObjectLevel = $ObjectLevel
ExcludeDatabaseMapping = $ExcludeDatabaseMapping
}
Update-SqlPermission @splatPermission

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.

[P1] Reconcile destination permissions when the source list is empty

This new in-place call preserves the destination principal, but Update-SqlPermission only enumerates and revokes destination server permissions inside foreach ($perm in $SourceServer.EnumServerPermissions($loginName)). When the source has no direct server permissions, that entire reconciliation is skipped.

A concrete case is a source login that receives CONNECT SQL through a user-defined server role: grant CONNECT SQL to the role, add app_login, then REVOKE CONNECT SQL FROM app_login. Create the same-SID login and corresponding role on the destination, with an additional direct GRANT CONTROL SERVER TO app_login. Copy-DbaLogin -UpdateExisting -Force updates the existing login but leaves CONTROL SERVER in place. Force-only replacement removes that grant when it drops the principal.

SMO's server-permission enumeration filters sys.server_permissions by the named grantee; it does not include inherited role permissions. I independently executed this exact helper under PowerShell 7.6.6 with recording server objects: the empty source list was read once, the destination list was never read, and no revoke occurred. This was a control-flow probe, not a live SQL Server integration run.

Move destination permission reconciliation outside the source-permission loop so it runs even for an empty source list. Add a regression asserting removal of the extra direct destination grant while preserving the intended role-based access.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, thanks. Fixed in b3c77091f: Update-SqlPermission compared the direct server permissions of the destination login with those of the source inside the loop over the source permissions, so for a source login without any, the comparison never ran. It runs once after that loop now, and the login updated in place loses every direct server permission the source login does not have.

The same applies to the other paths through the helper: Sync-DbaLoginPermission, and a login that -Force recreates, now also lose a direct CONNECT SQL that the source login does not have. That is what the helper already did whenever the source login had any other direct permission.

The new context "A direct server permission is revoked from the login updated in place when the source login has none" sets up your case: on the source, the login is in a user-defined server role with CONNECT SQL, and its own CONNECT SQL is revoked; on the destination, the login with the same SID has CONNECT SQL and CONTROL SERVER granted directly, and the role exists without it. It checks that neither permission is granted directly after -UpdateExisting, that the login is in the role, and that it connects to the destination with the source password.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 44 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6, Sync-DbaLoginPermission.Tests.ps1 10 of 10 on both. With the Update-SqlPermission.ps1 of 89979b576, the new test fails with CONTROL SERVER and CONNECT SQL still granted.


This text was created by Claude and reviewed by Andreas Jordan.

@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 head b3c77091f39d968e8e9f70e0ada6738372d79007, its tests, prior fixes, and passing CI. The earlier permission-sync issues are addressed. One material destructive failure remains in the new pre-drop password-hash guard.

[P1] Do not drop a destination login when its source password hash could not be read (public/Copy-DbaLogin.ps1, guard near lines 352–383). On a SQL Server 2025 source and a 2019 destination, for example, use source credentials permitted to enumerate logins but lacking permission to read their password hashes (or cause the hash lookup to fail) and destination credentials authorized to replace an existing SQL login. Get-LoginPasswordHash then returns no usable hash or throws; this guard catches the error and sets $sourceHash = $null. Neither -like check matches, so $hashFormat is null and -Force (including -UpdateExisting -Force for a mismatched SID) proceeds to drop the existing destination login. New-DbaLogin then also cannot retrieve the source password hash and fails, leaving the destination login deleted. This is precisely the loss-of-login path the pre-drop guard is intended to prevent. LOGINPROPERTY(..., 'PasswordHash') requires additional source permissions, so this is a realistic deployment configuration, not merely a fabricated input.

Required correction: fail closed before any drop whenever a SQL login's source password hash is unavailable/unreadable (and, separately, when its format is incompatible), producing a Failed/Skipped row; add a regression test asserting the existing destination login survives an unreadable hash. The full healthy-hash and transaction tests do not exercise this path.

@andreasjordan

Copy link
Copy Markdown
Collaborator Author

Confirmed, thanks. Fixed in 0177c409d: the check before the drop read the password hash only for a source of SQL Server 2012 or later and a destination older than SQL Server 2022, and it took a hash it could not read for a compatible one. It now reads the hash of every SQL login before anything is dropped. A hash that could not be read gives a Failed row with the note "Could not read the password hash" and a warning, with -Force as with -UpdateExisting -Force. The format check runs for the same versions as before. development drops the login the same way with -Force alone, so that path is fixed too.

The new context "An existing login is not dropped when the password hash of the source login cannot be read" connects to the source as a login that has VIEW ANY DEFINITION only: it sees the logins, but LOGINPROPERTY returns no hash to it, which the context checks first. The destination login has a SID of its own. The context runs -Force and then -UpdateExisting -Force, and checks the Failed row, the warning, and that the destination login is still there with its SID.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 47 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6, Start-DbaMigration.Tests.ps1 15 of 15 on Windows PowerShell 5.1. With the Copy-DbaLogin.ps1 of b3c77091f, both new tests fail with the note "Invalid cast from 'System.DBNull' to 'System.Byte[]'", and the destination login is gone.

Also in this push, 2c3bfc3a0: the skip of -UpdateExisting for a destination older than SQL Server 2005 is gone. Since #10813, Connect-DbaInstance refuses such an instance, and the version guide asks for no new SQL Server 2000 or 2005 code paths.


This text was created by Claude and reviewed by Andreas Jordan.

andreasjordan and others added 8 commits November 11, 2026 20:05
Without -Force an existing login was skipped, so a password changed on the
source never reached the destination. With -Force it was dropped and
recreated after its databases and jobs were given to sa.

-UpdateExisting compares the password hash, CHECK_POLICY, CHECK_EXPIRATION,
default database, default language and disabled state, and applies the
differences with ALTER LOGIN in one transaction on the SMO connection.
SQL Server refuses a hashed password while CHECK_POLICY is on (error 33020),
so the policy is turned off and set back around it. SID, ownerships and
sessions stay; permissions are synced as after a copy. A login with a
different SID is skipped with a warning unless -Force is also given, and the
Windows group lockout check of #8572 applies as for a drop.

Also fixes two cases where -Force dropped a login it could not recreate,
now one check per SQL login before anything is dropped:
- 2012+ source to an older destination: the existing check called
  Stop-Function without -Continue, so the copy went on after the warning.
- SQL Server 2025 hashes (0x0300), which SQL Server 2019 refuses.
Windows logins are still copied to older destinations.

Part of #10772

(do Copy-DbaLogin)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…login with the source name

Update-SqlPermission checked whether the destination login was in a server role the source login is
not in, and then dropped the name of the source login from that role. Without a rename both names
are the same. With -LoginRenameHashtable they differ, and -UpdateExisting reaches this with the
memberships the existing login already has: the renamed login stayed in the role, and a different
login that happens to have the name of the source login lost it. The role messages named the
undefined $destRoleName and said "Adding" for the removal.

The new context renames a login that is in no server role to one that is a sysadmin on the
destination, next to another sysadmin with the name of the source login.

Part of #10772

(do Copy-DbaLogin)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ace fails

The destination can be a server object of the caller with a transaction open on its connection.
The update then began a nested transaction, and after a failing statement its rollback, which SMO
sends as an unnamed ROLLBACK TRANSACTION, took back the whole transaction of the caller with the
work done in it. The update 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 fails the last statement of an update inside a transaction of the test and checks
on that connection that the transaction and its temporary table are still there and the login is
as it was.

Part of #10772

(do Copy-DbaLogin)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ion older than SQL Server 2005

The check for database mappings on a destination older than SQL Server 2005 ended with a bare
continue in a function without a loop of its own, so it went on with the next iteration of the loop
of the caller. Sync-DbaLoginPermission, which accepts SQL Server 2000 destinations, calls it in its
loop over the logins: the rest of the iteration was skipped, and every login came without a result
row, neither Successful nor Failed. Copy-DbaLogin calls it in a loop of its own around the call and
was not affected. It is a return now.

No lab or CI instance runs SQL Server 2000, so there is no regression test; the binding was checked
with a probe of the same shape on Windows PowerShell 5.1 and PowerShell 7.6.

(do Copy-DbaLogin, Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does not have

Update-SqlPermission walked the server roles of the source only. A user-defined server role that
exists on the destination only was never visited, so a login updated in place with -UpdateExisting
kept its membership there, and with it a permission like CONTROL SERVER of that role, while a login
that -Force drops and recreates loses it. The helper now also walks the roles of the destination
that the source does not have and takes the login out of them; the role and its other members stay.
Sync-DbaLoginPermission uses the same helper and removes such a membership now as well, as its
description of a sync says.

The new context creates such a role with CONTROL SERVER on the destination, with the login and a
second login as members, and checks that only the updated login left it.

Part of #10772

(do Copy-DbaLogin, Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… login has none

Update-SqlPermission compared the direct server permissions of the destination login with those of
the source inside the loop over the source permissions. For a source login without any direct
server permission, one that gets CONNECT SQL through a server role for example, that loop never
ran, so a login updated in place with -UpdateExisting kept every direct grant it had on the
destination, CONTROL SERVER included, while a login that -Force drops and recreates loses it. The
comparison runs once after that loop now. Sync-DbaLoginPermission uses the same helper and revokes
such a grant now as well, and a recreated login loses the CONNECT SQL of CREATE LOGIN when the
source login has none, as the helper already did for a source login with other permissions.

The new context revokes CONNECT SQL from the source login, which connects through a user-defined
server role, grants CONNECT SQL and CONTROL SERVER directly to the destination login, and checks that
both are gone after the update while the login connects through the role.

Part of #10772

(do Copy-DbaLogin, Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The check before the drop read the password hash of a SQL login only for a source of SQL Server 2012
or later and a destination older than SQL Server 2022, and it treated a hash it could not read as
compatible. LOGINPROPERTY returns no hash to a source connection without CONTROL SERVER, even when
that connection can see the logins, so -Force, and -UpdateExisting -Force for a login with a
different SID, dropped the destination login, and New-DbaLogin then failed with "Invalid cast from
'System.DBNull' to 'System.Byte[]'" and left the login missing. Development does the same with
-Force. The hash is now read for every SQL login before anything is dropped, and a hash that could
not be read gives a Failed row with a warning. The check of the hash format still runs for the same
versions as before. Sync-DbaAvailabilityGroup and Start-DbaMigration call Copy-DbaLogin and get the
same check.

The new context connects to the source as a login with VIEW ANY DEFINITION only and checks that
-Force and -UpdateExisting -Force leave the destination login with its SID.

Part of #10772

(do Copy-DbaLogin, Sync-DbaAvailabilityGroup, Start-DbaMigration)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r than SQL Server 2005

-UpdateExisting skipped a destination older than SQL Server 2005, because ALTER LOGIN came with
2005. Since #10813, Connect-DbaInstance refuses every instance older than SQL Server 2008, so the
check can no longer be reached, and the version guide asks for no new SQL Server 2000 or 2005 code
paths.

Part of #10772

(do Copy-DbaLogin)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@potatoqualitee
potatoqualitee force-pushed the copy-dbalogin-update-existing branch from 2c3bfc3 to 1e9a458 Compare November 11, 2026 19:05

@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 head 1e9a458ee73dc05e332d623bb4f426e229cde808.

Re-reviewed the current head following earlier change requests. The password-hash availability guard now runs before a destructive replacement and fails closed for unreadable source hashes; a regression case exercises that path. The permission sync fixes and in-place update behavior are also covered. Previous blocking finding is addressed; no further material defect identified.

@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.

Fresh full review of head 1e9a458ee73dc05e332d623bb4f426e229cde808.

Requesting changes for two material defects in the newly reachable in-place permission synchronization, detailed inline:

  1. Destination-only permissions on server securables, including IMPERSONATE on a privileged login, survive reconciliation.
  2. GrantWithGrant -> Grant can revoke the desired permission itself, including CONNECT SQL.

The earlier unreadable-password-hash and caller-transaction fixes are present. These remaining cases are not covered by the new integration contexts. I independently validated both findings against the current source, SMO's enumeration/overload contracts and permission semantics, then executed exact-helper recording probes. Those probes establish the calls/arguments; they are not live SQL integration tests. The current import/smoke/security checks pass, while the main ci-azure run remains pending.

Comment thread public/Copy-DbaLogin.ps1
ObjectLevel = $ObjectLevel
ExcludeDatabaseMapping = $ExcludeDatabaseMapping
}
Update-SqlPermission @splatPermission

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.

[P1] Reconcile permissions on server securables before treating an in-place login as synchronized

With the same-SID app_login on both instances, give only the destination GRANT IMPERSONATE ON LOGIN::sa TO app_login, then run Copy-DbaLogin -Source src -Destination dst -Login app_login -UpdateExisting -Force. This branch retains the principal and calls Update-SqlPermission, but that helper only calls Server.EnumServerPermissions for server permission reconciliation. SMO's SrvYukonPerm.xml explicitly restricts those results to prmssn.class = 100; the impersonation grant is class 101 and belongs to the separate object-permission enumeration. No such enumeration/removal occurs here. The login therefore retains the ability to impersonate sa, whereas the previous Force replacement removes the grant when the principal is dropped. -ObjectLevel in the helper only handles database-user objects and does not cover this.

Please reconcile destination-only permissions on server securables as well as class-100 permissions, matching the securable and mapped grantee correctly, and add a real SQL regression asserting that a matching-SID destination login loses its extra impersonation grant while unrelated principals are preserved.

Independently checked against Microsoft's server-permission enumerator and object-permission enumerator. An exact-helper recording probe also confirms zero calls to object-permission enumeration; this probe was not a live SQL integration run.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, thanks. Fixed in 19a92e973: Update-SqlPermission now also compares the permissions of the login on logins and server roles (Server.EnumObjectPermissions, class 101) with those of the source login, by securable and permission, and revokes the ones the source login does not have. The source login is read under its own name and the destination login under its new name, so a rename is covered. As with the server permissions, only the grant option goes when the source login has the permission without it. Nothing is granted, because Copy-DbaLogin gives a created login none of these permissions either, so the update ends where -Force ends.

One deliberate limit: permissions on endpoints and availability groups stay. They belong to the replica, not to the login. In an availability group whose replicas run under different service accounts, a secondary grants CONNECT on its endpoint to the service account of the primary, and that login has no such permission on the primary. A sync of all server securables would revoke it on every secondary with Sync-DbaAvailabilityGroup -UpdateExistingLogin and stop the data movement. The help of -UpdateExisting and of Sync-DbaLoginPermission says so. If you want endpoints and availability groups included anyway, it is a change of the class filter.

The new context "Permissions on logins and server roles that the source login does not have are revoked from the login updated in place" gives the destination login IMPERSONATE on another login, VIEW DEFINITION on that login (the source login has it too), CONTROL on a server role with grant option (without it on the source) and CONNECT on an endpoint. A second login gets IMPERSONATE on the same login. Afterwards IMPERSONATE is gone, VIEW DEFINITION stays, CONTROL is granted without the grant option, the endpoint permission stays, and the second login keeps its IMPERSONATE.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 57 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6. With the Update-SqlPermission.ps1 of 1e9a458ee, IMPERSONATE on the other login is still granted, and CONTROL on the server role keeps its grant option.


This text was created by Claude and reviewed by Andreas Jordan.

$grantWithGrant = $false
}

$DestServer.PSObject.Methods["Revoke"].Invoke($permSet, $newLoginName, $false, $grantWithGrant)

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.

[P1] Preserve the permission when downgrading GrantWithGrant to Grant

A matching-SID login can have ordinary CONNECT SQL on the source and CONNECT SQL WITH GRANT OPTION on the destination. The grant loop first applies ordinary GRANT, which does not remove an existing grant option. This comparison then sees destination GrantWithGrant versus source Grant and reaches this call with revokeGrant = false, cascade = true (verified against the actual SMO overload). That revokes CONNECT SQL itself, and this helper never grants it again. With no role providing alternative CONNECT SQL, the new in-place path leaves the login unable to connect even though the source login still can. The same problem removes other desired permissions, such as VIEW SERVER STATE.

Please explicitly handle the grant-option downgrade while retaining the source permission, or perform reconciliation/revocation before applying the final desired grants. Add a real SQL regression that starts with destination WITH GRANT OPTION, expects final state GRANT and verifies continued login connectivity.

I traced the exact current helper: Grant CONNECT SQL (false) is followed by Revoke CONNECT SQL (revokeGrant=false, cascade=true). This was a recording-boundary probe, not live SQL. Microsoft's REVOKE server permissions documentation distinguishes revoking the permission from GRANT OPTION FOR; the existing tests do not cover this transition.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, thanks. Fixed in 5f8f777ef: the GRANT of the source permission leaves an existing grant option in place, and the sync then found no source permission in the state GrantWithGrant and revoked the permission itself with CASCADE, without granting it again. When the source login has the permission without the grant option, only the grant option is revoked now: REVOKE GRANT OPTION FOR ... CASCADE, through the revokeGrant argument of the same SMO overload.

The same loop had one more case, fixed in f69a33f00: when the source login is denied a permission that the destination login has with grant option, the DENY failed with "To revoke or deny grantable privileges, specify the CASCADE option." and a warning, and the sync then revoked the permission, so the login ended with neither the grant nor the deny. The DENY runs with CASCADE now, which SQL Server also accepts for a login that does not have the permission (checked in the lab). Sync-DbaLoginPermission, the other caller of Update-SqlPermission, had both defects as well.

The new context "A server permission the login updated in place has with grant option is synced to the state of the source" gives the destination login CONNECT SQL, VIEW SERVER STATE and ALTER TRACE with grant option. The source login has the first two without it and is denied the third. The context checks that there is no warning, that the first two end as GRANT and the third as DENY, and that the login can connect to the destination.

Lab, the copy pair SQL Server 2022/2025: Copy-DbaLogin.Tests.ps1 57 passed and 2 skipped on Windows PowerShell 5.1 and on PowerShell 7.6, Sync-DbaLoginPermission.Tests.ps1 10 of 10. With the Update-SqlPermission.ps1 of 1e9a458ee, the new context fails on both: the login ends with no server permission at all, CONNECT SQL, VIEW SERVER STATE and ALTER TRACE are gone, it cannot connect ("Login failed"), and the DENY warns "specify the CASCADE option".


This text was created by Claude and reviewed by Andreas Jordan.

@potatoqualitee

Copy link
Copy Markdown
Member

first review ran as medium effort and second one as Pro effort in chatgpt.

andreasjordan and others added 3 commits November 11, 2026 21:55
… has the server permission without it

A login updated in place can have a server permission WITH GRANT OPTION on the destination and
without it on the source. The GRANT of the source permission leaves the grant option in place, and
the sync of Update-SqlPermission then found no source permission with the same state and revoked
the permission itself with CASCADE. It never granted it again, so the login lost CONNECT SQL, or
VIEW SERVER STATE and the like, which the source login still has. When the source login has the
permission without the grant option, only the grant option is now revoked (REVOKE GRANT OPTION FOR
... CASCADE).

Update-SqlPermission is shared: Copy-DbaLogin calls it for created and updated logins, and
Sync-DbaLoginPermission for every existing login, which had the same defect.

The new context gives the destination login CONNECT SQL and VIEW SERVER STATE with grant option,
checks that both stay granted without it, and that the login can still connect.

Part of #10772

(do Copy-DbaLogin, Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ith grant option

When the source login is denied a server permission that the existing destination login has WITH
GRANT OPTION, the DENY of Update-SqlPermission failed with "To revoke or deny grantable privileges,
specify the CASCADE option." and a warning. The sync then revoked the permission with CASCADE, so the
login ended with neither the grant nor the deny. The DENY now runs with CASCADE, which SQL Server
also accepts for a login without the permission.

Found while fixing the grant option finding of the review. Update-SqlPermission is shared:
Copy-DbaLogin calls it for created and updated logins, and Sync-DbaLoginPermission for every
existing login, which had the same defect.

The grant option context now also denies ALTER TRACE on the source and grants it with grant option
on the destination, and checks that it ends as DENY without a warning.

Part of #10772

(do Copy-DbaLogin, Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t the source login does not have

Update-SqlPermission synced the permissions on the server only: Server.EnumServerPermissions returns
class 100 and nothing else. A login updated in place therefore kept a permission like IMPERSONATE ON
LOGIN::sa or CONTROL ON SERVER ROLE that only the destination login had, while -Force drops the login
and the permission with it. The permissions of the login on logins and server roles
(Server.EnumObjectPermissions, class 101) are now compared with those of the source login by
securable and permission, and those the source login does not have are revoked. As for the server
permissions, only the grant option goes when the source login has the permission without it.

A created login gets none of these permissions from the source, so nothing is granted. Permissions
on endpoints and availability groups stay: they belong to the replica, and a secondary grants
CONNECT on its endpoint to the service account of the primary, which has no such permission on the
primary. The help of Copy-DbaLogin -UpdateExisting and of Sync-DbaLoginPermission says so.

Update-SqlPermission is shared: Copy-DbaLogin calls it for created and updated logins, and
Sync-DbaLoginPermission for every existing login, which now revokes these permissions as well.

The new context gives the destination login IMPERSONATE on another login, CONTROL on a server role
with grant option and CONNECT on an endpoint, and gives a second login IMPERSONATE on the same login.

Part of #10772

(do Copy-DbaLogin, Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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