Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import-D365AadUser with AD lookup is using invalid email as network alias #856

Closed
gertvdh opened this issue Sep 13, 2024 · 13 comments · Fixed by #860
Closed

Import-D365AadUser with AD lookup is using invalid email as network alias #856

gertvdh opened this issue Sep 13, 2024 · 13 comments · Fixed by #860
Labels
0.7.18 Release 0.7.18

Comments

@gertvdh
Copy link
Contributor

gertvdh commented Sep 13, 2024

When importing users, the email address is looked up on Entra and then used as network alias for the user.
In our case Entra is giving a full email address ([email protected]) and not the Entra id ([email protected]).

Can we foresee an override option to use the UserPrincipalName as email?

@FH-Inway
Copy link
Member

Hi @gertvdh , thanks for the suggestion.

If I understand correctly, this would work similar to the existing parameters IdValue or NameValue. Let's call it $EmailValue, with the options

  • Mail: default, current behavior
  • UserPrincipalName: new behavior

With the new behavior, It would use $user.UserPrincipalName as second parameter in the call to the internal function Import-AadUserIntoD365FO.

Import-AadUserIntoD365FO $SqlCommand $user.Mail $name $id $sid $StartupCompany $identityProvider $networkDomain $user.ObjectId

For anyone else like me wondering where this then eventually ends up in the network alias (which is displayed as Email in the D365FO user interface): The second parameter of Import-AadUserIntoD365FO is then passed as $SignInName to another internal function New-D365FOUser. This then calls the prepared SQL script add-aaduserintod365fo.sql, which uses it to populate the NetworkDomain field.

Would this work for you?

@gertvdh
Copy link
Contributor Author

gertvdh commented Sep 13, 2024

Yes, you are correct. Do you want me to foresee a pull request for this?

@FH-Inway
Copy link
Member

That would be great if you could do that, yes!

@Splaxi
Copy link
Collaborator

Splaxi commented Sep 19, 2024

@gertvdh

Did you get time to look at this? I'm doing a major refactor of this specific cmdlet - as we want to move away from the AzureAD module dependency.

Let me know, if I should incorporate your needed changes - while doing the refactor 👍

@gertvdh
Copy link
Contributor Author

gertvdh commented Sep 19, 2024

I was planning on doing the work this week or early next next

@Splaxi
Copy link
Collaborator

Splaxi commented Sep 19, 2024

Please do - then I'll do the initial implementation for the rest calls, and then we can work together to merge your needs into my PR afterwards

@Splaxi
Copy link
Collaborator

Splaxi commented Sep 23, 2024

Could any of you test this?

