From 3fd41b9dbd86d8f38e730164f383d0661bcf1fd4 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 5 Aug 2025 05:01:23 +0000 Subject: [PATCH 01/24] added ability for user_role to take a list for roles --- galaxy.yml | 2 +- plugins/modules/user_role.ps1 | 144 +++++++++++++++++++++------------- plugins/modules/user_role.py | 31 +++++++- sqlserver | 1 + 4 files changed, 122 insertions(+), 56 deletions(-) create mode 120000 sqlserver diff --git a/galaxy.yml b/galaxy.yml index a7991f2e..59af6548 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -2,7 +2,7 @@ namespace: lowlydba name: sqlserver -version: 2.6.1 +version: 2.6.2 readme: README.md authors: - John McCall (github.com/lowlydba) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 8a9f7fe4..e7be111e 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -15,8 +15,9 @@ $spec = @{ options = @{ database = @{type = 'str'; required = $true } username = @{type = 'str'; required = $true } - role = @{type = 'str'; required = $true } + roles = @{type = 'list'; elements = 'str'; required = $true; aliases = 'role' } state = @{type = 'str'; required = $false; default = 'present'; choices = @('present', 'absent') } + remove_unlisted = @{type = 'bool'; required = $false; default = 'false' } } } @@ -24,8 +25,9 @@ $module = [Ansible.Basic.AnsibleModule]::Create($args, $spec, @(Get-LowlyDbaSqlS $sqlInstance, $sqlCredential = Get-SqlCredential -Module $module $username = $module.Params.username $database = $module.Params.database -$role = $module.Params.role +$roles = $module.Params.roles $state = $module.Params.state +$remove_unlisted = $module.Params.remove_unlisted $checkMode = $module.CheckMode $module.Result.changed = $false @@ -37,78 +39,114 @@ $getUserSplat = @{ User = $username EnableException = $true } -$getRoleSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - Database = $database - Role = $role - EnableException = $true + + +# Verify user and role(s) exist, DBATools currently fails silently +$existingUser = Get-DbaDbUser @getUserSplat +if ($null -eq $existingUser) { + $module.FailJson("User [$username] does not exist in database [$database].") } + +$roles | ForEach-Object { + $thisRole = $_ + $getRoleSplat = @{ + SqlInstance = $sqlInstance + SqlCredential = $sqlCredential + Database = $database + Role = $thisrole + EnableException = $true + } + $existingRole = Get-DbaDbRole @getRoleSplat + if ($null -eq $existingRole) { + $module.FailJson("Role [$thisRole] does not exist in database [$database].") + } +} + +# Get role members of all roles we care about to compare against later $getRoleMemberSplat = @{ SqlInstance = $sqlInstance SqlCredential = $sqlCredential Database = $database - Role = $role + # Role = $role IncludeSystemUser = $true EnableException = $true } - -# Verify user and role exist, DBATools currently fails silently -$existingUser = Get-DbaDbUser @getUserSplat -if ($null -eq $existingUser) { - $module.FailJson("User [$username] does not exist in database [$database].") -} -$existingRole = Get-DbaDbRole @getRoleSplat -if ($null -eq $existingRole) { - $module.FailJson("Role [$role] does not exist in database [$database].") -} - -# Get role members -$existingRoleMembers = Get-DbaDbRoleMember @getRoleMemberSplat +$existingRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role if ($state -eq "absent") { - if ($existingRoleMembers.username -contains $username) { - try { - $removeRoleMemberSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - User = $username - Database = $database - Role = $role - EnableException = $true - WhatIf = $checkMode - Confirm = $false + $roles | ForEach-Object { + $thisRole = $_ + if ( $existingRoleMembership -contains $thisRole ) { + try { + $removeRoleMemberSplat = @{ + SqlInstance = $sqlInstance + SqlCredential = $sqlCredential + User = $username + Database = $database + Role = $thisRole + EnableException = $true + WhatIf = $checkMode + Confirm = $false + } + $output = Remove-DbaDbRoleMember @removeRoleMemberSplat + $module.Result.changed = $true + } + catch { + $module.FailJson("Removing user [$username] from database role [$thisRole] failed: $($_.Exception.Message)", $_) } - $output = Remove-DbaDbRoleMember @removeRoleMemberSplat - $module.Result.changed = $true - } - catch { - $module.FailJson("Removing user [$username] from database role [$role] failed: $($_.Exception.Message)", $_) } } } elseif ($state -eq "present") { # Add user to role - if ($existingRoleMembers.username -notcontains $username) { - try { - $addRoleMemberSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - User = $username - Database = $database - Role = $role - EnableException = $true - WhatIf = $checkMode - Confirm = $false + $roles | ForEach-Object { + $thisRole = $_ + if ($existingRoleMembership -notcontains $thisRole) { + try { + $addRoleMemberSplat = @{ + SqlInstance = $sqlInstance + SqlCredential = $sqlCredential + User = $username + Database = $database + Role = $thisRole + EnableException = $true + WhatIf = $checkMode + Confirm = $false + } + $output = Add-DbaDbRoleMember @addRoleMemberSplat + $module.Result.changed = $true + } + catch { + $module.FailJson("Adding user [$username] to database role [$thisRole] failed: $($_.Exception.Message)", $_) } - $output = Add-DbaDbRoleMember @addRoleMemberSplat - $module.Result.changed = $true } - catch { - $module.FailJson("Adding user [$username] to database role [$role] failed: $($_.Exception.Message)", $_) + } +} +if ($state -eq "present" -and $remove_unlisted -eq $true) { + $existingRoleMembership | ForEach-Object { + $thisRole = $_ + if ($roles -notcontains $thisRole) { + try { + $removeRoleMemberSplat = @{ + SqlInstance = $sqlInstance + SqlCredential = $sqlCredential + User = $username + Database = $database + Role = $thisRole + EnableException = $true + WhatIf = $checkMode + Confirm = $false + } + $output = Remove-DbaDbRoleMember @removeRoleMemberSplat + $module.Result.changed = $true + } + catch { + $module.FailJson("Removing user [$username] from extra unlisted database role [$thisRole] failed: $($_.Exception.Message)", $_) + } } } } + try { if ($null -ne $output) { $resultData = ConvertTo-SerializableObject -InputObject $output diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index f120ba74..a3cc5872 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -22,11 +22,17 @@ - Name of the user. type: str required: true - role: + roles: description: - The database role for the user to be modified. - type: str + type: list required: true + remove_unlisted: + description: + - When set to true, will remove any other roles that aren't listed in roles. + type: boolean + required: false + default: false author: "John McCall (@lowlydba)" requirements: - L(dbatools,https://www.powershellgallery.com/packages/dbatools/) PowerShell module @@ -45,6 +51,15 @@ database: InternProject1 role: db_owner +- name: Add a user to a list of db roles + lowlydba.sqlserver.user_role: + sql_instance: sql-01.myco.io + username: TheIntern + database: InternProject1 + role: + - db_datareader + - db_datawriter + - name: Remove a user from a fixed db role lowlydba.sqlserver.login: sql_instance: sql-01.myco.io @@ -60,6 +75,18 @@ database: InternProject1 role: db_intern state: absent + +- name: Specify a list of roles that user should be in and remove all others + lowlydba.sqlserver.login: + sql_instance: sql-01.myco.io + username: TheIntern + database: InternProject1 + role: + - db_datareader + - db_datawriter + state: present + remove_unlisted: true + ''' RETURN = r''' diff --git a/sqlserver b/sqlserver new file mode 120000 index 00000000..9b6e9ec2 --- /dev/null +++ b/sqlserver @@ -0,0 +1 @@ +/home/cameron/ansible-collections-dev/lowlydba/sqlserver \ No newline at end of file From 8386b2a53bf81dc19ef2f098a10f1d2d3653f909 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 5 Aug 2025 05:17:03 +0000 Subject: [PATCH 02/24] updated changelog --- changelogs/changelog.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml index 93dd4dbe..247978c0 100644 --- a/changelogs/changelog.yaml +++ b/changelogs/changelog.yaml @@ -573,3 +573,7 @@ releases: fragments: - 314-ansible-2-19-compatibility.yml release_date: '2025-05-03' + 2.6.2: + changes: + minor_changes: + - Added ability for user_role module to take a list of roles. Optionally can also remove unlisted roles. From 40e634ebe7d7a5ce2cd181c5816ed844284a137d Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 5 Aug 2025 05:19:34 +0000 Subject: [PATCH 03/24] updated documentation for new option --- plugins/modules/user_role.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index a3cc5872..6fb01311 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -33,6 +33,7 @@ type: boolean required: false default: false + version_added: 2.6.2 author: "John McCall (@lowlydba)" requirements: - L(dbatools,https://www.powershellgallery.com/packages/dbatools/) PowerShell module From 4c41f7b27799ddb22aa306c0ce2f669d21c8549b Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Wed, 6 Aug 2025 01:36:28 +0000 Subject: [PATCH 04/24] change output of module so that unit testing can test on more things (breaking change?) --- plugins/modules/user_role.ps1 | 22 +++++++++--- .../targets/user_role/tasks/main.yml | 36 +++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index e7be111e..f7979d07 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -40,6 +40,7 @@ $getUserSplat = @{ EnableException = $true } +$outputProps = @{} # Verify user and role(s) exist, DBATools currently fails silently $existingUser = Get-DbaDbUser @getUserSplat @@ -67,11 +68,10 @@ $getRoleMemberSplat = @{ SqlInstance = $sqlInstance SqlCredential = $sqlCredential Database = $database - # Role = $role IncludeSystemUser = $true EnableException = $true } -$existingRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role +$existingRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role | Sort-Object if ($state -eq "absent") { $roles | ForEach-Object { @@ -88,7 +88,7 @@ if ($state -eq "absent") { WhatIf = $checkMode Confirm = $false } - $output = Remove-DbaDbRoleMember @removeRoleMemberSplat + Remove-DbaDbRoleMember @removeRoleMemberSplat $module.Result.changed = $true } catch { @@ -113,7 +113,7 @@ elseif ($state -eq "present") { WhatIf = $checkMode Confirm = $false } - $output = Add-DbaDbRoleMember @addRoleMemberSplat + Add-DbaDbRoleMember @addRoleMemberSplat $module.Result.changed = $true } catch { @@ -123,6 +123,7 @@ elseif ($state -eq "present") { } } if ($state -eq "present" -and $remove_unlisted -eq $true) { + #remove users from roles that weren't listed (if we got the remove_unlisted option set to true) $existingRoleMembership | ForEach-Object { $thisRole = $_ if ($roles -notcontains $thisRole) { @@ -137,7 +138,7 @@ if ($state -eq "present" -and $remove_unlisted -eq $true) { WhatIf = $checkMode Confirm = $false } - $output = Remove-DbaDbRoleMember @removeRoleMemberSplat + Remove-DbaDbRoleMember @removeRoleMemberSplat $module.Result.changed = $true } catch { @@ -147,6 +148,17 @@ if ($state -eq "present" -and $remove_unlisted -eq $true) { } } +try { + #after changing any roles above, see what our new membership is and report it back + $newRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role | Sort-Object +} +catch { + $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) +} +$outputProps.newRoleMembership = [array]$newRoleMembership +$outputProps.oldRoleMembership = [array]$existingRoleMembership +$output = New-Object -TypeName PSCustomObject -Property $outputProps + try { if ($null -ne $output) { $resultData = ConvertTo-SerializableObject -InputObject $output diff --git a/tests/integration/targets/user_role/tasks/main.yml b/tests/integration/targets/user_role/tasks/main.yml index ec5936b7..364628bb 100644 --- a/tests/integration/targets/user_role/tasks/main.yml +++ b/tests/integration/targets/user_role/tasks/main.yml @@ -72,6 +72,7 @@ - assert: that: - result is changed + - result.newRoleMembership == ["db_owner"] - name: Add user to non-existent database role lowlydba.sqlserver.user_role: @@ -102,13 +103,48 @@ that: - result is not changed + - name: Add user to list of database roles + lowlydba.sqlserver.user_role: + role: + - db_datareader + - db_datawriter + register: result + - assert: + that: + - result is changed + - result.newRoleMembership == ["db_datareader", "db_datawriter", "db_owner"] + + - name: Add user to list of database roles again + lowlydba.sqlserver.user_role: + role: + - db_datareader + - db_datawriter + register: result + - assert: + that: + - result is not changed + + - name: Add user to list of database roles, and remove unlisted roles + lowlydba.sqlserver.user_role: + role: + - db_datareader + - db_datawriter + remove_unlisted: true + register: result + - assert: + that: + - result is changed + - result.newRoleMembership == ["db_datareader", "db_datawriter"] + - name: Remove user from database role lowlydba.sqlserver.user_role: state: "absent" + role: db_datawriter register: result - assert: that: - result is changed + - result.newRoleMembership == ["db_datareader"] always: - name: Drop user From c8f0821bc2ec8798becdd40ddced5ba3413de704 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Wed, 6 Aug 2025 02:06:13 +0000 Subject: [PATCH 05/24] typo in unit tests --- tests/integration/targets/user_role/tasks/main.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/targets/user_role/tasks/main.yml b/tests/integration/targets/user_role/tasks/main.yml index 364628bb..7401fc13 100644 --- a/tests/integration/targets/user_role/tasks/main.yml +++ b/tests/integration/targets/user_role/tasks/main.yml @@ -72,7 +72,7 @@ - assert: that: - result is changed - - result.newRoleMembership == ["db_owner"] + - result.data.newRoleMembership == ["db_owner"] - name: Add user to non-existent database role lowlydba.sqlserver.user_role: @@ -112,7 +112,7 @@ - assert: that: - result is changed - - result.newRoleMembership == ["db_datareader", "db_datawriter", "db_owner"] + - result.data.newRoleMembership == ["db_datareader", "db_datawriter", "db_owner"] - name: Add user to list of database roles again lowlydba.sqlserver.user_role: @@ -134,7 +134,7 @@ - assert: that: - result is changed - - result.newRoleMembership == ["db_datareader", "db_datawriter"] + - result.data.newRoleMembership == ["db_datareader", "db_datawriter"] - name: Remove user from database role lowlydba.sqlserver.user_role: @@ -144,7 +144,7 @@ - assert: that: - result is changed - - result.newRoleMembership == ["db_datareader"] + - result.data.newRoleMembership == ["db_datareader"] always: - name: Drop user From 62db4fa3fb694a6ba8d0f3a3ac13bdc322744968 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 03:11:24 +0000 Subject: [PATCH 06/24] refactor to implement add/remove/set functionality --- plugins/modules/user_role.ps1 | 215 ++++++++++-------- plugins/modules/user_role.py | 75 ++++-- .../targets/user_role/tasks/main.yml | 118 ++++++++-- 3 files changed, 270 insertions(+), 138 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index f7979d07..1fdcdca6 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -15,149 +15,166 @@ $spec = @{ options = @{ database = @{type = 'str'; required = $true } username = @{type = 'str'; required = $true } - roles = @{type = 'list'; elements = 'str'; required = $true; aliases = 'role' } state = @{type = 'str'; required = $false; default = 'present'; choices = @('present', 'absent') } - remove_unlisted = @{type = 'bool'; required = $false; default = 'false' } + role = @{type = 'str'; required = $false} + roles = @{ + default = @{} + type = 'dict' + options = @{ + add = @{ + default = @() + type = 'list' + elements = 'str' + } + remove = @{ + default = @() + type = 'list' + elements = 'str' + } + set = @{ + default = $null + type = 'list' + elements = 'str' + } + } + } } + required_one_of = @( + , @("role", "roles") + ) + mutually_exclusive = @( + , @("role", "roles") + ) } $module = [Ansible.Basic.AnsibleModule]::Create($args, $spec, @(Get-LowlyDbaSqlServerAuthSpec)) $sqlInstance, $sqlCredential = Get-SqlCredential -Module $module $username = $module.Params.username $database = $module.Params.database +$role = $module.Params.role $roles = $module.Params.roles $state = $module.Params.state $remove_unlisted = $module.Params.remove_unlisted $checkMode = $module.CheckMode +$compatibilityMode = $false $module.Result.changed = $false -$getUserSplat = @{ +# Map the "old" style way of using state and role on to the add/remove/set methods +if ($role -and $state -eq 'present') { + $roles.add = @(, $role) + $compatibilityMode = $true +} +if ($role -and $state -eq 'absent') { + $roles.remove = @(, $role) + $compatibilityMode = $true +} + +$commonParamSplat = @{ SqlInstance = $sqlInstance SqlCredential = $sqlCredential Database = $database - User = $username EnableException = $true } $outputProps = @{} # Verify user and role(s) exist, DBATools currently fails silently -$existingUser = Get-DbaDbUser @getUserSplat +$existingUser = Get-DbaDbUser @commonParamSplat -user $username if ($null -eq $existingUser) { $module.FailJson("User [$username] does not exist in database [$database].") } -$roles | ForEach-Object { +$combinedRoles = ( $roles['set'] + $roles['add'] + $roles['remove'] ) | Select-Object -Unique +$combinedRoles | ForEach-Object { $thisRole = $_ - $getRoleSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - Database = $database - Role = $thisrole - EnableException = $true - } - $existingRole = Get-DbaDbRole @getRoleSplat + $existingRole = Get-DbaDbRole @commonParamSplat -role $thisRole if ($null -eq $existingRole) { $module.FailJson("Role [$thisRole] does not exist in database [$database].") } } -# Get role members of all roles we care about to compare against later -$getRoleMemberSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - Database = $database - IncludeSystemUser = $true - EnableException = $true +# Sanity check on the add/remove clause not having the same role. +$sameRoles = Compare-Object $roles['add'] $roles['remove'] -IncludeEqual | Where-Object {$_.SideIndicator -eq '=='} | Select-Object -ExpandProperty InputObject +if ( $sameRoles.count -ge 1 ) { + $module.FailJson("Role [$($sameRoles -join ', ')] exists in both the add and remove lists.") } -$existingRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role | Sort-Object - -if ($state -eq "absent") { - $roles | ForEach-Object { - $thisRole = $_ - if ( $existingRoleMembership -contains $thisRole ) { - try { - $removeRoleMemberSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - User = $username - Database = $database - Role = $thisRole - EnableException = $true - WhatIf = $checkMode - Confirm = $false - } - Remove-DbaDbRoleMember @removeRoleMemberSplat - $module.Result.changed = $true - } - catch { - $module.FailJson("Removing user [$username] from database role [$thisRole] failed: $($_.Exception.Message)", $_) - } - } - } + +# Get current role membership of all roles for the user to compare against +$existingRoleMembership = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true ` + | Where-Object {$_.UserName -eq $username} ` + | Select -ExpandProperty role ` + | Sort-Object +if ($null -eq $existingRoleMembership) {$existingRoleMembership = @()} + +if ( $null -ne $roles['set'] ) { + $comparison = Compare-Object $existingRoleMembership $roles['set'] + $rolesToAdd = $comparison | Where-Object {$_.SideIndicator -eq '=>'} | Select-Object -ExpandProperty InputObject + $rolesToRemove = $comparison | Where-Object {$_.SideIndicator -eq '<='} | Select-Object -ExpandProperty InputObject +} else { + $rolesToAdd = Compare-Object $existingRoleMembership $roles['add'] | Where-Object {$_.SideIndicator -eq '=>'} | Select-Object -ExpandProperty InputObject + $rolesToRemove = Compare-Object $existingRoleMembership $roles['remove'] -IncludeEqual | Where-Object {$_.SideIndicator -eq '=='} | Select-Object -ExpandProperty InputObject } -elseif ($state -eq "present") { - # Add user to role - $roles | ForEach-Object { - $thisRole = $_ - if ($existingRoleMembership -notcontains $thisRole) { - try { - $addRoleMemberSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - User = $username - Database = $database - Role = $thisRole - EnableException = $true - WhatIf = $checkMode - Confirm = $false - } - Add-DbaDbRoleMember @addRoleMemberSplat - $module.Result.changed = $true - } - catch { - $module.FailJson("Adding user [$username] to database role [$thisRole] failed: $($_.Exception.Message)", $_) - } + +# Add user to new roles +$rolesToAdd | ForEach-Object { + $thisRole = $_ + try { + $addRoleMemberSplat = @{ + User = $username + Role = $thisRole + WhatIf = $checkMode + Confirm = $false } + $commandResult = Add-DbaDbRoleMember @commonParamSplat @addRoleMemberSplat + $module.Result.changed = $true + } + catch { + $module.FailJson("Adding user [$username] to database role [$thisRole] failed: $($_.Exception.Message)", $_) } } -if ($state -eq "present" -and $remove_unlisted -eq $true) { - #remove users from roles that weren't listed (if we got the remove_unlisted option set to true) - $existingRoleMembership | ForEach-Object { - $thisRole = $_ - if ($roles -notcontains $thisRole) { - try { - $removeRoleMemberSplat = @{ - SqlInstance = $sqlInstance - SqlCredential = $sqlCredential - User = $username - Database = $database - Role = $thisRole - EnableException = $true - WhatIf = $checkMode - Confirm = $false - } - Remove-DbaDbRoleMember @removeRoleMemberSplat - $module.Result.changed = $true - } - catch { - $module.FailJson("Removing user [$username] from extra unlisted database role [$thisRole] failed: $($_.Exception.Message)", $_) - } + +# remove user from unneeded roles +$rolesToRemove | ForEach-Object { + $thisRole = $_ + try { + $removeRoleMemberSplat = @{ + User = $username + Role = $thisRole + WhatIf = $checkMode + Confirm = $false } + $commandResult = Remove-DbaDbRoleMember @commonParamSplat @removeRoleMemberSplat + $module.Result.changed = $true + } + catch { + $module.FailJson("Removing user [$username] from database role [$thisRole] failed: $($_.Exception.Message)", $_) } } -try { - #after changing any roles above, see what our new membership is and report it back - $newRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role | Sort-Object -} -catch { - $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) +# if we're still using old mode (using $state and $role) save command result as results, +# otherwise send back full list of old and new roles. +if ($compatibilityMode) { + $output = $commandResult +} else { + try { + # after changing any roles above, see what our new membership is and report it back + $newRoleMembership = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true ` + | Where-Object {$_.UserName -eq $username} ` + | Select-Object -ExpandProperty role ` + | Sort-Object + } + catch { + $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) + } + $outputProps.roleMembership = [array]$newRoleMembership + if ( $module.Result.changed ) { + $outputProps.diff = @{} + $outputProps.diff.after = [array]$newRoleMembership + $outputProps.diff.before = [array]$existingRoleMembership + } + $output = New-Object -TypeName PSCustomObject -Property $outputProps } -$outputProps.newRoleMembership = [array]$newRoleMembership -$outputProps.oldRoleMembership = [array]$existingRoleMembership -$output = New-Object -TypeName PSCustomObject -Property $outputProps try { if ($null -ne $output) { diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index 6fb01311..0c45bc95 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -22,17 +22,38 @@ - Name of the user. type: str required: true - roles: - description: + role: + description - The database role for the user to be modified. - type: list - required: true - remove_unlisted: + - When used with State set to present, will add the user to this role. + - When used with State set to absent, will remove the user from this role. + - Mutually exclusive with roles + type: str + roles: description: - - When set to true, will remove any other roles that aren't listed in roles. - type: boolean - required: false - default: false + - The database roles for the user to be added, removed or set. + - Mutually exclusive with role + type: dict + suboptions: + add: + description: + - Adds the user to the specified roles, keeping the + existing role membership if they are not specified. + type: list + elements: str + remove: + description: + - Removes the user from the specified roles, keeping the + existing role membership if they are not specified. + type: list + elements: str + set: + description: + - Adds the user to the specified roles. + - User will be removed from any other roles not specified. + - Set this to an empty list to remove all members from a group.. + type: list + elements: str version_added: 2.6.2 author: "John McCall (@lowlydba)" requirements: @@ -57,12 +78,13 @@ sql_instance: sql-01.myco.io username: TheIntern database: InternProject1 - role: - - db_datareader - - db_datawriter + roles: + add: + - db_datareader + - db_datawriter - name: Remove a user from a fixed db role - lowlydba.sqlserver.login: + lowlydba.sqlserver.user_role: sql_instance: sql-01.myco.io username: TheIntern database: InternProject1 @@ -70,29 +92,40 @@ state: absent - name: Add a user to a custom db role - lowlydba.sqlserver.login: + lowlydba.sqlserver.user_role: sql_instance: sql-01.myco.io username: TheIntern database: InternProject1 role: db_intern - state: absent + state: present - name: Specify a list of roles that user should be in and remove all others - lowlydba.sqlserver.login: + lowlydba.sqlserver.user_role: sql_instance: sql-01.myco.io username: TheIntern database: InternProject1 - role: - - db_datareader - - db_datawriter + roles: + set: + - db_datareader + - db_datawriter state: present - remove_unlisted: true +- name: Remove user from all roles on this database + lowlydba.sqlserver.user_role: + sql_instance: sql-01.myco.io + username: TheIntern + database: InternProject1 + roles: + set: [] + state: present ''' RETURN = r''' data: - description: Output from the C(Remove-DbaDbRoleMember), (Get-DbaDbRoleMember), or C(Add-DbaDbRoleMember) functions. + description: + - If called with role, then data is output from the C(Remove-DbaDbRoleMember), (Get-DbaDbRoleMember), or C(Add-DbaDbRoleMember) functions. + - If called with roles, then data returned roleMembership, which is an array of roles that the user is now a member of. + - If called without either role or roles, then data returned is roleMembership which is users current list of roles. returned: success, but not in check_mode. type: dict ''' diff --git a/tests/integration/targets/user_role/tasks/main.yml b/tests/integration/targets/user_role/tasks/main.yml index 7401fc13..17db9569 100644 --- a/tests/integration/targets/user_role/tasks/main.yml +++ b/tests/integration/targets/user_role/tasks/main.yml @@ -41,7 +41,6 @@ sql_password: "{{ sqlserver_password }}" database: "{{ database }}" username: "{{ username }}" - role: "{{ role }}" state: present tags: ["sqlserver.user"] block: @@ -68,6 +67,9 @@ - name: Add user to database role lowlydba.sqlserver.user_role: + roles: + add: + - db_owner register: result - assert: that: @@ -76,7 +78,9 @@ - name: Add user to non-existent database role lowlydba.sqlserver.user_role: - role: db_IMadeThisOneUp + roles: + add: + - db_IMadeThisOneUp register: error_result failed_when: error_result.failed ignore_errors: true @@ -88,6 +92,9 @@ - name: Add non-existent user to database role lowlydba.sqlserver.user_role: username: NewUserWhoThis + roles: + add: + - db_owner register: error_result failed_when: error_result.failed ignore_errors: true @@ -98,6 +105,9 @@ - name: Add user again to database role lowlydba.sqlserver.user_role: + roles: + add: + - db_owner register: result - assert: that: @@ -105,46 +115,118 @@ - name: Add user to list of database roles lowlydba.sqlserver.user_role: - role: - - db_datareader - - db_datawriter + roles: + add: + - db_datareader + - db_datawriter register: result - assert: that: - result is changed - - result.data.newRoleMembership == ["db_datareader", "db_datawriter", "db_owner"] + - result.data.roleMembership == ["db_datareader", "db_datawriter", "db_owner"] - name: Add user to list of database roles again lowlydba.sqlserver.user_role: - role: - - db_datareader - - db_datawriter + roles: + add: + - db_datareader + - db_datawriter register: result - assert: that: - result is not changed - - name: Add user to list of database roles, and remove unlisted roles + - name: Add user to list of database roles using set, to remove unlisted roles lowlydba.sqlserver.user_role: - role: - - db_datareader - - db_datawriter - remove_unlisted: true + roles: + set: + - db_datareader + - db_owner register: result - assert: that: - result is changed - - result.data.newRoleMembership == ["db_datareader", "db_datawriter"] + - result.data.roleMembership == ["db_datareader", "db_owner"] - - name: Remove user from database role + - name: Add user to list of database roles using set again + lowlydba.sqlserver.user_role: + roles: + set: + - db_datareader + - db_owner + register: result + - assert: + that: + - result is not changed + + - name: Remove user from all roles using set with blank array + lowlydba.sqlserver.user_role: + roles: + set: [] + register: result + - assert: + that: + - result is changed + - result.data.roleMembership == null + + - name: Remove user from all roles using set with blank array again + lowlydba.sqlserver.user_role: + roles: + set: [] + register: result + - assert: + that: + - result is not changed + + - name: Add user to database role (old method) + lowlydba.sqlserver.user_role: + role: "db_datareader" + register: result + - assert: + that: + - result is changed + + - name: Add user to database role (old method) again + lowlydba.sqlserver.user_role: + role: "db_datareader" + register: result + - assert: + that: + - result is not changed + + - name: Get user rolelist to check if old method task worked (old method doesn't return membershipList) + lowlydba.sqlserver.user_role: + register: result + - assert: + that: + - result is not changed + - result.data.roleMembership == ["db_datareader"] + + - name: Remove user from database role (old method) lowlydba.sqlserver.user_role: state: "absent" - role: db_datawriter + role: "db_datareader" register: result - assert: that: - result is changed - - result.data.newRoleMembership == ["db_datareader"] + + - name: Remove user from database role (old method) again + lowlydba.sqlserver.user_role: + state: "absent" + role: "db_datareader" + register: result + - assert: + that: + - result is not changed + + - name: Get user rolelist to check if old method task worked (old method doesn't return membershipList) + lowlydba.sqlserver.user_role: + register: result + - assert: + that: + - result is not changed + - result.data.roleMembership == null always: - name: Drop user From 97b68c743736953e22baf819c163d699cc5b8b70 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 03:24:45 +0000 Subject: [PATCH 07/24] removing random symlink --- sqlserver | 1 - 1 file changed, 1 deletion(-) delete mode 120000 sqlserver diff --git a/sqlserver b/sqlserver deleted file mode 120000 index 9b6e9ec2..00000000 --- a/sqlserver +++ /dev/null @@ -1 +0,0 @@ -/home/cameron/ansible-collections-dev/lowlydba/sqlserver \ No newline at end of file From d66a924ee33da382e51030ade4ca6ec8af691552 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 03:53:00 +0000 Subject: [PATCH 08/24] remove unused param, plus linting --- plugins/modules/user_role.ps1 | 37 +++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 1fdcdca6..82592ece 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -13,10 +13,10 @@ $ErrorActionPreference = "Stop" $spec = @{ supports_check_mode = $true options = @{ - database = @{type = 'str'; required = $true } - username = @{type = 'str'; required = $true } - state = @{type = 'str'; required = $false; default = 'present'; choices = @('present', 'absent') } - role = @{type = 'str'; required = $false} + database = @{ type = 'str'; required = $true } + username = @{ type = 'str'; required = $true } + state = @{ type = 'str'; required = $false; default = 'present'; choices = @('present', 'absent') } + role = @{ type = 'str'; required = $false } roles = @{ default = @{} type = 'dict' @@ -54,7 +54,6 @@ $database = $module.Params.database $role = $module.Params.role $roles = $module.Params.roles $state = $module.Params.state -$remove_unlisted = $module.Params.remove_unlisted $checkMode = $module.CheckMode $compatibilityMode = $false @@ -95,25 +94,29 @@ $combinedRoles | ForEach-Object { } # Sanity check on the add/remove clause not having the same role. -$sameRoles = Compare-Object $roles['add'] $roles['remove'] -IncludeEqual | Where-Object {$_.SideIndicator -eq '=='} | Select-Object -ExpandProperty InputObject +$sameRoles = Compare-Object $roles['add'] $roles['remove'] -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } | Select-Object -ExpandProperty InputObject if ( $sameRoles.count -ge 1 ) { $module.FailJson("Role [$($sameRoles -join ', ')] exists in both the add and remove lists.") } # Get current role membership of all roles for the user to compare against $existingRoleMembership = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true ` - | Where-Object {$_.UserName -eq $username} ` - | Select -ExpandProperty role ` - | Sort-Object -if ($null -eq $existingRoleMembership) {$existingRoleMembership = @()} + | Where-Object { $_.UserName -eq $username } ` + | Select-Object -ExpandProperty role ` + | Sort-Object +if ($null -eq $existingRoleMembership) { $existingRoleMembership = @() } if ( $null -ne $roles['set'] ) { $comparison = Compare-Object $existingRoleMembership $roles['set'] - $rolesToAdd = $comparison | Where-Object {$_.SideIndicator -eq '=>'} | Select-Object -ExpandProperty InputObject - $rolesToRemove = $comparison | Where-Object {$_.SideIndicator -eq '<='} | Select-Object -ExpandProperty InputObject + $rolesToAdd = $comparison | Where-Object { $_.SideIndicator -eq '=>' } | Select-Object -ExpandProperty InputObject + $rolesToRemove = $comparison | Where-Object { $_.SideIndicator -eq '<=' } | Select-Object -ExpandProperty InputObject } else { - $rolesToAdd = Compare-Object $existingRoleMembership $roles['add'] | Where-Object {$_.SideIndicator -eq '=>'} | Select-Object -ExpandProperty InputObject - $rolesToRemove = Compare-Object $existingRoleMembership $roles['remove'] -IncludeEqual | Where-Object {$_.SideIndicator -eq '=='} | Select-Object -ExpandProperty InputObject + $rolesToAdd = Compare-Object $existingRoleMembership $roles['add'] ` + | Where-Object { $_.SideIndicator -eq '=>' } ` + | Select-Object -ExpandProperty InputObject + $rolesToRemove = Compare-Object $existingRoleMembership $roles['remove'] -IncludeEqual ` + | Where-Object { $_.SideIndicator -eq '==' } ` + | Select-Object -ExpandProperty InputObject } # Add user to new roles @@ -160,9 +163,9 @@ if ($compatibilityMode) { try { # after changing any roles above, see what our new membership is and report it back $newRoleMembership = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true ` - | Where-Object {$_.UserName -eq $username} ` - | Select-Object -ExpandProperty role ` - | Sort-Object + | Where-Object { $_.UserName -eq $username } ` + | Select-Object -ExpandProperty role ` + | Sort-Object } catch { $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) From 5c2ab581ca28c84894d10e37159d0115d89f25c2 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 03:53:10 +0000 Subject: [PATCH 09/24] fix typo in docs --- plugins/modules/user_role.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index 0c45bc95..4ee2f4ca 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -23,7 +23,7 @@ type: str required: true role: - description + description: - The database role for the user to be modified. - When used with State set to present, will add the user to this role. - When used with State set to absent, will remove the user from this role. From 90783b14309e44e48d1dbf49303eb4bdd25f1233 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 03:53:24 +0000 Subject: [PATCH 10/24] fix typo in tests --- tests/integration/targets/user_role/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/targets/user_role/tasks/main.yml b/tests/integration/targets/user_role/tasks/main.yml index 17db9569..85eb3f55 100644 --- a/tests/integration/targets/user_role/tasks/main.yml +++ b/tests/integration/targets/user_role/tasks/main.yml @@ -74,7 +74,7 @@ - assert: that: - result is changed - - result.data.newRoleMembership == ["db_owner"] + - result.data.roleMembership == ["db_owner"] - name: Add user to non-existent database role lowlydba.sqlserver.user_role: From ceb87e6d1d7af53656e3d85365b4cb14627eff9e Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 05:02:07 +0000 Subject: [PATCH 11/24] updating for more linting / style --- plugins/modules/user_role.ps1 | 39 +++++++++++++++-------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 82592ece..dd13b997 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -94,29 +94,25 @@ $combinedRoles | ForEach-Object { } # Sanity check on the add/remove clause not having the same role. -$sameRoles = Compare-Object $roles['add'] $roles['remove'] -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } | Select-Object -ExpandProperty InputObject -if ( $sameRoles.count -ge 1 ) { +$sameRoles = ( Compare-Object $roles['add'] $roles['remove'] -IncludeEqual Where-Object { $_.SideIndicator -eq '==' } ).InputObject +if ($sameRoles.count -ge 1) { $module.FailJson("Role [$($sameRoles -join ', ')] exists in both the add and remove lists.") } # Get current role membership of all roles for the user to compare against -$existingRoleMembership = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true ` - | Where-Object { $_.UserName -eq $username } ` - | Select-Object -ExpandProperty role ` - | Sort-Object +$membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } +$existingRoleMembership = $membershipObjects.role | Sort-Object + if ($null -eq $existingRoleMembership) { $existingRoleMembership = @() } -if ( $null -ne $roles['set'] ) { +if ($null -ne $roles['set']) { $comparison = Compare-Object $existingRoleMembership $roles['set'] - $rolesToAdd = $comparison | Where-Object { $_.SideIndicator -eq '=>' } | Select-Object -ExpandProperty InputObject - $rolesToRemove = $comparison | Where-Object { $_.SideIndicator -eq '<=' } | Select-Object -ExpandProperty InputObject -} else { - $rolesToAdd = Compare-Object $existingRoleMembership $roles['add'] ` - | Where-Object { $_.SideIndicator -eq '=>' } ` - | Select-Object -ExpandProperty InputObject - $rolesToRemove = Compare-Object $existingRoleMembership $roles['remove'] -IncludeEqual ` - | Where-Object { $_.SideIndicator -eq '==' } ` - | Select-Object -ExpandProperty InputObject + $rolesToAdd = ( $comparison | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject + $rolesToRemove = ( $comparison | Where-Object { $_.SideIndicator -eq '<=' } ).InputObject +} +else { + $rolesToAdd = ( Compare-Object $existingRoleMembership $roles['add'] | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject + $rolesToRemove = ( Compare-Object $existingRoleMembership $roles['remove'] -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } ).InputObject } # Add user to new roles @@ -159,19 +155,18 @@ $rolesToRemove | ForEach-Object { # otherwise send back full list of old and new roles. if ($compatibilityMode) { $output = $commandResult -} else { +} +else { try { # after changing any roles above, see what our new membership is and report it back - $newRoleMembership = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true ` - | Where-Object { $_.UserName -eq $username } ` - | Select-Object -ExpandProperty role ` - | Sort-Object + $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } + $roleMembership = $membershipObjects.role | Sort-Object } catch { $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) } $outputProps.roleMembership = [array]$newRoleMembership - if ( $module.Result.changed ) { + if ($module.Result.changed) { $outputProps.diff = @{} $outputProps.diff.after = [array]$newRoleMembership $outputProps.diff.before = [array]$existingRoleMembership From fca53e77389c4275c893c7b82cb2b3b0b941a1f7 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 05:07:29 +0000 Subject: [PATCH 12/24] roleMembership typo --- plugins/modules/user_role.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index dd13b997..52ea799e 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -160,7 +160,7 @@ else { try { # after changing any roles above, see what our new membership is and report it back $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } - $roleMembership = $membershipObjects.role | Sort-Object + $newRoleMembership = $membershipObjects.role | Sort-Object } catch { $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) From dd8946dce94df26c5c59b775b657c37a8db9cedf Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 05:49:14 +0000 Subject: [PATCH 13/24] typos --- plugins/modules/user_role.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 52ea799e..3c9b270c 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -94,7 +94,7 @@ $combinedRoles | ForEach-Object { } # Sanity check on the add/remove clause not having the same role. -$sameRoles = ( Compare-Object $roles['add'] $roles['remove'] -IncludeEqual Where-Object { $_.SideIndicator -eq '==' } ).InputObject +$sameRoles = ( Compare-Object $roles['add'] $roles['remove'] -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } ).InputObject if ($sameRoles.count -ge 1) { $module.FailJson("Role [$($sameRoles -join ', ')] exists in both the add and remove lists.") } From 832774c34f38142f9294b49c1f7ed8983cdf598b Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 05:49:24 +0000 Subject: [PATCH 14/24] defaults in doc to match code --- plugins/modules/user_role.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index 4ee2f4ca..ab0266a3 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -34,6 +34,7 @@ - The database roles for the user to be added, removed or set. - Mutually exclusive with role type: dict + default: {} suboptions: add: description: @@ -41,12 +42,14 @@ existing role membership if they are not specified. type: list elements: str + default: [] remove: description: - Removes the user from the specified roles, keeping the existing role membership if they are not specified. type: list elements: str + default: [] set: description: - Adds the user to the specified roles. @@ -54,7 +57,7 @@ - Set this to an empty list to remove all members from a group.. type: list elements: str - version_added: 2.6.2 + version_added: 2.7 author: "John McCall (@lowlydba)" requirements: - L(dbatools,https://www.powershellgallery.com/packages/dbatools/) PowerShell module From d85b0a49752500b4d64c8ef3b0c3423b7064dfae Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 22:48:49 +0000 Subject: [PATCH 15/24] change to other type of foreach --- plugins/modules/user_role.ps1 | 6 ++---- plugins/modules/user_role.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 3c9b270c..b55012f2 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -116,8 +116,7 @@ else { } # Add user to new roles -$rolesToAdd | ForEach-Object { - $thisRole = $_ +foreach ($thisRole in $rolesToAdd) { try { $addRoleMemberSplat = @{ User = $username @@ -134,8 +133,7 @@ $rolesToAdd | ForEach-Object { } # remove user from unneeded roles -$rolesToRemove | ForEach-Object { - $thisRole = $_ +foreach ($thisRole in $rolesToRemove) { try { $removeRoleMemberSplat = @{ User = $username diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index ab0266a3..b9f8e363 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -57,7 +57,7 @@ - Set this to an empty list to remove all members from a group.. type: list elements: str - version_added: 2.7 + version_added: 2.7.0 author: "John McCall (@lowlydba)" requirements: - L(dbatools,https://www.powershellgallery.com/packages/dbatools/) PowerShell module From 16ae9fa36bfd198c97fb3842e754cacf39660f94 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 26 Aug 2025 23:29:43 +0000 Subject: [PATCH 16/24] alter results so it returns empty arrays instead of null --- plugins/modules/user_role.ps1 | 10 +++++----- tests/integration/targets/user_role/tasks/main.yml | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index b55012f2..a2816c8d 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -101,7 +101,7 @@ if ($sameRoles.count -ge 1) { # Get current role membership of all roles for the user to compare against $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } -$existingRoleMembership = $membershipObjects.role | Sort-Object +$existingRoleMembership = @(, $membershipObjects.role | Sort-Object) if ($null -eq $existingRoleMembership) { $existingRoleMembership = @() } @@ -158,16 +158,16 @@ else { try { # after changing any roles above, see what our new membership is and report it back $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } - $newRoleMembership = $membershipObjects.role | Sort-Object + $newRoleMembership = @(, $membershipObjects.role | Sort-Object) } catch { $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) } - $outputProps.roleMembership = [array]$newRoleMembership + $outputProps.roleMembership = $newRoleMembership if ($module.Result.changed) { $outputProps.diff = @{} - $outputProps.diff.after = [array]$newRoleMembership - $outputProps.diff.before = [array]$existingRoleMembership + $outputProps.diff.after = $newRoleMembership + $outputProps.diff.before = $existingRoleMembership } $output = New-Object -TypeName PSCustomObject -Property $outputProps } diff --git a/tests/integration/targets/user_role/tasks/main.yml b/tests/integration/targets/user_role/tasks/main.yml index 85eb3f55..a944dcb3 100644 --- a/tests/integration/targets/user_role/tasks/main.yml +++ b/tests/integration/targets/user_role/tasks/main.yml @@ -167,7 +167,7 @@ - assert: that: - result is changed - - result.data.roleMembership == null + - result.data.roleMembership == [] - name: Remove user from all roles using set with blank array again lowlydba.sqlserver.user_role: @@ -226,7 +226,7 @@ - assert: that: - result is not changed - - result.data.roleMembership == null + - result.data.roleMembership == [] always: - name: Drop user From e33a41257bfd40a27b1adf1a7678e26690b049c6 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 9 Sep 2025 03:36:04 +0000 Subject: [PATCH 17/24] use proper array casts --- plugins/modules/user_role.ps1 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index a2816c8d..01887b9d 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -101,18 +101,18 @@ if ($sameRoles.count -ge 1) { # Get current role membership of all roles for the user to compare against $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } -$existingRoleMembership = @(, $membershipObjects.role | Sort-Object) +$existingRoleMembership = [array]($membershipObjects.role | Sort-Object) if ($null -eq $existingRoleMembership) { $existingRoleMembership = @() } if ($null -ne $roles['set']) { - $comparison = Compare-Object $existingRoleMembership $roles['set'] + $comparison = Compare-Object $existingRoleMembership ([array]$roles['set']) $rolesToAdd = ( $comparison | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject $rolesToRemove = ( $comparison | Where-Object { $_.SideIndicator -eq '<=' } ).InputObject } else { - $rolesToAdd = ( Compare-Object $existingRoleMembership $roles['add'] | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject - $rolesToRemove = ( Compare-Object $existingRoleMembership $roles['remove'] -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } ).InputObject + $rolesToAdd = ( Compare-Object $existingRoleMembership ([array]$roles['add']) | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject + $rolesToRemove = ( Compare-Object $existingRoleMembership ([array]$roles['remove']) -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } ).InputObject } # Add user to new roles @@ -158,7 +158,7 @@ else { try { # after changing any roles above, see what our new membership is and report it back $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } - $newRoleMembership = @(, $membershipObjects.role | Sort-Object) + $newRoleMembership = [array]($membershipObjects.role | Sort-Object) } catch { $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) From 0fcedae209abfa20fcd1078aeeb535aa060218c9 Mon Sep 17 00:00:00 2001 From: Cameron Hart Date: Tue, 9 Sep 2025 04:07:31 +0000 Subject: [PATCH 18/24] catch null and convert to empty array on output rolemembership --- plugins/modules/user_role.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 01887b9d..e67e1cd0 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -159,6 +159,7 @@ else { # after changing any roles above, see what our new membership is and report it back $membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username } $newRoleMembership = [array]($membershipObjects.role | Sort-Object) + if ($null -eq $newRoleMembership) { $newRoleMembership = @() } } catch { $module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_) From 0fc0192951aa72b5652a297f62c0d9454c35c9cf Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 8 Jan 2026 14:05:39 -0500 Subject: [PATCH 19/24] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- changelogs/changelog.yaml | 4 ++++ plugins/modules/user_role.py | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml index 2dec16a6..74dfd580 100644 --- a/changelogs/changelog.yaml +++ b/changelogs/changelog.yaml @@ -587,3 +587,7 @@ releases: changes: minor_changes: - Added ability for user_role module to take a list of roles. Optionally can also remove unlisted roles. + release_summary: Added support for managing multiple roles per user with the + user_role module. + fragments: + - 333-user-role-list.yml diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index b9f8e363..aed592f0 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -54,7 +54,7 @@ description: - Adds the user to the specified roles. - User will be removed from any other roles not specified. - - Set this to an empty list to remove all members from a group.. + - Set this to an empty list to remove all members from a group. type: list elements: str version_added: 2.7.0 @@ -127,8 +127,8 @@ data: description: - If called with role, then data is output from the C(Remove-DbaDbRoleMember), (Get-DbaDbRoleMember), or C(Add-DbaDbRoleMember) functions. - - If called with roles, then data returned roleMembership, which is an array of roles that the user is now a member of. - - If called without either role or roles, then data returned is roleMembership which is users current list of roles. + - If called with roles, then data returned is roleMembership, which is an array of roles that the user is now a member of. + - If called without either role or roles, then the data returned is roleMembership which is the user's current list of roles. returned: success, but not in check_mode. type: dict ''' From d9f34858e6f8f2208620b62aa694d0afdad5a4a0 Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 8 Jan 2026 14:07:25 -0500 Subject: [PATCH 20/24] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- plugins/modules/user_role.ps1 | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index e67e1cd0..34512cbb 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -39,9 +39,6 @@ $spec = @{ } } } - required_one_of = @( - , @("role", "roles") - ) mutually_exclusive = @( , @("role", "roles") ) From 8eee9602497b6330703e3555ff28e5b80c463871 Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 8 Jan 2026 14:08:38 -0500 Subject: [PATCH 21/24] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- plugins/modules/user_role.ps1 | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 34512cbb..e10eff54 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -81,7 +81,22 @@ if ($null -eq $existingUser) { $module.FailJson("User [$username] does not exist in database [$database].") } -$combinedRoles = ( $roles['set'] + $roles['add'] + $roles['remove'] ) | Select-Object -Unique +# Ensure that when using the 'roles' parameter directly, at least one operation is specified. +if (-not $compatibilityMode) { + $hasSet = ($null -ne $roles['set'] -and @($roles['set']).Count -gt 0) + $hasAdd = ($null -ne $roles['add'] -and @($roles['add']).Count -gt 0) + $hasRemove = ($null -ne $roles['remove'] -and @($roles['remove']).Count -gt 0) + + if (-not ($hasSet -or $hasAdd -or $hasRemove)) { + $module.FailJson("When using the 'roles' parameter, you must specify at least one of: roles.set, roles.add, or roles.remove.") + } +} + +$rolesSet = if ($null -ne $roles['set']) { @($roles['set']) } else { @() } +$rolesAdd = if ($null -ne $roles['add']) { @($roles['add']) } else { @() } +$rolesRemove = if ($null -ne $roles['remove']) { @($roles['remove']) } else { @() } + +$combinedRoles = ($rolesSet + $rolesAdd + $rolesRemove) | Select-Object -Unique $combinedRoles | ForEach-Object { $thisRole = $_ $existingRole = Get-DbaDbRole @commonParamSplat -role $thisRole From 720455d1eb32e1ef7a3d166b161b77d57e16140a Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 8 Jan 2026 14:09:48 -0500 Subject: [PATCH 22/24] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- plugins/modules/user_role.ps1 | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index e10eff54..2bfceec1 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -119,12 +119,31 @@ if ($null -eq $existingRoleMembership) { $existingRoleMembership = @() } if ($null -ne $roles['set']) { $comparison = Compare-Object $existingRoleMembership ([array]$roles['set']) - $rolesToAdd = ( $comparison | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject - $rolesToRemove = ( $comparison | Where-Object { $_.SideIndicator -eq '<=' } ).InputObject + if ($null -eq $comparison) { + $rolesToAdd = @() + $rolesToRemove = @() + } + else { + $rolesToAdd = ( $comparison | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject + $rolesToRemove = ( $comparison | Where-Object { $_.SideIndicator -eq '<=' } ).InputObject + } } else { - $rolesToAdd = ( Compare-Object $existingRoleMembership ([array]$roles['add']) | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject - $rolesToRemove = ( Compare-Object $existingRoleMembership ([array]$roles['remove']) -IncludeEqual | Where-Object { $_.SideIndicator -eq '==' } ).InputObject + $comparisonAdd = Compare-Object $existingRoleMembership ([array]$roles['add']) + if ($null -eq $comparisonAdd) { + $rolesToAdd = @() + } + else { + $rolesToAdd = ( $comparisonAdd | Where-Object { $_.SideIndicator -eq '=>' } ).InputObject + } + + $comparisonRemove = Compare-Object $existingRoleMembership ([array]$roles['remove']) -IncludeEqual + if ($null -eq $comparisonRemove) { + $rolesToRemove = @() + } + else { + $rolesToRemove = ( $comparisonRemove | Where-Object { $_.SideIndicator -eq '==' } ).InputObject + } } # Add user to new roles From 29bccba7d585b747a5fa08bc7b48100f1f59dcf4 Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 8 Jan 2026 14:18:35 -0500 Subject: [PATCH 23/24] Apply suggestions from code review --- plugins/modules/user_role.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 2bfceec1..95adedda 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -83,8 +83,8 @@ if ($null -eq $existingUser) { # Ensure that when using the 'roles' parameter directly, at least one operation is specified. if (-not $compatibilityMode) { - $hasSet = ($null -ne $roles['set'] -and @($roles['set']).Count -gt 0) - $hasAdd = ($null -ne $roles['add'] -and @($roles['add']).Count -gt 0) + $hasSet = ($null -ne $roles['set'] -and @($roles['set']).Count -gt 0) + $hasAdd = ($null -ne $roles['add'] -and @($roles['add']).Count -gt 0) $hasRemove = ($null -ne $roles['remove'] -and @($roles['remove']).Count -gt 0) if (-not ($hasSet -or $hasAdd -or $hasRemove)) { @@ -92,8 +92,8 @@ if (-not $compatibilityMode) { } } -$rolesSet = if ($null -ne $roles['set']) { @($roles['set']) } else { @() } -$rolesAdd = if ($null -ne $roles['add']) { @($roles['add']) } else { @() } +$rolesSet = if ($null -ne $roles['set']) { @($roles['set']) } else { @() } +$rolesAdd = if ($null -ne $roles['add']) { @($roles['add']) } else { @() } $rolesRemove = if ($null -ne $roles['remove']) { @($roles['remove']) } else { @() } $combinedRoles = ($rolesSet + $rolesAdd + $rolesRemove) | Select-Object -Unique From 28a3a2c31dd39d6275f320dcecae7d4418edebee Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 8 Jan 2026 14:23:52 -0500 Subject: [PATCH 24/24] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- changelogs/changelog.yaml | 2 -- plugins/modules/user_role.ps1 | 3 +++ plugins/modules/user_role.py | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml index 74dfd580..3c9391a1 100644 --- a/changelogs/changelog.yaml +++ b/changelogs/changelog.yaml @@ -589,5 +589,3 @@ releases: - Added ability for user_role module to take a list of roles. Optionally can also remove unlisted roles. release_summary: Added support for managing multiple roles per user with the user_role module. - fragments: - - 333-user-role-list.yml diff --git a/plugins/modules/user_role.ps1 b/plugins/modules/user_role.ps1 index 95adedda..b3f8cfbf 100644 --- a/plugins/modules/user_role.ps1 +++ b/plugins/modules/user_role.ps1 @@ -32,6 +32,9 @@ $spec = @{ elements = 'str' } set = @{ + # Intentionally use $null (not @()) so later checks (e.g. $null -ne $roles["set"]) + # can distinguish "not provided" (null) from "provided as empty array" ([]), + # allowing roles.set: [] to remove all roles. default = $null type = 'list' elements = 'str' diff --git a/plugins/modules/user_role.py b/plugins/modules/user_role.py index aed592f0..1ac34b17 100644 --- a/plugins/modules/user_role.py +++ b/plugins/modules/user_role.py @@ -54,10 +54,10 @@ description: - Adds the user to the specified roles. - User will be removed from any other roles not specified. - - Set this to an empty list to remove all members from a group. + - Set this to an empty list to remove all role memberships from the user. type: list elements: str - version_added: 2.7.0 + version_added: 2.8.0 author: "John McCall (@lowlydba)" requirements: - L(dbatools,https://www.powershellgallery.com/packages/dbatools/) PowerShell module