> On Nov. 3, 2017, 10:33 p.m., kalyan kumar kalvagadda wrote: > > There are couple of missing pieces > > 1. AuditLogger does not reacord all the the authorization requests > > 2. All the tests using filr based provider as removed. > > 3. Kerberos support for File based provider. > > > > You need not address them in this patch but please open jira's for them and > > link them to this jira for reference.
I am working on supporting file based provider as well. Regarding (1), I will file a SOLR jira. > On Nov. 3, 2017, 10:33 p.m., kalyan kumar kalvagadda wrote: > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java > > Lines 188-191 (patched) > > <https://reviews.apache.org/r/63250/diff/4/?file=1879944#file1879944line188> > > > > class PermissionNameProvider, dpesn't have proper documentation > > satating what each permission means. It would be great if you could > > document it so that it will be easy for the sentry understand/maintain/use > > solr plug-in Filed SOLR-10388 > On Nov. 3, 2017, 10:33 p.m., kalyan kumar kalvagadda wrote: > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrPolicyEngineLocalFS.java > > Lines 27 (patched) > > <https://reviews.apache.org/r/63250/diff/4/?file=1879967#file1879967line27> > > > > There are no tests use this class. we discussed about it. Please raise > > a jira for this and link it to this jira. The tests are implemented in the base class (AbstractTestSolrPolicyEngine). This class inherits them via the base class. > On Nov. 3, 2017, 10:33 p.m., kalyan kumar kalvagadda wrote: > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrModelAuthorizable.java > > Lines 21 (patched) > > <https://reviews.apache.org/r/63250/diff/4/?file=1879995#file1879995line21> > > > > why can't you change SolrModelAuthorizable into a abstract class and > > move common code in all the child classes like > > Admin,Collection,Config,Field,Schema into this class. > > > > When you do that your child class looks something like this. > > > > public class Field extends SolrModelAuthorizable { > > /** > > * Represents all fields > > */ > > public static final Field ALL = new Field(SolrConstants.ALL); > > > > public Field(String name) { > > super(name); > > } > > @Override > > public String toString() { > > return "Field [name=" + name + "]"; > > } > > @Override > > public String getTypeName() { > > return AuthorizableType.Field.name(); > > } > > } > > > > Currently there is duplicate code in all the > > Admin,Collection,Config,Field and Schema classes. I think it is a good suggestion. But it is not related to the current patch (since I am following the same patten as the current codebase) and hence can be handled separately. - Hrishikesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63250/#review190068 ----------------------------------------------------------- On Nov. 3, 2017, 9:49 p.m., Hrishikesh Gadre wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63250/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2017, 9:49 p.m.) > > > Review request for sentry and Sergio Pena. > > > Bugs: SENTRY-1475 > https://issues.apache.org/jira/browse/SENTRY-1475 > > > Repository: sentry > > > Description > ------- > > - Upgraded Solr to latest version (v7.1.0) > - Introduced following new authorizable entity types, > - Admin (to authorize various cluster admin operations) > - Note that privilege configuration for Solr collections admin > operations have changed from "collection=admin->action= > <some_action>" to "admin=collections->action=<some_action>". A migration tool > will be provided as part of SENTRY-1480). > - Config (to authorize various Solr collection configuration operations) > - Schema (to authorize Solr schema changes) > - Renamed sentry-core-model-search module to sentry-core-model-solr to > reflect the fact that it represents logic for Apache SOLR and is consistent > with naming convention used for other components (e.g. kafka) > - Moved the Solr audit log functionality to sentry-binding-solr module so > that it can be integrated with the Solr/Sentry authorization plugin. > - Deleted all the custom Solr request handlers in the > sentry-solr/solr-sentry-handlers module since the Solr authorization plugin > handles the authorization of all solr requests. This module now contains just > the functionality required for implementing Solr document level security. > > > Diffs > ----- > > pom.xml af54480 > sentry-binding/sentry-binding-solr/pom.xml ed2624b > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrAuthorizationException.java > 938dbfd > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java > 0a818e5 > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzUtil.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/conf/SolrAuthzConf.java > 37efa5b > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/solr/sentry/AuditLogger.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/solr/sentry/RollingFileWithoutDeleteAppender.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/HdfsTestUtil.java > 859c793 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java > 7a88d90 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/AbstractTestSearchPolicyEngine.java > 3df6ecf > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/AbstractTestSolrPolicyEngine.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/SearchPolicyTestUtil.java > e198b5c > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/SolrPolicyTestUtil.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestCollectionRequiredInRole.java > 76211dd > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSearchAuthorizationProviderGeneralCases.java > b4aa684 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSearchAuthorizationProviderSpecialCases.java > 371f361 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSearchModelAuthorizables.java > e7da13a > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSearchPolicyEngineDFS.java > 59283ea > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSearchPolicyEngineLocalFS.java > 0ff4502 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSearchPolicyNegative.java > 20fee76 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrAuthorizationProviderGeneralCases.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrAuthorizationProviderSpecialCases.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrModelAuthorizables.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrPolicyEngineDFS.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrPolicyEngineLocalFS.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/policy/solr/TestSolrPolicyNegative.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/privilege/solr/TestCommonPrivilegeForSearch.java > de6d6e0 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/privilege/solr/TestCommonPrivilegeForSolr.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/test/resources/test-authz-provider.ini > 56317db > sentry-core/pom.xml 6b91767 > sentry-core/sentry-core-model-search/pom.xml 5917a63 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/Collection.java > 26ea287 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/Field.java > 2dd9065 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchActionFactory.java > 3f10726 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java > a2b17fc > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAction.java > 48ac267 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAuthorizable.java > 5a55963 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchModelAuthorizables.java > 2b190e5 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java > 9429a25 > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/AbstractSearchPrivilegeValidator.java > c06131c > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/validator/CollectionRequiredInPrivilege.java > 93b3861 > > sentry-core/sentry-core-model-search/src/test/java/org/apache/sentry/core/search/TestCollection.java > 2311401 > > sentry-core/sentry-core-model-search/src/test/java/org/apache/sentry/core/search/TestSearchBitFieldAction.java > 0056f40 > sentry-core/sentry-core-model-solr/pom.xml PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/AdminOperation.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/Collection.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/Config.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/Field.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/Schema.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrActionFactory.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrConstants.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrModelAction.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrModelAuthorizable.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrModelAuthorizables.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/SolrPrivilegeModel.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/validator/SolrPrivilegeValidator.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/test/java/org/apache/sentry/core/solr/TestCollection.java > PRE-CREATION > > sentry-core/sentry-core-model-solr/src/test/java/org/apache/sentry/core/solr/TestSolrBitFieldAction.java > PRE-CREATION > sentry-dist/pom.xml 2d7f57e > sentry-provider/sentry-provider-db/pom.xml cd19032 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java > d8b4887 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java > 51d6df9 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java > 77d3919 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java > 34c2107 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryGMPrivilege.java > 258721e > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java > 9be4a8b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java > b7f0774 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > ac8b2a7 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java > 3685073 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > 55831a4 > sentry-solr/pom.xml 133ea60 > sentry-solr/solr-sentry-core/pom.xml e788262 > > sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/AuditLogger.java > 7f3e391 > > sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/RollingFileWithoutDeleteAppender.java > f749740 > > sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SecureRequestHandlerUtil.java > be9642b > > sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java > 8bd93ad > sentry-solr/solr-sentry-handlers/pom.xml 5b024db > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureDocumentAnalysisRequestHandler.java > 1c1f6f8 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureFieldAnalysisRequestHandler.java > 62f9a19 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRealTimeGetHandler.java > db182ef > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureReplicationHandler.java > bdcd830 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureAdminHandlers.java > 44db3b4 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java > b5edf20 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCoreAdminHandler.java > ff6e281 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureInfoHandler.java > 628d1d7 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java > 933db43 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryIndexAuthorizationComponent.java > 5fbb743 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/SecureRealTimeGetComponent.java > 7d55a7f > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/update/processor/UpdateIndexAuthorizationProcessor.java > d995a7d > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/update/processor/UpdateIndexAuthorizationProcessorFactory.java > 07f7f28 > > sentry-solr/solr-sentry-handlers/src/main/resources/sentry-handlers/solr/collection1/lib/classes/empty-file-main-lib.txt > 8b13789 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/TestSecureAnalysisHandlers.java > 28406e2 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/TestSecureReplicationHandler.java > 6367814 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureAdminHandlersTest.java > aea44f7 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureCollectionsHandlerTest.java > 218302e > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureCoreAdminHandlerTest.java > f93fb65 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureInfoHandlerTest.java > 54784f4 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/QueryDocAuthorizationComponentTest.java > 1f44628 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/QueryIndexAuthorizationComponentTest.java > a1f3760 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java > c294cf3 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentrySingletonTestInstance.java > 579f791 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryTestBase.java > e1a1ba8 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/update/processor/UpdateIndexAuthorizationProcessorTest.java > 630ca7c > sentry-tests/sentry-tests-solr/pom.xml 311e441 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java > 7ddd1e2 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestCase.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java > e50e3f8 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DummyAuthPluginImpl.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/ModifiableUserAuthenticationFilter.java > ac676a8 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestCollAdminCoreOperations.java > b0d6db1 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java > 71452e2 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestQueryOperations.java > f8ed955 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java > f9b6c07 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSentryServer.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSolrAdminOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSolrCollectionOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSolrConfigOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestSolrSchemaOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestUpdateOperations.java > 2b246b5 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java > 71c3cb6 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrAdminOperations.java > c07b3b8 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrDocLevelOperations.java > 7f1fdfd > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrQueryOperations.java > 3eb6c0f > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrUpdateOperations.java > 9412325 > > sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-managed/conf/managed-schema > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-managed/conf/solrconfig.xml > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal/conf/schema.xml > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal/conf/solrconfig.xml > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_doc_level_security/conf/schema.xml > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/resources/solr/configsets/cloud-minimal_doc_level_security/conf/solrconfig.xml > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/resources/solr/security/security.json > PRE-CREATION > > > Diff: https://reviews.apache.org/r/63250/diff/5/ > > > Testing > ------- > > All Solr/Sentry unit tests passing > > > Thanks, > > Hrishikesh Gadre > >
