Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not flush schema on failed authentication #71

Open
rybakit opened this issue Mar 2, 2016 · 10 comments
Open

Do not flush schema on failed authentication #71

rybakit opened this issue Mar 2, 2016 · 10 comments
Assignees
Labels
feature A new functionality
Milestone

Comments

@rybakit
Copy link
Collaborator

rybakit commented Mar 2, 2016

$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'.

@bigbes
Copy link
Contributor

bigbes commented Mar 3, 2016

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

@bigbes bigbes closed this as completed Mar 3, 2016
@rybakit
Copy link
Collaborator Author

rybakit commented Mar 8, 2016

@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 rybakit reopened this Mar 8, 2016
@rybakit rybakit changed the title Wrong error message Inconsistent error/behavior when using authenticate() Mar 8, 2016
@bigbes bigbes self-assigned this Mar 9, 2016
@bigbes
Copy link
Contributor

bigbes commented Mar 9, 2016

@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

@rybakit
Copy link
Collaborator Author

rybakit commented Mar 9, 2016

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.

@bigbes
Copy link
Contributor

bigbes commented Mar 9, 2016

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

@bigbes bigbes closed this as completed Mar 9, 2016
@rybakit
Copy link
Collaborator Author

rybakit commented Mar 9, 2016

👍 thanks

rybakit pushed a commit to tarantool-php/client that referenced this issue Mar 10, 2016
@rybakit
Copy link
Collaborator Author

rybakit commented Mar 10, 2016

@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);
}

@bigbes
Copy link
Contributor

bigbes commented May 4, 2016

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

@bigbes bigbes reopened this May 4, 2016
@rybakit
Copy link
Collaborator Author

rybakit commented May 4, 2016

I'm not sure either ;)

@bigbes bigbes added the feature A new functionality label Jun 22, 2016
@bigbes bigbes assigned bigbes and unassigned bigbes Jun 23, 2016
bigbes added a commit to bigbes/tarantool-php that referenced this issue Aug 14, 2018
bigbes added a commit to bigbes/tarantool-php that referenced this issue Dec 2, 2018
@Totktonada Totktonada added this to the wishlist milestone Mar 23, 2020
@Totktonada Totktonada changed the title Inconsistent error/behavior when using authenticate() Do not flush schema on failed authentication Mar 23, 2020
@Totktonada
Copy link
Member

Totktonada commented Mar 23, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new functionality
Projects
None yet
Development

No branches or pull requests

3 participants