dbup-sqlserver uses deprecated Microsoft.Azure.Services.AppAuthentication

[ernstad]

When updating our solutions packags we noticed that dbup-sqlserver has a dependency to Microsoft.Azure.Services.AppAuthentication >= 1.6.2 which is deprecated. Suggesting an alternate implementation with regards to the suggestion in issue DbUp/DbUp#641.

[mjauernig]

Is a special Azure Implementation necessary? Or would you use Azure with the dbup-sqlserver module and this Azure Connection Settings?

[mjauernig]

Found a migration guide. I will take a look in the next days.

[ernstad]

Nice. We currently don't even use sql server with Azure so if it could be opted in it would be great. Minimum the migration to Azure.Identity would be much appreciated.

[evochriso]

@mjauernig the linked PR should resolve this issue, can you review? Thanks!

[ernstad]

Any update on the PR?

[peymanr34]

The linked PR has some issues:

• There is a GetToken method which I think it should be used instead of GetTokenAsync.
• The tenantId parameter hasn't been used, it should've been passed to the TokenRequestContext.
• The azureAdInstance parameter hasn't been used (which I don't know where it should go, maybe it's redundant?).

[evochriso]

@peymanr34 @ernstad @mjauernig :

• I used GetTokenAsync since thats what was used with the current build. If that needs to be changed for this PR to be accepted, someone please let me know and I change it.
• Good catch, I'll fix this. I was going to let environment variables handle these settings, but since tenantId is already a param of the constructor, I agree its better to use that.
• I believe this has been removed, but additional parameters like that can be specified using environment variables, see the README here: https://www.nuget.org/packages/Azure.Identity

[evochriso]

@peymanr34 @ernstad @mjauernig @droyad
I have resubmitted a PR with the new repo setup to resolve this issue with the suggestions above, please review and approve/comment.

[Bartleby2718]

Looking very much forward to this being reviewed!

dbup-sqlserver depends on Microsoft.Azure.Services.AppAuthentication 1.6.2,
which depends on Microsoft.IdentityModel.Clients.ActiveDirectory 5.2.9,
which depends on a bunch of mostly deprecated packages:

     "Microsoft.CSharp": "4.3.0",
     "NETStandard.Library": "1.6.1",
     "System.ComponentModel.TypeConverter": "4.3.0",
     "System.Dynamic.Runtime": "4.3.0",
     "System.Net.Http": "4.3.4",
     "System.Private.Uri": "4.3.2",
     "System.Runtime.Serialization.Formatters": "4.3.0",
     "System.Runtime.Serialization.Json": "4.3.0",
     "System.Runtime.Serialization.Primitives": "4.3.0",
     "System.Security.Cryptography.X509Certificates": "4.3.0",
     "System.Security.SecureString": "4.3.0",
     "System.Xml.XDocument": "4.3.0",
     "System.Xml.XmlDocument": "4.3.0"

[Bartleby2718]

Unrelated, but I noticed in @evochriso's PR that dbup-sqlserver doesn't have a corresponding nuspec file. Is this intentional?

[evochriso]

Unrelated, but I noticed in @evochriso's PR that dbup-sqlserver doesn't have a corresponding nuspec file. Is this intentional?

This is intentional - the nuspec file gets generated by the build pipeline.

[ernstad]

Any update on the issue and linked PR?