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

Postgres jdbc kerberos plugin #417

Closed
wants to merge 10 commits into from

Conversation

devanshsoni9
Copy link
Collaborator

This PR is right now intended for comments/feedback on the sample plugin. I won't merge until before the deadline for 2020.2 or advised otherwise. I discussed with Ross and his idea was to include this plugin in 2020.2 only if the Postgres JDBC 42.2.10 driver is released before that. This is because SSPI won't work until the fix for issue #1482 is released in the new driver jar.

@lukewrites lukewrites changed the base branch from dev to dev-2020.2 February 13, 2020 23:59
@lukewrites
Copy link
Member

@devanshsoni9 I changed the base of this PR to the 2020-2 branch. if you're OK with it going in there you can mark it Ready for review.

@@ -0,0 +1,38 @@
<?xml version='1.0' encoding='utf-8' ?>

<connector-plugin class='postgres_jdbc_kerberos' superclass='jdbc' plugin-version='0.0.0' name='Postgres JDBC with Kerberos' version='18.1' min-version-tableau='2019.4'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to change the min version to 2020.2 if its only going to come in after that release ?

<option name="UseWindows" value="auth-integrated" />
<option name="UsernameAndPassword" value="username-password" />
</authentication-options>
<db-name-prompt value="Database: " />
Copy link
Contributor

Choose a reason for hiding this comment

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

indendation

<connection-config>
<authentication-mode value='ComboBoxIntegrated'/>
<authentication-options>
<option name="UseWindows" value="auth-integrated" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rosswbrown should we be using auth-kerberos in instead of auth-integrated ?
Also, should we be documenting kerberos in the supported auth modes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sidwray same question to you about documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I can tell in the code auth-kerberos happens to be some teradata specific implementation detail and doesn't have broad platform meaning. I think this is correct, but I defer to you as person authoring/testing this. We should probably consider changing this sample over to using modular connection dialog definition, since this is a forward looking sample. This notion of which auth to use simplifies greatly with that style.

I don't believe we document anything until we're planning to ship this feature, which I thought was post 2020.2.

@devanshsoni9
Copy link
Collaborator Author

will not ship in 2020.2 in favor of using modular connection dialog

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

Successfully merging this pull request may close these issues.

None yet

4 participants