Validate redirect domain

This change introduces a validation step prior to redirect as
discussed in #77

internal/auth.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"time"
@@ -81,9 +82,35 @@ func ValidateEmail(email string) bool {
 	return found
 }
 
+// ValidateRedirect validates that the given redirect is valid and permitted for
+// the given request
+func ValidateRedirect(r *http.Request, redirect string) error {
+	redirectURL, err := url.Parse(redirect)
+
+	if err != nil {
+		return errors.New("Unable to parse redirect")
+	}
+
+	// If we're using an auth domain?
+	if use, base := useAuthDomain(r); use {
+		// If we are using an auth domain, they redirect must share a common
+		// suffix with the requested redirect
+		if !strings.HasSuffix(redirectURL.Host, base) {
+			return errors.New("Redirect host does not match any expected hosts (should match cookie domain when using auth host)")
+		}
+	} else {
+		// If not, we should only ever redirect to the same domain
+		if redirectURL.Host != r.Header.Get("X-Forwarded-Host") {
+			return errors.New("Redirect host does not match request host (must match when not using auth host)")
+		}
+	}
+
+	return nil
+}
+
 // Utility methods
 
-// Get the redirect base
+// Get the request base from forwarded request
 func redirectBase(r *http.Request) string {
 	proto := r.Header.Get("X-Forwarded-Proto")
 	host := r.Header.Get("X-Forwarded-Host")

internal/auth_test.go

@@ -95,6 +95,96 @@ func TestAuthValidateEmail(t *testing.T) {
 	assert.True(v, "should allow user in whitelist")
 }
 
+func TestAuthValidateRedirect(t *testing.T) {
+	assert := assert.New(t)
+	config, _ = NewConfig([]string{})
+
+	newRedirectRequest := func(urlStr string) *http.Request {
+		u, err := url.Parse(urlStr)
+		assert.Nil(err)
+
+		r, _ := http.NewRequest("GET", urlStr, nil)
+		r.Header.Add("X-Forwarded-Proto", u.Scheme)
+		r.Header.Add("X-Forwarded-Host", u.Host)
+		r.Header.Add("X-Forwarded-Uri", u.RequestURI())
+
+		return r
+	}
+
+	errStr := "Redirect host does not match request host (must match when not using auth host)"
+
+	err := ValidateRedirect(
+		newRedirectRequest("http://app.example.com/_oauth?state=123"),
+		"http://app.example.com.bad.com",
+	)
+	if assert.Error(err) {
+		assert.Equal(errStr, err.Error(), "Should not allow redirect to subdomain")
+	}
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://app.example.com/_oauth?state=123"),
+		"http://app.example.combad.com",
+	)
+	if assert.Error(err) {
+		assert.Equal(errStr, err.Error(), "Should not allow redirect to overlapping domain")
+	}
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://app.example.com/_oauth?state=123"),
+		"http://example.com",
+	)
+	if assert.Error(err) {
+		assert.Equal(errStr, err.Error(), "Should not allow redirect from subdomain")
+	}
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://app.example.com/_oauth?state=123"),
+		"http://app.example.com/profile",
+	)
+	assert.Nil(err, "Should allow same domain")
+
+	//
+	// With Auth Host
+	//
+	config.AuthHost = "auth.example.com"
+	config.CookieDomains = []CookieDomain{*NewCookieDomain("example.com")}
+	errStr = "Redirect host does not match any expected hosts (should match cookie domain when using auth host)"
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://app.example.com/_oauth?state=123"),
+		"http://app.example.com.bad.com",
+	)
+	if assert.Error(err) {
+		assert.Equal(errStr, err.Error(), "Should not allow redirect to subdomain")
+	}
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://app.example.com/_oauth?state=123"),
+		"http://app.example.combad.com",
+	)
+	if assert.Error(err) {
+		assert.Equal(errStr, err.Error(), "Should not allow redirect to overlapping domain")
+	}
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://auth.example.com/_oauth?state=123"),
+		"http://app.example.com/profile",
+	)
+	assert.Nil(err, "Should allow between subdomains when using auth host")
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://auth.example.com/_oauth?state=123"),
+		"http://auth.example.com/profile",
+	)
+	assert.Nil(err, "Should allow same domain when using auth host")
+
+	err = ValidateRedirect(
+		newRedirectRequest("http://auth.example.com/_oauth?state=123"),
+		"http://example.com/profile",
+	)
+	assert.Nil(err, "Should allow from subdomain when using auth host")
+}
+
 func TestRedirectUri(t *testing.T) {
 	assert := assert.New(t)
 

internal/server.go

@@ -143,6 +143,16 @@ func (s *Server) AuthCallbackHandler() http.HandlerFunc {
 		// Clear CSRF cookie
 		http.SetCookie(w, ClearCSRFCookie(r))
 
+		// Validate redirect
+		err = ValidateRedirect(r, redirect)
+		if err != nil {
+			logger.WithFields(logrus.Fields{
+				"receieved_redirect": redirect,
+			}).Warnf("Invalid redirect in CSRF. %v", err)
+			http.Error(w, "Not authorized", 401)
+			return
+		}
+
 		// Exchange code for token
 		token, err := p.ExchangeCode(redirectUri(r), r.URL.Query().Get("code"))
 		if err != nil {
@@ -193,7 +203,8 @@ func (s *Server) authRedirect(logger *logrus.Entry, w http.ResponseWriter, r *ht
 func (s *Server) logger(r *http.Request, rule, msg string) *logrus.Entry {
 	// Create logger
 	logger := log.WithFields(logrus.Fields{
-		"source_ip": r.Header.Get("X-Forwarded-For"),
+		"source_ip":    r.Header.Get("X-Forwarded-For"),
+		"request_host": r.Header.Get("X-Forwarded-Host"),
 	})
 
 	// Log request

internal/server_test.go

@@ -141,20 +141,20 @@ func TestServerAuthCallback(t *testing.T) {
 	assert.Equal(401, res.StatusCode, "auth callback without cookie shouldn't be authorised")
 
 	// Should catch invalid csrf cookie
-	req = newDefaultHttpRequest("/_oauth?state=12345678901234567890123456789012:http://redirect")
+	req = newDefaultHttpRequest("/_oauth?state=12345678901234567890123456789012:http://example.com")
 	c := MakeCSRFCookie(req, "nononononononononononononononono")
 	res, _ = doHttpRequest(req, c)
 	assert.Equal(401, res.StatusCode, "auth callback with invalid cookie shouldn't be authorised")
 
 	// Should redirect valid request
-	req = newDefaultHttpRequest("/_oauth?state=12345678901234567890123456789012:google:http://redirect")
+	req = newDefaultHttpRequest("/_oauth?state=12345678901234567890123456789012:google:http://example.com")
 	c = MakeCSRFCookie(req, "12345678901234567890123456789012")
 	res, _ = doHttpRequest(req, c)
-	assert.Equal(307, res.StatusCode, "valid auth callback should be allowed")
+	require.Equal(t, 307, res.StatusCode, "valid auth callback should be allowed")
 
 	fwd, _ := res.Location()
 	assert.Equal("http", fwd.Scheme, "valid request should be redirected to return url")
-	assert.Equal("redirect", fwd.Host, "valid request should be redirected to return url")
+	assert.Equal("example.com", fwd.Host, "valid request should be redirected to return url")
 	assert.Equal("", fwd.Path, "valid request should be redirected to return url")
 }