Skip to content

Commit ca484ee

Browse files
committed
refactor(tokens): derive expiry ceilings and defaults from the global strategy
The global expiry config traveled through server.Config three times: as the parsed RefreshTokenPolicy, again re-parsed into ExpiryCeilings, and a third time as raw strings in RefreshTokenDefaults. NewExpiryPolicy now derives the ceilings and the inheritance roots from the strategy itself, so both exported types, both Config fields, buildExpiryCeilings and the discard-logger string round-trip all disappear. The two validations it hosted move where they belong: a non-positive idTokens value is rejected where it is parsed, and negative refresh durations are rejected in NewRefreshTokenPolicy — which also covers the pre-existing case of a negative global instantly expiring every token. NewRefreshTokenPolicy returns to its original signature; the injected clock reaches override strategies through NewExpiryPolicy alone. Claude-Session: https://claude.ai/code/session_0144Jv4MhGfwhx67R3G6qqrC Signed-off-by: Helio Machado <0x2b3bfa0+git@googlemail.com>
1 parent 851ce19 commit ca484ee

13 files changed

Lines changed: 117 additions & 234 deletions

File tree

cmd/dex/serve.go

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,11 @@ func runServe(options serveOptions) error {
317317
if err != nil {
318318
return fmt.Errorf("invalid config value %q for id token expiry: %v", c.Expiry.IDTokens, err)
319319
}
320+
// The value doubles as the ceiling on per-connector overrides, where
321+
// 0 would read as "no ceiling".
322+
if idTokensValidFor <= 0 {
323+
return fmt.Errorf("invalid config value %q for id token expiry: must be positive", c.Expiry.IDTokens)
324+
}
320325
logger.Info("config id tokens", "valid_for", idTokensValidFor)
321326
}
322327

@@ -417,26 +422,13 @@ func runServe(options serveOptions) error {
417422
c.Expiry.RefreshTokens.ValidIfNotUsedFor,
418423
c.Expiry.RefreshTokens.AbsoluteLifetime,
419424
c.Expiry.RefreshTokens.ReuseInterval,
420-
time.Now,
421425
)
422426
if err != nil {
423427
return fmt.Errorf("invalid refresh token expiration policy config: %v", err)
424428
}
425429

426430
serverConfig.RefreshTokenPolicy = refreshTokenPolicy
427431

