[Geoserver-devel] Current build failure in app-schema

Andrea_Aime4 · July 6, 2011, 8:18pm

Hi,
I see the build is failing in the app-schema extension in a security
related test, and that the cause is the fix for:
http://jira.codehaus.org/browse/GEOS-4674

We have a number of tests in simple feature land that work properly,
and I’m pretty sure the previous behavior was buggy.

However along with the test I also hardened the security code against
store that might not respect property selection, something which
I could do for simple features, but not for complex ones, since
it’s security if I see there are more attributes than allowed in the
schema, and nobody cared to develop a retyping feature collection
for complex features I just throw an exception.

That might be the cause for the failure, not sure, don’t have the
code handy right now, but if the issue is what I think it is,
I’m wondering if it’s acceptable that
complex features store can return more attributes than allowed
by the security subsytem (rethoric question, it’s not unless I’m
missing something obvious)

Cheers
Andrea

–

Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

Ben_Caradoc-Davies · July 7, 2011, 2:13am

Thanks, Andrea. I am not sure what the problem is. I am investigating.

Kind regards,
Ben.

On 07/07/11 04:18, Andrea Aime wrote:

Hi,
I see the build is failing in the app-schema extension in a security
related test, and that the cause is the fix for:
http://jira.codehaus.org/browse/GEOS-4674

We have a number of tests in simple feature land that work properly,
and I'm pretty sure the previous behavior was buggy.

However along with the test I also hardened the security code against
store that might not respect property selection, something which
I could do for simple features, but not for complex ones, since
it's security if I see there are more attributes than allowed in the
schema, and nobody cared to develop a retyping feature collection
for complex features I just throw an exception.

That might be the cause for the failure, not sure, don't have the
code handy right now, but if the issue is what I think it is,
I'm wondering if it's acceptable that
complex features store can return more attributes than allowed
by the security subsytem (rethoric question, it's not unless I'm
missing something obvious)

Cheers
Andrea

--
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineering Team Leader
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre

Ben_Caradoc-Davies · July 7, 2011, 5:44am

Andrea,

I have been through the app-schema implementation and I am pretty sure we are doing the right thing. I'm testing a patch for SecuredFeatureSource.

In SecuredFeatureSource.getFeatures(Query) you assume that a data store is "bad" if it returns a collection with a schema that is larger than the requested number of attributes. I do not think that this is a justified assumption:

(1) As far as I know, data stores are under no obligation to provide a different schema when the security subsystem is limiting output attributes. What would be the point for app-schema, when the schemas are published elsewhere? But we do support attribute restrictions (honoured for optional properties): you can see the unfiltered schema, but that does not mean that you can see these properties populated in each feature. Just because simple features behave this was does not mean it is the required behaviour.

(2) Application schemas are all about interoperability. Servers delivering types conforming to application schemas cannot just go rewriting their schemas according to service owner preference. To conform to an application schema, encoded features must include all mandatory properties specified in the type definition, regardless of the security subsystem. This is different to the simple features world-view, in which schemas are made up on the fly and all properties are optional. For this reason, app-schema populates all mandatory properties for which it has mappings, even when the security subsystem says not to. Service owners can map the appropriate nil representation to a mandatory property that they do not wish to deliver. Another possible solution might be for app-schema to automatically encode a "withheld" OGC URI for a complex property that is mandatory but not included in the security allowed-attribute list.

I'll attach the patch to:
http://jira.codehaus.org/browse/GEOS-4674

Kind regards,
Ben.

On 07/07/11 04:18, Andrea Aime wrote:

Hi,
I see the build is failing in the app-schema extension in a security
related test, and that the cause is the fix for:
http://jira.codehaus.org/browse/GEOS-4674

We have a number of tests in simple feature land that work properly,
and I'm pretty sure the previous behavior was buggy.

However along with the test I also hardened the security code against
store that might not respect property selection, something which
I could do for simple features, but not for complex ones, since
it's security if I see there are more attributes than allowed in the
schema, and nobody cared to develop a retyping feature collection
for complex features I just throw an exception.

That might be the cause for the failure, not sure, don't have the
code handy right now, but if the issue is what I think it is,
I'm wondering if it's acceptable that
complex features store can return more attributes than allowed
by the security subsytem (rethoric question, it's not unless I'm
missing something obvious)

Cheers
Andrea

--
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineering Team Leader
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre

