> On Jan. 26, 2016, 2:59 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java, > > line 114 > > <https://reviews.apache.org/r/42682/diff/4/?file=1220907#file1220907line114> > > > > Currently, only collection for Solr, and validate this already in the > > previous code: > > > > if (authz instanceof Collection) { > > .... > > } else { > > throw new IllegalArgumentException > > } > > > > This method is not necessary for current model. > > Gregory Chanan wrote: > I don't know what you mean. What is the previous code? If you are > referring to SearchModelAuthorizables.from(keyValue), that returns null when > it encounters an unknown authorizable, so it doesn't do any validation. If > you don't do this, you can put bogus Authorizables (i.e. > bogus1=bogus1->collection=collection2->bogus2=bogus2->action=UPDATE > > The question is where the validations should happen. It seems like it > should happen on the sentry server because: > 1) it can't be done on the client, because we can't guarantee everyone > uses our client > 2) it can't be done well on the service (i.e. solr), because there's no > push from sentry to the service. So the user experience is pretty bad in > that you enter a bogus privilege, it gets accepted, and then the service > starts misbehaving. > > There appears to be some checking on the sentry service (i.e. that > collection exists, although bogus privileges seem to be accepted). This is > just locking down the client as much as possible, to only accept privileges > we know are valid. We can open it up later once we have the privilege > validation more thought out.
Thanks for clarify, make sense. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42682/#review116263 ----------------------------------------------------------- On Jan. 26, 2016, 8:11 p.m., Gregory Chanan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42682/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2016, 8:11 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Here is a solr shell, based on the existing hive shell. Actually most of the > work is specific to the generic model, there is only a small amount of > solr-specific code. So it should be easily extensible for other clients. > > Some limitations: > > It is not integrated with bin/sentryShell yet > It does not support any grant options > It is pretty restrictive in what it supports, i.e. it checks for know > authorizables and throws an exception if it doesn't know about any. So it > can't be used for anything outside of collection permissions. > > > Diffs > ----- > > sentry-provider/sentry-provider-db/pom.xml > 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/Command.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/CreateRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/DropRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GrantPrivilegeToRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/ListPrivilegesByRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/ListRolesCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/RevokePrivilegeFromRoleCmd.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConvertor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java > b1353c5312e9d53b6dd184edd1fa8c11a92b4374 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceIntegrationBase.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > 4732ea2fcac1f9cb6a885592378f7916edbde3a1 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/42682/diff/ > > > Testing > ------- > > Ran the unit tests. > > > Thanks, > > Gregory Chanan > >
