refactor: simplify per-connector expiry helpers

- Reuse the existing server.value() helper in idTokensValidForConn
  instead of a hand-rolled zero-check.
- Drop the duplicate per-connector refresh-token summary log; thread
  the connector_id into NewRefreshTokenPolicy via logger.With so the
  existing per-field logs carry it.
- Trim WHAT-only docstrings and tighten the rationale on
  ConnectorRefreshToken and buildConnectorExpiryOverrides.

Signed-off-by: Helio Machado <0x2b3bfa0+git@googlemail.com>

Author: 0x2b3bfa0

cmd/dex/config.go

@@ -566,20 +566,14 @@ type Connector struct {
 	Expiry *ConnectorExpiry `json:"expiry,omitempty"`
 }
 
-// ConnectorExpiry holds per-connector overrides for token lifetimes.
 type ConnectorExpiry struct {
-	// IDTokens overrides expiry.idTokens for tokens issued through this connector.
-	IDTokens string `json:"idTokens"`
-
-	// RefreshTokens overrides expiry.refreshTokens for sessions established
-	// through this connector. Any field left unset falls back to the global
-	// refresh token policy.
+	IDTokens      string                 `json:"idTokens"`
 	RefreshTokens *ConnectorRefreshToken `json:"refreshTokens"`
 }
 
-// ConnectorRefreshToken holds per-connector refresh token policy overrides.
-// Fields mirror the top-level RefreshToken struct; a nil pointer on DisableRotation
-// signals "inherit the global value", while the other string fields inherit when empty.
+// ConnectorRefreshToken mirrors RefreshToken but uses a pointer for
+// DisableRotation so that "unset" can be distinguished from "false",
+// allowing the field to inherit the global value when nil.
 type ConnectorRefreshToken struct {
 	DisableRotation   *bool  `json:"disableRotation"`
 	ReuseInterval     string `json:"reuseInterval"`

cmd/dex/serve.go

@@ -841,11 +841,9 @@ func parseSessionConfig(s *Sessions) (*server.SessionConfig, error) {
 }
 
 // buildConnectorExpiryOverrides parses per-connector `expiry` overrides.
-// Per-connector `idTokens` must not exceed the global `expiry.idTokens` value,
-// since the signer's key retention window is sized from it. Refresh token
-// fields have no such constraint: they are validated against storage, not
-// against signing keys. Connectors without an override are omitted from the
-// returned map and inherit the global policy at runtime.
+// Per-connector `idTokens` must not exceed the global value, since the signer's
+// key retention window is sized from it. Refresh-token fields have no such
+// constraint: they are validated against storage, not against signing keys.
 func buildConnectorExpiryOverrides(
 	logger *slog.Logger,
 	connectors []Connector,
@@ -910,18 +908,12 @@ func parseConnectorExpiry(
 		reuseInterval = globalRefresh.ReuseInterval
 	}
 
-	policy, err := server.NewRefreshTokenPolicy(logger, disableRotation, validIfNotUsedFor, absoluteLifetime, reuseInterval)
+	connLogger := logger.With("connector_id", conn.ID)
+	policy, err := server.NewRefreshTokenPolicy(connLogger, disableRotation, validIfNotUsedFor, absoluteLifetime, reuseInterval)
 	if err != nil {
 		return override, fmt.Errorf("refresh token policy: %v", err)
 	}
 	override.RefreshTokenPolicy = policy
-	logger.Info("config connector refresh tokens",
-		"connector_id", conn.ID,
-		"valid_if_not_used_for", validIfNotUsedFor,
-		"absolute_lifetime", absoluteLifetime,
-		"reuse_interval", reuseInterval,
-		"rotation_enabled", !disableRotation,
-	)
 	return override, nil
 }
 

server/oauth2.go

@@ -343,20 +343,13 @@ func genSubject(userID string, connID string) (string, error) {
 	return internal.Marshal(sub)
 }
 
-// idTokensValidForConn returns the ID token lifetime for the given connector,
-// falling back to the server-wide value when no per-connector override is set.
 func (s *Server) idTokensValidForConn(connID string) time.Duration {
-	if o, ok := s.connectorExpiryOverrides[connID]; ok && o.IDTokensValidFor > 0 {
-		return o.IDTokensValidFor
-	}
-	return s.idTokensValidFor
+	return value(s.connectorExpiryOverrides[connID].IDTokensValidFor, s.idTokensValidFor)
 }
 
-// refreshTokenPolicyForConn returns the refresh token policy for the given
-// connector, falling back to the server-wide policy when no override is set.
 func (s *Server) refreshTokenPolicyForConn(connID string) *RefreshTokenPolicy {
-	if o, ok := s.connectorExpiryOverrides[connID]; ok && o.RefreshTokenPolicy != nil {
-		return o.RefreshTokenPolicy
+	if p := s.connectorExpiryOverrides[connID].RefreshTokenPolicy; p != nil {
+		return p
 	}
 	return s.refreshTokenPolicy
 }

server/oauth2_test.go

@@ -1311,8 +1311,6 @@ func TestRefreshTokenPolicyForConn(t *testing.T) {
 		"missing entry should fall back to global")
 }
 
-// TestNewIDTokenUsesConnectorOverride verifies that newIDToken applies the
-// per-connector idTokensValidFor override at issuance time, not the global.
 func TestNewIDTokenUsesConnectorOverride(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()

server/server.go

@@ -213,7 +213,7 @@ func value(val, defaultValue time.Duration) time.Duration {
 }
 
 // ConnectorExpiryOverride carries per-connector token lifetime overrides.
-// A zero IDTokensValidFor or nil RefreshTokenPolicy inherits the global value.
+// A zero or nil field inherits the global value.
 type ConnectorExpiryOverride struct {
 	IDTokensValidFor   time.Duration
 	RefreshTokenPolicy *RefreshTokenPolicy
@@ -257,7 +257,6 @@ type Server struct {
 
 	refreshTokenPolicy *RefreshTokenPolicy
 
-	// connectorExpiryOverrides holds per-connector overrides for token lifetimes.
 	connectorExpiryOverrides map[string]ConnectorExpiryOverride
 
 	logger *slog.Logger