OHDSI / DatabaseConnector

An R package for connecting to databases using JDBC.
http://ohdsi.github.io/DatabaseConnector/
54 stars 81 forks source link

active directory authentication and temp table error #278

Open ginberg opened 4 months ago

ginberg commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago
image

When I write down my password I get this error.

schuemie commented 4 months ago

@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 commented 4 months ago

@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 commented 4 months ago

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

eric-fey-hus commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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?