Onboard aspnetcore into the new TimeProvider type from runtime

[joperezr]

Now that TimeProvider has been added to the platform in dotnet/runtime#83604, we should make the following changes in the aspnetcore layer for .NET 8:

• Remove all the copies of ISystemClock that exist in the repo (there are at least 6, with types that implement them), particularly all the ones that are not publicly exposed, and replace the places where they are used with TimeProvider.
• Two of these copies of ISystemClock are actually public and part of some of our API surface: Microsoft.AspNetCore.Authentication.ISystemClock, and Microsoft.AspNetCore.Server.kestrel.Core.Internal.Infrastructure.ISystemClock. We should consider deprecating those as well as the affected API that exposes them, and consider if there are new APIs/overloads that should be exposed that use TimeProvider as opposed to ISystemClock.

cc: @tarekgh @Tratcher @adityamandaleeka @davidfowl @mkArtakMSFT

• Authentication Obsolete and replace ISystemClock in Security, Identity #47472 #47717
• OutputCaching Replace ISystemClock with TimeProvider in OutputCaching, ResponseCaching #47753
• ResponseCaching Replace ISystemClock with TimeProvider in OutputCaching, ResponseCaching #47753
• YARP Using new TimeProvider  yarp#2108
• SignalR Common - Replace ISystemClock in SignalR #47895
• Kestrel - Consider not using the wall clock time for timeouts in Kestrel #13628 suggests using a monotonic value like SignalR.
• Reconcile to a shared MockTimeProvider across AspNetCore. - Consider not using the wall clock time for timeouts in Kestrel #13628

[joperezr]

Not sure if area-infrastructure is the right label for this, but given it is spread across multiple parts of the repo I'll let others triage appropriately.

[Tratcher]

@SebastianC was going to look at this. Woops, sorry.

I'm interested, but will be busy for a few weeks.

[davidfowl]

I think you meant @sebastienros 😄

[BrennanConroy]

We should also be aware of #13628 when doing this. I don't know if this work will make it easier or harder to convert Kestrel to wall clock (except for the Date header obviously)

[Tratcher]

Follow up:
Microsoft.Extensions.Internal.ISystemClock used by MemoryCacheOptions.Clock, used in CertificateValidationCache.

[halter73]

API Review Notes:

Proposed API

ISystemClock and SystemClock are now obsolete.

namespace Microsoft.AspNetCore.Authentication;

+ [Obsolete]
public interface ISystemClock
{
}

+ [Obsolete]
public class SystemClock : ISystemClock
{
}

Each auth handler and security token validator is obsoleting the existing constructor and adding a new one without ISystemClock. Their Clock property is now obsolete and replaced by TimeProvider. TimeProvider can now be set via options or via DI (IPostConfigureOptions).

public AuthenticationHandler<TOptions>
{
+ [Obsolete]
  protected AuthenticationHandler(IOptionsMonitor<TOptions> options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock)
+ protected AuthenticationHandler(IOptionsMonitor<TOptions> options, ILoggerFactory logger, UrlEncoder encoder)
+ [Obsolete]
  public ISystemClock Clock { get; }
+ public TimeProvider TimeProvider { get; }
}
public AuthenticaitonSchemeOptions
{
+ public TimeProvider? TimeProvider { get; set; }
}

public SecuirtyTokenValidator
{
+ [Obsolete]
  public SecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ISystemClock clock, ILoggerFactory logger)
+ public SecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ILoggerFactory logger)
+ [Obsolete]
  public ISystemClock Clock { get; }
+ public TimeProvider TimeProvider { get; }
}
public SecurityStampValidatorOptions
{
+ public TimeProvider? TimeProvider { get; set; }
}

---

• What do we think about the pattern of adding a new obsolete constructor?
  • [EditorBrowsable(Never)]? It could help with the OAuthHandler in particular when the derived types call base.
• What's the plan after .NET 8?
  • Remove default ISystemClock registration in AddAuthentication. Maybe keep the interface around for people who add it manually.
• Can we teach DI not to use [Obsolete] constructors
  • We'd have to change the contract for third-party DI containers to do the same. We cannot design an API expecting that feature right now.
• Can we add a TimeProvider property to AuthenticationSchemeOptions instead of adding the TimeProvider directly to DI?
  • This could make it harder to replace the TimerProvider globally for every scheme.

We'd prefer not to add new obsolete constructor to every auth handler if possible, but we don't like any of the other alternatives.

API Approved! (@Tratcher plans to fill in the "Proposed API" section above)

Updated proposal.

[davidfowl]

Why is the property and argument name "time" instead of "timeProvider"

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• We removed the TimeProvider accepting constructors from auth and identity and use properties on options instead.
• What do we think about TimeProvider being nullable instead of preset to TimeProvider.System?
  • Nullable makes it more obvious if it's been overridden.
• Do we like TimeProvider over Time for the property names? Yes.
• No namespace were modified.

namespace Microsoft.AspNetCore.Authentication;

+ [Obsolete]
public interface ISystemClock
{
}

+ [Obsolete]
public class SystemClock : ISystemClock
{
}

public AuthenticationHandler<TOptions>
{
+ [Obsolete]
  protected AuthenticationHandler(IOptionsMonitor<TOptions> options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock)
+ protected AuthenticationHandler(IOptionsMonitor<TOptions> options, ILoggerFactory logger, UrlEncoder encoder)
+ [Obsolete]
  public ISystemClock Clock { get; }
+ public TimeProvider TimeProvider { get; }
}

public AuthenticaitonSchemeOptions
{
+ public TimeProvider? TimeProvider { get; set; }
}

public SecuirtyTokenValidator
{
+ [Obsolete]
  public SecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ISystemClock clock, ILoggerFactory logger)
+ public SecurityStampValidator(IOptions<SecurityStampValidatorOptions> options, SignInManager<TUser> signInManager, ILoggerFactory logger)
+ [Obsolete]
  public ISystemClock Clock { get; }
+ public TimeProvider TimeProvider { get; }
}

public SecurityStampValidatorOptions
{
+ public TimeProvider? TimeProvider { get; set; }
}

API Approved as updated!