Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

> On June 12, 2017, 5 p.m., Kirk Lund wrote:
> > I recommend pulling out the changes to SecurityService creation. The 
> > changes I have on the feature branch prevent a Locator from allowing 
> > unsecured joins before the Locator creates its Cache.

Sounds good. I will rebase my change on top of yours then.


- Jinmei


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


On June 12, 2017, 2:44 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 12, 2017, 2:44 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
> 9b23f6c1a8ed3449d8a49029d6364f1e989e367c 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  2988ffd43ae1985c527a1082d91b2782b03eced0 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
>  deea55ff085762a2dd91ebdc57475e42724dee04 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
>  b682d93fd5c4b5340e2c30be72c5572e031e26ed 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
>  c52ccbc1cc5a293d70b177e38f03dc17c7db 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
>  01d99951bc70547fb311f2edbfec8dde1be799f7 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
>  2d6af1a22644d427ec2d17863cef27a8d8961491 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  f895b964794f99127f1f0c9564f3f85213e0af22 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de565a8301168f13e1917ea739a8879162c 
>   
> 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
>  c4946e768ee70db00030defa76da7d21d33c6e0c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
>  d328946632c1d0defc86aa0527208a841b9b45ba 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
>  f971deef0807534a014236d37ba48bafa307c56b 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
>  ef92bb7415c05ae09511e38d8a850f386de23033 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  be81582b74a4359f74d483ca64c6e42f6b081738 
>   
> 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/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda8437e06de818ead40731818f937c3aef5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  7ec7699821c9f5572aebeb0936ad3617e802c07e 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  365c6ae01994cd0d5c06e523c42b6bec19c14c5d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  345d688c10c0477904ceb4c5a52302b7bd3eaec9 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Kirk Lund]

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



I recommend pulling out the changes to SecurityService creation. The changes I 
have on the feature branch prevent a Locator from allowing unsecured joins 
before the Locator creates its Cache.

- Kirk Lund


On June 12, 2017, 2:44 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 12, 2017, 2:44 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
> 9b23f6c1a8ed3449d8a49029d6364f1e989e367c 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  2988ffd43ae1985c527a1082d91b2782b03eced0 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
>  deea55ff085762a2dd91ebdc57475e42724dee04 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
>  b682d93fd5c4b5340e2c30be72c5572e031e26ed 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
>  c52ccbc1cc5a293d70b177e38f03dc17c7db 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
>  01d99951bc70547fb311f2edbfec8dde1be799f7 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
>  2d6af1a22644d427ec2d17863cef27a8d8961491 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  f895b964794f99127f1f0c9564f3f85213e0af22 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de565a8301168f13e1917ea739a8879162c 
>   
> 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
>  c4946e768ee70db00030defa76da7d21d33c6e0c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
>  d328946632c1d0defc86aa0527208a841b9b45ba 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
>  f971deef0807534a014236d37ba48bafa307c56b 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
>  ef92bb7415c05ae09511e38d8a850f386de23033 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  be81582b74a4359f74d483ca64c6e42f6b081738 
>   
> 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/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda8437e06de818ead40731818f937c3aef5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  7ec7699821c9f5572aebeb0936ad3617e802c07e 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  365c6ae01994cd0d5c06e523c42b6bec19c14c5d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  345d688c10c0477904ceb4c5a52302b7bd3eaec9 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> 

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 12, 2017, 2:44 p.m.)


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


Changes
---

While rebasing this changeset to develop to pick up Kirk's security work, I got 
a bit carried away in refactoring. Below are the main points of this changeset:

