Skip to content

Commit 6725ed6

Browse files
AlexFenlonAlexFenlon
authored
Add OIDC timeout customization to ConfigMap (#8495)
1 parent 8c40b7f commit 6725ed6

File tree

12 files changed

+723
-26
lines changed

12 files changed

+723
-26
lines changed

‎internal/configs/config_params.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ type ConfigParams struct {
6868
MainAppProtectDosLogFormat []string
6969
MainAppProtectDosLogFormatEscaping string
7070
MainAppProtectDosArbFqdn string
71+
OIDC OIDC
7172
ProxyBuffering bool
7273
ProxyBuffers string
7374
ProxyBufferSize string
@@ -192,6 +193,15 @@ type ZoneSync struct {
192193
ResolverIPV6 *bool
193194
}
194195

196+
// OIDC holds OIDC configuration parameters.
197+
type OIDC struct {
198+
PKCETimeout string
199+
IDTokenTimeout string
200+
AccessTimeout string
201+
RefreshTimeout string
202+
SIDSTimeout string
203+
}
204+
195205
// MGMTSecrets holds mgmt block secret names
196206
type MGMTSecrets struct {
197207
License string
@@ -258,6 +268,13 @@ func NewDefaultConfigParams(ctx context.Context, isPlus bool) *ConfigParams {
258268
LimitReqZoneSize: "10m",
259269
LimitReqLogLevel: "error",
260270
LimitReqRejectCode: 429,
271+
OIDC: OIDC{
272+
PKCETimeout: "90s",
273+
IDTokenTimeout: "1h",
274+
AccessTimeout: "1h",
275+
RefreshTimeout: "8h",
276+
SIDSTimeout: "8h",
277+
},
261278
}
262279
}
263280

‎internal/configs/configmaps.go‎

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,11 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
459459
configOk = false
460460
}
461461

462+
err = parseConfigMapOIDC(l, cfgm, cfgParams, eventLog)
463+
if err != nil {
464+
configOk = false
465+
}
466+
462467
if upstreamZoneSize, exists := cfgm.Data["upstream-zone-size"]; exists {
463468
cfgParams.UpstreamZoneSize = upstreamZoneSize
464469
}
@@ -694,6 +699,61 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
694699
return cfgParams, configOk
695700
}
696701

