Skip to content

Add adal4j to support active directory authentication #230

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

Merged
merged 3 commits into from
May 31, 2022

Conversation

legiangthanh
Copy link
Member

@legiangthanh legiangthanh commented May 31, 2022

mssql-jdbc will dynamically load the ada4j library when authentication is ActiveDirectoryPassword. But currently, the gem's classpath doesn't contain the ada4j library, mssql-jdbc will throw an exception.

Caused by: com.microsoft.sqlserver.jdbc.SQLServerException: Failed to load ADAL4J Java library for performing ActiveDirectoryPassword authentication.
        at com.microsoft.sqlserver.jdbc.SQLServerConnection.validateAdalLibrary(SQLServerConnection.java:4233)

This PR will add the add4j library to the dependency of sqlserver to include this library in the gem's classpath.

@legiangthanh legiangthanh requested a review from dmikurube May 31, 2022 04:10
Copy link
Member

@dmikurube dmikurube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It basically looks good, but can you leave an explicit technical background description in the PR description how "Just adding the adal4j library makes it support the ACtive Directory-based authentication"?

@legiangthanh legiangthanh requested a review from dmikurube May 31, 2022 07:15
Copy link
Member

@dmikurube dmikurube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Okay, the description looks reasonable.

@dmikurube dmikurube requested a review from hito4t May 31, 2022 07:19
@dmikurube
Copy link
Member

@hito4t We're going to release this (for SQLServer) as 0.12.4, jfyi.

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