ensure cfg.SiteDomain ends any URL we redirect to

.golangci.yml

@@ -8,7 +8,7 @@ run:
   timeout: 5m
 linters-settings:
   govet:
-    check-shadowing: true
+    shadow: true
   golint:
     min-confidence: 0
   gocyclo:

Makefile

@@ -20,7 +20,6 @@ build: generate-prod
 
 .PHONY: lint 
 lint:
-	go install github.com/golangci/golangci-lint/cmd/[email protected]
 	golangci-lint run ./...
 
 .PHONY: debug
@@ -42,13 +41,13 @@ convey:
 
 .PHONY: generate-debug
 generate-debug: fetch-renderer-lib
-	cd assets; go run github.com/kevinburke/go-bindata/go-bindata -prefix $(CORE_ASSETS_PATH)/assets -debug -o data.go -pkg assets locales/... templates/... $(CORE_ASSETS_PATH)/assets/locales/... $(CORE_ASSETS_PATH)/assets/templates/...
+	cd assets; go-bindata -prefix $(CORE_ASSETS_PATH)/assets -debug -o data.go -pkg assets locales/... templates/... $(CORE_ASSETS_PATH)/assets/locales/... $(CORE_ASSETS_PATH)/assets/templates/...
 	{ echo "// +build debug\n"; cat assets/data.go; } > assets/debug.go.new
 	mv assets/debug.go.new assets/data.go
 
 .PHONY: generate-prod
 generate-prod: fetch-renderer-lib
-	cd assets; go run github.com/kevinburke/go-bindata/go-bindata -prefix $(CORE_ASSETS_PATH)/assets -o data.go -pkg assets locales/... templates/... $(CORE_ASSETS_PATH)/assets/locales/... $(CORE_ASSETS_PATH)/assets/templates/...
+	cd assets; go-bindata -prefix $(CORE_ASSETS_PATH)/assets -o data.go -pkg assets locales/... templates/... $(CORE_ASSETS_PATH)/assets/locales/... $(CORE_ASSETS_PATH)/assets/templates/...
 	{ echo "// +build production\n"; cat assets/data.go; } > assets/data.go.new
 	mv assets/data.go.new assets/data.go
 

ci/build.yml

@@ -6,7 +6,7 @@ image_resource:
   type: docker-image
   source:
     repository: golang
-    tag: 1.22.2-bullseye
+    tag: 1.22.5-bullseye
 
 inputs:
   - name: dp-frontend-feedback-controller

ci/component.yml

@@ -6,7 +6,7 @@ image_resource:
   type: docker-image
   source:
     repository: golang
-    tag: 1.22.2-bullseye
+    tag: 1.22.5-bullseye
 
 inputs:
   - name: dp-frontend-feedback-controller

ci/lint.yml

@@ -6,7 +6,7 @@ image_resource:
   type: docker-image
   source:
     repository: golang
-    tag: 1.22.2-bullseye
+    tag: 1.22.5-bullseye
 
 inputs:
   - name: dp-frontend-feedback-controller

ci/scripts/build.sh

@@ -1,5 +1,7 @@
 #!/bin/bash -eux
 
+go install github.com/kevinburke/go-bindata/v4/[email protected]
+
 pushd dp-frontend-feedback-controller
   make build
   cp build/dp-frontend-feedback-controller Dockerfile.concourse ../build

ci/scripts/lint.sh

@@ -1,5 +1,7 @@
 #!/bin/bash -eux
 
+go install github.com/golangci/golangci-lint/cmd/[email protected]
+
 pushd dp-frontend-feedback-controller
   make lint
 popd

ci/scripts/unit.sh

@@ -1,5 +1,7 @@
 #!/bin/bash -eux
 
+go install github.com/kevinburke/go-bindata/v4/[email protected]
+
 pushd dp-frontend-feedback-controller
   make test
 popd

ci/unit.yml

@@ -6,7 +6,7 @@ image_resource:
   type: docker-image
   source:
     repository: golang
