Closed shueybubbles closed 2 years ago
I think we are at "I am perennially late with reviews" and "someone should probably take over https://github.com/denisenkom/go-mssqldb/pull/547 and finish".
Azure SDKs for go have evolved a lot since #547 started.
They told me we should use github.com/Azure/azure-sdk-for-go/sdk/azidentity
now, which supports a broad range of authentication options with little extra code needed in the driver.
I'll comment in that PR
@kardianos @wrosenuance what do you envision as the interface for apps to use AAD auth with this driver? Is it purely through code via calling the connector and providing a token callback, or can we let the app pass an AAD authentication type in the connection DSN and have the driver use azidentity
directly to provide the implementation ? Or would we create a new package in the repo that has an azidentity
based implementation of the callbacks that an app could use and pass to the connector factory?
I suspect apps would prefer a codeless model if possible. They could use environment variables to provide tenant/client id/client secret for service principal auth, user name and password for AAD password auth, etc. go-mssqldb can provide the default implementation similarly to the way Microsoft.Data.SqlClient
implements the AAD auth types for C# apps.
@shueybubbles If I understand you correctly, you are asking: "Will it be possible to specify azidentity connection just in a DSN (text) string along with File System artifacts that the executable can access, or must you actually provide that in code?
I'm open to the DSN parser reaching out to the file system as required to get the needed artifacts, but I would object to the main code base doing so.
I will try to put together a PR to show what I mean.
For example - I would like an app simply to be able to add AzureActiveDirectoryAuthentication=Password
to the DSN and the parser would use the fedauth connector and use a callback that provides the user name and password with an azidentity NewUsernamePasswordCredential
instance.
Similarly, if AzureActiveDirectoryAuthentication=ManagedIdentity
is provided, the callback would use a NewManagedIdentityCredential
and use the user name from the DSN for a user-assigned managed identity. If the user name is empty it would use system-assigned managed identity.
The app could also use Default
and rely on environment variables to set client secrets etc.
That seems reasonable.
On Wed, Sep 29, 2021 at 10:31 AM David Shiflet @.***> wrote:
I will try to put together a PR to show what I mean. For example - I would like an app simply to be able to add AzureActiveDirectoryAuthentication=Password to the DSN and the parser would use the fedauth connector and use a callback that provides the user name and password with an azidentity NewUsernamePasswordCredential instance. Similarly, if AzureActiveDirectoryAuthentication=ManagedIdentity is provided, the callback would use a NewManagedIdentityCredential and use the user name from the DSN for a user-assigned managed identity. If the user name is empty it would use system-assigned managed identity. The app could also use Default and rely on environment variables to set client secrets etc.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/denisenkom/go-mssqldb/issues/696#issuecomment-930288477, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFYLMLZVBH6BZQG7W7MAKLUEMWMJANCNFSM5EFQ2GKA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
@wrosenuance @kardianos What work is remaining before you'll export the functions in fedauth.go, particularly newActiveDirectoryTokenConnector?
I'd like to add azidentity support to my application (and eventually directly into go-mssqldb itself). Without the fedauthinfo data from the server, though, I'll have to provide some way for the app user to provide those values if they are not using defaults.