From 391868c5d61c7844a9b71b9e93328d3479faf457 Mon Sep 17 00:00:00 2001
From: Julian Kornberger <jk+github@digineo.de>
Date: Wed, 21 Mar 2018 18:28:56 +0100
Subject: [PATCH] Use net.SplitHostPort to support IPv6

- Add some tests
- Make error message more helpful
---
 pkg/middleware/auth_proxy.go      | 32 ++++++++++++++-----------------
 pkg/middleware/middleware_test.go | 26 +++++++++++++++++++++----
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go
index e1801404453..adf0b7b53d5 100644
--- a/pkg/middleware/auth_proxy.go
+++ b/pkg/middleware/auth_proxy.go
@@ -1,8 +1,8 @@
 package middleware
 
 import (
-	"errors"
 	"fmt"
+	"net"
 	"strings"
 	"time"
 
@@ -25,7 +25,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool {
 	}
 
 	// if auth proxy ip(s) defined, check if request comes from one of those
-	if err := checkAuthenticationProxy(ctx, proxyHeaderValue); err != nil {
+	if err := checkAuthenticationProxy(ctx.Req.RemoteAddr, proxyHeaderValue); err != nil {
 		ctx.Handle(407, "Proxy authentication required", err)
 		return true
 	}
@@ -123,29 +123,25 @@ var syncGrafanaUserWithLdapUser = func(ctx *m.ReqContext, query *m.GetSignedInUs
 	return nil
 }
 
-func checkAuthenticationProxy(ctx *m.ReqContext, proxyHeaderValue string) error {
+func checkAuthenticationProxy(remoteAddr string, proxyHeaderValue string) error {
 	if len(strings.TrimSpace(setting.AuthProxyWhitelist)) == 0 {
 		return nil
 	}
+
 	proxies := strings.Split(setting.AuthProxyWhitelist, ",")
-	remoteAddrSplit := strings.Split(ctx.Req.RemoteAddr, ":")
-	sourceIP := remoteAddrSplit[0]
-
-	found := false
-	for _, proxyIP := range proxies {
-		if sourceIP == strings.TrimSpace(proxyIP) {
-			found = true
-			break
-		}
-	}
-
-	if !found {
-		msg := fmt.Sprintf("Request for user (%s) is not from the authentication proxy", proxyHeaderValue)
-		err := errors.New(msg)
+	sourceIP, _, err := net.SplitHostPort(remoteAddr)
+	if err != nil {
 		return err
 	}
 
-	return nil
+	// Compare allowed IP addresses to actual address
+	for _, proxyIP := range proxies {
+		if sourceIP == strings.TrimSpace(proxyIP) {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, sourceIP)
 }
 
 func getSignedInUserQueryForProxyAuth(headerVal string) *m.GetSignedInUserQuery {
diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go
index c8e9e535cfa..f80a30de02f 100644
--- a/pkg/middleware/middleware_test.go
+++ b/pkg/middleware/middleware_test.go
@@ -226,11 +226,11 @@ func TestMiddlewareContext(t *testing.T) {
 			})
 		})
 
-		middlewareScenario("When auth_proxy is enabled and request RemoteAddr is not trusted", func(sc *scenarioContext) {
+		middlewareScenario("When auth_proxy is enabled and IPv4 request RemoteAddr is not trusted", func(sc *scenarioContext) {
 			setting.AuthProxyEnabled = true
 			setting.AuthProxyHeaderName = "X-WEBAUTH-USER"
 			setting.AuthProxyHeaderProperty = "username"
-			setting.AuthProxyWhitelist = "192.168.1.1, 192.168.2.1"
+			setting.AuthProxyWhitelist = "192.168.1.1, 2001::23"
 
 			sc.fakeReq("GET", "/")
 			sc.req.Header.Add("X-WEBAUTH-USER", "torkelo")
@@ -239,6 +239,24 @@ func TestMiddlewareContext(t *testing.T) {
 
 			Convey("should return 407 status code", func() {
 				So(sc.resp.Code, ShouldEqual, 407)
+				So(sc.resp.Body.String(), ShouldContainSubstring, "Request for user (torkelo) from 192.168.3.1 is not from the authentication proxy")
+			})
+		})
+
+		middlewareScenario("When auth_proxy is enabled and IPv6 request RemoteAddr is not trusted", func(sc *scenarioContext) {
+			setting.AuthProxyEnabled = true
+			setting.AuthProxyHeaderName = "X-WEBAUTH-USER"
+			setting.AuthProxyHeaderProperty = "username"
+			setting.AuthProxyWhitelist = "192.168.1.1, 2001::23"
+
+			sc.fakeReq("GET", "/")
+			sc.req.Header.Add("X-WEBAUTH-USER", "torkelo")
+			sc.req.RemoteAddr = "[2001:23]:12345"
+			sc.exec()
+
+			Convey("should return 407 status code", func() {
+				So(sc.resp.Code, ShouldEqual, 407)
+				So(sc.resp.Body.String(), ShouldContainSubstring, "Request for user (torkelo) from 2001:23 is not from the authentication proxy")
 			})
 		})
 
@@ -246,7 +264,7 @@ func TestMiddlewareContext(t *testing.T) {
 			setting.AuthProxyEnabled = true
 			setting.AuthProxyHeaderName = "X-WEBAUTH-USER"
 			setting.AuthProxyHeaderProperty = "username"
-			setting.AuthProxyWhitelist = "192.168.1.1, 192.168.2.1"
+			setting.AuthProxyWhitelist = "192.168.1.1, 2001::23"
 
 			bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error {
 				query.Result = &m.SignedInUser{OrgId: 4, UserId: 33}
@@ -255,7 +273,7 @@ func TestMiddlewareContext(t *testing.T) {
 
 			sc.fakeReq("GET", "/")
 			sc.req.Header.Add("X-WEBAUTH-USER", "torkelo")
-			sc.req.RemoteAddr = "192.168.2.1:12345"
+			sc.req.RemoteAddr = "[2001::23]:12345"
 			sc.exec()
 
 			Convey("Should init context with user info", func() {