Skip to content
This repository has been archived by the owner on May 5, 2021. It is now read-only.

Added support for client side certificate validation #8

Merged
merged 4 commits into from Jul 31, 2020
Merged

Added support for client side certificate validation #8

merged 4 commits into from Jul 31, 2020

Conversation

Hakky54
Copy link
Contributor

@Hakky54 Hakky54 commented Jul 20, 2020

This PR is an implementation for the issue mentioned here: Feature Support for SSLContext

These code changes will enable users of asynkio to provide a custom ssl configuration by supplying a preconfigured SSLContext instance. That will provide the underlying client the options to validate the server certificate against your own list of trusted certificates. Besides validating the server certificate it also will have the support for mutual authentication.

@CuriousNikhil
Copy link
Owner

@Hakky54 That's awesome! Thanks for doing this.
Just one question, have you tested this on your own ?

@Hakky54
Copy link
Contributor Author

Hakky54 commented Jul 21, 2020

I just added a unit test to check if loading the sslcontext and executing a http request is working correctly. Please let me know what you thoughts are

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

public class NetworkCallTest {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind writing this in kotlin.
you can add kotlin support for test folder as well in gradle

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just changed it to kotlin.
But what do you mean exactly with:

you can add kotlin support for test folder as well in gradle

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot.
I thought kotlin dependency is not added for the test folder 🤦 .
but it already there, sorry my bad ✋.

Copy link
Owner

@CuriousNikhil CuriousNikhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting you to change the test file to kotlin.

@CuriousNikhil CuriousNikhil merged commit ec07633 into CuriousNikhil:master Jul 31, 2020
@CuriousNikhil
Copy link
Owner

Thanks for your contribution.

@Hakky54
Copy link
Contributor Author

Hakky54 commented Jul 31, 2020

It was my pleasure to contribute back 😊

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants