Skip to content

Commit 31f9a11

Browse files
committed
Addressed comments.
1 parent aaa5ba6 commit 31f9a11

6 files changed

Lines changed: 34 additions & 46 deletions

File tree

oauth2_http/java/com/google/auth/mtls/MtlsUtils.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
package com.google.auth.mtls;
3232

33+
import com.google.api.core.InternalApi;
3334
import com.google.auth.oauth2.EnvironmentProvider;
3435
import com.google.auth.oauth2.PropertyProvider;
3536
import com.google.common.base.Strings;
@@ -44,7 +45,8 @@
4445
*
4546
* <p>For internal use only.
4647
*/
47-
class MtlsUtils {
48+
@InternalApi
49+
public class MtlsUtils {
4850
static final String CERTIFICATE_CONFIGURATION_ENV_VARIABLE = "GOOGLE_API_CERTIFICATE_CONFIG";
4951
static final String WELL_KNOWN_CERTIFICATE_CONFIG_FILE = "certificate_config.json";
5052
static final String CLOUDSDK_CONFIG_DIRECTORY = "gcloud";
@@ -60,7 +62,7 @@ private MtlsUtils() {
6062
* @return The path to the certificate file.
6163
* @throws IOException if the certificate configuration cannot be found or loaded.
6264
*/
63-
static String getCertificatePath(
65+
public static String getCertificatePath(
6466
EnvironmentProvider envProvider, PropertyProvider propProvider, String certConfigPathOverride)
6567
throws IOException {
6668
String certPath =
@@ -103,7 +105,9 @@ static WorkloadCertificateConfiguration getWorkloadCertificateConfiguration(
103105
}
104106

105107
if (!certConfig.isFile()) {
106-
throw new CertificateSourceUnavailableException("File does not exist.");
108+
throw new CertificateSourceUnavailableException(
109+
"Certificate configuration file does not exist or is not a file: "
110+
+ certConfig.getAbsolutePath());
107111
}
108112
try (InputStream certConfigStream = new FileInputStream(certConfig)) {
109113
return WorkloadCertificateConfiguration.fromCertificateConfigurationStream(certConfigStream);

oauth2_http/java/com/google/auth/mtls/X509Provider.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,21 +95,6 @@ public X509Provider() {
9595
this(null);
9696
}
9797

98-
/**
99-
* Returns the path to the client certificate file specified by the loaded workload certificate
100-
* configuration.
101-
*
102-
* <p>If the configuration has not been loaded yet, this method will attempt to load it first by
103-
* searching the override path, environment variable, and well-known locations.
104-
*
105-
* @return The path to the certificate file.
106-
* @throws IOException if the certificate configuration cannot be found or loaded, or if the
107-
* configuration file does not specify a certificate path.
108-
*/
109-
public String getCertificatePath() throws IOException {
110-
return MtlsUtils.getCertificatePath(envProvider, propProvider, certConfigPathOverride);
111-
}
112-
11398
/**
11499
* Finds the certificate configuration file, then builds a Keystore using the X.509 certificate
115100
* and private key pointed to by the configuration. This will check the following locations in

oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import com.google.auth.http.HttpTransportFactory;
3535
import com.google.auth.mtls.MtlsHttpTransportFactory;
36+
import com.google.auth.mtls.MtlsUtils;
3637
import com.google.auth.mtls.X509Provider;
3738
import com.google.auth.oauth2.IdentityPoolCredentialSource.IdentityPoolCredentialSourceType;
3839
import com.google.common.annotations.VisibleForTesting;
@@ -166,7 +167,10 @@ private IdentityPoolSubjectTokenSupplier createCertificateSubjectTokenSupplier(
166167
this.transportFactory = new MtlsHttpTransportFactory(mtlsKeyStore);
167168

168169
// Initialize the subject token supplier with the certificate path.
169-
credentialSource.setCredentialLocation(x509Provider.getCertificatePath());
170+
String explicitCertConfigPath = getExplicitCertConfigPath(credentialSource);
171+
credentialSource.setCredentialLocation(
172+
MtlsUtils.getCertificatePath(
173+
getEnvironmentProvider(), getPropertyProvider(), explicitCertConfigPath));
170174
return new CertificateIdentityPoolSubjectTokenSupplier(credentialSource);
171175
}
172176

@@ -179,16 +183,21 @@ private X509Provider getX509Provider(
179183
X509Provider x509Provider = builder.x509Provider;
180184
if (x509Provider == null) {
181185
// Determine the certificate path based on the configuration.
182-
String explicitCertConfigPath =
183-
certConfig.useDefaultCertificateConfig()
184-
? null
185-
: certConfig.getCertificateConfigLocation();
186+
String explicitCertConfigPath = getExplicitCertConfigPath(credentialSource);
186187
x509Provider =
187188
new X509Provider(getEnvironmentProvider(), getPropertyProvider(), explicitCertConfigPath);
188189
}
189190
return x509Provider;
190191
}
191192

193+
private static String getExplicitCertConfigPath(IdentityPoolCredentialSource credentialSource) {
194+
IdentityPoolCredentialSource.CertificateConfig certConfig =
195+
credentialSource.getCertificateConfig();
196+
return certConfig.useDefaultCertificateConfig()
197+
? null
198+
: certConfig.getCertificateConfigLocation();
199+
}
200+
192201
public static class Builder extends ExternalAccountCredentials.Builder {
193202

194203
private IdentityPoolSubjectTokenSupplier subjectTokenSupplier;

oauth2_http/java/com/google/auth/oauth2/SystemEnvironmentProvider.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package com.google.auth.oauth2;
22

3+
import com.google.api.core.InternalApi;
34
import java.io.Serializable;
45

56
/**
67
* Represents the default system environment provider.
78
*
89
* <p>For internal use only.
910
*/
11+
@InternalApi
1012
public class SystemEnvironmentProvider implements EnvironmentProvider, Serializable {
1113
static final SystemEnvironmentProvider INSTANCE = new SystemEnvironmentProvider();
1214
private static final long serialVersionUID = -4698164985883575244L;

oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,12 @@ class X509ProviderTest {
6060
void x509Provider_fileDoesntExist_throws() {
6161
String certConfigPath = "badfile.txt";
6262
X509Provider testProvider = new X509Provider(certConfigPath);
63-
String expectedErrorMessage = "File does not exist.";
64-
63+
String expectedErrorMessage =
64+
"Certificate configuration file does not exist or is not a file: "
65+
+ new File(certConfigPath).getAbsolutePath();
6566
CertificateSourceUnavailableException exception =
6667
assertThrows(CertificateSourceUnavailableException.class, testProvider::getKeyStore);
67-
assertTrue(exception.getMessage().contains(expectedErrorMessage));
68+
assertEquals(expectedErrorMessage, exception.getMessage());
6869
}
6970

7071
@Test

oauth2_http/javatests/com/google/auth/oauth2/IdentityPoolCredentialsTest.java

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,6 @@ void build_withCertificateSourceAndCustomX509Provider_success()
11201120

11211121
// Verify the custom provider methods were called during build.
11221122
verify(x509Provider).getKeyStore();
1123-
verify(x509Provider).getCertificatePath();
11241123
}
11251124

11261125
@Test
@@ -1182,26 +1181,27 @@ void build_withCustomProvider_throwsOnGetKeyStore()
11821181
@Test
11831182
void build_withCustomProvider_throwsOnGetCertificatePath()
11841183
throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException {
1185-
// Simulate a scenario where the X509Provider cannot access or read the certificate
1186-
// configuration file needed to determine the certificate path, resulting in an IOException.
1184+
// Simulate a scenario where path resolution fails during build with a custom
1185+
// provider.
1186+
// We achieve this by passing a non-existent configuration path which causes
1187+
// MtlsUtils to throw
1188+
// IOException.
11871189
KeyStore keyStore = KeyStore.getInstance("JKS");
11881190
keyStore.load(null, null);
11891191
TestX509Provider x509Provider = new TestX509Provider(keyStore, "/path/to/certificate.json");
1190-
x509Provider.setShouldThrowOnGetCertificatePath(true); // Configure to throw
11911192

11921193
Map<String, Object> certificateMap = new HashMap<>();
1193-
certificateMap.put("certificate_config_location", "/path/to/certificate.json");
1194+
certificateMap.put("certificate_config_location", "/non/existent/path.json");
11941195

11951196
// Expect RuntimeException because the constructor wraps the IOException.
11961197
RuntimeException exception =
11971198
assertThrows(
11981199
RuntimeException.class,
11991200
() -> createCredentialsWithCertificate(x509Provider, certificateMap));
12001201

1201-
// Verify the cause is the expected IOException from the mock.
1202+
// Verify the cause is the expected IOException (or subclass) from MtlsUtils.
12021203
assertNotNull(exception.getCause());
12031204
assertTrue(exception.getCause() instanceof IOException);
1204-
assertEquals("Simulated IOException on certificate path", exception.getCause().getMessage());
12051205

12061206
// Verify the wrapper exception message
12071207
assertEquals(
@@ -1310,7 +1310,6 @@ private static class TestX509Provider extends X509Provider {
13101310
private final KeyStore keyStore;
13111311
private final String certificatePath;
13121312
private boolean shouldThrowOnGetKeyStore = false;
1313-
private boolean shouldThrowOnGetCertificatePath = false;
13141313

13151314
TestX509Provider(KeyStore keyStore, String certificatePath) {
13161315
super();
@@ -1326,20 +1325,8 @@ public KeyStore getKeyStore() throws IOException {
13261325
return keyStore;
13271326
}
13281327

1329-
@Override
1330-
public String getCertificatePath() throws IOException {
1331-
if (shouldThrowOnGetCertificatePath) {
1332-
throw new IOException("Simulated IOException on certificate path");
1333-
}
1334-
return certificatePath;
1335-
}
1336-
13371328
void setShouldThrowOnGetKeyStore(boolean shouldThrow) {
13381329
this.shouldThrowOnGetKeyStore = shouldThrow;
13391330
}
1340-
1341-
void setShouldThrowOnGetCertificatePath(boolean shouldThrow) {
1342-
this.shouldThrowOnGetCertificatePath = shouldThrow;
1343-
}
13441331
}
13451332
}

0 commit comments

Comments
 (0)