diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index 0cc68416f4..663a0bbd7c 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -38,10 +38,34 @@ #include "Transaction/SmartContext.h" #include "Transaction/Status.h" #include "Utils/CollectionNameResolver.h" +#include "Utils/ExecContext.h" #include #include +namespace { +bool authorized(std::string const& user) { + auto context = arangodb::ExecContext::CURRENT; + if (context == nullptr || !arangodb::ExecContext::isAuthEnabled()) { + return true; + } + + if (context->isSuperuser()) { + return true; + } + + return (user == context->user()); +} + +std::string currentUser() { + auto context = arangodb::ExecContext::CURRENT; + if (context == nullptr || !arangodb::ExecContext::isAuthEnabled()) { + return ""; + } + return context->user(); +} +} // namespace + namespace arangodb { namespace transaction { @@ -163,6 +187,7 @@ Manager::ManagedTrx::ManagedTrx(MetaType t, TransactionState* st) : type(t), usedTimeSecs(TRI_microtime()), state(st), + user(::currentUser()), finalStatus(Status::UNDEFINED), rwlock() {} @@ -431,7 +456,7 @@ std::shared_ptr Manager::leaseManagedTrx(TRI_voc_tid_t tid WRITE_LOCKER(writeLocker, _transactions[bucket]._lock); auto it = _transactions[bucket]._managed.find(tid); - if (it == _transactions[bucket]._managed.end()) { + if (it == _transactions[bucket]._managed.end() || !::authorized(it->second.user)) { return nullptr; } @@ -487,7 +512,7 @@ void Manager::returnManagedTrx(TRI_voc_tid_t tid, AccessMode::Type mode) noexcep WRITE_LOCKER(writeLocker, _transactions[bucket]._lock); auto it = _transactions[bucket]._managed.find(tid); - if (it == _transactions[bucket]._managed.end()) { + if (it == _transactions[bucket]._managed.end() || !::authorized(it->second.user)) { LOG_TOPIC("1d5b0", WARN, Logger::TRANSACTIONS) << "managed transaction was not found"; TRI_ASSERT(false); @@ -524,7 +549,7 @@ transaction::Status Manager::getManagedTrxStatus(TRI_voc_tid_t tid) const { READ_LOCKER(writeLocker, _transactions[bucket]._lock); auto it = _transactions[bucket]._managed.find(tid); - if (it == _transactions[bucket]._managed.end()) { + if (it == _transactions[bucket]._managed.end() || !::authorized(it->second.user)) { return transaction::Status::UNDEFINED; } @@ -566,7 +591,7 @@ Result Manager::updateTransaction(TRI_voc_tid_t tid, transaction::Status status, auto& buck = _transactions[bucket]; auto it = buck._managed.find(tid); - if (it == buck._managed.end()) { + if (it == buck._managed.end() || !::authorized(it->second.user)) { return res.reset(TRI_ERROR_TRANSACTION_NOT_FOUND); } @@ -844,10 +869,12 @@ void Manager::toVelocyPack(VPackBuilder& builder, std::string const& database, // merge with local transactions iterateManagedTrx([&builder](TRI_voc_tid_t tid, ManagedTrx const& trx) { - builder.openObject(true); - builder.add("id", VPackValue(std::to_string(tid))); - builder.add("state", VPackValue(transaction::statusString(trx.state->status()))); - builder.close(); + if (::authorized(trx.user)) { + builder.openObject(true); + builder.add("id", VPackValue(std::to_string(tid))); + builder.add("state", VPackValue(transaction::statusString(trx.state->status()))); + builder.close(); + } }); } diff --git a/arangod/Transaction/Manager.h b/arangod/Transaction/Manager.h index 4feac9a46b..c47bc5853f 100644 --- a/arangod/Transaction/Manager.h +++ b/arangod/Transaction/Manager.h @@ -77,6 +77,7 @@ class Manager final { MetaType type; /// managed, AQL or tombstone double usedTimeSecs; /// last time used TransactionState* state; /// Transaction, may be nullptr + std::string user; /// user owning the transaction /// @brief final TRX state that is valid if this is a tombstone /// necessary to avoid getting error on a 'diamond' commit or accidantally /// repeated commit / abort messages diff --git a/lib/Rest/GeneralResponse.cpp b/lib/Rest/GeneralResponse.cpp index fdb0a4ab4f..0d1f0a934d 100644 --- a/lib/Rest/GeneralResponse.cpp +++ b/lib/Rest/GeneralResponse.cpp @@ -382,6 +382,7 @@ rest::ResponseCode GeneralResponse::responseCode(int code) { case TRI_ERROR_QUERY_FULLTEXT_INDEX_MISSING: case TRI_ERROR_QUERY_NOT_FOUND: case TRI_ERROR_USER_NOT_FOUND: + case TRI_ERROR_TRANSACTION_NOT_FOUND: case TRI_ERROR_TASK_NOT_FOUND: case TRI_ERROR_GRAPH_NOT_FOUND: case TRI_ERROR_GRAPH_VERTEX_COL_DOES_NOT_EXIST: diff --git a/tests/js/client/load-balancing/load-balancing-transactions-auth-cluster.js b/tests/js/client/load-balancing/load-balancing-transactions-auth-cluster.js index f18d3b5859..d02312b24f 100644 --- a/tests/js/client/load-balancing/load-balancing-transactions-auth-cluster.js +++ b/tests/js/client/load-balancing/load-balancing-transactions-auth-cluster.js @@ -274,6 +274,124 @@ function TransactionsSuite () { } }, + testCreateAndCommitDifferentUser: function() { + const obj = { collections: { read: cn } }; + + let url = "/_api/transaction"; + let result = sendRequest(users[0], 'POST', url + "/begin", obj, true); + + assertEqual(result.status, 201); + assertFalse(result.body.result.id === undefined); + + let trx1 = result.body.result; + + try { + // commit with different user + result = sendRequest(users[1], 'PUT', url + "/" + encodeURIComponent(trx1.id), {}, false); + // should fail with not found + assertEqual(result.status, 404); + + result = sendRequest(users[0], 'PUT', url + "/" + encodeURIComponent(trx1.id), {}, false); + assertEqual(trx1.id, result.body.result.id); + assertEqual("committed", result.body.result.status); + + result = sendRequest(users[0], 'GET', url, {}, true); + assertNotInList(result.body.transactions, trx1); + } finally { + sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + } + }, + + testCreateAndAbortDifferentUser: function() { + const obj = { collections: { read: cn } }; + + let url = "/_api/transaction"; + let result = sendRequest(users[0], 'POST', url + "/begin", obj, true); + + assertEqual(result.status, 201); + assertFalse(result.body.result.id === undefined); + + let trx1 = result.body.result; + + try { + // abort with different user + result = sendRequest(users[1], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + // should fail with not found + assertEqual(result.status, 404); + + result = sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + + assertEqual(result.status, 200); + assertEqual(trx1.id, result.body.result.id); + assertEqual("aborted", result.body.result.status); + + result = sendRequest(users[0], 'GET', url, {}, true); + assertNotInList(result.body.transactions, trx1); + } finally { + sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + } + }, + + testCreateAndGetDifferentUser: function() { + const obj = { collections: { read: cn } }; + + let url = "/_api/transaction"; + let result = sendRequest(users[0], 'POST', url + "/begin", obj, true); + + assertEqual(result.status, 201); + assertFalse(result.body.result.id === undefined); + + let trx1 = result.body.result; + + try { + // abort with different user + result = sendRequest(users[1], 'GET', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + // should fail with not found + assertEqual(result.status, 404); + + result = sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + + assertEqual(result.status, 200); + assertEqual(trx1.id, result.body.result.id); + assertEqual("aborted", result.body.result.status); + + result = sendRequest(users[0], 'GET', url, {}, true); + assertNotInList(result.body.transactions, trx1); + } finally { + sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + } + }, + + testCreateAndGetListDifferentUser: function() { + const obj = { collections: { read: cn } }; + + let url = "/_api/transaction"; + let result = sendRequest(users[0], 'POST', url + "/begin", obj, true); + + assertEqual(result.status, 201); + assertFalse(result.body.result.id === undefined); + + let trx1 = result.body.result; + + try { + // get list with different user + result = sendRequest(users[1], 'GET', url, {}, true); + // should not find it in list + assertNotInList(result.body.transactions, trx1); + + result = sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + + assertEqual(result.status, 200); + assertEqual(trx1.id, result.body.result.id); + assertEqual("aborted", result.body.result.status); + + result = sendRequest(users[0], 'GET', url, {}, true); + assertNotInList(result.body.transactions, trx1); + } finally { + sendRequest(users[0], 'DELETE', '/_api/transaction/' + encodeURIComponent(trx1.id), {}, true); + } + }, + }; }