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.

Sign up for GitHub

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

Jump to bottom

SqlPermission: Add support for additional server permissions #1704

Closed
beargle opened this issue May 18, 2021 · 1 comment · Fixed by #1778
Closed

SqlPermission: Add support for additional server permissions #1704

beargle opened this issue May 18, 2021 · 1 comment · Fixed by #1778
Labels
enhancement The issue is an enhancement request.

Comments

@beargle
Copy link

beargle commented May 18, 2021 •
edited
Loading

Background

In the SqlPermission resource, it seems these are the currently supported server permissions

SqlServerDsc/source/DSCResources/DSC_SqlPermission/DSC_SqlPermission.psm1

Line 45 in 49f78a8

[ValidateSet('ConnectSql', 'AlterAnyAvailabilityGroup', 'ViewServerState', 'AlterAnyEndPoint')]

It would be useful to have additional permissions available (e.g. ConnectAnyDatabase, ViewAnyDefinition).

A complete list is available via ServerPermissionSetValue enum

✦ ❯ ([Microsoft.SqlServer.Management.Smo.ServerPermissionSetValue]).GetEnumNames()
AdministerBulkOperations
AlterAnyCredential
AlterAnyConnection
AlterAnyDatabase
AlterAnyEventNotification
AlterAnyEndpoint
AlterAnyLogin
AlterAnyLinkedServer
AlterResources
AlterServerState
AlterSettings
AlterTrace
AuthenticateServer
ControlServer
ConnectSql
CreateAnyDatabase
CreateDdlEventNotification
CreateEndpoint
CreateTraceEventNotification
Shutdown
ViewAnyDefinition
ViewAnyDatabase
ViewServerState
ExternalAccessAssembly
UnsafeAssembly
AlterAnyServerAudit
AlterAnyServerRole
CreateServerRole
AlterAnyAvailabilityGroup
CreateAvailabilityGroup
AlterAnyEventSession
SelectAllUserSecurables
ConnectAnyDatabase
ImpersonateAnyLogin

I'm guessing the available permissions could vary based on the SMO version.

Questions

Wondering if there is any desire to derive the $Permission parameter validation from these enum names?

Or is it preferred to maintain a hardcoded permission list, for a more stable interface?
@johlju
Copy link
Member

johlju commented May 18, 2021 •
edited
Loading

Happy to review a PR that adds more permissions and unit and integration tests that covers the additional permissions.

@johlju johlju added enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community. labels May 18, 2021
@johlju johlju changed the title Add support for additional server permissions SqlPermission: Add support for additional server permissions May 18, 2021
@johlju johlju added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Jul 30, 2022
@johlju johlju mentioned this issue Jul 30, 2022
Merged
9 tasks
johlju added a commit that referenced this issue Jul 31, 2022
@johlju
BREAKING CHANGE: SqlPermission: Refactored resource (class-based reso…
1fe1dce 
…urce) (#1778)

- SqlServerDsc
  - The following classes were added to the module:
    - `ServerPermission` - complex type for the DSC resource SqlPermission.
  - The following public functions were added to the module (see comment-based
    help for more information):
    - `Test-SqlDscIsLogin`
    - `ConvertFrom-SqlDscServerPermission`
    - `ConvertTo-SqlDscServerPermission`
    - `Get-SqlDscServerPermission`
    - `Set-SqlDscServerPermission`
  - SMO stubs (used in the unit tests)
    - Was updated to remove a bug related to the type `ServerPermissionInfo`
      when used with the type `ServerPermissionSet`. The stubs suggested that
      the property `PermissionType` (of type `ServerPermissionSet`)
      in `ServerPermissionInfo` should have been a array `ServerPermissionSet[]`.
      This conflicted with real SMO as it does not pass an array, but instead
      a single `ServerPermissionSet`. The stubs was modified to mimic the
      real SMO. At the same time some old mock code in the SMO stubs was removed
      as it was no longer in use.
- SqlPermission
  - BREAKING CHANGE: The resource has been refactored. The parameters
    `Permissions` has been replaced by parameters `Permission`,
    `PermissionToInclude`, and `PermissionToExclude`. These permissions
    parameters are now an instance of the type `ServerPermission`.
    The type `ServerPermission` contains two properties; `State` and
    `Permission`. This closes the issue [issue #1761](#1761),
    it also fixes the issues [issue #1773](#1773),
    [issue #1704](#1704),
    and [issue #752](#752).
  - The resource was refactored into a class-based resource.
- SqlDatabasePermission
  - Fixed comment-based help and cleaned up comments.
  - Fix localized string that referenced 'user' instead of 'principal',
    and correct localized string ID for each string.
- `Set-SqlDscDatabasePermission`
  - Minor code cleanup.
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement The issue is an enhancement request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BREAKING CHANGE: SqlPermission: Refactored resource (class-based resource) johlju/SqlServerDsc
2 participants
@beargle @johlju