Skip to content

Commit

Permalink
GEODE-7497: Check CQs prior to change authorizer (apache#4385)
Browse files Browse the repository at this point in the history 
The QueryConfigurationService now requires an extra flag to determine,
in the presence of continuous queries, whether to update the configured
MethodInvocationAuthorizer and invalidate the CQ's internal cache or
throw an exception and abort the update.

- Fixed minor warnings.
- Added unit and distributed tests.
- Updated docs for alter query-service command.
- Added 'forceUpdate' flag to 'AlterQueryServiceFunction' and
  'AlterQueryServiceCommand'.
- Fixed 'AlterQueryServiceCommand' to always require the
  'method-authorizer' parameter.
  • Loading branch information
@jujoramos
jujoramos authored Dec 4, 2019
1 parent 9929ed4 commit 6ebc419
Show file tree
Hide file tree
Showing 16 changed files with 728 additions and 462 deletions.
184 changes: 85 additions & 99 deletions ...geode/management/internal/cli/commands/AlterQueryServiceCommandWithSecurityDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@

package org.apache.geode.management.internal.cli.commands;

import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.AUTHORIZER_NAME;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.AUTHORIZER_PARAMETERS;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.COMMAND_NAME;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.METHOD_AUTHORIZER_NAME;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.NO_MEMBERS_FOUND_MESSAGE;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.SPLITTING_REGEX;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -61,81 +61,121 @@
public class AlterQueryServiceCommandWithSecurityDUnitTest {
private static final Class<RestrictedMethodAuthorizer> DEFAULT_AUTHORIZER_CLASS =
RestrictedMethodAuthorizer.class;
private static final String JAVA_BEAN_AUTHORIZER_PARAMS = "java.lang;java.util";
// A regex containing a comma to confirm that the '--authorizer-parameter' option is parsed
// correctly
private static final String TEST_AUTHORIZER_TXT = "TestMethodAuthorizer.txt";
private static final String USER_AUTHORIZER_PARAMETERS = "param1;param2;param3";
private static final String JAVA_BEAN_AUTHORIZER_PARAMETERS = "java.lang;java.util";
// A regex containing a comma to confirm that '--authorizer-parameter' is correctly parsed
private static final String REGEX_AUTHORIZER_PARAMETERS =
"^java.util.List..{4,8}$;^java.util.Set..{4,8}$";
private static final String USER_AUTHORIZER_PARAMETERS = "param1;param2;param3";
private static final String TEST_AUTHORIZER_TXT = "TestMethodAuthorizer.txt";
private final int serversToStart = 2;
private MemberVM locator;
private List<MemberVM> servers = new ArrayList<>();

@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();
public GfshCommandRule gfsh = new GfshCommandRule();

@Rule
public ClusterStartupRule cluster = new ClusterStartupRule();
public TemporaryFolder tempFolder = new TemporaryFolder();

@Rule
public GfshCommandRule gfsh = new GfshCommandRule();

private static MemberVM locator;
private static List<MemberVM> servers = new ArrayList<>();
public ClusterStartupRule cluster = new ClusterStartupRule();

@Before
public void setUp() throws Exception {
locator = cluster.startLocatorVM(0, l -> l.withSecurityManager(SimpleSecurityManager.class));
int locatorPort = locator.getPort();
IntStream.range(0, serversToStart).forEach(
i -> servers.add(i, cluster.startServerVM(i + 1, s -> s.withConnectionToLocator(locatorPort)
.withCredential("clusterManage", "clusterManage"))));
IntStream.range(0, 2).forEach(i -> servers.add(i, cluster.startServerVM(i + 1, s -> s
.withConnectionToLocator(locatorPort).withCredential("clusterManage", "clusterManage"))));
gfsh.connectAndVerify(locator);
}

private String buildCommand(String authorizerName, String authorizerParameters) {
StringBuilder commandBuilder = new StringBuilder(COMMAND_NAME);
commandBuilder.append(" --").append(AUTHORIZER_NAME).append("=").append(authorizerName);

if (authorizerParameters != null) {
commandBuilder.append(" --").append(AUTHORIZER_PARAMETERS).append("=")
.append(authorizerParameters);
}

return commandBuilder.toString();
}

private void executeAndAssertGfshCommandIsSuccessful(
Class<? extends MethodInvocationAuthorizer> authorizerClass,
String authorizerParameters) {
gfsh.executeAndAssertThat(buildCommand(authorizerClass.getName(), authorizerParameters))
.statusIsSuccess().hasTableSection().hasRowSize(servers.size());

verifyCurrentAuthorizerClass(authorizerClass);
}

private void startServerAndVerifyJavaBeanAccessorMethodAuthorizer() {
int locatorPort = locator.getPort();
MemberVM newServer = cluster.startServerVM(servers.size() + 1, s -> s
.withConnectionToLocator(locatorPort).withCredential("clusterManage", "clusterManage"));
newServer.invoke(() -> {
JavaBeanAccessorMethodAuthorizer methodAuthorizer =
(JavaBeanAccessorMethodAuthorizer) Objects.requireNonNull(ClusterStartupRule.getCache())
.getService(QueryConfigurationService.class).getMethodAuthorizer();
assertThat(methodAuthorizer.getClass()).isEqualTo(JavaBeanAccessorMethodAuthorizer.class);
Set<String> expectedParameters =
new HashSet<>(Arrays.asList(JAVA_BEAN_AUTHORIZER_PARAMETERS.split(SPLITTING_REGEX)));
assertThat(methodAuthorizer.getAllowedPackages()).isEqualTo(expectedParameters);
});
}

private void verifyCurrentAuthorizerClass(
Class<? extends MethodInvocationAuthorizer> authorizerClass) {
servers.forEach(server -> server.invoke(() -> {
MethodInvocationAuthorizer methodAuthorizer =
Objects.requireNonNull(ClusterStartupRule.getCache())
.getService(QueryConfigurationService.class).getMethodAuthorizer();
assertThat(methodAuthorizer.getClass()).isEqualTo(authorizerClass);
}));
}

private String getTestMethodAuthorizerFilePath() throws IOException {
URL url = getClass().getResource(TEST_AUTHORIZER_TXT);
File textFile = this.tempFolder.newFile(TEST_AUTHORIZER_TXT);
FileUtils.copyURLToFile(url, textFile);

return textFile.getAbsolutePath();
}

// This test verifies that the cluster starts with the default method authorizer configured, then
// changes the currently configured authorizer to each of the out-of-the-box authorizers.
@Test
public void alterQueryServiceCommandUpdatesMethodAuthorizerClass() {
verifyCurrentAuthorizerClass(DEFAULT_AUTHORIZER_CLASS);

executeAndAssertGfshCommandIsSuccessful(UnrestrictedMethodAuthorizer.class, "");

executeAndAssertGfshCommandIsSuccessful(JavaBeanAccessorMethodAuthorizer.class,
JAVA_BEAN_AUTHORIZER_PARAMS);

JAVA_BEAN_AUTHORIZER_PARAMETERS);
executeAndAssertGfshCommandIsSuccessful(RegExMethodAuthorizer.class,
REGEX_AUTHORIZER_PARAMETERS);

executeAndAssertGfshCommandIsSuccessful(RestrictedMethodAuthorizer.class, "");
}

@Test
public void alterQueryServiceCommandUpdatesMethodAuthorizerParameters() {
gfsh.executeAndAssertThat(COMMAND_NAME + " --" + METHOD_AUTHORIZER_NAME + "="
+ JavaBeanAccessorMethodAuthorizer.class.getName() + " --" + AUTHORIZER_PARAMETERS + "="
+ JAVA_BEAN_AUTHORIZER_PARAMS).statusIsSuccess().hasTableSection()
.hasRowSize(serversToStart);

gfsh.executeAndAssertThat(buildCommand(JavaBeanAccessorMethodAuthorizer.class.getName(),
JAVA_BEAN_AUTHORIZER_PARAMETERS)).statusIsSuccess().hasTableSection()
.hasRowSize(servers.size());
servers.forEach(server -> server.invoke(() -> {
JavaBeanAccessorMethodAuthorizer methodAuthorizer = (JavaBeanAccessorMethodAuthorizer) Objects
.requireNonNull(ClusterStartupRule.getCache()).getService(QueryConfigurationService.class)
.getMethodAuthorizer();

JavaBeanAccessorMethodAuthorizer methodAuthorizer =
(JavaBeanAccessorMethodAuthorizer) Objects.requireNonNull(ClusterStartupRule.getCache())
.getService(QueryConfigurationService.class).getMethodAuthorizer();
Set<String> expectedParameters =
new HashSet<>(Arrays.asList(JAVA_BEAN_AUTHORIZER_PARAMS.split(SPLITTING_REGEX)));
new HashSet<>(Arrays.asList(JAVA_BEAN_AUTHORIZER_PARAMETERS.split(SPLITTING_REGEX)));
assertThat(methodAuthorizer.getAllowedPackages()).isEqualTo(expectedParameters);
}));

gfsh.executeAndAssertThat(
COMMAND_NAME + " --" + METHOD_AUTHORIZER_NAME + "=" + RegExMethodAuthorizer.class.getName()
+ " --" + AUTHORIZER_PARAMETERS + "=" + REGEX_AUTHORIZER_PARAMETERS)
.statusIsSuccess().hasTableSection().hasRowSize(serversToStart);

buildCommand(RegExMethodAuthorizer.class.getName(), REGEX_AUTHORIZER_PARAMETERS))
.statusIsSuccess().hasTableSection().hasRowSize(servers.size());
servers.forEach(server -> server.invoke(() -> {
RegExMethodAuthorizer methodAuthorizer = (RegExMethodAuthorizer) Objects
.requireNonNull(ClusterStartupRule.getCache()).getService(QueryConfigurationService.class)
.getMethodAuthorizer();

RegExMethodAuthorizer methodAuthorizer =
(RegExMethodAuthorizer) Objects.requireNonNull(ClusterStartupRule.getCache())
.getService(QueryConfigurationService.class).getMethodAuthorizer();
Set<String> expectedParameters =
new HashSet<>(Arrays.asList(REGEX_AUTHORIZER_PARAMETERS.split(SPLITTING_REGEX)));
assertThat(methodAuthorizer.getAllowedPatterns()).isEqualTo(expectedParameters);
Expand All @@ -146,25 +186,20 @@ public void alterQueryServiceCommandUpdatesMethodAuthorizerParameters() {
public void alterQueryServiceCommandWithUserSpecifiedMethodAuthorizerUpdatesMethodAuthorizer()
throws IOException {
verifyCurrentAuthorizerClass(DEFAULT_AUTHORIZER_CLASS);

Class<TestMethodAuthorizer> authorizerClass = TestMethodAuthorizer.class;
String authorizerName = authorizerClass.getName();

String classContent =
new String(Files.readAllBytes(Paths.get(getFilePath(TEST_AUTHORIZER_TXT))));
new String(Files.readAllBytes(Paths.get(getTestMethodAuthorizerFilePath())));
String jarFileName = "testJar.jar";
File jarFile = tempFolder.newFile(jarFileName);
new ClassBuilder().writeJarFromContent(authorizerName, classContent, jarFile);

gfsh.executeAndAssertThat("deploy --jars=" + jarFile.getAbsolutePath()).statusIsSuccess();

executeAndAssertGfshCommandIsSuccessful(TestMethodAuthorizer.class, USER_AUTHORIZER_PARAMETERS);

servers.forEach(server -> server.invoke(() -> {
TestMethodAuthorizer methodAuthorizer = (TestMethodAuthorizer) Objects
.requireNonNull(ClusterStartupRule.getCache()).getService(QueryConfigurationService.class)
.getMethodAuthorizer();

TestMethodAuthorizer methodAuthorizer =
(TestMethodAuthorizer) Objects.requireNonNull(ClusterStartupRule.getCache())
.getService(QueryConfigurationService.class).getMethodAuthorizer();
Set<String> expectedParameters =
new HashSet<>(Arrays.asList(USER_AUTHORIZER_PARAMETERS.split(SPLITTING_REGEX)));
assertThat(methodAuthorizer.getParameters()).isEqualTo(expectedParameters);
Expand All @@ -174,71 +209,22 @@ public void alterQueryServiceCommandWithUserSpecifiedMethodAuthorizerUpdatesMeth
@Test
public void alterQueryServiceCommandUpdatesClusterConfigWhenNoMembersAreFound() {
servers.forEach(VMProvider::stop);

Class<JavaBeanAccessorMethodAuthorizer> authorizerClass =
JavaBeanAccessorMethodAuthorizer.class;
String authorizerName = authorizerClass.getName();

gfsh.executeAndAssertThat(COMMAND_NAME + " --" + METHOD_AUTHORIZER_NAME + "=" + authorizerName
+ " --" + AUTHORIZER_PARAMETERS + "=" + JAVA_BEAN_AUTHORIZER_PARAMS)
gfsh.executeAndAssertThat(buildCommand(authorizerName, JAVA_BEAN_AUTHORIZER_PARAMETERS))
.statusIsSuccess().hasInfoSection().hasLines().contains(NO_MEMBERS_FOUND_MESSAGE);

startServerAndVerifyJavaBeanAccessorMethodAuthorizer();
}

@Test
public void alterQueryServiceCommandDoesNotUpdateClusterConfigWhenFunctionExecutionFailsOnAllMembers() {
String authorizerName = "badAuthorizerName";

gfsh.executeAndAssertThat(COMMAND_NAME + " --" + METHOD_AUTHORIZER_NAME + "=" + authorizerName
+ " --" + AUTHORIZER_PARAMETERS + "=" + JAVA_BEAN_AUTHORIZER_PARAMS)
gfsh.executeAndAssertThat(buildCommand(authorizerName, JAVA_BEAN_AUTHORIZER_PARAMETERS))
.statusIsError();

locator.invoke(() -> assertThat(Objects.requireNonNull(ClusterStartupRule.getLocator())
.getConfigurationPersistenceService().getCacheConfig(null)).isNull());
}

private void executeAndAssertGfshCommandIsSuccessful(Class authorizerClass,
String authorizerParameters) {
gfsh.executeAndAssertThat(
COMMAND_NAME + " --" + METHOD_AUTHORIZER_NAME + "=" + authorizerClass.getName()
+ " --" + AUTHORIZER_PARAMETERS + "=" + authorizerParameters)
.statusIsSuccess()
.hasTableSection().hasRowSize(serversToStart);

verifyCurrentAuthorizerClass(authorizerClass);
}

private void startServerAndVerifyJavaBeanAccessorMethodAuthorizer() {
int locatorPort = locator.getPort();
MemberVM newServer = cluster.startServerVM(servers.size() + 1, s -> s
.withConnectionToLocator(locatorPort).withCredential("clusterManage", "clusterManage"));
newServer.invoke(() -> {
JavaBeanAccessorMethodAuthorizer methodAuthorizer =
(JavaBeanAccessorMethodAuthorizer) Objects.requireNonNull(ClusterStartupRule.getCache())
.getService(QueryConfigurationService.class).getMethodAuthorizer();
assertThat(methodAuthorizer.getClass()).isEqualTo(JavaBeanAccessorMethodAuthorizer.class);

Set<String> expectedParameters =
new HashSet<>(Arrays.asList(JAVA_BEAN_AUTHORIZER_PARAMS.split(SPLITTING_REGEX)));
assertThat(methodAuthorizer.getAllowedPackages()).isEqualTo(expectedParameters);
});
}

private void verifyCurrentAuthorizerClass(Class authorizerClass) {
servers.forEach(server -> server.invoke(() -> {
MethodInvocationAuthorizer methodAuthorizer = Objects
.requireNonNull(ClusterStartupRule.getCache()).getService(QueryConfigurationService.class)
.getMethodAuthorizer();
assertThat(methodAuthorizer.getClass()).isEqualTo(authorizerClass);
}));
}

private String getFilePath(String fileName) throws IOException {
URL url = getClass().getResource(fileName);
File textFile = this.tempFolder.newFile(fileName);
FileUtils.copyURLToFile(url, textFile);

return textFile.getAbsolutePath();
}
}
36 changes: 16 additions & 20 deletions ...de/management/internal/cli/commands/AlterQueryServiceCommandWithoutSecurityDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
*/
package org.apache.geode.management.internal.cli.commands;

import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.AUTHORIZER_NAME;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.COMMAND_NAME;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.METHOD_AUTHORIZER_NAME;
import static org.apache.geode.management.internal.cli.functions.AlterQueryServiceFunction.SECURITY_NOT_ENABLED_MESSAGE;
import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -37,36 +37,23 @@
import org.apache.geode.test.junit.rules.GfshCommandRule;

public class AlterQueryServiceCommandWithoutSecurityDUnitTest {
private final int serversToStart = 2;

@Rule
public ClusterStartupRule cluster = new ClusterStartupRule();
private static MemberVM locator;
private static List<MemberVM> servers = new ArrayList<>();

@Rule
public GfshCommandRule gfsh = new GfshCommandRule();

private static MemberVM locator;
private static List<MemberVM> servers = new ArrayList<>();
@Rule
public ClusterStartupRule cluster = new ClusterStartupRule();

@Before
public void setUp() throws Exception {
locator = cluster.startLocatorVM(0);
IntStream.range(0, serversToStart).forEach(
i -> servers.add(i, cluster.startServerVM(i + 1, locator.getPort())));
IntStream.range(0, 2)
.forEach(i -> servers.add(i, cluster.startServerVM(i + 1, locator.getPort())));
gfsh.connectAndVerify(locator);
}

@Test
public void alterQueryServiceCommandDoesNotUpdateMethodAuthorizerWhenSecurityIsNotEnabled() {
verifyNoOpAuthorizer();

String authorizerName = UnrestrictedMethodAuthorizer.class.getName();
gfsh.executeAndAssertThat(COMMAND_NAME + " --" + METHOD_AUTHORIZER_NAME + "=" + authorizerName)
.statusIsError().containsOutput(SECURITY_NOT_ENABLED_MESSAGE);

verifyNoOpAuthorizer();
}

private void verifyNoOpAuthorizer() {
servers.forEach(s -> s.invoke(() -> {
MethodInvocationAuthorizer methodAuthorizer = Objects
Expand All @@ -76,4 +63,13 @@ private void verifyNoOpAuthorizer() {
}));
}

@Test
public void alterQueryServiceCommandDoesNotUpdateMethodAuthorizerWhenSecurityIsNotEnabled() {
verifyNoOpAuthorizer();
String authorizerName = UnrestrictedMethodAuthorizer.class.getName();

gfsh.executeAndAssertThat(COMMAND_NAME + " --" + AUTHORIZER_NAME + "=" + authorizerName)
.statusIsError().containsOutput(SECURITY_NOT_ENABLED_MESSAGE);
verifyNoOpAuthorizer();
}
}
4 changes: 2 additions & 2 deletions ...g/apache/geode/management/internal/cli/commands/DescribeQueryServiceCommandDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
*/
package org.apache.geode.management.internal.cli.commands;

import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.AUTHORIZER_NAME;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.AUTHORIZER_PARAMETERS;
import static org.apache.geode.management.internal.cli.commands.AlterQueryServiceCommand.METHOD_AUTHORIZER_NAME;
import static org.apache.geode.management.internal.cli.commands.DescribeQueryServiceCommand.ALL_METHODS_ALLOWED;
import static org.apache.geode.management.internal.cli.commands.DescribeQueryServiceCommand.AUTHORIZER_CLASS_NAME;
import static org.apache.geode.management.internal.cli.commands.DescribeQueryServiceCommand.COMMAND_NAME;
Expand Down Expand Up @@ -92,7 +92,7 @@ private void changeMethodAuthorizerAndValidateDescribe() {
String authorizerName = JavaBeanAccessorMethodAuthorizer.class.getName();
String parameters = "param1;param2;param3";
String alterQueryService = AlterQueryServiceCommand.COMMAND_NAME + " --"
+ METHOD_AUTHORIZER_NAME + "=" + authorizerName
+ AUTHORIZER_NAME + "=" + authorizerName
+ " --" + AUTHORIZER_PARAMETERS + "=" + parameters;

gfsh.executeAndAssertThat(alterQueryService).statusIsSuccess();
Expand Down
2 changes: 1 addition & 1 deletion ...nTest/java/org/apache/geode/cache/query/internal/DefaultQuerySecurityIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public void setUp() throws ClassNotFoundException {

spiedCache = spy(server.getCache());
QueryConfigurationService queryConfig = spiedCache.getService(QueryConfigurationService.class);
queryConfig.updateMethodAuthorizer(spiedCache, SpyAuthorizer.class.getName(),
queryConfig.updateMethodAuthorizer(spiedCache, false, SpyAuthorizer.class.getName(),
Collections.emptySet());
}

Expand Down
6 changes: 3 additions & 3 deletions ...onTest/java/org/apache/geode/cache/query/internal/DefaultQueryServiceDeprecationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
package org.apache.geode.cache.query.internal;

import static org.apache.geode.distributed.internal.DistributionConfig.GEMFIRE_PREFIX;
import static org.apache.geode.cache.query.internal.QueryConfigurationServiceImpl.ALLOW_UNTRUSTED_METHOD_INVOCATION_SYSTEM_PROPERTY;
import static org.apache.geode.distributed.internal.DistributionConfig.LOG_FILE_NAME;

import java.io.File;
Expand All @@ -38,10 +38,10 @@ public class DefaultQueryServiceDeprecationTest implements Serializable {
public ServerStarterRule server = new ServerStarterRule();

@Test
@SuppressWarnings("deprecation")
public void warningMessageIsOnlyLoggedOnceWhenDeprecatedPropertyUsed() throws IOException {
File logFile = folderRule.newFile("customLog1.log");
System.setProperty(GEMFIRE_PREFIX + "QueryService.allowUntrustedMethodInvocation",
"true");
System.setProperty(ALLOW_UNTRUSTED_METHOD_INVOCATION_SYSTEM_PROPERTY, "true");
server.withProperty(LOG_FILE_NAME, logFile.getAbsolutePath()).startServer();
server.getCache().getQueryService();
server.getCache().getQueryService();
Expand Down
Loading

0 comments on commit 6ebc419

Please sign in to comment.