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

active directory authentication and temp table error #278

Open
ginberg opened this issue Jul 16, 2024 · 9 comments
Open

active directory authentication and temp table error #278

ginberg opened this issue Jul 16, 2024 · 9 comments

Comments

@ginberg
Copy link

ginberg commented Jul 16, 2024

For Darwin we have a data partner that is using active directory authentication to setup their connection to sql server and they are getting an error when creating a temp table. In the error their UID is mentioned as schema name..
Is this connection setup supported by DatabaseConnector?

library(DatabaseConnector)
connectionDetails <- DatabaseConnector::createDbiConnectionDetails(dbms = "sql server",
                                                                   drv = odbc::odbc(),
                                                                   Driver = "ODBC Driver 18 for SQL Server",
                                                                   Server = Sys.getenv("SQL_SERVER"),
                                                                   Database = Sys.getenv("SQL_DB"),
                                                                   Authentication = "ActiveDirectoryPassword",
                                                                   UID = "EXT19616221@hustietoallas.fi",
                                                                   PWD = rstudioapi::askForPassword("OMOP Password"),
                                                                   pathToDriver = Sys.getenv("DRIVER_PATH"))
con <- connect(connectionDetails)
DatabaseConnector::insertTable(
  connection = con,
  tableName = "#mtcars",
  data = mtcars,
  dropTableIfExists = TRUE,
  createTable = TRUE,
  progressBar = FALSE,
  tempTable = TRUE,
  tempEmulationSchema = NULL,
  camelCaseToSnakeCase = TRUE
)
error
@schuemie
Copy link
Member

I highly recommend not using the createDbiConnectionDetails() function, since it allows one to basically use any DBI driver, but we can only test against those invoked by createConnectionDetails() (Perhaps I should make that function private). It is hard enough to support 10 different database platforms, please let us use only 1 driver per platform! Different drivers can have vastly different behaviors, even for the same plaforms (as demonstrated here).

The default driver for SQL Server is the official Microsoft JDBC driver. As far as I can tell this should support Active Directory authentication as described here. By either providing a JDBC connection string with appropriate parameters, or by specifying the extraSettings argument in createConnectionDetails() you should be able to get it working. (Note that the extraSettings are simply appended to the connection string as you can see here). So probably something like

connectionDetails <- createConnectionDetails(
    dbms = "sql server",
    server = Sys.getenv("SQL_SERVER"),
    user = "EXT19616221@hustietoallas.fi",
    password = Sys.getenv("SQL_PASSWORD"),
    extraSettings = "authentication=ActiveDirectoryPassword"

would work, although I cannot test it myself.

Please also don't use rstudioapi::askForPassword() when calling createConnectionDetails(). For security reasons, the evaluation of the password argument is delayed to the time of connection, which may be in a thread. Far better to use a credential manager such as keyring.

@salmarachidi
Copy link

image When I write down my password I get this error.

@schuemie
Copy link
Member

@salmarachidi : this error message seems unrelated to how you specify the password. Perhaps your IT department can help you figure out the correct connection details when connecting using JDBC? (Such as the correct port)

@eric-fey-hus
Copy link

@schuemie We fixed the connection details, and it is showing "Connecting using SQL Server driver". But then error: "Failed to load MSAL4J Java library". When I try to fix this with jar files and jaddClassPath, I get other missing java libraries. Is there not a jar file that contains all the dependencies we need?

@ginberg
Copy link
Author

ginberg commented Jul 16, 2024

@eric-fey-hus did you download the driver using the downloadJdbcDrivers method? See this article

@eric-fey-hus
Copy link

Yes. I tried that earlier without success. Can try again tomorrow. But I would need a java 11 version. The download is java 8, at least the one I get.

@schuemie
Copy link
Member

To my knowledge all JAR files should work with Java 8, but I certainly could have missed something.

I suspect MSAL4J is a library needed especially to enable the Active Directory authentication (See https://learn.microsoft.com/en-us/entra/msal/java/) . Adding this jar to the same place where the driver JAR files are should solve this problem. For the JDBC driver version that is installed by downloadJdbcDrivers() (version 9.2) it seems that should be enough: https://learn.microsoft.com/en-us/sql/connect/jdbc/connecting-using-azure-active-directory-authentication?view=sql-server-ver16#client-setup-requirements

@eric-fey-hus
Copy link

@schuemie @ginberg Trying to get the java dependencies to work became to complicated. Instead, I fixed the source code of DatabaseConnector. In function insertTable.DatabaseConnectorDbiConnection there was a line that removed the "#" from the the. This is what caused the issue, so I commented it out. Another fix, dbms(connection) should be dbms(connection@dbiConnection), and it all worked out.

I could create a branch on the DatabaseConnector repo to show you exactly what I did?

@schuemie
Copy link
Member

Hi @eric-fey-hus. Glad you got it working! I would welcome a pull request. Would you mind forking the repo and create the PR from there?

eric-fey-hus added a commit to eric-fey-hus/ef_DatabaseConnector that referenced this issue Jul 18, 2024
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

No branches or pull requests

4 participants