Fix withdraw restrictions (#399)

* Added msg.sender check to withdraw method

* Move maturity check back to original line

* Add maturity check and adjust tests to expect appropriate senders

* Remove accidental import

contracts/core/accounts/facets/AccountsDepositWithdrawEndowments.sol

@@ -234,6 +234,10 @@ contract AccountsDepositWithdrawEndowments is
     AccountStorage.Endowment storage tempEndowment = state.Endowments[id];
     AccountStorage.EndowmentState storage tempEndowmentState = state.States[id];
 
+    // Check if maturity has been reached for the endowment (0 == no maturity date)
+    bool mature = (tempEndowment.maturityTime != 0 &&
+      block.timestamp >= tempEndowment.maturityTime);
+
     // If an Endowment is closed and a withdraw is executed for it, check that the sender is the closing
     // beneficiary on record and procees the withdraw if so ensure funds are indeed moving to that beneficiary
     if (tempEndowmentState.closingEndowment) {
@@ -255,6 +259,23 @@ contract AccountsDepositWithdrawEndowments is
         beneficiaryEndowId = tempEndowmentState.closingBeneficiary.data.endowId;
       }
     }
+    // If not closing,
+    else {
+      // only owner can call if not mature
+      if (!mature) {
+        require(msg.sender == tempEndowment.owner, "Unauthorized");
+      }
+      // otherwise the caller must be in the Allowlist
+      else {
+        require(
+          IterableMappingAddr.get(
+            state.Allowlists[id][LibAccounts.AllowlistType.MaturityAllowlist],
+            msg.sender
+          ),
+          "Unauthorized"
+        );
+      }
+    }
 
     // place an arbitrary cap on the qty of different tokens per withdraw to limit gas use
     require(tokens.length > 0, "No tokens provided");
@@ -273,10 +294,6 @@ contract AccountsDepositWithdrawEndowments is
       );
     }
 
-    // Check if maturity has been reached for the endowment (0 == no maturity date)
-    bool mature = (tempEndowment.maturityTime != 0 &&
-      block.timestamp >= tempEndowment.maturityTime);
-
     if (tempEndowment.endowType == LibAccounts.EndowmentType.Daf) {
       require(
         beneficiaryAddress == address(0),

test/core/accounts/AccountsDepositWithdrawEndowments.ts

@@ -1099,7 +1099,9 @@ describe("AccountsDepositWithdrawEndowments", function () {
           let finalAmountLeftover = amountLeftAfterApFees.sub(expectedFeeEndow);
 
           await expect(
-            facet.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
+            facet
+              .connect(endowOwner)
+              .withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
           )
             .to.emit(facet, "EndowmentWithdraw")
             .withArgs(
@@ -1145,7 +1147,9 @@ describe("AccountsDepositWithdrawEndowments", function () {
           await wait(state.setAllowlist(normalEndowId, 0, [beneficiaryAddress]));
 
           await expect(
-            facet.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
+            facet
+              .connect(endowOwner)
+              .withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
           )
             .to.emit(facet, "EndowmentWithdraw")
             .withArgs(
@@ -1323,7 +1327,7 @@ describe("AccountsDepositWithdrawEndowments", function () {
 
         await expect(
           facet.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
-        ).to.be.revertedWith("Beneficiary address is not listed in maturityAllowlist");
+        ).to.be.revertedWith("Unauthorized");
       });
 
       describe("LOCKED withdrawals", () => {
@@ -1337,7 +1341,13 @@ describe("AccountsDepositWithdrawEndowments", function () {
             maturityTime: currTime,
           };
           await wait(state.setEndowmentDetails(normalEndowId, matureEndowment));
-          await wait(state.setAllowlist(normalEndowId, 2, [beneficiaryAddress]));
+          const beneficiarySigner = new ethers.Wallet(
+            genWallet().privateKey, 
+            hre.ethers.provider
+          );
+          await accOwner.sendTransaction({value: ethers.utils.parseEther("1"), to: beneficiarySigner.address})
+
+          await wait(state.setAllowlist(normalEndowId, 2, [beneficiarySigner.address]));
 
           const acctType = VaultType.LOCKED;
           const beneficiaryId = 0;
@@ -1349,21 +1359,21 @@ describe("AccountsDepositWithdrawEndowments", function () {
           const finalAmountLeftover = amount.sub(expectedFeeAp);
 
           await expect(
-            facet.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
+            facet.connect(beneficiarySigner).withdraw(normalEndowId, acctType, beneficiarySigner.address, beneficiaryId, tokens)
           )
             .to.emit(facet, "EndowmentWithdraw")
             .withArgs(
               normalEndowId,
               tokens[0].addr,
               tokens[0].amnt,
               acctType,
-              beneficiaryAddress,
+              beneficiarySigner.address,
               beneficiaryId
             );
 
           expect(tokenFake.transfer).to.have.been.calledWith(treasury, expectedFeeAp);
           expect(tokenFake.transfer).to.have.been.calledWith(
-            beneficiaryAddress,
+            beneficiarySigner.address,
             finalAmountLeftover
           );