SqlServerDsc icon indicating copy to clipboard operation
SqlServerDsc copied to clipboard

SqlPermission can not grant permissions to a SqlRole

Open JDAVF opened this issue 10 months ago • 2 comments

Problem description

If you create a SQL Server Role with SqlRole, you can not grant permissions to this role with SqlPermission.

Verbose logs

System.InvalidOperationException: The name 'XXX' is not a login on the instance 'YYY'. (SP0004)

DSC configuration

-

Suggested solution

Change implementation to not just check for a login with Test-SqlDscIsLogin, but also for a role.

SQL Server edition and version

-

SQL Server PowerShell modules

Name      Version Path
----      ------- ----
SqlServer 22.3.0  C:\Program Files\WindowsPowerShell\Modules\SqlServer\SqlServer.psd1
SQLPS     15.0    D:\Program Files (x86)\Microsoft SQL Server\150\Tools\PowerShell\Modules\SQLPS\SQLPS.psd1

Operating system

OsName               : Microsoft Windows Server 2022 Standard
OsOperatingSystemSKU : StandardServerEdition
OsArchitecture       : 64-bit
WindowsVersion       : 2009
WindowsBuildLabEx    : 20348.1.amd64fre.fe_release.210507-1500
OsLanguage           : en-US
OsMuiLanguages       : {en-US}

PowerShell version

Name                           Value
----                           -----
PSVersion                      5.1.20348.2849
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.20348.2849
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

SqlServerDsc version

Name         Version Path
----         ------- ----
SqlServerDsc 17.0.0  C:\Program Files\WindowsPowerShell\Modules\SqlServerDsc\17.0.0\SqlServerDsc.psd1

JDAVF avatar Feb 20 '25 10:02 JDAVF

We shouldn't change Test-SqlDscIsLogin but rather add a new public command Test-SqlDscIsRole and also use that to check that the passed name is a principal that can be use to set permissions on.

Then the command Set-SqlDscServerPermission needs to be updated to support Test-SqlDscIsRole as well, and make sure Set-SqlDscServerPermission can set permissions on roles. https://github.com/dsccommunity/SqlServerDsc/blob/85ea1eda6a2d3c3bacd096983d55c99b75dba914/source/Public/Set-SqlDscServerPermission.ps1#L102

The the resource can also be changed. Maybe we don't even need this check in the resource as the public command will throw if there isn't a valid principal. That should probably be verified by an integration tests if there isn't one already.

https://github.com/dsccommunity/SqlServerDsc/blob/85ea1eda6a2d3c3bacd096983d55c99b75dba914/source/Classes/020.SqlPermission.ps1#L362

Happy to see a PRs that fixes this. I would prefer separate PRs for this so it is easier to get time to review:

  1. PR that adds Test-SqlDscIsRole with the corresponding unit tests. See issue #2058.
  2. PR that updates Set-SqlDscServerPermission to be able to set permission on roles, with the corresponding unit tests. See issue #2059.
  3. PR that removes the check if there is a login in the SqlPermission resource and update the corresponding unit tests and also adda a integration tests that checks that it correctly throws if there is no login och role principal (grantee name).

johlju avatar Feb 22 '25 09:02 johlju

@johlju thanks for the guidance - this is my first DSC script I'm writing, so please review my two PRs carefully.

IAMJDA avatar Feb 23 '25 16:02 IAMJDA