702+
// parseConfigMapOIDC parses OIDC timeout configuration from ConfigMap.
703+
func parseConfigMapOIDC(l *slog.Logger, cfgm *v1.ConfigMap, cfgParams *ConfigParams, eventLog record.EventRecorder) error {
704+
if oidcPKCETimeout, exists := cfgm.Data["oidc-pkce-timeout"]; exists {
705+
pkceTimeout, err := ParseTime(oidcPKCETimeout)
706+
if err != nil {
707+
errorText := fmt.Sprintf("ConfigMap %s/%s: invalid value for 'oidc-pkce-timeout': %q, must be a valid nginx time (e.g. '90s', '5m', '1h')", cfgm.Namespace, cfgm.Name, oidcPKCETimeout)
708+
nl.Warn(l, errorText)
709+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
710+
return err
711+
}
712+
cfgParams.OIDC.PKCETimeout = pkceTimeout
713+
}
714+
if oidcIDTokensTimeout, exists := cfgm.Data["oidc-id-tokens-timeout"]; exists {
715+
idTokensTimeout, err := ParseTime(oidcIDTokensTimeout)
716+
if err != nil {
717+
errorText := fmt.Sprintf("ConfigMap %s/%s: invalid value for 'oidc-id-tokens-timeout': %q, must be a valid nginx time (e.g. '1h', '30m', '2h')", cfgm.Namespace, cfgm.Name, oidcIDTokensTimeout)
718+
nl.Warn(l, errorText)
719+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
720+
return err
721+
}
722+
cfgParams.OIDC.IDTokenTimeout = idTokensTimeout
723+
}
724+
if oidcAccessTokensTimeout, exists := cfgm.Data["oidc-access-tokens-timeout"]; exists {
725+
accessTokensTimeout, err := ParseTime(oidcAccessTokensTimeout)
726+
if err != nil {
727+
errorText := fmt.Sprintf("ConfigMap %s/%s: invalid value for 'oidc-access-tokens-timeout': %q, must be a valid nginx time (e.g. '1h', '30m', '2h')", cfgm.Namespace, cfgm.Name, oidcAccessTokensTimeout)
728+
nl.Warn(l, errorText)
729+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
730+
return err
731+
}
732+
cfgParams.OIDC.AccessTimeout = accessTokensTimeout
733+
}
734+
if oidcRefreshTokensTimeout, exists := cfgm.Data["oidc-refresh-tokens-timeout"]; exists {
735+
refreshTokensTimeout, err := ParseTime(oidcRefreshTokensTimeout)
736+
if err != nil {
737+
errorText := fmt.Sprintf("ConfigMap %s/%s: invalid value for 'oidc-refresh-tokens-timeout': %q, must be a valid nginx time (e.g. '8h', '12h', '24h')", cfgm.Namespace, cfgm.Name, oidcRefreshTokensTimeout)
738+
nl.Warn(l, errorText)
739+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
740+
return err
741+
}
742+
cfgParams.OIDC.RefreshTimeout = refreshTokensTimeout
743+
}
744+
if oidcSIDSTimeout, exists := cfgm.Data["oidc-sids-timeout"]; exists {
745+
sidsTimeout, err := ParseTime(oidcSIDSTimeout)
746+
if err != nil {
747+
errorText := fmt.Sprintf("ConfigMap %s/%s: invalid value for 'oidc-sids-timeout': %q, must be a valid nginx time (e.g. '8h', '12h', '24h')", cfgm.Namespace, cfgm.Name, oidcSIDSTimeout)
748+
nl.Warn(l, errorText)
749+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
750+
return err
751+
}
752+
cfgParams.OIDC.SIDSTimeout = sidsTimeout
753+
}
754+
return nil
755+
}
756+
697757
//nolint:gocyclo
698758
func parseConfigMapZoneSync(l *slog.Logger, cfgm *v1.ConfigMap, cfgParams *ConfigParams, eventLog record.EventRecorder, nginxPlus bool) (*ZoneSync, error) {
699759
if zoneSync, exists, err := GetMapKeyAsBool(cfgm.Data, "zone-sync", cfgm); exists {
@@ -1121,11 +1181,18 @@ func GenerateNginxMainConfig(staticCfgParams *StaticConfigParams, config *Config
11211181
InternalRouteServer: staticCfgParams.EnableInternalRoutes,
11221182
InternalRouteServerName: staticCfgParams.InternalRouteServerName,
11231183
LatencyMetrics: staticCfgParams.EnableLatencyMetrics,
1124-
OIDC: staticCfgParams.EnableOIDC,
1125-
ZoneSyncConfig: zoneSyncConfig,
1126-
DynamicSSLReloadEnabled: staticCfgParams.DynamicSSLReload,
1127-
StaticSSLPath: staticCfgParams.StaticSSLPath,
1128-
NginxVersion: staticCfgParams.NginxVersion,
1184+
OIDC: version1.OIDCConfig{
1185+
Enable: staticCfgParams.EnableOIDC,
1186+
PKCETimeout: config.OIDC.PKCETimeout,
1187+
IDTokenTimeout: config.OIDC.IDTokenTimeout,
1188+
AccessTimeout: config.OIDC.AccessTimeout,
1189+
RefreshTimeout: config.OIDC.RefreshTimeout,
1190+
SIDSTimeout: config.OIDC.SIDSTimeout,
1191+
},
1192+
ZoneSyncConfig: zoneSyncConfig,
1193+
DynamicSSLReloadEnabled: staticCfgParams.DynamicSSLReload,
1194+
StaticSSLPath: staticCfgParams.StaticSSLPath,
1195+
NginxVersion: staticCfgParams.NginxVersion,
11291196
}
11301197
return nginxCfg
11311198
}

‎internal/configs/configmaps_test.go‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,246 @@ func TestParseConfigMapAccessLogDefault(t *testing.T) {
298298
}
299299
}
300300

