Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unclear whether data is missing or unavailable because of security #151

Open
pjhaest opened this issue Feb 4, 2019 · 7 comments
Open

Unclear whether data is missing or unavailable because of security #151

pjhaest opened this issue Feb 4, 2019 · 7 comments
Assignees
Labels
dov Related to the DOV webservices

Comments

@pjhaest
Copy link
Contributor

pjhaest commented Feb 4, 2019

A special one I guess:
Looking for the 'Boring' of a 'Filter' I came across the following (took quite some time before I figured it out):
The filter is n°1 of peilput 3-0513b (https://www.dov.vlaanderen.be/data/filter/2005-007148)
Parsing the Filter xml I retrieve the gw_id. This usually maps to the 'putnummer' in the Boring search. But in this case, there is no 'boring' for that 'peilput'.
When querying Boring for property 'putnummer', you get no result, or error of server timeout

boring = BoringSearch()
query = PropertyIsEqualTo(propertyname='putnummer', literal='3-0513b')
df = boring.search(query=query, return_fields=('pkey_boring',))

Maybe it's a good idea to give some more feedback on what goes wrong if a query is 'valid' but gives no result? Because you have to admit that a 'filter', without a 'boring' is something special, if you don't know that it all actually maps to 'put' (if I remember correctly)?

@pjhaest pjhaest changed the title Error when no response on inconsistency in database Error when no response on unexpected results in database Feb 4, 2019
@Roel Roel added the dov Related to the DOV webservices label Feb 4, 2019
@Roel Roel self-assigned this Feb 4, 2019
@Roel
Copy link
Member

Roel commented Feb 5, 2019

Interesting case.. :)

Let me start with what you're trying to achieve: linking Filters to Boringen. This can be done without the 'putnummer' route, using the 'boringfiche' field of the Filter:

fs = GrondwaterFilterSearch()
fs.get_fields()['boringfiche']

{'cost': 1,
'definition': "URL die verwijst naar de gegevens van de boring op de website. Voeg '.xml' toe om een XML voorstelling van deze gegevens te verkrijgen. Wanneer dit veld leeg is is de put niet gekoppeld aan een boring.",
'name': 'boringfiche',
'notnull': False,
'query': True,
'type': 'string'}

