openiddict / openiddict-core

Flexible and versatile OAuth 2.0/OpenID Connect stack for .NET
https://openiddict.com/
Apache License 2.0
4.42k stars 517 forks source link

Two calls to the Authorization endpoint produces an exception #718

Closed jonathanantoine closed 5 years ago

jonathanantoine commented 5 years ago

When I call twice the authorization endpoint with the below parameters, the second call receive a 500 status code with an exception "Cannot insert duplicate key row in object 'portal.OpenIddictTokens'

Call made : client_id=client_id_custom&redirect_uri=http%3A%2F%2Flocalhost%3A5000&response_type=code&scope=openid&state=15480681920150.40786101043532397&code_challenge=63ZdUz7TVmbTdcssm0vgNGirHdL13v0nlXv3Lzd5kO0&code_challenge_method=S256

Exception : SqlException: Cannot insert duplicate key row in object 'portal.OpenIddictTokens' with unique index 'IX_OpenIddictTokens_ReferenceId'. The duplicate key value is (<NULL>). The statement has been terminated. Stacktrace :


Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(DbContext _, ValueTuple<IEnumerable<ModificationCommandBatch>, IRelationalConnection> parameters, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync<TState, TResult>(TState state, Func<DbContext, TState, CancellationToken, Task<TResult>> operation, Func<DbContext, TState, CancellationToken, Task<ExecutionResult<TResult>>> verifySucceeded, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IReadOnlyList<InternalEntityEntry> entriesToSave, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken)
OpenIddict.Core.OpenIddictTokenManager<TToken>.CreateAsync(TToken token, CancellationToken cancellationToken)
OpenIddict.Core.OpenIddictTokenManager<TToken>.CreateAsync(OpenIddictTokenDescriptor descriptor, CancellationToken cancellationToken)
OpenIddict.Core.OpenIddictTokenManager<TToken>.OpenIddict.Abstractions.IOpenIddictTokenManager.CreateAsync(OpenIddictTokenDescriptor descriptor, CancellationToken cancellationToken)
OpenIddict.Server.Internal.OpenIddictServerProvider.CreateTokenAsync(string type, AuthenticationTicket ticket, OpenIddictServerOptions options, OpenIdConnectRequest request, ISecureDataFormat<AuthenticationTicket> format)
OpenIddict.Server.Internal.OpenIddictServerProvider.SerializeAuthorizationCode(SerializeAuthorizationCodeContext context)
AspNet.Security.OpenIdConnect.Server.OpenIdConnectServerHandler.SerializeAuthorizationCodeAsync(ClaimsPrincipal principal, AuthenticationProperties properties, OpenIdConnectRequest request, OpenIdConnectResponse response)
AspNet.Security.OpenIdConnect.Server.OpenIdConnectServerHandler.SignInAsync(AuthenticationTicket ticket)
Microsoft.AspNetCore.Authentication.AuthenticationService.SignInAsync(HttpContext context, string scheme, ClaimsPrincipal principal, AuthenticationProperties properties)
Microsoft.AspNetCore.Mvc.SignInResult.ExecuteResultAsync(ActionContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeResultAsync(IActionResult result)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResultFilterAsync<TFilter, TFilterAsync>()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResultExecutedContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.ResultNext<TFilter, TFilterAsync>(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeResultFilters()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)```
kevinchalet commented 5 years ago

Can you please double-check that the IX_OpenIddictTokens_ReferenceId index includes an IS NOT NULL filter?

E.g:

CREATE UNIQUE NONCLUSTERED INDEX [IX_OpenIddictTokens_ReferenceId]
    ON [dbo].[OpenIddictTokens]([ReferenceId] ASC) WHERE ([ReferenceId] IS NOT NULL);
jonathanantoine commented 5 years ago

it does not include this filters (I used the default created database btw).

kevinchalet commented 5 years ago

What version of EF Core did you use to create the database?

It's weird, because EF Core is expected to add a non-null filter when using the MSSQL provider:

When using the SQL Server provider EF adds a 'IS NOT NULL' filter for all nullable columns that are part of a unique index. To override this convention you can supply a null value.

https://docs.microsoft.com/en-us/ef/core/modeling/relational/indexes#fluent-api

jonathanantoine commented 5 years ago

I use the 2.1.4 version of EF.CORE. I have this when I dump it's sql create script :

CREATE TABLE [fake].[OpenIddictTokens]
([ApplicationId]    [bigint]            NULL,
 [AuthorizationId]  [bigint]            NULL        INDEX [IX_OpenIddictTokens_AuthorizationId] NONCLUSTERED,
 [ConcurrencyToken] [nvarchar](50)      NULL,
 [CreationDate]     [datetimeoffset](7) NULL,
 [ExpirationDate]   [datetimeoffset](7) NULL,
 [Id]               [bigint]            NOT NULL    IDENTITY (1,1),
 [Payload]          [nvarchar](MAX)     NULL,
 [Properties]       [nvarchar](MAX)     NULL,
 [ReferenceId]      [nvarchar](100)     NULL        INDEX [IX_OpenIddictTokens_ReferenceId] UNIQUE NONCLUSTERED,
 [Status]           [nvarchar](25)      NOT NULL,
 [Subject]          [nvarchar](450)     NOT NULL,
 [Type]             [nvarchar](25)      NOT NULL,

 CONSTRAINT [PK_OpenIddictTokens] PRIMARY KEY CLUSTERED ([Id]),
 CONSTRAINT [FK_OpenIddictTokens_OpenIddictApplications_ApplicationId] FOREIGN KEY ([ApplicationId])
    REFERENCES [mo].[OpenIddictApplications] ([Id]),
 CONSTRAINT [FK_OpenIddictTokens_OpenIddictAuthorizations_AuthorizationId] FOREIGN KEY ([AuthorizationId])
    REFERENCES [mo].[OpenIddictAuthorizations] ([Id])
)
kevinchalet commented 5 years ago

I am unable to reproduce that locally. With EF Core 2.1.4, here's what it generates on my machine (with SQL Server Express 2014):

CREATE TABLE [dbo].[OpenIddictTokens] (
    [ApplicationId]    BIGINT             NULL,
    [AuthorizationId]  BIGINT             NULL,
    [ConcurrencyToken] NVARCHAR (50)      NULL,
    [CreationDate]     DATETIMEOFFSET (7) NULL,
    [ExpirationDate]   DATETIMEOFFSET (7) NULL,
    [Id]               BIGINT             IDENTITY (1, 1) NOT NULL,
    [Payload]          NVARCHAR (MAX)     NULL,
    [Properties]       NVARCHAR (MAX)     NULL,
    [ReferenceId]      NVARCHAR (100)     NULL,
    [Status]           NVARCHAR (25)      NOT NULL,
    [Subject]          NVARCHAR (450)     NOT NULL,
    [Type]             NVARCHAR (25)      NOT NULL,
    CONSTRAINT [PK_OpenIddictTokens] PRIMARY KEY CLUSTERED ([Id] ASC),
    CONSTRAINT [FK_OpenIddictTokens_OpenIddictApplications_ApplicationId] FOREIGN KEY ([ApplicationId]) REFERENCES [dbo].[OpenIddictApplications] ([Id]),
    CONSTRAINT [FK_OpenIddictTokens_OpenIddictAuthorizations_AuthorizationId] FOREIGN KEY ([AuthorizationId]) REFERENCES [dbo].[OpenIddictAuthorizations] ([Id])
);

GO
CREATE NONCLUSTERED INDEX [IX_OpenIddictTokens_AuthorizationId]
    ON [dbo].[OpenIddictTokens]([AuthorizationId] ASC);

GO
CREATE UNIQUE NONCLUSTERED INDEX [IX_OpenIddictTokens_ReferenceId]
    ON [dbo].[OpenIddictTokens]([ReferenceId] ASC) WHERE ([ReferenceId] IS NOT NULL);

GO
CREATE NONCLUSTERED INDEX [IX_OpenIddictTokens_ApplicationId_Status_Subject_Type]
    ON [dbo].[OpenIddictTokens]([ApplicationId] ASC, [Status] ASC, [Subject] ASC, [Type] ASC);
kevinchalet commented 5 years ago

Even when using migrations, it generates a correct index:

migrationBuilder.CreateIndex(
    name: "IX_OpenIddictTokens_ReferenceId",
    table: "OpenIddictTokens",
    column: "ReferenceId",
    unique: true,
    filter: "[ReferenceId] IS NOT NULL");
CREATE TABLE [dbo].[OpenIddictTokens] (
    [ApplicationId]    BIGINT             NULL,
    [AuthorizationId]  BIGINT             NULL,
    [ConcurrencyToken] NVARCHAR (50)      NULL,
    [CreationDate]     DATETIMEOFFSET (7) NULL,
    [ExpirationDate]   DATETIMEOFFSET (7) NULL,
    [Id]               BIGINT             IDENTITY (1, 1) NOT NULL,
    [Payload]          NVARCHAR (MAX)     NULL,
    [Properties]       NVARCHAR (MAX)     NULL,
    [ReferenceId]      NVARCHAR (100)     NULL,
    [Status]           NVARCHAR (25)      NOT NULL,
    [Subject]          NVARCHAR (450)     NOT NULL,
    [Type]             NVARCHAR (25)      NOT NULL,
    CONSTRAINT [PK_OpenIddictTokens] PRIMARY KEY CLUSTERED ([Id] ASC),
    CONSTRAINT [FK_OpenIddictTokens_OpenIddictApplications_ApplicationId] FOREIGN KEY ([ApplicationId]) REFERENCES [dbo].[OpenIddictApplications] ([Id]),
    CONSTRAINT [FK_OpenIddictTokens_OpenIddictAuthorizations_AuthorizationId] FOREIGN KEY ([AuthorizationId]) REFERENCES [dbo].[OpenIddictAuthorizations] ([Id])
);

GO
CREATE NONCLUSTERED INDEX [IX_OpenIddictTokens_AuthorizationId]
    ON [dbo].[OpenIddictTokens]([AuthorizationId] ASC);

GO
CREATE UNIQUE NONCLUSTERED INDEX [IX_OpenIddictTokens_ReferenceId]
    ON [dbo].[OpenIddictTokens]([ReferenceId] ASC) WHERE ([ReferenceId] IS NOT NULL);

GO
CREATE NONCLUSTERED INDEX [IX_OpenIddictTokens_ApplicationId_Status_Subject_Type]
    ON [dbo].[OpenIddictTokens]([ApplicationId] ASC, [Status] ASC, [Subject] ASC, [Type] ASC);
jonathanantoine commented 5 years ago

I digged a little more and seems it's an issue with the tool I used to dump the database and recreate it (using sql scripts in automated tests). Sorry for the disturbance.