From 64a2f7aa4feffa66dff54de17010e309560acb57 Mon Sep 17 00:00:00 2001
From: Nicola Murino <nicola.murino@gmail.com>
Date: Mon, 1 Jul 2024 19:06:11 +0200
Subject: [PATCH] oidc refresh token: validate nonce only if set

As clarified in OpenID core spec errata 2, section 12.2

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
---
 internal/httpd/oidc.go      | 10 ++++++----
 internal/httpd/oidc_test.go |  6 ++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go
index 3b127f6e..bf4027b1 100644
--- a/internal/httpd/oidc.go
+++ b/internal/httpd/oidc.go
@@ -16,6 +16,7 @@ package httpd
 
 import (
 	"context"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"net/http"
@@ -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()),
 	}
@@ -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)
diff --git a/internal/httpd/oidc_test.go b/internal/httpd/oidc_test.go
index efd9a6b2..ec59b988 100644
--- a/internal/httpd/oidc_test.go
+++ b/internal/httpd/oidc_test.go
@@ -626,7 +626,9 @@ 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) {
@@ -634,7 +636,7 @@ func TestOIDCRefreshToken(t *testing.T) {
 	}
 	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)