-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add mtls support for NetHttpTransport #1147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to add tests for this
google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/util/SslUtils.java
Outdated
Show resolved
Hide resolved
Tests are added now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're going to hold off on approving and merging this approach (especially the isMtls()
accessor) until the whole design is complete and approved.
google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java
Outdated
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really close.
google-http-client/src/test/java/com/google/api/client/util/SecurityUtilsTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For open source projects like this one, it's very helpful to have designs and issues on Github rather than in corp.
MTLS support will also need to be implemented for the Apache transport as well. |
I added a summary of the new functions added, I think they are self explanatory. The design doc is for client libs, http client is just a building block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits
google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java
Outdated
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java
Outdated
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/util/SecurityUtils.java
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/util/SslUtils.java
Outdated
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/util/SslUtils.java
Outdated
Show resolved
Hide resolved
google-http-client/src/main/java/com/google/api/client/util/SslUtils.java
Outdated
Show resolved
Hide resolved
google-http-client/src/test/java/com/google/api/client/util/SecurityUtilsTest.java
Outdated
Show resolved
Hide resolved
google-http-client/src/test/java/com/google/api/client/util/SecurityUtilsTest.java
Outdated
Show resolved
Hide resolved
google-http-client/src/test/java/com/google/api/client/util/SecurityUtilsTest.java
Outdated
Show resolved
Hide resolved
…vanet/NetHttpTransport.java Co-authored-by: Jeff Ching <chingor@google.com>
…vanet/NetHttpTransport.java Co-authored-by: Jeff Ching <chingor@google.com>
…lUtils.java Co-authored-by: Jeff Ching <chingor@google.com>
…lUtils.java Co-authored-by: Jeff Ching <chingor@google.com>
…curityUtilsTest.java Co-authored-by: Jeff Ching <chingor@google.com>
…lUtils.java Co-authored-by: Jeff Ching <chingor@google.com>
@arithmetic1728 Let's follow up with a PR to add |
🤖 I have created a release \*beep\* \*boop\* --- ## [1.38.0](https://www.github.com/googleapis/google-http-java-client/compare/v1.37.0...v1.38.0) (2020-11-02) ### Features * add isMtls property to ApacheHttpTransport ([#1168](https://www.github.com/googleapis/google-http-java-client/issues/1168)) ([c416e20](https://www.github.com/googleapis/google-http-java-client/commit/c416e201c92a5c5fc1b1c59c5dd63e8ec1463f5f)) * add mtls support for NetHttpTransport ([#1147](https://www.github.com/googleapis/google-http-java-client/issues/1147)) ([51762f2](https://www.github.com/googleapis/google-http-java-client/commit/51762f221ec8ab38da03149c8012e63aec0433dc)) ### Dependencies * guava 30.0-android ([#1151](https://www.github.com/googleapis/google-http-java-client/issues/1151)) ([969dbbf](https://www.github.com/googleapis/google-http-java-client/commit/969dbbf127708aff16309f82538aca6f0a651638)) ### Documentation * libraries-bom 13.4.0 ([#1170](https://www.github.com/googleapis/google-http-java-client/issues/1170)) ([6818a02](https://www.github.com/googleapis/google-http-java-client/commit/6818a02a15e1bef8e9f5ea56a4ecc2b8d0646f9b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
go/java-apiary-client-mtls (section 2.2.2.1)
Support
(1) passing keystore to SslContext constructor for client certificate and private key.
(2) creating keystore from certAndKey input stream.
Summary of new functions: