SqlLogin: Add AllowRecreate parameter to enforce Sid on existing logins

[coderabbitai]

Context

In PR #2453, the Sid parameter was added to the SqlLogin resource to allow setting the security identifier (SID) for newly created SQL logins. However, the SID validation logic for existing logins is being removed from Test-TargetResource to avoid infinite drift loops, since SQL Server does not support changing a login's SID after creation.

This issue tracks the work to re-add SID validation and implement login recreation functionality in a future PR.

References:

• PR: SqlLogin: add sid parameter #2453
• Comment: SqlLogin: add sid parameter #2453 (comment)
• Original issue: SqlLogin: Should support creating viable SqlLogins across a cluster #1470
• Requested by: @johlju

---

Requirements

1. Re-add SID validation to Test-TargetResource

• Check if the Sid parameter is bound
• Compare the current login's SID with the desired SID
• Return $false if they don't match (when AllowRecreate is $true)
• If AllowRecreate is not set and SIDs mismatch, emit a warning but don't fail the test

2. Add login recreation logic to Set-TargetResource

• When Sid is bound and the login exists with a different SID
• Check if AllowRecreate parameter is $true
• If yes, drop the existing login and recreate it with the correct SID
• If no, emit a warning/error explaining manual intervention is required

3. Add AllowRecreate opt-in parameter

• New boolean parameter on all three functions (Get/Set/Test-TargetResource)
• Defaults to $false for safety
• When $true, allows the resource to drop and recreate the login to enforce the SID
• Must be explicitly set by the user to prevent accidental login deletion

---

Implementation Plan

Phase 1: Add AllowRecreate parameter

1. Add AllowRecreate parameter to Set-TargetResource, Test-TargetResource, and Get-TargetResource (for consistency)
2. Add parameter to schema MOF file (DSC_SqlLogin.schema.mof)
3. Add localized strings for recreation messages
4. Update comment-based help with parameter description and warnings

Phase 2: Implement Set-TargetResource logic

1. In the Present branch where login already exists:
   • After checking login existence, add SID validation block
   • If Sid is bound and login SID doesn't match:
     • If AllowRecreate is $true:
       • Log warning about recreation
       • Use $PSCmdlet.ShouldProcess for confirmation
       • Drop the existing login using Remove-SQLServerLogin
       • Continue to creation logic (reuse existing creation code)
     • If AllowRecreate is $false:
       • Throw terminating error with clear message that SID cannot be changed without recreation
       • Instruct user to set AllowRecreate = $true if recreation is acceptable

Phase 3: Update Test-TargetResource logic

