-
Notifications
You must be signed in to change notification settings - Fork 3.8k
CEP-50/CASS-20834: Enable IAuthenticator to declare supported and alterable role options #4427
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1294,7 +1294,7 @@ public static GuardrailsOptions getGuardrailsConfig() | |
| return guardrails; | ||
| } | ||
|
|
||
| private static void applyGuardrails() | ||
| public static void applyGuardrails() | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not thrilled with this, but I didn't see a cleaner way of initializing or defaulting guardrails config to enable CassandraRoleManager to construct in a unit test environment, without fully initializing DatabaseDescriptor. (If guardrails aren't explicitly initialized, CassandraRoleManager NPEs in the constructor.) Open to other suggestions.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jcshepherd what about calling DatabaseDescriptor.daemonInitialization() ? If you grep its usage, it is widely used in tests for these reasons. You can also mock static methods like this, similar technique for DD you would be doing here |
||
| { | ||
| try | ||
| { | ||
|
|
@@ -1959,16 +1959,52 @@ public static void setCryptoProvider(AbstractCryptoProvider cryptoProvider) | |
| DatabaseDescriptor.cryptoProvider = cryptoProvider; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the authenticator configured for this node. | ||
| */ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future this will become getDefaultAuthenticator(). Additional methods will be added to enable/disable negotiation and provide access to the node's supported authenticators. |
||
| public static IAuthenticator getAuthenticator() | ||
| { | ||
| return authenticator; | ||
| } | ||
|
|
||
| /** | ||
| * Returns an authenticator configured for this node, if it is of the requested type. | ||
| * @param clazz The class of the requested authenticator: e.g. PasswordAuthenticator.class. | ||
| * @return An Optional of the configured authenticator, if it is of the requested type; otherwise | ||
| * returns an empty Optional. | ||
| */ | ||
| public static <T extends IAuthenticator> Optional<T> getAuthenticator(Class<T> clazz) | ||
| { | ||
| return hasAuthenticator(clazz) ? Optional.of(clazz.cast(authenticator)) : Optional.empty(); | ||
| } | ||
|
|
||
| /** | ||
| * Sets the authenticator used by this node to authenticate clients. | ||
| */ | ||
| public static void setAuthenticator(IAuthenticator authenticator) | ||
| { | ||
| DatabaseDescriptor.authenticator = authenticator; | ||
| } | ||
|
|
||
| /** | ||
| * Indicates if this node uses an authenticator that requires authentication. | ||
| */ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future, the semantics will change from "'The' authenticator requires authentication" to "Any supported authenticator requires authentication." Existing callers to this method should be unaffected by that change. |
||
| public static boolean isAuthenticationRequired() | ||
| { | ||
| return DatabaseDescriptor.getAuthenticator().requireAuthentication(); | ||
| } | ||
|
|
||
| /** | ||
| * Indicates if this node is configured with an authenticator of the specified type. | ||
| * @param clazz The class of the authenticator. | ||
| * @return True if this node has an authenticator of the specified type, false otherwise. | ||
| */ | ||
| private static boolean hasAuthenticator(Class<? extends IAuthenticator> clazz) | ||
| { | ||
| return clazz.isAssignableFrom(authenticator.getClass()); | ||
| } | ||
|
|
||
|
|
||
| public static IAuthorizer getAuthorizer() | ||
| { | ||
| return authorizer; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,6 @@ | |
|
|
||
| import java.util.Optional; | ||
|
|
||
| import org.apache.cassandra.auth.IAuthenticator; | ||
| import org.apache.cassandra.auth.PasswordAuthenticator; | ||
| import org.apache.cassandra.config.DatabaseDescriptor; | ||
| import org.apache.cassandra.db.marshal.UTF8Type; | ||
|
|
@@ -31,7 +30,7 @@ final class CredentialsCacheKeysTable extends AbstractMutableVirtualTable | |
| private static final String ROLE = "role"; | ||
|
|
||
| @SuppressWarnings("OptionalUsedAsFieldOrParameterType") | ||
| private final Optional<PasswordAuthenticator> passwordAuthenticatorOptional; | ||
| private final Optional<PasswordAuthenticator> maybePasswordAuthenticator; | ||
|
|
||
| CredentialsCacheKeysTable(String keyspace) | ||
| { | ||
|
|
@@ -42,18 +41,14 @@ final class CredentialsCacheKeysTable extends AbstractMutableVirtualTable | |
| .addPartitionKeyColumn(ROLE, UTF8Type.instance) | ||
| .build()); | ||
|
|
||
| IAuthenticator authenticator = DatabaseDescriptor.getAuthenticator(); | ||
| if (authenticator instanceof PasswordAuthenticator) | ||
| this.passwordAuthenticatorOptional = Optional.of((PasswordAuthenticator) authenticator); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think now we have the opportunity to change name of this variable to We also do not need |
||
| else | ||
| this.passwordAuthenticatorOptional = Optional.empty(); | ||
| maybePasswordAuthenticator = DatabaseDescriptor.getAuthenticator(PasswordAuthenticator.class); | ||
| } | ||
|
|
||
| public DataSet data() | ||
| { | ||
| SimpleDataSet result = new SimpleDataSet(metadata()); | ||
|
|
||
| passwordAuthenticatorOptional | ||
| maybePasswordAuthenticator | ||
| .ifPresent(passwordAuthenticator -> passwordAuthenticator.getCredentialsCache().getAll() | ||
| .forEach((roleName, ignored) -> result.row(roleName))); | ||
|
|
||
|
|
@@ -65,14 +60,14 @@ protected void applyPartitionDeletion(ColumnValues partitionKey) | |
| { | ||
| String roleName = partitionKey.value(0); | ||
|
|
||
| passwordAuthenticatorOptional | ||
| maybePasswordAuthenticator | ||
| .ifPresent(passwordAuthenticator -> passwordAuthenticator.getCredentialsCache().invalidate(roleName)); | ||
| } | ||
|
|
||
| @Override | ||
| public void truncate() | ||
| { | ||
| passwordAuthenticatorOptional | ||
| maybePasswordAuthenticator | ||
| .ifPresent(passwordAuthenticator -> passwordAuthenticator.getCredentialsCache().invalidate()); | ||
| } | ||
| } | ||
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.
Specifically, see AuthConfigTest in this same PR.