From 4ce891bc8ba2581f85d3c1cfdd56af05709e5d5f Mon Sep 17 00:00:00 2001 From: Eytan Kidron Date: Thu, 7 Sep 2023 19:24:33 +0000 Subject: [PATCH 1/4] Add GetPayload() that only returns the payload, skipping the token verification. Also make the error message for token expiration slightly more informative. Both of these changes are made in order to improve debugability in cases where verification fails. --- idtoken/validate.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/idtoken/validate.go b/idtoken/validate.go index e2b84f0b67a..418f816090e 100644 --- a/idtoken/validate.go +++ b/idtoken/validate.go @@ -118,6 +118,34 @@ func Validate(ctx context.Context, idToken string, audience string) (*Payload, e return defaultValidator.validate(ctx, idToken, audience) } +// GetPayload just gets the payload part of the token. +// +// WARNING: THIS FUNCTION DOES NOT VALIDATE THE TOKEN. +// +// In fact, it explicitly skips the validation part. It should only be used to inspect the payload +// content of a token, perhaps for debugging purposes, as a means to try to figure out why the +// validation failed. Note that if Validate() succeeds, it already returns the exact payload that +// this function returns. +func (v *Validator) GetPayload(ctx context.Context, idToken string) (*Payload, error) { + jwt, err := parseJWT(idToken) + if err != nil { + return nil, err + } + return jwt.parsedPayload() +} + +// GetPayload just gets the payload part of the token. +// +// WARNING: THIS FUNCTION DOES NOT VALIDATE THE TOKEN. +// +// In fact, it explicitly skips the validation part. It should only be used to inspect the payload +// content of a token, perhaps for debugging purposes, as a means to try to figure out why the +// validation failed. Note that if Validate() succeeds, it already returns the exact payload that +// this function returns. +func GetPayload(ctx context.Context, idToken string) (*Payload, error) { + return defaultValidator.GetPayload(ctx, idToken) +} + func (v *Validator) validate(ctx context.Context, idToken string, audience string) (*Payload, error) { jwt, err := parseJWT(idToken) if err != nil { @@ -141,7 +169,7 @@ func (v *Validator) validate(ctx context.Context, idToken string, audience strin } if now().Unix() > payload.Expires { - return nil, fmt.Errorf("idtoken: token expired") + return nil, fmt.Errorf("idtoken: token expired: now=%v, expires=%v", now().Unix(), payload.Expires) } switch header.Algorithm { From d404e8542cfbfa4e8408729f909150bb66418297 Mon Sep 17 00:00:00 2001 From: Eytan Kidron Date: Wed, 13 Sep 2023 13:47:50 +0000 Subject: [PATCH 2/4] Updated function header comments --- idtoken/validate.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/idtoken/validate.go b/idtoken/validate.go index 418f816090e..fd9d7f27a83 100644 --- a/idtoken/validate.go +++ b/idtoken/validate.go @@ -118,14 +118,15 @@ func Validate(ctx context.Context, idToken string, audience string) (*Payload, e return defaultValidator.validate(ctx, idToken, audience) } -// GetPayload just gets the payload part of the token. +// GetPayload parses the given token and returns its payload. // -// WARNING: THIS FUNCTION DOES NOT VALIDATE THE TOKEN. +// Warning: This function does not validate the token prior to parsing it. // -// In fact, it explicitly skips the validation part. It should only be used to inspect the payload -// content of a token, perhaps for debugging purposes, as a means to try to figure out why the -// validation failed. Note that if Validate() succeeds, it already returns the exact payload that -// this function returns. +// GetPayload is primarily meant to be used to inspect a token's payload. This is +// useful when validation fails and the payload needs to be inspected. +// +// Note: A successful Validate() invocation with the same token will return an +// identical payload. func (v *Validator) GetPayload(ctx context.Context, idToken string) (*Payload, error) { jwt, err := parseJWT(idToken) if err != nil { @@ -134,14 +135,15 @@ func (v *Validator) GetPayload(ctx context.Context, idToken string) (*Payload, e return jwt.parsedPayload() } -// GetPayload just gets the payload part of the token. +// GetPayload parses the given token and returns its payload. +// +// Warning: This function does not validate the token prior to parsing it. // -// WARNING: THIS FUNCTION DOES NOT VALIDATE THE TOKEN. +// GetPayload is primarily meant to be used to inspect a token's payload. This is +// useful when validation fails and the payload needs to be inspected. // -// In fact, it explicitly skips the validation part. It should only be used to inspect the payload -// content of a token, perhaps for debugging purposes, as a means to try to figure out why the -// validation failed. Note that if Validate() succeeds, it already returns the exact payload that -// this function returns. +// Note: A successful Validate() invocation with the same token will return an +// identical payload. func GetPayload(ctx context.Context, idToken string) (*Payload, error) { return defaultValidator.GetPayload(ctx, idToken) } From 6ef543e75df8eff349c81e71af3251c9ba9f35f1 Mon Sep 17 00:00:00 2001 From: Eytan Kidron Date: Wed, 13 Sep 2023 18:17:47 +0000 Subject: [PATCH 3/4] Add unit test for GetPayload --- idtoken/validate_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/idtoken/validate_test.go b/idtoken/validate_test.go index 46528db2747..a69944702c9 100644 --- a/idtoken/validate_test.go +++ b/idtoken/validate_test.go @@ -231,6 +231,42 @@ func TestValidateES256(t *testing.T) { } } +func TestGetPayload(t *testing.T) { + idToken, _ := createRS256JWT(t) + tests := []struct { + name string + token string + wantPayloadAudience string + wantPayload *Payload + wantErr bool + }{{ + name: "valid token", + token: idToken, + wantPayloadAudience: testAudience, + }, { + name: "unparseable token", + token: "aaa.bbb.ccc", + wantErr: true, + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + v := &Validator{} + payload, err := v.GetPayload(ctx, tt.token) + gotErr := err != nil + if gotErr != tt.wantErr { + t.Errorf("GetPayload(ctx, %q) got error %v, wantErr = %v", tt.token, err, tt.wantErr) + } + if tt.wantPayloadAudience != "" { + if payload == nil || payload.Audience != tt.wantPayloadAudience { + t.Errorf("GetPayload(ctx, %q) got payload %+v, want payload with audience = %v", tt.token, payload, tt.wantPayloadAudience) + } + } + }) + } +} + func createES256JWT(t *testing.T) (string, ecdsa.PublicKey) { t.Helper() token := commonToken(t, "ES256") From 8ed04be6937227b07f965202e4ebfe1765c0acba Mon Sep 17 00:00:00 2001 From: Eytan Kidron Date: Thu, 14 Sep 2023 15:15:48 +0000 Subject: [PATCH 4/4] Change GetPayload to ParsePayload, remove Validator methos and leave just function and some other minor fixes. --- idtoken/validate.go | 19 +++---------------- idtoken/validate_test.go | 11 ++++------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/idtoken/validate.go b/idtoken/validate.go index c2a2645ceb4..47d314f50e5 100644 --- a/idtoken/validate.go +++ b/idtoken/validate.go @@ -122,16 +122,16 @@ func Validate(ctx context.Context, idToken string, audience string) (*Payload, e return defaultValidator.validate(ctx, idToken, audience) } -// GetPayload parses the given token and returns its payload. +// ParsePayload parses the given token and returns its payload. // // Warning: This function does not validate the token prior to parsing it. // -// GetPayload is primarily meant to be used to inspect a token's payload. This is +// ParsePayload is primarily meant to be used to inspect a token's payload. This is // useful when validation fails and the payload needs to be inspected. // // Note: A successful Validate() invocation with the same token will return an // identical payload. -func (v *Validator) GetPayload(ctx context.Context, idToken string) (*Payload, error) { +func ParsePayload(idToken string) (*Payload, error) { jwt, err := parseJWT(idToken) if err != nil { return nil, err @@ -139,19 +139,6 @@ func (v *Validator) GetPayload(ctx context.Context, idToken string) (*Payload, e return jwt.parsedPayload() } -// GetPayload parses the given token and returns its payload. -// -// Warning: This function does not validate the token prior to parsing it. -// -// GetPayload is primarily meant to be used to inspect a token's payload. This is -// useful when validation fails and the payload needs to be inspected. -// -// Note: A successful Validate() invocation with the same token will return an -// identical payload. -func GetPayload(ctx context.Context, idToken string) (*Payload, error) { - return defaultValidator.GetPayload(ctx, idToken) -} - func (v *Validator) validate(ctx context.Context, idToken string, audience string) (*Payload, error) { jwt, err := parseJWT(idToken) if err != nil { diff --git a/idtoken/validate_test.go b/idtoken/validate_test.go index a69944702c9..6c254c7c783 100644 --- a/idtoken/validate_test.go +++ b/idtoken/validate_test.go @@ -231,13 +231,12 @@ func TestValidateES256(t *testing.T) { } } -func TestGetPayload(t *testing.T) { +func TestParsePayload(t *testing.T) { idToken, _ := createRS256JWT(t) tests := []struct { name string token string wantPayloadAudience string - wantPayload *Payload wantErr bool }{{ name: "valid token", @@ -251,16 +250,14 @@ func TestGetPayload(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - v := &Validator{} - payload, err := v.GetPayload(ctx, tt.token) + payload, err := ParsePayload(tt.token) gotErr := err != nil if gotErr != tt.wantErr { - t.Errorf("GetPayload(ctx, %q) got error %v, wantErr = %v", tt.token, err, tt.wantErr) + t.Errorf("ParsePayload(%q) got error %v, wantErr = %v", tt.token, err, tt.wantErr) } if tt.wantPayloadAudience != "" { if payload == nil || payload.Audience != tt.wantPayloadAudience { - t.Errorf("GetPayload(ctx, %q) got payload %+v, want payload with audience = %v", tt.token, payload, tt.wantPayloadAudience) + t.Errorf("ParsePayload(%q) got payload %+v, want payload with audience = %q", tt.token, payload, tt.wantPayloadAudience) } } })