Skip to content

Commit 9f9063b

Browse files
tcoffee-googletcoffee-google
committed
Establish precedence of auth methods including API key
Use following order of precedence for authentication, avoiding conflicts: 1. explicitly set credentials 2. explicitly set API key 3. Application Default Credentials (e.g., through GOOGLE_APPLICATION_CREDENTIALS) 4. default API key (through GOOGLE_API_KEY)
1 parent 639fefc commit 9f9063b

4 files changed

Lines changed: 106 additions & 27 deletions

File tree

‎google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public abstract class ServiceOptions<ServiceT extends Service<OptionsT>,
8686
private final String serviceRpcFactoryClassName;
8787
private final String serviceFactoryClassName;
8888
private final ApiClock clock;
89-
private final Credentials credentials;
89+
protected Credentials credentials;
9090
private final TransportOptions transportOptions;
9191

9292
private transient ServiceRpcFactory<OptionsT> serviceRpcFactory;
@@ -107,7 +107,7 @@ public abstract static class Builder<ServiceT extends Service<OptionsT>,
107107

108108
private String projectId;
109109
private String host;
110-
private Credentials credentials;
110+
protected Credentials credentials;
111111
private RetrySettings retrySettings;
112112
private ServiceFactory<ServiceT, OptionsT> serviceFactory;
113113
private ServiceRpcFactory<OptionsT> serviceRpcFactory;

‎google-cloud-translate/src/main/java/com/google/cloud/translate/TranslateOptions.java‎

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.common.base.MoreObjects.firstNonNull;
2020

21+
import com.google.auth.Credentials;
2122
import com.google.cloud.ServiceDefaults;
2223
import com.google.cloud.ServiceOptions;
2324
import com.google.cloud.ServiceRpc;
@@ -81,6 +82,12 @@ private Builder(TranslateOptions options) {
8182
this.apiKey = options.apiKey;
8283
}
8384

85+
/**
86+
* Returns the authentication credentials.
87+
*/
88+
public Credentials getCredentials() {
89+
return credentials;
90+
}
8491

