dataplat / dbachecks

✔ SQL Server Environmental Validation
https://dbachecks.readthedocs.io/en/latest/
MIT License
460 stars 141 forks source link

New check - GuestUserConnect #975

Closed jpomfret closed 1 year ago

jpomfret commented 1 year ago

Changes this PR brings

Adding a new check for GuestUserConnect - but I think there is an issue with the original check so my results aren't matching.

I have run the following against localhost,7401 and the old test is saying we're good - but the new test correctly identifies the issue.

Wouldn't if someone could confirm my suspicions before we merge it in - cc @SQLDBAWithABeard, @ClaudioESSilva

USE pubs
GO
GRANT CONNECT TO guest

Running Invoke-PerfAndValidateCheck says we have issues...

╰─ff Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks -showTestResults
Running in PowerShell 7.3.4 64-bit.
Starting trace.
Stopwatch is high resolution, max resolution of timestamps is 100ns.
[16:12:48][Invoke-DbcCheckv4]
Key        Value
---        -----
Script     C:\GitHub\dbachecks\source\checks\Database.Tests.ps1
Tag        {GuestUserConnect}
ExcludeTag
Show       All
PassThru   True

Pester v4.10.1
Executing all tests in 'C:\GitHub\dbachecks\source\checks\Database.Tests.ps1' with Tags GuestUserConnect

Executing script C:\GitHub\dbachecks\source\checks\Database.Tests.ps1

  Describing Guest User

    Context Testing Guest user has CONNECT permission on localhost,7401
      [+] Database Guest user should return no CONNECT permissions in model on localhost,7401 1.05s
      [+] Database Guest user should return no CONNECT permissions in Northwind on localhost,7401 980ms
      [+] Database Guest user should return no CONNECT permissions in pubs on localhost,7401 880ms

  Describing Guest User

    Context Testing Guest user has CONNECT permission on localhost,7402
      [+] Database Guest user should return no CONNECT permissions in model on localhost,7402 1.17s

  Describing Guest User

    Context Testing Guest user has CONNECT permission on localhost,7403
      [+] Database Guest user should return no CONNECT permissions in AdventureWorks2022 on localhost,7403 29.8s
      [+] Database Guest user should return no CONNECT permissions in model on localhost,7403 26.85s
      [+] Database Guest user should return no CONNECT permissions in Northwind on localhost,7403 28.74s
      [+] Database Guest user should return no CONNECT permissions in pubs on localhost,7403 52.16s
Tests completed in 143.23s
Tests Passed: 8, Failed: 0, Skipped: 0, Pending: 0, Inconclusive: 0
Run finished after 00:02:24.5956554.
Tracing done. Got 52873 trace events.
Processing 52873 trace events.
Figuring out flow. (13ms)
Sorting events into lines. (31ms)
Counting averages and percentages for lines. (15ms)
Sorting events into functions. (31ms)
Counting averages and percentages for functions. (1ms)
Getting Top50 lines with the longest Duration. (12ms)
Getting Top50 lines with the longest SelfDuration. (17ms)
Getting Top50 lines with the most hits. (16ms)
Getting Top50 functions with the longest Duration. (2ms)
Getting Top50 functions with the longest SelfDuration. (2ms)
Getting Top50 functions with the most hits. (2ms)
Progress: A: 00:00:04.9370577 (0 ms) -> A: 00:00:03.3143926 (-1623 ms) -> A: 00:03:25.1832515 (201869 ms) -> A: 00:00:03.7320989 (-201451 ms) -> A: 00:02:24.5955714 (140863 ms)
Done. Try $originalCodetrace.Top50SelfDuration to get the report. There are also Top50Duration, Top50HitCount, Top50FunctionSelfDuration, Top50FunctionDuration, Top50FunctionHitCount AllLines and Events.
Running in PowerShell 7.3.4 64-bit.
Starting trace.
Stopwatch is high resolution, max resolution of timestamps is 100ns.
[16:15:15][Invoke-DbcCheck]
Key           Value
---           -----
SqlInstance   {localhost,7401, localhost,7402, localhost,7403}
Check         {GuestUserConnect}
SqlCredential System.Management.Automation.PSCredential

Pester v5.4.1

Starting discovery in 1 files.
Discovery found 297 tests in 1.98s.
Filter 'Tag' set to ('GuestUserConnect').
Filters selected 8 tests to run.
Running tests.

