Potential issue with unawaited UpdateClientAssertion event

[ovska]

https://github.com/IdentityModel/IdentityModel.AspNetCore.OAuth2Introspection/blob/main/src/OAuth2IntrospectionHandler.cs#L211

The OnUpdateClientAssertion event is invoked without an await, probably because it is inside a lock. This can potentially cause hard to diagnose bugs if the event is assigned to something that requires awaiting. For example the current HttpContext is one of the properties, and using it after the request is ended can throw object disposed exceptions.

My suggestion is to replace the lock with a SemaphoreSlim which works fine in async context as well, and the lifetime of OAuth2IntrospectionOptions shouldn't be a problem as SemaphoreSlim wouldn't need to be disposed in this case.

Tangentially, ConfigureAwait(false) is missing from some all awaits on events. Is this intentional or an oversight?

[leastprivilege]

My understanding is that ASP.NET Core has no synchronization context - so ConfigureAwait(false) is not required.

Would you agree?

[ovska]

Correct, was just wondering why it was used in some awaits and not all :)

[leastprivilege]

I guess because it is ultimately an old code base..

[leastprivilege]

@brockallen could you have a look when time permits.

@ovska do you want to propose a PR?

[ovska]

Here's more or less the minimal fix I could come up with: #164