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


I'm not sure if this comment should go here or elsewhere as it is more about 
the overall approach and not the actual code.

Specifically with regard to securing the REST endpoint(s). My feeling is that 
we are being way too prescriptive about the protocol we are defining. Seeing 
that our REST endpoints are already based on Spring, we should rather be 
providing means to plug directly into the Spring Security flow so that 
customers can implement whatever protocol they want. Whether it's something 
custom or something better known like JWT.

Remember that our API not only becomes the Java interfaces that a customer 
needs to implement, but (and possibly more importantly), the API exposed to 
clients. In addition, we're not providing any client implementations so every 
user is going to have to provide/write their own. I think this will be a 
significant barrier to using this new capability.

The cons to this approach would be that it could be more work for the user to 
configure. One way to mitigate would be to provide a comprehensive reference 
implementation.

- Jens Deppe


On Aug. 7, 2015, 3:35 p.m., Tushar Khairnar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37209/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2015, 3:35 p.m.)
> 
> 
> Review request for geode, Amogh Shetkar and Jens Deppe.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-77 : Integrated Security Code Merge
> 
> This is manual merge of code from int_security branch.
> 
> Testing done : JMX RMI-connector testing done from JConsole, Gfsh interactive 
> testing with different roles. DUnits are not yet integrated into open.
> 
> 
> Diffs
> -----
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java
>  d25063c 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/DistributedSystem.java
>  b7b2cd8 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java
>  472959d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java
>  10094a9 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java
>  b8dfeb3 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
>  f5ae3e5 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/security/AuthorizeRequest.java
>  8ba07a2 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/CacheServerMXBean.java
>  59f6537 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/DiskStoreMXBean.java
>  f14d16c 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/DistributedSystemMXBean.java
>  f0a0a79 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/GatewayReceiverMXBean.java
>  3e5ba1a 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/GatewaySenderMXBean.java
>  b6c5219 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/LockServiceMXBean.java
>  e53d50a 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/ManagerMXBean.java 
> 04fda7e 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/MemberMXBean.java 
> e935fcd 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/ManagementAgent.java
>  43bfe73 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/RestAgent.java
>  74695ee 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/SystemManagementService.java
>  d8f6983 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ClientCommands.java
>  2eb1318 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommands.java
>  279fb45 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  919d6fe 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
>  9e60839 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommands.java
>  4591b53 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommands.java
>  4614ce7 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DurableClientCommands.java
>  01910d6 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java
>  d4134ad 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/FunctionCommands.java
>  0d8c54a 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/GfshHelpCommands.java
>  d9d4bea 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java
>  c978381 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommands.java
>  302d7bb 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/MemberCommands.java
>  797f654 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/MiscellaneousCommands.java
>  da8f11d 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/PDXCommands.java
>  d236d81 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommands.java
>  7b298d6 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/RegionCommands.java
>  80ba89e 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java
>  4bdab90 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/StatusCommands.java
>  5abd08a 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/WanCommands.java
>  a6d9abf 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/JmxOperationInvoker.java
>  864907b 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/AccessControl.java
>  58040cd 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/AccessControlContext.java
>  1926db5 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/AccessControlMXBean.java
>  e217045 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/CLIOperationContext.java
>  b0198e4 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/JMXOperationContext.java
>  375cc27 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
>  d85ce65 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/MBeanServerWrapper.java
>  50942c1 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/ManagementInterceptor.java
>  1851977 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/Resource.java
>  4dc27e1 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceConstants.java
>  3f4d7cb 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperation.java
>  f149479 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/security/ResourceOperationContext.java
>  aa1c38c 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/AbstractCommandsController.java
>  73ce926 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/ConfigCommandsController.java
>  517d942 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/DataCommandsController.java
>  6767ec1 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/DiskStoreCommandsController.java
>  2df3432 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/FunctionCommandsController.java
>  de81543 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/MiscellaneousCommandsController.java
>  66d344f 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/WanCommandsController.java
>  1e22bd9 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/controllers/support/EnvironmentVariablesHandlerInterceptor.java
>  8ebed02 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/http/support/SimpleHttpRequester.java
>  8bd9d37 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/shell/AbstractHttpOperationInvoker.java
>  dac1271 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/shell/RestHttpOperationInvoker.java
>  0dfbdbd 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/web/shell/SimpleHttpOperationInvoker.java
>  a122339 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/extension/mock/MockExtensionCommands.java
>  89644f0 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/CommandManagerJUnitTest.java
>  ab9333d 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshExecutionStrategyJUnitTest.java
>  44aef44 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthCodeTest.java
>  384493b 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResourceOperationJUnit.java
>  f061240 
>   
> gemfire-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/controllers/AbstractBaseController.java
>  feed8c7 
>   
> gemfire-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/controllers/BaseControllerAdvice.java
>  5ae88bc 
>   
> gemfire-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/controllers/CommonCrudController.java
>  ef52347 
>   
> gemfire-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/controllers/FunctionAccessController.java
>  45d6f66 
>   
> gemfire-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/controllers/PdxBasedCrudController.java
>  96551c6 
>   
> gemfire-web-api/src/main/java/com/gemstone/gemfire/rest/internal/web/controllers/QueryAccessController.java
>  b20c849 
>   gemfire-web-api/src/main/webapp/WEB-INF/web.xml 554ef4b 
> 
> Diff: https://reviews.apache.org/r/37209/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tushar Khairnar
> 
>

Reply via email to