function Import-D365AadUserV2 {
    [CmdletBinding(DefaultParameterSetName = 'UserListImport')]
    param (
        [Parameter(Mandatory = $true, Position = 1, ParameterSetName = "GroupNameImport")]
        [String] $AadGroupName,

        [Parameter(Mandatory = $true, Position = 1, ParameterSetName = "UserListImport")]
        [string[]]$Users,

        [Parameter(Mandatory = $false, Position = 2)]
        [string] $StartupCompany = 'DAT',

        [Parameter(Mandatory = $false, Position = 3)]
        [string] $DatabaseServer = $Script:DatabaseServer,

        [Parameter(Mandatory = $false, Position = 4)]
        [string] $DatabaseName = $Script:DatabaseName,

        [Parameter(Mandatory = $false, Position = 5)]
        [string] $SqlUser = $Script:DatabaseUserName,

        [Parameter(Mandatory = $false, Position = 6)]
        [string] $SqlPwd = $Script:DatabaseUserPassword,

        [Parameter(Mandatory = $false, Position = 7)]
        [string] $IdPrefix = "",

        [Parameter(Mandatory = $false, Position = 8)]
        [string] $NameSuffix = "",

        [Parameter(Mandatory = $false, Position = 9)]
        [ValidateSet('Login', 'FirstName', 'UserPrincipalName')]
        [string] $IdValue = "Login",

        [Parameter(Mandatory = $false, Position = 10)]
        [ValidateSet('FirstName', 'DisplayName')]
        [string] $NameValue = "DisplayName",

        [Parameter(Mandatory = $false, Position = 11)]
        [PSCredential] $AzureAdCredential,

        [Parameter(Mandatory = $false, Position = 12, ParameterSetName = "UserListImport")]
        [switch] $SkipAzureAd,

        [Parameter(Mandatory = $false, Position = 13, ParameterSetName = "GroupNameImport")]
        [switch] $ForceExactAadGroupName,

        [Parameter(Mandatory = $true, Position = 14, ParameterSetName = "GroupIdImport")]
        [string] $AadGroupId,

        [Parameter(Mandatory = $false, Position = 15)]
        [ValidateSet('Mail', 'UserPrincipalName')]
        [string] $EmailValue = "Mail"
    )

    $UseTrustedConnection = Test-TrustedConnection $PSBoundParameters

    $SqlParams = @{ DatabaseServer = $DatabaseServer; DatabaseName = $DatabaseName;
        SqlUser = $SqlUser; SqlPwd = $SqlPwd
    }

    $SqlCommand = Get-SqlCommand @SqlParams -TrustedConnection $UseTrustedConnection

    $instanceProvider = Get-InstanceIdentityProvider
    $canonicalProvider = Get-CanonicalIdentityProvider

    try {
        Write-PSFMessage -Level Verbose -Message "Trying to connect to the Azure Active Directory"

        if ($PSBoundParameters.ContainsKey("AzureAdCredential") -eq $true) {
            Login-AzAccount -Credential $AzureAdCredential -ErrorAction Stop
        }
        else {
            if ($SkipAzureAd -eq $false) {
                Login-AzAccount -ErrorAction Stop
            }
        }
    }
    catch {
        Write-PSFMessage -Level Host -Message "Something went wrong while connecting to Azure Active Directory" -Exception $PSItem.Exception
        Stop-PSFFunction -Message "Stopping because of errors"
        return
    }

    $azureAdUsers = New-Object -TypeName "System.Collections.ArrayList"

    if (( $PSCmdlet.ParameterSetName -eq "GroupNameImport") -or ($PSCmdlet.ParameterSetName -eq "GroupIdImport")) {

        if ($PSCmdlet.ParameterSetName -eq 'GroupIdImport') {
            Write-PSFMessage -Level Verbose -Message "Search AadGroup by its ID : $AadGroupId"

            $resObj = Invoke-AzRestMethod -Uri "https://graph.microsoft.com/v1.0/groups/$AadGroupId"

            if ($resObj.StatusCode -like "2**") {
                $group = $resObj.Content | ConvertFrom-Json
            }
        }
        else {
            if ($ForceExactAadGroupName) {
                Write-PSFMessage -Level Verbose -Message "Search AadGroup by its exactly name : $AadGroupName"

                $resObj = Invoke-AzRestMethod -Uri "https://graph.microsoft.com/v1.0/groups?`$filter=DisplayName eq '$AadGroupName'"

                if ($resObj.StatusCode -like "2**") {
                    $group = $resObj.Content | ConvertFrom-Json | Select-Object -ExpandProperty value
                }
            }
            else {
                Write-PSFMessage -Level Verbose -Message "Search AadGroup by searching with its name : $AadGroupName"

                $resObj = Invoke-AzRestMethod -Uri "https://graph.microsoft.com/v1.0/groups?`$filter=startswith(DisplayName,'$AadGroupName')"

                if ($resObj.StatusCode -like "2**") {
                    $group = $resObj.Content | ConvertFrom-Json | Select-Object -ExpandProperty value
                }
            }
        }

        if ($null -eq $group) {
            Write-PSFMessage -Level Host -Message "Unable to find the specified group in the AAD. Please ensure the group exists and that you have enough permissions to access it."
            Stop-PSFFunction -Message "Stopping because of errors"
            return
        }
        else {
            Write-PSFMessage -Level Host -Message "Processing Azure AD user Group `"$($group[0].DisplayName)`""
        }

        if ($group.Length -gt 1) {
            Write-PSFMessage -Level Host -Message "More than one group found"
            foreach ($foundGroup in $group) {
                Write-PSFMessage -Level Host -Message "Group found $($foundGroup.DisplayName)"
            }
            Stop-PSFFunction -Message "Stopping because of errors"
            return
        }

        $resMembersObj = Invoke-AzRestMethod -Uri "https://graph.microsoft.com/v1.0/groups/$($group.id)/members"

        if ($resMembersObj.StatusCode -like "2**") {
            $userlist = $resMembersObj.Content | ConvertFrom-Json | Select-Object -ExpandProperty value
        }

        foreach ($user in $userlist) {
            if ($user.'@odata.type' -eq "#microsoft.graph.user") {

                $azureAdUser = Invoke-AzRestMethod -Uri "https://graph.microsoft.com/v1.0/users/$($user.id)"

                if ($null -eq $azureAdUser.mail) {
                    Write-PSFMessage -Level Critical "User $($user.ObjectId) did not have an Mail"
                }
                else {
                    $null = $azureAdUsers.Add($azureAdUser)
                }
            }
        }
    }
    else {
        foreach ($user in $Users) {

            if ($SkipAzureAd -eq $true) {
                $name = Get-LoginFromEmail $user
                $null = $azureAdUsers.Add([PSCustomObject]@{
                        mail              = $user
                        givenName         = $name
                        displayName       = $name
                        ObjectId          = ''
                        userPrincipalName = $user
                    })
            }
            else {
                $resObj = Invoke-AzRestMethod -Uri "https://graph.microsoft.com/v1.0/users?`$filter=mail eq '$user' or userPrincipalName eq '$user'"

                if ($resObj.StatusCode -like "2**") {
                    $aadUser = $resObj.Content | ConvertFrom-Json | Select-Object -ExpandProperty value | Select-Object -First 1
                }

                if ($null -eq $aadUser) {
                    Write-PSFMessage -Level Critical "Could not find user $user in AzureAAd"
                }
                else {
                    $null = $azureAdUsers.Add($aadUser)
                }
            }
        }
    }

    try {
        $sqlCommand.Connection.Open()

        foreach ($user in $azureAdUsers) {

            $identityProvider = $canonicalProvider

            Write-PSFMessage -Level Verbose -Message "Getting tenant from $($user.Mail)."
            $tenant = Get-TenantFromEmail $user.Mail

            Write-PSFMessage -Level Verbose -Message "Getting domain from $($user.Mail)."
            $networkDomain = Get-NetworkDomain $user.Mail

            Write-PSFMessage -Level Verbose -Message "InstanceProvider : $InstanceProvider"
            Write-PSFMessage -Level Verbose -Message "Tenant : $Tenant"

            if ($user.Mail.ToLower().Contains("outlook.com") -eq $true) {
                $identityProvider = "live.com"
            }
            else {
                if ($instanceProvider.ToLower().Contains($tenant.ToLower()) -ne $True) {
                    Write-PSFMessage -Level Verbose -Message "Getting identity provider from  $($user.Mail)."
                    $identityProvider = Get-IdentityProvider $user.Mail
                }
            }

            Write-PSFMessage -Level Verbose -Message "Getting sid from  $($user.Mail) and identity provider : $identityProvider."
            $sid = Get-UserSIDFromAad $user.Mail $identityProvider
            Write-PSFMessage -Level Verbose -Message "Generated SID : $sid"
            $id = ""
            if ($IdValue -eq 'Login') {
                $id = $IdPrefix + $(Get-LoginFromEmail $user.Mail)
            }
            elseif ($IdValue -eq 'UserPrincipalName') {
                $id = $IdPrefix + $user.UserPrincipalName
            }
            else {
                $id = $IdPrefix + $user.GivenName
            }
            
            if ($id.Length -gt 20) {
                $oldId = $id
                $id = $id -replace '^(.{0,20}).*', '$1'
                Write-PSFMessage -Level Host -Message "The id <c='em'>'$oldId'</c> does not fit the <c='em'>20 character limit</c> on UserInfo table's ID field and will be truncated to <c='em'>'$id'</c>"
            }
            

            $name = ""
            if ($NameValue -eq 'DisplayName') {
                $name = $user.DisplayName + $NameSuffix
            }
            else {
                $name = $user.GivenName + $NameSuffix
            }

            $email = ""
            if ($EmailValue -eq 'Mail') {
                $email = $user.Mail
            }
            else {
                $email = $user.UserPrincipalName
            }

            Write-PSFMessage -Level Verbose -Message "Id for user $email : $id"
            Write-PSFMessage -Level Verbose -Message "Name for user $email : $name"
            Write-PSFMessage -Level Verbose -Message "Importing $email - SID $sid - Provider $identityProvider"

            Import-AadUserIntoD365FO $SqlCommand $email $name $id $sid $StartupCompany $identityProvider $networkDomain $user.ObjectId

            if (Test-PSFFunctionInterrupt) { return }
        }
    }
    catch {
        Write-PSFMessage -Level Host -Message "Something went wrong while working against the database" -Exception $PSItem.Exception
        Stop-PSFFunction -Message "Stopping because of errors"
        return
    }
    finally {
        if ($sqlCommand.Connection.State -ne [System.Data.ConnectionState]::Closed) {
            $sqlCommand.Connection.Close()
        }
        $sqlCommand.Dispose()
    }
}

