Initial support for new_context clientCertificates

[okraus-ari]

I would like to contribute support for authentication with client certificates, added in Playwright version 1.46:

https://playwright.dev/docs/release-notes#version-146

Initial version of this functionality covers only adding certificates with keyword New Context:

New Context    clientCertificates=[{'origin': 'https://playwright.dev', 'pfxPath': 'certificate.p12', 'passphrase': 'password'}]

As I am pretty new to Python and Robot framework, I did not cover the implementation with tests. I would probably need some help with that.

Fixes #3860

[aaltat]

Hi
You are off with good start. The https://github.com/MarketSquare/robotframework-browser/blob/main/CONTRIBUTING.md has information how to setup your development environment if you have not done that already, invoke to perform actions, like invoke lint or ìnvoke atest. Also you PR is in the right course, you added the argument in the right place.

Few notes

1. We document arguments also in the keyword documentation so you should add short description in there too.
2. Node side should be OK, unless testing reveal something else.
3. Acceptance test is needed, most likely somewhere here

Writing test is tricky, because in our test app we do not have support for this one (I think, I did not look into it that deeply.) At least there should be test which uses the argument. But it would be really nice if you could do modifications on the test app to use the certificate.

[aaltat]

Also look at the CI failure, it seems that you should run invoke lint to fix the problem and commit/push again.

[aaltat]

@allcontributors please add @okraus-ari for code.

[allcontributors]

@aaltat

I've put up a pull request to add @okraus-ari! 🎉

[okraus-ari]

Ok, I have added the documentation and did some tidying, all tests passed.

Would you like to go forward without the tests or should we cover the case? I think this should not be that complicated if we can agree on adding a certificate just for the client and omitting mTLS authentication part on the server side. That would be quite complicated, I think.

[aaltat]

Yes, we can go forward with just the clint side tests.

[aaltat]

Really nice work on writing that test.

[aaltat]

There is small linting problem with test you wrote. Running invoke lint and commit + push should fixit.

[aaltat]

Attached logs because there seems to be some sort of an error in the new tests
ubuntu-latest 3.11 20.x Clean install results.zip

[okraus-ari]

I'm sorry, I have missed your comments, it was work in progress.

In the end I had to drop support for encrypted private keys as I was not able to convince NodeJS to accept the passphrase. I think that it should be ok to stick with plain private keys for now and we can try to add some encryption afterwards.

There is also one problem, I might have broken something. I had to strenghten checks in stop_test_server() and now some other tests started to fail. Can you look into it? I can of course move my code to some other function, but there is a chance, that these problems were previously hidden.

One last thing, I tried to set ${HTTPS_SERVER_PORT} the same way ${SERVER_PORT} is set in init.robot, but it did not work for me and sticked to the default value in variables.resource. Am I missing something?

[aaltat]

At least one run had some unrelated error. Rerunning.

[aaltat]

Looks like Windows file paths are problematic:
windows-latest 3.11 18.x Clean install results.zip

And for Mac there some other problem:
Test results-macos-latest-1-3.11-20.x.zip

[aaltat]

Linux failure:
ubuntu-latest 3.11 20.x Clean install results.zip

Windows failure
Test results-windows-latest-1-3.10-18.x.zip

[okraus-ari]

It looks like a bug in Robot Framework, in Windows log you can see that this path:

'D:\a\robotframework-browser\robotframework-browser\atest\output\pabot_results\0/client.crt'

gets translated into:

'D:\x07\robotframework-browser\robotframework-browser\x07test\\output\\pabot_results\x00/client.crt'

[aaltat]

Well, you could add logging in Python and Node side, in various places to see where path is converted incorrectly.