Skip to content

Commit

Permalink
Merge pull request #13 from devtron-labs/auth-fix
Browse files Browse the repository at this point in the history
fix: error handling in verifyAppState (/auth/callback API)
  • Loading branch information
prkhrkat authored Oct 14, 2024
2 parents 30a0275 + 178a13f commit 87207db
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
5 changes: 3 additions & 2 deletions authenticator/client/oidcClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"net/http"
"net/url"
"path"
"sync"
"time"
)

Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 15 additions & 4 deletions authenticator/oidc/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"path"
"regexp"
"strings"
"sync"
"time"

gooidc "github.com/coreos/go-oidc/v3/oidc"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 87207db

Please sign in to comment.