Skip to content

Commit

Permalink
fix: log all token verification failures (argoproj#16625)
Browse files Browse the repository at this point in the history
* fix: log all token verification failures

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* better

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Kevin Lyda <kevin@lyda.ie>
  • Loading branch information
crenshaw-dev authored and lyda committed Mar 28, 2024
1 parent 0e5c73c commit 2b87291
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions util/oidc/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ func (p *providerImpl) newGoOIDCProvider() (*gooidc.Provider, error) {
return prov, nil
}

type tokenVerificationError struct {
errorsByAudience map[string]error
}

func (t tokenVerificationError) Error() string {
var errorStrings []string
for aud, err := range t.errorsByAudience {
errorStrings = append(errorStrings, fmt.Sprintf("error for aud %q: %v", aud, err))
}
return fmt.Sprintf("token verification failed for all audiences: %s", strings.Join(errorStrings, ", "))
}

func (p *providerImpl) Verify(tokenString string, argoSettings *settings.ArgoCDSettings) (*gooidc.IDToken, error) {
// According to the JWT spec, the aud claim is optional. The spec also says (emphasis mine):
//
Expand Down Expand Up @@ -104,6 +116,7 @@ func (p *providerImpl) Verify(tokenString string, argoSettings *settings.ArgoCDS
if len(allowedAudiences) == 0 {
return nil, errors.New("token has an audience claim, but no allowed audiences are configured")
}
tokenVerificationErrors := make(map[string]error)
// Token must be verified for at least one allowed audience
for _, aud := range allowedAudiences {
idToken, err = p.verify(aud, tokenString, false)
Expand All @@ -117,6 +130,13 @@ func (p *providerImpl) Verify(tokenString string, argoSettings *settings.ArgoCDS
if err == nil {
break
}
// We store the error for each audience so that we can return a more detailed error message to the user.
// If this gets merged, we'll be able to detect failures unrelated to audiences and short-circuit this loop
// to avoid logging irrelevant warnings: https://github.com/coreos/go-oidc/pull/406
tokenVerificationErrors[aud] = err
}
if len(tokenVerificationErrors) > 0 {
err = tokenVerificationError{errorsByAudience: tokenVerificationErrors}
}
}

Expand Down

0 comments on commit 2b87291

Please sign in to comment.