rpuch commented on code in PR #1569:
URL: https://github.com/apache/ignite-3/pull/1569#discussion_r1115514252
##########
modules/api/src/main/java/org/apache/ignite/IgnitionManager.java:
##########
@@ -137,22 +138,28 @@ public static void stop(String nodeName, @Nullable
ClassLoader clsLdr) {
* @param nodeName Name of the node that the initialization request will
be sent to.
* @param metaStorageNodeNames names of nodes that will host the Meta
Storage and the CMG.
* @param clusterName Human-readable name of the cluster.
+ * @param restAuthConfig REST authentication configuration thaw will be
applied after the initialization.
Review Comment:
```suggestion
* @param restAuthConfig REST authentication configuration that will be
applied after the initialization.
```
##########
modules/api/src/main/java/org/apache/ignite/rest/AuthProviderConfig.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.rest;
+
+/** General interface for all authentication providers. */
Review Comment:
It sseems to be an interface for configuration provider's configs, not for
configuration providers themselves
##########
modules/api/src/main/java/org/apache/ignite/rest/AuthType.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.rest;
+
+import java.util.Arrays;
+import org.jetbrains.annotations.Nullable;
+
+/** Authentication types. */
+public enum AuthType {
+ BASIC;
+
+ /**
+ * Parses {@link AuthType} from the given string.
+ *
+ * @param type string
+ * @return parsed {@link AuthType} or null
+ * */
Review Comment:
`*/` is usually placed on a line of its own
##########
modules/api/src/main/java/org/apache/ignite/IgnitionManager.java:
##########
@@ -162,29 +169,32 @@ public static synchronized void init(String nodeName,
Collection<String> metaSto
* @param metaStorageNodeNames names of nodes that will host the Meta
Storage.
* @param cmgNodeNames names of nodes that will host the CMG.
* @param clusterName Human-readable name of the cluster.
+ * @param restAuthConfig REST authentication configuration thaw will be
applied after the initialization.
Review Comment:
```suggestion
* @param restAuthConfig REST authentication configuration that will be
applied after the initialization.
```
##########
modules/api/src/main/java/org/apache/ignite/rest/AuthProviderConfig.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.rest;
+
+/** General interface for all authentication providers. */
+public interface AuthProviderConfig {
+ AuthType type();
Review Comment:
Please add javadocs for the methods
##########
modules/api/src/main/java/org/apache/ignite/rest/AuthType.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.rest;
+
+import java.util.Arrays;
+import org.jetbrains.annotations.Nullable;
+
+/** Authentication types. */
+public enum AuthType {
+ BASIC;
+
+ /**
+ * Parses {@link AuthType} from the given string.
+ *
+ * @param type string
+ * @return parsed {@link AuthType} or null
+ * */
+ @Nullable
Review Comment:
Is it useful to return a `null` here? Was an alternative to throw an
exception if we don't know such an auth type considered?
##########
modules/api/src/main/java/org/apache/ignite/rest/RestAuthConfig.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.rest;
+
+import java.util.Collections;
+import java.util.List;
+
+/** Configuration of REST authentication. */
+public class RestAuthConfig {
+ private final boolean enabled;
+
+ private final List<AuthProviderConfig> providers;
+
+ /** Constructor. */
+ public RestAuthConfig(boolean enabled, List<AuthProviderConfig> providers)
{
+ if (providers == null) {
+ throw new IllegalArgumentException("providers cannot be null");
+ }
+
+ if (enabled && providers.isEmpty()) {
+ throw new IllegalArgumentException("providers cannot be empty, if
authentication is enabled");
+ }
+
+ this.enabled = enabled;
+ this.providers = providers;
+ }
+
+ public static RestAuthConfig disabledAuth() {
+ return new RestAuthConfig(false, Collections.emptyList());
+ }
+
+ public boolean enabled() {
Review Comment:
Please add javadoc explaining what it means that an auth config is enabled
##########
modules/api/src/main/java/org/apache/ignite/rest/RestAuthConfig.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.rest;
+
+import java.util.Collections;
+import java.util.List;
+
+/** Configuration of REST authentication. */
+public class RestAuthConfig {
+ private final boolean enabled;
+
+ private final List<AuthProviderConfig> providers;
+
+ /** Constructor. */
+ public RestAuthConfig(boolean enabled, List<AuthProviderConfig> providers)
{
+ if (providers == null) {
+ throw new IllegalArgumentException("providers cannot be null");
+ }
+
+ if (enabled && providers.isEmpty()) {
+ throw new IllegalArgumentException("providers cannot be empty, if
authentication is enabled");
+ }
+
+ this.enabled = enabled;
+ this.providers = providers;
Review Comment:
Let's make a defensive copy (`this.providers = List.of(providers);`)
##########
modules/api/src/main/java/org/apache/ignite/rest/RestAuthConfig.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.rest;
+
+import java.util.Collections;
+import java.util.List;
+
+/** Configuration of REST authentication. */
+public class RestAuthConfig {
+ private final boolean enabled;
+
+ private final List<AuthProviderConfig> providers;
+
+ /** Constructor. */
+ public RestAuthConfig(boolean enabled, List<AuthProviderConfig> providers)
{
+ if (providers == null) {
+ throw new IllegalArgumentException("providers cannot be null");
+ }
+
+ if (enabled && providers.isEmpty()) {
+ throw new IllegalArgumentException("providers cannot be empty, if
authentication is enabled");
+ }
+
+ this.enabled = enabled;
+ this.providers = providers;
+ }
+
+ public static RestAuthConfig disabledAuth() {
+ return new RestAuthConfig(false, Collections.emptyList());
+ }
+
+ public boolean enabled() {
+ return enabled;
+ }
+
+ public List<AuthProviderConfig> providers() {
Review Comment:
Pleaes add javadoc explaining what these providers do
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/cluster/ClusterInitCall.java:
##########
@@ -41,6 +42,7 @@ public DefaultCallOutput<String> execute(ClusterInitCallInput
input) {
.metaStorageNodes(input.getMetaStorageNodes())
.cmgNodes(input.getCmgNodes())
.clusterName(input.getClusterName())
+ .authConfig(new AuthConfig().enabled(false))
Review Comment:
Is it a safe thing to silently configure an insecure cluster by default?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -366,6 +390,36 @@ private void onElectedAsLeader(long term) {
LOG.warn("Error when executing onLeaderElected
callback", e);
}
});
+
+ raftServiceAfterJoin().thenCompose(service ->
service.readClusterState()
+ .thenCompose(state -> {
+ if (state == null) {
+ LOG.info("No CMG state found in the Raft storage");
+ return completedFuture(null);
+ } else if (state.restAuthToApply() == null) {
Review Comment:
How can it happen? Does it mean that the auth config has already been
successfully pushed to the distributed configuration? If yes, please add a
comment explaining this.
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -366,6 +390,36 @@ private void onElectedAsLeader(long term) {
LOG.warn("Error when executing onLeaderElected
callback", e);
}
});
+
+ raftServiceAfterJoin().thenCompose(service ->
service.readClusterState()
+ .thenCompose(state -> {
+ if (state == null) {
+ LOG.info("No CMG state found in the Raft storage");
+ return completedFuture(null);
+ } else if (state.restAuthToApply() == null) {
+ LOG.info("No REST auth configuration found in the Raft
storage");
+ return completedFuture(null);
+ } else {
+ LOG.info("REST auth configuration found in the Raft
storage, going to apply it");
+ return
distributedConfigurationUpdater.updateRestAuthConfiguration(toRestAuthConfig(state.restAuthToApply()))
+ .thenCompose(unused -> {
+ ClusterState clusterState =
ClusterState.clusterState(
Review Comment:
Let's extract these lines to a method with an explaining name like
`removeAuthConfigFromClusterState()`
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/DistributedConfigurationUpdater.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.cluster.management;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.rest.configuration.AuthConfiguration;
+import org.apache.ignite.internal.rest.configuration.AuthProviderChange;
+import org.apache.ignite.internal.rest.configuration.BasicAuthProviderChange;
+import org.apache.ignite.internal.rest.configuration.ClusterRestConfiguration;
+import org.apache.ignite.rest.AuthProviderConfig;
+import org.apache.ignite.rest.AuthType;
+import org.apache.ignite.rest.BasicAuthProviderConfig;
+import org.apache.ignite.rest.RestAuthConfig;
+
+/**
+ * Updater is responsible for applying changes to the cluster configuration
when it's ready.
+ */
+public class DistributedConfigurationUpdater {
+
+ private static final IgniteLogger LOG =
Loggers.forClass(DistributedConfigurationUpdater.class);
+
+ private final CompletableFuture<ClusterRestConfiguration>
clusterRestConfigurationFuture = new CompletableFuture<>();
+
+ public void setClusterRestConfiguration(ClusterRestConfiguration
clusterRestConfiguration) {
+ clusterRestConfigurationFuture.complete(clusterRestConfiguration);
Review Comment:
This future never gets completed with an exception, but during `IgniteImpl`
start it might happen that CMG manager already got this future and waits on it,
then Ignite start fails before distributed config manager is ready, so this
future never gets completed. How about completing it with an exception in the
very last `whenComplete()` in `IgniteImpl` (if there is an exception), to make
sure we let depending futures complete?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -366,6 +390,36 @@ private void onElectedAsLeader(long term) {
LOG.warn("Error when executing onLeaderElected
callback", e);
}
});
+
+ raftServiceAfterJoin().thenCompose(service ->
service.readClusterState()
+ .thenCompose(state -> {
+ if (state == null) {
+ LOG.info("No CMG state found in the Raft storage");
+ return completedFuture(null);
+ } else if (state.restAuthToApply() == null) {
+ LOG.info("No REST auth configuration found in the Raft
storage");
+ return completedFuture(null);
+ } else {
+ LOG.info("REST auth configuration found in the Raft
storage, going to apply it");
+ return
distributedConfigurationUpdater.updateRestAuthConfiguration(toRestAuthConfig(state.restAuthToApply()))
Review Comment:
Imagine that this code gets delayed. Do we take any measures to only allow
connections after auth is configured cluster-wide?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/RestAuthConverter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.cluster.management;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.ignite.internal.cluster.management.network.auth.AuthProvider;
+import
org.apache.ignite.internal.cluster.management.network.auth.BasicAuthProvider;
+import org.apache.ignite.internal.cluster.management.network.auth.RestAuth;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.rest.AuthProviderConfig;
+import org.apache.ignite.rest.AuthType;
+import org.apache.ignite.rest.BasicAuthProviderConfig;
+import org.apache.ignite.rest.RestAuthConfig;
+import org.jetbrains.annotations.Nullable;
+
+/** Converter for {@link RestAuth}. */
+public class RestAuthConverter {
+
+ private static final IgniteLogger LOG =
Loggers.forClass(DistributedConfigurationUpdater.class);
Review Comment:
The class name for which logger is obtained does not match current class
name. Is this intentional? It might make it harder to find the exact line that
produced a log entry.
##########
modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/CmgRaftGroupListenerTest.java:
##########
@@ -146,6 +149,31 @@ void restoreFromSnapshotTriggersTopologyLeapEvent() {
verify(logicalTopology).fireTopologyLeap();
}
+ @Test
+ void writeAndReadRestAuth() {
Review Comment:
The test seems to make sure that auth config gets erased if cluster state is
written in an update without an auth config. I propose to change the method
name to highlight this, like `authConfiglessUpdateErasesAuthConfig()`
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.java:
##########
@@ -70,8 +85,14 @@ public CompletableFuture<Void> init(@Body InitCommand
initCommand) {
LOG.info("Received init command [metaStorageNodes={},
cmgNodes={}]", initCommand.metaStorageNodes(),
initCommand.cmgNodes());
}
-
- return clusterInitializer.initCluster(initCommand.metaStorageNodes(),
initCommand.cmgNodes(), initCommand.clusterName())
+ AuthConfigDto authConfigDto = initCommand.authConfig();
+ RestAuthConfig restAuthConfig = authConfigDto == null ? disabledAuth()
: authConfigDtoToRestAuthConfig(authConfigDto);
Review Comment:
When are we sent null as `authConfigDto`?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/RestAuthConverter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.cluster.management;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.ignite.internal.cluster.management.network.auth.AuthProvider;
+import
org.apache.ignite.internal.cluster.management.network.auth.BasicAuthProvider;
+import org.apache.ignite.internal.cluster.management.network.auth.RestAuth;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.rest.AuthProviderConfig;
+import org.apache.ignite.rest.AuthType;
+import org.apache.ignite.rest.BasicAuthProviderConfig;
+import org.apache.ignite.rest.RestAuthConfig;
+import org.jetbrains.annotations.Nullable;
+
+/** Converter for {@link RestAuth}. */
+public class RestAuthConverter {
+
+ private static final IgniteLogger LOG =
Loggers.forClass(DistributedConfigurationUpdater.class);
+
+ /**
+ * Converts {@link RestAuth} to {@link RestAuthConfig}.
+ *
+ * @param restAuth Rest auth.
+ * @return Rest auth config.
+ */
+ public static RestAuthConfig toRestAuthConfig(RestAuth restAuth) {
+ List<AuthProviderConfig> providerConfigs = restAuth.providers()
+ .stream()
+ .map(RestAuthConverter::toAuthProviderConfig)
+ .collect(Collectors.toList());
+
+ return new RestAuthConfig(restAuth.enabled(), providerConfigs);
+ }
+
+ @Nullable
+ private static AuthProviderConfig toAuthProviderConfig(AuthProvider
authProvider) {
+ AuthType type = AuthType.parse(authProvider.type());
+ if (type == AuthType.BASIC) {
+ BasicAuthProvider basicAuthProvider = (BasicAuthProvider)
authProvider;
+ return new BasicAuthProviderConfig(basicAuthProvider.name(),
basicAuthProvider.login(), basicAuthProvider.password());
+ } else {
+ LOG.error("Unsupported authentication type: " +
authProvider.type());
+ return null;
Review Comment:
Should we fail instead? It would be scary to silently make a cluster
insecure because of a typo, like 'vasic' instead of 'basic', or even better,
change latin A to a similarly looking non-latin character.
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterManagementGroupManager.java:
##########
@@ -366,6 +390,36 @@ private void onElectedAsLeader(long term) {
LOG.warn("Error when executing onLeaderElected
callback", e);
}
});
+
+ raftServiceAfterJoin().thenCompose(service ->
service.readClusterState()
Review Comment:
Let's extract this block of code to a method of its own
(`pushAuthConfigToCluster()`?) as it seems to has an intent quiet different
from the other code in this method.
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/cluster/auth/AuthProviderConfigDto.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.rest.api.cluster.auth;
+
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import org.apache.ignite.rest.AuthType;
+
+/**
+ * REST representation of {@link org.apache.ignite.rest.AuthProviderConfig}.
+ */
+@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
+ include = JsonTypeInfo.As.PROPERTY,
+ property = "type")
+@JsonSubTypes({
+ @JsonSubTypes.Type(value = BasicAuthProviderConfigDto.class, name =
"basic")
+})
+public interface AuthProviderConfigDto {
+ AuthType type();
Review Comment:
Please add javadocs to the methods
##########
gradle/libs.versions.toml:
##########
@@ -126,6 +128,10 @@ micronaut-http-core = { module =
"io.micronaut:micronaut-http", version.ref = "m
micronaut-http-server-core = { module = "io.micronaut:micronaut-http-server",
version.ref = "micronaut" }
micronaut-http-client = { module = "io.micronaut:micronaut-http-client",
version.ref = "micronaut" }
micronaut-http-server-netty = { module =
"io.micronaut:micronaut-http-server-netty", version.ref = "micronaut" }
+micronaut-security = { module = "io.micronaut.security:micronaut-security",
version.ref = "micronautSecurity" }
+micronaut-security-annotations = { module =
"io.micronaut.security:micronaut-security-annotations", version.ref =
"micronautSecurity" }
+micronaut-reactor = { module = "io.micronaut.reactor:micronaut-reactor",
version.ref = "micronautReactor" }
Review Comment:
Will we always shade reactor-core by putting it to own own package?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/auth/AuthenticatorFactory.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.rest.auth;
+
+import java.util.Objects;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.rest.configuration.AuthProviderView;
+import org.apache.ignite.internal.rest.configuration.BasicAuthProviderView;
+import org.apache.ignite.rest.AuthType;
+import org.jetbrains.annotations.Nullable;
+
+/** Factory for {@link Authenticator}. */
+class AuthenticatorFactory {
+
+ private static final IgniteLogger LOG =
Loggers.forClass(AuthenticatorFactory.class);
+
+ @Nullable
+ static Authenticator create(AuthProviderView view) {
+ AuthType authType = AuthType.parse(view.type());
+ if (authType != null) {
+ return create(authType, view);
+ } else {
+ LOG.error("Unknown auth type: " + view);
Review Comment:
It seems safer to throw an exception here if the configuration is
misconfigured
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/exception/handler/ConversionErrorHandlerReplacement.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.rest.exception.handler;
+
+import io.micronaut.context.annotation.Replaces;
+import io.micronaut.context.annotation.Requires;
+import io.micronaut.core.convert.exceptions.ConversionErrorException;
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.server.exceptions.ConversionErrorHandler;
+import io.micronaut.http.server.exceptions.ExceptionHandler;
+import jakarta.inject.Singleton;
+import org.apache.ignite.internal.rest.api.Problem;
+import org.apache.ignite.internal.rest.constants.HttpCode;
+import org.apache.ignite.internal.rest.problem.HttpProblemResponse;
+import org.apache.ignite.internal.util.ExceptionUtils;
+
+/**
+ * Replacement for {@link ConversionErrorHandlerReplacement}. Returns {@link
HttpProblemResponse}.
Review Comment:
```suggestion
* Replacement for {@link ConversionErrorHandler}. Returns {@link
HttpProblemResponse}.
```
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/auth/AuthenticatorFactory.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.rest.auth;
+
+import java.util.Objects;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.rest.configuration.AuthProviderView;
+import org.apache.ignite.internal.rest.configuration.BasicAuthProviderView;
+import org.apache.ignite.rest.AuthType;
+import org.jetbrains.annotations.Nullable;
+
+/** Factory for {@link Authenticator}. */
+class AuthenticatorFactory {
+
+ private static final IgniteLogger LOG =
Loggers.forClass(AuthenticatorFactory.class);
+
+ @Nullable
+ static Authenticator create(AuthProviderView view) {
+ AuthType authType = AuthType.parse(view.type());
+ if (authType != null) {
+ return create(authType, view);
+ } else {
+ LOG.error("Unknown auth type: " + view);
+ return null;
+ }
+ }
+
+ @Nullable
+ private static Authenticator create(AuthType type, AuthProviderView view) {
+ if (Objects.requireNonNull(type) == AuthType.BASIC) {
+ BasicAuthProviderView basicAuthProviderView =
(BasicAuthProviderView) view;
+ return new BasicAuthenticator(basicAuthProviderView.login(),
basicAuthProviderView.password());
+ } else {
+ LOG.error("Couldn't create authenticator: " + view);
Review Comment:
Same thing: what's the advantage of returning a null?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/configuration/AuthConfigurationValidatorImpl.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.rest.configuration;
+
+import org.apache.ignite.configuration.validation.ValidationContext;
+import org.apache.ignite.configuration.validation.ValidationIssue;
+import org.apache.ignite.configuration.validation.Validator;
+
+/**
+ * Auth schema configuration validator implementation.
+ */
+public class AuthConfigurationValidatorImpl implements
Validator<AuthConfigurationValidator, AuthView> {
+
+ public static final AuthConfigurationValidatorImpl INSTANCE = new
AuthConfigurationValidatorImpl();
+
+ @Override
+ public void validate(AuthConfigurationValidator annotation,
ValidationContext<AuthView> ctx) {
+ AuthView view = ctx.getNewValue();
+
+ if (view == null) {
+ ctx.addIssue(new ValidationIssue(ctx.currentKey(), "Auth config
must not be null"));
+ return;
+ }
+
+ if (view.enabled() && view.providers().size() == 0) {
Review Comment:
If there are providers, but `enabled` is false, do we consider the config
valid?
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProvider.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.rest.auth;
+
+import io.micronaut.http.HttpRequest;
+import io.micronaut.security.authentication.AuthenticationProvider;
+import io.micronaut.security.authentication.AuthenticationRequest;
+import io.micronaut.security.authentication.AuthenticationResponse;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.stream.Collectors;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.notifications.ConfigurationListener;
+import
org.apache.ignite.configuration.notifications.ConfigurationNotificationEvent;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.rest.configuration.AuthConfiguration;
+import org.apache.ignite.internal.rest.configuration.AuthProviderView;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.jetbrains.annotations.Nullable;
+import org.reactivestreams.Publisher;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.FluxSink;
+
+/**
+ * Implementation of {@link AuthenticationProvider}. Creates a list of {@link
Authenticator} according to provided {@link AuthConfiguration}
+ * and updates them on configuration changes. Delegates {@link
AuthenticationRequest} to the list of {@link Authenticator}.
+ */
+public class DelegatingAuthenticationProvider implements
AuthenticationProvider, ConfigurationListener<AuthView> {
+
+ private static final IgniteLogger LOG =
Loggers.forClass(DelegatingAuthenticationProvider.class);
+
+ private final ReadWriteLock rwLock = new ReentrantReadWriteLock();
+ private final List<Authenticator> authenticators = new ArrayList<>();
+ private boolean authEnabled = false;
+
+ @Override
+ public Publisher<AuthenticationResponse> authenticate(HttpRequest<?>
httpRequest, AuthenticationRequest<?, ?> authenticationRequest) {
+ return Flux.create(emitter -> {
+ rwLock.readLock().lock();
+ try {
+ if (authEnabled) {
+ Optional<AuthenticationResponse> successResponse =
authenticators.stream()
+ .map(it -> it.authenticate(authenticationRequest))
+ .filter(AuthenticationResponse::isAuthenticated)
+ .findFirst();
+ if (successResponse.isPresent()) {
+ emitter.next(successResponse.get());
+ emitter.complete();
+ } else {
+ emitter.error(AuthenticationResponse.exception());
+ }
+ } else {
+ emitter.next(AuthenticationResponse.success("Unknown"));
+ emitter.complete();
+ }
+ } finally {
+ rwLock.readLock().unlock();
+ }
+ }, FluxSink.OverflowStrategy.ERROR);
+ }
+
+ @Override
+ public CompletableFuture<?>
onUpdate(ConfigurationNotificationEvent<AuthView> ctx) {
+ return CompletableFuture.runAsync(() ->
refreshProviders(ctx.newValue()));
+ }
+
+ private void refreshProviders(@Nullable AuthView view) {
+ rwLock.writeLock().lock();
+ try {
+ if (view == null || !view.enabled()) {
+ authEnabled = false;
+ authenticators.clear();
+ } else if (view.enabled() && view.providers().size() != 0) {
+ authenticators.clear();
+ authenticators.addAll(providersFromAuthView(view));
+ authEnabled = true;
+ } else {
+ LOG.error("Invalid configuration: auth enabled, but no
providers. Remain the old settings");
Review Comment:
```suggestion
LOG.error("Invalid configuration: auth enabled, but no
providers. Leave the old settings");
```
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProviderTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.rest.auth;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpMethod;
+import io.micronaut.http.simple.SimpleHttpRequest;
+import io.micronaut.security.authentication.AuthenticationException;
+import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import java.util.Collections;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthViewEvent;
+import
org.apache.ignite.internal.rest.configuration.stub.StubBasicAuthProviderView;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DelegatingAuthenticationProviderTest {
+
+ private final SimpleHttpRequest<Object> httpRequest = new
SimpleHttpRequest<>(HttpMethod.GET, "/", null);
+
+ private DelegatingAuthenticationProvider authenticationProvider;
+
+ @BeforeEach
+ void setUp() {
+ authenticationProvider = new DelegatingAuthenticationProvider();
+ }
+
+
+ @Test
+ public void enableAuth() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials validCredentials = new
UsernamePasswordCredentials("admin", "admin");
+ authenticationProvider.authenticate(httpRequest,
validCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+
+ UsernamePasswordCredentials invalidCredentials = new
UsernamePasswordCredentials("admin", "password");
+ authenticationProvider.authenticate(httpRequest,
invalidCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertThat(subscriber.lastError(),
is(instanceOf(AuthenticationException.class))),
+ () -> assertNull(subscriber.lastResponse())
+ );
+ }
+
+ @Test
+ public void enableAuthWithEmptyProviders() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true,
Collections.emptyList());
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials emptyCredentials = new
UsernamePasswordCredentials();
+ authenticationProvider.authenticate(httpRequest,
emptyCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+ }
+
+ @Test
+ public void disableAuth() {
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
Review Comment:
`enabledAuthView`?
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProviderTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.rest.auth;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpMethod;
+import io.micronaut.http.simple.SimpleHttpRequest;
+import io.micronaut.security.authentication.AuthenticationException;
+import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import java.util.Collections;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthViewEvent;
+import
org.apache.ignite.internal.rest.configuration.stub.StubBasicAuthProviderView;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DelegatingAuthenticationProviderTest {
+
+ private final SimpleHttpRequest<Object> httpRequest = new
SimpleHttpRequest<>(HttpMethod.GET, "/", null);
+
+ private DelegatingAuthenticationProvider authenticationProvider;
+
+ @BeforeEach
+ void setUp() {
+ authenticationProvider = new DelegatingAuthenticationProvider();
Review Comment:
Let's move this to the field initializer (and make the field `final`)
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProviderTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.rest.auth;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpMethod;
+import io.micronaut.http.simple.SimpleHttpRequest;
+import io.micronaut.security.authentication.AuthenticationException;
+import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import java.util.Collections;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthViewEvent;
+import
org.apache.ignite.internal.rest.configuration.stub.StubBasicAuthProviderView;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DelegatingAuthenticationProviderTest {
+
+ private final SimpleHttpRequest<Object> httpRequest = new
SimpleHttpRequest<>(HttpMethod.GET, "/", null);
+
+ private DelegatingAuthenticationProvider authenticationProvider;
+
+ @BeforeEach
+ void setUp() {
+ authenticationProvider = new DelegatingAuthenticationProvider();
+ }
+
+
+ @Test
+ public void enableAuth() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials validCredentials = new
UsernamePasswordCredentials("admin", "admin");
+ authenticationProvider.authenticate(httpRequest,
validCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+
+ UsernamePasswordCredentials invalidCredentials = new
UsernamePasswordCredentials("admin", "password");
+ authenticationProvider.authenticate(httpRequest,
invalidCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertThat(subscriber.lastError(),
is(instanceOf(AuthenticationException.class))),
+ () -> assertNull(subscriber.lastResponse())
+ );
+ }
+
+ @Test
+ public void enableAuthWithEmptyProviders() {
Review Comment:
Is this a valid configuration? Auth is enabled, but noone to check the
credentials?
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProviderTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.rest.auth;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpMethod;
+import io.micronaut.http.simple.SimpleHttpRequest;
+import io.micronaut.security.authentication.AuthenticationException;
+import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import java.util.Collections;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthViewEvent;
+import
org.apache.ignite.internal.rest.configuration.stub.StubBasicAuthProviderView;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DelegatingAuthenticationProviderTest {
+
+ private final SimpleHttpRequest<Object> httpRequest = new
SimpleHttpRequest<>(HttpMethod.GET, "/", null);
+
+ private DelegatingAuthenticationProvider authenticationProvider;
+
+ @BeforeEach
+ void setUp() {
+ authenticationProvider = new DelegatingAuthenticationProvider();
+ }
+
+
Review Comment:
A nit-pick: is this blank line intentional?
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProviderTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.rest.auth;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpMethod;
+import io.micronaut.http.simple.SimpleHttpRequest;
+import io.micronaut.security.authentication.AuthenticationException;
+import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import java.util.Collections;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthViewEvent;
+import
org.apache.ignite.internal.rest.configuration.stub.StubBasicAuthProviderView;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DelegatingAuthenticationProviderTest {
+
+ private final SimpleHttpRequest<Object> httpRequest = new
SimpleHttpRequest<>(HttpMethod.GET, "/", null);
+
+ private DelegatingAuthenticationProvider authenticationProvider;
+
+ @BeforeEach
+ void setUp() {
+ authenticationProvider = new DelegatingAuthenticationProvider();
+ }
+
+
+ @Test
+ public void enableAuth() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials validCredentials = new
UsernamePasswordCredentials("admin", "admin");
+ authenticationProvider.authenticate(httpRequest,
validCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+
+ UsernamePasswordCredentials invalidCredentials = new
UsernamePasswordCredentials("admin", "password");
+ authenticationProvider.authenticate(httpRequest,
invalidCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertThat(subscriber.lastError(),
is(instanceOf(AuthenticationException.class))),
+ () -> assertNull(subscriber.lastResponse())
+ );
+ }
+
+ @Test
+ public void enableAuthWithEmptyProviders() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true,
Collections.emptyList());
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials emptyCredentials = new
UsernamePasswordCredentials();
+ authenticationProvider.authenticate(httpRequest,
emptyCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+ }
+
+ @Test
+ public void disableAuth() {
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials validCredentials = new
UsernamePasswordCredentials("admin", "admin");
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+ authenticationProvider.authenticate(httpRequest,
validCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated()));
+
+ AuthView disableAuthView = new StubAuthView(false,
Collections.emptyList());
+ authenticationProvider.onUpdate(new StubAuthViewEvent(enableAuthView,
disableAuthView)).join();
+
+ UsernamePasswordCredentials emptyCredentials = new
UsernamePasswordCredentials();
+ authenticationProvider.authenticate(httpRequest,
emptyCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+ }
+
+ @Test
+ public void changedCredentials() {
+ AuthView adminAdminAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
adminAdminAuthView)).join();
+
+ UsernamePasswordCredentials adminAdminCredentials = new
UsernamePasswordCredentials("admin", "admin");
Review Comment:
How about changing the passwords from 'admin' and 'password' to
'old-password' and 'new-password', respectively? This will probably make it
easier to understand what's going on.
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/configuration/ClusterRestConfigurationModuleTest.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.rest.configuration;
+
+import static
org.apache.ignite.configuration.annotation.ConfigurationType.DISTRIBUTED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+
+import org.junit.jupiter.api.Test;
+
+class ClusterRestConfigurationModuleTest {
+
+ private final ClusterRestConfigurationModule module = new
ClusterRestConfigurationModule();
+
+ @Test
+ void typeIsDistributed() {
+ assertThat(module.type(), is(DISTRIBUTED));
+ }
+
+ @Test
+ void hasNoConfigurationRoots() {
Review Comment:
Test name does not correspond to the test logic
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/DelegatingAuthenticationProviderTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.rest.auth;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpMethod;
+import io.micronaut.http.simple.SimpleHttpRequest;
+import io.micronaut.security.authentication.AuthenticationException;
+import io.micronaut.security.authentication.UsernamePasswordCredentials;
+import java.util.Collections;
+import org.apache.ignite.internal.rest.configuration.AuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthView;
+import org.apache.ignite.internal.rest.configuration.stub.StubAuthViewEvent;
+import
org.apache.ignite.internal.rest.configuration.stub.StubBasicAuthProviderView;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DelegatingAuthenticationProviderTest {
+
+ private final SimpleHttpRequest<Object> httpRequest = new
SimpleHttpRequest<>(HttpMethod.GET, "/", null);
+
+ private DelegatingAuthenticationProvider authenticationProvider;
+
+ @BeforeEach
+ void setUp() {
+ authenticationProvider = new DelegatingAuthenticationProvider();
+ }
+
+
+ @Test
+ public void enableAuth() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials validCredentials = new
UsernamePasswordCredentials("admin", "admin");
+ authenticationProvider.authenticate(httpRequest,
validCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+
+ UsernamePasswordCredentials invalidCredentials = new
UsernamePasswordCredentials("admin", "password");
+ authenticationProvider.authenticate(httpRequest,
invalidCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertThat(subscriber.lastError(),
is(instanceOf(AuthenticationException.class))),
+ () -> assertNull(subscriber.lastResponse())
+ );
+ }
+
+ @Test
+ public void enableAuthWithEmptyProviders() {
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+
+ AuthView enableAuthView = new StubAuthView(true,
Collections.emptyList());
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials emptyCredentials = new
UsernamePasswordCredentials();
+ authenticationProvider.authenticate(httpRequest,
emptyCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
+ () -> assertTrue(subscriber.lastResponse().isAuthenticated())
+ );
+ }
+
+ @Test
+ public void disableAuth() {
+ AuthView enableAuthView = new StubAuthView(true, new
StubBasicAuthProviderView("admin", "admin"));
+
+ authenticationProvider.onUpdate(new StubAuthViewEvent(null,
enableAuthView)).join();
+
+ UsernamePasswordCredentials validCredentials = new
UsernamePasswordCredentials("admin", "admin");
+ TestAuthSubscriber subscriber = new TestAuthSubscriber();
+ authenticationProvider.authenticate(httpRequest,
validCredentials).subscribe(subscriber);
+ assertAll(
+ () -> assertNull(subscriber.lastError()),
Review Comment:
The test seems to declare itself as testing auth disablement. Why would we
need to test for enablement here as well?
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/auth/TestAuthSubscriber.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.rest.auth;
+
+import io.micronaut.security.authentication.AuthenticationResponse;
+import org.jetbrains.annotations.Nullable;
+import org.reactivestreams.Subscriber;
+import org.reactivestreams.Subscription;
+
+/** Implementation of {@link Subscriber} for tests. */
+public class TestAuthSubscriber implements Subscriber<AuthenticationResponse> {
+
+ private volatile Subscription source;
+
+ @Nullable
+ private volatile AuthenticationResponse lastResponse;
+
+ @Nullable
+ private volatile Throwable lastError;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ source = subscription;
+ source.request(1);
+ }
+
+ @Override
+ public void onNext(AuthenticationResponse authenticationResponse) {
+ lastError = null;
+ lastResponse = authenticationResponse;
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ lastResponse = null;
+ lastError = throwable;
+ }
+
+ @Override
+ public void onComplete() {
+
+ }
+
+ public AuthenticationResponse lastResponse() {
Review Comment:
Probably the getters should also be annotated as `@Nullable`
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/configuration/ClusterRestConfigurationModuleTest.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.rest.configuration;
+
+import static
org.apache.ignite.configuration.annotation.ConfigurationType.DISTRIBUTED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+
+import org.junit.jupiter.api.Test;
+
+class ClusterRestConfigurationModuleTest {
+
+ private final ClusterRestConfigurationModule module = new
ClusterRestConfigurationModule();
+
+ @Test
+ void typeIsDistributed() {
+ assertThat(module.type(), is(DISTRIBUTED));
+ }
+
+ @Test
+ void hasNoConfigurationRoots() {
+ assertThat(module.rootKeys(), contains(ClusterRestConfiguration.KEY));
+ }
+
+ @Test
+ void providesNoValidators() {
Review Comment:
Test name has to be changed: it actually asserts that the expected
validators are provided
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/configuration/ClusterRestConfigurationModuleTest.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.rest.configuration;
+
+import static
org.apache.ignite.configuration.annotation.ConfigurationType.DISTRIBUTED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+
+import org.junit.jupiter.api.Test;
+
+class ClusterRestConfigurationModuleTest {
+
+ private final ClusterRestConfigurationModule module = new
ClusterRestConfigurationModule();
+
+ @Test
+ void typeIsDistributed() {
+ assertThat(module.type(), is(DISTRIBUTED));
+ }
+
+ @Test
+ void hasNoConfigurationRoots() {
+ assertThat(module.rootKeys(), contains(ClusterRestConfiguration.KEY));
+ }
+
+ @Test
+ void providesNoValidators() {
+ assertThat(module.validators(),
+ hasItems(
+ instanceOf(AuthConfigurationValidatorImpl.class),
+ instanceOf(AuthProvidersValidatorImpl.class))
+ );
+ }
+
+ @Test
+ void providesExternalTableConfigurationSchemaAsInternalExtension() {
Review Comment:
Test name is misleading
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/configuration/stub/StubAuthProviderListView.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.rest.configuration.stub;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.internal.rest.configuration.AuthProviderView;
+import org.jetbrains.annotations.Nullable;
+
+/** Stub of {@link NamedListView} for tests. */
+public class StubAuthProviderListView implements
NamedListView<AuthProviderView> {
+
+ private final List<AuthProviderView> providers;
+
+ public StubAuthProviderListView(List<AuthProviderView> providers) {
+ this.providers = providers;
Review Comment:
How about making a defensive copy?
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/rest/auth/ItAuthRestTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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.rest.auth;
+
+
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.rest.RestNode;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+
+/** Tests for the REST authentication configuration. */
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItAuthRestTest {
+
+ /** HTTP client that is expected to be defined in subclasses. */
+ private HttpClient client;
+
+ private List<RestNode> nodes;
+
+ @WorkDirectory
+ private Path workDir;
+
+ @BeforeEach
+ void setUp(TestInfo testInfo) {
+ client = HttpClient.newBuilder()
+ .build();
+
+ nodes = IntStream.range(0, 3)
+ .mapToObj(id -> {
+ return RestNode.builder()
+ .setWorkDir(workDir)
+ .setName(testNodeName(testInfo, id))
+ .setNetworkPort(3344 + id)
+ .setHttpPort(10300 + id)
+ .setHttpsPort(10400 + id)
+ .setSslEnabled(false)
+ .setDualProtocol(false)
+ .build();
+ })
+ .collect(Collectors.toList());
Review Comment:
If `toList()` is imported statically, this line starts to read naturally in
plain English: `collect(toList())`
##########
modules/rest/src/test/java/org/apache/ignite/internal/rest/configuration/ClusterRestConfigurationModuleTest.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.rest.configuration;
+
+import static
org.apache.ignite.configuration.annotation.ConfigurationType.DISTRIBUTED;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+
+import org.junit.jupiter.api.Test;
+
+class ClusterRestConfigurationModuleTest {
+
+ private final ClusterRestConfigurationModule module = new
ClusterRestConfigurationModule();
+
+ @Test
+ void typeIsDistributed() {
+ assertThat(module.type(), is(DISTRIBUTED));
+ }
+
+ @Test
+ void hasNoConfigurationRoots() {
+ assertThat(module.rootKeys(), contains(ClusterRestConfiguration.KEY));
+ }
+
+ @Test
+ void providesNoValidators() {
+ assertThat(module.validators(),
+ hasItems(
+ instanceOf(AuthConfigurationValidatorImpl.class),
+ instanceOf(AuthProvidersValidatorImpl.class))
+ );
+ }
+
+ @Test
+ void providesExternalTableConfigurationSchemaAsInternalExtension() {
+ assertThat(module.internalSchemaExtensions(), is(empty()));
+ }
+
+ @Test
+ void providesNoPolymorphicSchemaExtensions() {
Review Comment:
Test name is misleading
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/rest/auth/ItAuthRestTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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.rest.auth;
+
+
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.rest.RestNode;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+
+/** Tests for the REST authentication configuration. */
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItAuthRestTest {
+
+ /** HTTP client that is expected to be defined in subclasses. */
+ private HttpClient client;
+
+ private List<RestNode> nodes;
+
+ @WorkDirectory
+ private Path workDir;
+
+ @BeforeEach
+ void setUp(TestInfo testInfo) {
+ client = HttpClient.newBuilder()
+ .build();
+
+ nodes = IntStream.range(0, 3)
+ .mapToObj(id -> {
+ return RestNode.builder()
+ .setWorkDir(workDir)
+ .setName(testNodeName(testInfo, id))
+ .setNetworkPort(3344 + id)
+ .setHttpPort(10300 + id)
+ .setHttpsPort(10400 + id)
+ .setSslEnabled(false)
+ .setDualProtocol(false)
+ .build();
+ })
+ .collect(Collectors.toList());
+
+ nodes.forEach(RestNode::start);
+ }
+
+ @Test
+ public void disabledAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + " \"" + metaStorageNode.name() + "\"\n"
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": false\n"
+ + " }\n"
+ + "}";
+
+ URI uri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(uri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ URI clusterConfigUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/configuration/cluster/");
Review Comment:
If I understand correctly, this line and the remainder of the test just make
sure that authentication does not prevent us from executing any REST request.
If this is true, please extract these lines to a method like
'assertThatRestRequestsAreAllowed()` (or something like that) to make the
tests' core logic easier to see.
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/rest/auth/ItAuthRestTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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.rest.auth;
+
+
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.rest.RestNode;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+
+/** Tests for the REST authentication configuration. */
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItAuthRestTest {
+
+ /** HTTP client that is expected to be defined in subclasses. */
+ private HttpClient client;
+
+ private List<RestNode> nodes;
+
+ @WorkDirectory
+ private Path workDir;
+
+ @BeforeEach
+ void setUp(TestInfo testInfo) {
+ client = HttpClient.newBuilder()
+ .build();
+
+ nodes = IntStream.range(0, 3)
+ .mapToObj(id -> {
+ return RestNode.builder()
+ .setWorkDir(workDir)
+ .setName(testNodeName(testInfo, id))
+ .setNetworkPort(3344 + id)
+ .setHttpPort(10300 + id)
+ .setHttpsPort(10400 + id)
+ .setSslEnabled(false)
+ .setDualProtocol(false)
+ .build();
+ })
+ .collect(Collectors.toList());
+
+ nodes.forEach(RestNode::start);
+ }
+
+ @Test
+ public void disabledAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + " \"" + metaStorageNode.name() + "\"\n"
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": false\n"
+ + " }\n"
+ + "}";
+
+ URI uri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
Review Comment:
Let's extract a method that inits the cluster as it's used by all tests. So
many secondary details distract attention from the test logic, they should be
hidden away.
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/rest/auth/ItAuthRestTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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.rest.auth;
+
+
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.rest.RestNode;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+
+/** Tests for the REST authentication configuration. */
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItAuthRestTest {
+
+ /** HTTP client that is expected to be defined in subclasses. */
+ private HttpClient client;
+
+ private List<RestNode> nodes;
+
+ @WorkDirectory
+ private Path workDir;
+
+ @BeforeEach
+ void setUp(TestInfo testInfo) {
+ client = HttpClient.newBuilder()
+ .build();
+
+ nodes = IntStream.range(0, 3)
+ .mapToObj(id -> {
+ return RestNode.builder()
+ .setWorkDir(workDir)
+ .setName(testNodeName(testInfo, id))
+ .setNetworkPort(3344 + id)
+ .setHttpPort(10300 + id)
+ .setHttpsPort(10400 + id)
+ .setSslEnabled(false)
+ .setDualProtocol(false)
+ .build();
+ })
+ .collect(Collectors.toList());
+
+ nodes.forEach(RestNode::start);
+ }
+
+ @Test
+ public void disabledAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + " \"" + metaStorageNode.name() + "\"\n"
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": false\n"
+ + " }\n"
+ + "}";
+
+ URI uri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(uri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ URI clusterConfigUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .build();
+
+ HttpResponse<String> httpResponse = client.send(clusterConfigRequest,
BodyHandlers.ofString());
+ assertEquals(200, httpResponse.statusCode());
+ }
+
+ @Test
+ public void basicAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String msNodes = nodes.stream()
+ .map(it -> it.name())
+ .map(name -> "\"" + name + "\"")
+ .collect(Collectors.joining(","));
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + msNodes
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": true,\n"
+ + " \"providers\": [\n"
+ + " {\n"
+ + " \"name\": \"basic\",\n"
+ + " \"type\": \"basic\",\n"
+ + " \"login\": \"admin\",\n"
+ + " \"password\": \"admin\"\n"
+ + " }\n"
+ + " ]\n"
+ + " }\n"
+ + " }";
+
+ URI clusterInitUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(clusterInitUri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri).build();
+ assertTrue(waitForCondition(() -> {
+ try {
+ return client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode() == 401;
+ } catch (Exception e) {
+ return false;
+ }
+ }, Duration.ofSeconds(5).toMillis()));
+ }
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .header("Authorization",
basicAuthenticationHeader("admin", "admin"))
+ .build();
+ assertEquals(200, client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode());
+ }
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .header("Authorization",
basicAuthenticationHeader("admin", "password"))
Review Comment:
Let's change 'password' to 'wrong-password' to make it speak for itself
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/rest/auth/ItAuthRestTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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.rest.auth;
+
+
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.rest.RestNode;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+
+/** Tests for the REST authentication configuration. */
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItAuthRestTest {
+
+ /** HTTP client that is expected to be defined in subclasses. */
+ private HttpClient client;
+
+ private List<RestNode> nodes;
+
+ @WorkDirectory
+ private Path workDir;
+
+ @BeforeEach
+ void setUp(TestInfo testInfo) {
+ client = HttpClient.newBuilder()
+ .build();
+
+ nodes = IntStream.range(0, 3)
+ .mapToObj(id -> {
+ return RestNode.builder()
+ .setWorkDir(workDir)
+ .setName(testNodeName(testInfo, id))
+ .setNetworkPort(3344 + id)
+ .setHttpPort(10300 + id)
+ .setHttpsPort(10400 + id)
+ .setSslEnabled(false)
+ .setDualProtocol(false)
+ .build();
+ })
+ .collect(Collectors.toList());
+
+ nodes.forEach(RestNode::start);
+ }
+
+ @Test
+ public void disabledAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + " \"" + metaStorageNode.name() + "\"\n"
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": false\n"
+ + " }\n"
+ + "}";
+
+ URI uri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(uri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ URI clusterConfigUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .build();
+
+ HttpResponse<String> httpResponse = client.send(clusterConfigRequest,
BodyHandlers.ofString());
+ assertEquals(200, httpResponse.statusCode());
+ }
+
+ @Test
+ public void basicAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String msNodes = nodes.stream()
+ .map(it -> it.name())
+ .map(name -> "\"" + name + "\"")
+ .collect(Collectors.joining(","));
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + msNodes
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": true,\n"
+ + " \"providers\": [\n"
+ + " {\n"
+ + " \"name\": \"basic\",\n"
+ + " \"type\": \"basic\",\n"
+ + " \"login\": \"admin\",\n"
+ + " \"password\": \"admin\"\n"
+ + " }\n"
+ + " ]\n"
+ + " }\n"
+ + " }";
+
+ URI clusterInitUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(clusterInitUri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri).build();
+ assertTrue(waitForCondition(() -> {
+ try {
+ return client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode() == 401;
+ } catch (Exception e) {
+ return false;
+ }
+ }, Duration.ofSeconds(5).toMillis()));
+ }
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .header("Authorization",
basicAuthenticationHeader("admin", "admin"))
+ .build();
+ assertEquals(200, client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode());
+ }
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .header("Authorization",
basicAuthenticationHeader("admin", "password"))
+ .build();
+ assertEquals(401, client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode());
+ }
+
+ String updateRestAuthConfigBody = "{\n"
+ + " \"rest\": {\n"
+ + " \"authConfiguration\": {\n"
+ + " \"enabled\": true,\n"
+ + " \"providers\": [\n"
+ + " {\n"
+ + " \"name\": \"basic\",\n"
+ + " \"type\": \"basic\",\n"
+ + " \"login\": \"admin\",\n"
+ + " \"password\": \"password\"\n"
+ + " }\n"
+ + " ]\n"
+ + " }\n"
+ + " }\n"
+ + "}";
+
+ URI updateClusterConfigUri = URI.create(metaStorageNode.httpAddress()
+ "/management/v1/configuration/cluster/");
+ HttpRequest updateClusterConfigRequest =
HttpRequest.newBuilder(updateClusterConfigUri)
+ .header("content-type", "text/plain")
+ .header("Authorization", basicAuthenticationHeader("admin",
"admin"))
+ .method("PATCH",
BodyPublishers.ofString(updateRestAuthConfigBody))
+ .build();
+
+ HttpResponse<String> updateClusterConfigResponse =
client.send(updateClusterConfigRequest, BodyHandlers.ofString());
+ assertThat(updateClusterConfigResponse.statusCode(), is(200));
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .header("Authorization",
basicAuthenticationHeader("admin", "admin"))
+ .build();
+
+ assertTrue(waitForCondition(() -> {
+ try {
+ return client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode() == 401;
+ } catch (Exception e) {
+ return false;
+ }
+ }, Duration.ofSeconds(5).toMillis()));
+ }
+
+ for (RestNode node : nodes) {
+ URI clusterConfigUri = URI.create(node.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .header("Authorization",
basicAuthenticationHeader("admin", "password"))
+ .build();
+ assertEquals(200, client.send(clusterConfigRequest,
BodyHandlers.ofString()).statusCode());
+ }
+ }
+
+ @AfterEach
+ void tearDown() {
+ nodes.forEach(RestNode::stop);
+ }
+
+ private static void checkAllNodesStarted(List<RestNode> nodes) {
Review Comment:
It seems that the main purpose of this method is to wait till all nodes
started, not just check that they are already started. I suggest to rename it,
like `waitForAllNodesStarted()`
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/rest/auth/ItAuthRestTest.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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.rest.auth;
+
+
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.testNodeName;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.rest.RestNode;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+
+/** Tests for the REST authentication configuration. */
+@ExtendWith(WorkDirectoryExtension.class)
+public class ItAuthRestTest {
+
+ /** HTTP client that is expected to be defined in subclasses. */
+ private HttpClient client;
+
+ private List<RestNode> nodes;
+
+ @WorkDirectory
+ private Path workDir;
+
+ @BeforeEach
+ void setUp(TestInfo testInfo) {
+ client = HttpClient.newBuilder()
+ .build();
+
+ nodes = IntStream.range(0, 3)
+ .mapToObj(id -> {
+ return RestNode.builder()
+ .setWorkDir(workDir)
+ .setName(testNodeName(testInfo, id))
+ .setNetworkPort(3344 + id)
+ .setHttpPort(10300 + id)
+ .setHttpsPort(10400 + id)
+ .setSslEnabled(false)
+ .setDualProtocol(false)
+ .build();
+ })
+ .collect(Collectors.toList());
+
+ nodes.forEach(RestNode::start);
+ }
+
+ @Test
+ public void disabledAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + " \"" + metaStorageNode.name() + "\"\n"
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": false\n"
+ + " }\n"
+ + "}";
+
+ URI uri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(uri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ URI clusterConfigUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/configuration/cluster/");
+ HttpRequest clusterConfigRequest =
HttpRequest.newBuilder(clusterConfigUri)
+ .build();
+
+ HttpResponse<String> httpResponse = client.send(clusterConfigRequest,
BodyHandlers.ofString());
+ assertEquals(200, httpResponse.statusCode());
+ }
+
+ @Test
+ public void basicAuthentication(TestInfo testInfo) throws IOException,
InterruptedException {
+ RestNode metaStorageNode = nodes.get(0);
+
+ String msNodes = nodes.stream()
+ .map(it -> it.name())
+ .map(name -> "\"" + name + "\"")
+ .collect(Collectors.joining(","));
+
+ String initClusterBody = "{\n"
+ + " \"metaStorageNodes\": [\n"
+ + msNodes
+ + " ],\n"
+ + " \"cmgNodes\": [],\n"
+ + " \"clusterName\": \"cluster\",\n"
+ + " \"authConfig\": {\n"
+ + " \"enabled\": true,\n"
+ + " \"providers\": [\n"
+ + " {\n"
+ + " \"name\": \"basic\",\n"
+ + " \"type\": \"basic\",\n"
+ + " \"login\": \"admin\",\n"
+ + " \"password\": \"admin\"\n"
+ + " }\n"
+ + " ]\n"
+ + " }\n"
+ + " }";
+
+ URI clusterInitUri = URI.create(metaStorageNode.httpAddress() +
"/management/v1/cluster/init");
+ HttpRequest initRequest = HttpRequest.newBuilder(clusterInitUri)
+ .header("content-type", "application/json")
+ .POST(BodyPublishers.ofString(initClusterBody))
+ .build();
+
+ HttpResponse<String> initResponse = client.send(initRequest,
BodyHandlers.ofString());
+ assertThat(initResponse.statusCode(), is(200));
+ checkAllNodesStarted(nodes);
+
+ for (RestNode node : nodes) {
Review Comment:
We should not have such long methods. It seems to perfectly break into
consecutive blocks like 'configure BASIC auth', 'wait till auth config is
applied', 'check with correct password', 'check with wrong password', etc.
Please extract the corresponding methods to make it easier to work with this
method.
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -499,16 +508,21 @@ public class IgniteImpl implements Ignite {
}
private RestComponent createRestComponent(String name) {
+ AuthConfiguration authConfiguration =
clusterCfgMgr.configurationRegistry()
Review Comment:
It would be nice to break this method into blocks using blank lines
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -263,6 +268,9 @@ public class IgniteImpl implements Ignite {
private final ScheduledExecutorService raftExecutorService;
+ /** CompletableFuture of {@link ClusterRestConfiguration} that will be
resolved after the cluster initialization. */
Review Comment:
It is not a `CompletableFuture`, it probably contains the future. Looks like
this comment needs to be refined.
##########
modules/cluster-management/build.gradle:
##########
@@ -35,6 +35,7 @@ dependencies {
implementation project(':ignite-raft-api')
implementation project(':ignite-vault')
implementation project(':ignite-rocksdb-common')
+ implementation project(':ignite-rest')
Review Comment:
Why is it needed here?
##########
modules/api/src/main/java/org/apache/ignite/Ignition.java:
##########
@@ -168,8 +170,15 @@ CompletableFuture<Ignite> start(
* @param metaStorageNodeNames Names of nodes that will host the Meta
Storage.
* @param cmgNodeNames Names of nodes that will host the CMG.
* @param clusterName Human-readable name of the cluster.
+ * @param restAuthConfig REST authentication configuration.
* @throws IgniteException If the given node has not been started or has
been stopped.
* @see <a
href="https://cwiki.apache.org/confluence/display/IGNITE/IEP-77%3A+Node+Join+Protocol+and+Initialization+for+Ignite+3";>IEP-77</a>
*/
- void init(String nodeName, Collection<String> metaStorageNodeNames,
Collection<String> cmgNodeNames, String clusterName);
+ void init(
+ String nodeName,
+ Collection<String> metaStorageNodeNames,
+ Collection<String> cmgNodeNames,
+ String clusterName,
+ RestAuthConfig restAuthConfig
Review Comment:
Shouldn't we have overloads without `RestAuthConfig` which init an insecured
cluster? This should be probably coordinated with the guys responsible for our
public API.
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/auth/Authenticator.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.rest.auth;
+
+import io.micronaut.security.authentication.AuthenticationRequest;
+import io.micronaut.security.authentication.AuthenticationResponse;
+
+interface Authenticator {
Review Comment:
The method still does not seem to have a javadoc on it. Am I looking in a
wrong direction?
--
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]