1) move the creation of the security service back to GemfireCacheImpl, so that 
the security manager property recevied in the cluster configuration will take 
effect. (mainly reverted the changes under distributed.internal.membership 
package)
2) consolidate the implementation of SecurityService into two: 
IntegratedSecurityService and LegacySecurityService to avoid code duplication.
3) added default implemenation of SecurityService (debateble)
4) reworked SecurityServicefactory and add more tests.


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
9b23f6c1a8ed3449d8a49029d6364f1e989e367c 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
 2988ffd43ae1985c527a1082d91b2782b03eced0 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/SecurityConfig.java
 deea55ff085762a2dd91ebdc57475e42724dee04 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberFactory.java
 b682d93fd5c4b5340e2c30be72c5572e031e26ed 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberServices.java
 c52ccbc1cc5a293d70b177e38f03dc17c7db 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
 01d99951bc70547fb311f2edbfec8dde1be799f7 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
 2d6af1a22644d427ec2d17863cef27a8d8961491 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 f895b964794f99127f1f0c9564f3f85213e0af22 
  
geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
 84f97de565a8301168f13e1917ea739a8879162c 
  
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
 c4946e768ee70db00030defa76da7d21d33c6e0c 
  
geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
 d328946632c1d0defc86aa0527208a841b9b45ba 
  
geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
 f971deef0807534a014236d37ba48bafa307c56b 
  
geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
 ef92bb7415c05ae09511e38d8a850f386de23033 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 be81582b74a4359f74d483ca64c6e42f6b081738 
  
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/management/internal/cli/commands/DiskStoreCommands.java
 64fafda8437e06de818ead40731818f937c3aef5 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
 7ec7699821c9f5572aebeb0936ad3617e802c07e 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
 365c6ae01994cd0d5c06e523c42b6bec19c14c5d 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
 345d688c10c0477904ceb4c5a52302b7bd3eaec9 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a87b558772902cf14580f3e14fca97b3 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da464419779773c9116d824fcf11774bafbd79 
  

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Kirk Lund]

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


Ship it!




Ship it after GEODE-2632 is merged to develop.

- Kirk Lund


On June 5, 2017, 6:32 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 5, 2017, 6:32 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de565a8301168f13e1917ea739a8879162c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  f9fade1cfe8c181b0a0874869a66643c00300f98 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391212095413c0d577cfc65de7247080b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda8437e06de818ead40731818f937c3aef5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e1425d71af9d2ea59046b17afd70ad30dd68 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  6514a33e52611994ddc16a58414146ebaad75c65 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
>   
> geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
>  2d6fbcaeb470c79f383626b8e15e3bd8650377dd 
>   geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
> 6080b5de8c4b11f013d0800baf2a1d9f18cb7f1d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
>  80ff719b015ae0ffb5a648fe026bb01bc6128df8 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/7/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 5, 2017, 6:32 p.m.)


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


Changes
---

added a new interface method according review.


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
 84f97de565a8301168f13e1917ea739a8879162c 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 f9fade1cfe8c181b0a0874869a66643c00300f98 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 14784c391212095413c0d577cfc65de7247080b5 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 64fafda8437e06de818ead40731818f937c3aef5 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
 c2c6e1425d71af9d2ea59046b17afd70ad30dd68 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
 6514a33e52611994ddc16a58414146ebaad75c65 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
 fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a87b558772902cf14580f3e14fca97b3 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da464419779773c9116d824fcf11774bafbd79 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271efb876d471b35e002c5b110919f10fcc 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
  
geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
 2d6fbcaeb470c79f383626b8e15e3bd8650377dd 
  geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
6080b5de8c4b11f013d0800baf2a1d9f18cb7f1d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 80ff719b015ae0ffb5a648fe026bb01bc6128df8 


Diff: https://reviews.apache.org/r/59692/diff/7/

Changes: https://reviews.apache.org/r/59692/diff/6-7/


Testing
---

precheckin runing


Thanks,

Jinmei Liao

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Patrick Rhomberg]

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




geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
Line 71 (original)


We could use an `authorize(Resource r, Operation o, Target t)` that infers 
`key = "*"`


- Patrick Rhomberg


On June 2, 2017, 4:08 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 2, 2017, 4:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de56 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  f9fade1cf 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e1425 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  6514a33e5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da46441 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271e 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4 
>   
> geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
>  2d6fbcaeb 
>   geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
> 6080b5de8 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d19 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
>  80ff719b0 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/6/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

