Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

ADFS 2.0 detection and fix by replacing strings #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rattuscz
Copy link
Contributor

related #16
Well I've ended up on this "solution".

Basically we need to evaluate available logs on the remote machines themselves, then replace "AD FS" with "AD FS 2.0" in log name, provider and also in query.

This is due to strings formatted before the log/provider name is sent to remote server ( MakeQuery receives already processed query and log.

Also not sure about using hardcoded strings in replacement, but using $script on remote would mean to either pass them all as params OR prepopulate the session with them before calling MakeQuery
Due to this I believe hardcoding AD FS 2.0 is a bit better as it's legacy and thus is not likely to ever change again.

Suggestions welcome in any way :-)

@bongiovimatthew-microsoft
Copy link
Contributor

I think this change looks good. @rattuscz, would you mind running the tests to make sure nothing has been regressed?

@rattuscz
Copy link
Contributor Author

rattuscz commented Jun 5, 2018

Could not make it work, even with the original code Pester fails.
I've installed Pester by just unzipping source to modules directory, which should be enough according to Pester install docs, commands are available so it's recognized correctly.

PS C:\adfs\logs> Invoke-Pester -Script .\Test.AdfsEventsModule.ps1
Executing all tests in '.\Test.AdfsEventsModule.ps1'
Executing script .\Test.AdfsEventsModule.ps1
  Describing Basic functionality of Get-ADFSEvents
    [-] Error occurred in Describe block 408ms
      ParameterBindingValidationException: Cannot bind argument to parameter 'Path' because it is null.
      at <ScriptBlock>, C:\adfs\logs\Test.AdfsEventsModule.ps1: line 71
      at Invoke-Blocks, C:\Program Files\WindowsPowerShell\Modules\pester\Functions\SetupTeardown.ps1: line 140
      at Invoke-TestGroupTeardownBlocks, C:\Program Files\WindowsPowerShell\Modules\pester\Functions\SetupTeardown.ps1:line 130
      at DescribeImpl, C:\Program Files\WindowsPowerShell\Modules\pester\Functions\Describe.ps1: line 166
Tests completed in 408ms
Tests Passed: 0, Failed: 1, Skipped: 0, Pending: 0, Inconclusive: 0

PS C:\adfs\logs> $PSVersionTable

Name                           Value
PSVersion                      5.1.14409.1005
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14409.1005
CLRVersion                     4.0.30319.36440
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

@bongiovimatthew-microsoft
Copy link
Contributor

That's strange. It looks like the tests all pass for me (both the original version, and using your code changes). This merge should be good to go once I get one more sign off from my side.

I'm not sure why the tests didn't work on your end. Maybe your Pester version isn't current? Here's my output:

PS C:\Users\Administrator\Desktop> Invoke-Pester -Script .\Test.AdfsEventsModule.ps1
Executing all tests in '.\Test.AdfsEventsModule.ps1'

Executing script .\Test.AdfsEventsModule.ps1

  Describing Basic functionality of Get-ADFSEvents
    [+] [00000]: 'All' Flag Returns CorrIDs that are valid guids 16.31s
    [+] [00000]: 'All' Flag Returns Multiple Aggregate Objects, with Multiple Events 2.8s
    [+] [00000]: 'All' Flag Returns Aggregate Objects, with Events by correlation ID 2.79s
    [+] [00100]: 'All' Flag with FromFile Returns Non-Empty Events List 1.49s
    [+] [01000]: 'All' Flag with AnalysisData Returns Analysis Objects 5.98s
    [+] [01001]: ByTime with AnalysisData Returns Analysis Objects 5.35s
    [+] [01001]: ByTime returns Multiple Aggregate Objects, with Multiple Events 1.58s
    [+] [01100]: 'All' Flag with AnalysisData with FromFile Returns Non-Empty Events List 5.5s
    [+] [01101]: ByTime with AnalysisData with FromFile returns Non-Empty Events List 5.67s
    [+] [10000]: CorrelationID Call Returns Exactly 1 Aggregate Object 8.85s
    [+] [10000]: CorrelationID Call Returns Non-Empty Events list 9.1s
    [+] [10000]: CorrelationID Call Returns Events list with 403 and 404 9.08s
    [+] [10000]: CorrelationID Call Returns Analysis Data With Single Request 12.6s
    [+] [10000]: CorrelationID Call Returns Analysis Data With Single Timeline Event 12.7s
    [+] [10100]: CorrelationID Call with FromFile returns Non-Empty Events List 2.01s
    [+] [10101]: ByTime with CorrelationID is not a valid scenario 64ms
    [+] [11100]: CorrelationID Call with AnalysisData with FromFile returns Non-Empty Events List 5.53s
Tests completed in 107.41s
Tests Passed: 17, Failed: 0, Skipped: 0, Pending: 0, Inconclusive: 0

PS C:\Users\Administrator\Desktop> $PSVersionTable

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


PS C:\Users\Administrator\Desktop> get-command invoke-pester

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Function        Invoke-Pester                                      4.3.1      Pester

@rattuscz
Copy link
Contributor Author

rattuscz commented Jun 5, 2018

I've actually used same version - 4.3.1, was latest stable in Pester repo.
Version table seems lower on my end by some minor versions.
As far as I can understand the errors somehow the arguments are not bound correctly to the Describe blocks, though I have no clue why or how I can fix it 🤷‍♂️

@bongiovimatthew-microsoft
Copy link
Contributor

That's very strange. Well I was able to run the test pass for you, and it looks good. If you're interested in debugging the tests on your side, you could try running in the PowerShell ISE and see if you can learn anything more. I'm happy to help debug if you are able to get more info from ISE.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants