tpope / vim-dadbod

dadbod.vim: Modern database interface for Vim
https://www.vim.org/scripts/script.php?script_id=5665
3.46k stars 122 forks source link

mssql support for azure ad auth (`sqlcmd -G`) #152

Open michaelstack opened 6 months ago

michaelstack commented 6 months ago

I am connecting to an azure sql db instance that uses azure ad auth.

I see the following line in the *dadbod-sqlserver* section in the vim-dadbod docs:

To set the integratedSecurity connection property and use a trusted connection, omit the user and password.

This sounds like it means that if I omit the username and password in a connection I will get the azure ad auth.

▸  Error connecting to db cg4nouser: DB exec error: mssql: login error: Login fai
▸  led for user ''.^@mssql: login error: Login failed for user ''.

I had a look in vim-dadbod/autoload/db/adapter/sqlserver.vim and tried to add a -G in to likely-looking places but I am not familiar enough with vimscript to get it working.

Are there any plans to add support for the new -G option?

The microsoft docs for azure ad auth and the -G option are at:

https://learn.microsoft.com/en-gb/sql/tools/sqlcmd/sqlcmd-authentication?view=sql-server-ver16&tabs=odbc https://learn.microsoft.com/en-gb/sql/tools/sqlcmd/sqlcmd-authentication?view=sql-server-ver16&tabs=go

tpope commented 6 months ago

Generally speaking, to add support for a new command-line argument, the general procedure is to find the corresponding connection property, and turn it into a query parameter. For example, the property corresponding to the command-line argument -C is trustServerCertificate, so to pass -C, you append ?trustServerCertificate=true to the URL. Another example is that -E corresponds to the integratedSecurity property. (But we don't actually require that one; we use the lack of username and password as you found in the documentation.)

So what's needed to support -G is the name of an equivalent connection property. Unfortunately, the docs can be really obtuse about this. The docs for -C do mention "the ADO.NET option TRUSTSERVERCERTIFICATE = true" (what's with the uppercase?), but the docs for -E don't mention integratedSecurity at all.

So the question is, what's the corresponding connection property for -G? The docs for -G make no mention of a connection property either. They do mention the "scripting variable SQLCMDUSEAAD = true", but that's something different.

marhaasa commented 4 months ago

I am experiencing issues connecting to a SQL server in Azure with the "-G" flag with vim-dadbod as well. I've tried altering the sqlserver.vim adapter to include the "-G" and this seems to be working(I get the browser pop-up like in sqlcmd and I am able to successfully authenticate), but I only see a very limited set of schemas(dbo and sys) and cannot seem to access other objects that I successfully can access using sqlcmd directly. Any ideas where to start troubleshooting?

danarth commented 4 months ago

@marhaasa are you specifying a target database in your connection string?

I have also been playing around with the sqlserver.vim adapter to include the -G flag - I can access all of my objects. The issue I have is that I have the browser pop-up appearing every time a query is made. I need to get a bit more familiar with the sqlcmd tool to see if there is anything you can do to prevent that from appearing every time

marhaasa commented 4 months ago

Thank you for reaching out @danarth and prompting me to further investigate my issue.

I have now been fumbling around with this problem of mine a bit more and found a solution. I am a complete newbie on vim/neovim and vim plugins, so I am sure the problem was quite easy to adress to begin with.

The problem I was facing with the adapter and specifying a database was due to how I altered the db#adapter#sqlserver#interactive function. I constructed the sqlcmd incorrectly with: -d mydatabase instead of separately as: -d mydatabase. I also tried with and without the -I flag, and this is how I ended up constructing my scqlcmd and it works as I expect it to: let cmd = ['sqlcmd', '-S', 'databaseURL', '-G', '-d', 'mydatabase'].

When using the Azure CLI to run az login in my terminal session before firing up neovim and vim-dadbod I am able to log onto my database and query it without thousands of pop-ups.

danarth commented 4 months ago

No problem @marhaasa! Glad to hear you have it working now. I was able to follow the same steps with az login etc and have mine working now too, so thank you for the help!

Looking at the go-sqlcmd code, it is using the connection string param fedauth to directly specify the token connection type, e.g. ActiveDirectoryDefault or ActiveDirectoryInteractive.

A quick play around with sqlcmd shows that I can connect to my Azure SQL database by explicitly specifying --authentication-method ActiveDirectoryDefault and omitting the -G flag. It looks like the -G flag is just a convenience to try and work out the best authentication method to use with the underlying driver.

@tpope so a potential solution to this is to avoid the -G flag altogether and add support for the fedauth parameter of the connection string? Let me know what you think and I'd be happy to have a crack at working on this.

Docs for the fedauth connection string parameter are here

tpope commented 3 months ago

@danarth I think you're onto something. But fedauth in particular looks like it might be specific to go-mssqldb? It also uses user id while we use user or userName.

If we look at the list of documented connection properties (which is where user and userName can be found), I see an authentication property that appears to map to what we want. So the change I would propose is to accept a authentication query parameter, and map that onto a --authentication-method command line argument. Make sense?

danarth commented 3 months ago

@tpope that's a really good point about compatibility with the old CLI/driver. However, even though authentication is a supported connection parameter, the old CLI doesn't support an --authentication-method parameter. Instead, it generates a connection string based on the presence of -U and -G flags (explained here)

I think it's going to be tricky to provide a solution for this adapter that is compatible with both the ODBC version and go version of the sqlcmd utility - so do we proceed and use the authentication parameter and caveat that this can only be used in conjunction with the go-sqlcmd CLI in the documentation?

Thanks for the help by the way!

tpope commented 3 months ago

I think it's going to be tricky to provide a solution for this adapter that is compatible with both the ODBC version and go version of the sqlcmd utility

Tricky at best, maybe impossible. I'm not going to hold out for the impossible.

so do we proceed and use the authentication parameter and caveat that this can only be used in conjunction with the go-sqlcmd CLI in the documentation?

Yeah, that's fine, and I wouldn't spill too many words on it. Perhaps just encourage the use of go-sqlcmd (which I take is intended to become the canonical implementation?) for full functionality.