Skip to content

Commit

Permalink
fix: allow expired ID token hint to end sessions (#522)
Browse files Browse the repository at this point in the history
* fix: allow expired ID token hint to end sessions

This change adds a specific error for expired ID Token hints, including too old "issued at" and "max auth age".
The error is returned VerifyIDTokenHint so that the end session handler can choose to ignore this error.

This fixes the behavior to be in line with [OpenID Connect RP-Initiated Logout 1.0, section 4](https://openid.net/specs/openid-connect-rpinitiated-1_0.html#ValidationAndErrorHandling).

* Tes IDTokenHintExpiredError
  • Loading branch information
muhlemmer authored Jan 19, 2024
1 parent 3f26eb1 commit b8e520a
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 31 deletions.
2 changes: 1 addition & 1 deletion pkg/oidc/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var (
ErrNonceInvalid = errors.New("nonce does not match")
ErrAcrInvalid = errors.New("acr is invalid")
ErrAuthTimeNotPresent = errors.New("claim `auth_time` of token is missing")
ErrAuthTimeToOld = errors.New("auth time of token is to old")
ErrAuthTimeToOld = errors.New("auth time of token is too old")
ErrAtHash = errors.New("at_hash does not correspond to access token")
)

Expand Down
3 changes: 2 additions & 1 deletion pkg/op/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package op

import (
"context"
"errors"
"net/http"
"net/url"
"path"
Expand Down Expand Up @@ -68,7 +69,7 @@ func ValidateEndSessionRequest(ctx context.Context, req *oidc.EndSessionRequest,
}
if req.IdTokenHint != "" {
claims, err := VerifyIDTokenHint[*oidc.IDTokenClaims](ctx, req.IdTokenHint, ender.IDTokenHintVerifier(ctx))
if err != nil {
if err != nil && !errors.As(err, &IDTokenHintExpiredError{}) {
return nil, oidc.ErrInvalidRequest().WithDescription("id_token_hint invalid").WithParent(err)
}
session.UserID = claims.GetSubject()
Expand Down
30 changes: 23 additions & 7 deletions pkg/op/verifier_id_token_hint.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package op

import (
"context"
"errors"

"github.com/zitadel/oidc/v3/pkg/oidc"
)
Expand All @@ -27,8 +28,23 @@ func NewIDTokenHintVerifier(issuer string, keySet oidc.KeySet, opts ...IDTokenHi
return verifier
}

type IDTokenHintExpiredError struct {
error
}

func (e IDTokenHintExpiredError) Unwrap() error {
return e.error
}

func (e IDTokenHintExpiredError) Is(err error) bool {
return errors.Is(err, e.error)
}

// VerifyIDTokenHint validates the id token according to
// https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
// https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation.
// In case of an expired token both the Claims and first encountered expiry related error
// is returned of type [IDTokenHintExpiredError]. In that case the caller can choose to still
// trust the token for cases like logout, as signature and other verifications succeeded.
func VerifyIDTokenHint[C oidc.Claims](ctx context.Context, token string, v *IDTokenHintVerifier) (claims C, err error) {
var nilClaims C

Expand All @@ -49,20 +65,20 @@ func VerifyIDTokenHint[C oidc.Claims](ctx context.Context, token string, v *IDTo
return nilClaims, err
}

if err = oidc.CheckExpiration(claims, v.Offset); err != nil {
if err = oidc.CheckAuthorizationContextClassReference(claims, v.ACR); err != nil {
return nilClaims, err
}

if err = oidc.CheckIssuedAt(claims, v.MaxAgeIAT, v.Offset); err != nil {
return nilClaims, err
if err = oidc.CheckExpiration(claims, v.Offset); err != nil {
return claims, IDTokenHintExpiredError{err}
}

if err = oidc.CheckAuthorizationContextClassReference(claims, v.ACR); err != nil {
return nilClaims, err
if err = oidc.CheckIssuedAt(claims, v.MaxAgeIAT, v.Offset); err != nil {
return claims, IDTokenHintExpiredError{err}
}

if err = oidc.CheckAuthTime(claims, v.MaxAge); err != nil {
return nilClaims, err
return claims, IDTokenHintExpiredError{err}
}
return claims, nil
}
55 changes: 33 additions & 22 deletions pkg/op/verifier_id_token_hint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package op

import (
"context"
"errors"
"testing"
"time"

Expand Down Expand Up @@ -57,6 +58,13 @@ func TestNewIDTokenHintVerifier(t *testing.T) {
}
}

func Test_IDTokenHintExpiredError(t *testing.T) {
var err error = IDTokenHintExpiredError{oidc.ErrExpired}
assert.True(t, errors.Unwrap(err) == oidc.ErrExpired)
assert.ErrorIs(t, err, oidc.ErrExpired)
assert.ErrorAs(t, err, &IDTokenHintExpiredError{})
}

func TestVerifyIDTokenHint(t *testing.T) {
verifier := &IDTokenHintVerifier{
Issuer: tu.ValidIssuer,
Expand All @@ -71,21 +79,23 @@ func TestVerifyIDTokenHint(t *testing.T) {
tests := []struct {
name string
tokenClaims func() (string, *oidc.IDTokenClaims)
wantErr bool
wantClaims bool
wantErr error
}{
{
name: "success",
tokenClaims: tu.ValidIDToken,
wantClaims: true,
},
{
name: "parse err",
tokenClaims: func() (string, *oidc.IDTokenClaims) { return "~~~~", nil },
wantErr: true,
wantErr: oidc.ErrParse,
},
{
name: "invalid signature",
tokenClaims: func() (string, *oidc.IDTokenClaims) { return tu.InvalidSignatureToken, nil },
wantErr: true,
wantErr: oidc.ErrSignatureUnsupportedAlg,
},
{
name: "wrong issuer",
Expand All @@ -96,40 +106,42 @@ func TestVerifyIDTokenHint(t *testing.T) {
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
)
},
wantErr: true,
wantErr: oidc.ErrIssuerInvalid,
},
{
name: "expired",
name: "wrong acr",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
tu.ValidExpiration.Add(-time.Hour), tu.ValidAuthTime, tu.ValidNonce,
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
tu.ValidExpiration, tu.ValidAuthTime, tu.ValidNonce,
"else", tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
)
},
wantErr: true,
wantErr: oidc.ErrAcrInvalid,
},
{
name: "wrong IAT",
name: "expired",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
tu.ValidExpiration, tu.ValidAuthTime, tu.ValidNonce,
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, -time.Hour, "",
tu.ValidExpiration.Add(-time.Hour), tu.ValidAuthTime, tu.ValidNonce,
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
)
},
wantErr: true,
wantClaims: true,
wantErr: IDTokenHintExpiredError{oidc.ErrExpired},
},
{
name: "wrong acr",
name: "IAT too old",
tokenClaims: func() (string, *oidc.IDTokenClaims) {
return tu.NewIDToken(
tu.ValidIssuer, tu.ValidSubject, tu.ValidAudience,
tu.ValidExpiration, tu.ValidAuthTime, tu.ValidNonce,
"else", tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, time.Hour, "",
)
},
wantErr: true,
wantClaims: true,
wantErr: IDTokenHintExpiredError{oidc.ErrIatToOld},
},
{
name: "expired auth",
Expand All @@ -140,22 +152,21 @@ func TestVerifyIDTokenHint(t *testing.T) {
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
)
},
wantErr: true,
wantClaims: true,
wantErr: IDTokenHintExpiredError{oidc.ErrAuthTimeToOld},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
token, want := tt.tokenClaims()

got, err := VerifyIDTokenHint[*oidc.IDTokenClaims](context.Background(), token, verifier)
if tt.wantErr {
assert.Error(t, err)
assert.Nil(t, got)
require.ErrorIs(t, err, tt.wantErr)
if tt.wantClaims {
assert.Equal(t, got, want, "claims")
return
}
require.NoError(t, err)
require.NotNil(t, got)
assert.Equal(t, got, want)
assert.Nil(t, got, "claims")
})
}
}

0 comments on commit b8e520a

Please sign in to comment.