You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2017/06/12 20:42:12 UTC

Review Request 60028: GEODE-2632: consolidate different types of SecurityService

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/
-----------------------------------------------------------

Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.


Repository: geode


Description
-------

* combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
  geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
  geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
  geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
  geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
  geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
  geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
  geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java c551ca9104a85dcf54c0bebbc4178fce4114a416 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
  geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
  geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/1/


Testing
-------

precheckin running


Thanks,

Jinmei Liao


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Jinmei Liao <ji...@pivotal.io>.

> On June 13, 2017, 7 p.m., Kirk Lund wrote:
> > -1 I don't see why these changes are needed other than to wipe out what I did? For example, you've undone my introduction of RealmInitializer and ConfigInitializer. I introduced those so I can continue writing more mocking tests for SecurityService so that our unit tests don't have to be integration tests that actually interact with Shiro. We should be able to separate our code. Also, I really prefer using a "DisabledSecurityService" when security is disabled rather than using a "LegacySecurityService" -- when security is disabled, it isn't "legacy".

what about the consolidation between the EnabledSecurityService and CustomSecurityService? There are lots of duplicated code there. Also regarding DisabledSecurityService, as far the product is concerned, there is no DisabledSecurityService though, only test would use them, so I think it would be a good idea to lump them together. It does get rid of a log of duplicate code though.

I do agree that the RealmInitializer would help for mocking. I will bring it back.


- Jinmei


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/#review177781
-----------------------------------------------------------


On June 13, 2017, 4:49 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 4:49 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
>   geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
>   geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
>   geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
>   geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java c551ca9104a85dcf54c0bebbc4178fce4114a416 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
>   geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
>   geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
>   geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 
> 
> 
> Diff: https://reviews.apache.org/r/60028/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/#review177781
-----------------------------------------------------------



-1 I don't see why these changes are needed other than to wipe out what I did? For example, you've undone my introduction of RealmInitializer and ConfigInitializer. I introduced those so I can continue writing more mocking tests for SecurityService so that our unit tests don't have to be integration tests that actually interact with Shiro. We should be able to separate our code. Also, I really prefer using a "DisabledSecurityService" when security is disabled rather than using a "LegacySecurityService" -- when security is disabled, it isn't "legacy".

- Kirk Lund


On June 13, 2017, 4:49 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 4:49 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
>   geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
>   geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
>   geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
>   geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java c551ca9104a85dcf54c0bebbc4178fce4114a416 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
>   geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
>   geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
>   geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 
> 
> 
> Diff: https://reviews.apache.org/r/60028/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/#review177920
-----------------------------------------------------------


Ship it!




Ship It!

- Kirk Lund


On June 13, 2017, 9:28 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 9:28 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * combine ConfigInitalizer and RealmInitilizer
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
>   geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ShiroSecurityManagerInitializer.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
>   geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
>   geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
>   geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
>   geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
>   geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 
> 
> 
> Diff: https://reviews.apache.org/r/60028/diff/3/
> 
> 
> Testing
> -------
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/
-----------------------------------------------------------

(Updated June 15, 2017, 3:23 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.


Repository: geode


Description
-------

* combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* combine ConfigInitalizer and RealmInitilizer
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
  geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
  geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
  geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
  geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
  geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
  geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
  geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
  geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
  geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
  geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/4/


Testing (updated)
-------

precheckin all green


Thanks,

Jinmei Liao


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/
-----------------------------------------------------------

(Updated June 15, 2017, 3:33 a.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.


Changes
-------

Changed the SecuirtyManagerInitializer to SecurityManagerProvider after discussion with Kirk. This should help better mocking and better unit tests.


Repository: geode


Description
-------

* combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* combine ConfigInitalizer and RealmInitilizer
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
  geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
  geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
  geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
  geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
  geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
  geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
  geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
  geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
  geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
  geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/4/

Changes: https://reviews.apache.org/r/60028/diff/3-4/


Testing
-------

precheckin running


Thanks,

Jinmei Liao


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/#review177951
-----------------------------------------------------------



I pulled this diff and incorporated everything into my finer-grained-security things, since we were overlapping a lot.

- Patrick Rhomberg


On June 13, 2017, 9:28 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 9:28 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * combine ConfigInitalizer and RealmInitilizer
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
>   geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   geode-core/src/main/java/org/apache/geode/internal/security/shiro/ShiroSecurityManagerInitializer.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
>   geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
>   geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
>   geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
>   geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
>   geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
>   geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
>   geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 
> 
> 
> Diff: https://reviews.apache.org/r/60028/diff/3/
> 
> 
> Testing
> -------
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/
-----------------------------------------------------------

(Updated June 13, 2017, 9:28 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.


Changes
-------

test update
this changeset was started when I was rebasing my finer graned security on top of Kirk's change and I found out that I had to implemented the newly added interaface methods in all the implemtations and introduce more duplicate codes. 
I do believe this changeset makes some improvement, but if the team thinks this is not necessary, I am happy to bury it.


Repository: geode


Description (updated)
-------

* combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* combine ConfigInitalizer and RealmInitilizer
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
  geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
  geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ShiroSecurityManagerInitializer.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
  geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
  geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
  geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
  geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
  geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
  geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
  geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/3/

Changes: https://reviews.apache.org/r/60028/diff/2-3/


Testing
-------

precheckin running


Thanks,

Jinmei Liao


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/
-----------------------------------------------------------

(Updated June 13, 2017, 8:26 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.


Changes
-------

consolidating the ConfigInitializer and RealmInitanilizer into ShiroSecurityManagerInitilizer. This helps UnitTset mock and also further simplify the code in SecurityServiceFactory.


Repository: geode


Description
-------

* combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
  geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
  geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ShiroSecurityManagerInitializer.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
  geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
  geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
  geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
  geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
  geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
  geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
  geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/2/

Changes: https://reviews.apache.org/r/60028/diff/1-2/


Testing (updated)
-------

precheckin running


Thanks,

Jinmei Liao


Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60028/
-----------------------------------------------------------

(Updated June 13, 2017, 4:49 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.


Repository: geode


Description
-------

* combine EnabledSecurityService and CustomSecurityService into IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java 3ff632d3857189513243959ee96da89da66d5a27 
  geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java b50569036db1acac927f08ee5c1771c72ede3c1d 
  geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java 44562537bc426e47a42d680bb410dc59bf9bd854 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java a4041e198f1a9a4961915504c51256d0b03aa7c2 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java 60f014b9c33732a4ea134a654e666a9439b210bb 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java a0c3cf3074051990cc50755131f8024db0b1faad 
  geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java cacbeed957c3b87d08c93db74e38e0134565f699 
  geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java fca7eae9413cee98d351db5349fd950d3aa56180 
  geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java 70823443b5c4f776c86bb28ed49a73e690c5c872 
  geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java f027a4367b38681f83dad2d4c4add67759633644 
  geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java 44893520962331bcd41e972afa661538c28d4fb2 
  geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java 857c0be8940b4acde2aa4992fac0122b687391c2 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java c551ca9104a85dcf54c0bebbc4178fce4114a416 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java 1caedbcede239d6a96640381cc6941948637b442 
  geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java cdb90f1b580edaf6a2762883d4159a45d69c4728 
  geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/1/


Testing (updated)
-------

precheckin successful


Thanks,

Jinmei Liao