fix: allow expired ID token hint to end sessions (#522)

* 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
This commit is contained in:
Tim Möhlmann 2024-01-19 12:30:51 +02:00 committed by GitHub
parent 3f26eb10ad
commit b8e520afd0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 74 additions and 46 deletions

View file

@ -57,7 +57,7 @@ var (
ErrNonceInvalid = errors.New("nonce does not match") ErrNonceInvalid = errors.New("nonce does not match")
ErrAcrInvalid = errors.New("acr is invalid") ErrAcrInvalid = errors.New("acr is invalid")
ErrAuthTimeNotPresent = errors.New("claim `auth_time` of token is missing") 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") ErrAtHash = errors.New("at_hash does not correspond to access token")
) )

View file

@ -2,6 +2,7 @@ package op
import ( import (
"context" "context"
"errors"
"net/http" "net/http"
"net/url" "net/url"
"path" "path"
@ -68,7 +69,7 @@ func ValidateEndSessionRequest(ctx context.Context, req *oidc.EndSessionRequest,
} }
if req.IdTokenHint != "" { if req.IdTokenHint != "" {
claims, err := VerifyIDTokenHint[*oidc.IDTokenClaims](ctx, req.IdTokenHint, ender.IDTokenHintVerifier(ctx)) 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) return nil, oidc.ErrInvalidRequest().WithDescription("id_token_hint invalid").WithParent(err)
} }
session.UserID = claims.GetSubject() session.UserID = claims.GetSubject()

View file

@ -2,6 +2,7 @@ package op
import ( import (
"context" "context"
"errors"
"github.com/zitadel/oidc/v3/pkg/oidc" "github.com/zitadel/oidc/v3/pkg/oidc"
) )
@ -27,8 +28,23 @@ func NewIDTokenHintVerifier(issuer string, keySet oidc.KeySet, opts ...IDTokenHi
return verifier 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 // 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) { func VerifyIDTokenHint[C oidc.Claims](ctx context.Context, token string, v *IDTokenHintVerifier) (claims C, err error) {
var nilClaims C var nilClaims C
@ -49,20 +65,20 @@ func VerifyIDTokenHint[C oidc.Claims](ctx context.Context, token string, v *IDTo
return nilClaims, err return nilClaims, err
} }
if err = oidc.CheckExpiration(claims, v.Offset); err != nil {
return nilClaims, err
}
if err = oidc.CheckIssuedAt(claims, v.MaxAgeIAT, v.Offset); err != nil {
return nilClaims, err
}
if err = oidc.CheckAuthorizationContextClassReference(claims, v.ACR); err != nil { if err = oidc.CheckAuthorizationContextClassReference(claims, v.ACR); err != nil {
return nilClaims, err return nilClaims, err
} }
if err = oidc.CheckExpiration(claims, v.Offset); err != nil {
return claims, IDTokenHintExpiredError{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 { if err = oidc.CheckAuthTime(claims, v.MaxAge); err != nil {
return nilClaims, err return claims, IDTokenHintExpiredError{err}
} }
return claims, nil return claims, nil
} }

View file

@ -2,6 +2,7 @@ package op
import ( import (
"context" "context"
"errors"
"testing" "testing"
"time" "time"
@ -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) { func TestVerifyIDTokenHint(t *testing.T) {
verifier := &IDTokenHintVerifier{ verifier := &IDTokenHintVerifier{
Issuer: tu.ValidIssuer, Issuer: tu.ValidIssuer,
@ -71,21 +79,23 @@ func TestVerifyIDTokenHint(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
tokenClaims func() (string, *oidc.IDTokenClaims) tokenClaims func() (string, *oidc.IDTokenClaims)
wantErr bool wantClaims bool
wantErr error
}{ }{
{ {
name: "success", name: "success",
tokenClaims: tu.ValidIDToken, tokenClaims: tu.ValidIDToken,
wantClaims: true,
}, },
{ {
name: "parse err", name: "parse err",
tokenClaims: func() (string, *oidc.IDTokenClaims) { return "~~~~", nil }, tokenClaims: func() (string, *oidc.IDTokenClaims) { return "~~~~", nil },
wantErr: true, wantErr: oidc.ErrParse,
}, },
{ {
name: "invalid signature", name: "invalid signature",
tokenClaims: func() (string, *oidc.IDTokenClaims) { return tu.InvalidSignatureToken, nil }, tokenClaims: func() (string, *oidc.IDTokenClaims) { return tu.InvalidSignatureToken, nil },
wantErr: true, wantErr: oidc.ErrSignatureUnsupportedAlg,
}, },
{ {
name: "wrong issuer", name: "wrong issuer",
@ -96,29 +106,7 @@ func TestVerifyIDTokenHint(t *testing.T) {
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "", tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
) )
}, },
wantErr: true, wantErr: oidc.ErrIssuerInvalid,
},
{
name: "expired",
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, "",
)
},
wantErr: true,
},
{
name: "wrong IAT",
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, "",
)
},
wantErr: true,
}, },
{ {
name: "wrong acr", name: "wrong acr",
@ -129,7 +117,31 @@ func TestVerifyIDTokenHint(t *testing.T) {
"else", tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "", "else", tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
) )
}, },
wantErr: true, wantErr: oidc.ErrAcrInvalid,
},
{
name: "expired",
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, "",
)
},
wantClaims: true,
wantErr: IDTokenHintExpiredError{oidc.ErrExpired},
},
{
name: "IAT too old",
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, "",
)
},
wantClaims: true,
wantErr: IDTokenHintExpiredError{oidc.ErrIatToOld},
}, },
{ {
name: "expired auth", name: "expired auth",
@ -140,7 +152,8 @@ func TestVerifyIDTokenHint(t *testing.T) {
tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "", tu.ValidACR, tu.ValidAMR, tu.ValidClientID, tu.ValidSkew, "",
) )
}, },
wantErr: true, wantClaims: true,
wantErr: IDTokenHintExpiredError{oidc.ErrAuthTimeToOld},
}, },
} }
for _, tt := range tests { for _, tt := range tests {
@ -148,14 +161,12 @@ func TestVerifyIDTokenHint(t *testing.T) {
token, want := tt.tokenClaims() token, want := tt.tokenClaims()
got, err := VerifyIDTokenHint[*oidc.IDTokenClaims](context.Background(), token, verifier) got, err := VerifyIDTokenHint[*oidc.IDTokenClaims](context.Background(), token, verifier)
if tt.wantErr { require.ErrorIs(t, err, tt.wantErr)
assert.Error(t, err) if tt.wantClaims {
assert.Nil(t, got) assert.Equal(t, got, want, "claims")
return return
} }
require.NoError(t, err) assert.Nil(t, got, "claims")
require.NotNil(t, got)
assert.Equal(t, got, want)
}) })
} }
} }