Skip to content

[KeyVault Certificates] v4.0.2 hotfix for #9005 and #9020#9122

Merged
sadasant merged 5 commits intoAzure:hotfix/keyvault-challengeAuth-certificates-from-4.0.0from
sadasant:hotfix/keyvault-challengeAuth-certificates-from-4.0.0-9005
Jun 1, 2020
Merged

[KeyVault Certificates] v4.0.2 hotfix for #9005 and #9020#9122
sadasant merged 5 commits intoAzure:hotfix/keyvault-challengeAuth-certificates-from-4.0.0from
sadasant:hotfix/keyvault-challengeAuth-certificates-from-4.0.0-9005

Conversation

@sadasant
Copy link
Copy Markdown
Contributor

@sadasant sadasant commented May 27, 2020

This PR updates @azure/keyvault-certificates to version 4.0.2. It includes a fix for the issue #9005, which consists in ensuring that parallel requests load the already cached credentials appropriately, and a fix for the issue #9020, which consists on ensuring that the updateCertificateProperties method correctly uses the certificate attributes.

I'm using the same base branch as the one used for the previous hotfix (4.0.1): https://github.com/Azure/azure-sdk-for-js/tree/hotfix/keyvault-challengeAuth-certificates-from-4.0.0

The fixes in greater detail are:

For the issue #9005

PR to master #9059

The recent hotfix showed an outstanding bug now that the cache is working correctly: Multiple async requests running at the same time quickly fail rather than re-using the credentials of the first authenticated.

This fix ensures that anytime we receive a response that indicates that authentication is necessary we update the original request with the available credentials and send it again, even if we already had a valid token and challenge in memory.

I also refactored the logic to make it easier to follow, so that future changes can be made more straightforwardly.

For the issue #9020

PR to master #9135

The bug is that updateCertificateProperties is not correctly passing the attributes required by the underlying method.

The fix I'm making is to change the updateCertificateProperties method to correctly set the certificateAttributes in the options bag that is passed to the generated updateCertificate method:

-        this.setParentSpan(span, requestOptions)
+        {
+          ...this.setParentSpan(span, requestOptions),
+          certificateAttributes: toCoreAttributes(options)
+        }

@sadasant sadasant self-assigned this May 27, 2020
@sadasant sadasant changed the base branch from master to hotfix/keyvault-challengeAuth-certificates-from-4.0.0 May 27, 2020 00:42
@sadasant
Copy link
Copy Markdown
Contributor Author

sadasant commented May 27, 2020

This PR is green, but I'm waiting for @xirzec 's approval on #9059

Update: That is done!

@sadasant sadasant changed the title [KeyVault Certificates] v4.0.2 Challenge based authentication hotfix [KeyVault Certificates] v4.0.2 hotfix for #9059 and #9135 May 27, 2020
@sadasant sadasant changed the title [KeyVault Certificates] v4.0.2 hotfix for #9059 and #9135 [KeyVault Certificates] v4.0.2 hotfix for #9059 and #9020 May 27, 2020
@sadasant sadasant changed the title [KeyVault Certificates] v4.0.2 hotfix for #9059 and #9020 [KeyVault Certificates] v4.0.2 hotfix for #9005 and #9020 May 27, 2020
@sadasant sadasant force-pushed the hotfix/keyvault-challengeAuth-certificates-from-4.0.0-9005 branch from 1c21b18 to 06ca42e Compare May 27, 2020 15:33
@sadasant sadasant marked this pull request as ready for review May 27, 2020 20:56
@sadasant sadasant requested a review from sophiajt as a code owner May 27, 2020 20:56
@sadasant
Copy link
Copy Markdown
Contributor Author

/check-enforcer evaluate

@KarishmaGhiya
Copy link
Copy Markdown
Member

/check-enforcer evaluate

@sadasant sadasant merged commit f5ec03c into Azure:hotfix/keyvault-challengeAuth-certificates-from-4.0.0 Jun 1, 2020
@sadasant sadasant deleted the hotfix/keyvault-challengeAuth-certificates-from-4.0.0-9005 branch June 1, 2020 21:22
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.

3 participants