Andrea_Aime4 · July 7, 2011, 6:22am

On Thu, Jul 7, 2011 at 7:44 AM, Ben Caradoc-Davies Ben.Caradoc-Davies@anonymised.com wrote:

Andrea,

I have been through the app-schema implementation and I am pretty sure we are doing the right thing. I’m testing a patch for SecuredFeatureSource.

In SecuredFeatureSource.getFeatures(Query) you assume that a data store is “bad” if it returns a collection with a schema that is larger than the requested number of attributes. I do not think that this is a justified assumption:

It is, app schema is breaking the rules that have been there for ages. Checking that the schema is respected is not business of the data store, returning more data is bad for rendering and processing.
GML is not the whole world, stores are a low level concept and should be ready to anything, but unfortunately app-schema has been developed with a very narrow view of the world in mind and with the architecture upside down, trying to do anything and everything within it, it shows in a number of ways, starting from the fact that using complex features is close to impossible outside of it due to lack of infrastructure (where are the feature builders, the collection wrappers, the basic facilities to manipulate data?).
A store is there to serve any kind of data manipulation, by bringing in WFS concerns into the store you severely limited its usefulness and you’re also breaking the security model.
I’ll patch the code, but also make it log a warning each time since either the store is mishbehaving or the administrator has limited some property that shouln’t have been there.

Cheers
Andrea

–

Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

Ben_Caradoc-Davies · July 7, 2011, 6:32am

Andrea, I have already committed a tiny adjustment to fix the build.

Can you please point me to these "rules that have been there for ages"? I do not recall seeing them in the developer guide or javadoc. I'm not sure that app-schema can conform to these rules, for the reasons outlined in my email.

On 07/07/11 14:22, Andrea Aime wrote:

On Thu, Jul 7, 2011 at 7:44 AM, Ben Caradoc-Davies<Ben.Caradoc-Davies@anonymised.com> wrote:
Andrea,

I have been through the app-schema implementation and I am pretty sure we are doing the right thing. I'm testing a patch for SecuredFeatureSource.

In SecuredFeatureSource.getFeatures(Query) you assume that a data store is "bad" if it returns a collection with a schema that is larger than the requested number of attributes. I do not think that this is a justified assumption:

It is, app schema is breaking the rules that have been there for ages. Checking that the schema is respected is not business of the data store, returning more data is bad for rendering and processing.
GML is not the whole world, stores are a low level concept and should be ready to anything, but unfortunately app-schema has been developed with a very narrow view of the world in mind and with the architecture upside down, trying to do anything and everything within it, it shows in a number of ways, starting from the fact that using complex features is close to impossible outside of it due to lack of infrastructure (where are the feature builders, the collection wrappers, the basic facilities to manipulate data?).
A store is there to serve any kind of data manipulation, by bringing in WFS concerns into the store you severely limited its usefulness and you're also breaking the security model.
I'll patch the code, but also make it log a warning each time since either the store is mishbehaving or the administrator has limited some property that shouln't have been there.

Cheers
Andrea

--
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineering Team Leader
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre

Andrea_Aime4 · July 7, 2011, 6:44am

On Thu, Jul 7, 2011 at 8:32 AM, Ben Caradoc-Davies Ben.Caradoc-Davies@anonymised.com wrote:

Andrea, I have already committed a tiny adjustment to fix the build.

Can you please point me to these “rules that have been there for ages”? I do not recall seeing them in the developer guide or javadoc. I’m not sure that app-schema can conform to these rules, for the reasons outlined in my email.

