[Kerberos] Refactoring and remove configs with defaults #32152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit does some refactoring to remove support package and move class KerberosTicketValidator to kerberos package. That was the only class in that package, so no need for it to be in separate package. Changes done to use default values for jaas configuration options for the ones which we can use defaults. Fix couple of random failures in tests. Modified `refreshKrb5Config` to use default value `false` in KerberosTicketValidator. If the krb5.conf file is modified then we will need to restart JVM as the config will not be refreshed. For testing, `refreshKrb5Config` is set to `true` as we keep changing the kdc port. This is set in SpnegoClient and only for tests.
bizybot
added
>feature
:Security/Authentication
|
Pinging @elastic/es-security |
jaymode
reviewed
Jul 18, 2018
| @@ -41,7 +41,7 @@ | |||
| * It may respond with token which needs to be communicated with the peer. | |||
| */ | |||
| public class KerberosTicketValidator { | |||
| static final Oid SPNEGO_OID = getSpnegoOid(); | |||
| public static final Oid SPNEGO_OID = getSpnegoOid(); | |||
There was a problem hiding this comment.
why did this become public? Did you remove the support package in the test directory structure
There was a problem hiding this comment.
Yes, I did not move the test support package classes, now I have moved them I do not need to make it public. Thank you.
| @@ -49,7 +50,7 @@ | |||
| * Use {@link #close()} to release and dispose {@link LoginContext} and | |||
| * {@link GSSContext} after usage. | |||
| */ | |||
| class SpnegoClient implements AutoCloseable { | |||
| public class SpnegoClient implements AutoCloseable { | |||
There was a problem hiding this comment.
you can revert these changes if you remove the support package here too
There was a problem hiding this comment.
Yes, I did not move the test support package classes, now I have moved them so I do not need to make it public. Thank you.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit does some refactoring to remove support package
and move class KerberosTicketValidator to kerberos package.
That was the only class in that package, so no need for it to be in
separate package.
Changes done to use default values for jaas configuration options
to avoid unnecessary configuration.
Fix couple of random failures in tests.
Modified
refreshKrb5Configto use default valuefalseinKerberosTicketValidator. If the krb5.conf file is modified then we
will need to restart JVM as the config will not be refreshed. This
will need to be documented.
For testing,
refreshKrb5Configis set totrueas we keepchanging the kdc port. This is set in SpnegoClient and only for tests.