DuendeSoftware / Support

Support for Duende Software products
21 stars 0 forks source link

EntityFrameworkServerSideSessions updating Expiry even though refresh fails #1342

Closed iseneirik closed 1 month ago

iseneirik commented 3 months ago

Which version of Duende BFF are you using?

2.2.0

Which version of .NET are you using?

net8.0

Describe the bug

I'm not quite sure if this is a bug, or just poor configuration from my part, but the situation is:

I have set up a client with:

The consuming application is using bff with EF server side sessions:

// stuff
.AddBff(options => { options.EnableSessionCleanup = true; })
.AddEntityFrameworkServerSideSessions(options => options.UseSqlServer( /* ... */ ))
// more stuff

And Cookie Authentication:

.AddCookie(options =>
{
    // stuff
    options.ExpireTimeSpan = TimeSpan.FromMinutes(120);
    options.Cookie.MaxAge = TimeSpan.FromMinutes(120);
    // more stuff
})

This seems to work fine and dandy, with the user session being refreshed around every hour (halfway through the ExpireTimeSpan) resulting in the Renewed-column being updated and Expiry-column being pushed two hours. However, when we reach the "Absolute refresh token lifetime" and a refresh is attempted the following error is logged from Duende.AccessTokenManagement.OpenIdConnect.UserAccessAccessTokenManagementService:

Error refreshing access token. Error = invalid_grant

But it seems like the Expiry- and Renewed-column are both being updated even though the refresh was unsuccessful. This continues which hinders the session cleanup in clearing the session from the table.

Expected behavior

I would expect the user session to not be updated when a refresh fails so that when the session ends up invalid and a new login is requested.

As mentioned, I'm not sure if this is a bug or just poor config. If it is the latter, a point in the right direction would be appreciated!

RolandGuijt commented 2 months ago

The ServerSideSessions option just has impact on the user session which is represented by the cookie that is set by the BFF. Access and refresh tokens are meant for the APIs the BFF is accessing on behalf of the SPA. So they are used for a different process than the session. However: they are stored by the BFF in the context of the current session. So when the session ends the tokens will be gone.

When the SPA tries to access an API (after authentication) through the BFF the following happens:

Does this help?

RolandGuijt commented 2 months ago

@iseneirik Did my answer help with your problem? If you have a follow-up please post else I'll close this issue.

iseneirik commented 2 months ago

@RolandGuijt Thanks for the response, I'm just back from vacation, so didn't catch it before now.

I think the problem lies in bullet point 4:

If the access token is expired an attempt will be made to refresh it (here the refresh token config comes in play)

When the attempt to refresh the access token fails (which will be the case after absolute expiry has passed), a new cookie is still issued by the BFF with the context being updated. If the refresh fails, shouldn't this result in a 401?

RolandGuijt commented 1 month ago

The access/refresh token mechanism is separate from the session mechanism. When refreshing an access token fails this doesn't affect the session. I'm assuming that by "the context" you mean the expiry and renewed columns. They are about the session and not about access/refresh tokens. Does this make it clearer for you?

iseneirik commented 1 month ago

@RolandGuijt I see, thanks for the clarification. I thought that was how I understood it, but upon further digging I see that I didn't completely view it that way after all.

If I understand it correctly, I guess it would make sense to set SlidingExpriation to false in the Cookie-settings and make sure that the Cooke.MaxAge and ExpireTimeSpan are shorter than or equal to the Access Token lifetime? Or is there a hook somewhere that can let me invalidate a session in the ISessionStorage based on the response from an attempted Access Token Refresh?

RolandGuijt commented 1 month ago

There is the CoordinateClientLifetimesWithUserSession option in IdentityServer that could help with this. But please keep in mind this is about the identity provider session. So when a user logs out of the BFF makes sure that ends the identity provider session too.

RolandGuijt commented 1 month ago

@iseneirik Do you have anything to add to this issue? IIf so please do so. If not I would like to close it.

iseneirik commented 1 month ago

@RolandGuijt Thanks for the help. I believe I have enough to go on now, so consider it closed!