From b7afb0d501f01e96ead295112de8d87c50fa54d8 Mon Sep 17 00:00:00 2001 From: prakhar katiyar Date: Mon, 14 Oct 2024 17:18:47 +0530 Subject: [PATCH] fix: error handling in verifyAppState (/auth/callback API) --- authenticator/client/oidcClient.go | 5 +++-- authenticator/oidc/oidc.go | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/authenticator/client/oidcClient.go b/authenticator/client/oidcClient.go index 4c65a6ae9..b7dcba2f9 100644 --- a/authenticator/client/oidcClient.go +++ b/authenticator/client/oidcClient.go @@ -26,6 +26,7 @@ import ( "net/http" "net/url" "path" + "sync" "time" ) @@ -65,8 +66,8 @@ func getOidcClient(dexServerAddress string, settings *oidc.Settings, userVerifie }, } dexProxy := oidc.NewDexHTTPReverseProxy(dexServerAddress, dexClient.Transport) - cahecStore := &oidc.Cache{OidcState: map[string]*oidc.OIDCState{}} - oidcClient, err := oidc.NewClientApp(settings, cahecStore, "/", userVerifier, RedirectUrlSanitiser) + cacheStore := &oidc.Cache{OidcState: sync.Map{}} + oidcClient, err := oidc.NewClientApp(settings, cacheStore, "/", userVerifier, RedirectUrlSanitiser) if err != nil { return nil, nil, err } diff --git a/authenticator/oidc/oidc.go b/authenticator/oidc/oidc.go index 4ffdeb62f..a44e53c2c 100644 --- a/authenticator/oidc/oidc.go +++ b/authenticator/oidc/oidc.go @@ -32,6 +32,7 @@ import ( "path" "regexp" "strings" + "sync" "time" gooidc "github.com/coreos/go-oidc/v3/oidc" @@ -69,16 +70,23 @@ type OIDCStateStorage interface { } type Cache struct { - OidcState map[string]*OIDCState + OidcState sync.Map } func (c *Cache) GetOIDCState(key string) (*OIDCState, error) { - state := c.OidcState[key] + value, exists := c.OidcState.Load(key) + if !exists { + return nil, ErrCacheMiss + } + state, ok := value.(*OIDCState) + if !ok || state == nil { + return nil, ErrInvalidState + } return state, nil } func (c *Cache) SetOIDCState(key string, state *OIDCState) error { - c.OidcState[key] = state + c.OidcState.Store(key, state) return nil } @@ -287,12 +295,15 @@ func (a *ClientApp) generateAppState(returnURL string) string { } var ErrCacheMiss = errors.New("cache: key is missing") +var ErrInvalidState = errors.New("invalid app state") func (a *ClientApp) verifyAppState(state string) (*OIDCState, error) { res, err := a.cache.GetOIDCState(state) if err != nil { - if err == ErrCacheMiss { + if errors.Is(err, ErrCacheMiss) { return nil, fmt.Errorf("unknown app state %s", state) + } else if errors.Is(err, ErrInvalidState) { + return nil, fmt.Errorf("invalid app state %s", state) } else { return nil, fmt.Errorf("failed to verify app state %s: %v", state, err) }