Hi all,
I sent an email last week about the status of the security work currently sitting as a patch on this jira ticket:
https://jira.codehaus.org/browse/GEOS-4554
I didn’t really get any answer for that so I a created a branch to play around and try to address some fo the issues brought up on the ticket, as well as implement some of the requirements of the customer such as support for LDAP and OAuth. Out of that I have some general comments and questions. Much of this is redundant with what Andrea mentioned in his initial review (see the jira issue for details).
- Service/Store factories and singleton use
As Andrea mentioned the new security stuff seems to roll its own plugin magic… whereas really it should be using the established spring infrastructure . As Christian mentions in the response we might have to mandate that role and usergroup services plugg-ed in via spring be declared as prototype beans.
The factories also maintain a mapping of role/usergroup service to store. What about adding some api to the service interfaces directly:
interface GeoServerUserGroupService {
boolean canCreateStore();
GeoServerUserGroupStore createStore(…);
}
- Separate out jdbc specific stuff into its own module
As mentioned by Andrea, I think we will need to do some organization of the security modules to facilitate the development of plugins for different types of security backends. I think jdbc is a good candidate for a module outside of main. The structure I have in mind:
src/
main/ (core security stuff and xml backend)
security/ (parent module of all other security backends)
jdbc/
ldap/
oauth/
…
With the various modules contributing granted authority and user group beans/plugins via spring.
- Non aliasing in config.xml files
Again brought up on the ticket. What I was thinking for this one was to again add some api to UserGroupService and GrantedAuthorityService interfaces:
interface GeoServerUserGroupService {
void configure(XStream xs);
}
Basically giving each plugin a chance to configure the xstream (setting up aliases, etc…) before configuration is loaded/saved.
So the workflow would be when loading configuration from the disk:
- create an xstream instance, and run it through all plugins calling configure() to set it up
- read the config file
- match the config.getClassName() property back to one of the service plugins
- configure the service
Similar for workflow for persisting configuration
- Util class is bloated
The Util class in the security package seems to do a lot. Aside from actually your typical utility methods there is a lot of other method for managing persistence of role/usergroup services, and some life cycle management as well. I would like to see the “non typical utility methods” moved out somewhere else.
Mirroring the catalog and configuration apis I think it would be nice to have a top level facade that also acts like a dao for loading/saving, etc… As currently written it would seem that GeoServerUserDetailsService is the appropriate candidate. Ideally this class would do everything related to persistence, and managing of life cycle of all plugged in role/usergroup backends. Loading them, checking they are properly configured, etc…
- Using term “Role” rather than “GrantedAuthority”
Much more concise 
- Plugging in authentication providers
As an experiment on the branch I wanted to implement backend authenticating via LDAP. Doing this cleanly seems to require using a specific LdapAuthenticationProvider class in the ProviderManager chain. However i don’t see a way of cleanly doing this. Currently we have three authentication providers in the chain:
- dao → forwards onto our custom user details object
- anonymous
- remember me
So… should we provide a hook into the chain to latch on other authentication providers? Or does a hook already exist that i am missing? I was able to hack up support in the ldap module by using a bean post processor on the the ProviderManager bean to inject the LdapAuthenticationProvider into the chain and got it working.
That is it for now. Again it would be really great to hear from Christian about collaborating. Ideally we could set up a shared development branch to start pushing/pulling changes back and forth. Any chance you are using git for local development Christian?
-Justin
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.