Skip to content

Commit

Permalink
More helpful error when allowed redirect domains are defined (#104)
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
mccutchen authored Nov 16, 2022
1 parent 8d00add commit 5fffb29
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
13 changes: 12 additions & 1 deletion httpbin/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"net/url"
"sort"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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
}
}
Expand Down
12 changes: 11 additions & 1 deletion httpbin/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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)
}
})
}
}
Expand Down

0 comments on commit 5fffb29

Please sign in to comment.