{"action":"created","comment":{"url":"https://api.github.com/repos/Teradata/presto/pulls/comments/61765902","id":61765902,"diff_hunk":"@@ -0,0 +1,66 @@\n+/*\n+ * Licensed under the Apache License, Version 2.0 (the \"License\");\n+ * you may not use this file except in compliance with the License.\n+ * You may obtain a copy of the License at\n+ *\n+ *     http://www.apache.org/licenses/LICENSE-2.0\n+ *\n+ * Unless required by applicable law or agreed to in writing, software\n+ * distributed under the License is distributed on an \"AS IS\" BASIS,\n+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n+ * See the License for the specific language governing permissions and\n+ * limitations under the License.\n+ */\n+\n+package com.facebook.presto.tests.hive;\n+\n+import com.teradata.tempto.ProductTest;\n+import com.teradata.tempto.query.QueryExecutor;\n+import org.testng.annotations.Test;\n+\n+import static com.facebook.presto.tests.TestGroups.HIVE_CONNECTOR;\n+import static com.facebook.presto.tests.TestGroups.QUARANTINE;\n+import static com.facebook.presto.tests.utils.QueryExecutors.connectToPresto;\n+import static com.teradata.tempto.assertions.QueryAssert.assertThat;\n+import static java.lang.String.format;\n+\n+public class TestGrantRevoke\n+    extends ProductTest\n+{\n+    /* This test has been quarantined since hive.security property is NOT set by default and\n+     * doing so may affect other tests.\n+     *\n+     * Pre-requisites for the test:\n+     * (1) In hive.properties file, set this property: hive.security=sql-standard;\n+     * (2) tempto-configuration.yaml file should have two connections to Presto server:\n+     * \"prestoConfigUserAlice\" that has \"jdbc_user: alice\" and\n+     * \"prestoConfigUserBob\" that has \"jdbc_user: bob\"\n+     * (all other values of the connection are same as the default \"presto\" connection).\n+     * You can get these configurations from presto-product-tests/conf/tempto/tempto-configuration.yaml file.\n+     *\n+    */\n+    @Test(groups = {HIVE_CONNECTOR, QUARANTINE})\n+    public void testGrantRevoke()\n+    {\n+        String tableName = \"alice_owned_table\";\n+        QueryExecutor queryExecutorForAlice = connectToPresto(\"prestoConfigUserAlice\");\n+        QueryExecutor queryExecutorForBob = connectToPresto(\"prestoConfigUserBob\");\n+\n+        queryExecutorForAlice.executeQuery(format(\"DROP TABLE IF EXISTS %s\", tableName));\n+        queryExecutorForAlice.executeQuery(format(\"CREATE TABLE %s(month varchar, day bigint)\", tableName));\n+        assertThat(queryExecutorForAlice.executeQuery(format(\"SELECT * from %s\", tableName))).hasNoRows();\n+        assertThat(() -> queryExecutorForBob.executeQuery(format(\"SELECT * from %s\", tableName))).\n+                failsWithMessage(format(\"Access Denied: Cannot select from table default.%s\", tableName));\n+\n+        //test GRANT\n+        queryExecutorForAlice.executeQuery(format(\"GRANT INSERT, SELECT ON %s to bob\", tableName));\n+        assertThat(queryExecutorForBob.executeQuery(format(\"INSERT INTO %s values ('t', 3)\", tableName))).hasRowsCount(1);\n+        assertThat(queryExecutorForBob.executeQuery(format(\"SELECT * from %s\", tableName))).hasRowsCount(1);","path":"presto-product-tests/src/main/java/com/facebook/presto/tests/hive/TestGrantRevoke.java","position":58,"original_position":58,"commit_id":"c2dffc9320f8d96d5683fcf2b2103389330d5aa2","original_commit_id":"a6d086501a25dfd320ef974532c9669ef85b4bfe","user":{"login":"amrutagokhale","id":9261099,"avatar_url":"https://avatars.githubusercontent.com/u/9261099?v=3","gravatar_id":"","url":"https://api.github.com/users/amrutagokhale","html_url":"https://github.com/amrutagokhale","followers_url":"https://api.github.com/users/amrutagokhale/followers","following_url":"https://api.github.com/users/amrutagokhale/following{/other_user}","gists_url":"https://api.github.com/users/amrutagokhale/gists{/gist_id}","starred_url":"https://api.github.com/users/amrutagokhale/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amrutagokhale/subscriptions","organizations_url":"https://api.github.com/users/amrutagokhale/orgs","repos_url":"https://api.github.com/users/amrutagokhale/repos","events_url":"https://api.github.com/users/amrutagokhale/events{/privacy}","received_events_url":"https://api.github.com/users/amrutagokhale/received_events","type":"User","site_admin":false},"body":"Actually, I modified this so as to add a check that bob cannot \"delete\" from table instead. Reason being that `drop table` is not allowed to users other than table owners. So even if we GRANT ALL PRIVILEGES to bob, `drop table` will never succeed for bob. See https://github.com/Teradata/presto/pull/204/commits/c2dffc9320f8d96d5683fcf2b2103389330d5aa2","created_at":"2016-05-02T16:52:05Z","updated_at":"2016-05-02T16:52:05Z","html_url":"https://github.com/Teradata/presto/pull/204#discussion_r61765902","pull_request_url":"https://api.github.com/repos/Teradata/presto/pulls/204","_links":{"self":{"href":"https://api.github.com/repos/Teradata/presto/pulls/comments/61765902"},"html":{"href":"https://github.com/Teradata/presto/pull/204#discussion_r61765902"},"pull_request":{"href":"https://api.github.com/repos/Teradata/presto/pulls/204"}}},"pull_request":{"url":"https://api.github.com/repos/Teradata/presto/pulls/204","id":68427976,"html_url":"https://github.com/Teradata/presto/pull/204","diff_url":"https://github.com/Teradata/presto/pull/204.diff","patch_url":"https://github.com/Teradata/presto/pull/204.patch","issue_url":"https://api.github.com/repos/Teradata/presto/issues/204","number":204,"state":"open","locked":false,"title":"Add product test for grant and revoke","user":{"login":"amrutagokhale","id":9261099,"avatar_url":"https://avatars.githubusercontent.com/u/9261099?v=3","gravatar_id":"","url":"https://api.github.com/users/amrutagokhale","html_url":"https://github.com/amrutagokhale","followers_url":"https://api.github.com/users/amrutagokhale/followers","following_url":"https://api.github.com/users/amrutagokhale/following{/other_user}","gists_url":"https://api.github.com/users/amrutagokhale/gists{/gist_id}","starred_url":"https://api.github.com/users/amrutagokhale/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amrutagokhale/subscriptions","organizations_url":"https://api.github.com/users/amrutagokhale/orgs","repos_url":"https://api.github.com/users/amrutagokhale/repos","events_url":"https://api.github.com/users/amrutagokhale/events{/privacy}","received_events_url":"https://api.github.com/users/amrutagokhale/received_events","type":"User","site_admin":false},"body":"@kokosing @anusudarsan Please review.\r\n\r\nI have quarantined the test for grant/revoke. Here is the reasoning behind doing so. \r\n\r\nRunning the grant/revoke test requires one to set the property `hive.security=sql-standard` in hive.properties file. If we set this property by default, this essentially turns on the access control checks for ALL the tests.\r\n\r\nIf in future, someone is writing a new product test that makes a new connection to Presto server, they have to be cognizant of the fact that access control checks have been turned on by default. If not, there is a chance that their test will fail. Such a failure of their test is no indication of the failure of the functionality that they want to test, but has to do with the fact that the testing code is not respecting the access control checks. The additional code that needs to be written so as to respect access control checks is unnecessary as far as testing their specific functionality is concerned. \r\n\r\nI ended up debugging the existing product tests when the hive.security property is turned on. The reasons for these failures had nothing to do with the specific functionalities for which the tests were written. To fix the failing tests, I have added new configuration properties in hive-site.xml file (in a separate PR). I also changed the code in PrestoCLITests to pass `—-user` option when opening connection to Presto server. Although these changes make sure that the existing tests pass if we turn on the property, there is no guarantee that this will hold true for tests that will be written in future.","created_at":"2016-04-29T20:19:27Z","updated_at":"2016-05-02T16:52:05Z","closed_at":null,"merged_at":null,"merge_commit_sha":"8a4e7dbeeac07e07a58ce8b07e781f6993af92ff","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/Teradata/presto/pulls/204/commits","review_comments_url":"https://api.github.com/repos/Teradata/presto/pulls/204/comments","review_comment_url":"https://api.github.com/repos/Teradata/presto/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Teradata/presto/issues/204/comments","statuses_url":"https://api.github.com/repos/Teradata/presto/statuses/c2dffc9320f8d96d5683fcf2b2103389330d5aa2","head":{"label":"Teradata:test-revoke","ref":"test-revoke","sha":"c2dffc9320f8d96d5683fcf2b2103389330d5aa2","user":{"login":"Teradata","id":1615979,"avatar_url":"https://avatars.githubusercontent.com/u/1615979?v=3","gravatar_id":"","url":"https://api.github.com/users/Teradata","html_url":"https://github.com/Teradata","followers_url":"https://api.github.com/users/Teradata/followers","following_url":"https://api.github.com/users/Teradata/following{/other_user}","gists_url":"https://api.github.com/users/Teradata/gists{/gist_id}","starred_url":"https://api.github.com/users/Teradata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Teradata/subscriptions","organizations_url":"https://api.github.com/users/Teradata/orgs","repos_url":"https://api.github.com/users/Teradata/repos","events_url":"https://api.github.com/users/Teradata/events{/privacy}","received_events_url":"https://api.github.com/users/Teradata/received_events","type":"Organization","site_admin":false},"repo":{"id":36903368,"name":"presto","full_name":"Teradata/presto","owner":{"login":"Teradata","id":1615979,"avatar_url":"https://avatars.githubusercontent.com/u/1615979?v=3","gravatar_id":"","url":"https://api.github.com/users/Teradata","html_url":"https://github.com/Teradata","followers_url":"https://api.github.com/users/Teradata/followers","following_url":"https://api.github.com/users/Teradata/following{/other_user}","gists_url":"https://api.github.com/users/Teradata/gists{/gist_id}","starred_url":"https://api.github.com/users/Teradata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Teradata/subscriptions","organizations_url":"https://api.github.com/users/Teradata/orgs","repos_url":"https://api.github.com/users/Teradata/repos","events_url":"https://api.github.com/users/Teradata/events{/privacy}","received_events_url":"https://api.github.com/users/Teradata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Teradata/presto","description":"Teradata's Certified Version of Presto -- A Distributed SQL Query Engine for Big Data","fork":true,"url":"https://api.github.com/repos/Teradata/presto","forks_url":"https://api.github.com/repos/Teradata/presto/forks","keys_url":"https://api.github.com/repos/Teradata/presto/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Teradata/presto/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Teradata/presto/teams","hooks_url":"https://api.github.com/repos/Teradata/presto/hooks","issue_events_url":"https://api.github.com/repos/Teradata/presto/issues/events{/number}","events_url":"https://api.github.com/repos/Teradata/presto/events","assignees_url":"https://api.github.com/repos/Teradata/presto/assignees{/user}","branches_url":"https://api.github.com/repos/Teradata/presto/branches{/branch}","tags_url":"https://api.github.com/repos/Teradata/presto/tags","blobs_url":"https://api.github.com/repos/Teradata/presto/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Teradata/presto/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Teradata/presto/git/refs{/sha}","trees_url":"https://api.github.com/repos/Teradata/presto/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Teradata/presto/statuses/{sha}","languages_url":"https://api.github.com/repos/Teradata/presto/languages","stargazers_url":"https://api.github.com/repos/Teradata/presto/stargazers","contributors_url":"https://api.github.com/repos/Teradata/presto/contributors","subscribers_url":"https://api.github.com/repos/Teradata/presto/subscribers","subscription_url":"https://api.github.com/repos/Teradata/presto/subscription","commits_url":"https://api.github.com/repos/Teradata/presto/commits{/sha}","git_commits_url":"https://api.github.com/repos/Teradata/presto/git/commits{/sha}","comments_url":"https://api.github.com/repos/Teradata/presto/comments{/number}","issue_comment_url":"https://api.github.com/repos/Teradata/presto/issues/comments{/number}","contents_url":"https://api.github.com/repos/Teradata/presto/contents/{+path}","compare_url":"https://api.github.com/repos/Teradata/presto/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Teradata/presto/merges","archive_url":"https://api.github.com/repos/Teradata/presto/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Teradata/presto/downloads","issues_url":"https://api.github.com/repos/Teradata/presto/issues{/number}","pulls_url":"https://api.github.com/repos/Teradata/presto/pulls{/number}","milestones_url":"https://api.github.com/repos/Teradata/presto/milestones{/number}","notifications_url":"https://api.github.com/repos/Teradata/presto/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Teradata/presto/labels{/name}","releases_url":"https://api.github.com/repos/Teradata/presto/releases{/id}","deployments_url":"https://api.github.com/repos/Teradata/presto/deployments","created_at":"2015-06-05T00:38:37Z","updated_at":"2016-04-27T18:21:10Z","pushed_at":"2016-05-02T16:51:18Z","git_url":"git://github.com/Teradata/presto.git","ssh_url":"git@github.com:Teradata/presto.git","clone_url":"https://github.com/Teradata/presto.git","svn_url":"https://github.com/Teradata/presto","homepage":"http://www.teradata.com/presto","size":137860,"stargazers_count":47,"watchers_count":47,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":8,"mirror_url":null,"open_issues_count":6,"forks":8,"open_issues":6,"watchers":47,"default_branch":"master"}},"base":{"label":"Teradata:sprint-30","ref":"sprint-30","sha":"afaf43546630a63596861cd09b3cd20cfc6ccedf","user":{"login":"Teradata","id":1615979,"avatar_url":"https://avatars.githubusercontent.com/u/1615979?v=3","gravatar_id":"","url":"https://api.github.com/users/Teradata","html_url":"https://github.com/Teradata","followers_url":"https://api.github.com/users/Teradata/followers","following_url":"https://api.github.com/users/Teradata/following{/other_user}","gists_url":"https://api.github.com/users/Teradata/gists{/gist_id}","starred_url":"https://api.github.com/users/Teradata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Teradata/subscriptions","organizations_url":"https://api.github.com/users/Teradata/orgs","repos_url":"https://api.github.com/users/Teradata/repos","events_url":"https://api.github.com/users/Teradata/events{/privacy}","received_events_url":"https://api.github.com/users/Teradata/received_events","type":"Organization","site_admin":false},"repo":{"id":36903368,"name":"presto","full_name":"Teradata/presto","owner":{"login":"Teradata","id":1615979,"avatar_url":"https://avatars.githubusercontent.com/u/1615979?v=3","gravatar_id":"","url":"https://api.github.com/users/Teradata","html_url":"https://github.com/Teradata","followers_url":"https://api.github.com/users/Teradata/followers","following_url":"https://api.github.com/users/Teradata/following{/other_user}","gists_url":"https://api.github.com/users/Teradata/gists{/gist_id}","starred_url":"https://api.github.com/users/Teradata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Teradata/subscriptions","organizations_url":"https://api.github.com/users/Teradata/orgs","repos_url":"https://api.github.com/users/Teradata/repos","events_url":"https://api.github.com/users/Teradata/events{/privacy}","received_events_url":"https://api.github.com/users/Teradata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Teradata/presto","description":"Teradata's Certified Version of Presto -- A Distributed SQL Query Engine for Big Data","fork":true,"url":"https://api.github.com/repos/Teradata/presto","forks_url":"https://api.github.com/repos/Teradata/presto/forks","keys_url":"https://api.github.com/repos/Teradata/presto/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Teradata/presto/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Teradata/presto/teams","hooks_url":"https://api.github.com/repos/Teradata/presto/hooks","issue_events_url":"https://api.github.com/repos/Teradata/presto/issues/events{/number}","events_url":"https://api.github.com/repos/Teradata/presto/events","assignees_url":"https://api.github.com/repos/Teradata/presto/assignees{/user}","branches_url":"https://api.github.com/repos/Teradata/presto/branches{/branch}","tags_url":"https://api.github.com/repos/Teradata/presto/tags","blobs_url":"https://api.github.com/repos/Teradata/presto/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Teradata/presto/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Teradata/presto/git/refs{/sha}","trees_url":"https://api.github.com/repos/Teradata/presto/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Teradata/presto/statuses/{sha}","languages_url":"https://api.github.com/repos/Teradata/presto/languages","stargazers_url":"https://api.github.com/repos/Teradata/presto/stargazers","contributors_url":"https://api.github.com/repos/Teradata/presto/contributors","subscribers_url":"https://api.github.com/repos/Teradata/presto/subscribers","subscription_url":"https://api.github.com/repos/Teradata/presto/subscription","commits_url":"https://api.github.com/repos/Teradata/presto/commits{/sha}","git_commits_url":"https://api.github.com/repos/Teradata/presto/git/commits{/sha}","comments_url":"https://api.github.com/repos/Teradata/presto/comments{/number}","issue_comment_url":"https://api.github.com/repos/Teradata/presto/issues/comments{/number}","contents_url":"https://api.github.com/repos/Teradata/presto/contents/{+path}","compare_url":"https://api.github.com/repos/Teradata/presto/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Teradata/presto/merges","archive_url":"https://api.github.com/repos/Teradata/presto/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Teradata/presto/downloads","issues_url":"https://api.github.com/repos/Teradata/presto/issues{/number}","pulls_url":"https://api.github.com/repos/Teradata/presto/pulls{/number}","milestones_url":"https://api.github.com/repos/Teradata/presto/milestones{/number}","notifications_url":"https://api.github.com/repos/Teradata/presto/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Teradata/presto/labels{/name}","releases_url":"https://api.github.com/repos/Teradata/presto/releases{/id}","deployments_url":"https://api.github.com/repos/Teradata/presto/deployments","created_at":"2015-06-05T00:38:37Z","updated_at":"2016-04-27T18:21:10Z","pushed_at":"2016-05-02T16:51:18Z","git_url":"git://github.com/Teradata/presto.git","ssh_url":"git@github.com:Teradata/presto.git","clone_url":"https://github.com/Teradata/presto.git","svn_url":"https://github.com/Teradata/presto","homepage":"http://www.teradata.com/presto","size":137860,"stargazers_count":47,"watchers_count":47,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":8,"mirror_url":null,"open_issues_count":6,"forks":8,"open_issues":6,"watchers":47,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Teradata/presto/pulls/204"},"html":{"href":"https://github.com/Teradata/presto/pull/204"},"issue":{"href":"https://api.github.com/repos/Teradata/presto/issues/204"},"comments":{"href":"https://api.github.com/repos/Teradata/presto/issues/204/comments"},"review_comments":{"href":"https://api.github.com/repos/Teradata/presto/pulls/204/comments"},"review_comment":{"href":"https://api.github.com/repos/Teradata/presto/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Teradata/presto/pulls/204/commits"},"statuses":{"href":"https://api.github.com/repos/Teradata/presto/statuses/c2dffc9320f8d96d5683fcf2b2103389330d5aa2"}}}}