Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion pkg/authserver/storage/redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ func (s *RedisStorage) RegisterClient(ctx context.Context, client fosite.Client)
return fmt.Errorf("failed to marshal client: %w", err)
}

// Public clients (from DCR) expire to prevent unbounded growth.
// Public clients (from DCR) expire to prevent unbounded growth; GetClient
// renews this TTL on use so actively-used clients are not evicted.
// Confidential clients don't expire.
ttl := time.Duration(0)
if client.IsPublic() {
Expand All @@ -192,6 +193,16 @@ func (s *RedisStorage) RegisterClient(ctx context.Context, client fosite.Client)
}

// GetClient loads the client by its ID.
//
// For public (DCR) clients, a successful load renews the registration TTL to
// DefaultPublicClientTTL, making expiry a sliding window of inactivity rather than
// a fixed window from registration time. Without this, an actively-used public
// client is evicted DefaultPublicClientTTL after it registered regardless of use,
// which surfaces to the client as invalid_client on its next token request and
// forces a full re-registration. Abandoned clients still expire after the
// inactivity window, preserving the anti-bloat intent of RegisterClient.
// Confidential clients are stored without a TTL and are left untouched. TTL
// renewal is best-effort: a failure is logged and does not fail the lookup.
func (s *RedisStorage) GetClient(ctx context.Context, id string) (fosite.Client, error) {
key := redisKey(s.keyPrefix, KeyTypeClient, id)

Expand All @@ -208,6 +219,15 @@ func (s *RedisStorage) GetClient(ctx context.Context, id string) (fosite.Client,
return nil, fmt.Errorf("failed to unmarshal client: %w", err)
}

// Renew the registration TTL on use so actively-used public (DCR) clients are
// not evicted mid-lifecycle. Mirrors the TTL set in RegisterClient; confidential
// clients have no TTL and are left untouched.
if stored.Public {
if err := s.client.Expire(ctx, key, DefaultPublicClientTTL).Err(); err != nil {
slog.Warn("failed to renew public client TTL", "client_id", id, "error", err)
}
}

return &redisClient{storedClient: stored}, nil
}

Expand Down
46 changes: 46 additions & 0 deletions pkg/authserver/storage/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,52 @@ func TestRedisStorage_RegisterClient(t *testing.T) {
})
}

// TestRedisStorage_GetClientTTLRenewal verifies that GetClient renews the
// registration TTL for public (DCR) clients on use — making expiry a sliding
// window of inactivity — while leaving confidential clients (which have no TTL)
// untouched. Without renewal an actively-used public client is evicted
// DefaultPublicClientTTL after registration and the client then fails with
// invalid_client on its next token request.
func TestRedisStorage_GetClientTTLRenewal(t *testing.T) {
t.Parallel()

t.Run("public client TTL is renewed on read", func(t *testing.T) {
withRedisStorage(t, func(ctx context.Context, s *RedisStorage, mr *miniredis.Miniredis) {
key := redisKey(s.keyPrefix, KeyTypeClient, "public-client")
require.NoError(t, s.RegisterClient(ctx, &mockClient{id: "public-client", public: true}))

// Age the registration so only a small slice of the TTL remains.
mr.FastForward(DefaultPublicClientTTL - time.Hour)
ttlBefore := mr.TTL(key)
require.Positive(t, ttlBefore)
require.Less(t, ttlBefore, 2*time.Hour, "precondition: TTL should be near expiry")

client, err := s.GetClient(ctx, "public-client")
require.NoError(t, err)
assert.Equal(t, "public-client", client.GetID())

ttlAfter := mr.TTL(key)
assert.Greater(t, ttlAfter, ttlBefore, "GetClient should renew the public client TTL")
assert.InDelta(t, DefaultPublicClientTTL.Seconds(), ttlAfter.Seconds(), 60,
"renewed TTL should be ~DefaultPublicClientTTL")
})
})

t.Run("confidential client stays persistent on read", func(t *testing.T) {
withRedisStorage(t, func(ctx context.Context, s *RedisStorage, mr *miniredis.Miniredis) {
key := redisKey(s.keyPrefix, KeyTypeClient, "conf-client")
require.NoError(t, s.RegisterClient(ctx, &mockClient{id: "conf-client", public: false}))
require.Equal(t, time.Duration(0), mr.TTL(key), "confidential clients must not have a TTL")

_, err := s.GetClient(ctx, "conf-client")
require.NoError(t, err)

assert.Equal(t, time.Duration(0), mr.TTL(key),
"GetClient must not introduce a TTL on a confidential client")
})
})
}

func TestRedisStorage_ClientAssertionJWT(t *testing.T) {
t.Parallel()

Expand Down