fix: check parent for account

Author: morremeyer

internal/controllers/account.go

@@ -96,10 +96,8 @@ func GetAccounts(c *gin.Context) {
 
 // GetAccount retrieves an account by its ID.
 func GetAccount(c *gin.Context) {
-	var account models.Account
-	err := models.DB.First(&account, c.Param("accountId")).Error
+	account, err := getAccount(c)
 	if err != nil {
-		FetchErrorHandler(c, err)
 		return
 	}
 

internal/controllers/account_test.go

@@ -82,6 +82,22 @@ func TestNoAccountNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestAccountInvalidIDs verifies that on non-number requests for account IDs,
+// the API returs a Bad Request status code.
+func TestAccountInvalidIDs(t *testing.T) {
+	r := test.Request(t, "GET", "/v1/budgets/1/accounts/-56", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/1/accounts/notANumber", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/-61/accounts/56", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/RandomStringThatIsNotAUint64/accounts/1", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+}
+
 // TestNonexistingBudgetAccounts404 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
@@ -91,6 +107,25 @@ func TestNonexistingBudgetAccounts404(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestAccountParentChecked is a regression test for https://github.com/envelope-zero/backend/issues/90.
+//
+// It verifies that the account details endpoint for a budget only returns accounts that belong to the
+// budget.
+func TestAccountParentChecked(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+"/accounts/1", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &r)
+
+	r = test.Request(t, "DELETE", path, "")
+	test.AssertHTTPStatus(t, http.StatusNoContent, &r)
+}
+
 func TestCreateAccount(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/accounts", `{ "name": "New Account", "note": "More tests something something" }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)

internal/controllers/budget_test.go

@@ -48,6 +48,16 @@ func TestNoBudgetNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestBudgetInvalidIDs verifies that on non-number requests for budget IDs,
+// the API returs a Bad Request status code.
+func TestBudgetInvalidIDs(t *testing.T) {
+	r := test.Request(t, "GET", "/v1/budgets/-17", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/DefinitelyNotAUint64", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+}
+
 func TestCreateBudget(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets", `{ "name": "New Budget", "note": "More tests something something" }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)

internal/controllers/category_test.go

@@ -49,6 +49,16 @@ func TestNoCategoryNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestCategoryInvalidIDs verifies that on non-number requests for category IDs,
+// the API returs a Bad Request status code.
+func TestCategoryInvalidIDs(t *testing.T) {
+	r := test.Request(t, "GET", "/v1/budgets/1/categories/-557", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/1/categories/HanShotFirst", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+}
+
 // TestNonexistingBudgetCategories404 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

internal/controllers/envelope_test.go

@@ -49,6 +49,16 @@ func TestNoEnvelopeNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestEnvelopeInvalidIDs verifies that on non-number requests for envelope IDs,
+// the API returs a Bad Request status code.
+func TestEnvelopeInvalidIDs(t *testing.T) {
+	r := test.Request(t, "GET", "/v1/budgets/1/categories/1/envelopes/-1985", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+
+	r = test.Request(t, "GET", "/v1/budgets/1/categories/1/envelopes/OhNoOurTable", "")
+	test.AssertHTTPStatus(t, http.StatusBadRequest, &r)
+}
+
 // TestNonexistingCategoryEnvelopes404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
 //
 // It verifies that for a non-existing category, the envelopes endpoint raises a 404

internal/controllers/helper.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"reflect"
 	"strconv"
 
 	"github.com/envelope-zero/backend/internal/models"
@@ -43,14 +44,29 @@ func requestURL(c *gin.Context) string {
 	return scheme + "://" + c.Request.Host + forwardedPrefix + c.Request.URL.Path
 }
 
+func parseID(c *gin.Context, param string) (uint64, error) {
+	var parsed uint64
+
+	parsed, err := strconv.ParseUint(c.Param(param), 10, 64)
+	if err != nil {
+		FetchErrorHandler(c, err)
+		return 0, err
+	}
+
+	return parsed, nil
+}
+
 // getBudget verifies that the budget from the URL parameters exists and returns it.
 func getBudget(c *gin.Context) (models.Budget, error) {
 	var budget models.Budget
 
-	budgetID, _ := strconv.ParseUint(c.Param("budgetId"), 10, 64)
+	budgetID, err := parseID(c, "budgetId")
+	if err != nil {
+		return models.Budget{}, err
+	}
 
 	// Check that the budget exists. If not, return a 404
-	err := models.DB.Where(&models.Budget{
+	err = models.DB.Where(&models.Budget{
 		Model: models.Model{
 			ID: budgetID,
 		},
@@ -63,15 +79,46 @@ func getBudget(c *gin.Context) (models.Budget, error) {
 	return budget, nil
 }
 
+// getAccount verifies that the request URI is valid for the account and returns it.
+func getAccount(c *gin.Context) (models.Account, error) {
+	var account models.Account
+
+	budget, err := getBudget(c)
+	if err != nil {
+		return models.Account{}, err
+	}
+
+	accountID, err := parseID(c, "accountId")
+	if err != nil {
+		return models.Account{}, err
+	}
+
+	err = models.DB.First(&account, &models.Account{
+		BudgetID: budget.ID,
+		Model: models.Model{
+			ID: accountID,
+		},
+	}).Error
+	if err != nil {
+		FetchErrorHandler(c, err)
+		return models.Account{}, err
+	}
+
+	return account, nil
+}
+
 // getCategory verifies that the category from the URL parameters exists and returns it
 //
 // It also verifies that the budget that is referred to exists.
 func getCategory(c *gin.Context) (models.Category, error) {
 	var category models.Category
 
-	categoryID, _ := strconv.ParseUint(c.Param("categoryId"), 10, 64)
+	categoryID, err := parseID(c, "categoryId")
+	if err != nil {
+		return models.Category{}, err
+	}
 
-	_, err := getBudget(c)
+	_, err = getBudget(c)
 	if err != nil {
 		return models.Category{}, err
 	}
@@ -95,9 +142,12 @@ func getCategory(c *gin.Context) (models.Category, error) {
 func getEnvelope(c *gin.Context) (models.Envelope, error) {
 	var envelope models.Envelope
 
-	envelopeID, _ := strconv.ParseUint(c.Param("envelopeId"), 10, 64)
+	envelopeID, err := parseID(c, "envelopeId")
+	if err != nil {
+		return models.Envelope{}, err
+	}
 
-	_, err := getCategory(c)
+	_, err = getCategory(c)
 	if err != nil {
 		return models.Envelope{}, err
 	}
@@ -119,6 +169,10 @@ func getEnvelope(c *gin.Context) (models.Envelope, error) {
 func FetchErrorHandler(c *gin.Context, err error) {
 	if errors.Is(err, gorm.ErrRecordNotFound) {
 		c.AbortWithStatus(http.StatusNotFound)
+	} else if reflect.TypeOf(err) == reflect.TypeOf(&strconv.NumError{}) {
+		c.JSON(http.StatusBadRequest, gin.H{
+			"error": "An ID specified in the query string was not a valid uint64",
+		})
 	} else {
 		log.Error().Str("request-id", requestid.Get(c)).Msgf("%T: %v", err, err.Error())
 		c.JSON(http.StatusInternalServerError, gin.H{