From e8113e3770086df38f78add37139d04e00f502a4 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 21 Oct 2020 13:05:19 -0500 Subject: [PATCH] Add basic caching framework to ./internal/oidclient package. Signed-off-by: Matt Moyer --- internal/oidcclient/login.go | 41 ++++++++++++++++ internal/oidcclient/login_test.go | 82 +++++++++++++++++++++++++++++++ internal/oidcclient/types.go | 13 +++++ 3 files changed, 136 insertions(+) diff --git a/internal/oidcclient/login.go b/internal/oidcclient/login.go index 2de838df..90e8c1a9 100644 --- a/internal/oidcclient/login.go +++ b/internal/oidcclient/login.go @@ -10,6 +10,7 @@ import ( "net" "net/http" "net/url" + "sort" "time" "github.com/coreos/go-oidc" @@ -24,12 +25,20 @@ import ( "go.pinniped.dev/internal/oidcclient/state" ) +const ( + // minIDTokenValidity is the minimum amount of time that a cached ID token must be still be valid to be considered. + // This is non-zero to ensure that most of the time, your ID token won't expire in the middle of a multi-step k8s + // API operation. + minIDTokenValidity = 10 * time.Minute +) + type handlerState struct { // Basic parameters. ctx context.Context issuer string clientID string scopes []string + cache SessionCache // Parameters of the localhost listener. listenAddr string @@ -100,6 +109,20 @@ func WithBrowserOpen(openURL func(url string) error) Option { } } +// WithSessionCache sets the session cache backend for storing and retrieving previously-issued ID tokens and refresh tokens. +func WithSessionCache(cache SessionCache) Option { + return func(h *handlerState) error { + h.cache = cache + return nil + } +} + +// nopCache is a SessionCache that doesn't actually do anything. +type nopCache struct{} + +func (*nopCache) GetToken(SessionCacheKey) *Token { return nil } +func (*nopCache) PutToken(SessionCacheKey, *Token) {} + // Login performs an OAuth2/OIDC authorization code login using a localhost listener. func Login(issuer string, clientID string, opts ...Option) (*Token, error) { h := handlerState{ @@ -107,6 +130,7 @@ func Login(issuer string, clientID string, opts ...Option) (*Token, error) { clientID: clientID, listenAddr: "localhost:0", scopes: []string{"offline_access", "openid", "email", "profile"}, + cache: &nopCache{}, callbackPath: "/callback", ctx: context.Background(), callbacks: make(chan callbackResult), @@ -143,6 +167,22 @@ func Login(issuer string, clientID string, opts ...Option) (*Token, error) { return nil, err } + // Check the cache for a previous session issued with the same parameters. + sort.Strings(h.scopes) + cacheKey := SessionCacheKey{ + Issuer: h.issuer, + ClientID: h.clientID, + Scopes: h.scopes, + RedirectURI: (&url.URL{Scheme: "http", Host: h.listenAddr, Path: h.callbackPath}).String(), + } + + // If the ID token is still valid for a bit, return it immediately and skip the rest of the flow. + if cached := h.cache.GetToken(cacheKey); cached != nil && + cached.IDToken != nil && + time.Until(cached.IDToken.Expiry.Time) > minIDTokenValidity { + return cached, nil + } + // Perform OIDC discovery. provider, err := oidc.NewProvider(h.ctx, h.issuer) if err != nil { @@ -204,6 +244,7 @@ func Login(issuer string, clientID string, opts ...Option) (*Token, error) { if callback.err != nil { return nil, fmt.Errorf("error handling callback: %w", callback.err) } + h.cache.PutToken(cacheKey, callback.token) return callback.token, nil } } diff --git a/internal/oidcclient/login_test.go b/internal/oidcclient/login_test.go index e01b4435..1271b98d 100644 --- a/internal/oidcclient/login_test.go +++ b/internal/oidcclient/login_test.go @@ -27,6 +27,27 @@ import ( "go.pinniped.dev/internal/oidcclient/state" ) +// mockSessionCache exists to avoid an import cycle if we generate mocks into another package. +type mockSessionCache struct { + t *testing.T + getReturnsToken *Token + sawGetKeys []SessionCacheKey + sawPutKeys []SessionCacheKey + sawPutTokens []*Token +} + +func (m *mockSessionCache) GetToken(key SessionCacheKey) *Token { + m.t.Logf("saw mock session cache GetToken() with client ID %s", key.ClientID) + m.sawGetKeys = append(m.sawGetKeys, key) + return m.getReturnsToken +} + +func (m *mockSessionCache) PutToken(key SessionCacheKey, token *Token) { + m.t.Logf("saw mock session cache PutToken() with client ID %s and ID token %s", key.ClientID, token.IDToken.Token) + m.sawPutKeys = append(m.sawPutKeys, key) + m.sawPutTokens = append(m.sawPutTokens, token) +} + func TestLogin(t *testing.T) { time1 := time.Date(3020, 10, 12, 13, 14, 15, 16, time.UTC) testToken := Token{ @@ -116,6 +137,53 @@ func TestLogin(t *testing.T) { }, wantErr: "some error generating PKCE", }, + { + name: "session cache hit but token expired", + issuer: "test-issuer", + clientID: "test-client-id", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + cache := &mockSessionCache{t: t, getReturnsToken: &Token{ + IDToken: &IDToken{ + Token: "test-id-token", + Expiry: metav1.NewTime(time.Now()), // less than Now() + minIDTokenValidity + }, + }} + t.Cleanup(func() { + require.Equal(t, []SessionCacheKey{{ + Issuer: "test-issuer", + ClientID: "test-client-id", + Scopes: []string{"test-scope"}, + RedirectURI: "http://localhost:0/callback", + }}, cache.sawGetKeys) + require.Empty(t, cache.sawPutTokens) + }) + return WithSessionCache(cache)(h) + } + }, + wantErr: `could not perform OIDC discovery for "test-issuer": Get "test-issuer/.well-known/openid-configuration": unsupported protocol scheme ""`, + }, + { + name: "session cache hit with valid token", + issuer: "test-issuer", + clientID: "test-client-id", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + cache := &mockSessionCache{t: t, getReturnsToken: &testToken} + t.Cleanup(func() { + require.Equal(t, []SessionCacheKey{{ + Issuer: "test-issuer", + ClientID: "test-client-id", + Scopes: []string{"test-scope"}, + RedirectURI: "http://localhost:0/callback", + }}, cache.sawGetKeys) + require.Empty(t, cache.sawPutTokens) + }) + return WithSessionCache(cache)(h) + } + }, + wantToken: &testToken, + }, { name: "discovery failure", opt: func(t *testing.T) Option { @@ -187,6 +255,20 @@ func TestLogin(t *testing.T) { h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } + cache := &mockSessionCache{t: t, getReturnsToken: nil} + cacheKey := SessionCacheKey{ + Issuer: successServer.URL, + ClientID: "test-client-id", + Scopes: []string{"test-scope"}, + RedirectURI: "http://localhost:0/callback", + } + t.Cleanup(func() { + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawPutKeys) + require.Equal(t, []*Token{&testToken}, cache.sawPutTokens) + }) + require.NoError(t, WithSessionCache(cache)(h)) + h.openURL = func(actualURL string) error { parsedActualURL, err := url.Parse(actualURL) require.NoError(t, err) diff --git a/internal/oidcclient/types.go b/internal/oidcclient/types.go index a4a0f419..7fbf3a3f 100644 --- a/internal/oidcclient/types.go +++ b/internal/oidcclient/types.go @@ -47,3 +47,16 @@ type Token struct { // IDToken is an OpenID Connect ID token. IDToken *IDToken `json:"id,omitempty"` } + +// SessionCacheKey contains the data used to select a valid session cache entry. +type SessionCacheKey struct { + Issuer string `json:"issuer"` + ClientID string `json:"clientID"` + Scopes []string `json:"scopes"` + RedirectURI string `json:"redirect_uri"` +} + +type SessionCache interface { + GetToken(SessionCacheKey) *Token + PutToken(SessionCacheKey, *Token) +}