From 6c9d83360248dc5b965f2df51a75414f55cf2ae5 Mon Sep 17 00:00:00 2001 From: Jon McKenzie Date: Fri, 20 Mar 2020 16:50:27 -0400 Subject: [PATCH] AuthProxy: Fixes bug where long username could not be cached (#22926) --- pkg/middleware/auth_proxy/auth_proxy.go | 12 ++++++++++-- pkg/middleware/auth_proxy/auth_proxy_test.go | 9 ++++----- pkg/middleware/middleware_test.go | 3 +-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pkg/middleware/auth_proxy/auth_proxy.go b/pkg/middleware/auth_proxy/auth_proxy.go index f81c7a951ab..fa5a556e23c 100644 --- a/pkg/middleware/auth_proxy/auth_proxy.go +++ b/pkg/middleware/auth_proxy/auth_proxy.go @@ -1,8 +1,9 @@ package authproxy import ( - "encoding/base32" + "encoding/hex" "fmt" + "hash/fnv" "net" "net/mail" "reflect" @@ -146,6 +147,13 @@ func (auth *AuthProxy) IsAllowedIP() (bool, *Error) { return false, newError("Proxy authentication required", err) } +func HashCacheKey(key string) string { + hasher := fnv.New128a() + // according to the documentation, Hash.Write cannot error, but linter is complaining + hasher.Write([]byte(key)) // nolint: errcheck + return hex.EncodeToString(hasher.Sum(nil)) +} + // getKey forms a key for the cache based on the headers received as part of the authentication flow. // Our configuration supports multiple headers. The main header contains the email or username. // And the additional ones that allow us to specify extra attributes: Name, Email or Groups. @@ -156,7 +164,7 @@ func (auth *AuthProxy) getKey() string { key = strings.Join([]string{key, header}, "-") // compose the key with any additional headers }) - hashedKey := base32.StdEncoding.EncodeToString([]byte(key)) + hashedKey := HashCacheKey(key) return fmt.Sprintf(CachePrefix, hashedKey) } diff --git a/pkg/middleware/auth_proxy/auth_proxy_test.go b/pkg/middleware/auth_proxy/auth_proxy_test.go index e5ae5518120..ff3cf40312c 100644 --- a/pkg/middleware/auth_proxy/auth_proxy_test.go +++ b/pkg/middleware/auth_proxy/auth_proxy_test.go @@ -1,7 +1,6 @@ package authproxy import ( - "encoding/base32" "errors" "fmt" "net/http" @@ -79,7 +78,7 @@ func TestMiddlewareContext(t *testing.T) { Convey("with a simple cache key", func() { // Set cache key - key := fmt.Sprintf(CachePrefix, base32.StdEncoding.EncodeToString([]byte(name))) + key := fmt.Sprintf(CachePrefix, HashCacheKey(name)) err := store.Set(key, int64(33), 0) So(err, ShouldBeNil) @@ -88,7 +87,7 @@ func TestMiddlewareContext(t *testing.T) { id, err := auth.Login() So(err, ShouldBeNil) - So(auth.getKey(), ShouldEqual, "auth-proxy-sync-ttl:NVQXE23FNRXWO===") + So(auth.getKey(), ShouldEqual, "auth-proxy-sync-ttl:0a7f3374e9659b10980fd66247b0cf2f") So(id, ShouldEqual, 33) }) @@ -97,7 +96,7 @@ func TestMiddlewareContext(t *testing.T) { group := "grafana-core-team" req.Header.Add("X-WEBAUTH-GROUPS", group) - key := fmt.Sprintf(CachePrefix, base32.StdEncoding.EncodeToString([]byte(name+"-"+group))) + key := fmt.Sprintf(CachePrefix, HashCacheKey(name+"-"+group)) err := store.Set(key, int64(33), 0) So(err, ShouldBeNil) @@ -105,7 +104,7 @@ func TestMiddlewareContext(t *testing.T) { id, err := auth.Login() So(err, ShouldBeNil) - So(auth.getKey(), ShouldEqual, "auth-proxy-sync-ttl:NVQXE23FNRXWOLLHOJQWMYLOMEWWG33SMUWXIZLBNU======") + So(auth.getKey(), ShouldEqual, "auth-proxy-sync-ttl:14f69b7023baa0ac98c96b31cec07bc0") So(id, ShouldEqual, 33) }) diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 0157fc18824..0536442beb1 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -2,7 +2,6 @@ package middleware import ( "context" - "encoding/base32" "errors" "fmt" "net/http" @@ -364,7 +363,7 @@ func TestMiddlewareContext(t *testing.T) { return nil }) - key := fmt.Sprintf(authproxy.CachePrefix, base32.StdEncoding.EncodeToString([]byte(name+"-"+group))) + key := fmt.Sprintf(authproxy.CachePrefix, authproxy.HashCacheKey(name+"-"+group)) err := sc.remoteCacheService.Set(key, int64(33), 0) So(err, ShouldBeNil) sc.fakeReq("GET", "/")