From 5fffb2907bada0a11bb0685006586666dfb8ee1a Mon Sep 17 00:00:00 2001
From: Will McCutchen <will@mccutch.org>
Date: Wed, 16 Nov 2022 16:56:12 -0500
Subject: [PATCH] More helpful error when allowed redirect domains are defined
(#104)
Make the error message when a forbidden destination is given to
`/redirect-to`, to help users better understand what allowed usage looks
like. Motivated by #103.
Before:
```
$ curl http://localhost:8080/redirect-to?url=https://google.com
Forbidden redirect URL. Be careful with this link.
```
After:
```
$ curl http://localhost:8080/redirect-to?url=https://google.com
Forbidden redirect URL. Please be careful with this link.
Allowed redirect destinations:
- example.com
- example.org
```
---
httpbin/handlers.go | 13 ++++++++++++-
httpbin/handlers_test.go | 12 +++++++++++-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/httpbin/handlers.go b/httpbin/handlers.go
index d2ac406..6409930 100644
--- a/httpbin/handlers.go
+++ b/httpbin/handlers.go
@@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"net/url"
+ "sort"
"strconv"
"strings"
"time"
@@ -390,7 +391,17 @@ func (h *HTTPBin) RedirectTo(w http.ResponseWriter, r *http.Request) {
if u.IsAbs() && len(h.AllowedRedirectDomains) > 0 {
if _, ok := h.AllowedRedirectDomains[u.Hostname()]; !ok {
- http.Error(w, "Forbidden redirect URL. Be careful with this link.", http.StatusForbidden)
+ domainListItems := make([]string, 0, len(h.AllowedRedirectDomains))
+ for domain := range h.AllowedRedirectDomains {
+ domainListItems = append(domainListItems, fmt.Sprintf("- %s", domain))
+ }
+ sort.Strings(domainListItems)
+ formattedDomains := strings.Join(domainListItems, "\n")
+ msg := fmt.Sprintf(`Forbidden redirect URL. Please be careful with this link.
+
+Allowed redirect destinations:
+%s`, formattedDomains)
+ http.Error(w, msg, http.StatusForbidden)
return
}
}
diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go
index 5545db1..f317870 100644
--- a/httpbin/handlers_test.go
+++ b/httpbin/handlers_test.go
@@ -74,7 +74,7 @@ func assertBodyEquals(t *testing.T, w *httptest.ResponseRecorder, want string) {
t.Helper()
have := w.Body.String()
if want != have {
- t.Fatalf("expected body = %v, got %v", want, have)
+ t.Fatalf("expected body = %q, got %q", want, have)
}
}
@@ -1239,6 +1239,13 @@ func TestRedirectTo(t *testing.T) {
WithObserver(StdLogObserver(log.New(io.Discard, "", 0))),
).Handler()
+ allowedDomainsError := `Forbidden redirect URL. Please be careful with this link.
+
+Allowed redirect destinations:
+- example.org
+- httpbingo.org
+`
+
allowListTests := []struct {
url string
expectedStatus int
@@ -1257,6 +1264,9 @@ func TestRedirectTo(t *testing.T) {
w := httptest.NewRecorder()
allowListHandler.ServeHTTP(w, r)
assertStatusCode(t, w, test.expectedStatus)
+ if test.expectedStatus >= 400 {
+ assertBodyEquals(t, w, allowedDomainsError)
+ }
})
}
}
--
GitLab