fix: check parent for transaction

Author: morremeyer

internal/controllers/helper.go

@@ -165,6 +165,34 @@ func getEnvelope(c *gin.Context) (models.Envelope, error) {
 	return envelope, nil
 }
 
+// getTransaction verifies that the request URI is valid for the transaction and returns it.
+func getTransaction(c *gin.Context) (models.Transaction, error) {
+	var transaction models.Transaction
+
+	budget, err := getBudget(c)
+	if err != nil {
+		return models.Transaction{}, err
+	}
+
+	accountID, err := parseID(c, "transactionId")
+	if err != nil {
+		return models.Transaction{}, err
+	}
+
+	err = models.DB.First(&transaction, &models.Transaction{
+		BudgetID: budget.ID,
+		Model: models.Model{
+			ID: accountID,
+		},
+	}).Error
+	if err != nil {
+		FetchErrorHandler(c, err)
+		return models.Transaction{}, err
+	}
+
+	return transaction, nil
+}
+
 // FetchErrorHandler handles errors for fetching data from the database.
 func FetchErrorHandler(c *gin.Context, err error) {
 	if errors.Is(err, gorm.ErrRecordNotFound) {

internal/controllers/transaction.go

@@ -72,10 +72,8 @@ func GetTransactions(c *gin.Context) {
 
 // GetTransaction retrieves an transaction by its ID.
 func GetTransaction(c *gin.Context) {
-	var transaction models.Transaction
-	err := models.DB.First(&transaction, c.Param("transactionId")).Error
+	transaction, err := getTransaction(c)
 	if err != nil {
-		FetchErrorHandler(c, err)
 		return
 	}
 

internal/controllers/transaction_test.go

@@ -81,6 +81,22 @@ func TestNoTransactionNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestTransactionInvalidIDs verifies that on non-number requests for transaction IDs,
+// the API returs a Bad Request status code.
+func TestTransactionInvalidIDs(t *testing.T) {
+	r := test.Request(t, "GET", "/v1/budgets/1/transactions/-56", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/1/transactions/notANumber", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/-61/transactions/56", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/RandomStringThatIsNotAUint64/transactions/1", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+}
+
 // TestNonexistingBudgetTransactions404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
 //
 // It verifies that for a non-existing budget, the accounts endpoint raises a 404
@@ -90,6 +106,25 @@ func TestNonexistingBudgetTransactions404(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestTransactionParentChecked is a regression test for https://github.com/envelope-zero/backend/issues/90.
+//
+// It verifies that the transaction details endpoint for a budget only returns transactions that belong to the
+// budget.
+func TestTransactionParentChecked(t *testing.T) {
+	r := test.Request(t, "POST", "/v1/budgets", `{ "name": "New Budget", "note": "More tests something something" }`)
+	test.AssertHTTPStatus(t, http.StatusCreated, &r)
+
+	var budget BudgetDetailResponse
+	test.DecodeResponse(t, &r, &budget)
+
+	path := fmt.Sprintf("/v1/budgets/%v", budget.Data.ID)
+	r = test.Request(t, "GET", path+"/transactions/1", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &r)
+
+	r = test.Request(t, "DELETE", path, "")
+	test.AssertHTTPStatus(t, http.StatusNoContent, &r)
+}
+
 func TestCreateTransaction(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/transactions", `{ "note": "More tests something something", "amount": 1253.17 }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)