-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add delegated Kerberos support for JDBC driver #9606
Add delegated Kerberos support for JDBC driver #9606
Conversation
client/trino-client/src/main/java/io/trino/client/auth/kerberos/SpnegoHandler.java
Outdated
Show resolved
Hide resolved
client/trino-client/src/main/java/io/trino/client/auth/kerberos/SpnegoHandler.java
Outdated
Show resolved
Hide resolved
...nt/trino-client/src/main/java/io/trino/client/auth/kerberos/ContextBasedSubjectProvider.java
Outdated
Show resolved
Hide resolved
...nt/trino-client/src/main/java/io/trino/client/auth/kerberos/ContextBasedSubjectProvider.java
Outdated
Show resolved
Hide resolved
...o-product-tests/conf/environment/multinode-tls-kerberos-delegation/tempto-configuration.yaml
Show resolved
Hide resolved
d34255f
to
d03053f
Compare
d03053f
to
7f4f689
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % one question
...nt/trino-client/src/main/java/io/trino/client/auth/kerberos/ContextBasedSubjectProvider.java
Outdated
Show resolved
Hide resolved
7f4f689
to
8d05ecf
Compare
@kokosing , @aczajkowski AC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM %
One NIT idea to check. In ContextBasedSubjectProvider If ticket is not renewable i would check it's validity (https://docs.oracle.com/javase/7/docs/api/javax/security/auth/kerberos/KerberosTicket.html#getEndTime()) and if not valid throw Ex. But not sure if this is a problem or not. Just thought about it.
Thanks @Praveen2112 . This PR seems to have copied the core code from #5377. |
We have enabled this feature via a connection property, some minor code level cleanup and added a ProductTests for it. |
@electrum @Praveen2112 Hi, I would really appreciate if the author is notified about the urgency of a change before copying over the code/logic and raising a separate PR. I raised the PR last year and spent a significant amount of effort in identifying the issue, fixing and testing it and followed up numerous times initially to get this merged and I might have missed an email off late but was never aware that this was a priority and now my PR has been closed by copying over the logic/code to a new PR :-) |
@ankuragarawal My sincerest apologies for the above miscommunication and issue due to that. I would like to thank you for your valuable contribution without which the feature might not be included in Trino. |
This allows us to reuse the Kerberos context is setup outside trino-jdbc-driver. It is currently enabled by the setting property
KerberosDelegation=true
.Supersedes #5377.