fix comments

XRPLF · Nov 13, 2024 · 6edf6bc · 6edf6bc
1 parent 7619ec5
commit 6edf6bc
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 23 deletions.
diff --git a/src/rpc/CredentialHelpers.cpp b/src/rpc/CredentialHelpers.cpp
@@ -79,14 +79,21 @@ parseAuthorizeCredentials(boost::json::array const& jv)
 {
     ripple::STArray arr;
     for (auto const& jo : jv) {
+        ASSERT(
+            jo.at(JS(issuer)).is_string(),
+            "issuer must be string, should already be checked in AuthorizeCredentialValidator"
+        );
         auto const issuer =
             ripple::parseBase58<ripple::AccountID>(static_cast<std::string>(jo.at(JS(issuer)).as_string()));
         ASSERT(
             issuer.has_value(), "issuer must be present, should already be checked in AuthorizeCredentialValidator."
         );
 
+        ASSERT(
+            jo.at(JS(credential_type)).is_string(),
+            "credential_type must be string, should already be checked in AuthorizeCredentialValidator"
+        );
         auto const credentialType = ripple::strUnHex(static_cast<std::string>(jo.at(JS(credential_type)).as_string()));
-
         ASSERT(
             credentialType.has_value(),
             "credential_type must be present, should already be checked in AuthorizeCredentialValidator."
@@ -104,6 +111,7 @@ parseAuthorizeCredentials(boost::json::array const& jv)
 std::expected<ripple::STArray, Status>
 fetchCredentialArray(
     std::optional<boost::json::array> const& credID,
+    ripple::AccountID const& srcAcc,
     BackendInterface const& backend,
     ripple::LedgerHeader const& info,
     boost::asio::yield_context const& yield
@@ -127,17 +135,20 @@ fetchCredentialArray(
         auto const credKeylet = ripple::keylet::credential(credHash).key;
         auto const credLedgerObject = backend.fetchLedgerObject(credKeylet, info.seq, yield);
         if (!credLedgerObject)
-            return Error{Status{RippledError::rpcBAD_CREDENTIALS, "credentials aren't accepted."}};
+            return Error{Status{RippledError::rpcBAD_CREDENTIALS, "credentials don't exist."}};
 
         auto credIt = ripple::SerialIter{credLedgerObject->data(), credLedgerObject->size()};
         auto const sleCred = ripple::SLE{credIt, credKeylet};
 
-        if (!credLedgerObject || (sleCred.getType() != ripple::ltCREDENTIAL) ||
+        if ((sleCred.getType() != ripple::ltCREDENTIAL) ||
             ((sleCred.getFieldU32(ripple::sfFlags) & ripple::lsfAccepted) == 0u))
-            return Error{Status{RippledError::rpcBAD_CREDENTIALS}};
+            return Error{Status{RippledError::rpcBAD_CREDENTIALS, "credentials aren't accepted"}};
 
         if (credentials::checkExpired(sleCred, info))
-            return Error{Status{RippledError::rpcBAD_CREDENTIALS}};
+            return Error{Status{RippledError::rpcBAD_CREDENTIALS, "credentials are expired"}};
+
+        if (sleCred.getAccountID(ripple::sfIssuer) != srcAcc)
+            return Error{Status{RippledError::rpcBAD_CREDENTIALS, "credentials doesn't belong to the root account"}};
 
         auto credential = ripple::STObject::makeInnerObject(ripple::sfCredential);
         credential.setAccountID(ripple::sfIssuer, sleCred.getAccountID(ripple::sfIssuer));

diff --git a/src/rpc/CredentialHelpers.hpp b/src/rpc/CredentialHelpers.hpp
@@ -71,6 +71,7 @@ parseAuthorizeCredentials(boost::json::array const& jv);
  * @brief Get Array of Credential objects
  *
  * @param credID Array of CredentialID's to parse
+ * @param srcAcc The Source Account
  * @param backend backend interface
  * @param info The ledger header
  * @param yield The coroutine context
@@ -79,6 +80,7 @@ parseAuthorizeCredentials(boost::json::array const& jv);
 std::expected<ripple::STArray, Status>
 fetchCredentialArray(
     std::optional<boost::json::array> const& credID,
+    ripple::AccountID const& srcAcc,
     BackendInterface const& backend,
     ripple::LedgerHeader const& info,
     boost::asio::yield_context const& yield

diff --git a/src/rpc/common/Validators.cpp b/src/rpc/common/Validators.cpp
@@ -290,7 +290,7 @@ CustomValidator CustomValidators::AuthorizeCredentialValidator =
 
         if (authCred.size() > ripple::maxCredentialsArraySize) {
             return Error{Status{
-                RippledError::rpcINVALID_PARAMS,
+                ClioError::rpcMALFORMED_AUTHORIZED_CREDENTIALS,
                 fmt::format(
                     "Max {} number of credentials in authorized_credentials array", ripple::maxCredentialsArraySize
                 )

diff --git a/src/rpc/common/Validators.hpp b/src/rpc/common/Validators.hpp
@@ -597,10 +597,7 @@ struct Hex256ItemType final {
         for (auto const& elem : res.as_array()) {
             ripple::uint256 num;
             if (!elem.is_string() || !num.parseHex(elem.as_string())) {
-                return Error{Status{
-                    RippledError::rpcINVALID_PARAMS,
-                    "Invalid field 'credentials', not an array of CredentialID(hash256)."
-                }};
+                return Error{Status{RippledError::rpcINVALID_PARAMS, "Item is not a valid uint256 type."}};
             }
         }
         return {};

diff --git a/src/rpc/handlers/DepositAuthorized.cpp b/src/rpc/handlers/DepositAuthorized.cpp
@@ -94,8 +94,9 @@ DepositAuthorizedHandler::process(DepositAuthorizedHandler::Input input, Context
         if (creds.value().size() > ripple::maxCredentialsArraySize) {
             return Error{Status{RippledError::rpcINVALID_PARAMS, "credential array too long."}};
         }
-        auto const credArray =
-            credentials::fetchCredentialArray(input.credentials, *sharedPtrBackend_, lgrInfo, ctx.yield);
+        auto const credArray = credentials::fetchCredentialArray(
+            input.credentials, *sourceAccountID, *sharedPtrBackend_, lgrInfo, ctx.yield
+        );
         if (!credArray.has_value())
             return Error{std::move(credArray).error()};
         authCreds = std::move(credArray).value();

diff --git a/tests/common/util/TestObject.cpp b/tests/common/util/TestObject.cpp
@@ -1216,7 +1216,7 @@ CreateOracleObject(
     return ledgerObject;
 }
 
-// acc2 issue credential for acc1 so acc2 is issuer
+// acc1 issue credential for acc2 so acc1 is issuer
 ripple::STObject
 CreateCredentialObject(
     std::string_view acc1,
@@ -1229,8 +1229,8 @@ CreateCredentialObject(
     ripple::STObject credObj(ripple::sfCredential);
     credObj.setFieldU16(ripple::sfLedgerEntryType, ripple::ltCREDENTIAL);
     credObj.setFieldVL(ripple::sfCredentialType, ripple::Blob{credType.begin(), credType.end()});
-    credObj.setAccountID(ripple::sfSubject, GetAccountIDWithString(acc1));
-    credObj.setAccountID(ripple::sfIssuer, GetAccountIDWithString(acc2));
+    credObj.setAccountID(ripple::sfSubject, GetAccountIDWithString(acc2));
+    credObj.setAccountID(ripple::sfIssuer, GetAccountIDWithString(acc1));
     if (expiration.has_value())
         credObj.setFieldU32(ripple::sfExpiration, expiration.value());
 

diff --git a/tests/unit/rpc/handlers/CredentialHelpersTests.cpp b/tests/unit/rpc/handlers/CredentialHelpersTests.cpp
@@ -125,11 +125,12 @@ TEST_F(CredentialHelperTest, GetInvalidCredentialArray)
     auto const info = CreateLedgerHeader(INDEX1, 30);
 
     boost::asio::spawn(ctx, [&](boost::asio::yield_context yield) {
-        auto const ret = credentials::fetchCredentialArray(credentialsArray, *backend, info, yield);
+        auto const ret =
+            credentials::fetchCredentialArray(credentialsArray, GetAccountIDWithString(ACCOUNT), *backend, info, yield);
         ASSERT_FALSE(ret.has_value());
         auto const status = ret.error();
         EXPECT_EQ(status, RippledError::rpcBAD_CREDENTIALS);
-        EXPECT_EQ(status.message, "credentials aren't accepted.");
+        EXPECT_EQ(status.message, "credentials don't exist.");
     });
     ctx.run();
 }
@@ -148,12 +149,14 @@ TEST_F(CredentialHelperTest, GetValidCredentialArray)
 
     ripple::STArray expectedAuthCreds;
     ripple::STObject credential(ripple::sfCredential);
-    credential.setAccountID(ripple::sfIssuer, GetAccountIDWithString(ACCOUNT));
+    credential.setAccountID(ripple::sfIssuer, GetAccountIDWithString(ACCOUNT2));
     credential.setFieldVL(ripple::sfCredentialType, ripple::Blob{std::begin(CREDENTIALTYPE), std::end(CREDENTIALTYPE)});
     expectedAuthCreds.push_back(std::move(credential));
 
     boost::asio::spawn(ctx, [&](boost::asio::yield_context yield) {
-        auto const result = credentials::fetchCredentialArray(credentialsArray, *backend, ledgerHeader, yield);
+        auto const result = credentials::fetchCredentialArray(
+            credentialsArray, GetAccountIDWithString(ACCOUNT2), *backend, ledgerHeader, yield
+        );
         ASSERT_TRUE(result.has_value());
         EXPECT_EQ(result.value(), expectedAuthCreds);
     });

diff --git a/tests/unit/rpc/handlers/DepositAuthorizedTests.cpp b/tests/unit/rpc/handlers/DepositAuthorizedTests.cpp
@@ -185,7 +185,7 @@ generateTestValuesForParametersTest()
                 "credentials": [123]
             })",
             "invalidParams",
-            "Invalid field 'credentials', not an array of CredentialID(hash256).",
+            "Item is not a valid uint256 type.",
         },
         {
             "CredentialsNotHexedStringInArray",
@@ -196,7 +196,7 @@ generateTestValuesForParametersTest()
                 "credentials": ["234", "432"]
             })",
             "invalidParams",
-            "Invalid field 'credentials', not an array of CredentialID(hash256).",
+            "Item is not a valid uint256 type.",
         }
     };
 }
@@ -684,7 +684,7 @@ TEST_F(RPCDepositAuthorizedTest, CredentialNotAuthorizedReturnsFalse)
         ASSERT_FALSE(output);
         auto const err = rpc::makeError(output.result.error());
         EXPECT_EQ(err.at("error").as_string(), "badCredentials");
-        EXPECT_EQ(err.at("error_message").as_string(), "Credentials do not exist, are not accepted, or have expired.");
+        EXPECT_EQ(err.at("error_message").as_string(), "credentials aren't accepted");
     });
 }
 
@@ -742,7 +742,7 @@ TEST_F(RPCDepositAuthorizedTest, CredentialExpiredReturnsFalse)
         ASSERT_FALSE(output);
         auto const err = rpc::makeError(output.result.error());
         EXPECT_EQ(err.at("error").as_string(), "badCredentials");
-        EXPECT_EQ(err.at("error_message").as_string(), "Credentials do not exist, are not accepted, or have expired.");
+        EXPECT_EQ(err.at("error_message").as_string(), "credentials are expired");
     });
 }
 
