Cannot run Key Vault tests using -UserAuth test resources switch parameter

[heaths]

I tried running New-TestResources.ps1 keyvault -AdditionalParameters @{enableHsm=$true} -UserAuth and, though provisioning worked, my tests still tried to run against the SP I created. This is likely because I had the AZURE_* env vars defined. I'm deleting them and trying again, but this may be something we want to clean up in the script.

For now, I'm opening this issue more as a tracking issue in case we see more of it and discuss what we can do about it. /cc @benbp

[heaths]

One problem I found is that the persisted, encrypted AZURE_CLIENT_SECRET is an empty string "", which makes sense since my user was used. I need that for several tests that rely on ClientSecretCredentials. /cc @christothes @weshaggard for ideas.

One option might just be to tell people to run these on an AD-joined machine e.g., DevBox. Or don't we have a way to do this in a CI? Thing is, Key Vault has some special instructions to completely re-record since since tests are exclusive to net47 while the major we want to first run in net7.0 or whatever the latest is for the repo.

[heaths]

Tentatively, this seems to work:

        protected TokenCredential GetCredential(string tenantId)
        {
            if (Mode == RecordedTestMode.Playback)
            {
                return new MockCredential();
            }

            return string.IsNullOrEmpty(TestEnvironment.ClientSecret)
                ? new AzurePowerShellCredential(
                    new AzurePowerShellCredentialOptions()
                    {
                        AuthorityHost = new Uri(TestEnvironment.AuthorityHostUrl),
                        AdditionallyAllowedTenants = { TestEnvironment.TenantId },
                    })
                : new ClientSecretCredential(
                    tenantId ?? TestEnvironment.TenantId,
                    TestEnvironment.ClientId,
                    TestEnvironment.ClientSecret,
                    new ClientSecretCredentialOptions()
                    {
                        AuthorityHost = new Uri(TestEnvironment.AuthorityHostUrl),
                        AdditionallyAllowedTenants = { TestEnvironment.TenantId },
                    });
        }

[JoshLove-msft]

I tried running New-TestResources.ps1 keyvault -AdditionalParameters @{enableHsm=$true} -UserAuth and, though provisioning worked, my tests still tried to run against the SP I created. This is likely because I had the AZURE_* env vars defined. I'm deleting them and trying again, but this may be something we want to clean up in the script.

Yes, the Credential logic is here. It will use the ClientSecretCredential if the secret env var is populated. The script should have still created your resources using your user, but the tests will use your service principal if the env vars are set.

[heaths]

Yes, the Credential logic is here. It will use the ClientSecretCredential if the secret env var is populated. The script should have still created your resources using your user, but the tests will use your service principal if the env vars are set.

The problem is that we have a script - test-resources-post.ps1 - that assigns RBAC roles because that's how it has to work for Managed HSM...so reused for KV. Those do use the test application credentials, which seem to use the test provisioner (user) account info, which now seems wrong:

        $userAccount = (Get-AzADUser -UserPrincipalName (Get-AzContext).Account)
        $TestApplicationOid = $userAccount.Id
        $TestApplicationId = $testApplicationOid

That's from New-TestResources.ps1.

[JoshLove-msft]

Can the test-resources-post script be updated to work for the UserAuth case?

[heaths]

It should already work. I'm saying the changes to the New-TestResources.ps1 script are wrong if it's using the user account to run tests. You said,

The script should have still created your resources using your user, but the tests will use your service principal if the env vars are set.

[JoshLove-msft]

Not sure what you mean there. The Test Framework code that chooses the credential is separate from the New-TestResources script.

[heaths]

The point was that, when I wrote the original scripts, there is the test provisioner identity and the test runner identity. They were separate on purpose. You said the tests will use the service principal if the env vars were set, which they were, but that identity was not granted the right RBAC permissions because of the change to the script I pasted above where test runner identity (test app) used the user identity.

[heaths]

The parameter docs read,

.PARAMETER UserAuth
Create the resource group and deploy the template using the signed in user's credentials.
No service principal will be created or used.

The environment file will be named for the test resources template that it was
generated for. For ARM templates, it will be test-resources.json.env. For
Bicep templates, test-resources.bicep.env.

That's a test provisioner. The test app, given this description, should've been left as-is. There's nothing wrong with my test-resources-post.ps1 script as written/intended: the test runner is who needs to have those permissions to run. /cc @benbp @hallipr

[JoshLove-msft]

You said the tests will use the service principal if the env vars were set, which they were, but that identity was not granted the right RBAC permissions because of the change to the script I pasted above where test runner identity (test app) used the user identity.

The service principal secret from your env var was not output by the New-TestResources script when run with UserAuth. I'm still not seeing how this is an issue with the New-TestResources script. Can you clarify what you think should change?

[heaths]

Right - because for a user, there is no secret - not emitted by the API anyway.

The problem is that the changes conflate the provisioning user with the running user. Look at the code fragment in the comment above again. That's wrong. If indeed -UserAuth is supposed to use the user for provisioning, the test application should not be set to the provisioning user. Those have always been distinct identities (defaulting to the same) for a reason: we can run with fewer privileges than provision. The initial scripts I wrote did this to reflect what most libraries in .NET and some in other languages were effectively requiring through manual provisioning e.g., auth as you to the Azure CLI to create an SP, set env vars on your machine with that SP's info, then run your tests as that SP. My test-resources-post.ps1 script was written with that design in mind.

The changes whoever made for -UserAuth have now effectively said, "use the user to provision and run tests." Not only is that less secure, it's apparently not what you intended you said,

The script should have still created your resources using your user, but the tests will use your service principal if the env vars are set.

My env vars were set to an SP, but because the changes set the test application to the provisioning identity, the wrong identity was given the necessary permissions. So, yes, based on everything discussed above, the changes to New-TestResources.ps1 are wrong. /cc @benbp

[JoshLove-msft]

The changes whoever made for -UserAuth have now effectively said, "use the user to provision and run tests." Not only is that less secure, it's apparently not what you intended you said,

The goal of the change is to use the user to provision and run tests. The reason the test is being run with an SP is because you have an env var that has been set externally to the New-TestResources script. If you clear out the env var, everything should work fine. That said, we may want to avoid setting the test application Id when userAuth is being used to avoid confusion.

[heaths]

I don't advise that. We are owners in a couple subs and you want to run tests as owners? Again, there's a reason there's a separate provisioner and it's how we always did things: we create an SP, if needed, and give that only the permissions required to run the test.

@weshaggard can you weigh in here? I think the recent user auth changes to the test resources scripts have made the system less secure. We should not be running tests as a highly-privileged provisioner. I appreciate the goal was not to leak SPs, but those should be lower-privileged typically, and ephemeral unlike user principals.

[JoshLove-msft]

I appreciate the goal was not to leak SPs, but those should be lower-privileged typically, and ephemeral unlike user principals.

This wasn't the goal. This change was necessitated by the recent policy change to not allow using SPs outside of Microsoft networks. Remote employees are not able to run live tests with SPs.