From c9ff8714a0beda15b985a5d0061df3bcbc2f8f9d Mon Sep 17 00:00:00 2001
From: Jordan Wright <jmwright798@gmail.com>
Date: Thu, 22 Feb 2018 23:02:27 -0600
Subject: [PATCH] Moved rid parameter to a separate constant. Fixes #911

---
 controllers/phish.go      | 10 ++++++++--
 controllers/phish_test.go | 12 ++++++------
 models/campaign.go        |  3 +++
 models/maillog_test.go    | 10 +++++-----
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/controllers/phish.go b/controllers/phish.go
index 6c35c01f..6be3f685 100644
--- a/controllers/phish.go
+++ b/controllers/phish.go
@@ -127,13 +127,19 @@ func PhishHandler(w http.ResponseWriter, r *http.Request) {
 	if fn == "" {
 		fn = f.Address
 	}
+
+	phishURL, _ := url.Parse(c.URL)
+	q := phishURL.Query()
+	q.Set(models.RecipientParameter, rs.RId)
+	phishURL.RawQuery = q.Encode()
+
 	rsf := struct {
 		models.Result
 		URL  string
 		From string
 	}{
 		rs,
-		c.URL + "?rid=" + rs.RId,
+		phishURL.String(),
 		fn,
 	}
 	err = tmpl.Execute(&htmlBuff, rsf)
@@ -156,7 +162,7 @@ func setupContext(r *http.Request) (error, *http.Request) {
 		Logger.Println(err)
 		return err, r
 	}
-	id := r.Form.Get("rid")
+	id := r.Form.Get(models.RecipientParameter)
 	if id == "" {
 		return ErrInvalidRequest, r
 	}
diff --git a/controllers/phish_test.go b/controllers/phish_test.go
index 5a18ac4d..91de41eb 100644
--- a/controllers/phish_test.go
+++ b/controllers/phish_test.go
@@ -16,7 +16,7 @@ func (s *ControllersSuite) getFirstCampaign() models.Campaign {
 }
 
 func (s *ControllersSuite) openEmail(rid string) {
-	resp, err := http.Get(fmt.Sprintf("%s/track?rid=%s", ps.URL, rid))
+	resp, err := http.Get(fmt.Sprintf("%s/track?%s=%s", ps.URL, models.RecipientParameter, rid))
 	s.Nil(err)
 	defer resp.Body.Close()
 	body, err := ioutil.ReadAll(resp.Body)
@@ -27,7 +27,7 @@ func (s *ControllersSuite) openEmail(rid string) {
 }
 
 func (s *ControllersSuite) openEmail404(rid string) {
-	resp, err := http.Get(fmt.Sprintf("%s/track?rid=%s", ps.URL, rid))
+	resp, err := http.Get(fmt.Sprintf("%s/track?%s=%s", ps.URL, models.RecipientParameter, rid))
 	s.Nil(err)
 	defer resp.Body.Close()
 	s.Nil(err)
@@ -35,7 +35,7 @@ func (s *ControllersSuite) openEmail404(rid string) {
 }
 
 func (s *ControllersSuite) clickLink(rid string, campaign models.Campaign) {
-	resp, err := http.Get(fmt.Sprintf("%s/?rid=%s", ps.URL, rid))
+	resp, err := http.Get(fmt.Sprintf("%s/?%s=%s", ps.URL, models.RecipientParameter, rid))
 	s.Nil(err)
 	defer resp.Body.Close()
 	body, err := ioutil.ReadAll(resp.Body)
@@ -44,7 +44,7 @@ func (s *ControllersSuite) clickLink(rid string, campaign models.Campaign) {
 }
 
 func (s *ControllersSuite) clickLink404(rid string) {
-	resp, err := http.Get(fmt.Sprintf("%s/?rid=%s", ps.URL, rid))
+	resp, err := http.Get(fmt.Sprintf("%s/?%s=%s", ps.URL, models.RecipientParameter, rid))
 	s.Nil(err)
 	defer resp.Body.Close()
 	s.Nil(err)
@@ -88,11 +88,11 @@ func (s *ControllersSuite) TestNoRecipientID() {
 
 func (s *ControllersSuite) TestInvalidRecipientID() {
 	rid := "XXXXXXXXXX"
-	resp, err := http.Get(fmt.Sprintf("%s/track?rid=%s", ps.URL, rid))
+	resp, err := http.Get(fmt.Sprintf("%s/track?%s=%s", ps.URL, models.RecipientParameter, rid))
 	s.Nil(err)
 	s.Equal(resp.StatusCode, http.StatusNotFound)
 
-	resp, err = http.Get(fmt.Sprintf("%s/?rid=%s", ps.URL, rid))
+	resp, err = http.Get(fmt.Sprintf("%s/?%s=%s", ps.URL, models.RecipientParameter, rid))
 	s.Nil(err)
 	s.Equal(resp.StatusCode, http.StatusNotFound)
 }
diff --git a/models/campaign.go b/models/campaign.go
index 5abe0d1e..ed6e402b 100644
--- a/models/campaign.go
+++ b/models/campaign.go
@@ -91,6 +91,9 @@ var ErrPageNotFound = errors.New("Page not found")
 // ErrSMTPNotFound indicates a sending profile specified by the user does not exist in the database
 var ErrSMTPNotFound = errors.New("Sending profile not found")
 
+// RecipientParameter is the URL parameter that points to the result ID for a recipient.
+const RecipientParameter = "rid"
+
 // Validate checks to make sure there are no invalid fields in a submitted campaign
 func (c *Campaign) Validate() error {
 	switch {
diff --git a/models/maillog_test.go b/models/maillog_test.go
index 2607245e..f77be871 100644
--- a/models/maillog_test.go
+++ b/models/maillog_test.go
@@ -241,10 +241,10 @@ func (s *ModelsSuite) TestUnlockAllMailLogs(ch *check.C) {
 
 func (s *ModelsSuite) TestURLTemplateRendering(ch *check.C) {
 	template := Template{
-		Name: "URLTemplate",
-		UserId: 1,
-		Text: "{{.URL}}",
-		HTML: "{{.URL}}",
+		Name:    "URLTemplate",
+		UserId:  1,
+		Text:    "{{.URL}}",
+		HTML:    "{{.URL}}",
 		Subject: "{{.URL}}",
 	}
 	ch.Assert(PostTemplate(&template), check.Equals, nil)
@@ -254,7 +254,7 @@ func (s *ModelsSuite) TestURLTemplateRendering(ch *check.C) {
 
 	ch.Assert(PostCampaign(&campaign, campaign.UserId), check.Equals, nil)
 	result := campaign.Results[0]
-	expectedURL := fmt.Sprintf("http://127.0.0.1/%s/?rid=%s", result.Email, result.RId)
+	expectedURL := fmt.Sprintf("http://127.0.0.1/%s/?%s=%s", result.Email, RecipientParameter, result.RId)
 
 	m := &MailLog{}
 	err := db.Where("r_id=? AND campaign_id=?", result.RId, campaign.Id).