false positive when accessing TEI-export

[martinantonmueller]

To access the TEI-export I use this URL:

https://pmb.acdh.oeaw.ac.at/apis/entities/tei/org/50

where the last number is the entry-number. Problem is: I can use any existing entity number in the database, not just org-numbers, and get a TEI-result.

[gregorpirgie]

This should be an easy fix. At the moment all tei-endpoints fetch each object only by id and don't check if the object is actually an instance of the requested entity.

So this problem not only applies to orgs.

[gregorpirgie]

@martinantonmueller or @csae8092 – As I personally haven't used this endpoint yet: Is my assumption correct that orgs in the TEI-endpoint are Institutions in the apis-datamodell? And that each instance of the Institution-Model, regardless of the instance's kind (Institution-kind), should be a valid return value for the org-endpoint?

If my assumption is correct I will implement a check in each endpoint if the requested object is an instance of the expected entity or otherwise return a 404-Error. This would be the desired behaviour, right?

[csae8092]

@martinantonmueller or @csae8092 – As I personally haven't used this endpoint yet: Is my assumption correct that orgs in the TEI-endpoint are Institutions in the apis-datamodell? And that each instance of the Institution-Model, regardless of the instance's kind (Institution-kind), should be a valid return value for the org-endpoint?

If my assumption is correct I will implement a check in each endpoint if the requested object is an instance of the expected entity or otherwise return a 404-Error. This would be the desired behaviour, right?

yes, org is the tei equivalent of an apis-institution; and your entity-type check would work for me as well, so green light from my side, Thanks!

[martinantonmueller]

yes, your assumption is correct, it is wrong for all entity types, I can use https://pmb.acdh.oeaw.ac.at/apis/entities/tei/person/50 as well even though only "place/50" should lead to an output

[csae8092]

yes, your assumption is correct, it is wrong for all entity types, I can use https://pmb.acdh.oeaw.ac.at/apis/entities/tei/person/50 as well even though only "place/50" should lead to an output

@martinantonmueller as you are mainly the only user of this endpoint, are you ok with the proposed solution which would return a 404-error for something like https://pmb.acdh.oeaw.ac.at/apis/entities/tei/person/50 ?

[martinantonmueller]

yes, sure!

[gregorpirgie]

Added a pull-request with fix and assigned @csae8092 as reviewer. Should be ready to merge.

[csae8092]

unfortunately your solution @gregorpirgie would break some other feature:

we need to use get_object_from_pk_or_uri because otherwise we'd loose the current feature to fetch entities by once used IDs, i.e. entities which were merged into another entity but the old ID is still referenced somewhere in the wild.

so I guess you'll need to implement something like

...
res = get_object_from_pk_or_uri(request, pk)
entity = res.entity  # this is obviously some pseude code as I always forget how to get from the TempEntityClass to the actual entity class
if isinstance(entity, Person):
   continue
else:
   raise Http404

[gregorpirgie]

Alright, makes sense. Didn't know about that reasoning.