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

Implementing WAM broker authentication #78

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tabs-not-spaces
Copy link

Pull Request

Pull Request (PR) description

WAM Broker auth is cool. This PR should implement it correctly...
More details on PR to come, for now - beer awaits.

Task list

  • The PR represents a single logical change. i.e. Cosmetic updates should go in different PRs.
  • Added an entry under the Unreleased section of in the CHANGELOG.md as per format.
  • Local clean build passes without issue or failed tests.
  • Markdown help added/updated.
  • Unit tests added/updated.

@PalmEmanuel PalmEmanuel added the enhancement Code improvements, feature or feature request label Jul 7, 2024
Copy link
Contributor

github-actions bot commented Jul 7, 2024

Test Results

130 tests   122 ✅  0s ⏱️
 12 suites    0 💤
  1 files      8 ❌

For more details on these failures, see this check.

Results for commit 69b2bee.

♻️ This comment has been updated with latest results.

@PalmEmanuel
Copy link
Owner

PalmEmanuel commented Jul 7, 2024

Your PR from a fork uncovered a bug in my testing workflow, which is now fixed (thanks! 👍). Let me know if you have any questions about the tests or such, most of them just check that proper docs and parameter tests are in place.

There's also an approval step to run the code, since the last part of the workflow actually connects to a real identity in my tenant and gets a token as an integration test.

@tabs-not-spaces
Copy link
Author

Your PR from a fork uncovered a bug in my testing workflow, which is now fixed (thanks! 👍). Let me know if you have any questions about the tests or such, most of them just check that proper docs and parameter tests are in place.

There's also an approval step to run the code, since the last part of the workflow actually connects to a real identity in my tenant and gets a token as an integration test.

Oh if you have integration tests for WAM / broker auth, you'll need to add the redirect uri for it. ms-appx-web://microsoft.aad.brokerplugin/{clientId}

@PalmEmanuel
Copy link
Owner

Only got integration tests for the workload identity federation from this repo atm, so that's fine right now.

@PalmEmanuel
Copy link
Owner

This closes #76 when merged

@PalmEmanuel
Copy link
Owner

Looking forward to the silent auth with -Broker 👌

FYI I have some basic logic to save credentials in the session, so there's already some "silent auth" built-in if you run an interactive flow such as Get-AzToken -Interactive, Get-AzToken -Broker or Get-AzToken -DeviceCode once, then you can keep running Get-AzToken. I might revisit this logic eventually to make it more intuitive, but it works by storing the credential object in the session (including authentication record with the refresh token).

https://github.com/PalmEmanuel/AzAuth/blob/main/source/AzAuth.Core/TokenManagerAuthMethods/TokenManager.NonInteractive.cs#L39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Code improvements, feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants