VirtoCommerce/vc-platform

Error on email conflict with Azure AD

j-mok opened this issue · 4 comments

j-mok commented

When local and Azure AD accounts matching fails (i.e. due to username/upn claim mismatch) and both accounts have the same email, an error results.

Steps to reproduce
Steps to reproduce the behavior:

  1. For an AD user create a local VC account that has a different name than that user's upn claim.
  2. Make sure both accounts have the same email and that AD email is included in the token, in either in upn or email claim.
  3. Attempt to sign on as that user through Azure AD.

** Result **
A 404 error response and the following log entries:

warn: Microsoft.AspNetCore.Identity.UserManager[13]
      User ***** validation failed: DuplicateEmail.
fail: Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware[1]
      An unhandled exception has occurred while executing the request.
System.InvalidOperationException: Failed to save a VC platform account due the errors: Email 'john.doe@foomail.com' is already taken.
   at VirtoCommerce.Platform.Web.Controllers.ExternalSignInController.SignInCallback(String returnUrl) in /home/runner/work/vc-platform/vc-platform/src/VirtoCommerce.Platform.Web/Controllers/ExternalSignInController.cs:line 100
   at lambda_method(Closure , Object )
   at Microsoft.Extensions.Internal.ObjectMethodExecutorAwaitable.Awaiter.GetResult()
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Logged|12_1(ControllerActionInvoker invoker)
   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>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|24_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   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|19_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Builder.Extensions.MapMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at VirtoCommerce.Platform.Web.Middleware.ApiErrorWrappingMiddleware.Invoke(HttpContext context) in /home/runner/work/vc-platform/vc-platform/src/VirtoCommerce.Platform.Web/Middleware/ApiErrorWrappingMiddleware.cs:line 52
   at VirtoCommerce.Platform.Web.Middleware.ApiErrorWrappingMiddleware.Invoke(HttpContext context) in /home/runner/work/vc-platform/vc-platform/src/VirtoCommerce.Platform.Web/Middleware/ApiErrorWrappingMiddleware.cs:line 52
   at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task)

Expected behavior
Either:

  • Allow account matching by email.
  • Gracefully handle the error by rejecting AAD sign-on attempt with a user-friendly explanation.

Version info (please complete the following information):

  • Platform version: 3.100.0

Additional considerations
Maybe account matching for Azure AD should be a pluggable strategy to allow more flexibility and customization?

dvvkgd commented

Hi, @j-mok .
Thanks for the feedback. Very helpful remark. I submitted this case to the platform team for research and estimate.
I will inform you about the decision.

  • 👍1
OlegoO commented

@dvvkgd we added it to next sprint - 7-18 March. We will add release note here.

DanilBayushev commented

Hi, @j-mok
This issue was resolved by the Platform Team.
Release: https://github.com/VirtoCommerce/vc-platform/releases/tag/3.208.0

  • 👍1