> On Feb. 29, 2016, 8:31 a.m., Dapeng Sun wrote: > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java, > > line 175 > > <https://reviews.apache.org/r/43979/diff/1/?file=1270547#file1270547line175> > > > > Is there any authorization at Kafka ACL commands? If not, it seems any > > user can modify the ACLs via Kafka commands.
Dapeng, thanks for the review. Kafka will authenticate users before passing user's request to Sentry to perform ACLs CRUD. Sentry can assume that users requests coming to it for performing ACLs CRUD are authenticated and authorized. > On Feb. 29, 2016, 8:31 a.m., Dapeng Sun wrote: > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java, > > line 67 > > <https://reviews.apache.org/r/43979/diff/1/?file=1270546#file1270546line67> > > > > Better to add some Unit Tests for ACL operations. I found it a bit complicated to be able to add Acls CRUD unit tests. I have added e2e tests for the same. - Ashish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43979/#review121205 ----------------------------------------------------------- On Feb. 25, 2016, 2 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43979/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2016, 2 a.m.) > > > Review request for sentry, Dapeng Sun and Hao Hao. > > > Bugs: SENTRY-1057 > https://issues.apache.org/jira/browse/SENTRY-1057 > > > Repository: sentry > > > Description > ------- > > SENTRY-1057: Add CRUD support for ACLs, roles and privileges for Kafka plugin. > > > Diffs > ----- > > sentry-binding/sentry-binding-kafka/pom.xml > bd24c20edcbffb5caee69aedddb93cb5a6c8c112 > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java > 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java > 9e72d7890e0db3710ab0971ccbb71a32d3d36e87 > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java > 92e50e6452a89e676eb559a3657b17ba563bfddc > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java > e75ec7eddaa1a48b731d551736859a94c08af593 > > sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java > eafe0f0ee5482fadee43ddec99bdc5da3f42e30f > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java > bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1 > > sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java > 20d5e8e518cb91b32fad930e9d39400764f40b55 > > sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java > f1ed0001883e5d4b362389c258f4f72697a1e5a4 > > sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java > 513c2719424f994e8eb773e72073401b465b7d58 > > sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java > bcc119860860d928d092ed3ddc0a5a28e841c9aa > > Diff: https://reviews.apache.org/r/43979/diff/ > > > Testing > ------- > > Tested with E2E tests added as part of SENTRY-1014. > > Note that this contains changes from SENTRY-1098, SENTRY-1056 and > SENTRY-1030, and will have to be rebased once they get it. > > > Thanks, > > Ashish Singh > >
