[tarantool-patches] Re: [PATCH v1 1/1] sql: check access rights of table in VIEW

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

On 24/04/2019 15:16, Kirill Shcherbatov wrote:
> When access is performed using VIEW, access rights should be
> checked against table[s] which it is referencing, not against
> VIEW itself. Added a test case to verify this behaviour.
> 
> Closes #4104
> ---
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4104-view-access-check
> Issue: https://github.com/tarantool/tarantool/issues/4104
> 
>  test/sql/gh-4104-view-access-check.result   | 64 +++++++++++++++++++++
>  test/sql/gh-4104-view-access-check.test.lua | 21 +++++++
>  2 files changed, 85 insertions(+)
>  create mode 100644 test/sql/gh-4104-view-access-check.result
>  create mode 100644 test/sql/gh-4104-view-access-check.test.lua
> 
> diff --git a/test/sql/gh-4104-view-access-check.result b/test/sql/gh-4104-view-access-check.result
> new file mode 100644
> index 000000000..1eb9bebe8
> --- /dev/null
> +++ b/test/sql/gh-4104-view-access-check.result
> @@ -0,0 +1,64 @@
> +test_run = require('test_run').new()

1. You do not use this object, so it can be omitted.

> +---
> +...
> +box.execute("CREATE TABLE supersecret(id INT PRIMARY KEY, data TEXT);")
> +---
> +- row_count: 1
> +...
> +box.execute("CREATE TABLE supersecret2(id INT PRIMARY KEY, data TEXT);")
> +---
> +- row_count: 1
> +...
> +box.execute("INSERT INTO supersecret VALUES(1, 'very very big secret');")
> +---
> +- row_count: 1
> +...
> +box.execute("INSERT INTO supersecret2 VALUES(1, 'very big secret 2');")
> +---
> +- row_count: 1
> +...
> +box.execute("CREATE VIEW supersecret_leak AS  SELECT * FROM supersecret, supersecret2;")
> +---
> +- row_count: 1
> +...
> +LISTEN = require('uri').parse(box.cfg.listen)
> +---
> +...
> +remote = require 'net.box'
> +---
> +...
> +cn = remote.connect(LISTEN.host, LISTEN.service)

2. You pass here box.cfg.listen directly, without splitting into
parts.

Please, consider my review fixes below and on the branch:

===================================================================
diff --git a/test/sql/gh-4104-view-access-check.result b/test/sql/gh-4104-view-access-check.result
index 1eb9bebe8..d38b633c3 100644
--- a/test/sql/gh-4104-view-access-check.result
+++ b/test/sql/gh-4104-view-access-check.result
@@ -1,6 +1,3 @@
-test_run = require('test_run').new()
----
-...
 box.execute("CREATE TABLE supersecret(id INT PRIMARY KEY, data TEXT);")
 ---
 - row_count: 1
@@ -21,13 +18,10 @@ box.execute("CREATE VIEW supersecret_leak AS  SELECT * FROM supersecret, superse
 ---
 - row_count: 1
 ...
-LISTEN = require('uri').parse(box.cfg.listen)
----
-...
 remote = require 'net.box'
 ---
 ...
-cn = remote.connect(LISTEN.host, LISTEN.service)
+cn = remote.connect(box.cfg.listen)
 ---
 ...
 box.schema.user.grant('guest','read', 'space', 'SUPERSECRET_LEAK')
diff --git a/test/sql/gh-4104-view-access-check.test.lua b/test/sql/gh-4104-view-access-check.test.lua
index 2a44516ce..d1d19fc28 100644
--- a/test/sql/gh-4104-view-access-check.test.lua
+++ b/test/sql/gh-4104-view-access-check.test.lua
@@ -1,13 +1,10 @@
-test_run = require('test_run').new()
-
 box.execute("CREATE TABLE supersecret(id INT PRIMARY KEY, data TEXT);")
 box.execute("CREATE TABLE supersecret2(id INT PRIMARY KEY, data TEXT);")
 box.execute("INSERT INTO supersecret VALUES(1, 'very very big secret');")
 box.execute("INSERT INTO supersecret2 VALUES(1, 'very big secret 2');")
 box.execute("CREATE VIEW supersecret_leak AS  SELECT * FROM supersecret, supersecret2;")
-LISTEN = require('uri').parse(box.cfg.listen)
 remote = require 'net.box'
-cn = remote.connect(LISTEN.host, LISTEN.service)
+cn = remote.connect(box.cfg.listen)
 
 box.schema.user.grant('guest','read', 'space', 'SUPERSECRET_LEAK')
 cn:execute('SELECT * FROM SUPERSECRET_LEAK')