-
Notifications
You must be signed in to change notification settings - Fork 4k
Stackoverflow exception when calling the Identity Server with an expired cookie. #4844
Comments
Meanwhile I've created a fix for me. public class FixedDefaultUserSession : IdentityServer4.Services.DefaultUserSession
{
bool _authenticateAsyncRunning = false;
public FixedDefaultUserSession(IHttpContextAccessor httpContextAccessor, IAuthenticationHandlerProvider handlers, IdentityServerOptions options, ISystemClock clock, ILogger<IUserSession> logger)
: base(httpContextAccessor, handlers, options, clock, logger)
{
}
protected override Task AuthenticateAsync()
{
if (_authenticateAsyncRunning)
return Task.CompletedTask;
try
{
_authenticateAsyncRunning = true;
return base.AuthenticateAsync();
}
finally
{
_authenticateAsyncRunning = false;
}
}
} and add this to the bottom of the services.RemoveAll<IdentityServer4.Services.IUserSession>();
services.AddScoped<IdentityServer4.Services.IUserSession, FixedDefaultUserSession>(); |
PR submitted. When we release a preview for the next patch, please test. Thanks. |
Hi @brockallen,
Which is outside the if block, and you can't put it in, because AuthenticateAsync(); should fill the Properties property if the user is not authenticated yet.
I'd be happy to provide you with a pull request containing a test that triggers this issue, and a possible fix. |
I'll have a look again. I thought I had repro'd your issue, but perhaps it was not the same. |
I just tried to reproduce it, but for whatever reason, couldn't... |
I debugged thru it again. Here's the sequence: Auth middleware calls Authenticate => Our auth decorator passes thru => CookieHandler.HandleAuth => raises ValidatePrincipal event => ASP.NET Identity calls Reject, and then Signout => Our auth decorator sets signed out flag, and passes thru => CookieHandler.Signout Perhaps this fix in a prior release is what fixed it for you? #4670 It used to work like this: Auth middleware calls Authenticate => Our auth decorator passes thru => CookieHandler.HandleAuth => raises ValidatePrincipal event => ASP.NET Identity calls Reject, and then Signout => Our auth decorator calls into UserSession service => UserSession calls Authenticate => repeat above |
Also, I still think the PR fixes a bug (perhaps a different one) :) |
…_for_expired_cookie add defensive check to fix bug for when session is expired #4844
I thought I've tried it with v4.0.4 which should contain this fix already. But maybe I used the wrong version somehow. Anyway, thank you very much for the work you've done. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
When we call our IdentityServer it crashes with a stack overflow.
What we've found out is that
Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler
sHandleAuthenticateAsync
method callsMicrosoft.AspNetCore.Identity.SignInManager.SignOutAsync
when the cookie provided is not valid or expired. This sign out get picked up by the identity server to create aLogoutNotificationContext
to distribute this logout to all clients the user was logged in, and callsDefaultUserSession.GetClientListAsync
which requires a logged on user and that fore callsAuthenticateAsync
, and the loop is complete.We are using version 4.0.4 which is the latest one currently.
Here are the packages we use:
Here is the DI configuration and startup code:
I've created a fix for that already: Ausm/IdentityServer4@e7c5b07
I just didn't want to create a PR unless this is discussed in an issue to respect your contributing guidelines.
Issue / Steps to reproduce the problem
Unfortunately this happens on different Machines, and I haven't found out why it happens sometimes and sometimes not.
Relevant parts of the log file
The log file just repeats
over and over again until it fails.
Whats more helpful is a stack trace after the 5th "Failed to validate a security stamp." log:
The text was updated successfully, but these errors were encountered: