You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/06/10 23:00:38 UTC

[GitHub] [hive] dantongdong opened a new pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

dantongdong opened a new pull request #2384:
URL: https://github.com/apache/hive/pull/2384


   [HIVE-25214](https://issues.apache.org/jira/browse/HIVE-25214): Add hive authorization support for Data connectors


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] dantongdong commented on a change in pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

Posted by GitBox <gi...@apache.org>.
dantongdong commented on a change in pull request #2384:
URL: https://github.com/apache/hive/pull/2384#discussion_r667219378



##########
File path: standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
##########
@@ -148,15 +148,21 @@ ALTER TABLE COMPLETED_COMPACTIONS ADD CC_INITIATOR_VERSION varchar(128);
 ALTER TABLE COMPLETED_COMPACTIONS ADD CC_WORKER_VERSION varchar(128);
 
 -- HIVE-24396
-CREATE TABLE "APP"."DATACONNECTORS" ("DC_NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
-CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("DC_NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
+CREATE TABLE "APP"."DATACONNECTORS" ("NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
+CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
 ALTER TABLE "APP"."DBS" ADD COLUMN "TYPE" VARCHAR(32) DEFAULT 'NATIVE' NOT NULL;
 ALTER TABLE "APP"."DBS" ADD COLUMN "DATACONNECTOR_NAME" VARCHAR(128);
 ALTER TABLE "APP"."DBS" ADD COLUMN "REMOTE_DBNAME" VARCHAR(128);
 UPDATE "APP"."DBS" SET TYPE='NATIVE' WHERE TYPE IS NULL;
-ALTER TABLE "APP"."DATACONNECTORS" ADD CONSTRAINT "DATACONNECTORS_KEY_PK" PRIMARY KEY ("DC_NAME");
-ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DATACONNECTOR_PARAMS_KEY_PK" PRIMARY KEY ("DC_NAME", "PARAM_KEY");
-ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DC_NAME_FK1" FOREIGN KEY ("DC_NAME") REFERENCES "APP"."DATACONNECTORS" ("DC_NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;
+ALTER TABLE "APP"."DATACONNECTORS" ADD CONSTRAINT "DATACONNECTORS_KEY_PK" PRIMARY KEY ("NAME");
+ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DATACONNECTOR_PARAMS_KEY_PK" PRIMARY KEY ("NAME", "PARAM_KEY");
+ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "NAME_FK1" FOREIGN KEY ("NAME") REFERENCES "APP"."DATACONNECTORS" ("NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;
+
+CREATE TABLE "APP"."DC_PRIVS" ("DC_GRANT_ID" BIGINT NOT NULL, "CREATE_TIME" INTEGER NOT NULL, "DC_NAME" VARCHAR(128), "GRANT_OPTION" SMALLINT NOT NULL, "GRANTOR" VARCHAR(128), "GRANTOR_TYPE" VARCHAR(128), "PRINCIPAL_NAME" VARCHAR(128), "PRINCIPAL_TYPE" VARCHAR(128), "DC_PRIV" VARCHAR(128), "AUTHORIZER" VARCHAR(128));

Review comment:
       Good catch. We will use NAME for both DATACONNECTOR, DATACONNECTOR_PARAMS, and DC_PRIVS. Will change.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] nrg4878 commented on a change in pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

Posted by GitBox <gi...@apache.org>.
nrg4878 commented on a change in pull request #2384:
URL: https://github.com/apache/hive/pull/2384#discussion_r667035410



##########
File path: standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
##########
@@ -148,15 +148,21 @@ ALTER TABLE COMPLETED_COMPACTIONS ADD CC_INITIATOR_VERSION varchar(128);
 ALTER TABLE COMPLETED_COMPACTIONS ADD CC_WORKER_VERSION varchar(128);
 
 -- HIVE-24396
-CREATE TABLE "APP"."DATACONNECTORS" ("DC_NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
-CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("DC_NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
+CREATE TABLE "APP"."DATACONNECTORS" ("NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);

Review comment:
       good catch. Thanks for fixing this.

##########
File path: standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
##########
@@ -148,15 +148,21 @@ ALTER TABLE COMPLETED_COMPACTIONS ADD CC_INITIATOR_VERSION varchar(128);
 ALTER TABLE COMPLETED_COMPACTIONS ADD CC_WORKER_VERSION varchar(128);
 
 -- HIVE-24396
-CREATE TABLE "APP"."DATACONNECTORS" ("DC_NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
-CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("DC_NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
+CREATE TABLE "APP"."DATACONNECTORS" ("NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
+CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
 ALTER TABLE "APP"."DBS" ADD COLUMN "TYPE" VARCHAR(32) DEFAULT 'NATIVE' NOT NULL;
 ALTER TABLE "APP"."DBS" ADD COLUMN "DATACONNECTOR_NAME" VARCHAR(128);
 ALTER TABLE "APP"."DBS" ADD COLUMN "REMOTE_DBNAME" VARCHAR(128);
 UPDATE "APP"."DBS" SET TYPE='NATIVE' WHERE TYPE IS NULL;
-ALTER TABLE "APP"."DATACONNECTORS" ADD CONSTRAINT "DATACONNECTORS_KEY_PK" PRIMARY KEY ("DC_NAME");
-ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DATACONNECTOR_PARAMS_KEY_PK" PRIMARY KEY ("DC_NAME", "PARAM_KEY");
-ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DC_NAME_FK1" FOREIGN KEY ("DC_NAME") REFERENCES "APP"."DATACONNECTORS" ("DC_NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;
+ALTER TABLE "APP"."DATACONNECTORS" ADD CONSTRAINT "DATACONNECTORS_KEY_PK" PRIMARY KEY ("NAME");
+ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DATACONNECTOR_PARAMS_KEY_PK" PRIMARY KEY ("NAME", "PARAM_KEY");
+ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "NAME_FK1" FOREIGN KEY ("NAME") REFERENCES "APP"."DATACONNECTORS" ("NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;
+
+CREATE TABLE "APP"."DC_PRIVS" ("DC_GRANT_ID" BIGINT NOT NULL, "CREATE_TIME" INTEGER NOT NULL, "DC_NAME" VARCHAR(128), "GRANT_OPTION" SMALLINT NOT NULL, "GRANTOR" VARCHAR(128), "GRANTOR_TYPE" VARCHAR(128), "PRINCIPAL_NAME" VARCHAR(128), "PRINCIPAL_TYPE" VARCHAR(128), "DC_PRIV" VARCHAR(128), "AUTHORIZER" VARCHAR(128));

Review comment:
       so here we are using DC_NAME whereas we used NAME in the DATACONNECTOR and DATACONNECTOR_PARAMS table. Should we make it consistent so it is not confusing?  

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
##########
@@ -35,6 +35,7 @@
 import org.apache.hadoop.hive.ql.hooks.Entity.Type;
 import org.apache.hadoop.hive.ql.hooks.WriteEntity;
 import org.apache.hadoop.hive.ql.hooks.WriteEntity.WriteType;
+import org.apache.hadoop.hive.ql.metadata.Hive;

Review comment:
       Is this import necessary ?

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
##########
@@ -6496,6 +6497,88 @@ public PrincipalPrivilegeSet getDBPrivilegeSet(String catName, String dbName,
     return ret;
   }
 
+  private List<PrivilegeGrantInfo> getConnectorPrivilege(String catName, String connectorName,
+     String principalName, PrincipalType principalType) {
+
+    // normalize string name
+    catName = normalizeIdentifier(catName);
+    connectorName = normalizeIdentifier(connectorName);
+
+    if (principalName != null) {
+      // get all data connector granted privilege
+      List<MDCPrivilege> userNameDcPriv = this.listPrincipalMDCGrants(
+              principalName, principalType, catName, connectorName);
+
+      // populate and return grantInfos
+      if (CollectionUtils.isNotEmpty(userNameDcPriv)) {
+        List<PrivilegeGrantInfo> grantInfos = new ArrayList<>(
+                userNameDcPriv.size());
+        for (int i = 0; i < userNameDcPriv.size(); i++) {
+          MDCPrivilege item = userNameDcPriv.get(i);
+          grantInfos.add(new PrivilegeGrantInfo(item.getPrivilege(), item
+                  .getCreateTime(), item.getGrantor(), getPrincipalTypeFromStr(item
+                  .getGrantorType()), item.getGrantOption()));
+        }
+        return grantInfos;
+      }
+    }
+
+    // return empty list if no principalName
+    return Collections.emptyList();
+  }
+
+  @Override
+  public PrincipalPrivilegeSet getConnectorPrivilegeSet (String catName, String connectorName,
+      String userName, List<String> groupNames)  throws InvalidObjectException,
+      MetaException {
+
+    boolean commited = false;
+    catName = normalizeIdentifier(catName);
+    connectorName = normalizeIdentifier(connectorName);
+
+    PrincipalPrivilegeSet ret = new PrincipalPrivilegeSet();
+    try {
+      openTransaction();
+
+      // get user privileges
+      if (userName != null) {
+        Map<String, List<PrivilegeGrantInfo>> connectorUserPriv = new HashMap<>();
+        connectorUserPriv.put(userName, getConnectorPrivilege(catName, connectorName, userName,
+                PrincipalType.USER));
+        ret.setUserPrivileges(connectorUserPriv);
+      }
+
+      // get group privileges
+      if (CollectionUtils.isNotEmpty(groupNames)) {
+        Map<String, List<PrivilegeGrantInfo>> dbGroupPriv = new HashMap<>();
+        for (String groupName : groupNames) {
+          dbGroupPriv.put(groupName, getConnectorPrivilege(catName, connectorName, groupName,
+                  PrincipalType.GROUP));
+        }
+        ret.setGroupPrivileges(dbGroupPriv);
+      }
+
+      // get role privileges
+      Set<String> roleNames = listAllRolesInHierarchy(userName, groupNames);
+      if (CollectionUtils.isNotEmpty(roleNames)) {
+        Map<String, List<PrivilegeGrantInfo>> dbRolePriv = new HashMap<>();
+        for (String roleName : roleNames) {
+          dbRolePriv

Review comment:
       nit:re-indent these 2 lines of code if possible.

##########
File path: ql/src/test/results/clientnegative/dataconnector_authfail_create.q.out
##########
@@ -0,0 +1 @@
+FAILED: HiveAccessControlException Permission denied: Principal [name=hive_test_user, type=USER] does not have following privileges for operation CREATEDATACONNECTOR [[ADMIN PRIVILEGE] on Object [type=DATACONNECTOR, name=mysql_auth]]

Review comment:
       This test output seems too short. I would have expected this to contain the PREHOOK output that prints out the query. But maybe the PREHOOK is run after the authorization. if this is the case, then it makes sense. Just asking you to double check.

##########
File path: standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
##########
@@ -48,6 +48,8 @@ CREATE TABLE "APP"."PARTITION_KEY_VALS" ("PART_ID" BIGINT NOT NULL, "PART_KEY_VA
 
 CREATE TABLE "APP"."DB_PRIVS" ("DB_GRANT_ID" BIGINT NOT NULL, "CREATE_TIME" INTEGER NOT NULL, "DB_ID" BIGINT, "GRANT_OPTION" SMALLINT NOT NULL, "GRANTOR" VARCHAR(128), "GRANTOR_TYPE" VARCHAR(128), "PRINCIPAL_NAME" VARCHAR(128), "PRINCIPAL_TYPE" VARCHAR(128), "DB_PRIV" VARCHAR(128), "AUTHORIZER" VARCHAR(128));
 
+CREATE TABLE "APP"."DC_PRIVS" ("DC_GRANT_ID" BIGINT NOT NULL, "CREATE_TIME" INTEGER NOT NULL, "DC_NAME" VARCHAR(128), "GRANT_OPTION" SMALLINT NOT NULL, "GRANTOR" VARCHAR(128), "GRANTOR_TYPE" VARCHAR(128), "PRINCIPAL_NAME" VARCHAR(128), "PRINCIPAL_TYPE" VARCHAR(128), "DC_PRIV" VARCHAR(128), "AUTHORIZER" VARCHAR(128));

Review comment:
       NAME vs DC_NAME

##########
File path: standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
##########
@@ -108,6 +108,24 @@ CREATE TABLE "DB_PRIVS" (
 );
 
 
+--
+-- Name: DC_PRIVS; Type: TABLE; Schema: public; Owner: hiveuser; Tablespace:
+--
+
+CREATE TABLE "DC_PRIVS" (

Review comment:
       nit: re-indent to match other tables please.

##########
File path: standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
##########
@@ -148,15 +148,21 @@ ALTER TABLE COMPLETED_COMPACTIONS ADD CC_INITIATOR_VERSION varchar(128);
 ALTER TABLE COMPLETED_COMPACTIONS ADD CC_WORKER_VERSION varchar(128);
 
 -- HIVE-24396
-CREATE TABLE "APP"."DATACONNECTORS" ("DC_NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
-CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("DC_NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
+CREATE TABLE "APP"."DATACONNECTORS" ("NAME" VARCHAR(128) NOT NULL, "TYPE" VARCHAR(128) NOT NULL, "COMMENT" VARCHAR(256), "OWNER_NAME" VARCHAR(256), "OWNER_TYPE" VARCHAR(10), "CREATE_TIME" INTEGER NOT NULL);
+CREATE TABLE "APP"."DATACONNECTOR_PARAMS" ("NAME" VARCHAR(128) NOT NULL, "PARAM_KEY" VARCHAR(180) NOT NULL, "PARAM_VALUE" VARCHAR(4000), "COMMENT" VARCHAR(256));
 ALTER TABLE "APP"."DBS" ADD COLUMN "TYPE" VARCHAR(32) DEFAULT 'NATIVE' NOT NULL;
 ALTER TABLE "APP"."DBS" ADD COLUMN "DATACONNECTOR_NAME" VARCHAR(128);
 ALTER TABLE "APP"."DBS" ADD COLUMN "REMOTE_DBNAME" VARCHAR(128);
 UPDATE "APP"."DBS" SET TYPE='NATIVE' WHERE TYPE IS NULL;
-ALTER TABLE "APP"."DATACONNECTORS" ADD CONSTRAINT "DATACONNECTORS_KEY_PK" PRIMARY KEY ("DC_NAME");
-ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DATACONNECTOR_PARAMS_KEY_PK" PRIMARY KEY ("DC_NAME", "PARAM_KEY");
-ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DC_NAME_FK1" FOREIGN KEY ("DC_NAME") REFERENCES "APP"."DATACONNECTORS" ("DC_NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;
+ALTER TABLE "APP"."DATACONNECTORS" ADD CONSTRAINT "DATACONNECTORS_KEY_PK" PRIMARY KEY ("NAME");
+ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "DATACONNECTOR_PARAMS_KEY_PK" PRIMARY KEY ("NAME", "PARAM_KEY");
+ALTER TABLE "APP"."DATACONNECTOR_PARAMS" ADD CONSTRAINT "NAME_FK1" FOREIGN KEY ("NAME") REFERENCES "APP"."DATACONNECTORS" ("NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;
+
+CREATE TABLE "APP"."DC_PRIVS" ("DC_GRANT_ID" BIGINT NOT NULL, "CREATE_TIME" INTEGER NOT NULL, "DC_NAME" VARCHAR(128), "GRANT_OPTION" SMALLINT NOT NULL, "GRANTOR" VARCHAR(128), "GRANTOR_TYPE" VARCHAR(128), "PRINCIPAL_NAME" VARCHAR(128), "PRINCIPAL_TYPE" VARCHAR(128), "DC_PRIV" VARCHAR(128), "AUTHORIZER" VARCHAR(128));
+ALTER TABLE "APP"."DC_PRIVS" ADD CONSTRAINT "DC_PRIVS_PK" PRIMARY KEY ("DC_GRANT_ID");
+ALTER TABLE "APP"."DC_PRIVS" ADD CONSTRAINT "DC_PRIVS_FK1" FOREIGN KEY ("DC_NAME") REFERENCES "APP"."DATACONNECTORS" ("NAME") ON DELETE NO ACTION ON UPDATE NO ACTION;

Review comment:
       if we are changing the name, it changes this FK reference as well.

##########
File path: standalone-metastore/metastore-server/src/main/resources/package.jdo
##########
@@ -1655,6 +1655,49 @@
           </value>
        </field>
     </class>
+    <class name="MDCPrivilege" table="DC_PRIVS" identity-type="datastore" detachable="true">
+          <index name="DCPrivilegeIndex" unique="true">
+            <column name="AUTHORIZER"/>
+            <column name="DC_NAME"/>
+            <column name="PRINCIPAL_NAME"/>
+            <column name="PRINCIPAL_TYPE"/>
+            <column name="DC_PRIV"/>
+            <column name="GRANTOR"/>
+            <column name="GRANTOR_TYPE"/>
+          </index>
+
+          <datastore-identity>
+            <column name="DC_GRANT_ID"/>
+          </datastore-identity>
+
+          <field name="principalName">
+            <column name="PRINCIPAL_NAME" length="128" jdbc-type="VARCHAR"/>
+          </field>
+          <field name="principalType">
+            <column name="PRINCIPAL_TYPE" length="128" jdbc-type="VARCHAR"/>
+          </field>
+          <field name="dataConnector">

Review comment:
       so MDCPrivilege.dataConnector is of type MDataConnector whereas this mapping indicates it is a string field. Shouldnt these types match? Am I missing something here? 

##########
File path: standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
##########
@@ -303,6 +303,31 @@ ALTER TABLE "DBS" ADD "DATACONNECTOR_NAME" character varying(128);
 ALTER TABLE "DBS" ADD "REMOTE_DBNAME" character varying(128);
 UPDATE "DBS" SET "TYPE"= 'NATIVE' WHERE "TYPE" IS NULL;
 
+
+CREATE TABLE "DC_PRIVS" (
+                            "DC_GRANT_ID" bigint NOT NULL,

Review comment:
       nit: re-indent to match other tables please

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/model/MDCPrivilege.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.model;
+
+public class MDCPrivilege {
+    private String principalName;
+
+    private String principalType;
+
+    private MDataConnector dataConnector;

Review comment:
       should the type match the mapping in package.jdo?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] dantongdong closed pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

Posted by GitBox <gi...@apache.org>.
dantongdong closed pull request #2384:
URL: https://github.com/apache/hive/pull/2384


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] dantongdong commented on a change in pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

Posted by GitBox <gi...@apache.org>.
dantongdong commented on a change in pull request #2384:
URL: https://github.com/apache/hive/pull/2384#discussion_r667139532



##########
File path: ql/src/test/results/clientnegative/dataconnector_authfail_create.q.out
##########
@@ -0,0 +1 @@
+FAILED: HiveAccessControlException Permission denied: Principal [name=hive_test_user, type=USER] does not have following privileges for operation CREATEDATACONNECTOR [[ADMIN PRIVILEGE] on Object [type=DATACONNECTOR, name=mysql_auth]]

Review comment:
       It seems like PREHOOK is run after authorization. I also double checked with other qtest related to authorization, it seems like the case.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] dantongdong commented on a change in pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

Posted by GitBox <gi...@apache.org>.
dantongdong commented on a change in pull request #2384:
URL: https://github.com/apache/hive/pull/2384#discussion_r667240750



##########
File path: standalone-metastore/metastore-server/src/main/resources/package.jdo
##########
@@ -1655,6 +1655,49 @@
           </value>
        </field>
     </class>
+    <class name="MDCPrivilege" table="DC_PRIVS" identity-type="datastore" detachable="true">
+          <index name="DCPrivilegeIndex" unique="true">
+            <column name="AUTHORIZER"/>
+            <column name="DC_NAME"/>
+            <column name="PRINCIPAL_NAME"/>
+            <column name="PRINCIPAL_TYPE"/>
+            <column name="DC_PRIV"/>
+            <column name="GRANTOR"/>
+            <column name="GRANTOR_TYPE"/>
+          </index>
+
+          <datastore-identity>
+            <column name="DC_GRANT_ID"/>
+          </datastore-identity>
+
+          <field name="principalName">
+            <column name="PRINCIPAL_NAME" length="128" jdbc-type="VARCHAR"/>
+          </field>
+          <field name="principalType">
+            <column name="PRINCIPAL_TYPE" length="128" jdbc-type="VARCHAR"/>
+          </field>
+          <field name="dataConnector">

Review comment:
       This is intended. MDCPrivilege has dataconnector as MDataConnector, but dataconnector itself is actually stored as String in backend Table (because it cannot store dataconnector type). 
   
   The reason why MDCPrivilege can retrieve dataconnector as MDataConnector even though it is stored as String in backendTable is that DC_NAME here actually serving as a Foreign Key referencing NAME in DATACONNECTORS table(as specified in schema when init).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] nrg4878 commented on pull request #2384: HIVE-25214: Add hive authorization support for Data connectors

Posted by GitBox <gi...@apache.org>.
nrg4878 commented on pull request #2384:
URL: https://github.com/apache/hive/pull/2384#issuecomment-885798784


   Fix has been pushed to master. Thank you for the work @dantongdong 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org