tolbertam commented on code in PR #4427:
URL: https://github.com/apache/cassandra/pull/4427#discussion_r3037027948
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -2037,16 +2010,52 @@ public static void
setCryptoProvider(AbstractCryptoProvider cryptoProvider)
DatabaseDescriptor.cryptoProvider = cryptoProvider;
}
+ /**
+ * Returns the authenticator configured for this node.
+ */
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)
Review Comment:
So in the context of CEP-50, we can expect this method to be used in such a
way that if the client says 'I want to authenticate with X', we'd consult this
method to retrieve that authenticator, and if its not present we would not
allow it? :+1:
##########
src/java/org/apache/cassandra/auth/IAuthenticator.java:
##########
@@ -66,6 +66,27 @@ default boolean supportsEarlyAuthentication()
*/
Set<? extends IResource> protectedResources();
+ /**
+ * Set of IRoleManager.Options used by this authenticator and supported by
CREATE ROLE and ALTER ROLE statements.
+ *
+ * @return A set of IRoleManager.Options that this authenticator requires
support for.
+ */
+ default Set<IRoleManager.Option> getSupportedRoleOptions()
+ {
+ return Set.of();
+ }
+
+ /**
+ * Set of IRoleManager.Options used by this authenticator that users are
allowed to alter via
+ * ALTER ROLE statements. Alterable role options must also be supported
role options.
+ *
+ * @return A set of supported role options that users are allowed to alter.
+ */
+ default Set<IRoleManager.Option> getAlterableRoleOptions()
+ {
+ return Set.of();
+ }
Review Comment:
Was initially concerned here, but i looks like in the existing
implementation of `CassandraRoleManager` that this was the effective behavior
(if not `instanceof PasswordAuthenicator`, return empty for both of these) 👍
##########
src/java/org/apache/cassandra/auth/IAuthenticator.java:
##########
@@ -66,6 +66,27 @@ default boolean supportsEarlyAuthentication()
*/
Set<? extends IResource> protectedResources();
+ /**
+ * Set of IRoleManager.Options used by this authenticator and supported by
CREATE ROLE and ALTER ROLE statements.
Review Comment:
I'm being a bit nit picky, but I see that `DEFAULT_SUPPORTED_ROLE_OPTIONS`
(LOGIN, SUPERUSER) / `DEFAULT_ALTERABLE_ROLE_OPTIONS` (currently empty) are
always included. Can we include 'Additional' (e.g. 'Additional set of...') to
each javadoc to make it more clear that these are additive?
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -2037,16 +2010,52 @@ public static void
setCryptoProvider(AbstractCryptoProvider cryptoProvider)
DatabaseDescriptor.cryptoProvider = cryptoProvider;
}
+ /**
+ * Returns the authenticator configured for this node.
+ */
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.
+ */
+ public static boolean isAuthenticationRequired()
+ {
+ return authenticator.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)
Review Comment:
this seems sort of extraneous right now given `getAuthenticator` returns an
`Optional` and could just inline `isAssignableFrom`, but it might make more
sense once the CEP-50 implementation gets fleshed out so seems fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
