Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

SQLServerRole uses contains() which is case sensitive #1153

Closed
brwilkinson opened this issue Jun 27, 2018 · 4 comments · Fixed by #1453
Closed

SQLServerRole uses contains() which is case sensitive #1153

brwilkinson opened this issue Jun 27, 2018 · 4 comments · Fixed by #1453
Labels
bug The issue is a bug. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub

Comments

@brwilkinson
Copy link

get-module -name sqlserverdsc -ListAvailable

Directory: C:\Program Files\WindowsPowerShell\Modules

ModuleType Version Name
Manifest 11.2.0.0 SqlServerDsc

https://github.com/PowerShell/SqlServerDsc/blob/dev/DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1#L113

Get-DscResource sqlserverrole -syntax

SqlServerRole [String] #ResourceName
{
    InstanceName = [string]
    ServerName = [string]
    ServerRoleName = [string]
    [DependsOn = [string[]]]
    [Ensure = [string]{ Absent | Present }]
    [Members = [string[]]]
    [MembersToExclude = [string[]]]
    **[MembersToInclude = [string[]]]**
    [PsDscRunAsCredential = [PSCredential]]
}

The MemberstoInclude property currently uses the contains method.

if ( -not ($membersInRole.Contains($memberToInclude)))

This is case sensitive . . .

e.g

$lista = 'abc','123'
$lista.Contains('abc')
> True
$lista.Contains('Abc')
> False

This is not desirable since possible the domain name or usename may be in a different case that the account already present on SQL.

The workaround might be to use the -In or the -contains operator.

$lista -contains 'abc'
>True
$lista -contains 'Abc'
> True
@johlju
Copy link
Member

johlju commented Jun 28, 2018

Thanks for reporting this! Labeling this as bug and help wanted so anyone in the community can run with this. Would be great to get a regression test for this too, so the same problem does not return in the future. 🙂

@johlju johlju added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub labels Jun 28, 2018
@paoIpao
Copy link

paoIpao commented Oct 10, 2018

Same problem.

@SteveL-MSFT SteveL-MSFT added this to Help Wanted in powershell/dscresources May 14, 2019
@SteveL-MSFT SteveL-MSFT removed this from Help Wanted in powershell/dscresources Nov 27, 2019
johlju pushed a commit that referenced this issue Jan 12, 2020
… checks (#1453)

- SqlServerRole
  - Add support for nested role membership (issue #1452).
  - Removed use of case-sensitive Contains() function when evalutating role membership
    (issue #1153).
  - Refactored mocks and unit tests to increase performance (issue #979).
@johlju
Copy link
Member

johlju commented Jan 12, 2020

It is supported in SQL Server to have the same login with different casing (in sensitive collations). But it will fail when compiling the mof-file as mentioned here in issue #1191.

@johlju johlju removed the help wanted The issue is up for grabs for anyone in the community. label Jan 12, 2020
@johlju
Copy link
Member

johlju commented Jan 12, 2020

This was resolved in PR #1453 since we can't enforce case-sensitivity in a mof resource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants