> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java,
> >  line 38
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204573#file1204573line38>
> >
> >     Could you explain why this is the case?

There's no why, that's just what it does.  It's an arbitrary way of generating 
documents.


> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java,
> >  line 117
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line117>
> >
> >     Could you add more comments on this method? It is very long.

added some more comments and restructed a bit.  Should be clearer now.


> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java,
> >  line 107
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line107>
> >
> >     If user is super user, does not need to add filter? Could you add 
> > comments here?

done.


> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java,
> >  line 165
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line165>
> >
> >     Spaces.

done.


> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java,
> >  line 142
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line142>
> >
> >     Check for null pointer here?

done.


> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml,
> >  line 1364
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204577#file1204577line1364>
> >
> >     Remove extra spaces. Same as other places.

cleaned up some; I didn't change this in the commit by the way and these are 
for the most part, example configs from solr that have the spacing issues.


> On Feb. 16, 2016, 8:25 p.m., Hao Hao wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java,
> >  line 118
> > <https://reviews.apache.org/r/42612/diff/1/?file=1204571#file1204571line118>
> >
> >     How about superusers case in prepare, since there is no adding of 
> > AddDocIdReturnFields if are superusers there?

Good point, changed this to a log and added a test.


- Gregory


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42612/#review119348
-----------------------------------------------------------


On Jan. 21, 2016, 8:02 p.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42612/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2016, 8:02 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> RealTimeGet just ignores filter queries currently in Solr (see SOLR-8436) 
> which is how document level security is implemented, so if you can guess the 
> document ids, you can access them.
> 
> Since we probably don't want to wait for a solr version with SOLR-8436 to be 
> released, this is a "temporary" workaround and some necessary testing.
> 
> At a high level this works as follows:
> - Run the normal RealTimeGet component
> - Filter the responses from the component through the Filter generated from 
> the doc-level component
> 
> Most of this is low-level solr/lucene code; most of the meat is in the 
> testing (TestRealTimeGet.java).
> 
> 
> Diffs
> -----
> 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java
>  PRE-CREATION 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java
>  371787df69cfe8ef891eb1760b32f93f0a1110ec 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java
>  2495a9eecc00e8de4b297022625b33a98ad7323a 
>   
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java
>  ff508e12898ab0bf9e79f0cc8e1108e4a5ef82ad 
>   
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/schema.xml
>  66449ffe59b459352f8a735a208f020e48f0d9b4 
>   
> sentry-tests/sentry-tests-solr/src/test/resources/solr/collection1/conf/solrconfig-doclevel.xml
>  4459c0d04c62aa39c096da0faba7ff04fc2bf21b 
>   
> sentry-tests/sentry-tests-solr/src/test/resources/solr/sentry/test-authz-provider.ini
>  bccc63eeeab503f7e5d3655771eb0c7bef926bba 
> 
> Diff: https://reviews.apache.org/r/42612/diff/
> 
> 
> Testing
> -------
> 
> Ran the unit tests.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>

Reply via email to