-    tag: 1.22.2-bullseye
+    tag: 1.22.5-bullseye
 
 inputs:
   - name: dp-frontend-feedback-controller

config/config.go

@@ -1,6 +1,8 @@
 package config
 
 import (
+	"net/url"
+	"strings"
 	"time"
 
 	"github.com/kelseyhightower/envconfig"
@@ -40,6 +42,10 @@ var cfg *Config
 // Get returns the default config with any modifications through environment
 // variables
 func Get() (*Config, error) {
+	if cfg != nil {
+		return cfg, nil
+	}
+
 	envCfg, err := get()
 	if err != nil {
 		return nil, err
@@ -50,14 +56,12 @@ func Get() (*Config, error) {
 	} else {
 		envCfg.PatternLibraryAssetsPath = "//cdn.ons.gov.uk/dp-design-system/e0a75c3"
 	}
-	return envCfg, nil
+
+	cfg = envCfg
+	return cfg, nil
 }
 
 func get() (*Config, error) {
-	if cfg != nil {
-		return cfg, nil
-	}
-
 	cfg := &Config{
 		APIRouterURL:                "http://localhost:23200/v1",
 		BindAddr:                    "localhost:25200",
@@ -86,3 +90,22 @@ func get() (*Config, error) {
 
 	return cfg, envconfig.Process("", cfg)
 }
+
+// IsSiteDomainURL is true when urlString is a URL and its host ends with `.`+siteDomain (when siteDomain is blank, uses cfg.SiteDomain)
+func IsSiteDomainURL(urlString, siteDomain string) bool {
+	if urlString == "" {
+		return false
+	}
+	if siteDomain == "" {
+		siteDomain = cfg.SiteDomain
+	}
+	urlObject, err := url.ParseRequestURI(urlString)
+	if err != nil {
+		return false
+	}
+	hostName := urlObject.Hostname()
+	if hostName != siteDomain && !strings.HasSuffix(hostName, "."+siteDomain) {
+		return false
+	}
+	return true
+}

config/config_test.go

@@ -9,8 +9,8 @@ import (
 
 func TestConfig(t *testing.T) {
 	Convey("Given an environment with no environment variables set", t, func() {
-		cfg, err := Get()
 		Convey("When the config values are retrieved", func() {
+			cfg, err := Get()
 			Convey("Then there should be no error returned", func() {
 				So(err, ShouldBeNil)
 			})
@@ -40,6 +40,35 @@ func TestConfig(t *testing.T) {
 				So(newErr, ShouldBeNil)
 				So(newCfg, ShouldResemble, cfg)
 			})
+			Convey("Then a sub-domain off an explicit site domain is recognised", func() {
+				isAllowedURL := IsSiteDomainURL("https://anything.ons.gov.uk:443/ook", "ons.gov.uk")
+				So(isAllowedURL, ShouldBeTrue)
+			})
+			Convey("Then a non-site domain URL is not recognised for explicit site domain", func() {
+				isAllowedURL := IsSiteDomainURL("https://anything.example.com", "ons.gov.uk")
+				So(isAllowedURL, ShouldBeFalse)
+			})
+
+			Convey("Then a non-URL is not recognised for explicit site domain", func() {
+				isAllowedURL := IsSiteDomainURL("blah", "ons.gov.uk")
+				So(isAllowedURL, ShouldBeFalse)
+			})
+			Convey("Then a URL of the config's site domain is recognised", func() {
+				isAllowedURL := IsSiteDomainURL("https://localhost", "")
+				So(isAllowedURL, ShouldBeTrue)
+			})
+			Convey("Then a sub-domain/host of the config's site domain is recognised", func() {
+				isAllowedURL := IsSiteDomainURL("https://anything.localhost", "")
+				So(isAllowedURL, ShouldBeTrue)
+			})
+			Convey("Then a non-site domain URL is not recognised for config's site domain", func() {
+				isAllowedURL := IsSiteDomainURL("https://not-site-domain.example.com", "")
+				So(isAllowedURL, ShouldBeFalse)
+			})
+			Convey("Then a non-URL is not recognised for config's site domain", func() {
+				isAllowedURL := IsSiteDomainURL("blah", "")
+				So(isAllowedURL, ShouldBeFalse)
+			})
 		})
 	})
 }

handlers/feedback.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"net/url"
 	"regexp"
 	"strings"
 
@@ -24,25 +23,16 @@ import (
 // FeedbackThanks loads the Feedback Thank you page
 func (f *Feedback) FeedbackThanks() http.HandlerFunc {
 	return dphandlers.ControllerHandler(func(w http.ResponseWriter, req *http.Request, lang, collectionID, accessToken string) {
-		feedbackThanks(w, req, req.Referer(), f.Render, f.CacheService, lang)
+		feedbackThanks(w, req, req.Referer(), f.Render, f.CacheService, lang, f.Config.SiteDomain, f.Config.EnableNewNavBar)
 	})
 }
 
-func feedbackThanks(w http.ResponseWriter, req *http.Request, uri string, rend interfaces.Renderer, cacheHelperService *cacheHelper.Helper, lang string) {
-	ctx := req.Context()
-	var wholeSite string
-
-	cfg, err := config.Get()
-	if err != nil {
-		log.Warn(ctx, "Unable to retrieve configuration", log.FormatErrors([]error{err}))
-	} else {
-		wholeSite = cfg.SiteDomain
-	}
-
+func feedbackThanks(w http.ResponseWriter, req *http.Request, uri string, rend interfaces.Renderer, cacheHelperService *cacheHelper.Helper, lang, siteDomain string, enableNewNavBar bool) {
 	basePage := rend.NewBasePageModel()
-	p := mapper.CreateGetFeedbackThanks(req, basePage, lang, uri, wholeSite)
+	p := mapper.CreateGetFeedbackThanks(req, basePage, lang, uri, siteDomain)
 
-	if cfg.EnableNewNavBar {
+	if enableNewNavBar {
+		ctx := req.Context()
 		mappedNavContent, err := cacheHelperService.GetMappedNavigationContent(ctx, lang)
 		if err == nil {
 			p.NavigationContent = mappedNavContent
@@ -55,20 +45,16 @@ func feedbackThanks(w http.ResponseWriter, req *http.Request, uri string, rend i
 // GetFeedback handles the loading of a feedback page
 func (f *Feedback) GetFeedback() http.HandlerFunc {
 	return dphandlers.ControllerHandler(func(w http.ResponseWriter, req *http.Request, lang, collectionID, accessToken string) {
-		getFeedback(w, req, []core.ErrorItem{}, model.FeedbackForm{URL: req.Referer()}, lang, f.Render, f.CacheService)
+		getFeedback(w, req, []core.ErrorItem{}, model.FeedbackForm{URL: req.Referer()}, lang, f.Render, f.CacheService, f.Config.EnableNewNavBar)
 	})
 }
 
-func getFeedback(w http.ResponseWriter, req *http.Request, validationErrors []core.ErrorItem, ff model.FeedbackForm, lang string, rend interfaces.Renderer, cacheHelperService *cacheHelper.Helper) {
+func getFeedback(w http.ResponseWriter, req *http.Request, validationErrors []core.ErrorItem, ff model.FeedbackForm, lang string, rend interfaces.Renderer, cacheHelperService *cacheHelper.Helper, enableNewNavBar bool) {
 	basePage := rend.NewBasePageModel()
 	p := mapper.CreateGetFeedback(req, basePage, validationErrors, ff, lang)
 
-	ctx := context.Background()
-	cfg, err := config.Get()
-	if err != nil {
-		log.Warn(ctx, "Unable to retrieve configuration", log.FormatErrors([]error{err}))
-	}
-	if cfg.EnableNewNavBar {
+	if enableNewNavBar {
+		ctx := context.Background()
 		mappedNavContent, err := cacheHelperService.GetMappedNavigationContent(ctx, lang)
 		if err == nil {
 			p.NavigationContent = mappedNavContent
@@ -81,11 +67,11 @@ func getFeedback(w http.ResponseWriter, req *http.Request, validationErrors []co
 // AddFeedback handles a users feedback request
 func (f *Feedback) AddFeedback() http.HandlerFunc {
 	return dphandlers.ControllerHandler(func(w http.ResponseWriter, req *http.Request, lang, collectionID, accessToken string) {
-		addFeedback(w, req, f.Render, f.EmailSender, f.Config.FeedbackFrom, f.Config.FeedbackTo, lang, f.CacheService)
+		addFeedback(w, req, f.Render, f.EmailSender, f.Config.FeedbackFrom, f.Config.FeedbackTo, lang, f.Config.SiteDomain, f.CacheService)
 	})
 }
 
-func addFeedback(w http.ResponseWriter, req *http.Request, rend interfaces.Renderer, emailSender email.Sender, from, to, lang string, cacheService *cacheHelper.Helper) {
+func addFeedback(w http.ResponseWriter, req *http.Request, rend interfaces.Renderer, emailSender email.Sender, from, to, lang, siteDomain string, cacheService *cacheHelper.Helper) {
 	ctx := req.Context()
 	if err := req.ParseForm(); err != nil {
 		log.Error(ctx, "unable to parse request form", err)
@@ -103,9 +89,9 @@ func addFeedback(w http.ResponseWriter, req *http.Request, rend interfaces.Rende
 		return
 	}
 
-	validationErrors := validateForm(&ff)
+	validationErrors := validateForm(&ff, siteDomain)
 	if len(validationErrors) > 0 {
-		getFeedback(w, req, validationErrors, ff, lang, rend, cacheService)
+		getFeedback(w, req, validationErrors, ff, lang, rend, cacheService, false)
 		return
 	}
 
@@ -134,7 +120,7 @@ func addFeedback(w http.ResponseWriter, req *http.Request, rend interfaces.Rende
 }
 
 // validateForm is a helper function that validates a slice of FeedbackForm to determine if there are form validation errors
-func validateForm(ff *model.FeedbackForm) (validationErrors []core.ErrorItem) {
+func validateForm(ff *model.FeedbackForm, siteDomain string) (validationErrors []core.ErrorItem) {
 	if ff.Type == "" && ff.FormLocation != "footer" {
 		validationErrors = append(validationErrors, core.ErrorItem{
 			Description: core.Localisation{
@@ -147,28 +133,28 @@ func validateForm(ff *model.FeedbackForm) (validationErrors []core.ErrorItem) {
 	}
 
 	ff.URL = strings.TrimSpace(ff.URL)
-	if ff.Type == "A specific page" && ff.URL == "" {
-		validationErrors = append(validationErrors, core.ErrorItem{
-			Description: core.Localisation{
-				LocaleKey: "FeedbackWhatEnterURL",
-				Plural:    1,
-			},
-			URL: "#type-error",
-		})
-		ff.IsURLErr = true
-	}
-
-	if ff.Type == "A specific page" && ff.URL != "" {
-		_, err := url.ParseRequestURI(ff.URL)
-		if err != nil {
+	if ff.Type == "A specific page" {
+		if ff.URL == "" {
 			validationErrors = append(validationErrors, core.ErrorItem{
 				Description: core.Localisation{
-					LocaleKey: "FeedbackValidURL",
+					LocaleKey: "FeedbackWhatEnterURL",
 					Plural:    1,
 				},
 				URL: "#type-error",
 			})
 			ff.IsURLErr = true
+
+		} else {
+			if !config.IsSiteDomainURL(ff.URL, siteDomain) {
+				validationErrors = append(validationErrors, core.ErrorItem{
+					Description: core.Localisation{
+						LocaleKey: "FeedbackValidURL",
+						Plural:    1,
+					},
+					URL: "#type-error",
+				})
+				ff.IsURLErr = true
+			}
 		}
 	}