> On June 1, 2017, 9:47 p.m., Patrick Rhomberg wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
> > Line 29 (original), 30 (patched)
> > 
> >
> > Is it possible to make this `@Repeatable`?  There are some operations 
> > that require multiple security permissions, and it would be nice to be able 
> > to just annotate those functions twice.
> > 
> > For instance, `DistributedSystemMXBean.backupAllMembers` should have 
> > `DATA:READ` and `CLUSTER:WRITE:DISK`.
> 
> Jinmei Liao wrote:
> I tried to do this, but it's more involved than just adding the 
> repeatable annoation here. The user of these annotations will need to be 
> updated to handle multiple values. Possibly for future enhancement.
> 
> Patrick Rhomberg wrote:
> Here's a diff on my branch that I think does what we want.
> 
> 
> https://github.com/PurelyApplied/geode/commit/e82688ffb08e4b4542d2f440cb62d46d2b7bcf3c
> 
> Am I missing a use case where 
> `method.getAnnotation(ResourceOperation.class)` is going to be used by some 
> user's custom implementations?  Because otherwise we only need to change the 
> annotation processing in `CommandProcessor::executeCommand`, as far as I can 
> tell.

this works for annotations we added on commands. This annotation is also used 
on MXBeans, e.g. MemberMXBean, the place where it's parsing that info is in 
MBeanServerWrapper.getOperationContext(), that's where it's give us hickups.


- Jinmei


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


On June 2, 2017, 4:08 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 2, 2017, 4:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de56 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  f9fade1cf 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e1425 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  6514a33e5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da46441 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271e 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4 
>   
> geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
>  2d6fbcaeb 
>   geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
> 6080b5de8 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d19 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
>  80ff719b0 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/6/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Patrick Rhomberg]

> On June 1, 2017, 9:47 p.m., Patrick Rhomberg wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
> > Line 29 (original), 30 (patched)
> > 
> >
> > Is it possible to make this `@Repeatable`?  There are some operations 
> > that require multiple security permissions, and it would be nice to be able 
> > to just annotate those functions twice.
> > 
> > For instance, `DistributedSystemMXBean.backupAllMembers` should have 
> > `DATA:READ` and `CLUSTER:WRITE:DISK`.
> 
> Jinmei Liao wrote:
> I tried to do this, but it's more involved than just adding the 
> repeatable annoation here. The user of these annotations will need to be 
> updated to handle multiple values. Possibly for future enhancement.

Here's a diff on my branch that I think does what we want.

https://github.com/PurelyApplied/geode/commit/e82688ffb08e4b4542d2f440cb62d46d2b7bcf3c

Am I missing a use case where `method.getAnnotation(ResourceOperation.class)` 
is going to be used by some user's custom implementations?  Because otherwise 
we only need to change the annotation processing in 
`CommandProcessor::executeCommand`, as far as I can tell.


- Patrick


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


On June 2, 2017, 4:08 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 2, 2017, 4:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de56 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  f9fade1cf 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c2c6e1425 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  6514a33e5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da46441 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271e 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4 
>   
> geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
>  2d6fbcaeb 
>   geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
> 6080b5de8 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d19 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
>  80ff719b0 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/6/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 2, 2017, 4:08 p.m.)


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


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
 84f97de56 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 f9fade1cf 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 14784c391 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 64fafda84 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
 c2c6e1425 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
 6514a33e5 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
 fe79efbed 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da46441 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271e 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4 
  
geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
 2d6fbcaeb 
  geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
6080b5de8 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d19 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 80ff719b0 


Diff: https://reviews.apache.org/r/59692/diff/6/

Changes: https://reviews.apache.org/r/59692/diff/5-6/


Testing
---

precheckin runing


Thanks,

Jinmei Liao

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

