Skip to content

Commit e0e4d9d

Browse files
committed
[testapp] Get test passwords from test env
Don't spread the test passwords all over the files, but look them up from the test environment instead. Change-Id: Ib6e4dfdccaddbb7f46cc952ecd765e79f82c6e02 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/203178 Reviewed-by: Jim Walker <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent 1aa0661 commit e0e4d9d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+278
-186
lines changed

cluster_framework/auth_provider_service.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,19 @@ void AuthProviderService::upsertUser(UserEntry entry) {
165165
}
166166
}
167167

168+
std::optional<UserEntry> AuthProviderService::lookupUser(
169+
const std::string& user) {
170+
return users.withWLock([&user](auto& db) -> std::optional<UserEntry> {
171+
// check to see if we're replacing an entry
172+
for (auto& e : db) {
173+
if (e.username == user) {
174+
return e;
175+
}
176+
}
177+
return {};
178+
});
179+
}
180+
168181
void AuthProviderService::onRequest(bufferevent* bev,
169182
const mcbp::Request& req) {
170183
switch (req.getMagic()) {

cluster_framework/auth_provider_service.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ class AuthProviderService {
5353
void upsertUser(UserEntry entry);
5454
void removeUser(const std::string& user);
5555

56+
std::optional<UserEntry> lookupUser(const std::string& user);
57+
5658
protected:
5759
/// Handle the authenticate request and send the reply
5860
void onAuthenticate(bufferevent* bev, const cb::mcbp::Request& req);

cluster_framework/cluster.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ ClusterImpl::ClusterImpl(std::vector<std::unique_ptr<Node>>& nod,
107107
forAllNodes([this, &globalmap](const auto& node) {
108108
auto connection = node.getConnection();
109109
connection->connect();
110-
connection->authenticate("@admin", "password", "plain");
110+
connection->authenticate("@admin");
111111
connection->setAgentName("cluster_testapp");
112112
connection->setFeatures({cb::mcbp::Feature::MUTATION_SEQNO,
113113
cb::mcbp::Feature::XATTR,
@@ -160,7 +160,7 @@ void ClusterImpl::createBucketOnNode(const Node& node,
160160
std::string_view config) {
161161
auto connection = node.getConnection();
162162
connection->connect();
163-
connection->authenticate("@admin", "password", "plain");
163+
connection->authenticate("@admin");
164164
connection->setAgentName("cluster_testapp");
165165
connection->setFeatures({cb::mcbp::Feature::MUTATION_SEQNO,
166166
cb::mcbp::Feature::XATTR,
@@ -306,7 +306,7 @@ std::shared_ptr<Bucket> ClusterImpl::createBucket(
306306
forAllNodes([&name](const auto& node) {
307307
auto connection = node.getConnection();
308308
connection->connect();
309-
connection->authenticate("@admin", "password", "plain");
309+
connection->authenticate("@admin");
310310
try {
311311
connection->deleteBucket(name);
312312
} catch (const std::exception&) {
@@ -342,7 +342,7 @@ void ClusterImpl::deleteBucket(const std::string& name) {
342342
forAllNodes([name](const auto& node) {
343343
auto connection = node.getConnection();
344344
connection->connect();
345-
connection->authenticate("@admin", "password", "plain");
345+
connection->authenticate("@admin");
346346
connection->deleteBucket(name);
347347
// And nuke the files for the database on that node.
348348
removeWithRetry(node.directory / name);

protocol/connection/client_connection.cc

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@
4343

4444
static const bool packet_dump = getenv("COUCHBASE_PACKET_DUMP") != nullptr;
4545

46+
folly::Synchronized<std::function<std::string(const std::string&)>, std::mutex>
47+
MemcachedConnection::lookupPasswordCallback;
48+
void MemcachedConnection::setLookupUserPasswordFunction(
49+
std::function<std::string(const std::string&)> func) {
50+
*lookupPasswordCallback.lock() = std::move(func);
51+
}
52+
4653
/// Helper function to check if we're running in unit test mode or not
4754
static bool is_unit_test_mode() {
4855
return getenv("MEMCACHED_UNIT_TESTS") != nullptr;
@@ -1198,9 +1205,28 @@ void MemcachedConnection::recvResponse(BinprotResponse& response,
11981205
traceData = response.getTracingData();
11991206
}
12001207

1201-
void MemcachedConnection::authenticate(const std::string& username,
1202-
const std::string& password,
1203-
const std::string& mech) {
1208+
void MemcachedConnection::authenticate(
1209+
const std::string& user,
1210+
const std::optional<std::string>& password,
1211+
const std::string& mech) {
1212+
std::string pw;
1213+
if (password) {
1214+
pw = *password;
1215+
} else {
1216+
pw = lookupPasswordCallback.withLock([&user](auto& cb) -> std::string {
1217+
if (cb) {
1218+
return cb(user);
1219+
}
1220+
return {};
1221+
});
1222+
}
1223+
1224+
return doSaslAuthenticate(user, pw, mech);
1225+
}
1226+
1227+
void MemcachedConnection::doSaslAuthenticate(const std::string& username,
1228+
const std::string& password,
1229+
const std::string& mech) {
12041230
cb::sasl::client::ClientContext client(
12051231
[username]() -> std::string { return username; },
12061232
[password]() -> std::string { return password; },

protocol/connection/client_connection.h

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#pragma once
1111

1212
#include <engines/ewouldblock_engine/ewouldblock_engine.h>
13+
#include <folly/Synchronized.h>
1314
#include <folly/io/async/DelayedDestruction.h>
1415
#include <memcached/bucket_type.h>
1516
#include <memcached/engine_error.h>
@@ -407,14 +408,13 @@ class MemcachedConnection {
407408
}
408409

409410
/**
410-
* Perform a SASL authentication to memcached
411+
* Perform SASL authentication to memcached for the provided user
412+
* (looking up the password by using the callback function).
411413
*
412-
* @param username the username to use in authentication
413-
* @param password the password to use in authentication
414-
* @param mech the SASL mech to use
414+
* @param user The user to authenticate
415415
*/
416-
void authenticate(const std::string& username,
417-
const std::string& password,
416+
void authenticate(const std::string& user,
417+
const std::optional<std::string>& password = {},
418418
const std::string& mech = "PLAIN");
419419

420420
/**
@@ -1088,7 +1088,21 @@ class MemcachedConnection {
10881088
userValidateReceivedFrameCallback = std::move(callback);
10891089
}
10901090

1091+
static void setLookupUserPasswordFunction(
1092+
std::function<std::string(const std::string&)> func);
1093+
10911094
protected:
1095+
/**
1096+
* Perform a SASL authentication to memcached
1097+
*
1098+
* @param username the username to use in authentication
1099+
* @param password the password to use in authentication
1100+
* @param mech the SASL mech to use
1101+
*/
1102+
void doSaslAuthenticate(const std::string& username,
1103+
const std::string& password,
1104+
const std::string& mech);
1105+
10921106
void sendBuffer(const std::vector<iovec>& buf);
10931107
void sendBuffer(cb::const_byte_buffer buf);
10941108

@@ -1165,6 +1179,10 @@ class MemcachedConnection {
11651179
std::function<void(const cb::mcbp::Header&)>
11661180
userValidateReceivedFrameCallback;
11671181
std::unique_ptr<cb::json::SyntaxValidator> jsonValidator;
1182+
1183+
static folly::Synchronized<std::function<std::string(const std::string&)>,
1184+
std::mutex>
1185+
lookupPasswordCallback;
11681186
};
11691187

11701188
#include <fmt/ostream.h>

tests/testapp/testapp.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ void TestappTest::rebuildAdminConnection() {
103103
connectionMap.iterate([](const auto& c) {
104104
if (!adminConnection) {
105105
adminConnection = c.clone();
106-
adminConnection->authenticate(
107-
"@admin", mcd_env->getPassword("@admin"), "PLAIN");
106+
adminConnection->authenticate("@admin");
108107
adminConnection->unselectBucket();
109108

110109
std::vector<cb::mcbp::Feature> features = {
@@ -128,7 +127,7 @@ void TestappTest::rebuildUserConnection(bool tls) {
128127
connectionMap.iterate([&](const auto& c) {
129128
if (!userConnection && (c.isSsl() == tls)) {
130129
userConnection = c.clone();
131-
userConnection->authenticate("Luke", mcd_env->getPassword("Luke"));
130+
userConnection->authenticate("Luke");
132131
userConnection->selectBucket(bucketName);
133132
}
134133
});
@@ -682,7 +681,7 @@ SOCKET connect_to_server_plain() {
682681
}
683682
MemcachedConnection connection("127.0.0.1", port, AF_INET, false);
684683
connection.connect();
685-
connection.authenticate("Luke", mcd_env->getPassword("Luke"));
684+
connection.authenticate("Luke");
686685
connection.selectBucket("default");
687686
return connection.releaseSocket();
688687
}
@@ -1166,7 +1165,7 @@ MemcachedConnection& TestappTest::getConnection() {
11661165

11671166
MemcachedConnection& TestappTest::getAdminConnection() {
11681167
auto& conn = getConnection();
1169-
conn.authenticate("@admin", "password", conn.getSaslMechanisms());
1168+
conn.authenticate("@admin");
11701169
return conn;
11711170
}
11721171

@@ -1333,6 +1332,9 @@ int main(int argc, char** argv) {
13331332
exit(EXIT_FAILURE);
13341333
}
13351334

1335+
MemcachedConnection::setLookupUserPasswordFunction(
1336+
[](const auto& user) { return mcd_env->getPassword(user); });
1337+
13361338
cb::net::initialize();
13371339
try {
13381340
cb::backtrace::initialize();

tests/testapp/testapp_arithmetic.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ TEST_P(ArithmeticTest, TestConcurrentAccess) {
124124
const int incrDelta = 7;
125125
const int decrDelta = -3;
126126

127-
conn1->authenticate("Luke", mcd_env->getPassword("Luke"));
127+
conn1->authenticate("Luke");
128128
conn1->selectBucket(bucketName);
129-
conn2->authenticate("Luke", mcd_env->getPassword("Luke"));
129+
conn2->authenticate("Luke");
130130
conn2->selectBucket(bucketName);
131131

132132
// Create the starting point

tests/testapp/testapp_audit.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ TEST_P(AuditTest, ValidateAuditLogFileCreated) {
307307
*/
308308
TEST_P(AuditTest, AuditIllegalPacket) {
309309
auto& conn = getConnection();
310-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
310+
conn.authenticate("Luke");
311311
conn.selectBucket(bucketName);
312312

313313
// A set command should have 8 bytes of extra;
@@ -440,7 +440,7 @@ TEST_P(AuditTest, AuditSelectBucket) {
440440
// event.
441441
TEST_P(AuditTest, AuditSubdocLookup) {
442442
auto& conn = getConnection();
443-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
443+
conn.authenticate("Luke");
444444
conn.selectBucket(bucketName);
445445
conn.store("doc", Vbid(0), "{\"foo\": 1}");
446446
BinprotSubdocCommand cmd(cb::mcbp::ClientOpcode::SubdocGet,
@@ -467,7 +467,7 @@ TEST_P(AuditTest, AuditSubdocLookup) {
467467
// event.
468468
TEST_P(AuditTest, AuditSubdocMutation) {
469469
auto& conn = getConnection();
470-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
470+
conn.authenticate("Luke");
471471
conn.selectBucket(bucketName);
472472
BinprotSubdocCommand cmd(cb::mcbp::ClientOpcode::SubdocDictUpsert,
473473
"doc",
@@ -493,7 +493,7 @@ TEST_P(AuditTest, AuditSubdocMutation) {
493493
// event.
494494
TEST_P(AuditTest, AuditSubdocMultiMutation) {
495495
auto& conn = getConnection();
496-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
496+
conn.authenticate("Luke");
497497
conn.selectBucket(bucketName);
498498
BinprotSubdocMultiMutationCommand cmd(
499499
"doc",
@@ -524,7 +524,7 @@ TEST_P(AuditTest, AuditSubdocMultiMutation) {
524524
// Check that a delete via subdoc is audited correctly.
525525
TEST_P(AuditTest, AuditSubdocMultiMutationDelete) {
526526
auto& conn = getConnection();
527-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
527+
conn.authenticate("Luke");
528528
conn.selectBucket(bucketName);
529529
conn.store("doc", Vbid(0), "foo");
530530

@@ -611,7 +611,7 @@ TEST_P(AuditTest, MB33603_Filtering) {
611611
doc.value = "blah blah";
612612

613613
auto jane = userConnection->clone();
614-
jane->authenticate("Jane", mcd_env->getPassword("Jane"), "PLAIN");
614+
jane->authenticate("Jane");
615615
jane->selectBucket("default");
616616
// That should not generate an audit event
617617
jane->mutate(doc, Vbid{0}, MutationType::Set);
@@ -731,7 +731,7 @@ TEST_P(AuditTest, MB41183_UnifiedConnectionDescription) {
731731
TEST_P(AuditTest, MB51863) {
732732
auto& conn = getConnection();
733733

734-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
734+
conn.authenticate("Luke");
735735
conn.selectBucket(bucketName);
736736
std::vector<cb::mcbp::Feature> features = {
737737
{cb::mcbp::Feature::MUTATION_SEQNO,

tests/testapp/testapp_bucket.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ static void deleteBucket(
6464
const std::string& name,
6565
std::function<void(const std::string&)> stateCallback) {
6666
auto clone = conn.clone();
67-
clone->authenticate("@admin", "password", "PLAIN");
67+
clone->authenticate("@admin");
6868
const auto timeout =
6969
std::chrono::system_clock::now() + std::chrono::seconds{5};
7070
conn.sendCommand(
@@ -109,7 +109,7 @@ TEST_P(BucketTest, DeleteWhileClientSendCommand) {
109109
adminConnection->createBucket("bucket", "", BucketType::Memcached);
110110

111111
auto& second_conn = getConnection();
112-
second_conn.authenticate("Luke", mcd_env->getPassword("Luke"), "PLAIN");
112+
second_conn.authenticate("Luke");
113113
second_conn.selectBucket("bucket");
114114

115115
// We need to get the second connection sitting the `conn_read_packet_body`
@@ -146,7 +146,7 @@ TEST_P(BucketTest, DeleteWhileClientConnectedAndEWouldBlocked) {
146146
for (int jj = 0; jj < 5; ++jj) {
147147
connections.emplace_back(conn.clone());
148148
auto& c = connections.back();
149-
c->authenticate("Luke", mcd_env->getPassword("Luke"), "PLAIN");
149+
c->authenticate("Luke");
150150
c->selectBucket("bucket");
151151

152152
auto testfile =
@@ -212,7 +212,7 @@ TEST_P(BucketTest, DeleteWhileSendDataAndFullWriteBuffer) {
212212
BucketType::Memcached);
213213

214214
auto& conn = getConnection();
215-
conn.authenticate("Luke", mcd_env->getPassword("Luke"));
215+
conn.authenticate("Luke");
216216
const auto id = conn.getServerConnectionId();
217217
conn.selectBucket("bucket");
218218

@@ -291,7 +291,7 @@ TEST_P(BucketTest, TestNoAutoSelectOfBucketForNormalUser) {
291291
adminConnection->createBucket("rbac_test", "", BucketType::Memcached);
292292

293293
auto& conn = getConnection();
294-
conn.authenticate("smith", "smithpassword", "PLAIN");
294+
conn.authenticate("smith");
295295
auto response = conn.execute(
296296
BinprotGenericCommand{cb::mcbp::ClientOpcode::Get, name});
297297
EXPECT_EQ(cb::mcbp::Status::NoBucket, response.getStatus());
@@ -311,7 +311,7 @@ TEST_P(BucketTest, TestListSomeBuckets) {
311311

312312
// Reconnect and authenticate as a user with access to only one of them
313313
auto& conn = getConnection();
314-
conn.authenticate("smith", mcd_env->getPassword("smith"));
314+
conn.authenticate("smith");
315315
const std::vector<std::string> expected = {"rbac_test"};
316316
EXPECT_EQ(expected, conn.listBuckets());
317317

0 commit comments

Comments
 (0)