Skip to content

Fix/MFA authentication#298

Merged
erikdarlingdata merged 2 commits intoerikdarlingdata:devfrom
ClaudioESSilva:fix/MFA-Authentication
Apr 29, 2026
Merged

Fix/MFA authentication#298
erikdarlingdata merged 2 commits intoerikdarlingdata:devfrom
ClaudioESSilva:fix/MFA-Authentication

Conversation

@ClaudioESSilva
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #297

🤖 says:

Root cause: Microsoft.Data.SqlClient 7.0.0 introduced a breaking change — it extracted all Entra ID / Azure Active Directory authentication code into a separate package (Microsoft.Data.SqlClient.Extensions.Azure). Without it, attempting to use ActiveDirectoryInteractive (Entra MFA) throws the error you saw.

Fix applied: Added Microsoft.Data.SqlClient.Extensions.Azure (v1.0.0) to PlanViewer.Core.csproj. This package contains ActiveDirectoryAuthenticationProvider and all related Entra/Azure auth types that were removed from the core Microsoft.Data.SqlClient package. No code changes were required — just the missing dependency.

Which component(s) does this affect?

  • Desktop App (PlanViewer.App)
  • Core Library (PlanViewer.Core)
  • CLI Tool (PlanViewer.Cli)
  • SSMS Extension (PlanViewer.Ssms)
  • Tests
  • Documentation

How was this tested?

Tested to connect and succeeded
image

Checklist

  • I have read the contributing guide
  • My code builds with zero warnings (dotnet build -c Debug)
  • All tests pass (dotnet test)
  • I have not introduced any hardcoded credentials or server names

Copy link
Copy Markdown
Owner

@erikdarlingdata erikdarlingdata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Diagnosis matches the SqlClient 7.0.0 release notes — Entra/Azure auth was extracted into Microsoft.Data.SqlClient.Extensions.Azure, and adding the package reference is the documented migration. Confirmed the package on NuGet (Microsoft-published, 1.0.0, released 2026-03-17).

Removing the 80-char truncation is a quiet bonus fix — that line was hiding the actionable part of the new error messages, since SqlClient 7.0.0 specifically rewrote them to tell users which package to install. Pre-fix, the message was being chopped to Cannot find an authentication provider for 'ActiveDirectory...', which is exactly what burned the reporter.

The companion XAML change (horizontal → vertical, TextWrapping="Wrap", button HorizontalAlignment="Left") is the right shape for now-untruncated multi-line errors.

One tradeoff worth naming, not a blocker: this re-introduces Azure.Identity / Azure.Core / MSAL transitively for every user of PlanViewer.Core, even ones who only connect to on-prem SQL with Windows auth — exactly the footprint SqlClient 7.0.0 was designed to make optional. The alternative (an opt-in PlanViewer.Core.Azure package) is over-engineering for this tool. Accept the cost.

Two non-blocking nits:

  • Spacing dropped 6 → 8 in the test-status row would better match the rhythm of the other rows in the dialog (Margin="0,0,0,12", Spacing="16").
  • "Core Library (PlanViewer.Core)" component checkbox in the PR description should also be ticked since PlanViewer.Core.csproj is where the dep landed. Cosmetic.

Neither is worth holding the PR for.

@erikdarlingdata erikdarlingdata merged commit 7dc2122 into erikdarlingdata:dev Apr 29, 2026
2 checks passed
ClaudioESSilva pushed a commit to ClaudioESSilva/PerformanceStudio that referenced this pull request Apr 29, 2026
Match the visual rhythm of the surrounding rows in the dialog
(Margin 0,0,0,12 / inter-control Spacing 16). Follow-up nit from
review of erikdarlingdata#298.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 4, 2026
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.

2 participants