enonic / doc-xp

Enonic XP > 7.0 Reference Documentation
2 stars 4 forks source link

document multirepoconnect.query with sources set to an empty array queries ALL REPOS #160

Closed ComLock closed 3 years ago

ComLock commented 4 years ago

If this is true seems like a major security issue to me. I discovered it while working on https://github.com/enonic/app-explorer/issues/91 when querying an Explorer interface without any collections (repos) selected.

doc: https://developer.enonic.com/docs/xp/stable/api/lib-node#multirepoconnect

rymsha commented 4 years ago

I don't see it as a security issue - if user doing a query has read rights to read all repositories then empty array is just a shortcut to specify each and every repository.

Seems like multirepo query just replicates what elasticsearch does by default - if no indices are specified, search in all indices.

ComLock commented 4 years ago

Well the thing is you can run queries in a context. When programming that context often contain system:admin.

For instance in Explorer there is a context with read access for the explorer application, the user context is not applied anywhere.

Typically a query is provided on an contentstudio url. That url may or may not be protected by some permissions. Those permissions are however not relevant for the query itself. Adding premission granularity to the collections, would require them to be recollected/reindexed on every permission change.

rymsha commented 4 years ago

Well the thing is you can run queries in a context. When programming that context often contain system:admin.

This is a choice of an application that uses an API. Application itself can prohibit search in all (disallowing empty sources before doing a query)

Adding premission granularity to the collections, would require them to be recollected/reindexed on every permission change.

There are permission groups that can have roles assigned. Modification of a group does not trigger reindexing of entire data.

ComLock commented 4 years ago

disallowing empty sources before doing a query

Which is what I have done now.

I just dislike the hidden implicit thing that nothing suddenly means everything! I am explicitly saying search nothing, and the server responds: here you go, here is everything.

There are permission groups that can have roles assigned. Modification of a group does not trigger reindexing of entire data.

Yes, I have three groups explorer.admin, explorer.write and explorer.read In the future perhaps I need more granularity. I could make an admin ui to add roles per collection.

rymsha commented 4 years ago

Functionality won't change as it is likely to be desired one - how to specify to search in all repositories.

alansemenov commented 4 years ago

@sigdestad Should we allow empty array in sources as a way to connect to all available repos or is this a bug?

sigdestad commented 4 years ago

Sounds natural to me, what other options do we have? Also, changing this would effectively break API I guess?