Improve testing and used more mature and concurrency-safe LRU cache f…

…rom hashicorp

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/gorilla/mux v1.7.3
 	github.com/gorilla/websocket v1.4.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
+	github.com/hashicorp/golang-lru v0.5.4
 	github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect
 	github.com/kylelemons/godebug v1.1.0
 	github.com/lib/pq v1.3.0

go.sum

@@ -154,6 +154,8 @@ github.com/hashicorp/go-multierror v0.0.0-20161216184304-ed905158d874/go.mod h1:
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.1 h1:0hERBMJE1eitiLkihrMvRVBYAkpHzc/J3QdDN+dAcgU=
 github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
+github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
+github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
 github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
 github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=

server/oauth2.go

@@ -21,10 +21,10 @@ import (
 	"strings"
 	"time"
 
-	"github.com/golang/groupcache/lru"
-
 	jose "gopkg.in/square/go-jose.v2"
 
+	lru "github.com/hashicorp/golang-lru"
+
 	"github.com/dexidp/dex/connector"
 	"github.com/dexidp/dex/server/internal"
 	"github.com/dexidp/dex/storage"
@@ -559,7 +559,7 @@ func (s *Server) validateCrossClientTrust(clientID, peerID string) (trusted bool
 	return false, nil
 }
 
-func validateRedirectURI(client storage.Client, wildcardCache *lru.Cache, redirectURI string) bool {
+func validateRedirectURI(client storage.Client, wildcardCache *lru.ARCCache, redirectURI string) bool {
 	if !client.Public {
 		for _, uri := range client.RedirectURIs {
 			if redirectURI == uri {
@@ -594,7 +594,7 @@ func validateRedirectURI(client storage.Client, wildcardCache *lru.Cache, redire
 	return err == nil && host == "localhost"
 }
 
-func wildcardMatch(knownRedirectUri string, requestedRedirectUri string, wildcardCache *lru.Cache) bool {
+func wildcardMatch(knownRedirectUri string, requestedRedirectUri string, wildcardCache *lru.ARCCache) bool {
 	if wildcardCache == nil {
 		return false
 	}

server/oauth2_test.go

@@ -10,7 +10,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/golang/groupcache/lru"
+	"github.com/hashicorp/golang-lru"
 	"gopkg.in/square/go-jose.v2"
 
 	"github.com/dexidp/dex/storage"
@@ -286,6 +286,20 @@ func TestValidRedirectURI(t *testing.T) {
 			redirectURI: "http://abc.foo.com/bar",
 			wantValid:   true,
 		},
+		{
+			client: storage.Client{
+				RedirectURIs: []string{"http://abc.foo.com/bar"},
+			},
+			redirectURI: "http://abc.foo.com/bar",
+			wantValid:   true,
+		},
+		{
+			client: storage.Client{
+				RedirectURIs: []string{"http://b*.foo.com/bar"},
+			},
+			redirectURI: "http://abc.foo.com/bar",
+			wantValid:   false,
+		},
 		{
 			// URL must use http: or https: protocol
 			client: storage.Client{
@@ -374,15 +388,36 @@ func TestValidRedirectURI(t *testing.T) {
 		},
 	}
 
-	wildcardCache := lru.New(2)
+	maxCacheSize := 3
+	wildcardCache, _ := lru.NewARC(maxCacheSize)
 
-	for _, test := range tests {
+	if maxCacheSize >= len(tests) {
+		t.Errorf("cache tests pre-condition is that mac cache size is less than test suite size" +
+			" cacheMax=%d testItemsSize=%d", maxCacheSize, len(tests))
+	}
 
+	var firstWildcardCache = ""
+	for i, test := range tests {
 		got := validateRedirectURI(test.client, wildcardCache, test.redirectURI)
 		if got != test.wantValid {
 			t.Errorf("client=%#v, redirectURI=%q, wanted valid=%t, got=%t",
 				test.client, test.redirectURI, test.wantValid, got)
 		}
+
+		// expect redirectURI last used should be stored in cache
+		if !test.client.Public && len(test.client.RedirectURIs) > 0 && test.redirectURI != test.client.RedirectURIs[0] {
+			if !wildcardCache.Contains(test.client.RedirectURIs[0]) {
+				t.Errorf("test[%d] should contain redirectURI as cache key %s", i, test.client.RedirectURIs[0])
+			}
+			if len(firstWildcardCache) == 0 {
+				firstWildcardCache = test.client.RedirectURIs[0]
+			}
+		}
+	}
+
+	if wildcardCache.Contains(firstWildcardCache) {
+		// check eviction policed
+		t.Errorf("first redirectURI should have been evicted from cache %s", tests[0].client.RedirectURIs[0])
 	}
 }
 

server/server.go

@@ -15,12 +15,13 @@ import (
 	"time"
 
 	"github.com/felixge/httpsnoop"
-	"github.com/golang/groupcache/lru"
 	"github.com/gorilla/handlers"
 	"github.com/gorilla/mux"
 	"github.com/prometheus/client_golang/prometheus"
 	"golang.org/x/crypto/bcrypt"
 
+	lru "github.com/hashicorp/golang-lru"
+
 	"github.com/dexidp/dex/connector"
 	"github.com/dexidp/dex/connector/atlassiancrowd"
 	"github.com/dexidp/dex/connector/authproxy"
@@ -163,7 +164,7 @@ type Server struct {
 	logger log.Logger
 
 	// LRU cache of regex matchers for a given permitted redirect URI defined in Client
-	wildcardMatcherCache *lru.Cache
+	wildcardMatcherCache *lru.ARCCache
 }
 
 // NewServer constructs a server from the provided config.
@@ -216,6 +217,12 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 		now = time.Now
 	}
 
+	arcCache, err := lru.NewARC(500)
+	if err != nil {
+		c.Logger.Infof("ARC LRU case failed to load, wildcard redirect URIs disabled: %v", err)
+	}
+
+
 	s := &Server{
 		issuerURL:              *issuerURL,
 		connectors:             make(map[string]Connector),
@@ -229,7 +236,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 		templates:              tmpls,
 		passwordConnector:      c.PasswordConnector,
 		logger:                 c.Logger,
-		wildcardMatcherCache:   lru.New(500),
+		wildcardMatcherCache:   arcCache,
 	}
 
 	// Retrieves connector objects in backend storage. This list includes the static connectors