From e64001674acee7c2d3416f780733ac31a3f2a147 Mon Sep 17 00:00:00 2001 From: shaj13 Date: Wed, 24 Jun 2020 11:05:58 -0700 Subject: [PATCH] fix cached bugs --- auth/strategies/digest/cached.go | 43 +++++++++++---- auth/strategies/digest/cached_test.go | 35 ++++++++++--- auth/strategies/digest/header.go | 22 ++++++++ auth/strategies/digest/header_test.go | 75 +++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 15 deletions(-) diff --git a/auth/strategies/digest/cached.go b/auth/strategies/digest/cached.go index 3501664..b62da8a 100644 --- a/auth/strategies/digest/cached.go +++ b/auth/strategies/digest/cached.go @@ -9,6 +9,8 @@ import ( "github.com/shaj13/go-guardian/store" ) +const extensionKey = "x-go-guardian-digest" + // CachedStrategy caches digest strategy authentication response based on authorization nonce field. type CachedStrategy struct { *Strategy @@ -28,20 +30,43 @@ func (c *CachedStrategy) Authenticate(ctx context.Context, r *http.Request) (aut } if !ok { - info, err = c.Strategy.Authenticate(ctx, r) - if err == nil { - // cache result - err = c.Cache.Store(h.Nonce(), info, r) + info, err := c.Strategy.Authenticate(ctx, r) + + if err != nil { + return nil, err } - } - if err != nil { - return nil, err + ext := info.Extensions() + if ext == nil { + ext = make(map[string][]string) + } + + ext[extensionKey] = []string{h.String()} + info.SetExtensions(ext) + + // cache result + err = c.Cache.Store(h.Nonce(), info, r) + + return info, err } - if _, ok := info.(auth.Info); !ok { + v, ok := info.(auth.Info) + + if !ok { return nil, errors.NewInvalidType((*auth.Info)(nil), info) } - return info.(auth.Info), nil + sh := make(Header) + _ = sh.Parse(v.Extensions()[extensionKey][0]) + + h.SetNC("00000001") + h.SetURI(r.RequestURI) + sh.SetNC("00000001") + sh.SetURI(r.RequestURI) + + if err := sh.Compare(h); err != nil { + return nil, err + } + + return v, nil } diff --git a/auth/strategies/digest/cached_test.go b/auth/strategies/digest/cached_test.go index 2e925ab..a85c96c 100644 --- a/auth/strategies/digest/cached_test.go +++ b/auth/strategies/digest/cached_test.go @@ -20,31 +20,53 @@ func TestNewCahced(t *testing.T) { name string expectedErr bool info interface{} + key string header string }{ { name: "it return error when cache load return error", expectedErr: true, - header: "error", + header: "Digest nonce=error", info: nil, }, { name: "it return error when user authenticate func return error", expectedErr: true, - header: "ignore", + header: "Digest nonce=ignore", info: nil, }, { name: "it return error when cache store return error", expectedErr: true, - header: `Digest username="store-error", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="a43f3d765159a7248cce617eaaf7c07f", opaque="1"`, + header: `Digest username="a", realm="t", nonce="store-error", uri="/", cnonce="1=", nc=00000001, qop=auth, response="14979b5053904998faf57bc72a1d7a56", opaque="1"`, info: nil, }, { name: "it return error when cache return invalid type", expectedErr: true, info: "sample-data", - header: "invalid", + header: "Digest nonce=invalid", + key: "invalid", + }, + { + name: "it authinticate user", + expectedErr: false, + info: nil, + header: `Digest username="a", realm="t", nonce="150", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22b46269226822f5edde5a52985679ad", opaque="1"`, + }, + { + name: "it authinticate user even if nc, uri changed and load it from cache", + expectedErr: false, + info: auth.NewDefaultUser( + "test", + "1", + nil, + map[string][]string{ + extensionKey: {`Digest username="a", realm="t", nonce="150", uri="/abc", cnonce="1=", nc=00000001, qop=auth, response="22b46269226822f5edde5a52985679ad", opaque="1"`}, + }, + ), + key: "150", + header: `Digest username="a", realm="t", nonce="150", uri="/", cnonce="1=", nc=00000002, qop=auth, response="22b46269226822f5edde5a52985679ad", opaque="1"`, }, } @@ -60,7 +82,7 @@ func TestNewCahced(t *testing.T) { if userName == "error" { return "", nil, fmt.Errorf("Error #L 51") } - return "", nil, nil + return "", auth.NewDefaultUser("test", "1", nil, nil), nil }, Hash: func(algo string) hash.Hash { return md5.New() @@ -71,7 +93,8 @@ func TestNewCahced(t *testing.T) { r, _ := http.NewRequest("GET", "/", nil) r.Header.Set("Authorization", tt.header) - _ = cache.Store(tt.header, tt.info, r) + + _ = cache.Store(tt.key, tt.info, r) _, err := s.Authenticate(r.Context(), r) diff --git a/auth/strategies/digest/header.go b/auth/strategies/digest/header.go index fa42bd8..191580d 100644 --- a/auth/strategies/digest/header.go +++ b/auth/strategies/digest/header.go @@ -189,6 +189,28 @@ func (h Header) WWWAuthenticate() (string, error) { return s, nil } +// Compare server header vs client header returns error if any diff found. +func (h Header) Compare(ch Header) error { + for k, v := range h { + cv := ch[k] + if cv != v { + return fmt.Errorf("Digest: %s Does not match value in provided header", k) + } + } + return nil +} + +// String describe header as a string +func (h Header) String() string { + str := "Digest " + + for k, v := range h { + str = str + k + "=" + v + ", " + } + + return str[:len(str)-2] +} + func secretKey() (string, error) { secret := make([]byte, 16) _, err := rand.Read(secret) diff --git a/auth/strategies/digest/header_test.go b/auth/strategies/digest/header_test.go index 1c5fa04..6171288 100644 --- a/auth/strategies/digest/header_test.go +++ b/auth/strategies/digest/header_test.go @@ -111,6 +111,81 @@ func TestParams(t *testing.T) { } } +//nolint: lll +func TestCompare(t *testing.T) { + table := []struct { + serverHeader string + clientHeader string + expectedErr bool + }{ + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: false, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="b", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="c", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="2", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/a", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000002, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth-i, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="1"`, + expectedErr: true, + }, + { + serverHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc13", opaque="1"`, + clientHeader: `Digest username="a", realm="t", nonce="1", uri="/", cnonce="1=", nc=00000001, qop=auth, response="22cf307b29e6318dafba1fc1d564fc12", opaque="2"`, + expectedErr: true, + }, + } + + for _, tt := range table { + sh := make(Header) + ch := make(Header) + + _ = sh.Parse(tt.serverHeader) + _ = ch.Parse(tt.clientHeader) + + err := sh.Compare(ch) + + assert.Equal(t, tt.expectedErr, err != nil) + } +} + +func TestString(t *testing.T) { + h := make(Header) + h.SetUserName("test") + h.SetNC("00000001") + + assert.Equal(t, "Digest username=test, nc=00000001", h.String()) +} + func TestParse(t *testing.T) { h := make(Header)