timoninmaxim commented on a change in pull request #8892:
URL: https://github.com/apache/ignite/pull/8892#discussion_r613879151
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -73,11 +85,22 @@
import static org.apache.ignite.events.EventType.EVT_NODE_JOINED;
import static org.apache.ignite.events.EventType.EVT_NODE_LEFT;
import static
org.apache.ignite.internal.GridComponent.DiscoveryDataExchangeType.AUTH_PROC;
+import static
org.apache.ignite.internal.IgniteNodeAttributes.ATTR_AUTHENTICATION_ENABLED;
+import static
org.apache.ignite.internal.IgniteNodeAttributes.ATTR_IGNITE_INSTANCE_NAME;
+import static
org.apache.ignite.internal.IgniteNodeAttributes.ATTR_SECURITY_CREDENTIALS;
+import static
org.apache.ignite.internal.processors.authentication.User.DFAULT_USER_NAME;
+import static
org.apache.ignite.internal.processors.authentication.UserManagementOperation.OperationType.ADD;
+import static
org.apache.ignite.internal.processors.authentication.UserManagementOperation.OperationType.REMOVE;
+import static
org.apache.ignite.internal.processors.authentication.UserManagementOperation.OperationType.UPDATE;
+import static
org.apache.ignite.plugin.security.SecuritySubjectType.REMOTE_CLIENT;
+import static
org.apache.ignite.plugin.security.SecuritySubjectType.REMOTE_NODE;
/**
*
*/
-public class IgniteAuthenticationProcessor extends GridProcessorAdapter
implements MetastorageLifecycleListener {
+public class IgniteAuthenticationProcessor extends GridProcessorAdapter
implements GridSecurityProcessor,
Review comment:
Let's rename this class. It's responsible not only for authentication
but also for CRUD operations on users. So there are user-management and
authorization actions too.
##########
File path:
modules/clients/src/test/java/org/apache/ignite/common/RunningQueryInfoCheckInitiatorTest.java
##########
@@ -227,7 +227,7 @@ public void testJdbcV2InitiatorId() throws Exception {
GridTestUtils.runAsync(() -> {
try (Connection conn = DriverManager.getConnection(
- CFG_URL_PREFIX + "nodeId=" + grid0NodeId +
"@modules/clients/src/test/config/jdbc-config.xml")) {
+ CFG_URL_PREFIX + "nodeId=" + grid0NodeId +
"@modules/clients/src/test/config/jdbc-security-config.xml")) {
Review comment:
Why do you change config only for this test? It looks weird. Maybe it's
better to create a new class that inherits this one and overrides config path?
Or create a separate test class for security config? WDYT?
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -73,11 +85,22 @@
import static org.apache.ignite.events.EventType.EVT_NODE_JOINED;
import static org.apache.ignite.events.EventType.EVT_NODE_LEFT;
import static
org.apache.ignite.internal.GridComponent.DiscoveryDataExchangeType.AUTH_PROC;
+import static
org.apache.ignite.internal.IgniteNodeAttributes.ATTR_AUTHENTICATION_ENABLED;
+import static
org.apache.ignite.internal.IgniteNodeAttributes.ATTR_IGNITE_INSTANCE_NAME;
+import static
org.apache.ignite.internal.IgniteNodeAttributes.ATTR_SECURITY_CREDENTIALS;
+import static
org.apache.ignite.internal.processors.authentication.User.DFAULT_USER_NAME;
Review comment:
s/DFAULT/DEFAULT/
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -971,6 +888,91 @@ private static boolean isNodeHoldsUsers(ClusterNode n) {
return !n.isClient() && !n.isDaemon();
}
+ /** {@inheritDoc} */
+ @Override public SecurityContext authenticateNode(ClusterNode node,
SecurityCredentials cred) throws IgniteCheckedException {
+ return new SecurityContextImpl(
+ node.id(),
+ node.attribute(ATTR_IGNITE_INSTANCE_NAME),
+ REMOTE_NODE,
+ new InetSocketAddress(F.first(node.addresses()), 0));
+ }
+
+ /** {@inheritDoc} */
+ @Override public SecuritySubject authenticatedSubject(UUID subjId) throws
IgniteCheckedException {
+ return null;
+ }
+
+ /** {@inheritDoc} */
+ @Override public Collection<SecuritySubject> authenticatedSubjects()
throws IgniteCheckedException {
+ return null;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean isGlobalNodeAuthentication() {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void authorize(String name, SecurityPermission perm,
SecurityContext securityCtx) throws SecurityException {
Review comment:
Looks like this method should do a work of checkUserOperation?
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -212,53 +230,33 @@ else if (msg instanceof UserAuthenticateResponseMessage)
new LinkedBlockingQueue<>());
}
- /**
- * On cache processor started.
- */
- public void cacheProcessorStarted() {
- sharedCtx = ctx.cache().context();
- }
-
/** {@inheritDoc} */
@Override public void stop(boolean cancel) throws IgniteCheckedException {
- if (!isEnabled)
- return;
-
- ctx.io().removeMessageListener(GridTopic.TOPIC_AUTH, ioLsnr);
+ if (ioLsnr != null)
Review comment:
Why did this check for NULL appear?
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -971,6 +888,91 @@ private static boolean isNodeHoldsUsers(ClusterNode n) {
return !n.isClient() && !n.isDaemon();
}
+ /** {@inheritDoc} */
+ @Override public SecurityContext authenticateNode(ClusterNode node,
SecurityCredentials cred) throws IgniteCheckedException {
+ return new SecurityContextImpl(
+ node.id(),
+ node.attribute(ATTR_IGNITE_INSTANCE_NAME),
+ REMOTE_NODE,
+ new InetSocketAddress(F.first(node.addresses()), 0));
+ }
+
+ /** {@inheritDoc} */
+ @Override public SecuritySubject authenticatedSubject(UUID subjId) throws
IgniteCheckedException {
+ return null;
Review comment:
Why NULL? Let's make throw Exception or return a real value.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
