davidjrh/dnn.azureadprovider

User can't be create if another has same nickname

rodsmr opened this issue · 10 comments

Hi @davidjrh , I've got this problem: can you help me to find a way to solve it?
In my site, with private registration, I've two kind of login: one with your module, one with SPID (Public Digital Identity System (SPID) is the simple, fast and secure access key to digital services of local and central administrations)

The login phase goes when

  1. User login with SPID: it doesn't exist and it's created on portal DB
  2. Same user login with AAD: it doesn't exist, the process goes in error. From Admin Log, I've got this error The logged in user azure-MYEMAIL does not belong to PortalId 0

This is the inner stack trace

at DotNetNuke.Authentication.Azure.Components.AzureClient.AuthenticateUser(UserData user, PortalSettings settings, String IPAddress, Action`1 addCustomProperties, Action`1 onAuthenticated)
   at DotNetNuke.Services.Authentication.OAuth.OAuthLoginBase.OnLoad(EventArgs e)
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.LoadRecursive()
   at System.Web.UI.Control.<LoadRecursiveAsync>d__246.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Util.WithinCancellableCallbackTaskAwaitable.WithinCancellableCallbackTaskAwaiter.GetResult()
   at System.Web.UI.Page.<ProcessRequestMainAsync>d__523.MoveNext()

I think this error is caused by DisplayName field: SPID and AAD user have the same value

Thanks for the help

I don't think the DisplayName is causing this problem, I've tested other scenarios having mix of Auth providers working well. Can you share more details?

  • Has the DNN instance more than one portal?
  • Have you checked if the user is on the "deleted" users list and still on the recycle bin?

Hi @davidjrh , thanks for the response
Question 1: DNN instance has 1 portal
Question 2: there isn't deleted users

From better code analysis, I found AuthenticateUser method in Components/AzureClient.cs class. I put my eyes on this part of method

var usernamePrefixEnabled = bool.Parse(AzureConfig.GetSetting(AzureConfig.ServiceName, "UsernamePrefixEnabled", portalSettings.PortalId, "true"));
var usernameToFind = usernamePrefixEnabled ? $"azure-{userClaim.Value}" : userClaim.Value;
var userInfo = UserController.GetUserByName(portalSettings.PortalId, usernameToFind);
// If user doesn't exist on current portal, AuthenticateUser() will create it. 
// Otherwise, AuthenticateUser will perform a Response.Redirect, so we have to sinchronize the roles before that, to avoid the ThreadAbortException caused by the Response.Redirect
if (userInfo == null)
{
    base.AuthenticateUser(user, portalSettings, IPAddress, addCustomProperties, onAuthenticated);
    if (IsCurrentUserAuthorized())
    {
        userInfo = UserController.GetUserByName(portalSettings.PortalId, usernameToFind);
        if (userInfo == null)
        {
            throw new SecurityTokenException($"The logged in user {usernameToFind} does not belong to PortalId {portalSettings.PortalId}");
        }
        UpdateUserAndRoles(userInfo);
        MarkUserAsAad(userInfo);
    }
}
else
{
    if (IsCurrentUserAuthorized())
    {
        UpdateUserAndRoles(userInfo);
    }
    base.AuthenticateUser(user, portalSettings, IPAddress, addCustomProperties, onAuthenticated);
}

There is this comment

// If user doesn't exist on current portal, AuthenticateUser() will create it.
// Otherwise, AuthenticateUser will perform a Response.Redirect, so we have to sinchronize the roles before that, to avoid the ThreadAbortException caused by the Response.Redirect

In my case, userInfo IS NULL so it calls base.AuthenticateUser...but this base method doesn't create the user and it raise the exception The logged in user {usernameToFind} does not belong to PortalId {portalSettings.PortalId}

If you need more info, tell me

Thanks,
rodsmr

Can you share the collation of your database? I'm wondering if this line is causing the issue because the prefix should be "Azure-" instead of "azure-" being case sensitive

var usernameToFind = usernamePrefixEnabled ? $"azure-{userClaim.Value}" : userClaim.Value;

Anyway, I'm going to fix this to don't fail on case sensitive databases
var usernameToFind = usernamePrefixEnabled ? $"{AzureConfig.ServiceName}-{userClaim.Value}" : userClaim.Value;

Collation is SQL_Latin1_General_CP1_CI_AS.
Table Users, Username field, has this value: Azure-MYEMAIL

I try your changes but it doesn't work: same error :(

And in the Users table, is the user also with the correct PortalId? I'm wondering how this line of code is not returning a user. Can you check the value of portalSettings.PortalId and if there is any whitespace on the claim or on the Username field in the database?

userInfo = UserController.GetUserByName(portalSettings.PortalId, usernameToFind);

Hi @davidjrh , I've found the problem: DNN doesn't accept two users with same DisplayName. Look the attached image, from Users admin panel
image

In the code, I add a try/catch block

// If user doesn't exist on current portal, AuthenticateUser() will create it. 
// Otherwise, AuthenticateUser will perform a Response.Redirect, so we have to sinchronize the roles before that, to avoid the ThreadAbortException caused by the Response.Redirect
if (userInfo == null)
{
    TRY
    {
        base.AuthenticateUser(user, portalSettings, IPAddress, addCustomProperties, onAuthenticated);
        Logger.Error(String.Format("user var: {0}", Newtonsoft.Json.JsonConvert.SerializeObject(user)));
        
        if (IsCurrentUserAuthorized())
        {
            Logger.Error(String.Format("portalSettings.PortalId #2 var: {0}", Newtonsoft.Json.JsonConvert.SerializeObject(portalSettings.PortalId)));
            Logger.Error(String.Format("usernameToFind #2 var: {0}", Newtonsoft.Json.JsonConvert.SerializeObject(usernameToFind)));
            userInfo = UserController.GetUserByName(portalSettings.PortalId, usernameToFind);
            Logger.Error(String.Format("userInfo #2 var: {0}", Newtonsoft.Json.JsonConvert.SerializeObject(userInfo)));
            if (userInfo == null)
            {
                throw new SecurityTokenException($"The logged in user {usernameToFind} does not belong to PortalId {portalSettings.PortalId}");
            }
            UpdateUserAndRoles(userInfo);
            MarkUserAsAad(userInfo);
        }
    }
    CATCH(Exception e)
    {
        Logger.Error(String.Format("base.AuthenticateUser not work: {0}", e.Message));
    }
}
else
{
    if (IsCurrentUserAuthorized())
    {
        UpdateUserAndRoles(userInfo);
    }
    base.AuthenticateUser(user, portalSettings, IPAddress, addCustomProperties, onAuthenticated);
}

and now I can see the message "The Display Name is already in use."

Thanks for the support

PS: I report my log message

user var: {"given_name":"MYFIRSTNAME","family_name":"MYSURNAME","name":"MYFIRSTNAME MYSURNAME","id":"MYEMAIL","email":"MYEMAIL","emails":null,"gender":null,"locale":null,"timezone":null,"time_zone":null,"username":null,"website":null}
portalSettings.PortalId #2 var: 0
usernameToFind #2 var: "Azure-MYEMAIL"
userInfo #2 var: null
base.AuthenticateUser not work: The logged in user Azure-MYEMAIL does not belong to PortalId 0

@rodsmr by default, DNN supports duplicated display names unless you go and change that setting on Security > Member Accounts > Registration Settings. I will try to accommodate the code for that setting failing gracefully.

image

@davidjrh ah ok, in my case Require Unique Dispaly Name is set to true

Hi @davidjrh ,
I've got another "problem": sometimes, when AAD login create a new user firstname and lastname are inverted. I've not access to Azure Active Directory but the manteiner tells me the data are saved correctly

How can I indagate?

The easiest way would be to inspect the token that is stored in the browser cookie and inspect the claims. You can use the "EditThisCookie" Chrome/Edge addon, and then paste the content of the cookie on https://jwt.io to decode the token.

Other approach would be, if you have setup advanced settings with the App/Secret for "Application" type permissions, to query directly the Graph API to check those users.