Repository navigation
Some code reads DateTime.UtcNow instead of the TimeProvider #145
Description
Most of the code takes the time from the registered TimeProvider, so a host or a test can control the clock. A few places read DateTime.UtcNow directly and ignore it. Found while checking for #133 (PR #144), which is about a different problem: two reads of the same clock disagreeing.
| Where | Uses the system clock for | Effect |
|---|---|---|
InMemorySessionStore.GetAndRemoveExpiredSessions |
which sessions count as expired | Expired-session cleanup ignores the TimeProvider, so a test that advances a FakeTimeProvider can't expire sessions in the in-memory store. The EF IdentityServerServerSideSessionStore already uses the TimeProvider for the same query. |
TokenCleanupService (EntityFramework.Storage): RemoveGrantsAsync, RemoveDeviceCodesAsync, RemovePushedAuthorizationRequestsAsync |
which rows count as expired | Inside the query, EF Core translates DateTime.UtcNow to the database's own clock (GETUTCDATE() on SQL Server), while the expiry values were written with the app's TimeProvider. Clock skew between the app and database servers moves the cleanup earlier or later, and a fake clock has no effect. |
GrantValidationResult(subject, authenticationMethod, claims, …), the constructor without authTime |
the auth_time claim |
Custom resource owner and extension grant validators that use this constructor get the system time as auth_time. TestUserResourceOwnerPasswordValidator already passes the clock explicitly. |
Message<TModel>(TModel data) |
Created |
Documented as "Should only be used from unit tests", but ReturnUrlResult and both authorization parameters message stores call it. Nothing reads Created on those paths, so this is cosmetic. |
Created = DateTime.UtcNow defaults on the EF Client, ApiResource, IdentityResource and Secret entities |
the Created column of configuration rows |
Cosmetic. Entities can't take a TimeProvider, so this may be fine as it is. |
The sample hosts' AccountController also uses DateTimeOffset.UtcNow, but that's sample code.
Possible fix for the first two: take a TimeProvider through the constructor. For TokenCleanupService, read it once before the queries so EF sends the value as a parameter. Both constructors are public, so an added overload keeps it non-breaking. For GrantValidationResult, marking the constructor without authTime as obsolete would point implementers to the one that takes it. That's your call.
Happy to pick this up if you agree.
Activity