From 445bc79e72a3985ea9353b547c76f079eea80924 Mon Sep 17 00:00:00 2001
From: Paulo Reis <110939784+prbn021@users.noreply.github.com>
Date: Fri, 31 Jan 2025 15:45:50 -0300
Subject: [PATCH] fix: add validation to enforce instance name length limit
 (#162)

* fix: add validation to enforce instance name length limit

* refactor: extract certificate validation logic into separate function

* fix: remove unnecessary suffix length adjustment and refactor tests

* refactor: remove unnecessary Internal field assignment
---
 pkg/web/certificate.go      | 28 ++++++++++++++++++++++++++
 pkg/web/certificate_test.go | 39 +++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/pkg/web/certificate.go b/pkg/web/certificate.go
index ea39805d..919e3717 100644
--- a/pkg/web/certificate.go
+++ b/pkg/web/certificate.go
@@ -103,6 +103,11 @@ func updateCertManagerRequest(c echo.Context) error {
 		return err
 	}
 
+	err = validateCertManagerRequest(in)
+	if err != nil {
+		return err
+	}
+
 	err = manager.UpdateCertManagerRequest(ctx, c.Param("instance"), in)
 	if err != nil {
 		return err
@@ -111,6 +116,29 @@ func updateCertManagerRequest(c echo.Context) error {
 	return c.NoContent(http.StatusOK)
 }
 
+func validateCertManagerRequest(in types.CertManager) error {
+	const maxCommonNameLength = 64
+
+	if len(in.Name) > maxCommonNameLength {
+		return &rpaas.ValidationError{
+			Msg: fmt.Sprintf("The certificate name '%s' exceeds the limit of %d characters.", in.Name, maxCommonNameLength),
+		}
+	}
+
+	for _, dns := range in.DNSNames {
+		if len(dns) > maxCommonNameLength {
+			return &rpaas.ValidationError{
+				Msg: fmt.Sprintf("The DNS name '%s' exceeds the limit of %d characters.", dns, maxCommonNameLength),
+			}
+		}
+		if dns == "" {
+			return &rpaas.ValidationError{Msg: "DNS names cannot contain empty values."}
+		}
+	}
+
+	return nil
+}
+
 func deleteCertManagerRequest(c echo.Context) error {
 	ctx := c.Request().Context()
 
diff --git a/pkg/web/certificate_test.go b/pkg/web/certificate_test.go
index 8665032d..b74630ec 100644
--- a/pkg/web/certificate_test.go
+++ b/pkg/web/certificate_test.go
@@ -289,6 +289,45 @@ func Test_UpdateCertManagerRequest(t *testing.T) {
 		expectedCode int
 		expectedBody string
 	}{
+		"doing a request with exactly 64 characters": {
+			requestBody: `{"name": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "issuer": "my-issuer", "dnsNames": ["foo.example.com"]}`,
+			manager: &fake.RpaasManager{
+				FakeUpdateCertManagerRequest: func(instanceName string, in clientTypes.CertManager) error {
+					assert.Equal(t, "my-instance", instanceName)
+					assert.Equal(t, clientTypes.CertManager{
+						Name:     "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+						Issuer:   "my-issuer",
+						DNSNames: []string{"foo.example.com"},
+					}, in)
+					return nil
+				},
+			},
+			expectedCode: http.StatusOK,
+		},
+		"doing a request with invalid name": {
+			requestBody: `{"name": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "issuer": "my-issuer", "dnsNames": ["foo.example.com"]}`,
+			manager: &fake.RpaasManager{
+				FakeUpdateCertManagerRequest: func(instanceName string, in clientTypes.CertManager) error {
+					return &rpaas.ValidationError{
+						Msg: fmt.Sprintf("The certificate name '%s' exceeds the limit of 64 characters.", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+					}
+				},
+			},
+			expectedCode: http.StatusBadRequest, // It should fail because it has 65 characters
+			expectedBody: `{"message":"The certificate name 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' exceeds the limit of 64 characters."}`,
+		},
+		"doing a request with invalid dnsNames": {
+			requestBody: `{"name": "validCert", "issuer": "my-issuer", "dnsNames": ["foo.example.com", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"], "ipAddresses": ["169.196.100.1"]}`,
+			manager: &fake.RpaasManager{
+				FakeUpdateCertManagerRequest: func(instanceName string, in clientTypes.CertManager) error {
+					return &rpaas.ValidationError{
+						Msg: fmt.Sprintf("The DNS name '%s' exceeds the limit of 64 characters.", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+					}
+				},
+			},
+			expectedCode: http.StatusBadRequest, // It should fail because the second "dnsNames" has 65 characters
+			expectedBody: `{"message":"The DNS name 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' exceeds the limit of 64 characters."}`,
+		},
 		"doing a correct request": {
 			requestBody: `{"issuer": "my-issuer", "dnsNames": ["foo.example.com", "bar.example.com"], "ipAddresses": ["169.196.100.1"]}`,
 			manager: &fake.RpaasManager{