Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ INCLUDE_DIRECTORIES(.)
INCLUDE_DIRECTORIES(./src)
INCLUDE_DIRECTORIES(./third_party/libev)
ADD_LIBRARY(ev STATIC third_party/libev/ev.c)
ADD_LIBRARY(scrmbl STATIC third_party/scramble/scramble.c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you set scramble.c as a separate lib?

ADD_LIBRARY(sha STATIC third_party/sha1/sha1.c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question..

TARGET_COMPILE_DEFINITIONS(ev PRIVATE EV_STANDALONE=1)
TARGET_COMPILE_OPTIONS(ev PRIVATE -w)
ADD_EXECUTABLE(MempoolUnitTest.test src/Utils/Mempool.hpp test/MempoolUnitTest.cpp)
Expand All @@ -32,7 +34,7 @@ ADD_EXECUTABLE(Client.test src/Client/Connector.hpp test/ClientTest.cpp)
ADD_EXECUTABLE(ClientPerfTest.test src/Client/Connector.hpp test/ClientPerfTest.cpp)
ADD_EXECUTABLE(SimpleExample examples/Simple.cpp)
TARGET_LINK_LIBRARIES(ClientPerfTest.test ev)
TARGET_LINK_LIBRARIES(Client.test ev)
TARGET_LINK_LIBRARIES(Client.test ev scrmbl sha)

IF (benchmark_FOUND)
ADD_EXECUTABLE(BufferGPerf.test src/Buffer/Buffer.hpp test/BufferGPerfTest.cpp)
Expand Down
18 changes: 18 additions & 0 deletions src/Client/Connection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,15 @@ class Connection {
rid_t call(const std::string &func, const T &args);
rid_t ping();

/**
* Authorizate user with user_name and password
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not an authorization, but rather preparation of authorization request.

* @param user_name name of user
* @param password user's password
* @retval request id
*/
rid_t auth(const std::string& user_name, const std::string& password);


void setError(const std::string &msg);
std::string& getError();
void reset();
Expand Down Expand Up @@ -406,6 +415,15 @@ Connection<BUFFER, NetProvider>::select(const T &key, uint32_t space_id,
return RequestEncoder<BUFFER>::getSync();
}

template<class BUFFER, class NetProvider>
rid_t
Connection<BUFFER, NetProvider>::auth(const std::string& user_name, const std::string& password)
{
m_EndEncoded += m_Encoder.encodeAuth(user_name, password, m_Greeting);
m_Connector.readyToSend(*this);
return RequestEncoder<BUFFER>::getSync();
}

template<class BUFFER, class NetProvider>
void
Connection<BUFFER, NetProvider>::setError(const std::string &msg)
Expand Down
52 changes: 50 additions & 2 deletions src/Client/Connector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@
#include "DefaultNetProvider.hpp"
#include "../Utils/Timer.hpp"

struct Config {
const std::string addr;
unsigned port;
const std::string user_name = std::string();
const std::string password = std::string();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you have to call default constructor explicitly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better make Config to be class with proper constructors

constexpr static size_t DEFAULT_CONNECT_TIMEOUT = 2;
size_t timeout = DEFAULT_CONNECT_TIMEOUT;
};


template<class BUFFER, class NetProvider = DefaultNetProvider<BUFFER, NetworkEngine>>
class Connector
{
Expand All @@ -44,7 +54,11 @@ class Connector

int connect(Connection<BUFFER, NetProvider> &conn,
const std::string_view& addr, unsigned port,
size_t timeout = DEFAULT_CONNECT_TIMEOUT);
size_t timeout = Config::DEFAULT_CONNECT_TIMEOUT);


int connect(Connection<BUFFER, NetProvider> &conn, const Config& config);

void close(Connection<BUFFER, NetProvider> &conn);

int wait(Connection<BUFFER, NetProvider> &conn, rid_t future,
Expand All @@ -60,7 +74,7 @@ class Connector
void readyToDecode(Connection<BUFFER, NetProvider> &conn);
void readyToSend(Connection<BUFFER, NetProvider> &conn);

constexpr static size_t DEFAULT_CONNECT_TIMEOUT = 2;

private:
NetProvider m_NetProvider;
/**
Expand Down Expand Up @@ -102,6 +116,40 @@ Connector<BUFFER, NetProvider>::connect(Connection<BUFFER, NetProvider> &conn,
return 0;
}

template<class BUFFER, class NetProvider>
int
Connector<BUFFER, NetProvider>::connect(Connection<BUFFER, NetProvider> &conn, const Config& config)
{
if (conn.socket >= 0 && m_NetProvider.check(conn)) {
LOG_ERROR("Current connection to ", conn.socket, " is alive! "
"Please close it before connecting to the new address");
return -1;
}
if (m_NetProvider.connect(conn, config.addr, config.port, config.timeout) != 0) {
LOG_ERROR("Failed to connect to ", config.addr, ':', config.port);
LOG_ERROR("Reason: ", conn.getError());
return -1;
}
LOG_DEBUG("Connected to ", config.addr, ':', config.port, " has been established");
if (config.user_name != std::string() && config.password != std::string()) {
rid_t auth_f = conn.auth(config.user_name, config.password);
wait(conn, auth_f, 1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use magic constants.
You do forget to check wait return code.

std::optional<Response<BUFFER>> response = conn.getResponse(auth_f);


if (response->body.error_stack != std::nullopt) {
Error err = (*response->body.error_stack).error;
std::cout << "RESPONSE ERROR: msg=" << err.msg <<
" line=" << err.file << " file=" << err.file <<
" errno=" << err.saved_errno <<
" type=" << err.type_name <<
" code=" << err.errcode << std::endl;
return -1;
}
}
return 0;
}

template<class BUFFER, class NetProvider>
void
Connector<BUFFER, NetProvider>::close(Connection<BUFFER, NetProvider> &conn)
Expand Down
22 changes: 22 additions & 0 deletions src/Client/RequestEncoder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
#include <map>

#include "IprotoConstants.hpp"
#include "ResponseReader.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this header in encoder? Seems to be messy

#include "../mpp/mpp.hpp"
#include "../Utils/Logger.hpp"
#include "../../third_party/scramble/scramble.h"

enum IteratorType {
EQ = 0,
Expand Down Expand Up @@ -82,6 +84,7 @@ class RequestEncoder {
uint32_t index_id = 0,
uint32_t limit = UINT32_MAX, uint32_t offset = 0,
IteratorType iterator = EQ);
size_t encodeAuth(const std::string& user_name, const std::string& password, const Greeting& greeting);
template <class T>
size_t encodeCall(const std::string &func, const T &args);

Expand Down Expand Up @@ -236,6 +239,25 @@ RequestEncoder<BUFFER>::encodeSelect(const T &key,
return request_size + PREHEADER_SIZE;
}

template<class BUFFER>
size_t
RequestEncoder<BUFFER>::encodeAuth(const std::string& user_name, const std::string& password, const Greeting& greeting)
{
char scrambled[2 * SCRAMBLE_SIZE] = {};
scramble_prepare(scrambled, greeting.salt, password.c_str(), password.size());
iterator_t<BUFFER> request_start = m_Buf.end();
m_Buf.addBack('\xce');
m_Buf.addBack(uint32_t{0});
encodeHeader(Iproto::AUTH);
m_Enc.add(mpp::as_map(std::forward_as_tuple(
MPP_AS_CONST(Iproto::USER_NAME), user_name,
MPP_AS_CONST(Iproto::TUPLE), std::make_tuple("chap-sha1", scrambled))));
uint32_t request_size = (m_Buf.end() - request_start) - PREHEADER_SIZE;
m_Buf.set(request_start + 1, __builtin_bswap32(request_size));
return request_size + PREHEADER_SIZE;
}


template<class BUFFER>
template <class T>
size_t
Expand Down
125 changes: 102 additions & 23 deletions test/ClientTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#include "../src/Client/Connector.hpp"

const char *localhost = "127.0.0.1";
int port = 3301;
unsigned port = 3301;
int WAIT_TIMEOUT = 1000; //milliseconds

using Net_t = DefaultNetProvider<Buf_t, NetworkEngine>;
Expand All @@ -62,28 +62,29 @@ printResponse(Connection<BUFFER, NetProvider> &conn, Response<BUFFER> &response,
" code=" << err.errcode << std::endl;
return;
}
assert(response.body.data != std::nullopt);
Data<BUFFER>& data = *response.body.data;
if (data.tuples.empty()) {
std::cout << "Empty result" << std::endl;
return;
}
std::vector<UserTuple> tuples;
switch (format) {
case TUPLES:
tuples = decodeUserTuple(conn.getInBuf(), data);
break;
case MULTI_RETURN:
tuples = decodeMultiReturn(conn.getInBuf(), data);
break;
case SELECT_RETURN:
tuples = decodeSelectReturn(conn.getInBuf(), data);
break;
default:
assert(0);
}
for (auto const& t : tuples) {
std::cout << t << std::endl;
if (response.body.data != std::nullopt) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does assertion turn into if?

Data<BUFFER>& data = *response.body.data;
if (data.tuples.empty()) {
std::cout << "Empty result" << std::endl;
return;
}
std::vector<UserTuple> tuples;
switch (format) {
case TUPLES:
tuples = decodeUserTuple(conn.getInBuf(), data);
break;
case MULTI_RETURN:
tuples = decodeMultiReturn(conn.getInBuf(), data);
break;
case SELECT_RETURN:
tuples = decodeSelectReturn(conn.getInBuf(), data);
break;
default:
assert(0);
}
for (auto const& t : tuples) {
std::cout << t << std::endl;
}
}
}

Expand Down Expand Up @@ -557,6 +558,82 @@ single_conn_call(Connector<BUFFER, NetProvider> &client)
client.close(conn);
}

/* Authentication */
template <class BUFFER, class NetProvider = Net_t>
void
authentication_test(Connector<BUFFER, NetProvider> &client)
{
TEST_INIT(0);
TEST_CASE("Authentication within user_name and password");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case name is confusing

{
Connection<Buf_t, NetProvider> conn(client);
Config config = {localhost, port};
int rc = client.connect(conn, config);
fail_unless(rc == 0);
client.close(conn);
}

TEST_CASE("Authentication with user_name and password good");
{
Connection<Buf_t, NetProvider> conn(client);
Config config = {localhost, port, "Anastas", "123456"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use pre-defined constants:
const char *user = "..."

int rc = client.connect(conn, config);
fail_unless(rc == 0);
client.close(conn);
}

TEST_CASE("Authentication with user_name and wrong password");
{
Connection<Buf_t, NetProvider> conn(client);
Config config = {localhost, port, "Anastas", "1234567"};
int rc = client.connect(conn, config);
fail_unless(rc == -1);
client.close(conn);
}


TEST_CASE("Authentication with wrong user_name");
{
Connection<Buf_t, NetProvider> conn(client);
Config config = {localhost, port, "Anastasius", "123456"};
int rc = client.connect(conn, config);
fail_unless(rc == -1);
client.close(conn);
}

TEST_CASE("Authentication with some privileges without errors");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get assertion fault on this test:
Client.test: tntcxx/test/ClientTest.cpp:609: void authentication_test(Connector<BUFFER, NetProvider>&) [with BUFFER = tnt::Buffer<16384>; NetProvider = DefaultNetProvider<tnt::Buffer<16384>, NetworkEngine>]: Assertion `false' failed.

{
Connection<Buf_t, NetProvider> conn(client);
Config config = {localhost, port, "Anastas", "123456"};
int rc = client.connect(conn, config);
fail_unless(rc == 0);
uint32_t space_id = 512;
std::tuple data = std::make_tuple(666, "111", 1.01);
rid_t f1 = conn.space[space_id].replace(data);
data = std::make_tuple(777, "asd", 2.02);
rid_t f2 = conn.space[space_id].replace(data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authentication requires a bit more test cases. You can look at examples from Tarantool repo


client.wait(conn, f1, WAIT_TIMEOUT);
fail_unless(conn.futureIsReady(f1));
std::optional<Response<Buf_t>> response = conn.getResponse(f1);
printResponse<BUFFER, NetProvider>(conn, *response);
fail_unless(response != std::nullopt);
fail_unless(response->body.data != std::nullopt);
fail_unless(response->body.error_stack == std::nullopt);

client.wait(conn, f2, WAIT_TIMEOUT);
fail_unless(conn.futureIsReady(f2));
response = conn.getResponse(f2);
fail_unless(response != std::nullopt);
fail_unless(response->body.data != std::nullopt);
fail_unless(response->body.error_stack == std::nullopt);

client.close(conn);
}
}



int main()
{
if (cleanDir() != 0)
Expand Down Expand Up @@ -591,5 +668,7 @@ int main()
single_conn_upsert<Buf_t, NetLibEv_t>(another_client);
single_conn_select<Buf_t, NetLibEv_t>(another_client);
single_conn_call<Buf_t, NetLibEv_t>(another_client);

authentication_test<Buf_t>(client);
return 0;
}
3 changes: 3 additions & 0 deletions test/cfg.lua
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
box.cfg{listen = 3301, net_msg_max=10000, readahead=163200, log_level = 7, log='tarantool.txt'}
box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists=true})
box.schema.user.create('Anastas', {password='123456'})

if box.space.t then box.space.t:drop() end
s = box.schema.space.create('T')
s:format{{name='id',type='integer'},{name='a',type='string'},{name='b',type='number'}}
s:create_index('primary')
s:replace{1, 'asd', 1.123}

box.schema.user.grant('Anastas','read,write', 'space', 'T')

function remote_replace(arg1, arg2, arg3)
return box.space.T:replace({arg1, arg2, arg3})
end
Expand Down
Loading