Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bff with EF Server Side Sessions: Cannot insert duplicate key row in object 'Session.UserSessions' #1497

Open
iseneirik opened this issue Dec 2, 2024 · 10 comments
Assignees

Comments

@iseneirik
Copy link

Which version of Duende BFF are you using?

2.2.0

Which version of .NET are you using?

net8.0

Describe the bug

We have an API running four instances across two machines (two instances each). The API is set up with Bff and .AddEntityFrameworkServerSideSessions() and works fine. However, a few times each day (0 - 10) our logs show the following error:

Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while saving the entity changes. See the inner exception for details.
 ---> Microsoft.Data.SqlClient.SqlException (0x80131904): Cannot insert duplicate key row in object 'Session.UserSessions' with unique index 'IX_UserSessions_ApplicationName_SessionId'. The duplicate key value is (<AppName>, <SessionIdValue>).
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)
   at Microsoft.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)
   at Microsoft.Data.SqlClient.SqlDataReader.ReadAsyncExecute(Task task, Object state)
   at Microsoft.Data.SqlClient.SqlDataReader.InvokeAsyncCall[T](SqlDataReaderBaseAsyncCallContext`1 context)
--- End of stack trace from previous location ---
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetAsync(Int32 startCommandIndex, RelationalDataReader reader, CancellationToken cancellationToken)
ClientConnectionId:<...>
Error Number:2601,State:1,Class:14
   --- End of inner exception stack trace ---
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetAsync(Int32 startCommandIndex, RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeAsync(RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Update.Internal.SqlServerModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

I'm speculating that this might be because a user has multiple tabs open, and when they open the browser for the first time after the cookie has expired, they all simultaneously log in, causing a race-condition of sorts.

To Reproduce

Unable to reproduce

Expected behavior

I'm not quite sure... I'm uncertain if this error is ignorable or not. I'm assuming the conflict comes from the data already being stored from the call that won the race to Db, if so, maybe something like:

if DbUpdateException and the conflicting SessionId results in the same data attempting to be stored, ignore
else rethrow

Log output/exception with stacktrace

Already in description

@iseneirik iseneirik added the BFF label Dec 2, 2024
@RolandGuijt RolandGuijt self-assigned this Dec 5, 2024
@RolandGuijt
Copy link

Do I understand correctly that you're running a BFF that has internal API endpoints scaled out across 4 instances?

@iseneirik
Copy link
Author

Yes, that is correct. So instead of one BFF-instance proxying to four separate instances, we simply have four BFF-instances.

@RolandGuijt
Copy link

Can you please tell us more about how you've set this up?

  • Is there just one SPA for the BFF or are there multiple?
  • Can you share the startup code in program.cs for the BFF?
  • Any customizations you have active that have to do with sessions?

@iseneirik
Copy link
Author

Is there just one SPA for the BFF or are there multiple?

There is just one SPA for the BFF, this is served by IIS separately (not served by the BFF). Both the SPA and all instances of the BFF are behind a load balancer (NGINX), with all calls to /api/* being routed to the BFF.

Can you share the startup code in program.cs for the BFF?

We have quite a large program.cs, but here is an excerpt of the authentication-related parts:

builder.Services
    .AddBff(options =>
    {
        options.EnableSessionCleanup = true;
        options.ManagementBasePath = BffBasePath;
        options.LicenseKey = identityConfiguration.DuendeLicenseKey;
    })
    .AddEntityFrameworkServerSideSessions(options => options
        .UseSqlServer(connectionString, sqlServerOptions =>
        {
            sqlServerOptions.UseCompatibilityLevel(120);
            sqlServerOptions.MigrationsAssembly(Assembly.GetExecutingAssembly().FullName);
            sqlServerOptions.MigrationsHistoryTable(MigrationTableName, SchemaName);
        }))
    .ConfigureEntityFrameworkSessionStoreOptions(options => options.DefaultSchema = SchemaName)
    .AddAuthentication(options =>
    {
        options.DefaultScheme = CookieAuthenticationDefaults.AuthenticationScheme;
        options.DefaultChallengeScheme = OpenIdConnectDefaults.AuthenticationScheme;
        options.DefaultSignOutScheme = OpenIdConnectDefaults.AuthenticationScheme;
    })
    .AddCookie(options =>
    {
        options.Cookie.Name = identityConfiguration.CookieName;
        options.Cookie.SameSite = SameSiteMode.Strict;
        options.ExpireTimeSpan = TimeSpan.FromMinutes(identityConfiguration.CookieExpirationMinutes);
        options.Cookie.MaxAge = TimeSpan.FromMinutes(identityConfiguration.CookieExpirationMinutes);
        options.SlidingExpiration = identityConfiguration.SlidingExpiration;
        options.Cookie.SecurePolicy = CookieSecurePolicy.Always;
        options.Cookie.HttpOnly = true;
    })
    .AddOpenIdConnect(options =>
    {
        options.CallbackPath = "/api/oidc/signin";
        options.SignedOutCallbackPath = "/api/oidc/signout-callback";
        options.Authority = identityConfiguration.Authority;
        options.ClientId = identityConfiguration.ClientId;
        options.ClientSecret = identityConfiguration.ClientSecret;
        options.ResponseType = OpenIdConnectResponseType.Code;
        options.ResponseMode = OpenIdConnectResponseMode.Query;
        options.UsePkce = true;
        options.GetClaimsFromUserInfoEndpoint = true;
        options.MapInboundClaims = true;
        options.SaveTokens = true;
        options.ClaimActions.Add(new MapAllClaimsAction());
        options.TokenValidationParameters.NameClaimType = "name";
        options.TokenValidationParameters.RoleClaimType = "role";
        options.Scope.Clear();
        identityConfiguration.Scopes.ForEach(options.Scope.Add);
        options.Events.OnRedirectToIdentityProvider = AddAcrValues;
        options.Events.OnRemoteFailure = HandleRemoteFailure;
    });

// ... more setup

var app = builder.Build();

// NGINX sets the X-Forwarded-... headers
app.UseForwardedHeaders();

// ... more setup

app
    .UseAuthentication()
    .UseMiddleware<ForumWebClaimsMiddleware>()
    .UseBff()
    .UseAuthorization();
app.MapBffManagementEndpoints();
app.MapReverseProxy(config => config.UseAntiforgeryCheck());
app.MapControllers().AsBffApiEndpoint();

app.Run();

Any customizations you have active that have to do with sessions?

From the setup you can se that we use ServerSideSessions with EF-backed storage. Can't think of any particular customizations.

If there is more info I can provide you with, please let me know 🙂

@iseneirik
Copy link
Author

@RolandGuijt, is there any more information you need?

@RolandGuijt
Copy link

Just to rule it out, can you please check the data protection settings against this document?

If that doesn't lead to a solution, please share the data protection configuration code.

@iseneirik
Copy link
Author

iseneirik commented Dec 17, 2024

@RolandGuijt our Data Protection should be solid:

builder.Services
    .AddDbContext<ForumWebApiKeysContext>(/* ...options... */)
    .AddDataProtection(configure => configure.ApplicationDiscriminator = "...")
    .SetApplicationName("...")
    .PersistKeysToDbContext<ForumWebApiKeysContext>()
    .ProtectKeysWithDpapiNG();

And as mentioned, the four instances work fine together, so I would be surprised to find out that data protection is involved. At our peak hours I would estimate we have somewhere around 700 unique users with one or more sessions (multiple devices). The error is only popping up a few times each day (~10).

How is the SessionId calculated? And if two concurrent calls from the same browser attempt to renew their session (eg. with /silent-login), would these calls arrive at the same SessionId?

edit: the ApplicationDiscriminator and SetApplicationName are the same, though I've learned that SetApplicationName also sets ApplicationDiscriminator, so doing both is unnecessary

@RolandGuijt
Copy link

RolandGuijt commented Dec 17, 2024

The sid claim (sid = sessionid) is used to populate the SessionId column in the UserSessions table.
Can you please check if the identity provider returns the sid claim and a sub (user subject id) claim?

@iseneirik
Copy link
Author

@RolandGuijt As our UserSessions table is populated with sessions with SessionId and SubjectId set, it would be safe to assume that both values are returned, or am I mistaken? Either way, I'm pretty sure that both sid and sub are returned from IdentityServer.

And in this case, both the SubjectId and SessionId columns are non-nullable, so this would result in another exception entirely. The case here is that there is a conflict, there is already an entry in UserSessions with the same SessionId as the one attempted being stored, which is why I assume that some race-condition between multiple tabs is the culprit. All tabs will likely have an active session against Duende IdentityServer (the same session, as it is stored in a Cookie), in which case they will recieve the same sid and attempt to populate the table with the same SessionId in the case that they all do a /silent-login at the same time.

Have you considered the suggestion in "Expected Behaviour" of the original post?

@AndersAbel
Copy link
Member

We've done some further analysis and think that this might be a but in our library. If multiple tabs/windows/requests run the login flow when there is already an existing server side session it looks like we are not handling that correctly.

We have to do some further investigations, so I'm leaving this open for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants