Just a couple of quick issues, I can create tickets and patches for these if we want to go forward with them.
1: Is it intended that we ship a class in the default package? https://github.com/geoserver/geoserver/blob/master/src/security/ldap/src/main/java/LDAP.java
I guess we should either remove this one, move it over to src/test/java/, or put it in a package. Looks like it escaped from Justin’s sandbox though, so I suspect we don’t really need it.
2: I tore out a little hair last week trying to figure out why the configuration panel for the GeoNode security plugin wasn’t showing up in the GUI. Looks like the system allows for configuring more than just authentication filters, and those extras are configured but not displayed in the configuration GUI. There’s already a constraint on the type parameter for AuthenticationFilterPanel, but it requires SecurityFilterConfig. I think it would be helpful to future plugin developers if this constraint required SecurityAuthFilterConfig (which is just a marker interface) instead. In fact the web UI breaks if this marker interface is not present, but only on the filter list. Here’s the java file I’m suggesting to change: https://github.com/geoserver/geoserver/blob/master/src/web/security/src/main/java/org/geoserver/security/web/auth/AuthenticationFilterPanel.java
–
David Winslow
OpenGeo - http://opengeo.org/
On Mon, Jul 30, 2012 at 1:52 PM, David Winslow <dwinslow@anonymised.com> wrote:
Just a couple of quick issues, I can create tickets and patches for these if we want to go forward with them.
1: Is it intended that we ship a class in the default package? https://github.com/geoserver/geoserver/blob/master/src/security/ldap/src/main/java/LDAP.java
I guess we should either remove this one, move it over to src/test/java/, or put it in a package. Looks like it escaped from Justin’s sandbox though, so I suspect we don’t really need it.
Right. Yeah we don’t need this. Snuk in there accidentally.
2: I tore out a little hair last week trying to figure out why the configuration panel for the GeoNode security plugin wasn’t showing up in the GUI. Looks like the system allows for configuring more than just authentication filters, and those extras are configured but not displayed in the configuration GUI. There’s already a constraint on the type parameter for AuthenticationFilterPanel, but it requires SecurityFilterConfig. I think it would be helpful to future plugin developers if this constraint required SecurityAuthFilterConfig (which is just a marker interface) instead. In fact the web UI breaks if this marker interface is not present, but only on the filter list. Here’s the java file I’m suggesting to change: https://github.com/geoserver/geoserver/blob/master/src/web/security/src/main/java/org/geoserver/security/web/auth/AuthenticationFilterPanel.java
Right. Yeah, that makes sense. If we can change it and verify filters still load, tests pass, etc… i saw go ahead with the change.
–
David Winslow
OpenGeo - http://opengeo.org/
Live Security Virtual Conference
Exclusive live event will cover all the ways today’s security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
I went ahead and removed LDAP.java from the repo (this one seems pretty unambiguous.) For the other issue, it didn’t introduce any test failures but I didn’t commit it. The patch is attached to https://jira.codehaus.org/browse/GEOS-5241 for review.
–
David Winslow
OpenGeo - http://opengeo.org/
On Mon, Jul 30, 2012 at 9:51 PM, Justin Deoliveira <jdeolive@anonymised.com501…> wrote:
On Mon, Jul 30, 2012 at 1:52 PM, David Winslow <dwinslow@anonymised.com1…> wrote:
Just a couple of quick issues, I can create tickets and patches for these if we want to go forward with them.
1: Is it intended that we ship a class in the default package? https://github.com/geoserver/geoserver/blob/master/src/security/ldap/src/main/java/LDAP.java
I guess we should either remove this one, move it over to src/test/java/, or put it in a package. Looks like it escaped from Justin’s sandbox though, so I suspect we don’t really need it.
Right. Yeah we don’t need this. Snuk in there accidentally.
2: I tore out a little hair last week trying to figure out why the configuration panel for the GeoNode security plugin wasn’t showing up in the GUI. Looks like the system allows for configuring more than just authentication filters, and those extras are configured but not displayed in the configuration GUI. There’s already a constraint on the type parameter for AuthenticationFilterPanel, but it requires SecurityFilterConfig. I think it would be helpful to future plugin developers if this constraint required SecurityAuthFilterConfig (which is just a marker interface) instead. In fact the web UI breaks if this marker interface is not present, but only on the filter list. Here’s the java file I’m suggesting to change: https://github.com/geoserver/geoserver/blob/master/src/web/security/src/main/java/org/geoserver/security/web/auth/AuthenticationFilterPanel.java
Right. Yeah, that makes sense. If we can change it and verify filters still load, tests pass, etc… i saw go ahead with the change.
–
David Winslow
OpenGeo - http://opengeo.org/
Live Security Virtual Conference
Exclusive live event will cover all the ways today’s security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.