From 29a9d4187f569cc781cffbc712c1a3ddb3fcfb82 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 24 Jun 2020 17:31:05 +0300 Subject: [PATCH] Fix using current database while checking access rights. --- src/Interpreters/Context.cpp | 2 +- tests/integration/helpers/client.py | 17 ++++--- tests/integration/helpers/cluster.py | 16 +++---- .../test_create_user_and_login/test.py | 46 +++++++++++++------ 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/Interpreters/Context.cpp b/src/Interpreters/Context.cpp index 02060534aef..b691e9aaf60 100644 --- a/src/Interpreters/Context.cpp +++ b/src/Interpreters/Context.cpp @@ -1062,8 +1062,8 @@ void Context::setCurrentDatabase(const String & name) { DatabaseCatalog::instance().assertDatabaseExists(name); auto lock = getLock(); - calculateAccessRights(); current_database = name; + calculateAccessRights(); } diff --git a/tests/integration/helpers/client.py b/tests/integration/helpers/client.py index 10962cfb724..0ca6a977868 100644 --- a/tests/integration/helpers/client.py +++ b/tests/integration/helpers/client.py @@ -17,11 +17,11 @@ class Client: self.command += ['--host', self.host, '--port', str(self.port), '--stacktrace'] - def query(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, ignore_error=False): - return self.get_query_request(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, ignore_error=ignore_error).get_answer() + def query(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None, ignore_error=False): + return self.get_query_request(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database, ignore_error=ignore_error).get_answer() - def get_query_request(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, ignore_error=False): + def get_query_request(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None, ignore_error=False): command = self.command[:] if stdin is None: @@ -40,15 +40,18 @@ class Client: if password is not None: command += ['--password', password] + if database is not None: + command += ['--database', database] + return CommandRequest(command, stdin, timeout, ignore_error) - def query_and_get_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None): - return self.get_query_request(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password).get_error() + def query_and_get_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None): + return self.get_query_request(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database).get_error() - def query_and_get_answer_with_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None): - return self.get_query_request(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password).get_answer_and_error() + def query_and_get_answer_with_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None): + return self.get_query_request(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database).get_answer_and_error() class QueryTimeoutExceedException(Exception): pass diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index d291d06851f..dec14361a0f 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -737,15 +737,15 @@ class ClickHouseInstance: return "-fsanitize=thread" in build_opts # Connects to the instance via clickhouse-client, sends a query (1st argument) and returns the answer - def query(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, ignore_error=False): - return self.client.query(sql, stdin, timeout, settings, user, password, ignore_error) + def query(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None, ignore_error=False): + return self.client.query(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database, ignore_error=ignore_error) - def query_with_retry(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, ignore_error=False, + def query_with_retry(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None, ignore_error=False, retry_count=20, sleep_time=0.5, check_callback=lambda x: True): result = None for i in range(retry_count): try: - result = self.query(sql, stdin, timeout, settings, user, password, ignore_error) + result = self.query(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database, ignore_error=ignore_error) if check_callback(result): return result time.sleep(sleep_time) @@ -762,12 +762,12 @@ class ClickHouseInstance: return self.client.get_query_request(*args, **kwargs) # Connects to the instance via clickhouse-client, sends a query (1st argument), expects an error and return its code - def query_and_get_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None): - return self.client.query_and_get_error(sql, stdin, timeout, settings, user, password) + def query_and_get_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None): + return self.client.query_and_get_error(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database) # The same as query_and_get_error but ignores successful query. - def query_and_get_answer_with_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None): - return self.client.query_and_get_answer_with_error(sql, stdin, timeout, settings, user, password) + def query_and_get_answer_with_error(self, sql, stdin=None, timeout=None, settings=None, user=None, password=None, database=None): + return self.client.query_and_get_answer_with_error(sql, stdin=stdin, timeout=timeout, settings=settings, user=user, password=password, database=database) # Connects to the instance via HTTP interface, sends a query and returns the answer def http_query(self, sql, data=None, params=None, user=None, password=None, expect_fail_and_get_error=False): diff --git a/tests/integration/test_create_user_and_login/test.py b/tests/integration/test_create_user_and_login/test.py index ddb3e57c63b..ae75f69d28a 100644 --- a/tests/integration/test_create_user_and_login/test.py +++ b/tests/integration/test_create_user_and_login/test.py @@ -12,9 +12,10 @@ def started_cluster(): try: cluster.start() - instance.query("CREATE TABLE test_table(x UInt32, y UInt32) ENGINE = MergeTree ORDER BY tuple()") - instance.query("INSERT INTO test_table VALUES (1,5), (2,10)") - + instance.query("CREATE DATABASE test") + instance.query("CREATE TABLE test.table(x UInt32, y UInt32) ENGINE = MergeTree ORDER BY tuple()") + instance.query("INSERT INTO test.table VALUES (1,5), (2,10)") + yield cluster finally: @@ -27,6 +28,7 @@ def cleanup_after_test(): yield finally: instance.query("DROP USER IF EXISTS A, B") + instance.query("DROP TABLE IF EXISTS default.table") def test_login(): @@ -38,28 +40,28 @@ def test_login(): def test_grant_and_revoke(): instance.query("CREATE USER A") - assert "Not enough privileges" in instance.query_and_get_error("SELECT * FROM test_table", user='A') + assert "Not enough privileges" in instance.query_and_get_error("SELECT * FROM test.table", user='A') - instance.query('GRANT SELECT ON test_table TO A') - assert instance.query("SELECT * FROM test_table", user='A') == "1\t5\n2\t10\n" + instance.query('GRANT SELECT ON test.table TO A') + assert instance.query("SELECT * FROM test.table", user='A') == "1\t5\n2\t10\n" - instance.query('REVOKE SELECT ON test_table FROM A') - assert "Not enough privileges" in instance.query_and_get_error("SELECT * FROM test_table", user='A') + instance.query('REVOKE SELECT ON test.table FROM A') + assert "Not enough privileges" in instance.query_and_get_error("SELECT * FROM test.table", user='A') def test_grant_option(): instance.query("CREATE USER A") instance.query("CREATE USER B") - instance.query('GRANT SELECT ON test_table TO A') - assert instance.query("SELECT * FROM test_table", user='A') == "1\t5\n2\t10\n" - assert "Not enough privileges" in instance.query_and_get_error("GRANT SELECT ON test_table TO B", user='A') + instance.query('GRANT SELECT ON test.table TO A') + assert instance.query("SELECT * FROM test.table", user='A') == "1\t5\n2\t10\n" + assert "Not enough privileges" in instance.query_and_get_error("GRANT SELECT ON test.table TO B", user='A') - instance.query('GRANT SELECT ON test_table TO A WITH GRANT OPTION') - instance.query("GRANT SELECT ON test_table TO B", user='A') - assert instance.query("SELECT * FROM test_table", user='B') == "1\t5\n2\t10\n" + instance.query('GRANT SELECT ON test.table TO A WITH GRANT OPTION') + instance.query("GRANT SELECT ON test.table TO B", user='A') + assert instance.query("SELECT * FROM test.table", user='B') == "1\t5\n2\t10\n" - instance.query('REVOKE SELECT ON test_table FROM A, B') + instance.query('REVOKE SELECT ON test.table FROM A, B') def test_introspection(): @@ -100,3 +102,17 @@ def test_introspection(): TSV([[ "A", "\N", "SELECT", "test", "table", "\N", 0, 0 ], [ "B", "\N", "CREATE", "\N", "\N", "\N", 0, 0 ], [ "B", "\N", "CREATE", "\N", "\N", "\N", 0, 1 ]]) + + +def test_current_database(): + instance.query("CREATE USER A") + instance.query("GRANT SELECT ON table TO A", database="test") + + assert instance.query("SHOW GRANTS FOR A") == TSV([ "GRANT SELECT ON test.table TO A" ]) + assert instance.query("SHOW GRANTS FOR A", database="test") == TSV([ "GRANT SELECT ON test.table TO A" ]) + + assert instance.query("SELECT * FROM test.table", user='A') == "1\t5\n2\t10\n" + assert instance.query("SELECT * FROM table", user='A', database='test') == "1\t5\n2\t10\n" + + instance.query("CREATE TABLE default.table(x UInt32, y UInt32) ENGINE = MergeTree ORDER BY tuple()") + assert "Not enough privileges" in instance.query_and_get_error("SELECT * FROM table", user='A')