Note the v2 suffix. You will need to copy&paste the original cmdlet with above code.

@Splaxi Splaxi closed this as completed Sep 23, 2024
@Splaxi Splaxi reopened this Sep 23, 2024
@FH-Inway
Copy link
Member

I pushed that version into https://github.com/d365collaborative/d365fo.tools/tree/test-import-d365aaduserv2 to make it easier for testing and further discussion.

Also created a little test script Test-ImportD365AadUserV2.ps1 in https://gist.github.com/FH-Inway/ba12906c35df73cded460649cf742d54

Unfortunately, I could not get past the authentication. I have a user with sufficient rights and no MFA.

Without -AzureAdCredential, it tries to do the interactive authentication, but that seems to get canceled right away and also freezes the PowerShell session. In the screenshot I tried it with Login-AzAccount directly, but it looks similar when called with the cmdlet.

image

With -AzureAdCredential like in the test script, I get a message "Unable to acquire token for tenant x with error 'Authentication failed against tenant x. User interaction is required. This may be due to the conditional access policy settings such as multi-factor authentication (MFA). [...]'".
Again, the user has no MFA, I can log onto Azure Portal without MFA no problem.
Screenshot shows the direct attempt with Login-AzAccount, but it looks similar via Import-D365AadUser.

image

Unfortunately, I do not have any other users I can test this with.

