Skip to content

Commit cdf9cf9

Browse files
katcharovkatcharov
committed
Add expiresIn, address PR comments
1 parent be0cd71 commit cdf9cf9

File tree

6 files changed

+81
-53
lines changed

6 files changed

+81
-53
lines changed

‎driver-core/src/main/com/mongodb/ConnectionString.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@
4444
import java.util.Objects;
4545
import java.util.Set;
4646
import java.util.concurrent.TimeUnit;
47+
import java.util.stream.Collectors;
48+
import java.util.stream.Stream;
4749

50+
import static com.mongodb.MongoCredential.ALLOWED_HOSTS_KEY;
4851
import static com.mongodb.internal.connection.OidcAuthenticator.OidcValidator.validateCreateOidcCredential;
4952
import static java.lang.String.format;
5053
import static java.util.Arrays.asList;
@@ -272,6 +275,9 @@ public class ConnectionString {
272275
private static final Set<String> ALLOWED_OPTIONS_IN_TXT_RECORD =
273276
new HashSet<>(asList("authsource", "replicaset", "loadbalanced"));
274277
private static final Logger LOGGER = Loggers.getLogger("uri");
278+
private static final List<String> MECHANISM_KEYS_DISALLOWED_IN_CONNECTION_STRING = Stream.of(ALLOWED_HOSTS_KEY)
279+
.map(k -> k.toLowerCase())
280+
.collect(Collectors.toList());
275281

276282
private final MongoCredential credential;
277283
private final boolean isSrvProtocol;
@@ -902,6 +908,11 @@ private MongoCredential createCredentials(final Map<String, List<String>> option
902908
}
903909
String key = mechanismPropertyKeyValue[0].trim().toLowerCase();
904910
String value = mechanismPropertyKeyValue[1].trim();
911+
if (MECHANISM_KEYS_DISALLOWED_IN_CONNECTION_STRING.contains(key)) {
912+
throw new IllegalArgumentException(format("The connection string contains disallowed mechanism properties. "
913+
+ "'%s' must be set on the credential programmatically.", key));
914+
}
915+
905916
if (key.equals("canonicalize_host_name")) {
906917
credential = credential.withMechanismProperty(key, Boolean.valueOf(value));
907918
} else {

‎driver-core/src/main/com/mongodb/MongoCredential.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,13 +245,13 @@ public final class MongoCredential {
245245
* The list of allowed hosts that will be used if no
246246
* {@link MongoCredential#ALLOWED_HOSTS_KEY} value is supplied.
247247
* The default allowed hosts are:
248-
* {@code "*.mongodb.net", "*.mongodb-dev.net", "*.mongodbgov.net", "localhost", "127.0.0.1", "::1"}
248+
* {@code "*.mongodb.net", "*.mongodb-qa.net", "*.mongodb-dev.net", "*.mongodbgov.net", "localhost", "127.0.0.1", "::1"}
249249
*
250250
* @see #createOidcCredential(String)
251251
* @since 4.10
252252
*/
253253
public static final List<String> DEFAULT_ALLOWED_HOSTS = Collections.unmodifiableList(Arrays.asList(
254-
"*.mongodb.net", "*.mongodb-dev.net", "*.mongodbgov.net", "localhost", "127.0.0.1", "::1"));
254+
"*.mongodb.net", "*.mongodb-qa.net", "*.mongodb-dev.net", "*.mongodbgov.net", "localhost", "127.0.0.1", "::1"));
255255

256256
/**
257257
* Creates a MongoCredential instance with an unspecified mechanism. The client will negotiate the best mechanism based on the
@@ -708,28 +708,37 @@ public interface IdpInfo {
708708
/**
709709
* The response produced by an OIDC Identity Provider.
710710
*/
711-
@Evolving
712711
public static final class OidcCallbackResult {
713712

714713
private final String accessToken;
715714

715+
@Nullable
716+
private final Duration expiresIn;
717+
716718
@Nullable
717719
private final String refreshToken;
718720

719721
/**
720-
* @param accessToken The OIDC access token
722+
* @param accessToken The OIDC access token.
723+
* @param expiresIn Time until the access token expires. 0 is an infinite duration.
721724
*/
722-
public OidcCallbackResult(final String accessToken) {
723-
this(accessToken, null);
725+
public OidcCallbackResult(final String accessToken, final Duration expiresIn) {
726+
this(accessToken, expiresIn, null);
724727
}
725728

726729
/**
727-
* @param accessToken The OIDC access token
730+
* @param accessToken The OIDC access token.
731+
* @param expiresIn Time until the access token expires. 0 is an infinite duration.
728732
* @param refreshToken The refresh token. If null, refresh will not be attempted.
729733
*/
730-
public OidcCallbackResult(final String accessToken, @Nullable final String refreshToken) {
734+
public OidcCallbackResult(final String accessToken, @Nullable final Duration expiresIn,
735+
@Nullable final String refreshToken) {
731736
notNull("accessToken", accessToken);
737+
if (expiresIn != null && expiresIn.isNegative()) {
738+
throw new IllegalArgumentException("expiresIn must not be a negative value");
739+
}
732740
this.accessToken = accessToken;
741+
this.expiresIn = expiresIn;
733742
this.refreshToken = refreshToken;
734743
}
735744

‎driver-core/src/main/com/mongodb/internal/connection/OidcAuthenticator.java

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -157,20 +157,20 @@ private boolean isAutomaticAuthentication() {
157157
}
158158

159159
private boolean isHumanCallback() {
160-
return getMechanismProperty(OIDC_HUMAN_CALLBACK_KEY) != null;
160+
return getOidcCallbackMechanismProperty(OIDC_HUMAN_CALLBACK_KEY) != null;
161161
}
162162

163163
@Nullable
164-
private OidcCallback getMechanismProperty(final String key) {
164+
private OidcCallback getOidcCallbackMechanismProperty(final String key) {
165165
return getMongoCredentialWithCache()
166166
.getCredential()
167167
.getMechanismProperty(key, null);
168168
}
169169

170170
@Nullable
171171
private OidcCallback getRequestCallback() {
172-
OidcCallback machine = getMechanismProperty(OIDC_CALLBACK_KEY);
173-
return machine != null ? machine : getMechanismProperty(OIDC_HUMAN_CALLBACK_KEY);
172+
OidcCallback machine = getOidcCallbackMechanismProperty(OIDC_CALLBACK_KEY);
173+
return machine != null ? machine : getOidcCallbackMechanismProperty(OIDC_HUMAN_CALLBACK_KEY);
174174
}
175175

176176
@Override
@@ -259,7 +259,6 @@ private byte[] evaluate(final byte[] challenge) {
259259
// original IDP info will be present, if refresh token present
260260
assertNotNull(cachedIdpInfo);
261261
// Invoke Callback using cached Refresh Token
262-
validateAllowedHosts(getMongoCredential());
263262
fallbackState = FallbackState.PHASE_2_REFRESH_CALLBACK_TOKEN;
264263
OidcCallbackResult result = requestCallback.onRequest(new OidcCallbackContextImpl(
265264
CALLBACK_TIMEOUT, cachedIdpInfo, cachedRefreshToken));
@@ -335,26 +334,30 @@ private boolean clientIsComplete() {
335334
}
336335

337336
private boolean shouldRetryHandler() {
338-
MongoCredentialWithCache mongoCredentialWithCache = getMongoCredentialWithCache();
339-
OidcCacheEntry cacheEntry = mongoCredentialWithCache.getOidcCacheEntry();
340-
if (fallbackState == FallbackState.PHASE_1_CACHED_TOKEN) {
341-
// a cached access token failed
342-
mongoCredentialWithCache.setOidcCacheEntry(cacheEntry
343-
.clearAccessToken());
344-
return true;
345-
} else if (fallbackState == FallbackState.PHASE_2_REFRESH_CALLBACK_TOKEN) {
346-
// a refresh token failed
347-
mongoCredentialWithCache.setOidcCacheEntry(cacheEntry
348-
.clearAccessToken()
349-
.clearRefreshToken());
350-
return true;
351-
} else {
352-
// a clean-restart failed
353-
mongoCredentialWithCache.setOidcCacheEntry(cacheEntry
354-
.clearAccessToken()
355-
.clearRefreshToken());
356-
return false;
357-
}
337+
boolean[] result = new boolean[1];
338+
Locks.withLock(getMongoCredentialWithCache().getOidcLock(), () -> {
339+
MongoCredentialWithCache mongoCredentialWithCache = getMongoCredentialWithCache();
340+
OidcCacheEntry cacheEntry = mongoCredentialWithCache.getOidcCacheEntry();
341+
if (fallbackState == FallbackState.PHASE_1_CACHED_TOKEN) {
342+
// a cached access token failed
343+
mongoCredentialWithCache.setOidcCacheEntry(cacheEntry
344+
.clearAccessToken());
345+
result[0] = true;
346+
} else if (fallbackState == FallbackState.PHASE_2_REFRESH_CALLBACK_TOKEN) {
347+
// a refresh token failed
348+
mongoCredentialWithCache.setOidcCacheEntry(cacheEntry
349+
.clearAccessToken()
350+
.clearRefreshToken());
351+
result[0] = true;
352+
} else {
353+
// a clean-restart failed
354+
mongoCredentialWithCache.setOidcCacheEntry(cacheEntry
355+
.clearAccessToken()
356+
.clearRefreshToken());
357+
result[0] = false;
358+
}
359+
});
360+
return result[0];
358361
}
359362

360363
@Nullable
@@ -516,7 +519,8 @@ private void validateAllowedHosts(final MongoCredential credential) {
516519
});
517520
if (!permitted) {
518521
throw new MongoSecurityException(
519-
credential, "Host not permitted by " + ALLOWED_HOSTS_KEY + ": " + host);
522+
credential, "Host " + host + " not permitted by " + ALLOWED_HOSTS_KEY
523+
+ ", values: " + allowedHosts);
520524
}
521525
}
522526

@@ -568,30 +572,29 @@ public static void validateCreateOidcCredential(@Nullable final char[] password)
568572
public static void validateBeforeUse(final MongoCredential credential) {
569573
String userName = credential.getUserName();
570574
Object providerName = credential.getMechanismProperty(PROVIDER_NAME_KEY, null);
571-
Object requestCallback = credential.getMechanismProperty(OIDC_CALLBACK_KEY, null);
572-
Object refreshCallback = credential.getMechanismProperty(OIDC_HUMAN_CALLBACK_KEY, null);
575+
Object machineCallback = credential.getMechanismProperty(OIDC_CALLBACK_KEY, null);
576+
Object humanCallback = credential.getMechanismProperty(OIDC_HUMAN_CALLBACK_KEY, null);
573577
if (providerName == null) {
574578
// callback
575-
if (requestCallback == null && refreshCallback == null) {
579+
if (machineCallback == null && humanCallback == null) {
576580
throw new IllegalArgumentException("Either " + PROVIDER_NAME_KEY
577581
+ " or " + OIDC_CALLBACK_KEY
578582
+ " or " + OIDC_HUMAN_CALLBACK_KEY
579583
+ " must be specified");
580584
}
581-
if (requestCallback != null && refreshCallback != null) {
585+
if (machineCallback != null && humanCallback != null) {
582586
throw new IllegalArgumentException("Both " + OIDC_CALLBACK_KEY
583587
+ " and " + OIDC_HUMAN_CALLBACK_KEY
584588
+ " must not be specified");
585589
}
586590
} else {
587-
// automatic
588591
if (userName != null) {
589592
throw new IllegalArgumentException("user name must not be specified when " + PROVIDER_NAME_KEY + " is specified");
590593
}
591-
if (requestCallback != null) {
594+
if (machineCallback != null) {
592595
throw new IllegalArgumentException(OIDC_CALLBACK_KEY + " must not be specified when " + PROVIDER_NAME_KEY + " is specified");
593596
}
594-
if (refreshCallback != null) {
597+
if (humanCallback != null) {
595598
throw new IllegalArgumentException(OIDC_HUMAN_CALLBACK_KEY + " must not be specified when " + PROVIDER_NAME_KEY + " is specified");
596599
}
597600
}

‎driver-sync/src/test/functional/com/mongodb/client/unified/Entities.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ private void initClient(final BsonDocument entity, final String id,
530530
} catch (IOException e) {
531531
throw new RuntimeException(e);
532532
}
533-
return new MongoCredential.OidcCallbackResult(accessToken);
533+
return new MongoCredential.OidcCallbackResult(accessToken, null);
534534
}));
535535
break;
536536
}

‎driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedAuthTest.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,12 @@
2424
import java.net.URISyntaxException;
2525
import java.util.Collection;
2626

27-
import static com.mongodb.ClusterFixture.isServerlessTest;
28-
import static org.junit.Assume.assumeFalse;
29-
3027
public class UnifiedAuthTest extends UnifiedSyncTest {
3128
public UnifiedAuthTest(@SuppressWarnings("unused") final String fileDescription,
3229
@SuppressWarnings("unused") final String testDescription,
3330
final String schemaVersion, final BsonArray runOnRequirements, final BsonArray entitiesArray,
3431
final BsonArray initialData, final BsonDocument definition) {
3532
super(schemaVersion, runOnRequirements, entitiesArray, initialData, definition);
36-
assumeFalse(isServerlessTest());
3733
}
3834

3935
@Parameterized.Parameters(name = "{0}: {1}")

‎driver-sync/src/test/functional/com/mongodb/internal/connection/OidcAuthenticationProseTests.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ public void test2p3CallbackReturnsMissingData() {
201201
// conforming to the OIDCRequestTokenResult with missing field(s).
202202
OidcCallback onRequest = (context) -> {
203203
//noinspection ConstantConditions
204-
return new OidcCallbackResult(null);
204+
return new OidcCallbackResult(null, Duration.ZERO);
205205
};
206206
// we ensure that the error is propagated
207207
MongoClientSettings clientSettings = createSettings(getOidcUri(), onRequest, null);
@@ -268,7 +268,7 @@ public void test3p1AuthFailsWithCachedToken() throws ExecutionException, Interru
268268
@Test
269269
public void test3p2AuthFailsWithoutCachedToken() {
270270
MongoClientSettings clientSettings = createSettings(getOidcUri(),
271-
(x) -> new OidcCallbackResult("invalid_token"), null);
271+
(x) -> new OidcCallbackResult("invalid_token", Duration.ZERO), null);
272272
try (MongoClient mongoClient = createMongoClient(clientSettings)) {
273273
try {
274274
performFind(mongoClient);
@@ -358,7 +358,7 @@ public void testh1p6AllowedHostsBlocked() {
358358
MongoClientSettings settings1 = createSettings(
359359
getOidcUri(),
360360
createHumanCallback(), null, OIDC_HUMAN_CALLBACK_KEY, Collections.emptyList());
361-
performFind(settings1, MongoSecurityException.class, "Host not permitted by ALLOWED_HOSTS");
361+
performFind(settings1, MongoSecurityException.class, "not permitted by ALLOWED_HOSTS");
362362

363363
//- Create a client that uses the URL
364364
// ``mongodb://localhost/?authMechanism=MONGODB-OIDC&ignored=example.com``, a
@@ -367,7 +367,16 @@ public void testh1p6AllowedHostsBlocked() {
367367
MongoClientSettings settings2 = createSettings(
368368
getOidcUri() + "&ignored=example.com",
369369
createHumanCallback(), null, OIDC_HUMAN_CALLBACK_KEY, Arrays.asList("example.com"));
370-
performFind(settings2, MongoSecurityException.class, "Host not permitted by ALLOWED_HOSTS");
370+
performFind(settings2, MongoSecurityException.class, "not permitted by ALLOWED_HOSTS");
371+
}
372+
373+
// Not a prose test
374+
@Test
375+
public void testAllowedHostsDisallowedInConnectionString() {
376+
String string = "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:localhost";
377+
assertCause(IllegalArgumentException.class,
378+
"connection string contains disallowed mechanism properties",
379+
() -> new ConnectionString(string));
371380
}
372381

373382
@Test
@@ -397,13 +406,13 @@ public void testh2p2HumanCallbackReturnsMissingData() {
397406
"Result of callback must not be null");
398407

399408
//noinspection ConstantConditions
400-
OidcCallback onRequest = (context) -> new OidcCallbackResult(null);
409+
OidcCallback onRequest = (context) -> new OidcCallbackResult(null, Duration.ZERO);
401410
performFind(createHumanSettings(getOidcUri(), onRequest, null),
402411
IllegalArgumentException.class,
403412
"accessToken can not be null");
404413

405414
// additionally, check validation for refresh in machine workflow:
406-
OidcCallback onRequestMachineRefresh = (context) -> new OidcCallbackResult("access", "exists");
415+
OidcCallback onRequestMachineRefresh = (context) -> new OidcCallbackResult("access", Duration.ZERO, "exists");
407416
performFind(createSettings(getOidcUri(), onRequestMachineRefresh, null),
408417
MongoConfigurationException.class,
409418
"Refresh token must only be provided in human workflow");
@@ -789,7 +798,7 @@ private OidcCallbackResult callback() {
789798
if (testListener != null) {
790799
testListener.add("read access token: " + path.getFileName());
791800
}
792-
return new OidcCallbackResult(accessToken, refreshToken);
801+
return new OidcCallbackResult(accessToken, Duration.ZERO, refreshToken);
793802
} finally {
794803
if (concurrentTracker != null) {
795804
concurrentTracker.decrementAndGet();

0 commit comments

Comments
 (0)