Release a new version with an update ref to 'Azure.Identity'

[SimonCropp]

'Azure.Identity' 1.11.3 has a known moderate severity vulnerability, GHSA-m5vv-6r4h-3vj9

can you release a version that updates this reference

[dauinsight]

We'll look to have this scheduled for our next release

[SimonCropp]

this is a poor experience for consumers. for anyone who has CVE scanning turned on it breaks builds. and they need to explicitly add an azure reference to work around it. so for us it means doing a temporary hack on approx 40 projects just to build again. note that we dont even use azure.

if the sqlclient wants force a transitive azure ref on non azure users, then you should pro-actively release cve related fixes for azure.

[PureKrome]

if the sqlclient wants force a transitive azure ref on non azure users

or a version (stripped down?) without this nuget and whatever it offers? or how about the default version doesn't have this at all and a 2nd version has this so people who want to use Azure.Identity with their SqlClient can use that version. For the 99% rest of us, we don't need to worry about this transitive dep anymore/then.

[jnm2]

One reason of many to want #1108.

[ErikEJ]

@jnm2 Sure, but a better solution is to have a short timeline for release of s patch.

[jnm2]

For this individual occurrence, yes, but I'm also thinking of the future ones.

[CamiloTerevinto]

@jnm2 Sure, but a better solution is to have a short timeline for release of s patch.

Why is a better solution to have a shorter timeline on a dependency that many people don't even need? It would be one thing if SQL Server was Azure only, but there are the obvious cases of people hosting on-prem, on AWS and elsewhere - even on an Azure VM.

[ErikEJ]

@CamiloTerevinto if you are on premises I guess the vulnerability would not affect you, but Azure users would want a patch as soon as possible?

[CamiloTerevinto]

@ErikEJ it affects everyone that treats package vulnerability warnings as errors and those that use SCA as well.

[ErikEJ]

@CamiloTerevinto Exactly, so "a short timeline for release of a patch" would solve it for everyone! 😄

[JRahnama]

@CamiloTerevinto Exactly, so "a short timeline for release of a patch" would solve it for everyone! 😄

During last two months, Azure.Identity has released 5 consecutive version which four of them are deprecated now. Your suggestion indicates we should have had almost 3 patch releases per month.

An automated process could be created to check Azure.Identity latest version for deprecation notice each day and if there is any, hotfix release pipeline could be triggered to release a new version....

Let me discuss this internally with other team members and get back to you.

[ErikEJ]

@JRahnama yeah, it is a lot of updates, but would make all your consumers happy, and potentially avoid/obliviate the requirement to split the package.

[ErikEJ]

@JRahnama If you do this, consider that Azure.Identity depends on Microsoft.Identity.Client, so you should really not have an explicit dependcy on Micrsoft.Identity.Client, like you do today:

https://www.nuget.org/packages/Azure.Identity#dependencies-body-tab

    <AzureIdentityVersion>1.11.3</AzureIdentityVersion>
    <MicrosoftIdentityClientVersion>4.60.3</MicrosoftIdentityClientVersion>

[CamiloTerevinto]

@JRahnama yeah, it is a lot of updates, but would make all your consumers happy, and potentially avoid/obliviate the requirement to split the package.

@ErikEJ I don't understand this position to be honest. Why do you think that having dependencies that aren't needed for some clients but that are kept up to date somehow better than just not having the dependencies at all?
Even if you updated Azure.Identity and other dependencies quickly for security vulnerabilities, that's only meant for customers that are using Azure SQL - for the rest, you're making them see the release notes for this package to see how the release affects them.
You are also hoping that users will update this package quickly enough with the security vulnerability warning, and I have seen people ignore these on VS even.

@JRahnama If Azure.Identity is your dependency, to me, it doesn't matter how many security hotfix releases they do, it should be a high priority to get this package updated with the dependency patched. Otherwise, you are saying to users of this package that you don't care about their security.

[ErikEJ]

@CamiloTerevinto Because I seriously doubt the package split will ever happen, and given that, this would be the desired outcome that pleases most users expect the ones concerned about a few more MB payload.

[PureKrome]

Because I seriously doubt the package split will ever happen, and given that, this would be the desired outcome that pleases most users expect the ones concerned about a few more MB payload.

