> 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 > >
