Add user restrictions for streaming transactions. (#9796)

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 <velocypack/Iterator.h>
 #include <velocypack/velocypack-aliases.h>
 
+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<transaction::Context> 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();
+    }
   });
 }
 

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

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:

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);
+      }
+    },
+
   };
 }