What would work is authentication with the certificate from Secure one-box development environments - External integrations. See the second script Connect-AzAccountWithCertificate.ps1 in https://gist.github.com/FH-Inway/ba12906c35df73cded460649cf742d54
I tried something similar with the Microsoft Graph module in dec5de1

@Splaxi
Copy link
Collaborator

Splaxi commented Sep 24, 2024

Did - you in any way, have the PowerShell sessions running in elevated mode (Run As Admin)?

Please try a non-admin session.

The best test, is to get the ordinary Login-AzAccount to work - before utilizing the cmdlet. Then we can separate what issues you are facing.

@FH-Inway
Copy link
Member

I tried PowerShell in non-admin session, but no luck at first, got the same behavior as before.
Since the behavior is different when I tried Login-AzAccount locally, I updated the Az module on my CHE environment where I'm testing this. This got rid of the freeze during the interactive authentication. I can now authenticate, however, I still get the message that authentication failed.

I then tried what the message suggests, running Login-AzAccount with the -TenantId parameter. This worked! It also works with a provided credentials object.

So I added a -TenantId parameter to the new version of Import-D365AadUser: 9e4ae83
Running the import with that parameter (see updated test script) works!

@Splaxi
Copy link
Collaborator

Splaxi commented Sep 26, 2024

How do we move from here?

@FH-Inway
Copy link
Member

FH-Inway commented Oct 3, 2024

I created pull request #860 with @gertvdh, your and my changes.

@gertvdh Hope you don't mind me creating a new pull request. It made it easier to add @Splaxi 's and my changes and also target the master branch (we recently switched from development to master as the default branch). Also added you as an author to the cmdlet.

@FH-Inway
Copy link
Member

This is now available in version 0.7.18 🎉

@FH-Inway FH-Inway added the 0.7.18 Release 0.7.18 label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.7.18 Release 0.7.18
Projects
None yet
3 participants