You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2016/05/16 15:46:45 UTC

Review Request 47416: GEODE-17: make geode authorization case-sensitive since our region names are case sensitive

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

Review request for geode, Jens Deppe, Kevin Duling, and Kirk Lund.


Repository: geode


Description
-------

* Specify case sensitive when creating the permission context
* Specify case sensitive when resolving the permission from shiro-ini file
* rename shiro-init to security-shiro-init since it's security related in DistributionConfig
* For DATA operations, a "NULL" regionName is used when regionName couldn't be resolved yet. Since for permissions,
  DATA:READ is different from DATA:READ:NULL


Diffs
-----

  geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java dec716cc418f361c3430887749ce34fcf172f0b8 
  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java d38e1a9a925929f8c6df0ea5cee589e3b716030f 
  geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java c0e560c441519749cdc7b5d9a1ddd8ca0e75d08f 
  geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java a4c2f2faad5b2e1d21aa6a3b4f5fd9535148f49e 
  geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java 6e10f3f66ae88ae2e7b1e2431ad3a725fa79805f 
  geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/GeodePermissionResolver.java PRE-CREATION 
  geode-core/src/main/java/com/gemstone/gemfire/management/internal/SystemManagementService.java fd2a834c466efa941249a24c31be90772fb8bb4e 
  geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java 2e461040e6a4d5488f70ea03677de932fdad84b3 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CacheServerMBeanShiroJUnitTest.java 85a55a75bafae0c69d566662f858409dd2644fdb 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java 97260d82ea17db7eefc6065a1ccbf0d6b26cd092 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilCustomRealmJUnitTest.java 0bf3cab1009ecbac9671e6e89bc75ccb195a781a 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java fe80180c21e81555badc95b54debd1233bc412b4 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java 8eaaf6a085516964d8c4a87665c41364e6f4f491 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java 9e2e41aa5022be32255e7266ca80376398b2e5b8 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ShiroCacheStartRule.java 7d683f3871c7679d4f349daefc757e06b626f8fe 
  geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java 56eeeec43fd4e48f8ab9bd755cacf0510619d75d 

Diff: https://reviews.apache.org/r/47416/diff/


Testing
-------

precheckin


Thanks,

Jinmei Liao


Re: Review Request 47416: GEODE-17: make geode authorization case-sensitive since our region names are case sensitive

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

> On May 18, 2016, 12:38 a.m., Kirk Lund wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java, line 61
> > <https://reviews.apache.org/r/47416/diff/1/?file=1384578#file1384578line61>
> >
> >     Geode style is just to have this all on one line

I must have accidentally entered return here.


- Jinmei


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


On May 16, 2016, 3:46 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47416/
> -----------------------------------------------------------
> 
> (Updated May 16, 2016, 3:46 p.m.)
> 
> 
> Review request for geode, Jens Deppe, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * Specify case sensitive when creating the permission context
> * Specify case sensitive when resolving the permission from shiro-ini file
> * rename shiro-init to security-shiro-init since it's security related in DistributionConfig
> * For DATA operations, a "NULL" regionName is used when regionName couldn't be resolved yet. Since for permissions,
>   DATA:READ is different from DATA:READ:NULL
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java dec716cc418f361c3430887749ce34fcf172f0b8 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java d38e1a9a925929f8c6df0ea5cee589e3b716030f 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java c0e560c441519749cdc7b5d9a1ddd8ca0e75d08f 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java a4c2f2faad5b2e1d21aa6a3b4f5fd9535148f49e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java 6e10f3f66ae88ae2e7b1e2431ad3a725fa79805f 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/GeodePermissionResolver.java PRE-CREATION 
>   geode-core/src/main/java/com/gemstone/gemfire/management/internal/SystemManagementService.java fd2a834c466efa941249a24c31be90772fb8bb4e 
>   geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java 2e461040e6a4d5488f70ea03677de932fdad84b3 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CacheServerMBeanShiroJUnitTest.java 85a55a75bafae0c69d566662f858409dd2644fdb 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java 97260d82ea17db7eefc6065a1ccbf0d6b26cd092 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilCustomRealmJUnitTest.java 0bf3cab1009ecbac9671e6e89bc75ccb195a781a 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java fe80180c21e81555badc95b54debd1233bc412b4 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java 8eaaf6a085516964d8c4a87665c41364e6f4f491 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java 9e2e41aa5022be32255e7266ca80376398b2e5b8 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ShiroCacheStartRule.java 7d683f3871c7679d4f349daefc757e06b626f8fe 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java 56eeeec43fd4e48f8ab9bd755cacf0510619d75d 
> 
> Diff: https://reviews.apache.org/r/47416/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 47416: GEODE-17: make geode authorization case-sensitive since our region names are case sensitive

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


Fix it, then Ship it!




Fix it and Ship It!

Two minor style changes. If you're already using etc/intellijIdeaCodeStyle.xml let me know (maybe it needs to be fixed).

I think the "z" was accidentally added to that one test method (doesn't effect whether it'll run though).


geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java (line 55)
<https://reviews.apache.org/r/47416/#comment198235>

    Geode style is just to have this all on one line



geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/GeodePermissionResolver.java (line 25)
<https://reviews.apache.org/r/47416/#comment198238>

    Geode style should be:
    
    @Override
    public Permission...



geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CacheServerMBeanShiroJUnitTest.java (line 86)
<https://reviews.apache.org/r/47416/#comment198236>

    Remove "z"?


- Kirk Lund


On May 16, 2016, 3:46 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47416/
> -----------------------------------------------------------
> 
> (Updated May 16, 2016, 3:46 p.m.)
> 
> 
> Review request for geode, Jens Deppe, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * Specify case sensitive when creating the permission context
> * Specify case sensitive when resolving the permission from shiro-ini file
> * rename shiro-init to security-shiro-init since it's security related in DistributionConfig
> * For DATA operations, a "NULL" regionName is used when regionName couldn't be resolved yet. Since for permissions,
>   DATA:READ is different from DATA:READ:NULL
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java dec716cc418f361c3430887749ce34fcf172f0b8 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java d38e1a9a925929f8c6df0ea5cee589e3b716030f 
>   geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java c0e560c441519749cdc7b5d9a1ddd8ca0e75d08f 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java a4c2f2faad5b2e1d21aa6a3b4f5fd9535148f49e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java 6e10f3f66ae88ae2e7b1e2431ad3a725fa79805f 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/GeodePermissionResolver.java PRE-CREATION 
>   geode-core/src/main/java/com/gemstone/gemfire/management/internal/SystemManagementService.java fd2a834c466efa941249a24c31be90772fb8bb4e 
>   geode-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java 2e461040e6a4d5488f70ea03677de932fdad84b3 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CacheServerMBeanShiroJUnitTest.java 85a55a75bafae0c69d566662f858409dd2644fdb 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java 97260d82ea17db7eefc6065a1ccbf0d6b26cd092 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilCustomRealmJUnitTest.java 0bf3cab1009ecbac9671e6e89bc75ccb195a781a 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java fe80180c21e81555badc95b54debd1233bc412b4 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java 8eaaf6a085516964d8c4a87665c41364e6f4f491 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContextJUnitTest.java 9e2e41aa5022be32255e7266ca80376398b2e5b8 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ShiroCacheStartRule.java 7d683f3871c7679d4f349daefc757e06b626f8fe 
>   geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java 56eeeec43fd4e48f8ab9bd755cacf0510619d75d 
> 
> Diff: https://reviews.apache.org/r/47416/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>