[
https://issues.apache.org/jira/browse/GEODE-2053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15633607#comment-15633607
]
Jinmei Liao commented on GEODE-2053:
------------------------------------
[1]
https://github.com/spring-projects/spring-data-gemfire/blob/apache-geode/src/main/java/org/springframework/data/gemfire/config/annotation/EnableSecurity.java
[2] https://github.com/jxblum/contacts-application
[3] https://github.com/jxblum/contacts-application/tree/apache-geode
[4]
https://github.com/jxblum/contacts-application/tree/apache-geode/security-example
[5]
https://github.com/jxblum/contacts-application/blob/apache-geode/security-example/src/test/java/example/app/geode/security/GeodeSecurityIntegrationTests.java
[6]
https://github.com/jxblum/contacts-application/blob/apache-geode/security-example/src/test/java/example/app/geode/security/GeodeSecurityIntegrationTests.java#L280-L286
[7]
https://github.com/jxblum/contacts-application/blob/apache-geode/contacts-core/src/main/java/example/app/geode/security/provider/SimpleSecurityManager.java
[8]
http://geode.incubator.apache.org/releases/latest/javadoc/org/apache/geode/security/SecurityManager.html
[9]
https://github.com/jxblum/contacts-application/blob/apache-geode/contacts-core/src/main/java/example/app/geode/security/repository/SecurityRepository.java
[10] http://shiro.apache.org/architecture.html#detailed-architecture
[11]
https://github.com/jxblum/contacts-application/blob/apache-geode/security-example/src/test/java/example/app/geode/security/GeodeSecurityIntegrationTests.java#L311-L315
[12]
https://github.com/jxblum/contacts-application/blob/apache-geode/contacts-core/src/main/resources/geode-security-shiro.ini
[13]
https://github.com/jxblum/contacts-application/tree/apache-geode/contacts-core/src/main/java/example/app/geode/security/repository/support
[14]
https://github.com/jxblum/contacts-application/blob/apache-geode/security-example/src/test/java/example/app/geode/security/GeodeSecurityIntegrationTests.java#L317-L331
[15]
https://github.com/jxblum/contacts-application/blob/apache-geode/contacts-core/src/main/java/example/app/shiro/realm/SecurityRepositoryAuthorizingRealm.java
[16]
http://shiro.apache.org/static/1.3.2/apidocs/org/apache/shiro/realm/package-summary.html
[17]
https://github.com/jxblum/contacts-application/blob/apache-geode/security-example/src/test/java/example/app/geode/security/GeodeSecurityIntegrationTests.java#L333-L345
[18]
http://shiro.apache.org/static/1.3.2/apidocs/org/apache/shiro/realm/text/PropertiesRealm.html
[19]
https://github.com/jxblum/contacts-application/blob/apache-geode/contacts-core/src/main/resources/geode-security-shiro.properties
[20]
http://shiro.apache.org/static/1.3.2/apidocs/org/apache/shiro/mgt/SecurityManager.html
[21] http://docs.oracle.com/javase/8/docs/api/java/lang/SecurityManager.html
[22]
http://geode.incubator.apache.org/releases/latest/javadoc/org/apache/geode/security/package-frame.html
[23]
https://github.com/apache/incubator-geode/blob/develop/geode-core/src/main/java/org/apache/geode/internal/security/shiro/GeodePermissionResolver.java
[24] https://cwiki.apache.org/confluence/display/GEODE/Geode+Integrated+Security
[25]
https://github.com/jxblum/contacts-application/blob/apache-geode/security-example/src/test/java/example/app/geode/security/GeodeSecurityIntegrationTests.java#L342
[26]
http://shiro.apache.org/static/1.3.2/apidocs/org/apache/shiro/authz/permission/PermissionResolver.html
[27]
https://github.com/apache/incubator-geode/blob/develop/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
[28]
https://github.com/spring-projects/spring-data-gemfire/blob/apache-geode/src/main/java/org/springframework/data/gemfire/config/annotation/ApacheShiroSecurityConfiguration.java#L208-L224
[29]
https://github.com/apache/incubator-geode/tree/develop/geode-core/src/main/java/org/apache/geode/internal
[30]
https://github.com/apache/incubator-geode/tree/develop/geode-core/src/main/java/org/apache/geode/internal/security
[31]
https://github.com/apache/incubator-geode/tree/develop/geode-core/src/main/java/org/apache/geode/security/templates
[32]
https://github.com/apache/incubator-geode/tree/develop/geode-core/src/main/java/org/apache/geode/security
[33]
https://github.com/apache/incubator-geode/blob/develop/geode-core/src/main/java/org/apache/geode/security/SecurableCommunicationChannels.java
[34]
http://shiro.apache.org/static/1.3.2/apidocs/org/apache/shiro/subject/Subject.html
[35] http://docs.oracle.com/javase/8/docs/api/javax/security/auth/Subject.html
> Improve Geode Security Framework
> ---------------------------------
>
> Key: GEODE-2053
> URL: https://issues.apache.org/jira/browse/GEODE-2053
> Project: Geode
> Issue Type: Improvement
> Reporter: Jinmei Liao
>
> This is based on the feedback that John provided by using Geode Security in
> SDG.
> Below is his email:
> This is will be my final feedback (for now) regarding Geode's Integrated
> Security framework/feature. My following recommendations are based on
> extensive testing and experience trying to configure and enable Geode's
> security services.
> As you know, my goal was to be able to quickly, easily and conveniently
> configure Geode's security framework services and features using Spring.
> However, my techniques could equally apply in any context (e.g. PCF, or a
> simple test environment without any framework/container).
> To make it quick, easy and convenient, I added support for Geode Integrated
> Security in SD Geode by way of the new Annotation configuration model,
> specifically with the addition of the new @EnableSecurity annotation [1].
> The new @EnableSecurity annotation can be seen in action in the SDG Contacts
> Application RI [2] (apache-geode branch [3]), security-example [4],
> GeodeSecurityIntegrationTests class [5]. In this test class, I demonstrate 4
> different ways, each employing different methods an application developer
> might use to configure and enable Geode's Integrated Security feature to
> secure Apache Geode operationally:
> 1. I use Geode's security-manager (System) property to refer to an
> application implementation of "Geode's" SecurityManager interface. See here
> [6].
> As you may recall, the SimpleSecurityManager [7] implementation is a custom
> application implementation of Geode's SecurityManager interface [8] (not to
> be confused with Shiro's [20], or even Java's [21] SecurityManager, for that
> matter) employing the Repository (DAO) Design Pattern (via the
> SecurityRepository interface [9]) to load security configuration meta-data
> (e.g. Users, Roles and Permissions) from a variety of data sources. This is
> not at all unlike how the Apache Shiro framework, itself, is organized [10]
> (i.e. SecurityManager -> (pluggable) Realms).
> 2. Next [11], I use Geode's security-shiro-init (System) property to refer to
> a Apache Shiro INI security configuration file (e.g. geode-security-shiro.ini
> [12]).
> NOTE: the Users, Roles and Permissions I define are configured consistently
> throughout all my security configurations and data source examples (all based
> on my SecurityRepository implementations [13]).
> 3. Then [14], I create a custom Apache Shiro Realm based on my
> SecurityRepository interface (the SecurityRepositoryAuthorizingRealm [15]),
> enabling me, as a application developer, to plug in 1 of my implementations
> (i.e. JDBC or XML).
> NOTE:, I use XML since Apache Shiro provides no, OOTB implementation for XML,
> ironically (Shiro only supports Active Directory, JDBC, JNDI, LDAP and TEXT;
> see sub-packages below org.apache.shiro.realm [16]).
> 4. Finally [17], I use a canned, OOTB, Apache Shiro provided Realm
> implementation (PropertiesRealm [18]), that, as the name suggests, is based
> on the Java Properties file format (e.g. geode-security-shiro.properties
> [19]).
> This may seem like a lot of work, even overkill, but all these were pertinent
> in finding a number of bugs not only in SDG's @EnableSecurity annotation and
> supporting classes, but with Geode's own Integrated Security framework, and
> specifically the API [22]. Individually, or if I had stopped my testing
> efforts prematurely with only 1 or 2 examples, I definitely would not have
> uncovered all the problems I found.
> 1. First, the GeodePermissionResolver [23] is necessary to configure Apache
> Shiro's provided (OOTB) Realms correctly. Otherwise, the security
> Permissions are not enforced properly (in a hierarchical fashion as
> advertised [24], i.e. in section "3. Introduction of ResourcePermission").
> I used [25] the GeodePermissionResolver class to configure the Apache Shiro
> provided (OOTB) PropertiesRealm implementation [18].
> Therefore, the GeodePermissionResolver class must NOT be internal. This is
> particularly important if the user is using Apache Shiro to the fullest
> extent to configure and secure Apache Geode.
> Of course, I could have provided my own implementation of the Apache Shiro
> PermissionResolver interface [26] (especially given the simplicity of the
> GeodePermissionResolver implementation) but if that implementation every
> involves more logic behind the scenes, better to "reuse" then "reinvent" in
> this case.
> 2. I also think it is pertinent that the SecurityService interface [27] be in
> Geode's public API. Given this is just an interface, I am not sure why it
> was decided to make it "internal" anyway? I see no good reason NOT to expose
> this interface, especially since it would be particularly useful for both
> API/Framework (e.g. SDG) as well as tools developers developing extensions to
> Apache Geode.
> I actually did make use of the SecurityService interface [28] in SDG, despite
> my (usually) hard rule of NOT ever using any GemFire/Geode internal classes
> at all in SDG. Unfortunately, and all too often, in certain cases, such as
> the currently released version of Apache Geode, 1.0.0-incubating GA, there is
> simply no other way around it when providing support for securing Apache
> Geode, especially for making Apache Shiro first-class (something I want to
> similarly do for Spring Security).
> The usage in [28] is, well, a hack! I can certainly think of better uses of
> the SecurityService interface as alluded to above.
> 3. That brings me to a more general problem I have with the Geode (and
> GemFires) APIs and organization, which surfaces many anti-patterns. There is
> only 1 thing I will focus on here...
> We seem to keep propagating the broken "global", internal package
> organization [29] that has many, many, cross-cutting concerns in it, and
> recently adds "security" [30] into the fold.
> While Security IS a "cross-cutting" concern, it is definitely NOT a
> recommended way to organize a modular codebase. It will only make it more
> difficult to organize and modularize Geode into logical, (hopefully,
> "pluggable") functional units in the future. It would be better to see the
> org.apache.geode.internal.security package move under
> org.apache.geode.security. If Security is truly cross-cutting, then it ought
> to be a separate "module", independent of the other geode modules which can
> depend on it.
> While OSGi has lost it steam in recent years, the current organization would
> be problematic in this environment, especially where some of these
> classes/interface should have been made public.
> Still, given Java 9 is moving to a modular architecture, you might want to be
> a bit more forward thinking in how the codebase organization is going to
> affect modularity. You can bet users are going to want to leverage Geode in
> a very modular way once Java 9 is readily available.
> 4. Rather than having a org.apache.geode.security.templates package [31] in
> the public API, I might consider moving these classes to the examples. I
> would not want users thinking these Security classes/components are actually
> sufficient to securing Geode in a production environment (yikes)!
> Additionally, I would probably even package the classes under
> org.apache.geode.security [32] a bit differently, having more
> organization/cleanliness with sub-packages. The security packages feels like
> a dumping ground for anything and everything, all handling related but
> slightly different things, functionally... client vs. the server,
> authentication vs. authorization, securing communication channels, managing
> over all security, etc, etc. Organization is the key to architecture and
> making things easy to consume and understand by users.
> You could make a SecurableCommunicationChannels [33] an enum, too.
> 5. One more, and I will leave it at that for now... so I am thinking ahead,
> and... I really want to build support for Spring Security. This support can
> come by way of SDG to auto-enable this provider.
> Anyway, I think it is of paramount importance that we nail down the Geode
> Integrated Security SPI, allowing different "security providers" to be
> "plugged" in to secure Apache Geode in different contexts.
> Initially I was thinking the Geode SecurityManager interface would suffice,
> but I don't think so, not now. It could also be Geode's SecurityService
> interface and/or combination of Geode's SecurityManager, especially given
> that the Geode (Integrate)SecurityService seems to be used throughout the
> Geode codebase. Still this does not even feel quite right.
> I particularly like Apache Shiro's concept of and use of the Subject class
> [34]. This seems to be the focal point for all security related concerns
> using the Apache Shiro security framework. Unfortunately, they have their
> "own" implementation :( <sigh>.
> Apache Geode could adhere to the Java Subject API [35], though even the Java
> Subject is a final class (and not an interface, :( <sigh> ).
> The idea is that different adapters could be provided to different underlying
> security provider classes and interfaces (e.g. like Apache Shiro's Subject;
> SDG could provide an implementation, err.. Adapter, for Spring Security; the
> community might contribute additional ones for other security providers, and
> so on).
> So, maybe Apache Geode needs/provides a Subject interface (API/SPI) of it's
> "own" that is adapted for each security provider supported by Apache Geode or
> contributed by the community.
> Anyway, I am thinking out loud here, but this SPI and it's API will be
> crucially important to get more right than not in order to allow Geode to
> leverage the vast array of security providers available, making Geode as
> widely accepted in as many context's as possible.
> Phew, many thoughts to share; sorry for the length. Hopefully, by now, you
> realize the importance of, and understand the careful considerations
> necessary when designing an API, well, any feature for that matter.
> That's all for now. Hope this helps!
> Cheers,
> John
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
