-
Notifications
You must be signed in to change notification settings - Fork 506
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
Azure Active Directory Authentication is not supported? #446
Comments
Can you upload logs as specified in https://github.com/denisenkom/go-mssqldb/wiki#reporting-bugs |
Sure. I'm using /examples/simple/simple.go and here's the result I got with logging enabled: password:secret
|
Would be totally great to get AAD support - @denisenkom are you planning on it? Probably need this? - https://docs.microsoft.com/en-us/azure/go/azure-sdk-go-authorization |
Hi |
I'm working on adding access token auth (for AAD SPN's) here. This is what I see mostly used in applications on Azure. Would that help you as well, @zbynekbotlo, @denzilribeiro and @jason-johnson ? This would allow you to do something like the following. Note that here I'm using a client ID and a secret, but I could be using a certificate (instead of a secret) to get the access token. import (
"database/sql"
"fmt"
"os"
"github.com/Azure/go-autorest/autorest/adal"
"github.com/Azure/go-autorest/autorest/azure"
// blank import the driver for database/sql
_ "github.com/denisenkom/go-mssqldb"
)
const clientID = "guid"
const clientSecret = "guid"
func doSQLStuff() error {
oauthConfig, err := adal.NewOAuthConfig(azure.PublicCloud.ActiveDirectoryEndpoint, "AAD-tenant-id")
if err != nil {
return err
}
spt, err := adal.NewServicePrincipalToken(
*oauthConfig,
clientID,
clientSecret,
"https://database.windows.net/")
if err != nil {
return err
}
spt.EnsureFresh()
accessToken := spt.OAuthToken()
fmt.Println("Using accessToken:", accessToken)
db, err := sql.Open("mssql",
"Server=pmtestsql.database.windows.net;"+
"accessToken="+accessToken)
if err != nil {
return err
}
_, err = db.Exec("SELECT TOP (1000) * FROM [SalesLT].[Product]")
if err != nil {
return err
}
return nil
} |
Yes, this looks perfect. I was looking myself how the access tokens were done in TDS but looks like you're further along. |
Hi @paulmey I have set up a test for an Azure database I'm trying to connect to and set my github.com/denisenkom/go-mssqldb repo to have your repo as a remote and checked out your access token branch there so I could test it. Currently it still says "windows logins not supported in this version of SQL server". I guess that's expected at this point? Also, is there a way we could discuss on your fork? I tried to open an issue to discuss but there was no issues tab there. |
Hi @jason-johnson, I think discussing here is good, because it's easier to discover. |
@paulmey |
That error message is not in this code base and the SO link I posted has the exact same error coming through .NET. Did you set up AAD for your server and database? This is all very lightly documented and I haven't tested this yet, but this might be the error you get if you have not set a AAD admin for your server. That action might tie SQL and AAD together. |
@paulmey
I don't understand what you mean about the EDIT: I just verified here that |
It occurs to me that the issue here is probably the |
Ah, yes, the It might still be that the Azure AD integrated auth works slightly different and SQL may only work with access tokens for AD applications/SPNs. Easiest way is to try. |
I tried To re-iterate what I was saying before: I have an azure SQL database in our azure cloud. From my local company PC which is authenticated on the internal company AD I can use Microsoft SQL Server Management Studio to connect to the azure database using “Active Directory Integrated Authentication”. This connects with no password being entered. Now, given that the azure SQL Server talks via the TDS protocol, the only way this client can be authenticating is with a token or a certificate right? And this token or certificate must be retrievable from my client machine. So the question is, how do I get this token or certificate so I can pass it to your code? |
I'm not yet familiar with all the flavors of SQL auth and on-prem AD to AAD will need to somehow swap out your Kerberos ticket for an OAuth token. I'm not entirely sure how that works, but you should be able to do it with the code you have as well. Looking at the link you sent, I think your code should look like: token, err := cli.GetTokenFromCLI("https://database.windows.net/")
accessToken := token.AccessToken I have just tested this on my branch on Linux, but that should work. $ at=$(az account get-access-token --resource https://database.windows.net/ --q accessToken -o tsv)
$ export SQLSERVER_DSN="sqlserver://server.database.windows.net?database=testdb&accesstoken=$at"
$ go test -run TestSessionInitSQL
2020/01/16 07:50:59 initiating response reading
...
2020/01/16 07:50:59 response finished
PASS
2020/01/16 07:50:59
SET XACT_ABORT ON; -- 16384
SET ANSI_NULLS ON; -- 32
SET ARITHIGNORE ON; -- 128
ok github.com/denisenkom/go-mssqldb 0.397s |
I have also done work on this for my company that I was hoping to get upstreamed - I'm still waiting on internal approvals to create a PR though! I added support for AD logins with user accounts without 2FA, service principals and managed identities. Though on a re-read of the thread, I don't think it will address the ActiveDirectory Integrated scenario - we don't have that available to test with, so I didn't add it. I was mostly trying to support automated logins from containers. |
Interesting, how did you differentiate between normal username/password and AAD logins? |
I added the flags and TDS token stream types to support federated authentication and security token logins. Federated authentication is used for the AD sign-ins, like user/password and device codes, as well as the Integrated AD sign-in, AFAIK. |
Yes, I believe I have to get a kerberos token and use that. I'm investigating this now. |
@wrosenuance Are the managed identities and service principles able to login without password? I could live with it if that much worked. |
Yes - that was the point for me! They contact the IMDS that supplies system-assigned or user-assigned identities, either via the VM service or the Azure AD Pod Identity controller in Kubernetes. |
@wrosenuance Oh, I meant in the connection string. I saw in some places that they had some "Authentication=" label and I was considering adopting that. @jason-johnson I'm planning to make something like this for managed identity as well. You can then do something like conn, err := mssql.NewAccessTokenConnector(
"Server=test.database.windows.net;"+
"Database=testdb", tokenProvider) or in the case of managed identity conn, err := mssql.NewManagedIdentityConnector(
"Server=test.database.windows.net;"+
"Database=testdb", "managed-identity-identifier")
if err != nil {
return err
}
db := sql.OpenDB(conn) |
I see - I picked up the convention from the Java JDBC driver, they use Authentication=ActiveDirectoryPassword, or Authentication=ActiveDirectoryMSI. I used "fedauth" instead of "Authentication", so it became fedauth=ActiveDirectoryPassword, fedauth=ActiveDirectoryMSI, and added in the option fedauth=ActiveDirectoryApplication for the service principal security token logins with secret or certificate authentication. |
Ok, I haven't decided yet if I want all of that in the connection string, but if so, I'll align with what you have so that our PR's work well together. |
Thanks! I am so sorry to have to wait on sharing this! In the docs I had written the description of the connection string changes was:
|
@paulmey here is what I got:
And that is using your exact commands above, setting SQLSERVER_DSN etc |
@jason-johnson, I was able to repro this on Windows, looking into this. |
@jason-johnson pushed a fix to my branch, please retry. |
I realise I'm now late to the party 😞 but please take a look at PR #547 where I have the branch that is finally approved for release. |
Interestingly I think there's some overlap on the specific changes in the protocol handling, but the PRs are quite different. If I'm reading things correctly, @paulmey has kept the actual mechanism for obtaining the tokens outside of the driver and has a callback that driver users are expected to provide that does this work. This avoids new dependencies and I think makes it easier to support older Go versions, though it requires more sophistication from end users to implement than bundling the required libraries into the driver. I went with the approach of integrating the ADAL libraries - this means that the user experience is more like the existing - you can provide the information needed to authenticate in the DSN without any extra code. But the need to integrate more libraries means my PR is failing to build right now in the older Go environments. I can try to figure out a mitigation there, but I suspect I may run into issues coming up with a way to integrate the Microsoft libraries across pre-Go-module and post-Go-module code. |
@wrosenuance, I like the comprehensiveness of PR #547. It brings go-mssqldb in line with other client libraries. We should be able to merge both approaches later. Easiest way to make your implementation compile for various versions is to have version specific implementations using go build tags. Support the options for the go versions where the libraries are available or have small differences and just throw errors for previous versions. |
Thanks! I was able to get it compiling with a couple of small adjustments and the right go get calls. Next problem is the code coverage results look terrible 😅 because the CI environment doesn't support Azure SQL for testing. I think the approach you were taking with having a callback that could obtain a token is likely to be the best approach for supporting some use cases, such as the Azure AD integrated login that @jason-johnson is attempting. Looking at the JDBC driver, there is a call out to native code to get the token (sqljdbc_auth.dll) and it's not clear what steps are required to get the token it ultimately returns. |
@paulmey That seems to have fixed it! Using your statement above I'm getting "Login failed for user 'NT AUTHORITY\ANONYMOUS LOGIN'. But I wrote a dotnet core client which worked so I knew it had to be the token so I went back to my original approve which works correctly:
Regarding the discussion with @wrosenuance about API design, here is the dotnet core code I used:
where "servername" is my Azure SQL server. Personally, I think it would be good to consider the dotnet core API when deciding how to do the golang API because people using this library might be more likely to be coming from a dotnet background and expect the API to look similar. Just a thought though, you all are more familiar with golang than I am. |
@jason-johnson I was looking for a simple way to obtain an integrated authentication token, to see if I could figure out from source code what was happening, so thanks for providing one! Unfortunately it seems that simple code conceals a lot going on under the hood, making it hard to see how to easily port/replace all the necessary bits for a pure-Go implementation. In both the .NET and Java libraries that support AD Integrated authentication, there seems to be some magic happening where a connection to an endpoint serving WS-Trust responses is able to authorize the client. There's a page that suggests this magic happens through some hooking of the standard URL libraries, where a token is injected, and this is how the real authentication is happening. What's not clear is whether that's the only way to do this: there are some places that suggest it may be possible to pull out a token from somewhere, or obtain one outside a browser. The Microsoft docs also describe this, but doesn't have a lot of leads for re-implementation. All up I think it may be easier to stick with using some of the Microsoft-provided code for .NET or Java for this, like you have, and running it as an external helper. If combined with a callback-based approach as @paulmey proposed, you could call out to the helper when a token is needed. I found another page that described how Am I understanding you correctly when you say it is working that supplying the token you retrieve using the .NET code to the Go driver lets you authenticate to Azure SQL for the Go driver? |
Sorry for the confusion. I made a dotnet core program (i.e. platform independent that hopefully isn't using any proprietary calls to get the token) and verified that it worked. Then I wrote a pure go program using the branch from @paulmey. I also used the |
The CLI uses the refresh token for a user to get a new access token for the specified resource. |
In case of AD/AAD, AD federation services (adds) will swap your kerberos ticket for an oauth code to present to AAD for authentication. |
Tried to merge our changes, opened a draft PR to see if it passes CI and what the code coverage damage is (I feel like it should be @wrosenuance's PR, because that PR has way more lines...). |
Looks good to me, @paulmey! Does your branch include the second commit on my branch from yesterday that fixed things in AppVeyor? It may be blocking your PR too. |
Hi,
I am able to connect to Azure database using SQL authentication but when I use Azure AD credentials I receive TLS handshake error when pinging the database:
Cannot read handshake packet: read tcp: wsarecv: An existing connection was forcibly closed by the remote host.
Connection String:
sqlserver://user:[email protected]:1433?app+name=MyAppName&database=dbname&encrypt=true&hostNameInCertificate=%2A.database.windows.net&trustservercertificate=true
Is the Azure AD supported or not?
Thanks
The text was updated successfully, but these errors were encountered: