Fix format of ClientCertificate[Key]Data from auth plugins

[Frassle]

ClientCertificateData and ClientCertificateKeyData are expected to be base64 encoded strings by CertUtils.GeneratePfx. However auth plugins return the actual "--BEGIN X---...---END X---" strings, this commit base64 encodes that to match as if it was read from the config file.

[Frassle]

I missed this distinction of how these strings we're formatted in my last PR (#479).

/assign @brendandburns

[brendandburns]

This looks good, can you add a unit test to validate the behavior and protect against future breakage?

Thanks!

[Frassle]

can you add a unit test

I did have a look at this quickly and wasn't sure the best way to unit test. We're calling into System.Diagnostics.Process and I couldn't think of any easy way to mock that out for a test. I could make a config that has command set to cmd.exe or /bin/sh with arguments that just cause it to echo back an expected json blob, but that seemed a big ugly.

[brendandburns]

In other contexts, I have just used echo to echo back the json blob. I think that's actually the right way to do it.

Thanks!

[Frassle]

I've added another test to the auth tests copying bits of the external token test and the certificate tests.

N.B The external auth test looks like it would never run because it was ifdefed for NETSTANDARD_2_0 but the tests either run as netcoreapp2.0 or netcoreapp2.1 so I've removed that ifdef

[brendandburns]

Thanks, looks like the test is failing though with some JSON parse error...

[Frassle]

Looking in to it, I only checked it on my windows machine (removing the exclude clause to get it to run, seemed fine but I'm going to trust the comment that cert use is intermittent)

[Frassle]

Seems the problem is these external tests weren't ever running so the echo handling is wrong. I've raised #484 as a smaller PR to just enable the one existing external test and get it working. After that's merged I'll merge master into here and it should fix up this PR as well.

[brendandburns]

Cool, thanks for digging in here. I merged #484 so this should be ready once you rebase.

[brendandburns]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, Frassle

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [brendandburns]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment