Reference annotations require access to 'parent chain' up to projects, which may be forbidden

atruskie commented 9 years ago

:cry:

Related to:

Scenario: moving from nested library API to chatty API. Problem: Some audio events (reference annotations only) are returned despite the user not having permission to access that project (correct behaviour). Unfortunately this means that requesting resources for these reference annotations (like AudioRecordings, Sites, and Projects) are forbidden... which means we can't get sufficient data to show in the library.

Suggested solution: Take the security bypass implemented in #154 and #97 and generalize it so it works for all resources in the main security chain (Projects, Sites, AudioRecordings, AudioEvents, ?AudioEventsTags?, AudioEventsComments, and Media).

To recap, it basically works by: iff an audio_event_id QSP is attached to the request AND iff that audio event is_reference == true, then allow access to resource.

Additionally, if possible, I think it would be a good idea to limit the available information for any reference annotation resource. For most resources I generally need their ids, their normal parent/children ids (where appropriate), and a name for display purposes. Other information should not be included.

Lastly, as this concept is becoming more generalised, it might be helpful to stick a flag in the meta section of API responses when security limited (i.e. as per previous paragraph) resources are returned. Something like "meta":{ "security": { "restricted_resource": true } } }

atruskie commented 9 years ago

Currently blocking: https://github.com/QutBioacoustics/baw-client/issues/190

cofiem commented 9 years ago

Whoa. You really know how to find the tough bugs... I'm not sure how to address this.

cofiem commented 9 years ago

I have 2 counter proposals to what is essentially a complete re-work of our current security system:

Option 1: Why is showing this information even relevant/useful? Even if you have ids/names, clicking on them will go to a forbidden page. That seems silly. I agree that some information like absolute date/time info from audio recordings is important. So maybe access to the audio recording could be done in a similar way, if it isn't already.

Option 2: Perhaps adding a couple of custom fields to the annotation endpoint is a reasonable compromise? This depends on the amount of information that we need/want to add.

atruskie commented 9 years ago

So for option 2 at least, it is pretty much what we used to do. Technically, as far as I am concerned, it was a security breach - depends on how well you wrote the tests (I haven't looked). This proposal merely formalizes getting information that was already available. As far as the API consistency goes, such a large change... is not what I could call a compromise. Anyway, given the hacks already in audio_events_comments and media... I saw the generalization as more consistent.

Option 1: as I said above, merely replicating features of old library, but

Currently in client, I can build up a chain of associations... a virtual model... having these entities is required to make links to resources
even if resources are forbidden, returning some information
- allows us to link to / send user to appropriate warning pages (i.e. forbidden project's page)
- which allows for friendly user error messages
- Is easier to work with, programmatically at least... allows me the technical access I should have (given the security chain) so unexpected things don't happen (i.e. filter queries not returning expected items but still 200 OKing)

atruskie commented 9 years ago

Oh! I liked my title! :stuck_out_tongue_winking_eye:

cofiem commented 9 years ago

So, am I correct in saying that your suggestion means that for any project/site/audiorecording/etc that contains at least one audio event that has is_reference == true, then anyone logged in can access the id/name? so, the endpoints effectively have 2 categories of security: project permissions (full/no access) and restricted access (based on whether it contains one or more reference audio events or not)?

I do agree generalising the hacks for media and audio_events & co is better, but also very complex.

atruskie commented 9 years ago

This may be a non-issue. Just talking to zeroday... the way the library works currently is not real good.

Will update soon.

atruskie commented 9 years ago

So @jwim1 has suggested that the correct way for the library to work is to not show or link to any information that is forbidden.

It will create an inconsistent interface but site and project names are sensitive apparently.

I think at this stage, we'll close this issue until we get further feedback,

jwim1 commented 9 years ago

So says Zeroday! On 25 Jun 2015 12:08 pm, "Anthony Truskinger" notifications@github.com wrote:

So @jwim1 https://github.com/jwim1 has suggested that the correct way for the library to work is to not show or link to any information that is forbidden.

It will create an inconsistent interface but site and project names are sensitive apparently.

I think at this stage, we'll close this issue until we get further feedback,

— Reply to this email directly or view it on GitHub https://github.com/QutBioacoustics/baw-server/issues/216#issuecomment-115071224 .

QutEcoacoustics / baw-server

Reference annotations require access to 'parent chain' up to projects, which may be forbidden #216