fix: Check broker owner authorization only when a loan pays it

LoanSet required the loan broker's owner to be able to hold the vault asset
on every loan. Without an origination fee the owner's leg of the payout is
zero and moves nothing, yet the loan was still rejected with tecNO_AUTH when
the owner's MPT authorization had been revoked, or tecNO_LINE when its trust
line had been closed.

Behind fixCleanup3_5_0, check the broker owner's authorization only when the
origination fee is nonzero. Service and management fees are unaffected:
LoanPay already reroutes them to the broker pseudo-account when the owner
cannot receive them.

src/libxrpl/tx/transactors/lending/LoanSet.cpp

@@ -638,8 +638,15 @@ LoanSet::doApply()
         }
     }
 
-    if (auto const ter = requireAuth(view, vaultAsset, brokerOwner, AuthType::StrongAuth))
-        return ter;
+    // Without an origination fee the broker owner's leg of the send below is
+    // zero and does nothing, so the owner does not have to be able to hold the
+    // asset.
+    // Pre-fixCleanup3_5_0: the check ran on every loan.
+    if (originationFee != beast::kZero || !view.rules().enabled(fixCleanup3_5_0))
+    {
+        if (auto const ter = requireAuth(view, vaultAsset, brokerOwner, AuthType::StrongAuth))
+            return ter;
+    }
 
     if (auto const ter = accountSendMulti(
             view,

src/test/app/lending/LoanCoverFreezeAuth_test.cpp

@@ -866,14 +866,26 @@ private:
         env(credentials::deleteCred(broker, broker, issuer, credType));
         env.close();
 
-        // Create a loan, this should fail for tecNO_AUTH
+        // The loan has no origination fee, so nothing reaches the broker owner
+        // here; the service fee is paid later and LoanPay reroutes it to the
+        // broker pseudo-account.
+        // Pre-fixCleanup3_5_0: the owner's authorization was checked anyway.
+        bool const fix350Enabled = features[fixCleanup3_5_0];
+        auto const sleBroker = env.le(keylet::loanBroker(brokerInfo.brokerID));
+        if (!BEAST_EXPECT(sleBroker))
+            return;
+        auto const loanKeylet =
+            keylet::loan(brokerInfo.brokerID, SeqProxy::rawSequence(sleBroker->at(sfLoanSequence)));
+
         env(set(borrower, brokerInfo.brokerID, 10'000),
             Sig(sfCounterpartySignature, broker),
             kLoanServiceFee(mpt(100).value()),
             kPaymentInterval(100),
             Fee(XRP(100)),
-            Ter(tecNO_AUTH));
+            Ter(fix350Enabled ? TER{tesSUCCESS} : TER{tecNO_AUTH}));
         env.close();
+
+        BEAST_EXPECT(static_cast<bool>(env.le(loanKeylet)) == fix350Enabled);
     }
 
     void

src/test/app/lending/LoanSet_test.cpp

@@ -427,12 +427,13 @@ private:
                     Ter{tecNO_AUTH});
                 env.close();
 
-                // Cannot create loan, even without an origination fee
+                // Post-fixCleanup3_5_0 a loan without an origination fee pays
+                // the lender nothing, so its authorization is not checked
                 env(set(borrower, broker.brokerID, principalRequest),
                     kCounterparty(lender),
                     Sig(sfCounterpartySignature, lender),
                     Fee(env.current()->fees().base * 5),
-                    Ter{tecNO_AUTH});
+                    Ter{features[fixCleanup3_5_0] ? TER{tesSUCCESS} : TER{tecNO_AUTH}});
                 env.close();
 
                 // No MPToken for lender - no authorization and no payment
@@ -697,6 +698,129 @@ private:
         }
     }
 
