diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java index dc56e2e..9a90549 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java @@ -251,6 +251,11 @@ public class AuthorizationPreEventListener extends MetaStorePreEventListener { org.apache.hadoop.hive.metastore.api.Table t = context.getHandler().get_table( mapiPart.getDbName(), mapiPart.getTableName()); if (tPart.getSd() == null){ + // In the cases of create partition, by the time this event fires, the partition + // object has not yet come into existence, and thus will not yet have a + // location or an SD, but these are needed to create a ql.metadata.Partition, + // so we use the table's SD. The only place this is used is by the + // authorization hooks, so we will not affect code flow in the metastore itself. tPart.setSd(t.getSd()); } return new Partition(getTableFromApiTable(t),tPart); diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java index 795064a..2d45620 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java @@ -99,7 +99,7 @@ public abstract class HiveAuthorizationProviderBase implements private Configuration conf; public static final Log LOG = LogFactory.getLog( - HiveAuthenticationProvider.class); + HiveAuthorizationProvider.class); public void setConf(Configuration conf) { diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java index da166e4..98cb904 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java @@ -41,6 +41,23 @@ import org.apache.hadoop.hive.ql.metadata.HiveException; import org.apache.hadoop.hive.ql.metadata.Partition; import org.apache.hadoop.hive.ql.metadata.Table; +/** + * StorageBasedAuthorizationProvider is an implementation of + * HiveMetastoreAuthorizationProvider that tries to look at the hdfs + * permissions of files and directories associated with objects like + * databases, tables and partitions to determine whether or not an + * operation is allowed. The rule of thumb for which location to check + * in hdfs is as follows: + * + * CREATE : on location specified, or on location determined from metadata + * READS : not checked (the preeventlistener does not have an event to fire) + * UPDATES : on location in metadata + * DELETES : on location in metadata + * + * If the location does not yet exist, as the case is with creates, it steps + * out to the parent directory recursively to determine its permissions till + * it finds a parent that does exist. + */ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProviderBase implements HiveMetastoreAuthorizationProvider { @@ -79,6 +96,8 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider public void authorize(Table table, Privilege[] readRequiredPriv, Privilege[] writeRequiredPriv) throws HiveException, AuthorizationException { + // Table path can be null in the case of a new create table - in this case, + // we try to determine what the path would be after the create table is issued. Path path = null; try { String location = table.getTTable().getSd().getLocation(); @@ -104,6 +123,8 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider Privilege[] writeRequiredPriv) throws HiveException, AuthorizationException { + // Partition path can be null in the case of a new create partition - in this case, + // we try to default to checking the permissions of the parent table if (part.getLocation() == null) { authorize(table, readRequiredPriv, writeRequiredPriv); } else { @@ -145,9 +166,11 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider case DROP: return FsAction.WRITE; case INDEX: - return FsAction.WRITE; + throw new AuthorizationException( + "StorageBasedAuthorizationProvider cannot handle INDEX privilege"); case LOCK: - return FsAction.WRITE; + throw new AuthorizationException( + "StorageBasedAuthorizationProvider cannot handle LOCK privilege"); case SELECT: return FsAction.READ; case SHOW_DATABASE: @@ -223,7 +246,7 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider checkPermissions(fs, path, actions, authenticator.getUserName(), authenticator.getGroupNames()); } else if (path.getParent() != null) { - // find the ancestor which exists to check it's permissions + // find the ancestor which exists to check its permissions Path par = path.getParent(); while (par != null) { if (fs.exists(par)) { diff --git a/ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java b/ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java index 535cdbe..8119913 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java @@ -27,8 +27,8 @@ import junit.framework.TestCase; import org.apache.hadoop.hive.cli.CliSessionState; import org.apache.hadoop.hive.conf.HiveConf; -import org.apache.hadoop.hive.metastore.HiveMetaStore; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.metastore.MetaStoreUtils; import org.apache.hadoop.hive.metastore.api.Database; import org.apache.hadoop.hive.metastore.api.Partition; import org.apache.hadoop.hive.metastore.api.Table; @@ -36,6 +36,7 @@ import org.apache.hadoop.hive.ql.Driver; import org.apache.hadoop.hive.ql.security.DummyHiveMetastoreAuthorizationProvider.AuthCallContext; import org.apache.hadoop.hive.ql.security.authorization.AuthorizationPreEventListener; import org.apache.hadoop.hive.ql.session.SessionState; +import org.apache.hadoop.hive.shims.ShimLoader; /** * TestAuthorizationPreEventListener. Test case for @@ -43,30 +44,16 @@ import org.apache.hadoop.hive.ql.session.SessionState; * {@link org.apache.hadoop.hive.metastore.MetaStorePreEventListener} */ public class TestAuthorizationPreEventListener extends TestCase { - private static String msPort; private HiveConf clientHiveConf; private HiveMetaStoreClient msc; private Driver driver; - private static class RunMS implements Runnable { - - @Override - public void run() { - try { - HiveMetaStore.main(new String[]{msPort}); - } catch (Throwable e) { - e.printStackTrace(System.err); - assert false; - } - } - } - @Override protected void setUp() throws Exception { super.setUp(); - msPort = getFreeAvailablePort(); + int port = MetaStoreUtils.findFreePort(); System.setProperty(HiveConf.ConfVars.METASTORE_PRE_EVENT_LISTENERS.varname, AuthorizationPreEventListener.class.getName()); @@ -75,14 +62,11 @@ public class TestAuthorizationPreEventListener extends TestCase { System.setProperty(HiveConf.ConfVars.HIVE_METASTORE_AUTHENTICATOR_MANAGER.varname, HadoopDefaultMetastoreAuthenticator.class.getName()); - - Thread t = new Thread(new RunMS()); - t.start(); - Thread.sleep(40000); + MetaStoreUtils.startMetaStore(port, ShimLoader.getHadoopThriftAuthBridge()); clientHiveConf = new HiveConf(this.getClass()); - clientHiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + msPort); + clientHiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + port); clientHiveConf.setIntVar(HiveConf.ConfVars.METASTORETHRIFTRETRIES, 3); clientHiveConf.set(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY.varname, "false"); diff --git a/ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java b/ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java index d8ee026..660b4c5 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java @@ -18,8 +18,6 @@ package org.apache.hadoop.hive.ql.security; -import java.io.IOException; -import java.net.ServerSocket; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -28,8 +26,8 @@ import junit.framework.TestCase; import org.apache.hadoop.hive.cli.CliSessionState; import org.apache.hadoop.hive.conf.HiveConf; -import org.apache.hadoop.hive.metastore.HiveMetaStore; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.metastore.MetaStoreUtils; import org.apache.hadoop.hive.metastore.api.Database; import org.apache.hadoop.hive.metastore.api.FieldSchema; import org.apache.hadoop.hive.metastore.api.MetaException; @@ -57,31 +55,17 @@ import org.apache.hadoop.security.UserGroupInformation; * turn on server-side auth. */ public class TestDefaultHiveMetastoreAuthorizationProvider extends TestCase { - private static String msPort; private HiveConf clientHiveConf; private HiveMetaStoreClient msc; private Driver driver; private UserGroupInformation ugi; - private static class RunMS implements Runnable { - - @Override - public void run() { - try { - HiveMetaStore.main(new String[]{msPort}); - } catch (Throwable e) { - e.printStackTrace(System.err); - assert false; - } - } - } - @Override protected void setUp() throws Exception { super.setUp(); - msPort = getFreeAvailablePort(); + int port = MetaStoreUtils.findFreePort(); System.setProperty(HiveConf.ConfVars.METASTORE_PRE_EVENT_LISTENERS.varname, AuthorizationPreEventListener.class.getName()); @@ -92,15 +76,13 @@ public class TestDefaultHiveMetastoreAuthorizationProvider extends TestCase { System.setProperty(HiveConf.ConfVars.HIVE_AUTHORIZATION_TABLE_OWNER_GRANTS.varname, ""); - Thread t = new Thread(new RunMS()); - t.start(); - Thread.sleep(40000); + MetaStoreUtils.startMetaStore(port, ShimLoader.getHadoopThriftAuthBridge()); clientHiveConf = new HiveConf(this.getClass()); clientHiveConf.setBoolVar(HiveConf.ConfVars.HIVE_AUTHORIZATION_ENABLED,false); - clientHiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + msPort); + clientHiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + port); clientHiveConf.setIntVar(HiveConf.ConfVars.METASTORETHRIFTRETRIES, 3); clientHiveConf.set(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY.varname, "false"); @@ -114,14 +96,6 @@ public class TestDefaultHiveMetastoreAuthorizationProvider extends TestCase { driver = new Driver(clientHiveConf); } - private static String getFreeAvailablePort() throws IOException { - ServerSocket socket = new ServerSocket(0); - socket.setReuseAddress(true); - int port = socket.getLocalPort(); - socket.close(); - return "" + port; - } - @Override protected void tearDown() throws Exception { super.tearDown();