kirklund commented on a change in pull request #6865:
URL: https://github.com/apache/geode/pull/6865#discussion_r709355045
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationFunctionDUnitTest.java
##########
@@ -65,77 +76,327 @@
return Arrays.asList(CURRENT_VERSION, RELEASE_VERSION);
}
- private MemberVM serverVM;
- private ClientVM clientVM;
+ private MemberVM serverVM0;
+ private MemberVM serverVM1;
+ private MemberVM serverVM2;
@Rule
- public ClusterStartupRule lsRule = new ClusterStartupRule();
+ public ClusterStartupRule lsRule = new ClusterStartupRule(4);
+
+ @Rule
+ public ClientCacheRule clientCacheRule = new ClientCacheRule();
@Before
- public void setup() throws Exception {
- Properties properties = new Properties();
- properties.setProperty(SECURITY_MANAGER,
ExpirableSecurityManager.class.getName());
- properties.setProperty(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER,
+ public void setup() {
+ MemberVM locatorVM =
+ lsRule.startLocatorVM(0, l ->
l.withSecurityManager(ExpirableSecurityManager.class));
+ int locatorPort = locatorVM.getPort();
+
+ Properties serverProperties = new Properties();
+ serverProperties.setProperty(SECURITY_MANAGER,
ExpirableSecurityManager.class.getName());
+
serverProperties.setProperty(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER,
Review comment:
I would go ahead and `import static` the `SERIALIZABLE_OBJECT_FILTER` to
match `SECURITY_MANAGER`. I generally use `import static` as much as possible
for any statics that read better using it and only skip doing this for anything
that reads more clearly without it.
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationFunctionDUnitTest.java
##########
@@ -65,77 +76,327 @@
return Arrays.asList(CURRENT_VERSION, RELEASE_VERSION);
}
- private MemberVM serverVM;
- private ClientVM clientVM;
+ private MemberVM serverVM0;
+ private MemberVM serverVM1;
+ private MemberVM serverVM2;
@Rule
- public ClusterStartupRule lsRule = new ClusterStartupRule();
+ public ClusterStartupRule lsRule = new ClusterStartupRule(4);
+
+ @Rule
+ public ClientCacheRule clientCacheRule = new ClientCacheRule();
@Before
- public void setup() throws Exception {
- Properties properties = new Properties();
- properties.setProperty(SECURITY_MANAGER,
ExpirableSecurityManager.class.getName());
- properties.setProperty(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER,
+ public void setup() {
+ MemberVM locatorVM =
+ lsRule.startLocatorVM(0, l ->
l.withSecurityManager(ExpirableSecurityManager.class));
+ int locatorPort = locatorVM.getPort();
+
+ Properties serverProperties = new Properties();
+ serverProperties.setProperty(SECURITY_MANAGER,
ExpirableSecurityManager.class.getName());
+
serverProperties.setProperty(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER,
"org.apache.geode.management.internal.security.TestFunctions*");
- serverVM = lsRule.startServerVM(0, properties);
+ serverProperties.setProperty(GROUPS, "group");
+ serverProperties.setProperty(USER_NAME, "test");
+ serverProperties.setProperty(PASSWORD, "test");
- serverVM.invoke(() -> {
+ serverVM0 = lsRule.startServerVM(1, serverProperties, locatorPort);
+ serverVM1 = lsRule.startServerVM(2, serverProperties, locatorPort);
+ serverVM2 = lsRule.startServerVM(3, serverProperties, locatorPort);
+
+ VMProvider.invokeInEveryMember(() -> {
Objects.requireNonNull(ClusterStartupRule.getCache())
.createRegionFactory(RegionShortcut.REPLICATE).create("region");
- });
- int serverPort = serverVM.getPort();
- clientVM = lsRule.startClientVM(1, clientVersion, c1 -> c1
+ Objects.requireNonNull(ClusterStartupRule.getCache())
+
.createRegionFactory(RegionShortcut.PARTITION).create("partitionRegion");
+ }, serverVM0, serverVM1, serverVM2);
+
+ VMProvider.invokeInEveryMember(() -> writeFunction = new
TestFunctions.WriteFunction(),
+ serverVM0, serverVM1, serverVM2);
+
+ clientCacheRule
.withProperty(SECURITY_CLIENT_AUTH_INIT,
UpdatableUserAuthInitialize.class.getName())
.withPoolSubscription(true)
- .withServerConnection(serverPort));
+ .withLocatorConnection(locatorPort);
+ }
- VMProvider.invokeInEveryMember(() -> writeFunction = new
TestFunctions.WriteFunction(),
- serverVM, clientVM);
+ @Test
+ public void
clientShouldReAuthenticateWhenCredentialExpiredAndFunctionExecutionOnServerSucceed()
+ throws Exception {
+ ClientCache clientCache = clientCacheRule.createCache();
+ UpdatableUserAuthInitialize.setUser("data1");
+ writeFunction = new TestFunctions.WriteFunction();
+
+ ResultCollector rc =
onServer(clientCache.getDefaultPool()).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
+
+ // expire the current user
+ VMProvider.invokeInEveryMember(() ->
getSecurityManager().addExpiredUser("data1"),
+ serverVM0, serverVM1, serverVM2);
+
+ // do a second function execution, if this is successful, it means new
credentials are provided
+ UpdatableUserAuthInitialize.setUser("data2");
+ rc = onServer(clientCache.getDefaultPool()).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
+
+ // all function invocation authorizations are recorded
+ List<Object> resultsVM0 = collectSecurityManagerResults(serverVM0);
+ List<Object> resultsVM1 = collectSecurityManagerResults(serverVM1);
+ List<Object> resultsVM2 = collectSecurityManagerResults(serverVM2);
+
+ Set<String> combinedExpiredUsers = combineExpiredUsers(resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(combinedExpiredUsers.size()).isEqualTo(1);
+ assertThat(combinedExpiredUsers.contains("data1")).isTrue();
+ Map<String, List<String>> authorizedOps = collectVMOps(1, resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(authorizedOps.get("data1")).asList().hasSize(1);
+
assertThat(authorizedOps.get("data1")).asList().containsExactly("DATA:WRITE");
+ assertThat(authorizedOps.get("data2")).asList().hasSize(1);
+
assertThat(authorizedOps.get("data2")).asList().containsExactly("DATA:WRITE");
+
+ Map<String, List<String>> unauthorizedOps = collectVMOps(2, resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(unauthorizedOps.get("data1")).asList().hasSize(1);
+
assertThat(unauthorizedOps.get("data1")).asList().containsExactly("DATA:WRITE");
}
@Test
- public void
clientShouldReAuthenticateWhenCredentialExpiredAndFunctionExecutionSucceed() {
- clientVM.invoke(() -> {
- ClientCache clientCache = ClusterStartupRule.getClientCache();
- assertThat(clientCache).isNotNull();
- UpdatableUserAuthInitialize.setUser("data1");
- ResultCollector rc =
onServer(clientCache.getDefaultPool()).execute(writeFunction);
- assertThat(((ArrayList) rc.getResult()).get(0))
- .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
- });
+ public void
clientShouldReAuthenticateWhenCredentialExpiredAndFunctionExecutionOnServersSucceed()
+ throws Exception {
+ ClientCache clientCache = clientCacheRule.createCache();
+ UpdatableUserAuthInitialize.setUser("data1");
+ writeFunction = new TestFunctions.WriteFunction();
+
+ ResultCollector rc =
onServers(clientCache.getDefaultPool()).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
// expire the current user
- serverVM.invoke(() -> getSecurityManager().addExpiredUser("data1"));
+ VMProvider.invokeInEveryMember(() ->
getSecurityManager().addExpiredUser("data1"),
+ serverVM0, serverVM1, serverVM2);
// do a second function execution, if this is successful, it means new
credentials are provided
- clientVM.invoke(() -> {
- ClientCache clientCache = ClusterStartupRule.getClientCache();
- assertThat(clientCache).isNotNull();
- UpdatableUserAuthInitialize.setUser("data2");
- ResultCollector rc =
onServer(clientCache.getDefaultPool()).execute(writeFunction);
- assertThat(((ArrayList) rc.getResult()).get(0))
- .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
- });
+ UpdatableUserAuthInitialize.setUser("data2");
+ rc = onServers(clientCache.getDefaultPool()).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
+
+ // all function invocation authorizations are recorded
+ List<Object> resultsVM0 = collectSecurityManagerResults(serverVM0);
+ List<Object> resultsVM1 = collectSecurityManagerResults(serverVM1);
+ List<Object> resultsVM2 = collectSecurityManagerResults(serverVM2);
+
+ Set<String> combinedExpiredUsers = combineExpiredUsers(resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(combinedExpiredUsers.size()).isEqualTo(1);
+ assertThat(combinedExpiredUsers.contains("data1")).isTrue();
+ Map<String, List<String>> authorizedOps = collectVMOps(1, resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(authorizedOps.get("data1")).asList().hasSize(3);
+
assertThat(authorizedOps.get("data1")).asList().containsExactly("DATA:WRITE",
"DATA:WRITE",
+ "DATA:WRITE");
+ assertThat(authorizedOps.get("data2")).asList().hasSize(3);
+
assertThat(authorizedOps.get("data2")).asList().containsExactly("DATA:WRITE",
"DATA:WRITE",
+ "DATA:WRITE");
+
+ Map<String, List<String>> unauthorizedOps = collectVMOps(2, resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(unauthorizedOps.get("data1")).asList().hasSize(3);
+
assertThat(unauthorizedOps.get("data1")).asList().containsExactly("DATA:WRITE",
"DATA:WRITE",
+ "DATA:WRITE");
+ }
+
+ @Test
+ public void
clientShouldReAuthenticateWhenCredentialExpiredAndFunctionExecutionOnRegionSucceed()
+ throws Exception {
+ ClientCache clientCache = clientCacheRule.createCache();
+ UpdatableUserAuthInitialize.setUser("data1");
+ Region<Object, Object> region =
+
clientCache.createClientRegionFactory(ClientRegionShortcut.PROXY).create("region");
+ writeFunction = new TestFunctions.WriteFunction();
+
+ ResultCollector rc = onRegion(region).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
+
+ // expire the current user
+ VMProvider.invokeInEveryMember(() ->
getSecurityManager().addExpiredUser("data1"),
+ serverVM0, serverVM1, serverVM2);
+
+ // do a second function execution, if this is successful, it means new
credentials are provided
+ UpdatableUserAuthInitialize.setUser("data2");
+ rc = onRegion(region).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
+
+ // all function invocation authorizations are recorded
+ List<Object> resultsVM0 = collectSecurityManagerResults(serverVM0);
+ List<Object> resultsVM1 = collectSecurityManagerResults(serverVM1);
+ List<Object> resultsVM2 = collectSecurityManagerResults(serverVM2);
- // all put operation succeeded
- serverVM.invoke(() -> {
+ Set<String> combinedExpiredUsers = combineExpiredUsers(resultsVM0,
resultsVM1, resultsVM2);
+
+ assertThat(combinedExpiredUsers.size()).isEqualTo(1);
+ assertThat(combinedExpiredUsers.contains("data1")).isTrue();
Review comment:
These two assertions are used in several places. It would be better if
you use the `Collection` assertion everywhere instead of the `boolean`
assertion:
```
assertThat(combinedExpiredUsers).hasSize(1);
assertThat(combinedExpiredUsers).contains("data1");
```
Or:
```
assertThat(combinedExpiredUsers)
.hasSize(1)
.contains("data1");
```
Or:
```
assertThat(combinedExpiredUsers).containsExactly("data1");
```
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationFunctionDUnitTest.java
##########
@@ -65,77 +76,327 @@
return Arrays.asList(CURRENT_VERSION, RELEASE_VERSION);
}
- private MemberVM serverVM;
- private ClientVM clientVM;
+ private MemberVM serverVM0;
+ private MemberVM serverVM1;
+ private MemberVM serverVM2;
@Rule
- public ClusterStartupRule lsRule = new ClusterStartupRule();
+ public ClusterStartupRule lsRule = new ClusterStartupRule(4);
Review comment:
Not sure what `ls` refers to in `lsRule`. Maybe rename it to
`clusterRule`.
##########
File path:
geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationFunctionDUnitTest.java
##########
@@ -65,77 +76,327 @@
return Arrays.asList(CURRENT_VERSION, RELEASE_VERSION);
}
- private MemberVM serverVM;
- private ClientVM clientVM;
+ private MemberVM serverVM0;
+ private MemberVM serverVM1;
+ private MemberVM serverVM2;
@Rule
- public ClusterStartupRule lsRule = new ClusterStartupRule();
+ public ClusterStartupRule lsRule = new ClusterStartupRule(4);
+
+ @Rule
+ public ClientCacheRule clientCacheRule = new ClientCacheRule();
@Before
- public void setup() throws Exception {
- Properties properties = new Properties();
- properties.setProperty(SECURITY_MANAGER,
ExpirableSecurityManager.class.getName());
- properties.setProperty(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER,
+ public void setup() {
+ MemberVM locatorVM =
+ lsRule.startLocatorVM(0, l ->
l.withSecurityManager(ExpirableSecurityManager.class));
+ int locatorPort = locatorVM.getPort();
+
+ Properties serverProperties = new Properties();
+ serverProperties.setProperty(SECURITY_MANAGER,
ExpirableSecurityManager.class.getName());
+
serverProperties.setProperty(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER,
"org.apache.geode.management.internal.security.TestFunctions*");
- serverVM = lsRule.startServerVM(0, properties);
+ serverProperties.setProperty(GROUPS, "group");
+ serverProperties.setProperty(USER_NAME, "test");
+ serverProperties.setProperty(PASSWORD, "test");
- serverVM.invoke(() -> {
+ serverVM0 = lsRule.startServerVM(1, serverProperties, locatorPort);
+ serverVM1 = lsRule.startServerVM(2, serverProperties, locatorPort);
+ serverVM2 = lsRule.startServerVM(3, serverProperties, locatorPort);
+
+ VMProvider.invokeInEveryMember(() -> {
Objects.requireNonNull(ClusterStartupRule.getCache())
.createRegionFactory(RegionShortcut.REPLICATE).create("region");
- });
- int serverPort = serverVM.getPort();
- clientVM = lsRule.startClientVM(1, clientVersion, c1 -> c1
+ Objects.requireNonNull(ClusterStartupRule.getCache())
+
.createRegionFactory(RegionShortcut.PARTITION).create("partitionRegion");
+ }, serverVM0, serverVM1, serverVM2);
+
+ VMProvider.invokeInEveryMember(() -> writeFunction = new
TestFunctions.WriteFunction(),
+ serverVM0, serverVM1, serverVM2);
+
+ clientCacheRule
.withProperty(SECURITY_CLIENT_AUTH_INIT,
UpdatableUserAuthInitialize.class.getName())
.withPoolSubscription(true)
- .withServerConnection(serverPort));
+ .withLocatorConnection(locatorPort);
+ }
- VMProvider.invokeInEveryMember(() -> writeFunction = new
TestFunctions.WriteFunction(),
- serverVM, clientVM);
+ @Test
+ public void
clientShouldReAuthenticateWhenCredentialExpiredAndFunctionExecutionOnServerSucceed()
+ throws Exception {
+ ClientCache clientCache = clientCacheRule.createCache();
+ UpdatableUserAuthInitialize.setUser("data1");
+ writeFunction = new TestFunctions.WriteFunction();
+
+ ResultCollector rc =
onServer(clientCache.getDefaultPool()).execute(writeFunction);
+ assertThat(((ArrayList) rc.getResult()).get(0))
+ .isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
Review comment:
You should always try to cast to an interface rather than a concrete
implementation:
```
List<?> result = (List) rc.getResult();
assertThat(result.get(0))
.isEqualTo(TestFunctions.WriteFunction.SUCCESS_OUTPUT);
```
--
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]
