You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hrishikesh Gadre via Review Board <no...@reviews.apache.org> on 2019/01/07 21:37:25 UTC
Re: Review Request 69619: SENTRY-2482: Sentry Solr to support
multi-attribute document level security
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69619/#review211740
-----------------------------------------------------------
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/CachingUserAttributeSource.java
Lines 34 (patched)
<https://reviews.apache.org/r/69619/#comment297327>
Can we use an integer constant instead?
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSource.java
Lines 24 (patched)
<https://reviews.apache.org/r/69619/#comment297322>
Please add java docs for the interface.
sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSourceParams.java
Lines 21 (patched)
<https://reviews.apache.org/r/69619/#comment297324>
Please add javadocs
- Hrishikesh Gadre
On Dec. 21, 2018, 3:54 p.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69619/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2018, 3:54 p.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This is an improvement request to cover enhanced document level security for the Solr document level controls, specifically to cover:
>
> - Security controls against multiple fields
> - Filters based on user attributes as well as just Sentry roles
> - Different security predicates (AND, LessThan, GreaterThan - in addition to the currently supported OR)
> - Pluggable user attribute source ahead of Sentry enhancements.
> - Sample LDAP user attribute source
>
> The ambition is this will be a precursor to full complex predicate support being served by Sentry ABAC roadmap.
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/pom.xml 621d8325
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/CachingUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/DocAuthorizationComponent.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/FieldToAttributeMapping.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/LdapUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/LdapUserAttributeSourceParams.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 9da3d6e1
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SolrAttrBasedFilter.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SubsetQueryPlugin.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSourceParams.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/CachingUserAttributeSourceTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/LdapRegexTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/MockUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/SubsetQueryTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/schema-docValuesSubsetMatch.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig-subsetmatchcomponent.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig-subsetquery.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig.snippet.randomindexconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/pom.xml 7c28bda5
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestCase.java 3d4d555f
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java 40cc153e
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/SolrSentryServiceTestBase.java e1f789cb
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAbacOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java 7834f339
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSubsetQueryOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/ldap/ldap.ldiff PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/ldap/ldap.schema PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/log4j.properties d9418167
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/enumsConfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/solrconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match/conf/solrconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match_missing_false/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match_missing_false/conf/solrconfig.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69619/diff/1/
>
>
> Testing
> -------
>
> Unit tests created for CachingUserAttributeSource, for parts of the LDAPAttributeSource and for the SubsetQueryPlugin.
>
> End-to-end JUnit tests created which cover the revised QueryDocAuthorizationComponent and also the full functionality of SolAttrBasedFilter, the latter including an embedded LDAP server to test full capability.
>
>
> Thanks,
>
> Tristan Stevens
>
>
Re: Review Request 69619: SENTRY-2482: Sentry Solr to support
multi-attribute document level security
Posted by Tristan Stevens via Review Board <no...@reviews.apache.org>.
> On Jan. 7, 2019, 9:37 p.m., Hrishikesh Gadre wrote:
> >
Updated, including some revisions from zsgyulavari
> On Jan. 7, 2019, 9:37 p.m., Hrishikesh Gadre wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/CachingUserAttributeSource.java
> > Lines 34 (patched)
> > <https://reviews.apache.org/r/69619/diff/1/?file=2115663#file2115663line34>
> >
> > Can we use an integer constant instead?
I'll add a comment. This String should be of the format specified in https://google.github.io/guava/releases/14.0/api/docs/com/google/common/cache/CacheBuilderSpec.html. It's quite possible that someone would want to specify a more detailed cache. I'm not sure there's value in deconstructing the spec as a constant.
> On Jan. 7, 2019, 9:37 p.m., Hrishikesh Gadre wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSource.java
> > Lines 24 (patched)
> > <https://reviews.apache.org/r/69619/diff/1/?file=2115671#file2115671line24>
> >
> > Please add java docs for the interface.
Will add to next version of patch
> On Jan. 7, 2019, 9:37 p.m., Hrishikesh Gadre wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSourceParams.java
> > Lines 21 (patched)
> > <https://reviews.apache.org/r/69619/diff/1/?file=2115672#file2115672line21>
> >
> > Please add javadocs
Will add to next version of patch.
- Tristan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69619/#review211740
-----------------------------------------------------------
On Dec. 21, 2018, 3:54 p.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69619/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2018, 3:54 p.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This is an improvement request to cover enhanced document level security for the Solr document level controls, specifically to cover:
>
> - Security controls against multiple fields
> - Filters based on user attributes as well as just Sentry roles
> - Different security predicates (AND, LessThan, GreaterThan - in addition to the currently supported OR)
> - Pluggable user attribute source ahead of Sentry enhancements.
> - Sample LDAP user attribute source
>
> The ambition is this will be a precursor to full complex predicate support being served by Sentry ABAC roadmap.
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/pom.xml 621d8325
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/CachingUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/DocAuthorizationComponent.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/FieldToAttributeMapping.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/LdapUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/LdapUserAttributeSourceParams.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 9da3d6e1
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SolrAttrBasedFilter.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SubsetQueryPlugin.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSourceParams.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/CachingUserAttributeSourceTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/LdapRegexTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/MockUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/SubsetQueryTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/schema-docValuesSubsetMatch.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig-subsetmatchcomponent.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig-subsetquery.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig.snippet.randomindexconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/pom.xml 7c28bda5
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestCase.java 3d4d555f
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java 40cc153e
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/SolrSentryServiceTestBase.java e1f789cb
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAbacOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java 7834f339
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSubsetQueryOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/ldap/ldap.ldiff PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/ldap/ldap.schema PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/log4j.properties d9418167
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/enumsConfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/solrconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match/conf/solrconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match_missing_false/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match_missing_false/conf/solrconfig.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69619/diff/1/
>
>
> Testing
> -------
>
> Unit tests created for CachingUserAttributeSource, for parts of the LDAPAttributeSource and for the SubsetQueryPlugin.
>
> End-to-end JUnit tests created which cover the revised QueryDocAuthorizationComponent and also the full functionality of SolAttrBasedFilter, the latter including an embedded LDAP server to test full capability.
>
>
> Thanks,
>
> Tristan Stevens
>
>
Re: Review Request 69619: SENTRY-2482: Sentry Solr to support
multi-attribute document level security
Posted by Hrishikesh Gadre via Review Board <no...@reviews.apache.org>.
> On Jan. 7, 2019, 9:37 p.m., Hrishikesh Gadre wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/CachingUserAttributeSource.java
> > Lines 34 (patched)
> > <https://reviews.apache.org/r/69619/diff/1/?file=2115663#file2115663line34>
> >
> > Can we use an integer constant instead?
>
> Tristan Stevens wrote:
> I'll add a comment. This String should be of the format specified in https://google.github.io/guava/releases/14.0/api/docs/com/google/common/cache/CacheBuilderSpec.html. It's quite possible that someone would want to specify a more detailed cache. I'm not sure there's value in deconstructing the spec as a constant.
I don't really like the idea of exposing the details of cacheing implementation to the end user. e.g. what if we want to refactor the implementation to use some other caching library. But if you really want to maintain the flexibility configuring arbitrary cache properties, you can use a design pattern that I used for configuring Solr/Sentry plugin. Basically we allow user to specify arbitrary config properties (and optionally default values as well). You could implement something similar.
https://github.com/apache/sentry/blob/3ba5998b87595ea158d1d3b71b00cbcdb4092a0a/sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java#L71-L91
https://github.com/apache/sentry/blob/3ba5998b87595ea158d1d3b71b00cbcdb4092a0a/sentry-tests/sentry-tests-solr/src/test/resources/solr/security/security.json
- Hrishikesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69619/#review211740
-----------------------------------------------------------
On Jan. 8, 2019, 2:08 p.m., Tristan Stevens wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69619/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2019, 2:08 p.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This is an improvement request to cover enhanced document level security for the Solr document level controls, specifically to cover:
>
> - Security controls against multiple fields
> - Filters based on user attributes as well as just Sentry roles
> - Different security predicates (AND, LessThan, GreaterThan - in addition to the currently supported OR)
> - Pluggable user attribute source ahead of Sentry enhancements.
> - Sample LDAP user attribute source
>
> The ambition is this will be a precursor to full complex predicate support being served by Sentry ABAC roadmap.
>
>
> Diffs
> -----
>
> sentry-solr/solr-sentry-handlers/pom.xml 621d8325
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/CachingUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/DocAuthorizationComponent.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/FieldToAttributeMapping.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/LdapUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/LdapUserAttributeSourceParams.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java 9da3d6e1
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SolrAttrBasedFilter.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SubsetQueryPlugin.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/UserAttributeSourceParams.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/CachingUserAttributeSourceTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/LdapRegexTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/MockUserAttributeSource.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/SubsetQueryTest.java PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/schema-docValuesSubsetMatch.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig-subsetmatchcomponent.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig-subsetquery.xml PRE-CREATION
> sentry-solr/solr-sentry-handlers/src/test/resources/solr/collection1/solrconfig.snippet.randomindexconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/pom.xml 7c28bda5
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestCase.java 3d4d555f
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java 40cc153e
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/SolrSentryServiceTestBase.java e1f789cb
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestAbacOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java 7834f339
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSubsetQueryOperations.java PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/ldap/ldap.ldiff PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/ldap/ldap.schema PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/log4j.properties d9418167
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/enumsConfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_abac/conf/solrconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match/conf/solrconfig.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match_missing_false/conf/schema.xml PRE-CREATION
> sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_subset_match_missing_false/conf/solrconfig.xml PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/69619/diff/2/
>
>
> Testing
> -------
>
> Unit tests created for CachingUserAttributeSource, for parts of the LDAPAttributeSource and for the SubsetQueryPlugin.
>
> End-to-end JUnit tests created which cover the revised QueryDocAuthorizationComponent and also the full functionality of SolAttrBasedFilter, the latter including an embedded LDAP server to test full capability.
>
>
> Thanks,
>
> Tristan Stevens
>
>