Skip to content

add Adal4j lib to support option authentication ActiveDirectoryPassword #352

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luongnhattruong
Copy link

@luongnhattruong luongnhattruong commented Feb 11, 2025

Context: When i export data to MSSQL with driver options {password=***, user=xxx, authentication=ActiveDirectoryPassword} i got exception

com.microsoft.sqlserver.jdbc.SQLServerException: Failed to load ADAL4J Java library for performing ActiveDirectoryPassword authentication.

Purpose: Support jdbc option authentication=ActiveDirectoryPassword
Ref:
https://techcommunity.microsoft.com/blog/azuredbsupport/lesson-learned-366-cannot-open-server-microsoft-com-using-authentication--active/3842210
https://learn.microsoft.com/en-us/sql/connect/jdbc/feature-dependencies-of-microsoft-jdbc-driver-for-sql-server?view=sql-server-ver16

@luongnhattruong luongnhattruong requested a review from a team as a code owner February 11, 2025 12:21
@dmikurube
Copy link
Member

@luongnhattruong It looks to be the same purpose with embulk/embulk-input-jdbc#230, then it basically looks good to me. However, can you :

  1. Leave more background in the pull-req description?
  2. Add a comment in build.gradle so that this adal4j is required for this purpose?
  3. Explain why did you choose the verison 1.6.3?

@luongnhattruong
Copy link
Author

luongnhattruong commented Feb 12, 2025

@dmikurube ,
I have updated context for this PR in description for 1
I choose version 1.6.3 due to https://learn.microsoft.com/en-us/sql/connect/jdbc/feature-dependencies-of-microsoft-jdbc-driver-for-sql-server?view=sql-server-ver16

JDBC Driver version 7.2.2—Dependency versions: Adal4j (version 1.6.3), Client-Runtime-for-AutoRest (1.6.5), and their dependencies

We are using :

implementation 'com.microsoft.sqlserver:mssql-jdbc:7.2.2.jre8'

@dmikurube
Copy link
Member

Okay, however, my two cents on that we will forget that adal4j is there to couple with mssql-jdbc (for that purpose of ActiveDirectoryPassword) when we have to upgrade mssql-jdbc in the future.

To avoid an expected future mistake, please also add :

  • more explanation that adal4j is there to couple with the proper version of mssql-java for Azure Active Directory Authentication
  • more explanation that adal4j just needs to be there in the classpath without any reference from the plugin code, and it should not be removed even if it still compiles even if adal4j is removed
    • when we consider removing "unnecessary libraries" in the future, we may consider it can be removed because removing it won't make the compilation fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants