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

Stronger Verification for Azure Credential Factory test #86

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

Michael-A-McMahon
Copy link
Member

The test for TokenCredentialFactory will now invoke getToken() on the TokenCredential. This will light up some additional Azure SDK code, and catch defects like this: Azure/azure-sdk-for-java#39002

The defect linked to above will cause AZURE_DEFAULT/auto-detect authentication to fail when a AZURE_CLIENT_SECRET and AZURE_CLIENT_CERTIFICATE are not configured. Users may hit this when attempting to use AZURE_USERNAME and AZURE_PASSWORD for authentication.

Users can work around this by pulling in the latest azure-identity module in their own pom.xml:

    <dependency>
      <groupId>com.azure</groupId>
      <artifactId>azure-identity</artifactId>
      <version>1.12.1</version>
    </dependency>

@Michael-A-McMahon Michael-A-McMahon self-assigned this Jun 7, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 7, 2024
@Michael-A-McMahon
Copy link
Member Author

Michael-A-McMahon commented Jun 7, 2024

This is a real tricky one to catch! Notice how our CI run did not fail? That's because we have set AZURE_CLIENT_SECRET as an environment variable. When we do that, the DefaultCredential won't attempt to use the AZURE_USERNAME and AZURE_PASSWORD, even though we've set those as environment variables too.

Only way to really verify scenarios like this is to have two separate test runs which use different environment variables. So while the changes in this branch do add some value, they are not doing as much as I had hoped. To truly verify all scenarios, we'll need future work on the CI run. But for now, I'll add this test scenario to the list of those I verify manually before a release (the list already includes interactive authentication methods, might as well add this too) .

@Michael-A-McMahon Michael-A-McMahon merged commit 5a82224 into main Aug 20, 2024
2 checks passed
ting-lan-wang pushed a commit that referenced this pull request Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants