DuendeSoftware/Support

Issue with Entity Framework v7 and SQL table triggers

Opened this issue · 6 comments

mb418 commented

Which version of Duende IdentityServer are you using?

7.0.8

Which version of .NET are you using?

.NET 8

Describe the bug

We recently upgraded our IdentityServer from 3.1.4 to 7.0.8 following the excellent migration guides. Everything was working fine; however we host IdentityServer in two regions in Azure and use a service called Azure Data Sync to keep the two identity databases synchronised. On version 3.1.4 of IdentityServer, this has worked fine for years, but after upgrading to 7.0.8 we're receiving errors due to the incompatibility with Entity Framework v7 and the SQL table triggers created by Azure Data Sync.

"Could not save changes because the target table has database triggers. Please configure your table accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause for more information."

We can recreate the issue without Azure Data Sync by manually adding Triggers to SQL tables.

Microsoft have documented workarounds for this issue:

https://learn.microsoft.com/en-us/ef/core/what-is-new/ef-core-7.0/breaking-changes?tabs=v7#sqlserver-tables-with-triggers

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Blog>()
        .ToTable(tb => tb.HasTrigger("SomeTrigger"));
}

However, we haven't found success following Microsoft's guidance, and we were wondering if this is something that should be configurable through the IdentityServer nuget package.

