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

Consider removing dependency on System.Security.Cryptography.ProtectedData #45217

Closed
m-redding opened this issue Jul 27, 2024 · 4 comments · Fixed by #45232
Closed

Consider removing dependency on System.Security.Cryptography.ProtectedData #45217

m-redding opened this issue Jul 27, 2024 · 4 comments · Fixed by #45232
Assignees
Labels
Azure.Core.TestFramework Azure.Identity Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.

Comments

@m-redding
Copy link
Member

Currently we have a dependency on version 4.7.0 of System.Security.Cryptography.ProtectedData in Packages.Data.Props. Upgrading this to 6.0.0 results in warnings, because these APIs only work on Windows operating systems, and they added these warnings in .NET 5. From what I can tell, even though we aren't getting warnings in 4.7.0, they are still not safe for use with non-Windows operating systems.

We should consider removing this dependency because it's only used in these two places:

Related

@m-redding m-redding added Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. Azure.Identity Azure.Core.TestFramework labels Jul 27, 2024
@jsquire
Copy link
Member

jsquire commented Jul 27, 2024

The main reason that this assembly is used is for the local encryption of the test environment file for local runs, which is only done on Windows. I think that's what the TestFramework tests are checking. Since this is a critical part of the developer loop for most Azure SDK developers, I think we want to leave this in the test framework and ignore the warnings. @JoshLove-msft: Anything there that you disagree with?

@christothes or @schaabs would be the folks with the best insight on the MsalCacheReader.

@christothes
Copy link
Member

I don't recall when we ever used MsalCacheReader, but it seems unreferenced as @m-redding mentioned. Seems like we could remove it.

@christothes
Copy link
Member

Here's a PR that moves the reference to test projects only and removes it from Identity #45232 . I didn't upgrade the version, this is just to validate the movement.

@m-redding
Copy link
Member Author

@christothes Thank you - that would work great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core.TestFramework Azure.Identity Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.
Projects
3 participants