feat: Adds an X509 certificate provider in the auth libraries. #1624
Conversation
aeitzman
changed the title
oauth2_http/java/com/google/auth/mtls/WorkloadCertificateConfiguration.java
Show resolved
Hide resolved
| } | ||
| InputStream certConfigStream = null; | ||
| try { | ||
| if (!isFile(certConfig)) { |
There was a problem hiding this comment.
It looks like file.exists() will return true if the file is a directory, which we do not want here. isFile will return true only if it is a file
| } | ||
|
|
||
| boolean isFile(File file) { | ||
| return file.isFile(); |
There was a problem hiding this comment.
add null check here as good practice (even if file is not expected to be null)
There was a problem hiding this comment.
The purpose of this function is mainly just to help testing the provider by allowing us to mock the file read. I think we should just let the file.isFile() handle the null the check and throw the error instead of putting in our own error handling here.
I'll add comments to this block to make it more clear what the purpose of these functions are.
oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java
Outdated
Show resolved
Hide resolved
| return System.getProperty(property, def); | ||
| } | ||
|
|
||
| File getWellKnownCertificateConfigFile() { |
There was a problem hiding this comment.
I think all these helper methods should be "private" whenever possible right? and use protected for those that needs to be overridden by the test mock?
There was a problem hiding this comment.
I'll make this function private, for the methods that need to be overridden, I think its better to keep them package private instead of protected, since we wouldn't expect packages that ingest this library to override them. For context, this is the same pattern that we use for the Default credentials provider in this library (https://github.com/googleapis/google-auth-library-java/blob/main/oauth2_http/java/com/google/auth/oauth2/DefaultCredentialsProvider.java)
This PR creates a new "X509" certificate provider in a new Mtls package to handle workload X509 certificate reading. This is part of a larger feature described in go/x509-workload-auth-library-design
Merging this into a feature branch for now until the entire feature is ready.