IdentityServer / IdentityServer3.AccessTokenValidation

OWIN Middleware to validate access tokens from IdentityServer3
Apache License 2.0
91 stars 149 forks source link

Performance issues with ValidationMode.Local #154

Open SingleCopy opened 7 years ago

SingleCopy commented 7 years ago

The latest change introduced in v2.15.0 has a huge performance issue when the validation mode is set to local. The change was to remove the time check in the DiscoveryDocumentIssuerSecurityTokenProvider class when the security tokens should be refreshed.

Now the configurationManager.GetConfigurationAsync() is checking the AutomaticRefreshInterval and only returning a new OpenIdConnectConfiguration if it has been refreshed. However the RetrieveMetadata method is called before getting the Issuer, Audience and SecurityTokens. This is blocking other incoming http requests as the ReaderWriterLockSlim is locking for each RetrieveMetadata call.

I have created a new pull request that basically reverts some of the code so that the RetrieveMetadata method will only attempt to recreate the Issuer and SecurityTokens if the AutomaticRefreshInterval has lapsed.

For now, I have downgraded to v2.14.0. Pull Request: https://github.com/IdentityServer/IdentityServer3.AccessTokenValidation/pull/155

mclark1129 commented 7 years ago

This has also affected a production deployment for one of our APIs, causing timeouts when the API is under heavy load.

MartinWa commented 6 years ago

We have had intermittent timeouts and slow performance in our API for a while and running the profiler in Azure I found the following for a slow API call:

IdentityServerBearerTokenValidationMiddleware.Invoke | 4100.19 ms |  
OTHER <<AsyncTaskMethodBuilder.Start>> | 4100.19 ms |  
IdentityServerBearerTokenValidationMiddleware+<Invoke>d__8.MoveNext | 4100.19 ms |  
OTHER <<AuthenticationMiddleware`1.Invoke>> | 4096.43 ms |  
DiscoveryDocumentIssuerSecurityTokenProvider.get_SecurityTokens | 2870.17 ms |  
DiscoveryDocumentIssuerSecurityTokenProvider.RetrieveMetadata | 2870.17 ms |  
OTHER <<ReaderWriterLockSlim.TryEnterWriteLock>> | 2338.90 ms |  
BLOCKED_TIME | 2338.59 ms |  
CPU_TIME | 0.31 ms |  

After finding this issue I also downgraded to 2.14 and it seems like it solved the timeout problems we had.

brockallen commented 6 years ago

We did an update that's supposed to deal with the deadlock in Microsoft's config loader. Please test again the myget version and let us know. https://www.myget.org/gallery/identity

Thanks

kedwardsRenWeb commented 6 years ago

We believe we're seeing this as well.... did anyone test this latest 2.15.1? @brockallen I noticed this wasn't ever published to NuGet...

We went live with IdSrvr at the very end of June... and our first deployment was on 2.15.0.

Would you recommend us downgrading to 2.14 or utilizing 2.15.1?

morrisnc commented 6 years ago

@kedwardsRenWeb That would be a good start! We have not tested 2.15.1 yet as 2.14 has worked without issues and our next planned upgrade will move to .NET Core.

leastprivilege commented 6 years ago

Well - if no-one gives the bug fix a try, we'll never find out.