Open Closed

Azure AD 'Username Taken' on Subsequent Logins #1363


0
riley.trevillion created
  • ABP Framework version: 4.3.0
  • UI type: Angular
  • DB provider: EF Core
  • Tiered (MVC) or Identity Server Separated (Angular): No
  • Exception message and stack trace: See below
  • Steps to reproduce the issue: See below

Please see https://support.abp.io/QA/Questions/1291/Azure-AD-Configuration-and-Login-Issues--Angular for additional context

Steps To Reproduce

  1. Log in with a local admin user and turn on 'require users to confirm email' setting in ABP
  2. Log out
  3. Log in via Azure AD with a user that has never logged in before
  4. Observe the login process is successful and the user can begin using ABP
  5. Log out
  6. Attempt to log in via Azure AD with the same user again
  7. Observe the 'username taken' error

If you switch the 'require users to confirm email' setting off the issue doesn't occur anymore and an Azure AD user can log in and out multiple times without error. This isn't ideal though as we'd like to have local users with confirmed email addresses as well as Azure AD users.


13 Answer(s)
  • 0
    albert created
    Support Team

    fyi @gterdem

  • 0
    albert created
    Support Team

    we already created an internal ticket for this issue

  • 0
    gterdem created
    Support Team

    This is a misleading error. It should redirect to a User Not Allowed or Locked page since ExternalLoginSignInAsync returns NotAllowed. We'll fix it after deciding how to handle it. It will either be adding a Locked Out Page or throwing a user friendly error.

    If it is urgent for you, you can handle it as you like by overriding Login page and OnGetExternalLoginCallbackAsync method.

    <br>

    [UnitOfWork]
    public override async Task<IActionResult> OnGetExternalLoginCallbackAsync(string returnUrl = "",
        string returnUrlHash = "", string remoteError = null)
    {
        ... //Omitted
    
        var result = await SignInManager.ExternalLoginSignInAsync(
            loginInfo.LoginProvider,
            loginInfo.ProviderKey,
            isPersistent: true,
            bypassTwoFactor: true
        );
    
        if (!result.Succeeded)
        {
            await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext
            {
                Identity = IdentitySecurityLogIdentityConsts.IdentityExternal,
                Action = "Login" + result
            });
        }
    
        if (result.IsLockedOut)
        {
            throw new UserFriendlyException("Cannot proceed because user is locked out!");
        }
    
        // The 'NotAllowed' status occurs if the user exists in the system, but is not enabled
        if (result.IsNotAllowed)
        {
            Logger.LogWarning($"External login callback error: User is Not Allowed!");
            // Returns a view informing the user about the locked account
            // return RedirectToAction(nameof(Lockout));
            // Or throw error
            // throw new UserFriendlyException("Cannot proceed because user is locked out!");
        }
    
        if (result.Succeeded)
        {
            var user = await UserManager.FindByLoginAsync(loginInfo.LoginProvider, loginInfo.ProviderKey);
            if (IsLinkLogin)
            {
                using (CurrentPrincipalAccessor.Change(await SignInManager.CreateUserPrincipalAsync(user)))
                {
                    await IdentityLinkUserAppService.LinkAsync(new LinkUserInput
                    {
                        UserId = LinkUserId.Value,
                        TenantId = LinkTenantId,
                        Token = LinkToken
                    });
                }
            }
    
            await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext
            {
                Identity = IdentitySecurityLogIdentityConsts.IdentityExternal,
                Action = result.ToIdentitySecurityLogAction(),
                UserName = user.UserName
            });
    
            return RedirectSafely(returnUrl, returnUrlHash);
        }
    
       ... //Omitted
    
        return RedirectSafely(returnUrl, returnUrlHash);
    }
    
  • 0
    gterdem created
    Support Team

    This will be implemented for both sides, you can follow from https://github.com/abpframework/abp/issues/9132. It will be fixed in next patch.

  • 0
    riley.trevillion created

    @alper @gterdem

    Thankyou for responding to this one so promptly.

    I've taken a look at the code sample you have provided and I have been able to successfully implement a workaround to satisfy my immediate use case (i.e Azure AD login without error and local user login requiring confirmed email addresses without error). If the user is coming in from an external provider, I don't need them to confirm their email address like I would a local user as they would already have done this on the external providers end. I progress the login workflow as normal for an external user only (local users still go through email confirmation process).

    Will this workaround cause problems or break certain login workflows? If not, I am happy for this ticket to be closed.


    Some additional thoughts

    • An alternative workaround that I tried and appeared to work was to get the external Azure AD user to confirm their email address regardless. This may or may not be a better way than what I'm doing above. If it's a better way of addressing the issue, please let me know and I'll swap over to using it.

    • As part of a more permanent fix, given the user is logging in from an external provider, would it make sense to bypass the email confirmation check entirely (i.e NotAllowed) in the same way you are bypassing the two factor authentication check here for external providers?

    Would something like this be workable?

    Anyway, just some personal thoughts on the matter.

    Thankyou again

  • 0
    albert created
    Support Team

    I keep this issue open

  • 0
    riley.trevillion created

    Hello

    Just to provide a quick update. We've completed our demonstration with the customer and there were some conflicting view points on whether email confirmation should be enforced on SSO login or not. Ultimately the decision was made to enforce email verification (i.e not what I thought was necessary but not a big deal at the end of the day).

    In my mind, that means when an admin switches the 'Force email verification option' on in the settings, it applies to external users as well. It also seemed like something that would be pretty straight forward to implement in the code as shown below.

    I've put the following code block in when the external user is NotAllowed to force them to confirm their email. Exactly how it's being done for a local user.

    if (result.IsNotAllowed)
    {
        var user = await UserManager.FindByLoginAsync(loginInfo.LoginProvider, loginInfo.ProviderKey);
        await StoreConfirmUser(user);
        return RedirectToPage("./ConfirmUser", new
        {
            returnUrl = ReturnUrl,
            returnUrlHash = ReturnUrlHash
        });
    }
    

    <br> Unfortunately, I'm seeing a repeat of the same behaviour in https://support.abp.io/QA/Questions/1113/Self-Registering-User-Logged-In-Without-Confirming-Email-First where the very first time the user logs in, they are not prompted to verify their email and are dropped straight onto the home page logged in and can use the application. If you log out and log in again with the same external user, they are then prompted to verify.

    If you were to extend whatever fix you have done in https://support.abp.io/QA/Questions/1113/Self-Registering-User-Logged-In-Without-Confirming-Email-First for external users as well, I think that would resolve this issue if you are also adding in the verification code block if a user is NotAllowed.

    Happy to provide any further material to assist with this issue, Just let me know.

    Thanks

  • 0
    albert created
    Support Team

    is this issue enough what you need ? https://github.com/abpframework/abp/issues/9132

  • 0
    riley.trevillion created

    Hi @albert @alper @gterdem

    I've upgraded to 4.3.2 and verified the new code written by @gterdem is in place. Subsequent logins of the external user do not result in the exception screen and I am instead prompted to verify my email address instead. Great stuff!

    Unfortunately the issue described in my comment (https://support.abp.io/QA/Questions/1363#answer-ff0a88a4-1407-43a8-93d3-39fcf144b248) is still happening. If I am signing in / signing up as a brand new external user I am not prompted to verify my email first - I am taken straight to the homepage and can use the application immediately. If I log out and try log in again with that user it will force me to verify for subsequent logins (as intended).

    This exact issue was affecting users registering locally but it was fixed as part of https://support.abp.io/QA/Questions/1113/Self-Registering-User-Logged-In-Without-Confirming-Email-First

    If a similar fix can be extended to include newly registered external users, that should solve this issue and I believe the ticket would be good to close off (atleast in my eyes). Have you been able to confirm the behaviour I have described on your end?

    Thankyou

  • 0
    gterdem created
    Support Team

    Hello @riley.trevillion,

    I understand your issue that, when the external user is loggin in for the first time; new external user is created and logged in immidiately when even identity settings required email/phone is selected.

    I reproduced the problem and fixed the issue. It should be available in the next 4.3 patch release.

  • 0
    riley.trevillion created

    @gterdem yes that is the issue as you have described it. Good to hear you have fixed it, thankyou for that.

    Ordinarily I'd be happy to wait for the patch release to come out to pick the fix up, but I know there are people on my side who are quite keen to see this problem fixed with a certain degree of urgency.

    If the fix you have implemented is relatively simple (few lines of code / a couple of functions), would it be possible for you to share it here or privately with me so that I can add it temporarily to our codebase until the official patch release drops?

    Thankyou again

  • 0
    gterdem created
    Support Team

    Surely,

    In OnGetExternalLoginCallbackAsync method, there is a code piece where user is created then signed in: <br>

    ...
    
    var externalUser = await CreateExternalUserAsync(externalLoginInfo);
    
    await SignInManager.SignInAsync(externalUser, false);
    ...
    

    Add identity settings control between: <br>

    var externalUser = await CreateExternalUserAsync(externalLoginInfo);
    
    if (await CheckRequiredIdentitySettings()) // Checks SettingManagement for RequiredConfirmedEmail or RequiredConfirmedPhoneNumber
    {
        Logger.LogWarning($"New external user is created but confirmation is required!");
    
        await StoreConfirmUser(externalUser); // Stores data for confirmation page
        return RedirectToPage("./ConfirmUser", new // Redirects to confirmation page
        {
            returnUrl = ReturnUrl,
            returnUrlHash = ReturnUrlHash
        });
    }
    
    await SignInManager.SignInAsync(externalUser, false);
    

    Here is the CheckRequiredIdentitySettings method: <br>

    private async Task<bool> CheckRequiredIdentitySettings()
    {
        var requireConfirmedEmail = await SettingProvider.IsTrueAsync(IdentitySettingNames.SignIn.RequireConfirmedEmail);
        var requireConfirmedPhoneNumber = await SettingProvider.IsTrueAsync(IdentitySettingNames.SignIn.RequireConfirmedPhoneNumber);
        return requireConfirmedEmail || requireConfirmedPhoneNumber;
    }
    
  • 0
    riley.trevillion created

    @gterdem @albert

    Very much appreciated for providing the fix early.

    I have added the code to the relevant places in our solution and it appears to have resolved the issue. I am forced to verify my email on creation and on subsequent logins for an external user. I will pull in the new 4.3.X patch version when it becomes available, but for now this works very nicely, so thankyou

    I will close this ticket off now.