Azure / avdaccelerator

AVD Accelerator deployment automation to simplify the setup of AVD (Azure Virtual Desktop) based on best practices
MIT License
324 stars 207 forks source link

Domain Join Password cannot contain `|` (and possibly other characters too) #515

Closed SvenAelterman closed 10 months ago

SvenAelterman commented 10 months ago

What happened? Provide a clear and concise description of the bug, including deployment details.

Deploying the LZA with ADDS join for the FSLogix storage account failed because the domain join password contained a |. The | was interpreted by the PowerShell custom script extension command.

Please provide the correlation id associated with your error or bug.

1062a50d-f0af-4018-a636-8d21d471b9aa

What was the expected outcome?

Successful domain join of storage account to ADDS regardless of any "special" characters in the domain join password.

Relevant log output

No response

SvenAelterman commented 10 months ago

I will take a stab at fixing this. It might be as simple as putting the {1} between ' (correctly escaped for Bicep/ARM) to fix this.

danycontre commented 10 months ago

Hey @SvenAelterman thanks for your contribution, the following is the logic we have today:

function Set-EscapeCharacters { Param( [parameter(Mandatory = $true, Position = 0)] [String] $string ) $string = $string -replace '*', '*' $string = $string -replace '\\', '\' $string = $string -replace '\~', '~' $string = $string -replace '\;', ';' $string = $string -replace '(', '(' $string = $string -replace '\%', '%' $string = $string -replace '\?', '?' $string = $string -replace '\.', '.' $string = $string -replace '\:', ':' $string = $string -replace '\@', '@' $string = $string -replace '\/', '/' $string = $string -replace '\$', '$' $string }

We will need to add "|"

SvenAelterman commented 10 months ago

Then what about the next character? Wouldn't enclosing in single quotes (and then escaping single quotes) be a more robust solution?

danycontre commented 10 months ago

Agree, let's try out what you are referring sounds better than current setup.

SvenAelterman commented 10 months ago

Sounds good. I wasn't aware of the escape code, so it will probably need to be revised to not escape most characters or the password will be sent incorrectly to the VM.

danycontre commented 10 months ago

Addressed by @SvenAelterman on PR: https://github.com/Azure/avdaccelerator/pull/526