Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync-DbaLoginPassword - command to sync passwords based on hashed value #8985

Closed
wants to merge 22 commits into from

Conversation

wsmelton
Copy link
Member

@wsmelton wsmelton commented Jun 11, 2023

Includes bumping the version of PSSA, so some commands not being caught before with formatting issues are included in this PR.

  • Please confirm you have the smaller repo (85MB .git directory vs 275MB or 110MB or 185MB .git directory)

Type of Change

Purpose

Adding functionality to support syncing passwords between instances using a hashed password value.

Approach

Internal function added Get-LoginPasswordHash that uses T-SQL from Microsoft in the sp_help_revlogin to grab the hashed password value.

A number of updates to Set-DbaLogin to improve performance but also a new parameter, -PasswordHash, that allows it to handle setting the password by hash. It belongs in that command, and makes it where it can be used outside of the sync process if user desires.

This provides support for adding password sync to Sync-DbaAvailabiltyGroup now if anyone wishes.

Commands to test

Test included

Screenshots

image

image

Learning

https://learn.microsoft.com/en-us/troubleshoot/sql/database-engine/security/transfer-logins-passwords-between-instances

@wsmelton wsmelton changed the title Set-DbaLoginPassword - command to sync passwords based on hashed value Sync-DbaLoginPassword - command to sync passwords based on hashed value Jun 11, 2023
@andreasjordan
Copy link
Contributor

In case you have not seen it: AppVeyor says: Get-LoginPasswordHash.ps1 is not compliant with the OTBS formatting style

@wsmelton
Copy link
Member Author

The Set command runs fine on local instances for piping support. I'm giving up fixing tests at this point because it would be to many changes on this PR>

@potatoqualitee
Copy link
Member

if its 100% failure every time, that usually does point to something. could some sort of use of "localhost" be an issue?

@wsmelton
Copy link
Member Author

Why would splatting affect piping?

@wsmelton
Copy link
Member Author

Yeah, what I figured. I removed my changes to the command and it still fails just like the snapshot command (that isn't touched in this PR).

@potatoqualitee
Copy link
Member

k, rerunning just the fialed commands! I got a new code signing cert and will be margining singing this today.

@potatoqualitee
Copy link
Member

aw nooo, there are conflicts 😭 i will hae to return to this one later but i do very much want the changes in there

@potatoqualitee
Copy link
Member

lol this is getting worse, ill fix it when i have a chunk of time <3

@andreasjordan
Copy link
Contributor

There are so many changes in this PR that are not replated to the titel "Sync-DbaLoginPassword - command to sync passwords based on hashed value". @wsmelton : Maybe you can split this PR and move all the formatting stoff in a new PR?

@wsmelton
Copy link
Member Author

The fault is our testing so all the changes in this PR are related to the PR context where I attempt to have the tests pass for the branch.

@potatoqualitee
Copy link
Member

hey @wsmelton -- i got most things fixed except this one

Describe : Set-DbaDbOwner Integration Tests
Context  : Sets multiple database owners
Name     : It Sets the database owner on multiple databases
Result   : Failed
Message  : Expected 1, but got $null.

Any idea whats up?

@wsmelton
Copy link
Member Author

Yeah that's the one that won't run correctly in Appeyor. That test runs fine locally.

@wsmelton
Copy link
Member Author

wsmelton commented Oct 1, 2023

I'm trying to figure out where I can get a lab spun up and will start working on this one again...

@wsmelton wsmelton closed this Oct 8, 2023
@potatoqualitee potatoqualitee deleted the shawn/syncpwd branch October 23, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants