aws / aws-aspnet-cognito-identity-provider

ASP.NET Core Identity Provider for Amazon Cognito
https://aws.amazon.com/developer/language/net/
Apache License 2.0
213 stars 89 forks source link

Check Password for unconfirmed user. #229

Closed Philipp15 closed 1 year ago

Philipp15 commented 1 year ago

Describe the bug

when user is unconfirmed the following fails with "Error: Error Occured in controller: with exception User is not confirmed."

var passValid = await ((CognitoUserManager<CognitoUser>)_userManager).CheckPasswordAsync(findUser, model.Password).ConfigureAwait(false);

The idea is to check if password is correct for unconfirmed user without logging them in but to redirect them to Resend Email Confirmation page. I cannot just allow anyone resend confirmation by providing a valid registered email. Similarly once the user has signed up but decided for whatever reason to clear their localstorage or use another browser they will never be able to confirm their email. Is there some way around this ?

Expected Behavior

var passValid = await ((CognitoUserManager<CognitoUser>)_userManager).CheckPasswordAsync(findUser, model.Password).ConfigureAwait(false);

Either works for unconfirmed users or there is another way to verify that password is correct without logging a user in

Current Behavior

Error: Error Occured in controller: with exception User is not confirmed.

Reproduction Steps

var passValid = await ((CognitoUserManager<CognitoUser>)_userManager).CheckPasswordAsync(CognitoUser, model.Password).ConfigureAwait(false);

OR

var passValid = await _userManager.CheckPasswordAsync(CognitoUser, model.Password).ConfigureAwait(false);

for unconfirmed user fails

Possible Solution

No response

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Amazon.AspNetCore.Identity.Cognito 3.0.1

Targeted .NET Platform

.NET 6.0

Operating System and version

Windows 10

bhoradc commented 1 year ago

Hi @Philipp15,

Thank you for reporting the problem. I was able to reproduce the exception ‘UserNotConfirmedException’ on a sample ASP.NET MVC web application.

If the user exists in Cognito and the password is correct, then this exception is thrown.

You can handle this scenario in couple of ways.

  1. You can use the AdminGetUserAsync() to check if the user status type is ‘Unconfirmed’. If so, then provided the supplied username and password are valid, you can resend the confirmation code to user by ResendConfirmationCodeAsync() and redirect the view to ‘Email confirmation’ page.
                var user = await _userManager.FindByNameAsync(Input.UserName);

                AmazonCognitoIdentityProviderClient clientobj= new AmazonCognitoIdentityProviderClient();

                AdminGetUserRequest userRequest = new AdminGetUserRequest
                {
                    Username = Input.UserName,
                    UserPoolId = "******",
                };

                var response = await clientobj.AdminGetUserAsync(userRequest);

                if (response.UserStatus== UserStatusType.UNCONFIRMED)
                {
                    var codeRequest = new ResendConfirmationCodeRequest
                    {
                        ClientId = "******",
                        Username = Input.UserName,
                    };

                    var coderesponse = await clientobj.ResendConfirmationCodeAsync(codeRequest);
                    return RedirectToPage("./ConfirmEmail");
                }   
  1. Since this exception is thrown when the user exists in Cognito and the password is correct (entered while registration), you can catch the exception (UserNotConfirmedException) and redirect to the ‘Email Confirmation’ page after resending the confirmation code.

Please let me know if this would help resolve your problem or else kindly correct me.

Regards, Chaitanya

Philipp15 commented 1 year ago

Hi Chaitanya,

Thanks for your reply.

  1. If I follow the point 1 route ... that means that anyone with wrong password can force send a confirmation email to account they do not own ...I believe that`s not so good from security perspective

  2. Regarding your second point "For me when I run the following code

var result = await _signInManager.PasswordSignInAsync(model.Email, model.Password, true, false).ConfigureAwait(false);

for unconfirmed user no exception is thrown ... instead the result I get is just "result.Succeeded == false" with no exception

Full Code Segment:

try
                {
                    var result = await _signInManager.PasswordSignInAsync(model.Email,
                 model.Password, true, false).ConfigureAwait(false);

                    if (result.Succeeded)
                    {
                        // some more code if succeeded
                    }

                }

                catch (UserNotConfirmedException unce)
                {
                    _logger.LogError(unce.Message); // never hits this if password is correct
                }
                catch(Exception ex)
                {
                    _logger.LogError(ex.Message); // never hits this if password is incorrect either
                }
bhoradc commented 1 year ago

Hello @Philipp15,

Thank you for the response.

Approach 1 that I proposed was based on the fact that correct username and password is provided by the user and you get the exception. You shouldn’t be resending the confirmation code unless the username and password matches and the user status is ‘Unconfirmed’. Apologies, for the confusion.

When I execute your sample code, I am getting UserNotConfirmedException, provided I pass the username and the correct password.

public virtual async Task<SignInResult> PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure)

If you trying to use above method from Microsoft.AspNetCore.Identity, you are passing Model.Email as string whereas the method might be expecting a username. Hence you aren’t getting the exception.

PasswordSignInAsync() from Microsoft.AspNetCore.Identity.dll:

        public virtual async Task<SignInResult> PasswordSignInAsync(string userName, string password,
            bool isPersistent, bool lockoutOnFailure)
        {
            var user = await UserManager.FindByNameAsync(userName);
            if (user == null)
            {
                return SignInResult.Failed;
            }

            return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure);
        }

Please let me know for further questions.

Regards, Chaitanya

Philipp15 commented 1 year ago

Hi Chaitanya,

I can confirm that when I changed email to username the UserNotConfirmedException was thrown on correct password, thank you for your help and swift reply!!

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.