ololo3000 commented on a change in pull request #9290:
URL: https://github.com/apache/ignite/pull/9290#discussion_r691957863
##########
File path:
modules/clients/src/test/java/org/apache/ignite/common/ComputeTaskRemoteSecurityContextTest.java
##########
@@ -105,7 +105,7 @@
@RunWith(Parameterized.class)
public class ComputeTaskRemoteSecurityContextTest extends AbstractSecurityTest
{
/** Port for REST client connection. */
- private static final String DFLT_REST_PORT = "11080";
+ static final String DFLT_REST_PORT = "11080";
Review comment:
Done. AbstractEventSecurityContextTest was created.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCachePartitionExchangeManager.java
##########
@@ -2980,6 +2983,13 @@ public BooleanMetricImpl clusterRebalancedMetric() {
return rebalanced;
}
+ /** Returns current thread security context if security is enabled,
otherwise null. */
+ @Nullable private SecurityContext securityContext() {
+ IgniteSecurity security = cctx.kernalContext().security();
+
+ return security.enabled() ? security.securityContext() : null;
Review comment:
Done.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/SecurityAwareCustomMessageWrapper.java
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.managers.discovery;
+
+import java.util.UUID;
+import org.apache.ignite.spi.discovery.DiscoverySpiCustomMessage;
+import org.jetbrains.annotations.Nullable;
+
+/** Extends {@link CustomMessageWrapper} with ID of security subject that
initiated the current message. */
+public class SecurityAwareCustomMessageWrapper extends CustomMessageWrapper {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Security subject ID. */
+ private final UUID secSubjId;
+
+ /** */
+ public SecurityAwareCustomMessageWrapper(DiscoveryCustomMessage delegate,
UUID secSubjId) {
+ super(delegate);
+
+ this.secSubjId = secSubjId;
+ }
+
+ /** Gets security Subject ID. */
+ public UUID securitySubjectId() {
+ return secSubjId;
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable DiscoverySpiCustomMessage ackMessage() {
+ DiscoveryCustomMessage ack = delegate().ackMessage();
+
+ return ack == null ? null : new SecurityAwareCustomMessageWrapper(ack,
secSubjId);
+ }
+}
+
Review comment:
Done.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java
##########
@@ -917,13 +927,63 @@ else if (type == EVT_CLIENT_NODE_RECONNECTED) {
discoCache,
notification.getTopSnapshot(),
stateFinishMsg,
- notification.getSpanContainer()
+ notification.getSpanContainer(),
+ secCtx
)
);
if (type == EVT_CLIENT_NODE_DISCONNECTED)
discoWrk.awaitDisconnectEvent();
}
+
+ /** */
+ class SecurityAwareNotificationTask extends NotificationTask {
+ /** */
+ public SecurityAwareNotificationTask(DiscoveryNotification
notification) {
+ super(notification);
+ }
+
+ /** */
+ @Override public void run() {
+ DiscoverySpiCustomMessage customMsg =
notification.getCustomMsgData();
+
+ if (customMsg instanceof
SecurityAwareCustomMessageWrapper) {
+ UUID secSubjId =
((SecurityAwareCustomMessageWrapper)customMsg).securitySubjectId();
+
+ try (OperationSecurityContext ignored =
ctx.security().withContext(secSubjId)) {
+ super.run();
+ }
+ }
+ else {
+ SecurityContext initiatorNodeSecCtx =
nodeSecurityContext(
+ marshaller,
+ U.resolveClassLoader(ctx.config()),
+ notification.getNode()
+ );
+
+ try (OperationSecurityContext ignored =
ctx.security().withContext(initiatorNodeSecCtx)) {
+ super.run();
+ }
+ }
+ }
+ }
+
+ class NotificationTask implements Runnable {
Review comment:
Done.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java
##########
@@ -917,13 +927,63 @@ else if (type == EVT_CLIENT_NODE_RECONNECTED) {
discoCache,
notification.getTopSnapshot(),
stateFinishMsg,
- notification.getSpanContainer()
+ notification.getSpanContainer(),
+ secCtx
)
);
if (type == EVT_CLIENT_NODE_DISCONNECTED)
discoWrk.awaitDisconnectEvent();
}
+
+ /** */
+ class SecurityAwareNotificationTask extends NotificationTask {
+ /** */
+ public SecurityAwareNotificationTask(DiscoveryNotification
notification) {
+ super(notification);
+ }
+
+ /** */
+ @Override public void run() {
+ DiscoverySpiCustomMessage customMsg =
notification.getCustomMsgData();
+
+ if (customMsg instanceof
SecurityAwareCustomMessageWrapper) {
+ UUID secSubjId =
((SecurityAwareCustomMessageWrapper)customMsg).securitySubjectId();
+
+ try (OperationSecurityContext ignored =
ctx.security().withContext(secSubjId)) {
+ super.run();
+ }
+ }
+ else {
+ SecurityContext initiatorNodeSecCtx =
nodeSecurityContext(
+ marshaller,
+ U.resolveClassLoader(ctx.config()),
+ notification.getNode()
+ );
+
+ try (OperationSecurityContext ignored =
ctx.security().withContext(initiatorNodeSecCtx)) {
Review comment:
The main problem in this case is that
IgniteSecurityProcessor#withContext(UUID) method uses
GridDiscoveryManager#node(UUID) to obtain a node instance. But DiscoveryCache
is updated DURING event handling (see `topSnap.set(new
Snapshot(snapshot.topVer, discoCache));` calls in
DiscoverySpiListener#onDiscovery0) but we must get node security context BEFORE
event handling. So in case of NODE_JOINED event we will see NPE during
IgniteSecurityProcessor#withContext(UUID) call as no node with specified ID
will be found.
Since internal discovery events such as NODE_JOINED or NODE_LEFT does not
occur so often, I think that we can sacrifice caching. WDYT?
--
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]
