Skip to content

Conversation

@erich-wang
Copy link
Member

Description

  1. disable auto refresh context from token cache
  2. allow to refresh context from token cache using Get-AzContext -ListAvailable -RefreshContextFromTokenCache

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)
    • the markdown help files have been regenerated using the commands listed here

@adxsdkps
Copy link
Collaborator

adxsdkps commented Mar 6, 2020

Can one of the admins verify this patch?

public SwitchParameter ListAvailable { get; set; }

[Parameter(Mandatory = false, ParameterSetName = ListAllParameterSet, HelpMessage = "Refresh contexts from token cache")]
public SwitchParameter RefreshContextFromTokenCache { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

I just want to confirm that eventually we shall get rid of this parameter. Is that correct?

@isra-fel
Copy link
Member

isra-fel commented Mar 7, 2020

Hi Erich, thanks for the PR.
I'm just confused with one thing: I thought the idea is to disable refresh context unless one situation: user passes in a flag to force doing that, right?
If that's true, then I don't get the logic in ConnectAzureRmAccount.cs to store/restore the original refreshcontext... because it will always be false.
Also there's still one place we assign true to refreshcontext.... I think we should change it to false.

@erich-wang
Copy link
Member Author

Hi Erich, thanks for the PR.
I'm just confused with one thing: I thought the idea is to disable refresh context unless one situation: user passes in a flag to force doing that, right?
If that's true, then I don't get the logic in ConnectAzureRmAccount.cs to store/restore the original refreshcontext... because it will always be false.
Also there's still one place we assign true to refreshcontext.... I think we should change it to false.

Hi Yeming, thanks for your comments. First of all, the change in this PR is just short-term solution, we finally need to figure out long term solution to support SSO.

After this PR, user could only refresh context from token cache by running

Get-AzContext -ListAvailable -RefreshContextFromTokenCache

For the property ShouldRefreshContextFromCache, I leave them on purpose:

  1. Leave one tricky way for us for debug purpose
  2. It is helpful information when implementing final SSO solution.

@erich-wang erich-wang merged commit 33abc32 into msal-build Mar 7, 2020
@erich-wang erich-wang deleted the erich/msal-disable-auto-refresh-context branch March 10, 2020 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants