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

Add support for Windows Authentication #24

Closed
wsmelton opened this issue Dec 24, 2020 · 21 comments
Closed

Add support for Windows Authentication #24

wsmelton opened this issue Dec 24, 2020 · 21 comments
Assignees
Labels
feature request New feature or request module issues/feature effecting module as a whole (e.g. standards, formatting, types)

Comments

@wsmelton
Copy link
Contributor

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

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

@wsmelton wsmelton added the feature request New feature or request label Dec 24, 2020
@wsmelton wsmelton added this to the 1.0 milestone Dec 24, 2020
@wsmelton wsmelton self-assigned this Dec 24, 2020
@wsmelton wsmelton added the module issues/feature effecting module as a whole (e.g. standards, formatting, types) label Dec 24, 2020
@wsmelton wsmelton modified the milestones: 1.0 Release, Backlog Feb 1, 2021
@peetrike
Copy link
Contributor

Is there anything done with this?

@wsmelton wsmelton added this to To do in General ToDo Feb 11, 2021
@wsmelton
Copy link
Contributor Author

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

@peetrike
Copy link
Contributor

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

@wsmelton
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

...so not a simply change

@wsmelton
Copy link
Contributor Author

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

@peetrike
Copy link
Contributor

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

@peetrike
Copy link
Contributor

peetrike commented Feb 11, 2021

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
Copy link
Contributor Author

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

@peetrike
Copy link
Contributor

peetrike commented Feb 11, 2021

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
Copy link
Contributor Author

Sorry but Invoke-TssRestApi is off-limits.

Parts are basically treated as private functions.

@wsmelton
Copy link
Contributor Author

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
Copy link
Contributor

peetrike commented Feb 11, 2021

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
Copy link
Contributor Author

wsmelton commented Feb 11, 2021

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
Copy link
Contributor

meanwhile, found small difference in TssSession class and New-TssSession function: #73.

Should I also open issue to mark that?

@peetrike
Copy link
Contributor

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
Copy link
Contributor Author

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 wsmelton modified the milestones: Backlog, 0.5 Release Feb 15, 2021
@wsmelton
Copy link
Contributor Author

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
Copy link
Contributor Author

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

General ToDo automation moved this from To do to Done Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request module issues/feature effecting module as a whole (e.g. standards, formatting, types)
Projects
No open projects
General ToDo
  
Done
Development

No branches or pull requests

2 participants