1. Re-add SID comparison logic (similar to what was removed in PR SqlLogin: add sid parameter #2453)
2. If Sid is bound and mismatches:
   • If AllowRecreate is $true: return $false (not in desired state)
   • If AllowRecreate is $false: emit warning and return $true (avoid drift loop)

Phase 4: Testing and Documentation

1. Add unit tests for:
   • Login recreation when SID mismatches and AllowRecreate = $true
   • Error thrown when SID mismatches and AllowRecreate = $false
   • ShouldProcess confirmation for recreation
   • Test-TargetResource behavior with/without AllowRecreate
2. Add integration test for login recreation scenario
3. Update resource documentation (README.md)
4. Add example configuration demonstrating AllowRecreate usage
5. Update CHANGELOG.md

---

Security Considerations

• Recreating a login will drop all associated permissions, roles, and ownerships
• The AllowRecreate parameter must default to $false for safety
• Documentation should clearly warn users about the implications of login recreation
• Consider logging which permissions/roles are lost during recreation (if feasible)
• Ensure ShouldProcess is used with appropriate ConfirmImpact level (High)

---

Alternative Approaches Considered

1. No recreation support: Keep current limitation, document that SID can only be set on new logins

   • Simpler but less functional
   • Users would need to manually drop/recreate logins

2. Automatic recreation without opt-in: Always recreate when SID mismatches

   • Too dangerous, could accidentally delete production logins

3. Separate Force parameter: Use existing DSC patterns

   • Less explicit about the recreation action
   • AllowRecreate is more descriptive and intentional

[johlju]

Suggested changes:

In file source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1‎ and function Test-TargetResource:

        if ( $PSBoundParameters.ContainsKey('Sid') )
        {
            $infoSid = if ($null -ne $loginInfo.Sid)
            {
                '0x' + [System.BitConverter]::ToString($loginInfo.Sid).Replace('-', '')
            }

            if ($null -eq $infoSid -or $infoSid -ne $Sid)
            {
                Write-Verbose -Message (
                    $script:localizedData.WrongSid -f $Name, $infoSid, $Sid
                )

                $testPassed = $false
            }
        }

In file [source/DSCResources/DSC_SqlLogin/en-US/DSC_SqlLogin.strings.psd1‎:

ConvertFrom-StringData @'
    # existing code
    WrongSid = The login '{0}' has the Sid '{1}', but expected it to have the Sid '{2}'.
'@

[johlju]

@coderabbitai plan

We should also plan to add examples and update documentation to explain the opt-in parameter

[coderabbitai]

Coding Plan

Summary

• Follow the established DSC_SqlDatabaseUser Force parameter pattern for opt-in destructive operations, using terminating errors rather than ShouldProcess confirmation
• Leverage existing Remove-SqlDscLogin public function for login removal rather than implementing SMO calls directly
• Implement defensive SID mismatch handling in Test-TargetResource that avoids drift loops when AllowRecreate is disabled
• Maintain consistency with existing codebase patterns for localization, parameter placement, and error handling

Design Choices

Design Choice 1: PR `#2453` Merge Status

Options Considered:

1. Assume PR #2453 is already merged and Sid parameter exists
2. Include Sid parameter implementation as part of this work

Chosen Option: 1

Rationale: This ticket explicitly builds on PR #2453 and references the Sid parameter as existing; implementation should assume Sid is already present.

Design Choice 2: ShouldProcess vs Terminating Error Pattern

Options Considered:

1. Follow ticket suggestion and implement ShouldProcess with High ConfirmImpact
2. Follow existing DSC_SqlDatabaseUser pattern using terminating errors when opt-in is required

Chosen Option: 2

Rationale: Follow the established DSC_SqlDatabaseUser Force parameter pattern—throw a terminating error when recreation is needed but AllowRecreate is $false. DSC resources are declarative and don't use interactive confirmation; this aligns with existing codebase patterns.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Schema and Parameter Definition

Define the AllowRecreate parameter in the MOF schema and add it to all three target resource functions with appropriate comment-based help and localized strings.

Task 1: Add AllowRecreate to Schema MOF

Add the new boolean parameter definition to the DSC resource schema for DSC configuration validation.

• Update source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.schema.mof to add AllowRecreate as a Write property
• Define as Boolean type with default value behavior (defaults to $false in implementation)
• Place parameter after existing boolean parameters (Disabled) for logical grouping

Task 2: Add Localized Strings for Recreation Functionality

Create all localized message strings needed for recreation warnings, errors, and verbose output.

• Update source/DSCResources/DSC_SqlLogin/en-US/DSC_SqlLogin.strings.psd1 with new string entries
• Add error message for when recreation is required but AllowRecreate is $false (follow pattern from DSC_SqlDatabaseUser's ForceNotEnabled)
• Add verbose message for when recreation is being performed
• Add warning message about SID mismatch detected (for Test-TargetResource when AllowRecreate is $false)
• Add verbose message for SID comparison during test

Task 3: Add AllowRecreate Parameter to Resource Functions

Add the parameter definition to all three target resource functions with consistent signature and documentation.

• Update source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1 to add AllowRecreate parameter to Get-TargetResource, Set-TargetResource, and Test-TargetResource
• Define as [System.Boolean] type with no default value (will be treated as $false when not specified)
• Add comment-based help explaining the parameter's purpose and warning about destructive behavior
• Include in parameter block after the Disabled parameter for consistency

🤖 Prompt for AI agents

Define the AllowRecreate boolean parameter across the SqlLogin DSC resource
schema and implementation to enable opt-in SID-based login recreation.

**Schema Definition:**
- Add `AllowRecreate` as a Write Boolean property in
`source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.schema.mof`
- Position after existing boolean parameters like Disabled
- Defaults to $false in implementation when not specified

**Localized Strings:**
Update `source/DSCResources/DSC_SqlLogin/en-US/DSC_SqlLogin.strings.psd1` with:
- Error message for when recreation is required but AllowRecreate is $false
(follow DSC_SqlDatabaseUser's ForceNotEnabled pattern)
- Verbose message for recreation operation
- Warning message for SID mismatch when AllowRecreate is $false
- Verbose message for SID comparison during test

**Function Parameters:**
In `source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1`:
- Add AllowRecreate parameter to Get-TargetResource, Set-TargetResource, and
Test-TargetResource
- Type: `[System.Boolean]` with no default value
- Position: After Disabled parameter
- Include comment-based help warning about destructive drop-and-recreate
behavior

Phase 2: Core Recreation Logic Implementation

Implement the SID validation and recreation behavior in Test-TargetResource and Set-TargetResource functions.

Task 1: Implement SID Validation in Test-TargetResource

Add logic to compare current login SID with desired SID and return appropriate compliance status based on AllowRecreate setting.

• In source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1 Test-TargetResource function, add SID comparison block after existing property validations
• Check if Sid parameter is bound using $PSBoundParameters.ContainsKey('Sid')
• Retrieve current login SID from the SMO Login object's Sid property
• Convert binary SID to string for comparison (use .NET SecurityIdentifier class)
• If SIDs match, continue with existing test logic
• If SIDs don't match AND AllowRecreate is $true, set test result to $false (not in desired state)
• If SIDs don't match AND AllowRecreate is $false, emit warning about SID mismatch but return $true to avoid drift loop

Task 2: Implement Recreation Logic in Set-TargetResource

Add the drop-and-recreate workflow when SID mismatch is detected and AllowRecreate is enabled.

• In source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1 Set-TargetResource function, add SID validation block in the existing login update section (where Ensure is Present and login exists)
• Check if Sid parameter is bound and compare with current login SID
• If SIDs match, continue with existing update logic
• If SIDs don't match AND AllowRecreate is $false, throw terminating error using New-InvalidOperationException with localized message instructing user to set AllowRecreate = $true
• If SIDs don't match AND AllowRecreate is $true:
  • Write verbose message about recreation
  • Call Remove-SqlDscLogin to drop the existing login (use existing public function)
  • Allow execution to continue to the existing login creation code path (no need to duplicate creation logic)

Task 3: Add AllowRecreate to Get-TargetResource Output

Include AllowRecreate in the Get-TargetResource return hashtable for configuration reporting consistency.

• Update the return hashtable in Get-TargetResource to include AllowRecreate parameter value
• This maintains the DSC pattern where Get returns all configurable properties

🤖 Prompt for AI agents

Implement SID validation and drop-and-recreate logic in the SqlLogin resource to
enable SID enforcement when AllowRecreate is enabled.

**Test-TargetResource SID Validation:**
In `source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1` Test-TargetResource
function:
- Add SID comparison block after existing property validations
- Check if Sid parameter is bound using `$PSBoundParameters.ContainsKey('Sid')`
- Retrieve current login SID from SMO Login object's Sid property
- Convert binary SID to string using .NET SecurityIdentifier class
- **SID Match:** Continue with existing test logic
- **SID Mismatch + AllowRecreate = $true:** Return $false (not in desired state)
- **SID Mismatch + AllowRecreate = $false:** Emit warning (using localized
string) and return $true to prevent drift loop

**Set-TargetResource Recreation Logic:**
In Set-TargetResource function, within existing login update section (Ensure =
Present, login exists):
- Check if Sid parameter is bound and compare with current login SID
- **SID Match:** Continue with existing update logic
- **SID Mismatch + AllowRecreate = $false:** Throw terminating error using
`New-InvalidOperationException` with localized message directing user to set
AllowRecreate = $true
- **SID Mismatch + AllowRecreate = $true:**
  - Write verbose message about recreation operation
  - Call `Remove-SqlDscLogin` to drop the existing login
- Allow execution flow to continue to existing login creation code (reuse
creation logic)

**Get-TargetResource Output:**
- Add AllowRecreate to the return hashtable to maintain DSC pattern consistency

Phase 3: Testing and Documentation

Add comprehensive unit tests for the new functionality and update all relevant documentation.

Task 1: Add Unit Tests for AllowRecreate Functionality

Create test cases covering all recreation scenarios and edge cases.

• Update tests/Unit/DSC_SqlLogin.Tests.ps1 with new test contexts for AllowRecreate behavior
• Test Set-TargetResource: recreation succeeds when SID mismatches and AllowRecreate is $true
• Test Set-TargetResource: throws terminating error when SID mismatches and AllowRecreate is $false
• Test Set-TargetResource: no action when SIDs match (regardless of AllowRecreate value)
• Test Test-TargetResource: returns $false when SID mismatches and AllowRecreate is $true
• Test Test-TargetResource: returns $true with warning when SID mismatches and AllowRecreate is $false
• Test Test-TargetResource: returns $true when SIDs match
• Mock Remove-SqlDscLogin to verify it's called during recreation
• Mock SMO Login.Sid property in test fixtures

Task 2: Update Resource Documentation

Document the new parameter and its implications for users.

• Update source/DSCResources/DSC_SqlLogin/README.md (if exists) or main resource documentation
• Add AllowRecreate parameter description with clear warning about destructive behavior
• Document that recreation drops all permissions, roles, and ownerships associated with the login
• Provide example configuration showing AllowRecreate usage with Sid parameter

Task 3: Add Configuration Example

Create an example DSC configuration demonstrating AllowRecreate usage.

• Add example file in source/Examples/Resources/SqlLogin/ directory following existing example patterns
• Show configuration with Sid and AllowRecreate = $true
• Include comments explaining when and why to use AllowRecreate

Task 4: Update CHANGELOG

Document the changes for release notes.

• Update CHANGELOG.md in the Unreleased section
• Add entry under SqlLogin resource describing AllowRecreate parameter addition
• Reference the original issue (#1470) and PR #2453 for context

🤖 Prompt for AI agents

Add comprehensive unit tests and documentation for the AllowRecreate
functionality in the SqlLogin resource.

**Unit Tests:**
Update `tests/Unit/DSC_SqlLogin.Tests.ps1` with new test contexts:
- Mock SMO Login.Sid property in test fixtures
- Mock `Remove-SqlDscLogin` to verify recreation calls

**Set-TargetResource Tests:**
- Recreation succeeds when SID mismatches and AllowRecreate = $true (verify
Remove-SqlDscLogin called)
- Throws terminating error when SID mismatches and AllowRecreate = $false
- No recreation when SIDs match (regardless of AllowRecreate value)

**Test-TargetResource Tests:**
- Returns $false when SID mismatches and AllowRecreate = $true
- Returns $true with warning when SID mismatches and AllowRecreate = $false
- Returns $true when SIDs match

**Documentation:**
- Update `source/DSCResources/DSC_SqlLogin/README.md` (or main resource docs):
- Add AllowRecreate parameter description with clear warning about destructive
drop-and-recreate behavior
- Document that recreation drops all permissions, roles, and ownerships
associated with the login
  - Provide example configuration showing AllowRecreate with Sid parameter

**Configuration Example:**
- Add example file in `source/Examples/Resources/SqlLogin/` following existing
patterns
- Demonstrate Sid and AllowRecreate = $true usage
- Include comments explaining when and why to use AllowRecreate

**CHANGELOG:**
- Update `CHANGELOG.md` Unreleased section
- Add SqlLogin resource entry for AllowRecreate parameter
- Reference issue `#1470` and PR `#2453`

Research

SqlServerDsc is a PowerShell DSC module for SQL Server configuration. The SqlLogin resource is a script-based DSC resource located at source/DSCResources/DSC_SqlLogin/ with traditional Get/Set/Test-TargetResource functions. The codebase follows established patterns for destructive operations using opt-in parameters (like Force in DSC_SqlDatabaseUser), localized strings in en-US/ subdirectories, and public helper functions in source/Public/ for login management (Remove-SqlDscLogin, New-SqlDscLogin, etc.). DSC resources in this codebase do not use ShouldProcess for confirmation—instead they throw terminating errors when dangerous operations require explicit opt-in.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Define the AllowRecreate boolean parameter across the SqlLogin DSC resource
schema and implementation to enable opt-in SID-based login recreation.

**Schema Definition:**
- Add `AllowRecreate` as a Write Boolean property in
`source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.schema.mof`
- Position after existing boolean parameters like Disabled
- Defaults to $false in implementation when not specified

**Localized Strings:**
Update `source/DSCResources/DSC_SqlLogin/en-US/DSC_SqlLogin.strings.psd1` with:
- Error message for when recreation is required but AllowRecreate is $false
(follow DSC_SqlDatabaseUser's ForceNotEnabled pattern)
- Verbose message for recreation operation
- Warning message for SID mismatch when AllowRecreate is $false
- Verbose message for SID comparison during test

**Function Parameters:**
In `source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1`:
- Add AllowRecreate parameter to Get-TargetResource, Set-TargetResource, and
Test-TargetResource
- Type: `[System.Boolean]` with no default value
- Position: After Disabled parameter
- Include comment-based help warning about destructive drop-and-recreate
behavior
===============================================================================

Task: 2

Implement SID validation and drop-and-recreate logic in the SqlLogin resource to
enable SID enforcement when AllowRecreate is enabled.

**Test-TargetResource SID Validation:**
In `source/DSCResources/DSC_SqlLogin/DSC_SqlLogin.psm1` Test-TargetResource
function:
- Add SID comparison block after existing property validations
- Check if Sid parameter is bound using `$PSBoundParameters.ContainsKey('Sid')`
- Retrieve current login SID from SMO Login object's Sid property
- Convert binary SID to string using .NET SecurityIdentifier class
- **SID Match:** Continue with existing test logic
- **SID Mismatch + AllowRecreate = $true:** Return $false (not in desired state)
- **SID Mismatch + AllowRecreate = $false:** Emit warning (using localized
string) and return $true to prevent drift loop

**Set-TargetResource Recreation Logic:**
In Set-TargetResource function, within existing login update section (Ensure =
Present, login exists):
- Check if Sid parameter is bound and compare with current login SID
- **SID Match:** Continue with existing update logic
- **SID Mismatch + AllowRecreate = $false:** Throw terminating error using
`New-InvalidOperationException` with localized message directing user to set
AllowRecreate = $true
- **SID Mismatch + AllowRecreate = $true:**
  - Write verbose message about recreation operation
  - Call `Remove-SqlDscLogin` to drop the existing login
- Allow execution flow to continue to existing login creation code (reuse
creation logic)

**Get-TargetResource Output:**
- Add AllowRecreate to the return hashtable to maintain DSC pattern consistency
===============================================================================

Task: 3

Add comprehensive unit tests and documentation for the AllowRecreate
functionality in the SqlLogin resource.

**Unit Tests:**
Update `tests/Unit/DSC_SqlLogin.Tests.ps1` with new test contexts:
- Mock SMO Login.Sid property in test fixtures
- Mock `Remove-SqlDscLogin` to verify recreation calls

**Set-TargetResource Tests:**
- Recreation succeeds when SID mismatches and AllowRecreate = $true (verify
Remove-SqlDscLogin called)
- Throws terminating error when SID mismatches and AllowRecreate = $false
- No recreation when SIDs match (regardless of AllowRecreate value)

**Test-TargetResource Tests:**
- Returns $false when SID mismatches and AllowRecreate = $true
- Returns $true with warning when SID mismatches and AllowRecreate = $false
- Returns $true when SIDs match

**Documentation:**
- Update `source/DSCResources/DSC_SqlLogin/README.md` (or main resource docs):
- Add AllowRecreate parameter description with clear warning about destructive
drop-and-recreate behavior
- Document that recreation drops all permissions, roles, and ownerships
associated with the login
  - Provide example configuration showing AllowRecreate with Sid parameter

**Configuration Example:**
- Add example file in `source/Examples/Resources/SqlLogin/` following existing
patterns
- Demonstrate Sid and AllowRecreate = $true usage
- Include comments explaining when and why to use AllowRecreate

**CHANGELOG:**
- Update `CHANGELOG.md` Unreleased section
- Add SqlLogin resource entry for AllowRecreate parameter
- Reference issue `#1470` and PR `#2453`

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!