----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30103/#review69092 -----------------------------------------------------------
Cool! A few notes below, but good start. common/src/main/java/org/apache/sqoop/model/MPrincipal.java <https://reviews.apache.org/r/30103/#comment113690> MAccountableEntity comes with a lot of other metadata such as: user, creation data, persistance ID, etc. Does MPrincipal need all that? I can imagine this information being useful when updating roles provider (sentry, ranger, etc.), but not sure. common/src/main/java/org/apache/sqoop/model/MPrivilege.java <https://reviews.apache.org/r/30103/#comment113691> Same as MPrincipal common/src/main/java/org/apache/sqoop/model/MPrivilege.java <https://reviews.apache.org/r/30103/#comment113687> compare resource and action as well? common/src/main/java/org/apache/sqoop/model/MResource.java <https://reviews.apache.org/r/30103/#comment113692> Same as MPrincipal common/src/main/java/org/apache/sqoop/model/MRole.java <https://reviews.apache.org/r/30103/#comment113693> Same as MPrincipal core/src/main/java/org/apache/sqoop/security/AuthorizationAccessController.java <https://reviews.apache.org/r/30103/#comment113701> in Sqoop2 we've been using Abstract Classes over interfaces. If possible, could this remain and ABC? core/src/main/java/org/apache/sqoop/security/AuthorizationValidator.java <https://reviews.apache.org/r/30103/#comment113702> Use ABC? security/src/main/java/org/apache/sqoop/security/Authorization/DefaultAuthorizationValidator.java <https://reviews.apache.org/r/30103/#comment113684> Grammar nit: "always passed" => "always passes" or "always valid". - Abraham Elmahrek On Jan. 21, 2015, 6:16 a.m., richard zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30103/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2015, 6:16 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Default implementation of RBAC in Sqoop > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/MPrincipal.java PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MPrivilege.java PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MResource.java PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MRole.java PRE-CREATION > > core/src/main/java/org/apache/sqoop/security/AuthorizationAccessController.java > 698a940e47f9503bb5245d715d5ecc11662192aa > core/src/main/java/org/apache/sqoop/security/AuthorizationHandler.java > 865c6dc818db461b67c80ed2782c58ee8e23c691 > core/src/main/java/org/apache/sqoop/security/AuthorizationManager.java > 4d66bf798a0f79078052df76d3de2bc64454f2f4 > core/src/main/java/org/apache/sqoop/security/AuthorizationValidator.java > 7c4101512d85c03a2571e931f776c766390280af > > security/src/main/java/org/apache/sqoop/security/Authorization/DefaultAuthorizationAccessController.java > c8839f80fb7f7bf6f14e2196b7adfd342f2e3cd1 > > security/src/main/java/org/apache/sqoop/security/Authorization/DefaultAuthorizationHandler.java > a176b4dc4d30c6899d7625fcc2be1c3a0454f434 > > security/src/main/java/org/apache/sqoop/security/Authorization/DefaultAuthorizationValidator.java > 0842c81ea7d72ff78453fdb6358e7cd8e5f5d856 > > Diff: https://reviews.apache.org/r/30103/diff/ > > > Testing > ------- > > local > > > Thanks, > > richard zhou > >