If we would be able to Join using different column names (#152), you could use this to retrieve Boring information:

fs = GrondwaterFilterSearch()
bs = BoringSearch()

df_filters = fs.search(location=Box()), return_fields=('pkey_filter', 'boringfiche')
df_boringen = bs.search(query=Join(df_filters, on='pkey_boring', using='boringfiche'))

As you noticed, this is not a mandatory field. In our datamodel a Filter is always linked to a Grondwaterlocatie (in this case: Put), but the link between Put and Boring is optional. This is listed as such in the output of get_fields(), see the definition above and the 'notnull': False.

However, you have a special case here:

df = fs.search(
    query=PropertyIsEqualTo(
        'pkey_filter',
        'https://www.dov.vlaanderen.be/data/filter/2005-007148'),
    return_fields=('pkey_filter', 'pkey_grondwaterlocatie', 'boringfiche')
)

print(df.iloc[0].pkey_filter)
print(df.iloc[0].pkey_grondwaterlocatie)
print(df.iloc[0].boringfiche)

https://www.dov.vlaanderen.be/data/filter/2005-007148
https://www.dov.vlaanderen.be/data/put/2017-003385
https://www.dov.vlaanderen.be/data/boring/2005-094289

The 'boringfiche' is not null, so the Put is linked to a Boring. But if you'd search for this Boring, you receive an empty dataframe:

bs = BoringSearch()
df = bs.search(
    query=PropertyIsEqualTo(
        'pkey_boring',
        'https://www.dov.vlaanderen.be/data/boring/2005-094289'
    )
)

len(df.index)
> 0

Turns out this Boring is not public, so it is not available is our public service because of security settings. To align with the webview, we should not include the pkey of the boring in the 'boringfiche' field of the Filter service it that Boring is not public.

While digging through this, I also found that the 'putnummer' field of the Boring service is unreliable (will be fixed soonish).

To summarize:

  • A Filter is always linked to a Grondwaterlocatie (in this case: Put), but a Put is not always linked to a Boring.

  • Because of different security settings of the Put and Boring objects, it can happen that you don't see the Boring or Put because it is not public. This does not mean that there is none.

  • Right now, the 'boringfiche' field of the Filter misses a security check on our side, so includes permalinks to non-public Boring objects. To align with the webviews, I will add this check and as a result these links will disappear.

  • The 'putnummer' of the Boring service is currently unreliable. I will fix this and follow up here. For now it is better to avoid this and to use the 'boringfiche' of the Filter service instead.

  • It would be nice to be able to Join using different column names (Support Joins where column name is different from field #152), however I agree that more consistency in naming would be beneficial. Follow-up in Support Joins where column name is different from field #152.

@pjhaest
Copy link
Contributor Author

pjhaest commented Feb 5, 2019

👍 very thorough explanation, thanks!

I suggest to include a feedback to the user in case of 'security issues' blocking access to some data, instead of simply removing them from the output? It's not that the data don't exist, they are only not available without permission.
This feedback can be a certain abstract code instead of the pkey, or a pkey to an xml with the fields indicating 'permission required' or something alike?

@Roel
Copy link
Member

Roel commented Feb 5, 2019

While I agree that it would be good to be able to make the difference between 'data does not exists' and 'data exists but I'm not allowed to see it', this is not trivial at all and raises new questions:

  • First of all: this is everywhere, all of our object types and most subtypes have security on them (for instance Boring, Sondering, all interpretation types, Grondwaterlocatie (Put), Filter, all monster types, observations, Opdracht, ...). Currently (bugs aside), everything goes through the security filter, removing things you don't have access to. To keep everything constistent, all services and all webviews would need to be changed.

  • While we can include foreign keys with permalinks (like the 'boringfiche' of the Filter service) without causing too much issues, it requires resolving or joining to the Boring service to determine whether it is accessible.

  • What to do with fields that are derived from an object? For example the 'putnummer' of the Boring service, what to do when the Put exists but is not accessible? Same goes for Opdracht. We need to make the distinction between a valid putnummer and hidden data in a standardised, machine-readably manner.

  • Sometimes everything is inaccessible (f.ex. internal interpretation linked to internal Boring) and has no incoming links. I assume we don't want empty Boring objects linked to empty Interpretation objects.

  • We could use something like xsi:nil='True' and nilReason to indicate some hidden data (INSPIRE has something like http://inspire.ec.europa.eu/codelist/VoidReasonValue/Withheld for this). However, I'm not sure if this is supported in WFS < 2.0.0 and it for sure is not available in our XML services at the moment. This would also only work for hidden fields in non-hidden data, and not for objects that are entirely inaccessible and not linked to from anywhere.

@pjhaest
Copy link
Contributor Author

pjhaest commented Feb 5, 2019

:-( I was hoping it wouldn't be that difficult. Let's keep it to a warning in the docs than?

@Roel
Copy link
Member

Roel commented Feb 5, 2019

We can do that for sure, please go ahead!

I'm open to discuss about this issue at a next codesprint, but this is certainly something that needs thaughtful consideration and cannot be changed overnight.

@Roel Roel changed the title Error when no response on unexpected results in database Unclear whether data is missing or unavailable because of security Feb 5, 2019
@pjhaest pjhaest self-assigned this Feb 5, 2019
@Roel
Copy link
Member

Roel commented Mar 1, 2019

I have added the security check on Boring in the Filter (meetnetten) service. To align with the other services and the webviews, 'boornummer' and 'boringfiche' will be null when the Boring is not available publicly.

Still to do: fix the 'putnummer' field of the Boring service.

@Roel
Copy link
Member

Roel commented Apr 4, 2019

The 'putnummer' field of the Boring service has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dov Related to the DOV webservices
Projects
None yet
Development

No branches or pull requests

2 participants