> On June 1, 2017, 9:47 p.m., Patrick Rhomberg wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
> > Line 29 (original), 30 (patched)
> > 
> >
> > Is it possible to make this `@Repeatable`?  There are some operations 
> > that require multiple security permissions, and it would be nice to be able 
> > to just annotate those functions twice.
> > 
> > For instance, `DistributedSystemMXBean.backupAllMembers` should have 
> > `DATA:READ` and `CLUSTER:WRITE:DISK`.

I tried to do this, but it's more involved than just adding the repeatable 
annoation here. The user of these annotations will need to be updated to handle 
multiple values. Possibly for future enhancement.


- Jinmei


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


On June 2, 2017, 2:31 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 2, 2017, 2:31 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
>  84f97de56 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  f9fade1cf 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  64fafda84 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
>  6514a33e5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da46441 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271e 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4 
>   
> geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
>  2d6fbcaeb 
>   geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
> 6080b5de8 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d19 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
>  80ff719b0 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/5/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 2, 2017, 2:31 p.m.)


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


Changes
---

review changes


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/examples/security/ExampleSecurityManager.java
 84f97de56 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 f9fade1cf 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 14784c391 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 64fafda84 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java
 6514a33e5 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
 fe79efbed 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da46441 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271e 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4 
  
geode-core/src/test/java/org/apache/geode/security/SimpleSecurityManagerTest.java
 2d6fbcaeb 
  geode-core/src/test/java/org/apache/geode/security/TestSecurityManager.java 
6080b5de8 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d19 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 80ff719b0 


Diff: https://reviews.apache.org/r/59692/diff/5/

Changes: https://reviews.apache.org/r/59692/diff/4-5/


Testing
---

precheckin runing


Thanks,

Jinmei Liao

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Patrick Rhomberg]

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




geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
Line 29 (original), 30 (patched)


Is it possible to make this `@Repeatable`?  There are some operations that 
require multiple security permissions, and it would be nice to be able to just 
annotate those functions twice.

For instance, `DistributedSystemMXBean.backupAllMembers` should have 
`DATA:READ` and `CLUSTER:WRITE:DISK`.


- Patrick Rhomberg


On June 1, 2017, 5:21 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 1, 2017, 5:21 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  600d5462b1d18cfc702d400f6d91c1ac1fab3755 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391212095413c0d577cfc65de7247080b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/4/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Patrick Rhomberg]

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




geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
Lines 37-40 (patched)


This is internally inconsistent.  I'd either change the import all the way 
to 
`import org.apache.geode.security.ResourcePermission.Target;`
or update the above `Resource` and `Operation` references to extend their 
package, i.e. to include `ResourcePermission.Resource`.


- Patrick Rhomberg


On June 1, 2017, 5:21 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 1, 2017, 5:21 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  600d5462b1d18cfc702d400f6d91c1ac1fab3755 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391212095413c0d577cfc65de7247080b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/4/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Ken Howe]

> On June 1, 2017, 5:09 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
> > Lines 228 (patched)
> > 
> >
> > I think it might be nice to have a variant of `authorize()` that takes 
> > a Resource/Operation/Target rather than their String representations:
> > 
> > ```
> >   public void authorize(Resource resource, Operation operation){} 
> >   public void authorize(Resource resource, Operation operation, Target 
> > target){} 
> > 
> > ```
> > 
> > Then these methods would look like
> > ```
> > public void authorizeDiskManage() {
> > authorize(Resource.CLUSTER, Operation.MANAGE, 
> > ResourcePermission.Target.DISK);
> >   }
> > ```

Target can be a region name as well as the as a Target enum. Consequently, the 
ResourcePermission constructors that the authorize methods call currently all 
expect target as a string.


- Ken


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


