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

Fix -E parameter conflict with AAD authentication #187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alelapeyre
Copy link

When no username is passed to the sqlserver connection URL, the adapter adds the -E parameter to the sqlcmd command. However, when using ActiveDirectoryDefault in the --authentication-method parameter, it doesn't work because the go-sqlcmd program will choose integrated authentication instead of AAD authentication.

Integrated authentication is chosen in go-sqlcmd either because the -E parameter is provided or because the username is empty and the authentication method is not specified.

This commit prevents the -E parameter from being passed if the authentication method parameter is provided, ensuring that AAD authentication can be used properly.

When no username is passed to the sqlserver connection URL, the adapter
adds the -E parameter to the sqlcmd command. However, when using
ActiveDirectoryDefault in the --authentication-method parameter, it
doesn't work because the go-sqlcmd program will choose integrated
authentication instead of AAD authentication.

Integrated authentication is chosen in go-sqlcmd either because the -E
parameter is provided or because the username is empty and the
authentication method is not specified.

This commit prevents the -E parameter from being passed if the
authentication method parameter is provided, ensuring that AAD
authentication can be used properly.
@Dynge
Copy link

Dynge commented Mar 31, 2025

Can confirm this works and is required to work with AAD authentication

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