Skip to content

Fix InteractiveBrowserCredential caching#8352

Merged
chlowell merged 8 commits intorelease/dec-rollupfrom
browser-credential-cache
Nov 5, 2019
Merged

Fix InteractiveBrowserCredential caching#8352
chlowell merged 8 commits intorelease/dec-rollupfrom
browser-credential-cache

Conversation

@chlowell
Copy link
Member

@chlowell chlowell commented Nov 1, 2019

With these changes, InteractiveBrowserCredential delegates refresh token handling to MSAL, and so should only open a browser to login once (fixes #8339). This script provides a quick validation:

from azure.identity import InteractiveBrowserCredential

cred = InteractiveBrowserCredential()
tokens = {cred.get_token('https://management.azure.com/.default').token for _ in range(2)}
assert len(tokens) == 1

It should open a browser exactly once, and not raise AssertionError.

These changes also fix #8340, which was caused by calling super on HTTPServer (an old-style class).

@chlowell chlowell added Client This issue points to a problem in the data-plane of the library. Azure.Identity labels Nov 1, 2019
@chlowell chlowell requested a review from schaabs as a code owner November 1, 2019 21:31
@chlowell chlowell self-assigned this Nov 1, 2019
@adxsdk6
Copy link

adxsdk6 commented Nov 1, 2019

Can one of the admins verify this patch?

@chlowell
Copy link
Member Author

chlowell commented Nov 1, 2019

/azp run python - identity

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chlowell
Copy link
Member Author

chlowell commented Nov 2, 2019

python - identity - tests is the live test pipeline. I ran it against this branch before opening this PR. It failed as expected (because of #8314) but DevOps apparently associated that run with the commit, so this PR had failing (non-CI) tests the moment I opened it 🤷‍♂

Upshot: if not for this unexpected DevOps behavior, this would be a green PR.

@chlowell
Copy link
Member Author

chlowell commented Nov 2, 2019

/azp run python - identity - tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@chlowell chlowell changed the base branch from master to release/dec-rollup November 4, 2019 20:02
@chlowell chlowell force-pushed the browser-credential-cache branch from 7388d0e to fd71738 Compare November 4, 2019 20:02
@chlowell chlowell force-pushed the browser-credential-cache branch from fd71738 to de7ac7e Compare November 4, 2019 20:04
@chlowell chlowell force-pushed the browser-credential-cache branch 2 times, most recently from a1be35d to 9736f6b Compare November 5, 2019 16:36
@chlowell chlowell force-pushed the browser-credential-cache branch from 9736f6b to 18126e6 Compare November 5, 2019 17:00
@chlowell chlowell merged commit 73571eb into release/dec-rollup Nov 5, 2019
@chlowell chlowell deleted the browser-credential-cache branch November 5, 2019 17:19
xiangyan99 added a commit that referenced this pull request Nov 8, 2019
* Updated minor version for released packages in hotfix branch

* Refactor ClientCertificateCredential construction (#8315)

* Update MSAL dependency (#8359)

* Fix InteractiveBrowserCredential caching (#8352)

* Disable depends test (#8440)

* [EventHubs] Final README update (#8430)

* [EventHubs] Final docs update (#8428)

* Readme final update

* Update links

* update async sample in readme

* Simply samples in readme and make samples can run (#8432)

* Tiny readme fix

* Update checkpointstore README

* Update checkpointstore README

* Fix a README error

* App Configuration 2019-10-01  (#8394)

* update sdk\keyvault\azure-keyvault-secrets\README.md (#8396)

* bump warden version (#8460)

* Add passing kwargs and HttpLoggingPolicy plugged (#8053)

* Live tests for managed identity, username/password, browser credentials (#7622)

* policy v2019_09_01 (#8397)

* Fix media streaming bug (#8385)

* initial commit to fix media streaming bug

- Removed media streaming related checks in synchronized_request that was causing issues with DBAs with "media" in the name
- Refactored pipeline_client.run into a _PipelineRun method for testing purposes

* bumped version

* removed references to 'Media'

* bumped version in readme

* removed unused import for pylint

* undid test config changes

* updated HISTORY with breaking changes

* modified changelog

* Work around sphinx markdown heading link bug (#8255)

* move smoke tests

update smoke-test.yml

move smoketests to common folder

* consistent smoke test requirements

* remove smoke test invocations that don't exist

* use original interface

* remove phantom smoke tests

* New CI to check autorest when changes are made in azure-core (#8509)

* ADLS Gen2 API Implementation (#8473)

* ADLS Gen2 API Implementation (#8473)

* network parameter change (#8529)

* [AutoPR] appconfiguration/resource-manager (#8528)

* New azure-mgmt-web + fixed mixin script (#8334)

* list skus test

* adding api_version in mixin instance

* regenerated new package

* regeneated tests

* history and version

* [AutoPR] cognitiveservices/data-plane/LUIS/Authoring (#7816)

* [AutoPR cognitiveservices/data-plane/LUIS/Authoring] Add v3.0 swagger (#7815)

* Generated from 7a554c128d6eba201c576885c8c2ea78f5267481

Add v3.0 swagger

* Generated from d3a4f2d1674a63bc9a4a57015f956833c7d40f7b

Update LUIS-Authoring.json

fix semantic bugs

* regenerated luis

* updated history and version
@chlowell chlowell mentioned this pull request Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Identity Client This issue points to a problem in the data-plane of the library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments