thycotic-ps / thycotic.secretserver

PowerShell module for automating with Thycotic Secret Server REST API
https://thycotic-ps.github.io/thycotic.secretserver
MIT License
68 stars 22 forks source link

Add support for Windows Authentication #24

Closed wsmelton closed 3 years ago

wsmelton commented 3 years ago

Is your feature request related to a problem? Please describe.

Add support to New-TssSession for Windows Authentication with IWA.

peetrike commented 3 years ago

Is there anything done with this?

wsmelton commented 3 years ago

You can see the status of issues and work via the Projects tab of the repository.

peetrike commented 3 years ago

Would you mind me making PR for it? It should be simple to add...

wsmelton commented 3 years ago

Actually supporting WinAuth is not simply adding the URL, just as an FYI because not all REST API endpoints are available to the winauthwebservices/api/v1 path.

You are welcome to create a PR though if you work it out, the contributing guide has details on the testing setup. As the command to work on this affects the module as a whole. Each test for a function will also need to be modified to include testing both endpoint access types.

wsmelton commented 3 years ago

I have plans to get a part script implemented for validating the version of Secret Server and it may need to be expanded to also cover outputting whether a given endpoint is not there for winauth 🤔 .

peetrike commented 3 years ago

Actually supporting WinAuth is not simply adding the URL, just as an FYI because not all REST API endpoints are available to the winauthwebservices/api/v1 path.

And every single function should be changed, because Invoke-RestApi doesn't take TssSession object as input parameter. Even if it would, all functions still need to be changed ...

wsmelton commented 3 years ago

...so not a simply change

wsmelton commented 3 years ago

TssSession will always be a required parameter on the functions. That won't change.

peetrike commented 3 years ago

Yes. I'm still interested of the feature, so let me at least think how it could be integrated :)

peetrike commented 3 years ago

TssSession will always be a required parameter on the functions. That won't change.

I'm more about thinking how to add -TssSession to Invoke-RestApi. So that it doesn't break existing code...

wsmelton commented 3 years ago

There is no reason to touch Invoke-TssRestApi. It does not need any changes to support WinAuth.

peetrike commented 3 years ago

i know. But otherwise all functions would have to deal with new type of session. Or a new "part" should be added, that would deal creating 'Invoke-TssRestApi' call.

You don't use private functions in this module?

wsmelton commented 3 years ago

Sorry but Invoke-TssRestApi is off-limits.

Parts are basically treated as private functions.

wsmelton commented 3 years ago

All of this work, off the top of my head (in theory) should be able to deal with this from New-TssSession processing the session object.

peetrike commented 3 years ago

in the beginning i thought that I need to change New-TssSession to emit session with new TokenType, and change TssSession class. But every other function calls Invoke-TssRestApi and they shoud be able to add correct parameters to that call.

That makes it to a lot of repeating code :)

wsmelton commented 3 years ago

Adjusting the token type would be one method to use that would allow us in the functions to validate functions that are hit that do not have an equivalent winauthwebservices endpoint.

The method in which the parameters are being passed in each function to Invoke-TssRestApi should be irrelevant because it will handle the token missing if it was not found. Pester tests should catch this currently though I believe if it would be an issue or not.

peetrike commented 3 years ago

meanwhile, found small difference in TssSession class and New-TssSession function: https://github.com/thycotic-ps/thycotic.secretserver/pull/73.

Should I also open issue to mark that?

peetrike commented 3 years ago

I'll create a PR and put only changes to New-TssSession and TssSession class (plus tests). Then the changes to functions using that type of token could be filed in later.

wsmelton commented 3 years ago

Checked the OpenAPI docs for 10.9.33 and I'm not catching any differences between the endpoints, so there may be no need to touch the functions to validate anything.

wsmelton commented 3 years ago

Worked over the weekend and finally got this working; had to change a good bit on how the tests and commands were run. I did find a possible bug in the endpoints used by Set-TssSecret. Those are being skipped when Windows Auth is used running tests for now until I can work out why 404 error is being thrown (could be my local lab setup, but I will test in our main lab this week sometime).

image

I've got one test that continues to spit the warning out, will circle back around to fix that at a later time.

wsmelton commented 3 years ago

Working on building the release to publish now so should be out by mid-day.