----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37248/#review95171 -----------------------------------------------------------
Thanks Dian for the patch! I have two high level comments: core/src/main/java/org/apache/sqoop/core/ConfigurationConstants.java (lines 79 - 80) <https://reviews.apache.org/r/37248/#comment150010> Can we move this property to org.apache.sqoop.security.authentication space? Given the fact that it's specific to authentication it seems fair to have the property defined there. Also I would suggest to rename it from "static" to "default". security/src/main/java/org/apache/sqoop/security/authorization/AuthorizationEngine.java (line 175) <https://reviews.apache.org/r/37248/#comment150009> I think that this got added per our JIRA discussion where I brought up the point about whether we should allow to grant permissions to the default user and I think that we're not on the same page. This fragment will probihit the default user to grant or revoke privileges, however other users can still grant privileges to the default user. The later point is actually my question - should we allow let say user jarcec, to allow "grant all on everything to default user"? Jarcec - Jarek Cecho On Aug. 10, 2015, 3:21 a.m., Dian Fu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37248/ > ----------------------------------------------------------- > > (Updated Aug. 10, 2015, 3:21 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2439 > https://issues.apache.org/jira/browse/SQOOP-2439 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Use a default user when username isn't specified. > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/core/ConfigurationConstants.java > d7fe27b > core/src/main/java/org/apache/sqoop/security/SecurityError.java 9f85b9e > dist/src/main/server/conf/sqoop.properties ba6e09f > > security/src/main/java/org/apache/sqoop/security/authorization/AuthorizationEngine.java > 57e0da5 > server/src/main/java/org/apache/sqoop/server/RequestContext.java 01aed41 > > Diff: https://reviews.apache.org/r/37248/diff/ > > > Testing > ------- > > > Thanks, > > Dian Fu > >
