----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52889/#review152694 -----------------------------------------------------------
Many usages of ServerStarter have to specify "org/apache/geode/management/internal/security/clientServer.json". I think it would be convenient to have some static factory methods like ServerStarter.startWithSecurity() that specify a default set of Properties like SampleSecurityManager and "org/apache/geode/management/internal/security/clientServer.json". The same addition would be useful in CacheServerStartupRule. It would be convenient to not have to pass in "org/apache/geode/management/internal/security/cacheServer.json". RestSecurityService has authorize methods that show up as unused in Intellij. I know from looking at the other changes that they are used by @PreAuthorized annotations in the CrudControllers, but it might be nice to add comments to these methods explaining where they're used so that people in the future don't think they can be deleted. - Jared Stewart On Oct. 14, 2016, 4:19 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52889/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2016, 4:19 p.m.) > > > Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund. > > > Repository: geode > > > Description > ------- > > * created ServerStarter and LocatorStarter in the rule package > * refacterred LocatorServerConfigurationRule > * refactor tests to use these rules > > > Diffs > ----- > > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java > 59e00c8bd0a7f2759f579abf99a87fcd98b42a06 > > geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java > 79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 > > geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java > c22fff3655131cf908a54289b66faf84311c5c69 > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java > 388094840fc587e662d231783fd5c7c8faf7b485 > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java > 7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java > 721a4312456e0f1eaf41a6c1f41016cfe56450e4 > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java > 84155a9d82292dc70c6412908a1b57a09c1aea98 > > geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java > 0084cb8f5471ce7ecab086c955a842766d6ed790 > > geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java > 750ce2ac76ecd0860f5f678a22615697c9ea5009 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java > b64a6f7b1c63eb41f5823b4971d5041431748db9 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java > 9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java > 34fd5a994c5a69d64398de3cb867892a30827e2c > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java > def17920b198e88f358dfcf4025f4a3eee6fd0df > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java > 4d1bae904c4964ca2be713fd8450f2f5f6db2552 > > geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java > 4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 > > geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java > c544e6fe0f8d3c1975775979d1354c4adc7cf87a > > geode-core/src/test/java/org/apache/geode/management/internal/security/JsonAuthorizationCacheStartRule.java > 136319c06d9bd1e46b452219bf2894cf969fc1f1 > > geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java > 1377fb643d035ad911ee5ceb80b87dfd72855428 > > geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java > 4beff0bd55d69601266a0a856c8dcb80ffd4e1f7 > > geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanServerConnectionRule.java > 92430327e999af627e88956fc24aa1592e9a000d > > geode-core/src/test/java/org/apache/geode/management/internal/security/ManagerMBeanAuthorizationJUnitTest.java > 873b6491f6703378d87383d96dc35e299b19a3fa > > geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java > e5cbd15da9274d2a86be399f45c9239b2ae373be > > geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java > d6491ffde316401b215b712d33d60ced8a2b65cf > > geode-core/src/test/java/org/apache/geode/management/internal/security/ShiroCacheStartRule.java > 848c05c77e6b9082075b07c5c91e21eeadde1bc0 > > geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java > d2e44408317b0f815de1322a82c0e87f4892107f > > geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java > 3854bb1021eb5e206d465aa315cc151cce829d7b > > geode-core/src/test/java/org/apache/geode/security/IntegratedClientAuthDUnitTest.java > 2aa633ca373d04ac2cd95cdfe2c4dd0ec3f3e6f5 > > geode-core/src/test/java/org/apache/geode/security/NoShowValue1PostProcessorDUnitTest.java > d2a98875d96ff55f6b6eec11c8695041ce8cd309 > > geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java > cf0df1b66bfa25bec3d110fc398108a6fbca8d4e > > geode-core/src/test/java/org/apache/geode/security/PostProcessorDUnitTest.java > a7cdb0fbfed65a5af1249f15f698efb0454f064a > > geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java > 54c02f787f923ff7305bdc2ede81044fac446944 > > geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java > d5f868634e7b4f93a9a5d6da44ef0e2c0c2729ae > > geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java > 953cdb746beb220b148555952c9d27a2389e3a01 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java > 7f52ce1d54dd71e9a1c53323d3388e3ebca82ecd > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java > PRE-CREATION > geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java > 2386af1ec47e8f75350304d597128ef8aa63a3b7 > > geode-cq/src/test/java/org/apache/geode/security/CQPDXPostProcessorDUnitTest.java > 12f08ec49c42bec6aaf90b676d09d53b0eee8fd9 > > geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java > e2b555a416d1a1896898959f9207477237f2af36 > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java > 30532b944cf6d171a26cd7800acfd30e1d151f67 > > geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java > 35d8fb4629a3b706cb30c5b90e1a1d59ca0aa556 > > geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java > 3003b3d673c0ee5f075b93e204a65660ff3e568e > > geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java > 3d09f096a500e2533bb3e7e4ba330bffac209c35 > > Diff: https://reviews.apache.org/r/52889/diff/ > > > Testing > ------- > > precheckin successful > > > Thanks, > > Jinmei Liao > >