Skip to content

Commit

Permalink
oidc refresh token: validate nonce only if set
Browse files Browse the repository at this point in the history
As clarified in OpenID core spec errata 2, section 12.2

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
  • Loading branch information
drakkan committed Jul 1, 2024
1 parent 636a1c2 commit 55169eb
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
10 changes: 6 additions & 4 deletions internal/httpd/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package httpd

import (
"context"
"encoding/hex"
"errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -203,7 +204,7 @@ type oidcPendingAuth struct {
func newOIDCPendingAuth(audience tokenAudience) oidcPendingAuth {
return oidcPendingAuth{
State: xid.New().String(),
Nonce: xid.New().String(),
Nonce: hex.EncodeToString(util.GenerateRandomBytes(20)),
Audience: audience,
IssuedAt: util.GetTimeAsMsSinceEpoch(time.Now()),
}
Expand Down Expand Up @@ -345,14 +346,15 @@ func (t *oidcToken) refresh(ctx context.Context, config OAuth2Config, verifier O
logger.Debug(logSender, "", "unable to verify refreshed id token for cookie %q: %v", t.Cookie, err)
return err
}
if idToken.Nonce != t.Nonce {
logger.Debug(logSender, "", "unable to verify refreshed id token for cookie %q: nonce mismatch", t.Cookie)
if idToken.Nonce != "" && idToken.Nonce != t.Nonce {
logger.Warn(logSender, "", "unable to verify refreshed id token for cookie %q: nonce mismatch, expected: %q, actual: %q",
t.Cookie, t.Nonce, idToken.Nonce)
return errors.New("the refreshed token nonce mismatch")
}
claims := make(map[string]any)
err = idToken.Claims(&claims)
if err != nil {
logger.Debug(logSender, "", "unable to get refreshed id token claims for cookie %q: %v", t.Cookie, err)
logger.Warn(logSender, "", "unable to get refreshed id token claims for cookie %q: %v", t.Cookie, err)
return err
}
sid, ok := claims["sid"].(string)
Expand Down
6 changes: 4 additions & 2 deletions internal/httpd/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,15 +627,17 @@ func TestOIDCRefreshToken(t *testing.T) {
},
}
verifier = mockOIDCVerifier{
token: &oidc.IDToken{},
token: &oidc.IDToken{
Nonce: xid.New().String(), // nonce is different from the expected one
},
}
err = token.refresh(context.Background(), &config, &verifier, r)
if assert.Error(t, err) {
assert.Contains(t, err.Error(), "the refreshed token nonce mismatch")
}
verifier = mockOIDCVerifier{
token: &oidc.IDToken{
Nonce: token.Nonce,
Nonce: "", // empty token is fine on refresh but claims are not set
},
}
err = token.refresh(context.Background(), &config, &verifier, r)
Expand Down

0 comments on commit 55169eb

Please sign in to comment.