aws / aws-sdk-net-extensions-cognito

An extension library to assist in the Amazon Cognito User Pools authentication process
Apache License 2.0
102 stars 49 forks source link

Version 2.4.1 throws MissingMethodException when used with SignInManager #119

Closed jlastrachan closed 1 year ago

jlastrachan commented 1 year ago

Describe the bug

Upgrading from 2.4.0 to 2.4.1 starts producing the following error when trying to sign in via password (see await _signInManager.PasswordSignInAsync in the code snippet below).

Expected Behavior

PasswordSignInAsync works as it did in version 2.4.0 or creates a compile-time error exposed to me with the changes needed to upgrade the package.

Current Behavior

I get this stack trace when running the code:

System.MissingMethodException: Method not found: 'System.Threading.Tasks.Task`1<Amazon.Extensions.CognitoAuthentication.CognitoUser> Amazon.Extensions.CognitoAuthentication.CognitoUserPool.FindByIdAsync(System.String)'.\n   at Amazon.AspNetCore.Identity.Cognito.CognitoUserStore`1.FindByIdAsync(String userId, CancellationToken cancellationToken)\n   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)\n   at Amazon.AspNetCore.Identity.Cognito.CognitoUserStore`1.FindByIdAsync(String userId, CancellationToken cancellationToken)\n   at Amazon.AspNetCore.Identity.Cognito.CognitoUserManager`1.FindByIdAsync(String userId)\n   at Amazon.AspNetCore.Identity.Cognito.CognitoSignInManager`1.PasswordSignInAsync(String userId, String password, Boolean isPersistent, Boolean lockoutOnFailure)\n   at Mojo.Server.Admin.Areas.Account.Pages.LoginModel.OnPostAsync(String returnUrl) in /Users/Project/Areas/Account/Pages/Login.cshtml.cs:line 38\n  at
...

Reproduction Steps

Code that calls the SignInManager looks like:

    public class LoginModel : PageModel
    {
        private readonly SignInManager<CognitoUser> _signInManager;
        public LoginModel(SignInManager<CognitoUser> signInManager) => _signInManager = signInManager;

        [Required, Core.Infrastructure.Attributes.EmailAddress, DataType(DataType.EmailAddress), Display(Name = "Email")]
        public string Email { get; set; }

        [Required, DataType(DataType.Password), Display(Name = "Password")]
        public string Password { get; set; }

        public bool RememberMe { get; set; }

        public async Task<IActionResult> OnPostAsync(string returnUrl = null)
        {
            if (ModelState.IsValid)
            {
                returnUrl ??= Url.Content("~/");
                var result = await _signInManager.PasswordSignInAsync(Email, Password, RememberMe, lockoutOnFailure: false);
                if (result.Succeeded)
                {
                    return LocalRedirect(returnUrl);
                }

                if (result.RequiresTwoFactor)
                {
                    return RedirectToPage("Login2FA", new { RememberMe, returnUrl });
                }

                if (result.IsCognitoSignInResult() && result is CognitoSignInResult cognitoResult)
                {
                    if (cognitoResult.RequiresPasswordChange)
                    {
                        return RedirectToPage("ChangePassword", new { returnUrl });
                    }

                    if (cognitoResult.RequiresPasswordReset)
                    {
                        return RedirectToPage("ResetPassword", new { returnUrl });
                    }
                }

                ModelState.AddModelError(string.Empty, "Invalid login");
            }

            return Page();
        }
    }

Possible Solution

Looks like this commit that changed the call signatures with CancellationToken support is what broke this: https://github.com/aws/aws-sdk-net-extensions-cognito/commit/8ac3dd99c8d6014c198627ca2b9f690b4d553584

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Installed implicitly as dependencies of Amazon.Extensions.CognitoAuthentication 2.4.1 AWSSDK.CognitoIdentity 3.7.0.2 AWSSDK.CognitoIdentityProvider 3.7.0.2

Targeted .NET Platform

.NET 6

Operating System and version

OSX Ventura 13.2

ghendo commented 1 year ago

Confirmed also getting this

ashishdhingra commented 1 year ago

Reproducible. We would need to release new version of Amazon.AspNetCore.Identity.Cognito package that references the latest Amazon.Extensions.CognitoAuthentication 2.4.1 assembly. Would review with the team for quick action.

ashovlin commented 1 year ago

We just released version 2.4.2 restoring the original method signatures. Sorry about the inadvertent break, let us know if you see any further issues.

github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.