[xwiki-devs] [VOTE] Remove PR check for query languages other than XWQL and HQL
Hi devs, It seems that our SecureQueryManager [1] is preventing the execution of queries other than XWQL and HQL in the absence of PR. However, this is not at all a friendly policy when it comes to extensions. An example of where this is causing problems is Solr queries, where only users (well, document authors) with PR can execute them. As the subject says, I suggest removing this restriction and leaving rights check to be performed in each QueryExecutor's execute() method. The associated Jira issue is XWIKI-9386 [2] Here's my +1. Thanks, Eduard ---------- [1] https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwik... [2] http://jira.xwiki.org/browse/XWIKI-9386
It's not as simple as moving the ifs from SecureQueryExecutorManager to each QueryExecutor since the query executors don't know if they need to check rights. On Sun, Aug 4, 2013 at 7:34 PM, Eduard Moraru <[email protected]> wrote:
Hi devs,
It seems that our SecureQueryManager [1] is preventing the execution of queries other than XWQL and HQL in the absence of PR.
However, this is not at all a friendly policy when it comes to extensions. An example of where this is causing problems is Solr queries, where only users (well, document authors) with PR can execute them.
As the subject says, I suggest removing this restriction and leaving rights check to be performed in each QueryExecutor's execute() method.
The associated Jira issue is XWIKI-9386 [2]
Here's my +1.
Thanks, Eduard
---------- [1] https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwik... [2] http://jira.xwiki.org/browse/XWIKI-9386 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
We can add another marker interface, PrivilegedQueryExecutor or something like that, which informs that it can execute privileged queries, so it's up to the query manager to prevent them from running in an unprivileged environment. On 08/05/2013 03:16 AM, Thomas Mortagne wrote:
It's not as simple as moving the ifs from SecureQueryExecutorManager to each QueryExecutor since the query executors don't know if they need to check rights.
On Sun, Aug 4, 2013 at 7:34 PM, Eduard Moraru <[email protected]> wrote:
Hi devs,
It seems that our SecureQueryManager [1] is preventing the execution of queries other than XWQL and HQL in the absence of PR.
However, this is not at all a friendly policy when it comes to extensions. An example of where this is causing problems is Solr queries, where only users (well, document authors) with PR can execute them.
As the subject says, I suggest removing this restriction and leaving rights check to be performed in each QueryExecutor's execute() method.
The associated Jira issue is XWIKI-9386 [2]
Here's my +1.
Thanks, Eduard
-- Sergiu Dumitriu http://purl.org/net/sergiu
I don`t understand. Why not make it the contract of the QueryExecutor that, if it manages possibly sensitive data, it is in charge of checking rights? This is how we do it for Solr. Thanks, Eduard On Mon, Aug 5, 2013 at 1:45 PM, Sergiu Dumitriu <[email protected]> wrote:
We can add another marker interface, PrivilegedQueryExecutor or something like that, which informs that it can execute privileged queries, so it's up to the query manager to prevent them from running in an unprivileged environment.
On 08/05/2013 03:16 AM, Thomas Mortagne wrote:
It's not as simple as moving the ifs from SecureQueryExecutorManager to each QueryExecutor since the query executors don't know if they need to check rights.
On Sun, Aug 4, 2013 at 7:34 PM, Eduard Moraru <[email protected]> wrote:
Hi devs,
It seems that our SecureQueryManager [1] is preventing the execution of queries other than XWQL and HQL in the absence of PR.
However, this is not at all a friendly policy when it comes to extensions. An example of where this is causing problems is Solr queries, where only users (well, document authors) with PR can execute them.
As the subject says, I suggest removing this restriction and leaving rights check to be performed in each QueryExecutor's execute() method.
The associated Jira issue is XWIKI-9386 [2]
Here's my +1.
Thanks, Eduard
-- Sergiu Dumitriu http://purl.org/net/sergiu _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
1. So that we don't duplicate the same login in multiple places. 2. So that we don't create too many dependencies between modules (not every module should have to deal with programming rights, or rights in general) 3. So that security bugs aren't introduced when someone forgets to check the rights, or if the rights checking mechanism changes (as it is likely to happen soon for programming rights). This is a direct conclusion of the first item, though... 4. A contract when it comes to code is just saying somewhere in a javadoc that "this method must check rights". This is a soft contract, it relies on the attention and patience of the implementor (he should first read the javadoc, then he should actually implement the extra items of this contract). An interface is a hard contract between the implementation and the manager, and only one part needs to implement the contract, the manager. On 08/05/2013 12:39 PM, Eduard Moraru wrote:
I don`t understand. Why not make it the contract of the QueryExecutor that, if it manages possibly sensitive data, it is in charge of checking rights?
This is how we do it for Solr.
Thanks, Eduard
On Mon, Aug 5, 2013 at 1:45 PM, Sergiu Dumitriu <[email protected]> wrote:
We can add another marker interface, PrivilegedQueryExecutor or something like that, which informs that it can execute privileged queries, so it's up to the query manager to prevent them from running in an unprivileged environment.
On 08/05/2013 03:16 AM, Thomas Mortagne wrote:
It's not as simple as moving the ifs from SecureQueryExecutorManager to each QueryExecutor since the query executors don't know if they need to check rights.
On Sun, Aug 4, 2013 at 7:34 PM, Eduard Moraru <[email protected]> wrote:
Hi devs,
It seems that our SecureQueryManager [1] is preventing the execution of queries other than XWQL and HQL in the absence of PR.
However, this is not at all a friendly policy when it comes to extensions. An example of where this is causing problems is Solr queries, where only users (well, document authors) with PR can execute them.
As the subject says, I suggest removing this restriction and leaving rights check to be performed in each QueryExecutor's execute() method.
The associated Jira issue is XWIKI-9386 [2]
Here's my +1.
Thanks, Eduard
-- Sergiu Dumitriu http://purl.org/net/sergiu
I agree, but even if you have an interface to respect, there's nothing stopping the implementer to just "return true/null" in the method's body instead of doing it right, if you want to bring up contract breaches :) I really don`t see the benefits of marking the query executor as PrivilegedQueryExecutor since that would be the case for *all* our current query languages (rights need to be checked anyway). Also, let's say we do it this way... what will the query manager check? In our current state, the result is a List of undefined java.lang.objects, since every query implementation returns various results. Until we standardize that, the query manager can not inspect each result to verify rights. Also, using PR to allow or disallow the query completely is an unacceptable solution. My other suggestion, if you checked the jira [1], was to add some kind of QueryResult interface which the manager can use to inspect each item returned by the query and check the rights at top level. However, the problem I see with this approach is that we would have to define the list of possible result types and this can get messy fast: - list of documents (hql, xwql, solr) - list of objects (solr) - list of one or more random document fields (privileged hql or xwql query, i.e.: 'SELECT doc.space, doc.wiki FROM ....') - queries in custom table (hql, xwql) - other language specific results Any other ideas? Thanks, Eduard ---------- [1] http://jira.xwiki.org/browse/XWIKI-9386 On Mon, Aug 5, 2013 at 10:12 PM, Sergiu Dumitriu <[email protected]> wrote:
1. So that we don't duplicate the same login in multiple places. 2. So that we don't create too many dependencies between modules (not every module should have to deal with programming rights, or rights in general) 3. So that security bugs aren't introduced when someone forgets to check the rights, or if the rights checking mechanism changes (as it is likely to happen soon for programming rights). This is a direct conclusion of the first item, though... 4. A contract when it comes to code is just saying somewhere in a javadoc that "this method must check rights". This is a soft contract, it relies on the attention and patience of the implementor (he should first read the javadoc, then he should actually implement the extra items of this contract). An interface is a hard contract between the implementation and the manager, and only one part needs to implement the contract, the manager.
On 08/05/2013 12:39 PM, Eduard Moraru wrote:
I don`t understand. Why not make it the contract of the QueryExecutor that, if it manages possibly sensitive data, it is in charge of checking rights?
This is how we do it for Solr.
Thanks, Eduard
On Mon, Aug 5, 2013 at 1:45 PM, Sergiu Dumitriu <[email protected]> wrote:
We can add another marker interface, PrivilegedQueryExecutor or something like that, which informs that it can execute privileged queries, so it's up to the query manager to prevent them from running in an unprivileged environment.
On 08/05/2013 03:16 AM, Thomas Mortagne wrote:
It's not as simple as moving the ifs from SecureQueryExecutorManager to each QueryExecutor since the query executors don't know if they need to check rights.
On Sun, Aug 4, 2013 at 7:34 PM, Eduard Moraru <[email protected]> wrote:
Hi devs,
It seems that our SecureQueryManager [1] is preventing the execution of queries other than XWQL and HQL in the absence of PR.
However, this is not at all a friendly policy when it comes to extensions. An example of where this is causing problems is Solr queries, where only users (well, document authors) with PR can execute them.
As the subject says, I suggest removing this restriction and leaving rights check to be performed in each QueryExecutor's execute() method.
The associated Jira issue is XWIKI-9386 [2]
Here's my +1.
Thanks, Eduard
-- Sergiu Dumitriu http://purl.org/net/sergiu _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
participants (3)
-
Eduard Moraru -
Sergiu Dumitriu -
Thomas Mortagne