/**

Get the names of the properties that this Query will retrieve values for
as part of the returned {@linkplain org.geotools.feature.FeatureCollection}.
@return the xpath expressions to be used in the returned FeatureCollection.
@see #retrieveAllProperties()
*/
public List getProperties() {

This is in the Query interface. All stores besides app-schema honor it, and we
also have tests in place to ensure that this happens.

Cheers
Andrea

–

Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

Ben_Caradoc-Davies · July 7, 2011, 7:12am

I think this app-schema behaviour is configurable, and turned off for WMS. See: http://jira.codehaus.org/browse/GEOS-4380
(by Niels, approved by Justin).

If you look a bit higher up in Query:

     /**
      * When specifying properties to select, setting this hint flag true tells the datastore
      * to include mandatory properties (i.e. properties with minOccurs >= 1) in the end result,
      * irrespective of whether they are not included in the list of properties.
      *
      * Datastores may implement adding all mandatory properties to the end result
      * when this flag is set to true. For example:
      *
      * Object includeProps = query.getHints().get(Query.INCLUDE_MANDATORY_PROPS);
      * if (includeProps instanceof Boolean && ((Boolean)includeProps).booleanValue()) {
      * query.setProperties (DataUtilities.addMandatoryProperties(type, query.getProperties()));
      * }
      *
      */
     public static Hints.Key INCLUDE_MANDATORY_PROPS = new Hints.Key(Boolean.class);

On 07/07/11 14:44, Andrea Aime wrote:

On Thu, Jul 7, 2011 at 8:32 AM, Ben Caradoc-Davies<Ben.Caradoc-Davies@anonymised.com> wrote:
Andrea, I have already committed a tiny adjustment to fix the build.

Can you please point me to these "rules that have been there for ages"? I do not recall seeing them in the developer guide or javadoc. I'm not sure that app-schema can conform to these rules, for the reasons outlined in my email.

  /**
      * Get the names of the properties that this Query will retrieve values for
      * as part of the returned {@linkplain org.geotools.feature.FeatureCollection}.
      *
      * @return the xpath expressions to be used in the returned FeatureCollection.
      *
      * @see #retrieveAllProperties()
      */
     public List<PropertyName> getProperties() {

This is in the Query interface. All stores besides app-schema honor it, and we
also have tests in place to ensure that this happens.

Cheers
Andrea

--
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

-------------------------------------------------------

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineering Team Leader
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre

Andrea_Aime4 · July 7, 2011, 7:21am

On Thu, Jul 7, 2011 at 9:12 AM, Ben Caradoc-Davies Ben.Caradoc-Davies@anonymised.com wrote:

I think this app-schema behaviour is configurable, and turned off for WMS. See: http://jira.codehaus.org/browse/GEOS-4380
(by Niels, approved by Justin).

If you look a bit higher up in Query:

/**

When specifying properties to select, setting this hint flag true tells the datastore

to include mandatory properties (i.e. properties with minOccurs >= 1) in the end result,

irrespective of whether they are not included in the list of properties.

Datastores may implement adding all mandatory properties to the end result

when this flag is set to true. For example:

Object includeProps = query.getHints().get(Query.INCLUDE_MANDATORY_PROPS);

if (includeProps instanceof Boolean && ((Boolean)includeProps).booleanValue()) {

query.setProperties (DataUtilities.addMandatoryProperties(type, query.getProperties()));

}

*/
public static Hints.Key INCLUDE_MANDATORY_PROPS = new Hints.Key(Boolean.class);

Makes some sense, but has the defaults upside down. It must be off by default, and enabled
only in WFS and GetFeatureInfo

Cheers
Andrea

–

Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

Andrea_Aime4 · July 7, 2011, 7:23am

On Thu, Jul 7, 2011 at 9:21 AM, Andrea Aime <andrea.aime@anonymised.com1268…> wrote:

On Thu, Jul 7, 2011 at 9:12 AM, Ben Caradoc-Davies Ben.Caradoc-Davies@anonymised.com wrote:

I think this app-schema behaviour is configurable, and turned off for WMS. See: http://jira.codehaus.org/browse/GEOS-4380
(by Niels, approved by Justin).

If you look a bit higher up in Query:

/**

When specifying properties to select, setting this hint flag true tells the datastore

to include mandatory properties (i.e. properties with minOccurs >= 1) in the end result,

irrespective of whether they are not included in the list of properties.

Datastores may implement adding all mandatory properties to the end result

when this flag is set to true. For example:

Object includeProps = query.getHints().get(Query.INCLUDE_MANDATORY_PROPS);

if (includeProps instanceof Boolean && ((Boolean)includeProps).booleanValue()) {

query.setProperties (DataUtilities.addMandatoryProperties(type, query.getProperties()));

}

*/
public static Hints.Key INCLUDE_MANDATORY_PROPS = new Hints.Key(Boolean.class);

Makes some sense, but has the defaults upside down. It must be off by default, and enabled
only in WFS and GetFeatureInfo

Btw, the Query javadoc should also state that the above hint can be used to break the
getPropertyNames() contract

Cheers
Andrea

–

Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy

phone: +39 0584 962313
fax: +39 0584 962313

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf