Skip to content

Commit 0ad2480

Browse files
authored
fix!: allow expires to be an integer wrapped in a string value (#821)
Certain providers, for example Entra ID, return durations as quoted strings instead of JSON numbers. This is a deviation from the standard. This change allows for the successful parsing of such quoted duration strings. BREAKING CHANGE: The `expires_in field` has changed from type `int64` to `oidc.Duration` for various token response payloads. fixes #815 ### Definition of Ready - [x] I am happy with the code - [x] Short description of the feature/issue is added in the pr description - [x] PR is linked to the corresponding user story - [x] Acceptance criteria are met - [x] All open todos and follow ups are defined in a new ticket and justified - [ ] Deviations from the acceptance criteria and design are agreed with the PO and documented. - [x] No debug or dead code - [x] My code has no repetitions - [x] Critical parts are tested automatically - [ ] Where possible E2E tests are implemented - [ ] Documentation/examples are up-to-date - [ ] All non-functional requirements are met - [ ] Functionality of the acceptance criteria is checked manually on the dev system.
1 parent 455ac64 commit 0ad2480

File tree

10 files changed

+256
-9
lines changed

10 files changed

+256
-9
lines changed

UPGRADING.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,36 @@ All commands are executed from the root of the project that imports oidc package
55
on non-GNU systems, such as MacOS.
66
Alternatively, GNU sed can be installed on such systems. (`coreutils` package?).
77

8+
## V3 to V4
9+
10+
### oidc
11+
12+
#### `exp` claim in `AccessTokenResponse` and `TokenExchangeResponse`
13+
14+
The type of the `ExpiresIn` field has been changed from an `int64` to an `oidc.Duration` type.
15+
Unfortunately this upgrade cannot be described in a sed command and has to be done manually.
16+
17+
Example:
18+
```go
19+
validity := 30 * time.Seconds
20+
oidc.AccessTokenResponse{
21+
AccessToken: accessToken,
22+
TokenType: oidc.BearerToken,
23+
ExpiresIn: uint64(validity.Seconds()),
24+
Scope: tokenRequest.GetScopes(),
25+
}
26+
```
27+
becomes
28+
```go
29+
validity := 30 * time.Seconds
30+
oidc.AccessTokenResponse{
31+
AccessToken: accessToken,
32+
TokenType: oidc.BearerToken,
33+
ExpiresIn: oidc.Duration(validity),
34+
Scope: tokenRequest.GetScopes(),
35+
}
36+
```
37+
838
## V2 to V3
939

1040
**TL;DR** at the [bottom](#full-script) of this chapter is a full `sed` script

pkg/oidc/token.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ type AccessTokenResponse struct {
233233
AccessToken string `json:"access_token,omitempty" schema:"access_token,omitempty"`
234234
TokenType string `json:"token_type,omitempty" schema:"token_type,omitempty"`
235235
RefreshToken string `json:"refresh_token,omitempty" schema:"refresh_token,omitempty"`
236-
ExpiresIn uint64 `json:"expires_in,omitempty" schema:"expires_in,omitempty"`
236+
ExpiresIn Duration `json:"expires_in,omitempty" schema:"expires_in,omitempty"`
237237
IDToken string `json:"id_token,omitempty" schema:"id_token,omitempty"`
238238
State string `json:"state,omitempty" schema:"state,omitempty"`
239239
Scope SpaceDelimitedArray `json:"scope,omitempty" schema:"scope,omitempty"`
@@ -375,7 +375,7 @@ type TokenExchangeResponse struct {
375375
AccessToken string `json:"access_token"` // Can be access token or ID token
376376
IssuedTokenType TokenType `json:"issued_token_type"`
377377
TokenType string `json:"token_type"`
378-
ExpiresIn uint64 `json:"expires_in,omitempty"`
378+
ExpiresIn Duration `json:"expires_in,omitempty"`
379379
Scopes SpaceDelimitedArray `json:"scope,omitempty"`
380380
RefreshToken string `json:"refresh_token,omitempty"`
381381

pkg/oidc/token_test.go

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package oidc
22

33
import (
4+
"encoding/json"
45
"testing"
56
"time"
67

@@ -278,3 +279,174 @@ func TestNewLogoutTokenClaims(t *testing.T) {
278279

279280
assert.Equal(t, want, got)
280281
}
282+
283+
func TestTokenExchangeResponse_UnmarshalJSON(t *testing.T) {
284+
testCases := []struct {
285+
marshaled string
286+
want TokenExchangeResponse
287+
}{
288+
{
289+
marshaled: `
290+
{
291+
"access_token":"access_token",
292+
"issued_token_type":"test_token",
293+
"token_type":"test_token",
294+
"expires_in":12345,
295+
"scope": "lorem ipsum",
296+
"refresh_token": "refresh_token",
297+
"id_token": "id_token"
298+
}`,
299+
want: TokenExchangeResponse{
300+
AccessToken: "access_token",
301+
IssuedTokenType: "test_token",
302+
TokenType: "test_token",
303+
ExpiresIn: Duration(12345 * time.Second),
304+
Scopes: []string{"lorem", "ipsum"},
305+
RefreshToken: "refresh_token",
306+
IDToken: "id_token",
307+
},
308+
},
309+
{
310+
marshaled: `
311+
{
312+
"access_token":"access_token",
313+
"issued_token_type":"test_token",
314+
"token_type":"test_token",
315+
"expires_in":"12345",
316+
"scope": "lorem ipsum",
317+
"refresh_token": "refresh_token",
318+
"id_token": "id_token"
319+
}`,
320+
want: TokenExchangeResponse{
321+
AccessToken: "access_token",
322+
IssuedTokenType: "test_token",
323+
TokenType: "test_token",
324+
ExpiresIn: Duration(12345 * time.Second),
325+
Scopes: []string{"lorem", "ipsum"},
326+
RefreshToken: "refresh_token",
327+
IDToken: "id_token",
328+
},
329+
},
330+
}
331+
for _, testCase := range testCases {
332+
got := &TokenExchangeResponse{}
333+
err := json.Unmarshal([]byte(testCase.marshaled), got)
334+
assert.Equal(t, nil, err)
335+
assert.Equal(t, testCase.want.AccessToken, got.AccessToken)
336+
assert.Equal(t, testCase.want.IssuedTokenType, got.IssuedTokenType)
337+
assert.Equal(t, testCase.want.TokenType, got.TokenType)
338+
assert.Equal(t, testCase.want.ExpiresIn, got.ExpiresIn)
339+
assert.Equal(t, testCase.want.Scopes, got.Scopes)
340+
assert.Equal(t, testCase.want.RefreshToken, got.RefreshToken)
341+
assert.Equal(t, testCase.want.IDToken, got.IDToken)
342+
}
343+
}
344+
func TestTokenExchangeResponse_MarshalJSON(t *testing.T) {
345+
testCases := []struct {
346+
want string
347+
unmarshaled TokenExchangeResponse
348+
}{
349+
{
350+
want: `{"access_token":"access_token","issued_token_type":"test_token","token_type":"test_token","expires_in":12345,"scope":"lorem ipsum","refresh_token":"refresh_token","id_token":"id_token"}`,
351+
unmarshaled: TokenExchangeResponse{
352+
AccessToken: "access_token",
353+
IssuedTokenType: "test_token",
354+
TokenType: "test_token",
355+
ExpiresIn: Duration(12345 * time.Second),
356+
Scopes: []string{"lorem", "ipsum"},
357+
RefreshToken: "refresh_token",
358+
IDToken: "id_token",
359+
},
360+
},
361+
}
362+
for _, testCase := range testCases {
363+
got, err := json.Marshal(testCase.unmarshaled)
364+
assert.Equal(t, nil, err)
365+
assert.Equal(t, testCase.want, string(got))
366+
}
367+
}
368+
func TestAccessTokenResponse_UnmarshalJSON(t *testing.T) {
369+
testCases := []struct {
370+
marshaled string
371+
want AccessTokenResponse
372+
}{
373+
{
374+
marshaled: `
375+
{
376+
"access_token":"access_token",
377+
"token_type":"test_token",
378+
"refresh_token": "refresh_token",
379+
"expires_in":12345,
380+
"id_token": "id_token",
381+
"state": "state",
382+
"scope": "lorem ipsum"
383+
}`,
384+
want: AccessTokenResponse{
385+
AccessToken: "access_token",
386+
TokenType: "test_token",
387+
RefreshToken: "refresh_token",
388+
ExpiresIn: Duration(12345 * time.Second),
389+
IDToken: "id_token",
390+
State: "state",
391+
Scope: []string{"lorem", "ipsum"},
392+
},
393+
},
394+
{
395+
marshaled: `
396+
{
397+
"access_token":"access_token",
398+
"token_type":"test_token",
399+
"refresh_token": "refresh_token",
400+
"expires_in":"12345",
401+
"id_token": "id_token",
402+
"state": "state",
403+
"scope": "lorem ipsum"
404+
}`,
405+
want: AccessTokenResponse{
406+
AccessToken: "access_token",
407+
TokenType: "test_token",
408+
RefreshToken: "refresh_token",
409+
ExpiresIn: Duration(12345 * time.Second),
410+
IDToken: "id_token",
411+
State: "state",
412+
Scope: []string{"lorem", "ipsum"},
413+
},
414+
},
415+
}
416+
for _, testCase := range testCases {
417+
got := &AccessTokenResponse{}
418+
err := json.Unmarshal([]byte(testCase.marshaled), got)
419+
assert.Equal(t, nil, err)
420+
assert.Equal(t, testCase.want.AccessToken, got.AccessToken)
421+
assert.Equal(t, testCase.want.TokenType, got.TokenType)
422+
assert.Equal(t, testCase.want.RefreshToken, got.RefreshToken)
423+
assert.Equal(t, testCase.want.ExpiresIn, got.ExpiresIn)
424+
assert.Equal(t, testCase.want.IDToken, got.IDToken)
425+
assert.Equal(t, testCase.want.State, got.State)
426+
assert.Equal(t, testCase.want.Scope, got.Scope)
427+
}
428+
}
429+
func TestAccessTokenResponse_MarshalJSON(t *testing.T) {
430+
testCases := []struct {
431+
want string
432+
unmarshaled AccessTokenResponse
433+
}{
434+
{
435+
want: `{"access_token":"access_token","token_type":"test_token","refresh_token":"refresh_token","expires_in":12345,"id_token":"id_token","state":"state","scope":"lorem ipsum"}`,
436+
unmarshaled: AccessTokenResponse{
437+
AccessToken: "access_token",
438+
TokenType: "test_token",
439+
RefreshToken: "refresh_token",
440+
ExpiresIn: Duration(12345 * time.Second),
441+
IDToken: "id_token",
442+
State: "state",
443+
Scope: []string{"lorem", "ipsum"},
444+
},
445+
},
446+
}
447+
for _, testCase := range testCases {
448+
got, err := json.Marshal(testCase.unmarshaled)
449+
assert.Equal(t, nil, err)
450+
assert.Equal(t, testCase.want, string(got))
451+
}
452+
}

pkg/oidc/types.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"math"
89
"reflect"
10+
"strconv"
911
"strings"
1012
"time"
1113

@@ -300,3 +302,47 @@ func (r *RequestObject) GetIssuer() string {
300302
}
301303

302304
func (*RequestObject) SetSignatureAlgorithm(algorithm jose.SignatureAlgorithm) {}
305+
306+
type Duration time.Duration
307+
308+
func (d Duration) AsDuration() time.Duration {
309+
return time.Duration(d)
310+
}
311+
312+
func (d *Duration) UnmarshalJSON(data []byte) error {
313+
var v any
314+
if err := json.Unmarshal(data, &v); err != nil {
315+
return fmt.Errorf("oidc.Duration: %w", err)
316+
}
317+
switch x := v.(type) {
318+
case int64:
319+
*d = Duration(time.Second * time.Duration(x))
320+
case float64:
321+
mod := math.Mod(x, 1)
322+
if mod > 0 {
323+
return fmt.Errorf("oidc.Duration: unable to parse type %T with value %v", x, x)
324+
}
325+
*d = Duration(time.Second * time.Duration(x))
326+
case string:
327+
// Compatibility with EntraID:
328+
// https://github.com/zitadel/oidc/issues/815
329+
i, err := strconv.Atoi(x)
330+
if err != nil {
331+
return fmt.Errorf("oidc.Duration: %w", err)
332+
}
333+
*d = Duration(time.Second * time.Duration(i))
334+
case nil:
335+
*d = 0
336+
default:
337+
return fmt.Errorf("oidc.Duration: unable to parse type %T with value %v", x, x)
338+
}
339+
if *d < 0 {
340+
return fmt.Errorf("oidc.Duration: cannot be a negative time (%v)", *d)
341+
}
342+
return nil
343+
}
344+
345+
func (d Duration) MarshalJSON() ([]byte, error) {
346+
s := int64(d.AsDuration().Seconds())
347+
return []byte(strconv.FormatInt(s, 10)), nil
348+
}

pkg/op/device.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ func CreateDeviceTokenResponse(ctx context.Context, tokenRequest TokenRequest, c
343343
AccessToken: accessToken,
344344
RefreshToken: refreshToken,
345345
TokenType: oidc.BearerToken,
346-
ExpiresIn: uint64(validity.Seconds()),
346+
ExpiresIn: oidc.Duration(validity),
347347
Scope: tokenRequest.GetScopes(),
348348
}
349349

pkg/op/device_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ func TestCreateDeviceTokenResponse(t *testing.T) {
523523
return
524524
}
525525
require.NoError(t, err)
526-
assert.InDelta(t, 300, got.ExpiresIn, 2)
526+
assert.InDelta(t, 300, got.ExpiresIn.AsDuration().Seconds(), 2)
527527
if tt.wantAccessToken {
528528
assert.NotEmpty(t, got.AccessToken, "access token")
529529
}

pkg/op/token.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func CreateTokenResponse(ctx context.Context, request IDTokenRequest, client Cli
5757
}
5858
}
5959

60-
exp := uint64(validity.Seconds())
60+
exp := oidc.Duration(validity)
6161
return &oidc.AccessTokenResponse{
6262
AccessToken: accessToken,
6363
IDToken: idToken,

pkg/op/token_client_credentials.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func CreateClientCredentialsTokenResponse(ctx context.Context, tokenRequest Toke
119119
return &oidc.AccessTokenResponse{
120120
AccessToken: accessToken,
121121
TokenType: oidc.BearerToken,
122-
ExpiresIn: uint64(validity.Seconds()),
122+
ExpiresIn: oidc.Duration(validity),
123123
Scope: tokenRequest.GetScopes(),
124124
}, nil
125125
}

pkg/op/token_exchange.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,12 +402,11 @@ func CreateTokenExchangeResponse(
402402
oidc.ErrInvalidRequest().WithDescription("requested_token_type is invalid")
403403
}
404404

405-
exp := uint64(validity.Seconds())
406405
return &oidc.TokenExchangeResponse{
407406
AccessToken: token,
408407
IssuedTokenType: tokenExchangeRequest.GetRequestedTokenType(),
409408
TokenType: tokenType,
410-
ExpiresIn: exp,
409+
ExpiresIn: oidc.Duration(validity),
411410
RefreshToken: refreshToken,
412411
Scopes: tokenExchangeRequest.GetScopes(),
413412
}, nil

pkg/op/token_jwt_profile.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func CreateJWTTokenResponse(ctx context.Context, tokenRequest TokenRequest, crea
8888
return &oidc.AccessTokenResponse{
8989
AccessToken: accessToken,
9090
TokenType: oidc.BearerToken,
91-
ExpiresIn: uint64(validity.Seconds()),
91+
ExpiresIn: oidc.Duration(validity),
9292
Scope: tokenRequest.GetScopes(),
9393
}, nil
9494
}

0 commit comments

Comments
 (0)