+    // The broker owner is paid only when the loan carries an origination fee, so
+    // only then does it have to be able to hold the vault asset.
+    void
+    testLoanSetZeroFeeUnauthorizedOwner(FeatureBitset features)
+    {
+        using namespace jtx;
+        using namespace loan;
+
+        bool const fix350Enabled = features[fixCleanup3_5_0];
+        testcase << "LoanSet: broker owner cannot hold the vault asset"
+                 << (fix350Enabled ? "" : " pre-fixCleanup3_5_0");
+
+        Account const issuer{"issuer"};
+        Account const lender{"lender"};
+        Account const borrower{"borrower"};
+
+        // MPT. The issuer revokes the broker owner's authorization after the
+        // vault is funded. An origination fee still has to reach the owner, so
+        // only the zero-fee loan changes.
+        {
+            Env env(*this, features);
+            env.fund(XRP(1'000'000), issuer, lender, borrower);
+            env.close();
+
+            MPTTester mptt{env, issuer, kMptInitNoFund};
+            mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTRequireAuth});
+            env.close();
+            PrettyAsset const asset = mptt.issuanceID();
+            mptt.authorize({.account = lender});
+            mptt.authorize({.account = borrower});
+            env.close();
+            mptt.authorize({.account = issuer, .holder = lender});
+            mptt.authorize({.account = issuer, .holder = borrower});
+            env.close();
+
+            env(pay(issuer, lender, asset(10'000'000)));
+            env.close();
+
+            auto const broker = createVaultAndBroker(env, asset, lender);
+
+            // Pay out and delete the broker owner's MPToken.
+            auto const lenderMPToken = keylet::mptoken(mptt.issuanceID(), lender);
+            auto const sleLender = env.le(lenderMPToken);
+            if (!BEAST_EXPECT(sleLender))
+                return;
+            env(pay(lender, issuer, asset(sleLender->at(sfMPTAmount))));
+            env.close();
+            mptt.authorize({.account = lender, .flags = tfMPTUnauthorize});
+            env.close();
+            BEAST_EXPECT(!env.le(lenderMPToken));
+
+            env(set(borrower, broker.brokerID, asset(1'000).value()),
+                kLoanOriginationFee(asset(1).value()),
+                kCounterparty(lender),
+                Sig(sfCounterpartySignature, lender),
+                Fee(env.current()->fees().base * 5),
+                Ter{tecNO_AUTH});
+            env.close();
+
+            auto const sleBrokerBefore = env.le(keylet::loanBroker(broker.brokerID));
+            if (!BEAST_EXPECT(sleBrokerBefore))
+                return;
+            auto const loanKeylet = keylet::loan(
+                broker.brokerID, SeqProxy::rawSequence(sleBrokerBefore->at(sfLoanSequence)));
+
+            env(set(borrower, broker.brokerID, asset(1'000).value()),
+                kCounterparty(lender),
+                Sig(sfCounterpartySignature, lender),
+                Fee(env.current()->fees().base * 5),
+                Ter{fix350Enabled ? TER{tesSUCCESS} : TER{tecNO_AUTH}});
+            env.close();
+
+            BEAST_EXPECT(static_cast<bool>(env.le(loanKeylet)) == fix350Enabled);
+            // The broker owner was paid nothing, so it gained no holding.
+            BEAST_EXPECT(!env.le(lenderMPToken));
+        }
+
+        // IOU. The broker owner closes its trust line. A loan with a fee
+        // re-creates the line through addEmptyHolding, so the zero-fee loan is
+        // the only one the authorization check can block.
+        {
+            Env env(*this, features);
+            env.fund(XRP(1'000'000), issuer, lender, borrower);
+            env(fset(issuer, asfDefaultRipple));
+            env.close();
+
+            PrettyAsset const asset = issuer[iouCurrency_];
+            env(trust(lender, asset(10'000'000)));
+            env(trust(borrower, asset(10'000'000)));
+            env.close();
+            env(pay(issuer, lender, asset(10'000'000)));
+            env.close();
+
+            auto const broker = createVaultAndBroker(env, asset, lender);
+
+            // Pay out and close the broker owner's trust line.
+            auto const lenderLine = keylet::trustLine(lender, asset.raw().get<Issue>());
+            if (!BEAST_EXPECT(env.le(lenderLine)))
+                return;
+            env(pay(lender, issuer, asset(env.balance(lender, asset.raw()).number())));
+            env.close();
+            env(trust(lender, asset(0)));
+            env.close();
+            BEAST_EXPECT(!env.le(lenderLine));
+
+            auto const sleBrokerBefore = env.le(keylet::loanBroker(broker.brokerID));
+            if (!BEAST_EXPECT(sleBrokerBefore))
+                return;
+            auto const loanKeylet = keylet::loan(
+                broker.brokerID, SeqProxy::rawSequence(sleBrokerBefore->at(sfLoanSequence)));
+
+            env(set(borrower, broker.brokerID, asset(1'000).value()),
+                kCounterparty(lender),
+                Sig(sfCounterpartySignature, lender),
+                Fee(env.current()->fees().base * 5),
+                Ter{fix350Enabled ? TER{tesSUCCESS} : TER{tecNO_LINE}});
+            env.close();
+
+            BEAST_EXPECT(static_cast<bool>(env.le(loanKeylet)) == fix350Enabled);
+            BEAST_EXPECT(!env.le(lenderLine));
+        }
+    }
+
     // LoanSet in a closed-ended vault — phase gating and maturity bound.
     void
     testLoanSetClosedEnded()
@@ -940,6 +1064,8 @@ public:
         testLoanSetExistingLineAfterIssuerClearsDefaultRipple();
         testLoanSetOriginationFeeTwoMptCreates(all_);
         testLoanSetOriginationFeeTwoMptCreates(all_ - fixCleanup3_4_0);
+        testLoanSetZeroFeeUnauthorizedOwner(all_);
+        testLoanSetZeroFeeUnauthorizedOwner(all_ - fixCleanup3_5_0);
     }
 };