@@ -897,3 +897,54 @@ TEST_F(RPCDepositAuthorizedTest, MoreThanMaxNumberOfCredentialsReturnsFalse)
         EXPECT_EQ(err.at("error_message").as_string(), "credential array too long.");
     });
 }
+
+TEST_F(RPCDepositAuthorizedTest, DifferenIssuerAccountForCredentialReturnsFalse)
+{
+    backend->setRange(10, 30);
+
+    auto ledgerHeader = CreateLedgerHeader(LEDGERHASH, 30);
+
+    EXPECT_CALL(*backend, fetchLedgerByHash(ripple::uint256{LEDGERHASH}, _)).WillOnce(Return(ledgerHeader));
+
+    auto const account1Root = CreateAccountRootObject(ACCOUNT, 0, 2, 200, 2, INDEX1, 2);
+    auto const account2Root = CreateAccountRootObject(ACCOUNT2, ripple::lsfDepositAuth, 2, 200, 2, INDEX2, 2);
+    auto const credential = CreateCredentialObject(ACCOUNT, ACCOUNT, CREDENTIALTYPE);
+    auto const credentialIndex = ripple::keylet::credential(
+                                     GetAccountIDWithString(ACCOUNT),
+                                     GetAccountIDWithString(ACCOUNT2),
+                                     ripple::Slice(CREDENTIALTYPE.data(), CREDENTIALTYPE.size())
+    )
+                                     .key;
+
+    ON_CALL(*backend, doFetchLedgerObject(_, _, _)).WillByDefault(Return(std::optional<Blob>{{1, 2, 3}}));
+    ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(GetAccountIDWithString(ACCOUNT)).key, _, _))
+        .WillByDefault(Return(account1Root.getSerializer().peekData()));
+    ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(GetAccountIDWithString(ACCOUNT2)).key, _, _))
+        .WillByDefault(Return(account2Root.getSerializer().peekData()));
+    ON_CALL(*backend, doFetchLedgerObject(credentialIndex, _, _))
+        .WillByDefault(Return(credential.getSerializer().peekData()));
+    EXPECT_CALL(*backend, doFetchLedgerObject).Times(3);
+
+    auto const input = json::parse(fmt::format(
+        R"({{
+            "source_account": "{}",
+            "destination_account": "{}",
+            "ledger_hash": "{}",
+            "credentials": ["{}"]
+        }})",
+        ACCOUNT2,
+        ACCOUNT2,
+        LEDGERHASH,
+        ripple::strHex(credentialIndex)
+    ));
+
+    runSpawn([&, this](auto yield) {
+        auto const handler = AnyHandler{DepositAuthorizedHandler{backend}};
+        auto const output = handler.process(input, Context{yield});
+
+        ASSERT_FALSE(output);
+        auto const err = rpc::makeError(output.result.error());
+        EXPECT_EQ(err.at("error").as_string(), "badCredentials");
+        EXPECT_EQ(err.at("error_message").as_string(), "credentials doesn't belong to the root account");
+    });
+}