tarantool-php icon indicating copy to clipboard operation
tarantool-php copied to clipboard

Do not flush schema on failed authentication

Open rybakit opened this issue 8 years ago • 10 comments

$t = new Tarantool();
//var_dump($t->select('space_conn'));
$t->authenticate('user_foo', 'foo');
var_dump($t->select('space_conn'));

prints No space 'space_conn' defined error message, which is wrong as space_conn does exist. After uncommenting the second line the script starts throwing the correct message Query error 55: Read access denied for user 'user_foo' to space 'space_conn'.

rybakit avatar Mar 02 '16 16:03 rybakit

Your user doesn't know about existence and ID of 'space_conn', there's no way to get this messge.

bigbes avatar Mar 03 '16 05:03 bigbes

@bigbes As there's no way to get this message about existence of a space, I would expect the code produces the same result, namely No space 'space_conn' defined error regardless of what was called before authenticate(), which is not the case currently. /cc @kostja

rybakit avatar Mar 08 '16 12:03 rybakit

@rybakit so what's the problem now?

First

Input script:

<?php
        $t = new Tarantool();
        var_dump($t->select('example'));
        $t->authenticate('user_foo', 'foo');

Output:

blikh@bigbes-laptop:~/src/work/tarantool-php$ php -c tarantool-1.ini 1.php

Fatal error: Uncaught exception 'Exception' with message 'No space 'example' defined' in /Users/blikh/src/work/tarantool-php/1.php:3
Stack trace:
#0 /Users/blikh/src/work/tarantool-php/1.php(3): Tarantool->select('example')
#1 {main}
  thrown in /Users/blikh/src/work/tarantool-php/1.php on line 3

Second

Input script:

<?php
        $t = new Tarantool();
        $t->authenticate('user_foo', 'foo');
        var_dump($t->select('example'));

Output:

blikh@bigbes-laptop:~/src/work/tarantool-php$ php -c tarantool-1.ini 1.php

Fatal error: Uncaught exception 'Exception' with message 'No space 'example' defined' in /Users/blikh/src/work/tarantool-php/1.php:4
Stack trace:
#0 /Users/blikh/src/work/tarantool-php/1.php(4): Tarantool->select('example')
#1 {main}
  thrown in /Users/blikh/src/work/tarantool-php/1.php on line 4

bigbes avatar Mar 09 '16 16:03 bigbes

instance.lua:

box.cfg {
    listen = 3301,
    log_level = 6,
    wal_mode = 'none',
    snap_dir = '/tmp',
    slab_alloc_arena = .1,
}

box.schema.user.grant('guest', 'read,write,execute', 'universe')

local credentials = {
    user_foo = 'foo',
}

for username, password in pairs(credentials) do
    if box.schema.user.exists(username) then
        box.schema.user.drop(username)
    end

    box.schema.user.create(username, { password = password })
end

local function create_space(name)
    if box.space[name] then
        box.space[name]:drop()
    end

    return box.schema.space.create(name, {temporary = true})
end

local space = create_space('space_conn')
space:create_index('primary', {type = 'tree', parts = {1, 'num'}})

test1.php:

<?php

$t = new Tarantool();
$t->authenticate('user_foo', 'foo');
$t->select('space_conn');

Output:

Fatal error: Uncaught exception 'Exception' with message 'No space 'space_conn' defined' in ...

test2.php:

<?php

$t = new Tarantool();
$t->select('space_conn');
$t->authenticate('user_foo', 'foo');
$t->select('space_conn');

Output:

Fatal error: Uncaught exception 'Exception' with message 'Query error 55: Read access denied for user 'user_foo' to space 'space_conn'' in ...

So, the second script is leaking info about the space existence.

I also checked the python driver, it has the same issue:

import tarantool

con = tarantool.Connection('127.0.0.1', 3301)

# the fist select has influence on error type
# generated by the second select
# comment out the following line to test the difference
con.select('space_conn')

con.authenticate('user_foo', 'foo')
con.select('space_conn')

It looks like it's Tarantool server issue, but @kostja asked me to open an issue here, in the driver repo first.

rybakit avatar Mar 09 '16 17:03 rybakit

It was related to schema, that wasn't flushed when authenticate was called. Must be fixed in latest master.

bigbes avatar Mar 09 '16 21:03 bigbes

:+1: thanks

rybakit avatar Mar 09 '16 22:03 rybakit

@bigbes What do you think about flushing the schema only on successful authentication? Although it might be a very rare case, it can save at least 3 extra selects (according to my test) in the worst case in scenarios like this:

$res = $t->select('my_space', 1);

try {
    $t->authenticate('user_foo', 'incorrect_password');
} catch (Exception $e) {
    $res = $t->select('my_space', 2);
}

rybakit avatar Mar 10 '16 00:03 rybakit

@rybakit we may do this, but i'm not sure that it's worth it. I'll do it on PHP7 branch.

bigbes avatar May 04 '16 12:05 bigbes

I'm not sure either ;)

rybakit avatar May 04 '16 12:05 rybakit

Updated the title to reflect what is expected to do within the issue.

Totktonada avatar Mar 23 '20 18:03 Totktonada