Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Handle case where verifier Nonce func is nil #594

Merged
merged 2 commits into from May 2, 2024
Merged

fix: Handle case where verifier Nonce func is nil #594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f22732
Skip nonce check if verifier nonce func is nil
ymarcus93 Apr 23, 2024
883c156
add unit test
ymarcus93 Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions pkg/client/rp/verifier.go
View file
Open in desktop
Expand Up @@ -73,8 +73,10 @@
return nilClaims, err
}

if err = oidc.CheckNonce(claims, v.Nonce(ctx)); err != nil {
return nilClaims, err
if v.Nonce != nil {
if err = oidc.CheckNonce(claims, v.Nonce(ctx)); err != nil {
return nilClaims, err

Check warning on line 78 in pkg/client/rp/verifier.go

View check run for this annotation

Codecov / codecov/patch

pkg/client/rp/verifier.go#L78 

Added line #L78 was not covered by tests
}
}

if err = oidc.CheckAuthorizationContextClassReference(claims, v.ACR); err != nil {
Expand Down
60 changes: 34 additions & 26 deletions pkg/client/rp/verifier_test.go
View file
Open in desktop
Expand Up @@ -100,22 +100,21 @@ func TestVerifyIDToken(t *testing.T) {
MaxAge: 2 * time.Minute,
ACR: tu.ACRVerify,
Nonce: func(context.Context) string { return tu.ValidNonce },
ClientID: tu.ValidClientID,
}

tests := []struct {
name string
clientID string
tokenClaims func() (string, *oidc.IDTokenClaims)
wantErr bool
name string
tokenClaims func() (string, *oidc.IDTokenClaims)
customVerifier func(verifier *IDTokenVerifier)
wantErr bool
}{
{
name: "success",
clientID: tu.ValidClientID,
tokenClaims: tu.ValidIDToken,
},
{
name: "custom claims",
clientID: tu.ValidClientID,
name: "custom claims",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDTokenCustom(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
Expand All @@ -125,21 +124,31 @@ func TestVerifyIDToken(t *testing.T) {
)
},
},
{
name: "skip nonce check",
customVerifier: func(verifier *IDTokenVerifier) {
verifier.Nonce = nil
},
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
tu.ValidExpiration, tu.ValidAuthTime, "foo",
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
)
},
},
{
name: "parse err",
clientID: tu.ValidClientID,
tokenClaims: func() (string, *oidc.IDTokenClaims) { return "~~~~", nil },
wantErr: true,
},
{
name: "invalid signature",
clientID: tu.ValidClientID,
tokenClaims: func() (string, *oidc.IDTokenClaims) { return tu.InvalidSignatureToken, nil },
wantErr: true,
},
{
name: "empty subject",
clientID: tu.ValidClientID,
name: "empty subject",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, "", tu.ValidAudience,
Expand All @@ -150,8 +159,7 @@ func TestVerifyIDToken(t *testing.T) {
wantErr: true,
},
{
name: "wrong issuer",
clientID: tu.ValidClientID,
name: "wrong issuer",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
"foo", tu.ValidSubject, tu.ValidAudience,
Expand All @@ -162,14 +170,15 @@ func TestVerifyIDToken(t *testing.T) {
wantErr: true,
},
{
name: "wrong clientID",
clientID: "foo",
name: "wrong clientID",
customVerifier: func(verifier *IDTokenVerifier) {
verifier.ClientID = "foo"
},
tokenClaims: tu.ValidIDToken,
wantErr: true,
},
{
name: "expired",
clientID: tu.ValidClientID,
name: "expired",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
Expand All @@ -180,8 +189,7 @@ func TestVerifyIDToken(t *testing.T) {
wantErr: true,
},
{
name: "wrong IAT",
clientID: tu.ValidClientID,
name: "wrong IAT",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
Expand All @@ -192,8 +200,7 @@ func TestVerifyIDToken(t *testing.T) {
wantErr: true,
},
{
name: "wrong acr",
clientID: tu.ValidClientID,
name: "wrong acr",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
Expand All @@ -204,8 +211,7 @@ func TestVerifyIDToken(t *testing.T) {
wantErr: true,
},
{
name: "expired auth",
clientID: tu.ValidClientID,
name: "expired auth",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
Expand All @@ -216,8 +222,7 @@ func TestVerifyIDToken(t *testing.T) {
wantErr: true,
},
{
name: "wrong nonce",
clientID: tu.ValidClientID,
name: "wrong nonce",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
Expand All @@ -231,7 +236,10 @@ func TestVerifyIDToken(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
token, want := tt.tokenClaims()
verifier.ClientID = tt.clientID
if tt.customVerifier != nil {
tt.customVerifier(verifier)
}

got, err := VerifyIDToken[*oidc.IDTokenClaims](context.Background(), token, verifier)
if tt.wantErr {
assert.Error(t, err)
Expand Down