skoruba / Duende.IdentityServer.Admin

The administration for the Duende IdentityServer and Asp.Net Core Identity ⚡
Apache License 2.0
578 stars 197 forks source link

Delete user from admin panel of admin app returns an error #214

Closed griff158 closed 1 month ago

griff158 commented 6 months ago

First of all thanks very much for making this available and possible. I'm not certain is it's bug or a misconfiguration, but the sub claim of the identity is missing

Describe the bug

When i'm trying to delete a user from the administration site i get an error page

image

To Reproduce

Steps to reproduce the behavior: Goto specific user and try to delete the user.

Relevant parts of the log file

2024-05-13 11:27:57.996 +00:00 [ERR] Exception at route /Identity/UserDelete/df864d37-4b93-43a1-8e15-c01f39a1d2e7 System.InvalidOperationException: sub claim is missing at Duende.IdentityServer.Extensions.PrincipalExtensions.GetSubjectId(IIdentity identity) in //src/IdentityServer/Extensions/PrincipalExtensions.cs:line 80 at Duende.IdentityServer.Extensions.PrincipalExtensions.GetSubjectId(IPrincipal principal) in //src/IdentityServer/Extensions/PrincipalExtensions.cs:line 65 at Skoruba.Duende.IdentityServer.Admin.UI.Areas.AdminUI.Controllers.IdentityController`20.UserDelete(TUserDto user) at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.

The error related to https://github.com/skoruba/Duende.IdentityServer.Admin/blob/main/src/Skoruba.Duende.IdentityServer.Admin.UI/Areas/AdminUI/Controllers/IdentityController.cs#L420 is callling var currentUserId = User.GetSubjectId();

Which is in: https://github.com/DuendeSoftware/IdentityServer/blob/main/src/IdentityServer/Extensions/PrincipalExtensions.cs

2024-05-13 11:27:57.996 +00:00 [ERR] Exception at route /Identity/UserDelete/df864d37-4b93-43a1-8e15-c01f39a1d2e7
System.InvalidOperationException: sub claim is missing
   at Duende.IdentityServer.Extensions.PrincipalExtensions.GetSubjectId(IIdentity identity) in /_/src/IdentityServer/Extensions/PrincipalExtensions.cs:line 80
   at Duende.IdentityServer.Extensions.PrincipalExtensions.GetSubjectId(IPrincipal principal) in /_/src/IdentityServer/Extensions/PrincipalExtensions.cs:line 65
   at Skoruba.Duende.IdentityServer.Admin.UI.Areas.AdminUI.Controllers.IdentityController`20.UserDelete(TUserDto user)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|26_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ExceptionContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeNextResourceFilter()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context)
   at NWebsec.AspNetCore.Middleware.Middleware.CspMiddleware.Invoke(HttpContext context)
   at NWebsec.AspNetCore.Middleware.Middleware.MiddlewareBase.Invoke(HttpContext context)
   at NWebsec.AspNetCore.Middleware.Middleware.MiddlewareBase.Invoke(HttpContext context)
   at NWebsec.AspNetCore.Middleware.Middleware.MiddlewareBase.Invoke(HttpContext context)
   at NWebsec.AspNetCore.Middleware.Middleware.MiddlewareBase.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddlewareImpl.<Invoke>g__Awaited|10_0(ExceptionHandlerMiddlewareImpl middleware, HttpContext context, Task task)
smvjohansenbouvet commented 3 months ago

We are experiencing the same error when deleting users via the admin UI, and it is making it harder to comply with user data erasure requests. Would appreciate a fix.

From audit logs, it does look like the "sub" claim is being renamed to "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier" (aka ClaimTypes.NameIdentifier), even though we call JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear(); in our Startup file. I assume this worked before, so it may be that something changed in the upgrade to .NET 8.

XHunter74 commented 3 months ago

Possible fix:

Add Line: JsonWebTokenHandler.DefaultInboundClaimTypeMap.Clear();

public Startup(IWebHostEnvironment env, IConfiguration configuration) { JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear(); JsonWebTokenHandler.DefaultInboundClaimTypeMap.Clear(); HostingEnvironment = env; Configuration = configuration; }

to src\Skoruba.Duende.IdentityServer.Admin\Startup.cs

marisks commented 1 month ago

@skoruba Is this something that might be released soon? Now it is impossible to delete users from Admin UI, only through API.

skoruba commented 1 month ago

Hi @marisks - did you try what suggested @XHunter74 one comment above? Thanks!

smvjohansenbouvet commented 1 month ago

@skoruba We have tried many variants of the suggested workaround, but none of them have worked.

skoruba commented 1 month ago

Sorry to hear that, I am going to check it. I thought it is resolved.

marisks commented 1 month ago

@skoruba It helped locally. I'll try to deploy it to the server and see if it still works. I thought that it should be part of the lib itself :)

skoruba commented 1 month ago

Sure, I will fix it, but would be nice to know if it works for you as well. :)

skoruba commented 1 month ago

Please, check this commit - https://github.com/skoruba/Duende.IdentityServer.Admin/commit/55a23ca807a75750d371648154b10a19cd22373e

It works for me. You have to logout and login again, after this change. Let me know! thanks

skoruba commented 1 month ago

I will release it in version 2.5.0.

skoruba commented 1 month ago

Fixed in 2.5.0. Thanks!