Running tests from 'C:\GitHub\dbachecks\source\checks\Databasev5.Tests.ps1'
Describing Guest User
 Context Testing Guest user has CONNECT permission
   [+] Database Guest user should return no CONNECT permissions in model on localhost,7401 12ms (4ms|8ms)
   [+] Database Guest user should return no CONNECT permissions in Northwind on localhost,7401 2ms (1ms|1ms)
   [-] Database Guest user should return no CONNECT permissions in pubs on localhost,7401 5ms (4ms|1ms)
    Expected $false, because we don't want the guest user to have connect access to our database., but got $true.
    at $psitem.GuestUserConnect | Should -BeFalse -Because "we don't want the guest user to have connect access to our database.", C:\GitHub\dbachecks\source\checks\Databasev5.Tests.ps1:231
    at <ScriptBlock>, C:\GitHub\dbachecks\source\checks\Databasev5.Tests.ps1:231

Describing Guest User
 Context Testing Guest user has CONNECT permission
   [+] Database Guest user should return no CONNECT permissions in model on localhost,7402 5ms (1ms|4ms)

Describing Guest User
 Context Testing Guest user has CONNECT permission
   [+] Database Guest user should return no CONNECT permissions in AdventureWorks2022 on localhost,7403 7ms (2ms|6ms)
   [+] Database Guest user should return no CONNECT permissions in model on localhost,7403 2ms (1ms|1ms)
   [+] Database Guest user should return no CONNECT permissions in Northwind on localhost,7403 15ms (2ms|14ms)
   [-] Database Guest user should return no CONNECT permissions in pubs on localhost,7403 7ms (4ms|2ms)
    Expected $false, because we don't want the guest user to have connect access to our database., but got $true.
    at $psitem.GuestUserConnect | Should -BeFalse -Because "we don't want the guest user to have connect access to our database.", C:\GitHub\dbachecks\source\checks\Databasev5.Tests.ps1:231
    at <ScriptBlock>, C:\GitHub\dbachecks\source\checks\Databasev5.Tests.ps1:231
Tests completed in 2.23s
Tests Passed: 6, Failed: 2, Skipped: 0 NotRun: 289
Run finished after 00:00:02.8845418.
Tracing done. Got 150616 trace events.
Processing 150616 trace events.
Figuring out flow. (34ms)
Sorting events into lines. (41ms)
Counting averages and percentages for lines. (43ms)
Sorting events into functions. (44ms)
Counting averages and percentages for functions. (1ms)
Getting Top50 lines with the longest Duration. (23ms)
Getting Top50 lines with the longest SelfDuration. (22ms)
Getting Top50 lines with the most hits. (18ms)
Getting Top50 functions with the longest Duration. (2ms)
Getting Top50 functions with the longest SelfDuration. (2ms)
Getting Top50 functions with the most hits. (2ms)
Progress: A: 00:00:03.3143926 (-1623 ms) -> A: 00:03:25.1832515 (201869 ms) -> A: 00:00:03.7320989 (-201451 ms) -> A: 00:02:24.5955714 (140863 ms) -> A: 00:00:02.8843991 (-141711 ms)
Done. Try $NewCodetrace.Top50SelfDuration to get the report. There are also Top50Duration, Top50HitCount, Top50FunctionSelfDuration, Top50FunctionDuration, Top50FunctionHitCount AllLines and Events.
[16:15:18][Invoke-PerfAndValidateCheck]
Running with

GuestUserConnect

Checks against 3 SQL Containers

With original Code it takes 144.60 Seconds
With New Code it takes 2.88 Seconds

New Code for these 1 checks
is saving 141.71 seconds
from a run of 144.60 seconds
New Code runs in 1.99 % of the time

[16:15:18][Invoke-PerfAndValidateCheck]
The Tags are the same
[16:15:18][Invoke-PerfAndValidateCheck]
The Total Tests Run are the same 8 8

WARNING: [16:15:18][Invoke-PerfAndValidateCheck]
Uh-Oh - The total tests Passed between v4 and v5 are not the same somehow.
For v4 We Passed
8 tests
and
For v5 we Passed
6 tests

WARNING: [16:15:18][Invoke-PerfAndValidateCheck]
Uh-Oh - The total tests Failed between v4 and v5 are not the same somehow.
For v4 We Failed
0 tests
and
For v5 we Failed
2 tests

[16:15:18][Invoke-PerfAndValidateCheck]
The Total Tests Skipped are the same 0 0

Also the performance of the old one is bad against the 2022 instance - look at those durations 😱 - tried this a couple of times, and just the raw v4 Invoke-DbcCheck code and it was all slow - big performance gain for this one.

Describing Guest User

    Context Testing Guest user has CONNECT permission on localhost,7403
      [+] Database Guest user should return no CONNECT permissions in AdventureWorks2022 on localhost,7403 29.8s
      [+] Database Guest user should return no CONNECT permissions in model on localhost,7403 26.85s
      [+] Database Guest user should return no CONNECT permissions in Northwind on localhost,7403 28.74s
      [+] Database Guest user should return no CONNECT permissions in pubs on localhost,7403 52.16s
jpomfret commented 1 year ago

Confirmed issue with old check