Skip to content

Conversation

@msJinLei
Copy link
Contributor

@msJinLei msJinLei commented Mar 16, 2021

Description

  • Implemented CAE by adding handler to http pipeline
  • Improved error message when login is blocked by AAD
  • Improved error message when silent reauthentication failed
  • Enabled CAE for Get-AzTenant and Get-AzSubcription
  • Added test cases

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
      • {Please put the link here}
    • the markdown help files have been regenerated using the commands listed here

@msJinLei msJinLei force-pushed the credentialfacactory branch 3 times, most recently from b25e4bc to 436ce9e Compare March 17, 2021 11:49
@msJinLei msJinLei changed the title Apply credential factory to SilentAuthenticator CAE Mar 17, 2021
@msJinLei msJinLei force-pushed the credentialfacactory branch 3 times, most recently from f419108 to 0b70c09 Compare March 18, 2021 15:27
src/Aks/Aks.sln Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

@msJinLei msJinLei force-pushed the credentialfacactory branch 4 times, most recently from 56aaea0 to 41a997d Compare March 22, 2021 14:48
@msJinLei
Copy link
Contributor Author

/azp run azure-powershell - powershell-core

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@msJinLei msJinLei force-pushed the credentialfacactory branch 4 times, most recently from e53432d to b99b813 Compare March 23, 2021 09:13
@msJinLei msJinLei marked this pull request as ready for review March 23, 2021 09:15
@msJinLei msJinLei force-pushed the credentialfacactory branch 3 times, most recently from 0bf0dce to f81f41d Compare March 25, 2021 07:55
@msJinLei
Copy link
Contributor Author

/azp run azure-powershell - windows-powershell

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@msJinLei msJinLei force-pushed the credentialfacactory branch from f81f41d to 957ef58 Compare March 26, 2021 02:11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WriteWarning could not be executed in thread other than main thread, please refer to https://github.com/Azure/azure-powershell/blob/master/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs#L246

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO should be verified before release

@msJinLei msJinLei force-pushed the credentialfacactory branch 3 times, most recently from 09a86e7 to 8cd7cbe Compare March 26, 2021 09:00
@msJinLei msJinLei requested a review from erich-wang March 26, 2021 09:01
@isra-fel
Copy link
Member

isra-fel commented Jul 5, 2021

Let me move the pull request to draft state. Please feel free to mark it "Ready for review" when you feel comfortable. Thanks

@isra-fel isra-fel marked this pull request as draft July 5, 2021 05:27
@msJinLei
Copy link
Contributor Author

msJinLei commented Jul 5, 2021

Let me move the pull request to draft state. Please feel free to mark it "Ready for review" when you feel comfortable. Thanks

It is not because the feature is still in development that the PR is not merged. It is because the sever side is not ready that we cannot release the feature.

@isra-fel
Copy link
Member

isra-fel commented Jul 5, 2021

It is not because the feature is still in development that the PR is not merged. It is because the sever side is not ready that we cannot release the feature.

Got it. It's just easier for maintanence. Thanks for understanding 😀

@msJinLei msJinLei force-pushed the credentialfacactory branch from 8cd7cbe to 0fa0c9a Compare July 13, 2021 04:36
@msJinLei msJinLei changed the base branch from main to share/cae July 13, 2021 04:36
@msJinLei msJinLei force-pushed the credentialfacactory branch 6 times, most recently from 462c840 to ac04355 Compare July 15, 2021 10:45
erich-wang and others added 2 commits July 15, 2021 18:45
* Improved error message when login is blocked by AAD
* Improved error message when silent reauthentication failed
* Enable CAE for Get-AzTenant and Get-AzSubcription
* Add test case
@msJinLei msJinLei force-pushed the credentialfacactory branch from ac04355 to 44d3acf Compare July 15, 2021 10:45
@msJinLei msJinLei marked this pull request as ready for review July 15, 2021 14:48
@isra-fel isra-fel self-assigned this Jul 16, 2021
@msJinLei msJinLei merged commit 7cd63d2 into Azure:share/cae Jul 19, 2021
msJinLei added a commit to msJinLei/azure-powershell that referenced this pull request Feb 7, 2022
* prototype for CAE

* CAE issue fix

* Improved error message when login is blocked by AAD
* Improved error message when silent reauthentication failed
* Enable CAE for Get-AzTenant and Get-AzSubcription
* Add test case

Co-authored-by: Erich(Renyong) Wang <eriwan@microsoft.com>
msJinLei added a commit that referenced this pull request Feb 10, 2022
* CAE (#14567)

* prototype for CAE

* CAE issue fix

* Improved error message when login is blocked by AAD
* Improved error message when silent reauthentication failed
* Enable CAE for Get-AzTenant and Get-AzSubcription
* Add test case

Co-authored-by: Erich(Renyong) Wang <eriwan@microsoft.com>

* Migrate on-claim-chanllenge handler to the new authentication step for autorest

*Enable CAE for MSGraph (#16766)

* Update ChangeLog.md

* Address review comments

* Fix duplicate request issue

* Address review comments

Co-authored-by: Erich(Renyong) Wang <eriwan@microsoft.com>
Co-authored-by: Yunchi Wang <54880216+wyunchi-ms@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants