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

ActiveDirectoryAuthenticationProvider constructors implemented incorrectly #1327

Closed
smichtch opened this issue Oct 8, 2021 · 1 comment · Fixed by #1328
Closed

ActiveDirectoryAuthenticationProvider constructors implemented incorrectly #1327

smichtch opened this issue Oct 8, 2021 · 1 comment · Fixed by #1328

Comments

@smichtch
Copy link
Contributor

smichtch commented Oct 8, 2021

Describe the bug

public ActiveDirectoryAuthenticationProvider() and public ActiveDirectoryAuthenticationProvider(string applicationClientId) are not delegating initialization to public ActiveDirectoryAuthenticationProvider(Func<DeviceCodeResult, Task> deviceCodeFlowCallbackMethod, string applicationClientId).

Instead of this:

public ActiveDirectoryAuthenticationProvider() => new ActiveDirectoryAuthenticationProvider(DefaultDeviceFlowCallback);
public ActiveDirectoryAuthenticationProvider(string applicationClientId) => new ActiveDirectoryAuthenticationProvider(DefaultDeviceFlowCallback, applicationClientId);

it should be:

public ActiveDirectoryAuthenticationProvider() : this(DefaultDeviceFlowCallback) {}
public ActiveDirectoryAuthenticationProvider(string applicationClientId) : this(DefaultDeviceFlowCallback, applicationClientId) {}

To reproduce

var provider = new ActiveDirectoryAuthenticationProvider("<my app ID>");
SqlAuthenticationProvider.SetProvider(SqlAuthenticationMethod.ActiveDirectoryInteractive, provider);

provider still uses the default client app ID 2fd908ad-0664-4344-b9be-cd3e8b574c38 instead of <my app ID>.

Expected behavior

New ActiveDirectoryAuthenticationProvider instances should use the client app ID specified in the constructor argument.

Further technical details

Microsoft.Data.SqlClient version: 3.0.1
.NET target: net472, netcoreapp3.1, net5.0
SQL Server version: n/a
Operating system: Windows 10 Enterprise Build 19043

Additional context
None.

@JRahnama
Copy link
Contributor

JRahnama commented Oct 8, 2021

@smichtch thanks for bringing this up. As an open source we always welcome contribution from community. Can you make a pool request to address the possible issue that we can review that quickly?

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 a pull request may close this issue.

2 participants