resolve-ms-rest-azure-security-issue #5213
Conversation
|
@ramya-rao-a Could you help review this PR ? |
ramya-rao-a
left a comment
ramya-rao-a
left a comment
There was a problem hiding this comment.
Changes look good to me
Did you test this with any of the packages to ensure none of the breaking changes in adal-node affect us?
I run |
Please test one of the azure-arm-* packages with the changes being made for ms-rest-azure here |
|
Just realize all the packages azure-arm-* are using "^2.5.5" in their package,json which means non of them will use this package if we release it. Otherwise we will need to bump and release a new package for all the azure-arm-* packages. @ramya-rao-a What should we do ? |
|
By the way, I tested in for storage and it works fine with this new ms-rest-azure. |
Another option is to create a hotfix for ms-rest-azure with version 2.6.1 where you update the adal-node dependency. Given that we have limited investment in the packages in this repo as they are deprecated, this would be a better option in terms of effort. |
|
Hi @ramya-rao-a , after discussion, in order not to break our customers and make sure this security issue could be resolve at customer side. we will release both 2.6.1 and 3.0.1 for this change, and 2.6.1 will be changed based on 2.6.0 and 3.0.1 will be changed based on 3.0.0. |
|
That sounds good to me, thanks @qiaozha! |
|
Can you share the link for the PR for 2.6.1? |
|
Actually, I already release it, just revert this #5101 in my local and bump the adal-node version. I don't think we are going to merge this change in the code ? |
No, but ideally it would be preferred if a branch was maintained for v2 of ms-rest-azure. What if we had to do another hotfix on top of 2.6.1? |
|
Okay, I will send out the PR. for the change. Thanks |
|
@ramya-rao-a here's the PR #5216 |
to resolve issue #5212