Sadness. :(

Putting MB payload size and shorter timeframes, etc aside - the issue is really about RESPECT for the consumers here. I (and I feel like many others) find the poor solution design to bake in Azure in to the core product as message suggesting (assuming? or pushing from a sales perspective?) that users should be using SQL with SqlAzure. Not the other options/flavours.

A more respectful solution would be to split out the specific pieces of sql like SqlAzure, etc to their own nugets so people can use what is the bare necessity to them.

What we (the consumers) have to deal with now is the "dependency-hassle" of components/parts of SqlClient which we will never use. This hassle IS NOT about a payload size. sheesh!

And to hand wave this off as 'just deal with it/get over it/it's nothing', we feel is a bit rude :(

ESPECIALLY when this could potentially (I believe) be architected in way to split these concerns out.

If it's about (lack of) manpower - then say so.
If it's about (lack of) budget - then say so.
If it's about a strong design decision - then please educate us so we can understand the gap in understanding.
If it's just a low priority / can't be bothered - then also, please say so.
If you would accept a PR for this - also say as such.

[ErikEJ]

@PureKrome This has been discussed at very great length here: #2247

And based on the fact that this comment from @David-Engel is now 5 months old, I am considering alternative approaches

Also, I am not a Microsoft vendor, I am just an open source contributor from the .NET community.

[PureKrome]

Thanks heaps @ErikEJ - sincerely appreciate the links, comments and help 🤗 🍰

EDIT: Subscribed to #2247 👍🏻

[JRahnama]

@JRahnama If you do this, consider that Azure.Identity depends on Microsoft.Identity.Client, so you should really not have an explicit dependcy on Micrsoft.Identity.Client, like you do today:

https://www.nuget.org/packages/Azure.Identity#dependencies-body-tab

    <AzureIdentityVersion>1.11.3</AzureIdentityVersion>
    <MicrosoftIdentityClientVersion>4.60.3</MicrosoftIdentityClientVersion>

ActiveDirectoryAuthenticationProvider and some other classes is using Microsoft.Identity.Client for AAD authentication. There is a possibility to use Azure.Identity replacement for AAD, but that will limit the useage to Azure users. Can you add a bit more to clarify it?

[ErikEJ]

Can you add a bit more to clarify it?

@JRahnama
I just meant that you do not need an explicit reference and version spec to Microsoft.Identity.Client, it is already referenced via the use of Azure.Identity - I will create a PR to demonstrate

[JRahnama]

Can you add a bit more to clarify it?

@JRahnama I just meant that you do not need an explicit reference and version spec to Microsoft.Identity.Client, it is already referenced via the use of Azure.Identity - I will create a PR to demonstrate

Awesome, it will make it much easier to create an azure function to trigger build pipelines when a new version is released.

[ErikEJ]

@JRahnama Guess I should really backport the reference reduction change to 5.1 and 5.2 then?

[roji]

@JRahnama I'd highly recommend simply looking at enabling Dependabot for dependency changes, rather than building some custom mechanism you'd need to maintain (e.g. via Azure Functions). Dependabot is a built-in Github feature which automatically submits PRs to your repo whenever a dependency is released, and is specifically geared towards keeping your library secure in the face of security issues in its dependency graph (see docs). You basically wake up in the morning and get a PR bumping up e.g. the Azure.Identity version, and the automated CI checks tell you whether everything is OK.

Note that for Dependabot to function correctly, your project needs to follow some basic standard .NET patterns in how it represents the versions in csproj; I know SqlClient has a somewhat complicated build structure which may interfere with that. But i'd recommend looking into it, and cleaning up the SqlClient build system would ideally be a goal unto itself.

That takes care of bumping dependency versions - but when you actually release a new SqlClient version to your users is another question. I'd advise against trying to automatically release anything; not every patch version bump of Azure.Identity (or some other dependency) is something that merits immediately releasing a new SqlClient version - that would cause lots of churn to users that may not care (or even use Azure). I'd invest in being able to release very quickly for when a security issue (or other critical bug) is discovered, but otherwise keep the actual decision to release manual.

Finally, as discussed many times before, I'd keep the dependency management/releasing conversation distinct from whether SqlClient should depend on Azure.Identity at all - the two really are orthogonal (even if the dependency causes a lot of grief in terms of needing to release). I absolutely agree that SqlClient should not depend on Azure.Identity (#1108), but the dependency management/releasing question must be solved regardless of that. After all, even if the Azure.Identity dependency were split off to an external package (e.g. Microsoft.Data.SqlClient.Azure), the exact same problems would apply there.

[SimonCropp]

out of curiosity, do MS projects not do CVE checking? ie by enabling this in your Directory.Build.props

    <NuGetAuditMode>all</NuGetAuditMode>
    <NuGetAuditLevel>low</NuGetAuditLevel>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>

[cremor]

@JRahnama Guess I should really backport the reference reduction change to 5.1 and 5.2 then?

Did this happen?

[ErikEJ]

@cremor No.

[dauinsight]

Hotfixes 5.1.6 and 5.2.2 have been released which address this issue! Appreciate everyone's patience