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.

Sign up for GitHub

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

Jump to bottom

DAC: do not stop trying with dev credentials until obtaining a valid token or completing the chain #34733

Closed
xiangyan99 opened this issue May 1, 2023 · 2 comments · Fixed by #35771
Closed

DAC: do not stop trying with dev credentials until obtaining a valid token or completing the chain #34733

xiangyan99 opened this issue May 1, 2023 · 2 comments · Fixed by #35771
Assignees
@billwert
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Milestone
2023-07

Comments

@xiangyan99
Copy link
Member

xiangyan99 commented May 1, 2023 •
edited
Loading

Currently if we find an expired token in shared token credential, we stop attempting next credential.

Let's continue on all dev credentials: do not stop trying with dev credentials until obtaining a valid token or completing the chain
@xiangyan99 xiangyan99 added Client This issue points to a problem in the data-plane of the library. Azure.Identity labels May 1, 2023
This was referenced May 1, 2023
Closed
Closed
@xiangyan99
Copy link
Member Author

Azure/azure-sdk-for-net#35919
#34733
Azure/azure-sdk-for-js#25717
Azure/azure-sdk-for-python#30167

@xiangyan99 xiangyan99 mentioned this issue May 1, 2023
Closed
@joshfree joshfree added this to the 2023-06 milestone May 1, 2023
@xiangyan99 xiangyan99 changed the title [FEATURE REQ]Revisit DAC: should we not stop on shared token cache credential if we find an expired one DAC: do not stop trying with dev credentials until obtaining a valid token or completing the chain May 22, 2023
@xiangyan99
Copy link
Member Author

The Python PR: Azure/azure-sdk-for-python#30441

@joshfree joshfree modified the milestones: 2023-06, 2023-07 Jun 19, 2023
@billwert billwert modified the milestones: 2023-07, 2023-08 Jun 19, 2023
billwert added a commit to billwert/azure-sdk-for-java that referenced this issue Jul 6, 2023
@billwert
Don't stop trying dev credentials on failures
87a2af5 
Fixes Azure#34733

For our dev time credentials we want to always keep going. This change wraps any failure from the credentials in a `CredentialUnavailableException` so `ChainedTokenCredential` will continue them properly.
@billwert billwert mentioned this issue Jul 6, 2023
Merged
@joshfree joshfree moved this from Planned to In Progress in Azure Identity SDK Improvements Jul 10, 2023
billwert added a commit to billwert/azure-sdk-for-java that referenced this issue Jul 14, 2023
@billwert
Don't stop trying dev credentials on failures
33f6a77 
Fixes Azure#34733

For our dev time credentials we want to always keep going. This change wraps any failure from the credentials in a `CredentialUnavailableException` so `ChainedTokenCredential` will continue them properly. It only does so in the context of a `ChainedTokenCredential`. Regular uses of these credentials is unaffected.
billwert added a commit that referenced this issue Jul 21, 2023
@billwert
Don't stop trying dev credentials on failures (#35771)
0d8f074 
* Don't stop trying dev credentials on failures

Fixes #34733

For our dev time credentials we want to always keep going. This change wraps any failure from the credentials in a `CredentialUnavailableException` so `ChainedTokenCredential` will continue them properly. It only does so in the context of a `ChainedTokenCredential`. Regular uses of these credentials is unaffected.

* add chained to clone
@github-project-automation github-project-automation bot moved this from In Progress to Done in Azure Identity SDK Improvements Jul 21, 2023
billwert added a commit to billwert/azure-sdk-for-java that referenced this issue Sep 6, 2023
@billwert
Don't stop trying dev credentials on failures
043e39a 
Fixes Azure#34733

For our dev time credentials we want to always keep going. This change wraps any failure from the credentials in a `CredentialUnavailableException` so `ChainedTokenCredential` will continue them properly. It only does so in the context of a `ChainedTokenCredential`. Regular uses of these credentials is unaffected.
srnagar added a commit that referenced this issue Oct 11, 2023
@billwert @srnagar
Eg sync stack testproxy (#36656)
cf5db91 
* Add missing sync test cases

* Enable sync-stack

* Remove extra set of aeg-channel-name.

* enable AssertingHttpClient

* add back missing Context arguments

* Don't stop trying dev credentials on failures

Fixes #34733

For our dev time credentials we want to always keep going. This change wraps any failure from the credentials in a `CredentialUnavailableException` so `ChainedTokenCredential` will continue them properly. It only does so in the context of a `ChainedTokenCredential`. Regular uses of these credentials is unaffected.

* Tests now use test proxy and recordings are pushed to assets repo

* address pr comments

* fix merge conflicts

* add exports

* fix version

* remove unused imports

* use unreleased test version

* fix tag

* fix tag

* refactor

* fix spring test after sync stack migration

* log the test proxy response on error

* fix header sanitizer in Service Bus

* fix regex for quantum job tests

* Add missing sync test cases

* Enable sync-stack

* Remove extra set of aeg-channel-name.

* enable AssertingHttpClient

* add back missing Context arguments

* Tests now use test proxy and recordings are pushed to assets repo

* address pr comments

* add exports

* fix version

* use unreleased test version

* fix tag

* fix tag

* refactor

* fix spring test after sync stack migration

* log the test proxy response on error

* fix header sanitizer in Service Bus

* fix regex for quantum job tests

* Put a semaphore around starting the proxy.

* make the methods of `TestProxyManager` static and synchronized.

* Use the working directory to search for the root of the repo from.

* add missing sanitizer setup

* base class has this so it's not needed here

* removed this in merge accidentally

* pin azure-core version

---------

Co-authored-by: Srikanta Nagaraja <srnagar@microsoft.com>
Co-authored-by: Srikanta <51379715+srnagar@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@billwert billwert

Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Projects
Azure Identity SDK Improvements
Archived in project
Milestone
2023-07
Development

Successfully merging a pull request may close this issue.

Don't stop trying dev credentials on failures billwert/azure-sdk-for-java
3 participants
@joshfree @billwert @xiangyan99