Skip to content

Commit

Permalink
fix: add validation to enforce instance name length limit (#162)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
prbn021 authored Jan 31, 2025
1 parent 59c00c1 commit 445bc79
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 0 deletions.
28 changes: 28 additions & 0 deletions pkg/web/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()

Expand Down
39 changes: 39 additions & 0 deletions pkg/web/certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 445bc79

Please sign in to comment.