On June 1, 2017, 5:21 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 1, 2017, 5:21 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  600d5462b1d18cfc702d400f6d91c1ac1fab3755 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391212095413c0d577cfc65de7247080b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/4/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 1, 2017, 5:21 p.m.)


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


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 600d5462b1d18cfc702d400f6d91c1ac1fab3755 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 14784c391212095413c0d577cfc65de7247080b5 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
 fe79efbed0aa7ec9a3d27526df2f4a86794513c2 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a87b558772902cf14580f3e14fca97b3 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da464419779773c9116d824fcf11774bafbd79 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271efb876d471b35e002c5b110919f10fcc 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 


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

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


Testing
---

precheckin runing


Thanks,

Jinmei Liao

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jared Stewart]

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




geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
Lines 197 (patched)


I might have expected a `NotAuthorizedException` if the current user is 
`null`.  What makes us want to default to authorized in that case?



geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
Lines 228 (patched)


I think it might be nice to have a variant of `authorize()` that takes a 
Resource/Operation/Target rather than their String representations:

```
  public void authorize(Resource resource, Operation operation){} 
  public void authorize(Resource resource, Operation operation, Target 
target){} 

```

Then these methods would look like
```
public void authorizeDiskManage() {
authorize(Resource.CLUSTER, Operation.MANAGE, 
ResourcePermission.Target.DISK);
  }
```



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
Lines 410 (patched)


If you add the same style of static import used by `Resource` and 
`Operation` here for `Target` as well I think it will read a little nicer.


- Jared Stewart


On June 1, 2017, 4:41 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated June 1, 2017, 4:41 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  600d5462b1d18cfc702d400f6d91c1ac1fab3755 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  14784c391212095413c0d577cfc65de7247080b5 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
>  9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/3/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 1, 2017, 4:41 p.m.)


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


Changes
---

add more methods in security service


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 600d5462b1d18cfc702d400f6d91c1ac1fab3755 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 14784c391212095413c0d577cfc65de7247080b5 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a87b558772902cf14580f3e14fca97b3 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da464419779773c9116d824fcf11774bafbd79 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271efb876d471b35e002c5b110919f10fcc 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 


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

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


Testing
---

precheckin runing


Thanks,

Jinmei Liao

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

(Updated June 1, 2017, 3:50 p.m.)


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


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 600d5462b1d18cfc702d400f6d91c1ac1fab3755 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a87b558772902cf14580f3e14fca97b3 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da464419779773c9116d824fcf11774bafbd79 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271efb876d471b35e002c5b110919f10fcc 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
9cff80d1982bd30f6ba5d8a61ab7307a69862fd4 


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

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


Testing
---

precheckin runing


Thanks,

Jinmei Liao

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Ken Howe]

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




geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java
Line 77 (original), 95 (patched)


I think it would be better to use Region.SEPARATOR instead of "/".


- Ken Howe


On May 31, 2017, 8:55 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59692/
> ---
> 
> (Updated May 31, 2017, 8:55 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: add target for resource operation for finer grained security
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  600d5462b1d18cfc702d400f6d91c1ac1fab3755 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
>  db3a1872a87b558772902cf14580f3e14fca97b3 
>   geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
> 45da464419779773c9116d824fcf11774bafbd79 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
>  b728b271efb876d471b35e002c5b110919f10fcc 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
>  3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 
> 
> 
> Diff: https://reviews.apache.org/r/59692/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin runing
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

[Jinmei Liao]

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

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


Repository: geode


Description
---

GEODE-2925: add target for resource operation for finer grained security


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 600d5462b1d18cfc702d400f6d91c1ac1fab3755 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 226cfaf45fa2a1720a92e8e7ac2c179653240e2d 
  
geode-core/src/main/java/org/apache/geode/management/internal/security/ResourceOperation.java
 db3a1872a87b558772902cf14580f3e14fca97b3 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
45da464419779773c9116d824fcf11774bafbd79 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermissionTest.java
 b728b271efb876d471b35e002c5b110919f10fcc 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 3f8f4d9d4ee0a8f9c3385cd66ee20655d126d34d 


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


Testing
---

precheckin runing


Thanks,

Jinmei Liao