8592
@Override
8693
public Builder setTransportOptions(TransportOptions transportOptions) {
@@ -102,11 +109,12 @@ public Builder setProjectId(String projectId) {
102109
return self();
103110
}
104111

105-
106112
/**
107-
* Sets the API key used to issue requets. If not set, the API key is looked for in the
108-
* {@code GOOGLE_API_KEY} environment variable. For instructions on how to get an API key see
109-
* <a href="https://cloud.google.com/translate/v2/quickstart">Translate quickstart</a>.
113+
* Sets the API key used to issue requests. This will be ignored if credentials are explicitly
114+
* set with {@link ServiceOptions.Builder#setCredentials setCredentials}. If neither are set,
115+
* and no Application Default Credentials are available, an API key is looked for in the
116+
* {@code GOOGLE_API_KEY} environment variable. For instructions on how to get an API key see <a
117+
* href="https://cloud.google.com/translate/v2/quickstart">Translate quickstart</a>.
110118
*/
111119
public Builder setApiKey(String apiKey) {
112120
this.apiKey = apiKey;
@@ -115,7 +123,7 @@ public Builder setApiKey(String apiKey) {
115123

116124

117125
/**
118-
* Sets the code for the default target language. If not set, english ({@code en}) is used.
126+
* Sets the code for the default target language. If not set, English ({@code en}) is used.
119127
* {@link Translate#translate(List, TranslateOption...)} and
120128
* {@link Translate#translate(String, TranslateOption...)} calls will use this
121129
* value unless a {@link TranslateOption#targetLanguage(String)} option is explicitly
@@ -136,20 +144,45 @@ public TranslateOptions build() {
136144

137145
private TranslateOptions(Builder builder) {
138146
super(TranslateFactory.class, TranslateRpcFactory.class, builder, new TranslateDefaults());
139-
if (builder.apiKey != null) {
140-
this.apiKey = builder.apiKey;
141-
} else if (System.getenv(ServiceOptions.CREDENTIAL_ENV_NAME) == null) {
142-
this.apiKey = getDefaultApiKey();
143-
} else {
144-
this.apiKey = null;
147+
// Use following order of precedence for authentication, avoiding conflicts:
148+
// 1. explicitly set credentials
149+
// 2. explicitly set API key
150+
// 3. Application Default Credentials (e.g., through GOOGLE_APPLICATION_CREDENTIALS)
151+
// 4. default API key (through GOOGLE_API_KEY)
152+
if (builder.getCredentials() != null) {
153+
// credentials assigned from builder in superclass
154+
apiKey = null;
155+
if (builder.apiKey != null) {
156+
logger.log(
157+
Level.WARNING, "Ignoring API key: using explicit setting for credentials instead.");
158+
} else if (getDefaultApiKey() != null) {
159+
logger.log(
160+
Level.WARNING,
161+
String.format(
162+
"Ignoring API key set in environment variable %s: using explicit setting for credentials instead.",
163+
API_KEY_ENV_NAME));
164+
}
165+
} else if (builder.apiKey != null) {
166+
credentials = null;
167+
apiKey = builder.apiKey;
145168
logger.log(
146169
Level.WARNING,
147170
String.format(
148-
"API key in environment variable %s overridden by service account credentials " +
149-
"specified in environment variable %s",
150-
API_KEY_ENV_NAME, ServiceOptions.CREDENTIAL_ENV_NAME));
171+
"Ignoring Application Default Credentials: using explicit setting for API key instead.",
172+
ServiceOptions.CREDENTIAL_ENV_NAME));
173+
} else if (credentials != null) { // credentials assigned from ADC in superclass
174+
apiKey = null;
175+
if (getDefaultApiKey() != null) {
176+
logger.log(
177+
Level.WARNING,
178+
String.format(
179+
"Ignoring API key set in environment variable %s: using Application Default Credentials instead.",
180+
API_KEY_ENV_NAME));
181+
}
182+
} else {
183+
apiKey = getDefaultApiKey();
151184
}
152-
this.targetLanguage = firstNonNull(builder.targetLanguage, Locale.ENGLISH.getLanguage());
185+
targetLanguage = firstNonNull(builder.targetLanguage, Locale.ENGLISH.getLanguage());
153186
}
154187

155188
private static class TranslateDefaults implements
@@ -201,7 +234,7 @@ protected String getDefaultApiKey() {
201234

202235

203236
/**
204-
* Returns the API key, to be used used to send requests.
237+
* Returns the API key, to be used to send requests.
205238
*/
206239
public String getApiKey() {
207240
return apiKey;

‎google-cloud-translate/src/test/java/com/google/cloud/translate/TranslateImplTest.java‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,30 +17,33 @@
1717
package com.google.cloud.translate;
1818

1919
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNull;
2021
import static org.junit.Assert.assertSame;
2122

23+
import com.google.api.gax.retrying.RetrySettings;
2224
import com.google.api.services.translate.model.DetectionsResourceItems;
2325
import com.google.api.services.translate.model.LanguagesResource;
2426
import com.google.api.services.translate.model.TranslationsResource;
25-
import com.google.api.gax.retrying.RetrySettings;
27+
import com.google.auth.Credentials;
28+
import com.google.cloud.NoCredentials;
2629
import com.google.cloud.ServiceOptions;
2730
import com.google.cloud.translate.Translate.LanguageListOption;
2831
import com.google.cloud.translate.Translate.TranslateOption;
29-
import com.google.cloud.translate.spi.v2.TranslateRpc;
3032
import com.google.cloud.translate.spi.TranslateRpcFactory;
33+
import com.google.cloud.translate.spi.v2.TranslateRpc;
3134
import com.google.common.collect.ImmutableList;
3235
import com.google.common.collect.ImmutableMap;
3336

37+
import java.util.List;
38+
import java.util.Map;
39+
3440
import org.easymock.EasyMock;
3541
import org.junit.After;
3642
import org.junit.Before;
3743
import org.junit.Rule;
3844
import org.junit.Test;
3945
import org.junit.rules.ExpectedException;
4046

41-
import java.util.List;
42-
import java.util.Map;
43-
4447
public class TranslateImplTest {
4548

4649
private static final String API_KEY = "api_key";
@@ -118,8 +121,7 @@ public void setUp() {
118121
.build();
119122
}
120123

121-
@After
122-
public void tearDown() throws Exception {
124+
private void verify() {
123125
EasyMock.verify(rpcFactoryMock, translateRpcMock);
124126
}
125127

@@ -132,6 +134,7 @@ public void testGetOptions() {
132134
EasyMock.replay(translateRpcMock);
133135
initializeService();
134136
assertSame(options, translate.getOptions());
137+
verify();
135138
}
136139

137140
@Test
@@ -141,6 +144,7 @@ public void testListSupportedLanguages() {
141144
EasyMock.replay(translateRpcMock);
142145
initializeService();
143146
assertEquals(LANGUAGES1, translate.listSupportedLanguages());
147+
verify();
144148
}
145149

146150
@Test
@@ -152,6 +156,7 @@ public void testListSupportedLanguagesWithOptions() {
152156
assertEquals(
153157
LANGUAGES2,
154158
translate.listSupportedLanguages(LanguageListOption.targetLanguage(TARGET_LANGUAGE)));
159+
verify();
155160
}
156161

157162
@Test
@@ -163,6 +168,7 @@ public void testDetect() {
163168
EasyMock.replay(translateRpcMock);
164169
initializeService();
165170
assertEquals(DETECTION1, translate.detect(text));
171+
verify();
166172
}
167173

168174
@Test
@@ -177,6 +183,7 @@ public void testDetectMultipleDetections() {
177183
thrown.expect(IllegalStateException.class);
178184
thrown.expectMessage("Multiple detections found for text: text");
179185
translate.detect(text);
186+
verify();
180187
}
181188

182189
@Test
@@ -191,6 +198,7 @@ public void testDetectNoDetection() {
191198
thrown.expect(IllegalStateException.class);
192199
thrown.expectMessage("No detection found for text: text");
193200
translate.detect(text);
201+
verify();
194202
}
195203

196204
@Test
@@ -205,6 +213,7 @@ public void testDetectList() {
205213
EasyMock.replay(translateRpcMock);
206214
initializeService();
207215
assertEquals(ImmutableList.of(DETECTION1, DETECTION2), translate.detect(texts));
216+
verify();
208217
}
209218

210219
@Test
@@ -221,6 +230,7 @@ public void testDetectListMultipleDetections() {
221230
thrown.expect(IllegalStateException.class);
222231
thrown.expectMessage("Multiple detections found for text: text");
223232
translate.detect(texts);
233+
verify();
224234
}
225235

226236
@Test
@@ -237,6 +247,7 @@ public void testDetectListNoDetection() {
237247
thrown.expect(IllegalStateException.class);
238248
thrown.expectMessage("No detection found for text: other text");
239249
translate.detect(texts);
250+
verify();
240251
}
241252

242253
@Test
@@ -250,6 +261,7 @@ public void testDetectVararg() {
250261
EasyMock.replay(translateRpcMock);
251262
initializeService();
252263
assertEquals(ImmutableList.of(DETECTION1, DETECTION2), translate.detect(text1, text2));
264+
verify();
253265
}
254266

255267
@Test
@@ -265,6 +277,7 @@ public void testDetectVarargMultipleDetections() {
265277
thrown.expect(IllegalStateException.class);
266278
thrown.expectMessage("Multiple detections found for text: text");
267279
translate.detect(text1, text2);
280+
verify();
268281
}
269282

270283
@Test
@@ -280,6 +293,7 @@ public void testDetectVarargNoDetection() {
280293
thrown.expect(IllegalStateException.class);
281294
thrown.expectMessage("No detection found for text: other text");
282295
translate.detect(text1, text2);
296+
verify();
283297
}
284298

285299
@Test
@@ -290,6 +304,7 @@ public void testTranslate() {
290304
EasyMock.replay(translateRpcMock);
291305
initializeService();
292306
assertEquals(TRANSLATION1, translate.translate(text));
307+
verify();
293308
}
294309

295310
@Test
@@ -302,6 +317,7 @@ public void testTranslateWithOptions() {
302317
assertEquals(
303318
TRANSLATION2,
304319
translate.translate(text, TARGET_LANGUAGE_OPTION, SOURCE_LANGUAGE_OPTION, MODEL_OPTION));
320+
verify();
305321
}
306322

307323
@Test
@@ -314,6 +330,7 @@ public void testTranslateList() {
314330
EasyMock.replay(translateRpcMock);
315331
initializeService();
316332
assertEquals(ImmutableList.of(TRANSLATION1, TRANSLATION2), translate.translate(texts));
333+
verify();
317334
}
318335

319336
@Test
@@ -327,6 +344,7 @@ public void testTranslateListWithOptions() {
327344
assertEquals(
328345
ImmutableList.of(TRANSLATION2),
329346
translate.translate(texts, TARGET_LANGUAGE_OPTION, SOURCE_LANGUAGE_OPTION, MODEL_OPTION));
347+
verify();
330348
}
331349

332350
@Test
@@ -342,6 +360,7 @@ public void testRetryableException() {
342360
.build()
343361
.getService();
344362
assertEquals(LANGUAGES1, translate.listSupportedLanguages());
363+
verify();
345364
}
346365

347366
@Test
@@ -359,6 +378,7 @@ public void testNonRetryableException() {
359378
thrown.expect(TranslateException.class);
360379
thrown.expectMessage(exceptionMessage);
361380
translate.listSupportedLanguages();
381+
verify();
362382
}
363383

364384
@Test
@@ -376,5 +396,14 @@ public void testRuntimeException() {
376396
thrown.expect(TranslateException.class);
377397
thrown.expectMessage(exceptionMessage);
378398
translate.listSupportedLanguages();
399+
verify();
400+
}
401+
402+
@Test
403+
public void testCredentialsOverridesApiKey() {
404+
Credentials credentials = NoCredentials.getInstance();
405+
TranslateOptions overridden = options.toBuilder().setCredentials(credentials).build();
406+
assertSame(overridden.getCredentials(), credentials);
407+
assertNull(overridden.getApiKey());
379408
}
380409
}

‎google-cloud-translate/src/test/java/com/google/cloud/translate/it/ITTranslateTest.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,25 @@
1818

1919
import static org.junit.Assert.assertEquals;
2020
import static org.junit.Assert.assertNotNull;
21+
import static org.junit.Assert.assertNull;
2122
import static org.junit.Assert.assertTrue;
2223

2324
import com.google.cloud.translate.Detection;
2425
import com.google.cloud.translate.Language;
2526
import com.google.cloud.translate.Translate;
2627
import com.google.cloud.translate.Translate.LanguageListOption;
2728
import com.google.cloud.translate.Translate.TranslateOption;
29+
import com.google.cloud.translate.TranslateOptions;
2830
import com.google.cloud.translate.Translation;
2931
import com.google.cloud.translate.testing.RemoteTranslateHelper;
3032
import com.google.common.collect.ImmutableList;
3133

32-
import org.junit.Test;
33-
3434
import java.util.HashSet;
3535
import java.util.List;
3636
import java.util.Set;
3737

38+
import org.junit.Test;
39+
3840
public class ITTranslateTest {
3941

4042
private static final Translate TRANSLATE =
@@ -46,6 +48,7 @@ public class ITTranslateTest {
4648
"mt", "mi", "mr", "mn", "my", "ne", "no", "fa", "pl", "pt", "ro", "ru", "sr", "st", "si",
4749
"sk", "sl", "so", "es", "su", "sw", "sv", "tg", "ta", "te", "th", "tr", "uk", "ur", "uz",
4850
"vi", "cy", "yi", "yo", "zu"};
51+
private static final String API_KEY = "api_key";
4952

5053
@Test
5154
public void testListSupportedLanguages() {
@@ -137,4 +140,18 @@ public void testTranslateTextWithOptions() {
137140
assertEquals("Hallo", translation.getTranslatedText());
138141
assertEquals("es", translation.getSourceLanguage());
139142
}
143+
144+
@Test
145+
public void testApiKeyOverridesDefaultCredentials() {
146+
TranslateOptions options = RemoteTranslateHelper.create(API_KEY).getOptions();
147+
assertNull(options.getCredentials());
148+
assertEquals(options.getApiKey(), API_KEY);
149+
}
150+
151+
@Test
152+
public void testDefaultCredentialsOverridesDefaultApiKey() {
153+
TranslateOptions options = RemoteTranslateHelper.create().getOptions();
154+
assertNotNull(options.getCredentials());
155+
assertNull(options.getApiKey());
156+
}
140157
}

0 commit comments

Comments
 (0)