Re: 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/ --- (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:
Re: 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/#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 >
Re: 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/#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 > >
Re: 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/ --- (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
Re: 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/ --- (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:
Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService
> 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 > >
Re: 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/#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 > >
Re: 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/ --- (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