301+
func TestParseConfigMapOIDC(t *testing.T) {
302+
t.Parallel()
303+
tests := []struct {
304+
configMap *v1.ConfigMap
305+
want *OIDC
306+
msg string
307+
}{
308+
{
309+
configMap: &v1.ConfigMap{
310+
Data: map[string]string{},
311+
},
312+
want: &OIDC{
313+
PKCETimeout: "90s",
314+
IDTokenTimeout: "1h",
315+
AccessTimeout: "1h",
316+
RefreshTimeout: "8h",
317+
SIDSTimeout: "8h",
318+
},
319+
msg: "default OIDC values",
320+
},
321+
{
322+
configMap: &v1.ConfigMap{
323+
Data: map[string]string{
324+
"oidc-pkce-timeout": "5m",
325+
"oidc-id-tokens-timeout": "2h",
326+
"oidc-access-tokens-timeout": "3h",
327+
"oidc-refresh-tokens-timeout": "48h",
328+
"oidc-sids-timeout": "72h",
329+
},
330+
},
331+
want: &OIDC{
332+
PKCETimeout: "5m",
333+
IDTokenTimeout: "2h",
334+
AccessTimeout: "3h",
335+
RefreshTimeout: "48h",
336+
SIDSTimeout: "72h",
337+
},
338+
msg: "all timeout values custom",
339+
},
340+
{
341+
configMap: &v1.ConfigMap{
342+
Data: map[string]string{
343+
"oidc-pkce-timeout": "15m",
344+
},
345+
},
346+
want: &OIDC{
347+
PKCETimeout: "15m",
348+
},
349+
msg: "custom PKCE timeout only",
350+
},
351+
{
352+
configMap: &v1.ConfigMap{
353+
Data: map[string]string{
354+
"oidc-id-tokens-timeout": "90m",
355+
},
356+
},
357+
want: &OIDC{
358+
IDTokenTimeout: "90m",
359+
},
360+
msg: "custom ID token timeout only",
361+
},
362+
{
363+
configMap: &v1.ConfigMap{
364+
Data: map[string]string{
365+
"oidc-access-tokens-timeout": "4h",
366+
},
367+
},
368+
want: &OIDC{
369+
AccessTimeout: "4h",
370+
},
371+
msg: "custom access token timeout only",
372+
},
373+
{
374+
configMap: &v1.ConfigMap{
375+
Data: map[string]string{
376+
"oidc-refresh-tokens-timeout": "16h",
377+
},
378+
},
379+
want: &OIDC{
380+
RefreshTimeout: "16h",
381+
},
382+
msg: "custom refresh token timeout only",
383+
},
384+
{
385+
configMap: &v1.ConfigMap{
386+
Data: map[string]string{
387+
"oidc-sids-timeout": "12h",
388+
},
389+
},
390+
want: &OIDC{
391+
SIDSTimeout: "12h",
392+
},
393+
msg: "custom SIDS timeout only",
394+
},
395+
}
396+
397+
nginxPlus := true
398+
hasAppProtect := false
399+
hasAppProtectDos := false
400+
hasTLSPassthrough := false
401+
directiveAutoadjustEnabled := false
402+
403+
for _, test := range tests {
404+
t.Run(test.msg, func(t *testing.T) {
405+
result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger())
406+
if !configOk {
407+
t.Error("want configOk true, got configOk false")
408+
}
409+
410+
// Check only the specific fields that are set in the test expectation
411+
if test.want.PKCETimeout != "" {
412+
assert.Equal(t, test.want.PKCETimeout, result.OIDC.PKCETimeout)
413+
}
414+
if test.want.IDTokenTimeout != "" {
415+
assert.Equal(t, test.want.IDTokenTimeout, result.OIDC.IDTokenTimeout)
416+
}
417+
if test.want.AccessTimeout != "" {
418+
assert.Equal(t, test.want.AccessTimeout, result.OIDC.AccessTimeout)
419+
}
420+
if test.want.RefreshTimeout != "" {
421+
assert.Equal(t, test.want.RefreshTimeout, result.OIDC.RefreshTimeout)
422+
}
423+
if test.want.SIDSTimeout != "" {
424+
assert.Equal(t, test.want.SIDSTimeout, result.OIDC.SIDSTimeout)
425+
}
426+
})
427+
}
428+
}
429+
430+
func TestParseConfigMapOIDCErrors(t *testing.T) {
431+
t.Parallel()
432+
tests := []struct {
433+
configMap *v1.ConfigMap
434+
expectedErr bool
435+
msg string
436+
}{
437+
{
438+
configMap: &v1.ConfigMap{
439+
Data: map[string]string{
440+
"oidc-pkce-timeout": "invalid-time",
441+
},
442+
},
443+
expectedErr: true,
444+
msg: "invalid PKCE timeout format",
445+
},
446+
{
447+
configMap: &v1.ConfigMap{
448+
Data: map[string]string{
449+
"oidc-id-tokens-timeout": "abc123",
450+
},
451+
},
452+
expectedErr: true,
453+
msg: "invalid ID token timeout format",
454+
},
455+
{
456+
configMap: &v1.ConfigMap{
457+
Data: map[string]string{
458+
"oidc-access-tokens-timeout": "5x",
459+
},
460+
},
461+
expectedErr: true,
462+
msg: "invalid access token timeout format",
463+
},
464+
{
465+
configMap: &v1.ConfigMap{
466+
Data: map[string]string{
467+
"oidc-refresh-tokens-timeout": "",
468+
},
469+
},
470+
expectedErr: true,
471+
msg: "empty refresh token timeout",
472+
},
473+
{
474+
configMap: &v1.ConfigMap{
475+
Data: map[string]string{
476+
"oidc-sids-timeout": " ",
477+
},
478+
},
479+
expectedErr: true,
480+
msg: "whitespace-only SIDS timeout",
481+
},
482+
{
483+
configMap: &v1.ConfigMap{
484+
Data: map[string]string{
485+
"oidc-pkce-timeout": "-5m",
486+
},
487+
},
488+
expectedErr: true,
489+
msg: "negative PKCE timeout",
490+
},
491+
{
492+
configMap: &v1.ConfigMap{
493+
Data: map[string]string{
494+
"oidc-id-tokens-timeout": "1.5h",
495+
},
496+
},
497+
expectedErr: true,
498+
msg: "decimal in ID token timeout",
499+
},
500+
{
501+
configMap: &v1.ConfigMap{
502+
Data: map[string]string{
503+
"oidc-access-tokens-timeout": "5minutes",
504+
},
505+
},
506+
expectedErr: true,
507+
msg: "invalid time unit format",
508+
},
509+
510+
{
511+
configMap: &v1.ConfigMap{
512+
Data: map[string]string{
513+
"oidc-sids-timeout": "5s 10m",
514+
},
515+
},
516+
expectedErr: true,
517+
msg: "multiple time values without proper format",
518+
},
519+
}
520+
521+
nginxPlus := true
522+
hasAppProtect := false
523+
hasAppProtectDos := false
524+
hasTLSPassthrough := false
525+
directiveAutoadjustEnabled := false
526+
527+
for _, test := range tests {
528+
t.Run(test.msg, func(t *testing.T) {
529+
_, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger())
530+
531+
if test.expectedErr && configOk {
532+
t.Errorf("want configOk false, got configOk true for %s", test.msg)
533+
}
534+
if !test.expectedErr && !configOk {
535+
t.Errorf("want configOk true, got configOk false for %s", test.msg)
536+
}
537+
})
538+
}
539+
}
540+
301541
func TestParseMGMTConfigMapError(t *testing.T) {
302542
t.Parallel()
303543
tests := []struct {

‎internal/configs/oidc/oidc_common.conf‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@ map $http_x_forwarded_proto $proto {
1616
# JWK Set will be fetched from $oidc_jwks_uri and cached here - ensure writable by nginx user
1717
proxy_cache_path /var/cache/nginx/jwk levels=1 keys_zone=jwk:64k max_size=1m;
1818

19-
# Change timeout values to at least the validity period of each token type
20-
keyval_zone zone=oidc_id_tokens:1M timeout=1h sync;
21-
keyval_zone zone=oidc_access_tokens:1M timeout=1h sync;
22-
keyval_zone zone=refresh_tokens:1M timeout=8h sync;
23-
keyval_zone zone=oidc_sids:1M timeout=8h sync;
24-
2519
keyval $cookie_auth_token $session_jwt zone=oidc_id_tokens; # Exchange cookie for ID token(JWT)
2620
keyval $cookie_auth_token $access_token zone=oidc_access_tokens; # Exchange cookie for access token
2721
keyval $cookie_auth_token $refresh_token zone=refresh_tokens; # Exchange cookie for refresh token

0 commit comments

Comments
 (0)