Skip to content

Commit

Permalink
Simplify the data template validation
Browse files Browse the repository at this point in the history
Signed-off-by: Radoslav Dimitrov <[email protected]>
  • Loading branch information
rdimitrov committed Oct 28, 2024
1 parent e986be1 commit 1505818
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 55 deletions.
29 changes: 14 additions & 15 deletions internal/email/email.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"encoding/json"
"fmt"
"reflect"
"regexp"
"strings"
"text/template"

"github.com/ThreeDotsLabs/watermill/message"
"github.com/google/uuid"
Expand All @@ -31,15 +31,6 @@ const (
EmailBodyMaxLength = 10000
)

var (
// htmlTagRegex is a regex to match HTML tags
htmlTagRegex = regexp.MustCompile(`<\/?[a-z][\s\S]*?>`)
// htmlEntityRegex is a regex to match HTML entities
htmlEntityRegex = regexp.MustCompile(`&[a-zA-Z0-9#]+;`)
// htmlCommentRegex is a regex to match HTML comments
htmlCommentRegex = regexp.MustCompile(`<!--[\s\S]*?-->`)
)

// MailEventPayload is the event payload for sending an invitation email
type MailEventPayload struct {
Address string `json:"email"`
Expand Down Expand Up @@ -150,16 +141,24 @@ func getEmailSubject(project string) string {
return fmt.Sprintf("You have been invited to join the %s organization in Minder", project)
}

// isValidField checks if a string contains HTML tags, entities, or comments.
// isValidField checks if the string is empty or contains HTML or JavaScript injection
func isValidField(str string) error {
if str == "" {
return fmt.Errorf("string is empty")
}

// Check for HTML tags, entities, or comments
if htmlTagRegex.MatchString(str) || htmlEntityRegex.MatchString(str) || htmlCommentRegex.MatchString(str) {
return fmt.Errorf("string %s contains HTML tags, entities, or comments", str)
// Check for HTML content
escapedHTML := template.HTMLEscapeString(str)
if escapedHTML != str {
return fmt.Errorf("string %s contains HTML injection", str)
}

// Check for JavaScript content separately
escapedJS := template.JSEscaper(str)
if escapedJS != str {
return fmt.Errorf("string %s contains JavaScript injection", str)
}

return nil
}

Expand All @@ -179,7 +178,7 @@ func validateDataSourceTemplate(s interface{}) error {
// Execute your function on the field value
err := isValidField(strVal)
if err != nil {
return fmt.Errorf("field %s is empty or contains HTML injection - %s", v.Type().Field(i).Name, strVal)
return fmt.Errorf("field %s failed validation - %s", v.Type().Field(i).Name, strVal)
}
}
}
Expand Down
64 changes: 24 additions & 40 deletions internal/email/email_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ package email
import "testing"

func TestIsValidField(t *testing.T) {
t.Parallel() // make this sub-test run in parallel
t.Parallel()

tests := []struct {
input string
expectedErr bool
Expand All @@ -19,44 +20,28 @@ func TestIsValidField(t *testing.T) {
{"Just plain text", false, ""},

// Test case 3: String with HTML tags
{"<b>Bold Text</b>", true, "string <b>Bold Text</b> contains HTML tags, entities, or comments"},
{"<b>Bold Text</b>", true, "string <b>Bold Text</b> contains HTML injection"},

// Test case 4: String with HTML entity
{"This is a test &amp; example.", true, "string This is a test &amp; example. contains HTML tags, entities, or comments"},
{"This is a test &amp; example.", true, "string This is a test &amp; example. contains HTML injection"},

// Test case 5: String with multiple HTML entities
{"This &amp; that &lt; should &gt; work.", true, "string This &amp; that &lt; should &gt; work. contains HTML tags, entities, or comments"},

// Test case 6: String with special characters, but no HTML
{"Special chars! #$%^&*", false, ""},
{"This &amp; that &lt; should &gt; work.", true, "string This &amp; that &lt; should &gt; work. contains HTML injection"},

// Test case 7: Numeric HTML entity
{"This is a test &#1234;", true, "string This is a test &#1234; contains HTML tags, entities, or comments"},

// Test case 8: Valid URL (no HTML tags or entities)
// Test case 7: Valid URL (no HTML or JavaScript injection)
{"https://example.com", false, ""},

// Test case 9: Script tag injection
{"<script>alert('test');</script>", true, "string <script>alert('test');</script> contains HTML tags, entities, or comments"},

// Test case 10: Mixed content with HTML tag and entity
{"Hello <b>World</b> &amp; Universe.", true, "string Hello <b>World</b> &amp; Universe. contains HTML tags, entities, or comments"},
// Test case 8: Mixed content with HTML and JS
{"Hello <b>World</b> onload=alert('test');", true, "string Hello <b>World</b> onload=alert('test'); contains HTML injection"},

// Test case 11: Plain text with ampersand not forming an entity
{"AT&T is a company.", false, ""},

// Test case 12: Plain text with angle brackets but no tags
{"Angle brackets < and > in text.", false, ""},

// Test case 13: HTML-style comment
{"<!-- This is a comment -->", true, "string <!-- This is a comment --> contains HTML tags, entities, or comments"},
// Test case 11: HTML-style comment
{"<!-- This is a comment -->", true, "string <!-- This is a comment --> contains HTML injection"},
}

for _, tt := range tests {
tt := tt // capture range variable to avoid issues with parallel execution
tt := tt // capture range variable for parallel execution
t.Run(tt.input, func(t *testing.T) {
t.Parallel()

err := isValidField(tt.input)
if (err != nil) != tt.expectedErr {
t.Errorf("isValidField(%q) got error: %v, expected error: %v", tt.input, err, tt.expectedErr)
Expand All @@ -69,7 +54,7 @@ func TestIsValidField(t *testing.T) {
}

func TestValidateDataSourceTemplate(t *testing.T) {
t.Parallel() // make this sub-test run in parallel
t.Parallel()

tests := []struct {
input bodyData
Expand All @@ -93,7 +78,7 @@ func TestValidateDataSourceTemplate(t *testing.T) {
false, "",
},

// Test case 2: One field contains HTML tag
// Test case 2: AdminName contains HTML tags
{
bodyData{
AdminName: "John <b>Doe</b>",
Expand All @@ -107,32 +92,32 @@ func TestValidateDataSourceTemplate(t *testing.T) {
RoleName: "Administrator",
RoleVerb: "manage",
},
true, "field AdminName is empty or contains HTML injection - John <b>Doe</b>",
true, "field AdminName failed validation - John <b>Doe</b>",
},

// Test case 3: One field contains HTML entity
// Test case 3: OrganizationName contains HTML content
{
bodyData{
AdminName: "John Doe",
OrganizationName: "Acme Corp",
OrganizationName: "<script>alert('Hack');</script>",
InvitationURL: "https://invitation.com",
RecipientEmail: "[email protected]",
MinderURL: "https://minder.com",
TermsURL: "https://terms.com",
PrivacyURL: "https://privacy.com",
SignInURL: "https://signin.com",
RoleName: "Administrator",
RoleVerb: "approve &amp; manage",
RoleVerb: "manage",
},
true, "field RoleVerb is empty or contains HTML injection - approve &amp; manage",
true, "field OrganizationName failed validation - <script>alert('Hack');</script>",
},

// Test case 4: Multiple fields contain HTML content
// Test case 4: AdminName contains JavaScript code
{
bodyData{
AdminName: "John Doe",
OrganizationName: "<script>alert('Hack');</script>",
InvitationURL: "<a href='https://phishing.com'>Click here</a>",
AdminName: "onload=alert('test')",
OrganizationName: "Acme Corp",
InvitationURL: "https://invitation.com",
RecipientEmail: "[email protected]",
MinderURL: "https://minder.com",
TermsURL: "https://terms.com",
Expand All @@ -141,7 +126,7 @@ func TestValidateDataSourceTemplate(t *testing.T) {
RoleName: "Administrator",
RoleVerb: "manage",
},
true, "field OrganizationName is empty or contains HTML injection - <script>alert('Hack');</script>",
true, "field AdminName failed validation - onload=alert('test')",
},

// Test case 5: All fields contain valid plain text with some URLs
Expand All @@ -163,10 +148,9 @@ func TestValidateDataSourceTemplate(t *testing.T) {
}

for _, tt := range tests {
tt := tt // capture range variable to avoid issues with parallel execution
tt := tt // capture range variable for parallel execution
t.Run(tt.input.AdminName, func(t *testing.T) {
t.Parallel()

err := validateDataSourceTemplate(&tt.input)
if (err != nil) != tt.expectedErr {
t.Errorf("validateDataSourceTemplate(%+v) got error: %v, expected error: %v", tt.input, err, tt.expectedErr)
Expand Down

0 comments on commit 1505818

Please sign in to comment.