To Reproduce

  1. Set up an instance of IdentityServer version 7.0.8 backed by SQL Server (we're using Azure SQL Server)
  2. Add a Trigger to one of the database tables (e.g. dbo.Clients)
  3. IdentityServer seems to fail💥

Expected behavior

We would expect HasTrigger to be easily configurable for the tables managed by IdentityServer.

Log output/exception with stacktrace

Message: Could not save changes because the target table has database triggers. Please configure your table accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause for more information.
Microsoft.EntityFrameworkCore.SqlServer.Update.Internal.SqlServerModificationCommandBatch+<ExecuteAsync>d__15.MoveNext():216
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor+<ExecuteAsync>d__9.MoveNext():889
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor+<ExecuteAsync>d__9.MoveNext():1289
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor+<ExecuteAsync>d__9.MoveNext():1807
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager+<SaveChangesAsync>d__111.MoveNext():157
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager+<SaveChangesAsync>d__115.MoveNext():199
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy+<ExecuteAsync>d__7`2.MoveNext():147
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.EntityFrameworkCore.DbContext+<SaveChangesAsync>d__63.MoveNext():369
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
Microsoft.EntityFrameworkCore.DbContext+<SaveChangesAsync>d__63.MoveNext():953
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Duende.IdentityServer.EntityFramework.Stores.PersistedGrantStore+<StoreAsync>d__4.MoveNext() in /_/src/EntityFramework.Storage/Stores/PersistedGrantStore.cs:78
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Duende.IdentityServer.Stores.DefaultGrantStore`1+<StoreItemByHashedKeyAsync>d__25.MoveNext() in /_/src/IdentityServer/Stores/Default/DefaultGrantStore.cs:231
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Duende.IdentityServer.Stores.DefaultGrantStore`1+<CreateItemAsync>d__23.MoveNext() in /_/src/IdentityServer/Stores/Default/DefaultGrantStore.cs:177
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
 
Duende.IdentityServer.ResponseHandling.AuthorizeResponseGenerator+<CreateHybridFlowResponseAsync>d__9.MoveNext() in /_/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs:125
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
 
Duende.IdentityServer.ResponseHandling.AuthorizeResponseGenerator+<CreateResponseAsync>d__8.MoveNext() in /_/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs:108
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
 
Duende.IdentityServer.Endpoints.AuthorizeEndpointBase+<ProcessAuthorizeRequestAsync>d__17.MoveNext() in /_/src/IdentityServer/Endpoints/AuthorizeEndpointBase.cs:148
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
Duende.IdentityServer.Endpoints.AuthorizeEndpointBase+<ProcessAuthorizeRequestAsync>d__17.MoveNext() in /_/src/IdentityServer/Endpoints/AuthorizeEndpointBase.cs:83
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
 
Duende.IdentityServer.Endpoints.AuthorizeCallbackEndpoint+<ProcessAsync>d__1.MoveNext() in /_/src/IdentityServer/Endpoints/AuthorizeCallbackEndpoint.cs:51
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
 
Duende.IdentityServer.Hosting.IdentityServerMiddleware+<Invoke>d__3.MoveNext() in /_/src/IdentityServer/Hosting/IdentityServerMiddleware.cs:106
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
Duende.IdentityServer.Hosting.IdentityServerMiddleware+<Invoke>d__3.MoveNext() in /_/src/IdentityServer/Hosting/IdentityServerMiddleware.cs:128
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Duende.IdentityServer.Hosting.MutualTlsEndpointMiddleware+<Invoke>d__4.MoveNext() in /_/src/IdentityServer/Hosting/MutualTlsEndpointMiddleware.cs:95
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.AspNetCore.Authentication.AuthenticationMiddleware+<Invoke>d__6.MoveNext():958
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Duende.IdentityServer.Hosting.DynamicProviders.DynamicSchemeAuthenticationMiddleware+<Invoke>d__3.MoveNext() in /_/src/IdentityServer/Hosting/DynamicProviders/DynamicSchemes/DynamicSchemeAuthenticationMiddleware.cs:51
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Duende.IdentityServer.Hosting.BaseUrlMiddleware+<Invoke>d__2.MoveNext() in /_/src/IdentityServer/Hosting/BaseUrlMiddleware.cs:27
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Web.Middleware.LoadingScreenMiddleware+<ProcessLoadingScreen>d__1.MoveNext() in D:\a\1\s\src\Web\Middleware\LoadingScreenMiddleware.cs:35
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Web.Middleware.LoadingScreenMiddleware+<InvokeAsync>d__0.MoveNext() in D:\a\1\s\src\Web\Middleware\LoadingScreenMiddleware.cs:18
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.AspNetCore.Builder.UseMiddlewareExtensions+InterfaceMiddlewareBinder+<>c__DisplayClass2_0+<<CreateMiddleware>b__0>d.MoveNext():260
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.AspNetCore.Authorization.AuthorizationMiddleware+<Invoke>d__11.MoveNext():747
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
 
Microsoft.AspNetCore.Authentication.AuthenticationMiddleware+<Invoke>d__6.MoveNext():958
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.TaskAwaiter.GetResult()
 
Mindscape.Raygun4Net.AspNetCore.RaygunAspNetMiddleware+<Invoke>d__5.MoveNext():708

Message: The target table 'dbo.PersistedGrants' of the DML statement cannot have any enabled triggers if the statement contains an OUTPUT clause without INTO clause.
System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke():36
System.Threading.Tasks.Task+<>c.<.cctor>b__281_0(Object obj)
System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state):64
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state):128
System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread):150
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter.GetResult()
 
Microsoft.EntityFrameworkCore.Storage.RelationalCommand+<ExecuteReaderAsync>d__18.MoveNext():1073
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
Microsoft.EntityFrameworkCore.Storage.RelationalCommand+<ExecuteReaderAsync>d__18.MoveNext():1660
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw():17
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task):55
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options):45
System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter.GetResult()
 
Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch+<ExecuteAsync>d__50.MoveNext():217

The model configuration can be overriden by using a derived ConfigurationDbContext:

public class MyConfigurationDbContext : ConfigurationDbContext
{
    public MyConfigurationDbContext(DbContextOptions<ConfigurationDbContext> options) : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        modelBuilder.Entity<Client>().ToTable(tb => tb.HasTrigger("SomeTrigger"));
    }
}

Then in the registration, specify that type:

.AddConfigurationStore<MyConfigurationDbContext>(options =>
{
   options.ConfigureDbContext = builder => builder.UseSqlServer(connectionString);
})

Finally, there is a known issue in IdentityServer when using a custom ConfigurationDbContext: DuendeSoftware/IdentityServer#1616

To work around that you also need to add:

services.AddScoped<ConfigurationDbContext>(svcs => 
  svcs.GetRequiredService<MyConfigurationDbContext>());

Hi,

Thank you for the suggestion above. We have tried the above solution and its still not working for us - we are still seeing the same issue we saw previously (appears to have the same call stack as well):

Could not save changes because the target table has database triggers. Please configure your table accordingly, see https://aka.ms/efcore-docs-sqlserver-save-changes-and-output-clause for more information.

We did need to make some adjustments in order to get the solution to work for us, which I have outlined below.
We needed to add another line to the setup:

services.AddDbContext<ConfigurationDbContext>(options => options.UseSqlServer(Configuration.GetValue<string>("ConnectionStrings:Identity")));

Also, as the Azure Data Sync adds triggers to all tables, we wanted to make sure that we don't have to manually manage a list of tables which could change the next time we do a migration. For that purpose we also modified the OnModelCreating contents to loop through all the tables and set 'HasTrigger' to all tables:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    base.OnModelCreating(modelBuilder);

    var entityTypes = modelBuilder.Model.GetEntityTypes().ToList();

    foreach (var entityType in entityTypes)
    {
        var table = StoreObjectIdentifier.Create(entityType, StoreObjectType.Table);
        if (table != null)
        {
            modelBuilder.Entity(entityType.ClrType).ToTable(tb => tb.HasTrigger(table.Value.Name + "_Trigger"));
        }

        foreach (var fragment in entityType.GetMappingFragments(StoreObjectType.Table))
        {
            modelBuilder.Entity(entityType.ClrType).ToTable(tb => tb.HasTrigger(fragment.StoreObject.Name + "_Trigger"));
        }
    }
}

If either of these are likely to have caused the problem, please let us know.

The other possibility that we have considered is from our use of the migrations generated by Duende/Entity Framework (following the guides here: https://docs.duendesoftware.com/identityserver/v7/upgrades/v6.3_to_v7.0). Our concern is that because we have already run the database migrations, the solution is not being applied correctly to the database as we have passed the migration process. Could this be the issue? Is there a way for us to trigger another "migration" now that we have the code in place?

Let us know if you have any further questions or need more information from us. Any help you have provide is appreciated.

This entry looks like it could cause issues:

services.AddDbContext<ConfigurationDbContext>(options => options.UseSqlServer(Configuration.GetValue<string>("ConnectionStrings:Identity")));

With that, you are still registering the default ConfigurationDbContext and not you derived one (now I assume that ConfigurationDbContext refers to our implementation and that your derived class has a separate name). If you use a derived DbContext type, the original type should not be registered. What are the errors if you remove that line?

Looping through the entities in the OnModelCreating makes sense, I do not see a problem with that approach.

When Entity Framework applies the migrations, that is done in a database transaction. On errors executing the migration there is a rollback of all changes, so there is no risk of getting the database into some kind of half-migrated state.

With that, you are still registering the default ConfigurationDbContext and not you derived one (now I assume that ConfigurationDbContext refers to our implementation and that your derived class has a separate name). If you use a derived DbContext type, the original type should not be registered. What are the errors if you remove that line?

You are correct that ConfigurationDbContext refers to your implementation and our derived class has a separate name - lets call it DerivedConfigurationDbContext for simplicity.

Without that line we get this error:

InvalidOperationException: Unable to resolve service for type 'Microsoft.EntityFrameworkCore.DbContextOptions`1[Duende.IdentityServer.EntityFramework.DbContexts.ConfigurationDbContext]' while attempting to activate 'Web.Migrations.AddTriggerConvention'.

From my understanding of Container DI, we are getting that due to our DerivedConfigurationDbContext having a dependency on DbContextOptions - which is not defined in our Container DI anywhere. The line we added seemed to help with this issue and we were also able to put a breakpoint in OnModelCreating and it was being hit. It could just be that the ordering of our statements is incorrect, we did try to move things around a bit but unfortunately nothing really seemed to help.

I did just try to adjust the line to be:

services.AddDbContext<DerivedConfigurationDbContext>(options => options.UseSqlServer(Configuration.GetValue<string>("ConnectionStrings:Identity")));

but that does not seemed to have helped the issue. Any suggestions you have on how we should be setting this up is appreciated.

When Entity Framework applies the migrations, that is done in a database transaction. On errors executing the migration there is a rollback of all changes, so there is no risk of getting the database into some kind of half-migrated state.

Our concern is not that our database is half-migrated, but more at what point in the process the HasTrigger needs to be run in order to apply to the databases correctly. Can it just be run at any time or does it need to be run as part of a migration?

Any help you can provide is appreciated.

It looks like you have a custom Entity Framework convention defined that has a dependency on DbContextOptions<ConfigurationDbContext>. Could it be another attempt to solve the issue that is still hanging around?

We do not have any other dependency on DbContextOptions. The only class we have that has a dependency on that is the DerivedConfigurationDbContext that was recommended we add above.

public class MyConfigurationDbContext : ConfigurationDbContext
{
public MyConfigurationDbContext(DbContextOptions options) : base(options)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    base.OnModelCreating(modelBuilder);

    modelBuilder.Entity<Client>().ToTable(tb => tb.HasTrigger("SomeTrigger"));
}

}