428-
ceilings, err := buildExpiryCeilings(idTokensValidFor, c.Expiry.RefreshTokens)
429-
if err != nil {
430-
return fmt.Errorf("invalid global expiry config: %v", err)
431-
}
432-
serverConfig.ExpiryCeilings = ceilings
433-
serverConfig.RefreshTokenDefaults = tokens.RefreshTokenDefaults{
434-
DisableRotation: c.Expiry.RefreshTokens.DisableRotation,
435-
ValidIfNotUsedFor: c.Expiry.RefreshTokens.ValidIfNotUsedFor,
436-
AbsoluteLifetime: c.Expiry.RefreshTokens.AbsoluteLifetime,
437-
ReuseInterval: c.Expiry.RefreshTokens.ReuseInterval,
438-
}
439-
440432
if featureflags.SessionsEnabled.Enabled() {
441433
sessionConfig, err := parseSessionConfig(c.Sessions)
442434
if err != nil {
@@ -868,45 +860,6 @@ func parseSessionConfig(s *Sessions) (*session.Config, error) {
868860
return sc, nil
869861
}
870862

871-
// buildExpiryCeilings parses the global expiry config into the ceilings used
872-
// to validate per-connector overrides. The server uses these for both static
873-
// YAML connectors at startup and dynamic API writes at runtime.
874-
func buildExpiryCeilings(globalIDTokens time.Duration, globalRefresh RefreshToken) (tokens.ExpiryCeilings, error) {
875-
// A ceiling of 0 means "no ceiling"; reject values that would silently
876-
// disable enforcement.
877-
if globalIDTokens <= 0 {
878-
return tokens.ExpiryCeilings{}, fmt.Errorf("expiry.idTokens must be positive, got %v", globalIDTokens)
879-
}
880-
c := tokens.ExpiryCeilings{
881-
IDTokens: globalIDTokens,
882-
RefreshRotationDisabled: globalRefresh.DisableRotation,
883-
}
884-
for _, f := range []struct {
885-
name string
886-
value string
887-
dst *time.Duration
888-
}{
889-
{"expiry.refreshTokens.absoluteLifetime", globalRefresh.AbsoluteLifetime, &c.RefreshAbsoluteLifetime},
890-
{"expiry.refreshTokens.validIfNotUsedFor", globalRefresh.ValidIfNotUsedFor, &c.RefreshValidIfNotUsedFor},
891-
{"expiry.refreshTokens.reuseInterval", globalRefresh.ReuseInterval, &c.RefreshReuseInterval},
892-
} {
893-
if f.value == "" {
894-
continue
895-
}
896-
d, err := time.ParseDuration(f.value)
897-
if err != nil {
898-
return c, fmt.Errorf("invalid config value %q for %s: %v", f.value, f.name, err)
899-
}
900-
// A negative value would read as "no ceiling"; zero means "no
901-
// expiration" and stays legal.
902-
if d < 0 {
903-
return c, fmt.Errorf("%s must not be negative, got %v", f.name, d)
904-
}
905-
*f.dst = d
906-
}
907-
return c, nil
908-
}
909-
910863
func buildMFAProviders(authenticators []MFAAuthenticator, issuerURL string, logger *slog.Logger) map[string]mfa.Provider {
911864
if len(authenticators) == 0 {
912865
return nil

cmd/dex/serve_test.go

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@ package main
33
import (
44
"log/slog"
55
"testing"
6-
"time"
76

87
"github.com/stretchr/testify/assert"
98
"github.com/stretchr/testify/require"
10-
11-
"github.com/dexidp/dex/server/tokens"
129
)
1310

1411
func TestNewLogger(t *testing.T) {
@@ -32,80 +29,6 @@ func TestNewLogger(t *testing.T) {
3229
})
3330
}
3431

35-
func TestBuildExpiryCeilings(t *testing.T) {
36-
tests := []struct {
37-
name string
38-
idTokens time.Duration
39-
refresh RefreshToken
40-
want tokens.ExpiryCeilings
41-
wantErrContains string
42-
}{
43-
{
44-
name: "all fields set",
45-
idTokens: 24 * time.Hour,
46-
refresh: RefreshToken{
47-
AbsoluteLifetime: "100h",
48-
ValidIfNotUsedFor: "24h",
49-
ReuseInterval: "3s",
50-
},
51-
want: tokens.ExpiryCeilings{
52-
IDTokens: 24 * time.Hour,
53-
RefreshAbsoluteLifetime: 100 * time.Hour,
54-
RefreshValidIfNotUsedFor: 24 * time.Hour,
55-
RefreshReuseInterval: 3 * time.Second,
56-
},
57-
},
58-
{
59-
name: "refresh unset",
60-
idTokens: 24 * time.Hour,
61-
want: tokens.ExpiryCeilings{IDTokens: 24 * time.Hour},
62-
},
63-
{
64-
name: "rotation disabled propagates",
65-
idTokens: 24 * time.Hour,
66-
refresh: RefreshToken{DisableRotation: true},
67-
want: tokens.ExpiryCeilings{
68-
IDTokens: 24 * time.Hour,
69-
RefreshRotationDisabled: true,
70-
},
71-
},
72-
{
73-
name: "invalid duration",
74-
idTokens: 24 * time.Hour,
75-
refresh: RefreshToken{AbsoluteLifetime: "not-a-duration"},
76-
wantErrContains: `invalid config value "not-a-duration" for expiry.refreshTokens.absoluteLifetime`,
77-
},
78-
{
79-
name: "negative duration",
80-
idTokens: 24 * time.Hour,
81-
refresh: RefreshToken{ValidIfNotUsedFor: "-1h"},
82-
wantErrContains: "expiry.refreshTokens.validIfNotUsedFor must not be negative",
83-
},
84-
{
85-
name: "zero idTokens",
86-
idTokens: 0,
87-
wantErrContains: "expiry.idTokens must be positive",
88-
},
89-
{
90-
name: "negative idTokens",
91-
idTokens: -time.Hour,
92-
wantErrContains: "expiry.idTokens must be positive",
93-
},
94-
}
95-
for _, tc := range tests {
96-
t.Run(tc.name, func(t *testing.T) {
97-
got, err := buildExpiryCeilings(tc.idTokens, tc.refresh)
98-
if tc.wantErrContains != "" {
99-
require.Error(t, err)
100-
require.Contains(t, err.Error(), tc.wantErrContains)
101-
return
102-
}
103-
require.NoError(t, err)
104-
require.Equal(t, tc.want, got)
105-
})
106-
}
107-
}
108-
10932
func TestToStorageConnectorCarriesExpiry(t *testing.T) {
11033
disable := true
11134
sc, err := ToStorageConnector(Connector{

server/apiserver/api_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ func TestCreateConnectorExpiryHierarchy(t *testing.T) {
499499
logger := newLogger(t)
500500
s := memory.New(logger)
501501

502-
expiry := tokens.NewExpiryPolicy(time.Hour, nil, tokens.ExpiryCeilings{IDTokens: time.Hour}, tokens.RefreshTokenDefaults{}, nil)
502+
expiry := tokens.NewExpiryPolicy(time.Hour, nil, nil)
503503
client := newAPIWithExpiry(t, s, logger, expiry)
504504
defer client.Close()
505505

server/authflow/handler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func newTestHandler(t *testing.T, updateConfig func(c *testFlowConfig)) (*httpte
104104

105105
now := func() time.Time { return time.Now() }
106106
conns := connectors.NewCache(store, testResolveConnector)
107-
issuer := tokens.NewIssuer(store, sig, *issuerURL, tokens.NewExpiryPolicy(24*time.Hour, nil, tokens.ExpiryCeilings{}, tokens.RefreshTokenDefaults{}, now), now, logger)
107+
issuer := tokens.NewIssuer(store, sig, *issuerURL, tokens.NewExpiryPolicy(24*time.Hour, nil, now), now, logger)
108108

109109
tc := testFlowConfig{
110110
Handler: Handler{

server/config.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,10 @@ type Config struct {
6767
AuthRequestsValidFor time.Duration // Defaults to 24 hours
6868
DeviceRequestsValidFor time.Duration // Defaults to 5 minutes
6969

70-
// Refresh token expiration settings
70+
// Refresh token expiration settings. Also the ceiling per-connector expiry
71+
// overrides may not loosen.
7172
RefreshTokenPolicy *tokens.RefreshStrategy
7273

73-
// ExpiryCeilings define the upper bounds against which per-connector
74-
// expiry overrides are validated. A zero duration means "no ceiling".
75-
ExpiryCeilings tokens.ExpiryCeilings
76-
77-
// RefreshTokenDefaults provide the inheritance roots for per-connector
78-
// refresh-token overrides that leave fields unset.
79-
RefreshTokenDefaults tokens.RefreshTokenDefaults
80-
8174
// If set, the server will use this connector to handle password grants
8275
PasswordConnector string
8376

server/introspection/introspection_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func testAccessToken(t *testing.T) string {
4646
issURL, err := url.Parse(testIssuer)
4747
require.NoError(t, err)
4848

49-
issuer := tokens.NewIssuer(memory.New(logger), sig, *issURL, tokens.NewExpiryPolicy(time.Hour, nil, tokens.ExpiryCeilings{}, tokens.RefreshTokenDefaults{}, nil), time.Now, logger)
49+
issuer := tokens.NewIssuer(memory.New(logger), sig, *issURL, tokens.NewExpiryPolicy(time.Hour, nil, nil), time.Now, logger)
5050
token, _, err := issuer.SignIDToken(ctx, tokens.Authorization{
5151
Client: storage.Client{ID: "test"},
5252
Claims: storage.Claims{UserID: "1", Username: "jane"},

server/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func newServer(ctx context.Context, c Config) (*Server, error) {
104104
templates: rc.templates,
105105
logger: c.Logger,
106106
}
107-
s.expiryPolicy = tokens.NewExpiryPolicy(rc.idTokensValidFor, c.RefreshTokenPolicy, c.ExpiryCeilings, c.RefreshTokenDefaults, rc.now)
107+
s.expiryPolicy = tokens.NewExpiryPolicy(rc.idTokensValidFor, c.RefreshTokenPolicy, rc.now)
108108
s.sessions = &session.Manager{
109109
Storage: s.storage,
110110
Config: c.SessionConfig,

server/server_oauth2_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func TestNewIDTokenUsesStoredAlgorithmUntilNextRotation(t *testing.T) {
181181
require.NoError(t, err)
182182

183183
clock := func() time.Time { return now }
184-
issuer := tokens.NewIssuer(store, sig, *issuerURL, tokens.NewExpiryPolicy(time.Hour, nil, tokens.ExpiryCeilings{}, tokens.RefreshTokenDefaults{}, clock), clock, logger)
184+
issuer := tokens.NewIssuer(store, sig, *issuerURL, tokens.NewExpiryPolicy(time.Hour, nil, clock), clock, logger)
185185

186186
accessToken := "test-access-token"
187187
code := "test-auth-code"
@@ -261,7 +261,7 @@ func TestNewIDTokenContainsJTI(t *testing.T) {
261261
require.NoError(t, err)
262262

263263
clock := func() time.Time { return now }
264-
issuer := tokens.NewIssuer(store, sig, *issuerURL, tokens.NewExpiryPolicy(time.Hour, nil, tokens.ExpiryCeilings{}, tokens.RefreshTokenDefaults{}, clock), clock, logger)
264+
issuer := tokens.NewIssuer(store, sig, *issuerURL, tokens.NewExpiryPolicy(time.Hour, nil, clock), clock, logger)
265265

266266
keys, err := sig.ValidationKeys(ctx)
267267
require.NoError(t, err)

0 commit comments

Comments
 (0)