Skip to content

Commit f7747f2

Browse files
mohitc1mohitc1
authored
Improve tenant-based flights supporting code, Fixes AB#3321153 (#2717)
Fixes [AB#3321153](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3321153) - Refactored getting open-id configuration for AAD authority between classes (AzureActivityDirectory.java, AzureActiveDirectoryAudience and OpenIdProviderConfigurationClient) - Updated common flights manager based on changes in AzureAD/ad-accounts-for-android#3171 (Refer this PR for more details)
1 parent eb51187 commit f7747f2

File tree

9 files changed

+105
-62
lines changed

9 files changed

+105
-62
lines changed

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [Minor] Improve tenant-based flighting for broker supporting code (#2717)
34
- [PATCH] Fix caching of secret key and add retries for InvalidKeyException during unwrap (#2659)
45
- [MINOR] Replace AbstractSecretKeyLoader with ISecretKeyProvider (#2666)
56
- [MINOR] Update IP phone app teams signature constants to use SHA-512 format (#2700)

‎common/src/test/java/com/microsoft/identity/common/internal/mocks/MockCommonFlightsManager.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
// THE SOFTWARE.
2323
package com.microsoft.identity.common.internal.mocks;
2424

25+
import androidx.annotation.NonNull;
26+
2527
import com.microsoft.identity.common.java.flighting.IFlightsManager;
2628
import com.microsoft.identity.common.java.flighting.IFlightsProvider;
2729

@@ -46,4 +48,16 @@ public IFlightsProvider getFlightsProvider() {
4648
public IFlightsProvider getFlightsProviderForTenant(@NotNull String tenantId) {
4749
return mMockCommonFlightsProvider;
4850
}
51+
52+
@NonNull
53+
@Override
54+
public IFlightsProvider getFlightsProvider(long waitForConfigsWithTimeoutInMs) {
55+
return this.getFlightsProvider();
56+
}
57+
58+
@NonNull
59+
@Override
60+
public IFlightsProvider getFlightsProviderForTenant(@NonNull String tenantId, long waitForConfigsWithTimeoutInMs) {
61+
return this.getFlightsProviderForTenant(tenantId);
62+
}
4963
}

‎common/src/test/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClientTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import static org.junit.Assert.assertFalse;
2929
import static org.junit.Assert.assertTrue;
3030
import static org.mockito.ArgumentMatchers.any;
31+
import static org.mockito.ArgumentMatchers.anyLong;
3132
import static org.mockito.ArgumentMatchers.eq;
3233
import static org.mockito.Mockito.never;
3334
import static org.mockito.Mockito.when;
@@ -430,7 +431,7 @@ public void testOnReceivedSslError() {
430431
final IFlightsManager mockFlightsManager = Mockito.mock(IFlightsManager.class);
431432
final IFlightsProvider mockFlightsProvider = Mockito.mock(IFlightsProvider.class);
432433
when(mockFlightsProvider.isFlightEnabled(eq(CommonFlight.SHOULD_PRESERVE_WEBVIEW_FLOW_ON_SSL_ERROR))).thenReturn(true);
433-
when(mockFlightsManager.getFlightsProvider()).thenReturn(mockFlightsProvider);
434+
when(mockFlightsManager.getFlightsProvider(anyLong())).thenReturn(mockFlightsProvider);
434435
CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(mockFlightsManager);
435436
final SslErrorHandler mockHandler = Mockito.mock(android.webkit.SslErrorHandler.class);
436437
final SslError mockError = Mockito.mock(android.net.http.SslError.class);

‎common4j/src/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAudience.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,15 @@ public String getTenantUuidForAlias(@NonNull final String authority)
8787
final OpenIdProviderConfiguration providerConfiguration =
8888
loadOpenIdProviderConfigurationMetadata(authority);
8989

90-
final String issuer = providerConfiguration.getIssuer();
90+
return getTenantIdFromOpenIdProviderConfiguration(providerConfiguration);
91+
}
92+
93+
/**
94+
* Util method to extract tenant UUID from the OpenIdProviderConfiguration's Issuer. It's expected
95+
* configuration is of AAD authority, so that tenant UUID is present in the path of the issuer.
96+
*/
97+
public static String getTenantIdFromOpenIdProviderConfiguration(@NonNull final OpenIdProviderConfiguration configuration) throws ClientException {
98+
final String issuer = configuration.getIssuer();
9199
final CommonURIBuilder issuerUri;
92100
try {
93101
issuerUri = new CommonURIBuilder(issuer);
@@ -108,9 +116,9 @@ public String getTenantUuidForAlias(@NonNull final String authority)
108116

109117
throw new ClientException(errMsg);
110118
}
111-
final String tenantUUID = paths.get(0);
119+
final String tenantUuid = paths.get(0);
112120

113-
if (!StringUtil.isUuid(tenantUUID)) {
121+
if (!StringUtil.isUuid(tenantUuid)) {
114122
final String errMsg = "OpenId Metadata did not contain UUID in the path ";
115123
Logger.error(
116124
TAG,
@@ -120,7 +128,7 @@ public String getTenantUuidForAlias(@NonNull final String authority)
120128

121129
throw new ClientException(errMsg);
122130
}
123-
return tenantUUID;
131+
return tenantUuid;
124132
}
125133

126134
/**

‎common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlightsManager.kt‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ object CommonFlightsManager : IFlightsManager {
5050
mFlightsManager = DefaultValueFlightsManager
5151
}
5252

53-
override fun getFlightsProvider(): IFlightsProvider {
54-
return mFlightsManager.getFlightsProvider()
53+
override fun getFlightsProvider(waitForConfigsWithTimeoutInMs: Long): IFlightsProvider {
54+
return mFlightsManager.getFlightsProvider(waitForConfigsWithTimeoutInMs)
5555
}
5656

57-
override fun getFlightsProviderForTenant(tenantId: String): IFlightsProvider {
58-
return mFlightsManager.getFlightsProviderForTenant(tenantId)
57+
override fun getFlightsProviderForTenant(tenantId: String, waitForConfigsWithTimeoutInMs: Long): IFlightsProvider {
58+
return mFlightsManager.getFlightsProviderForTenant(tenantId, waitForConfigsWithTimeoutInMs)
5959
}
6060

6161
/**
@@ -97,11 +97,14 @@ object CommonFlightsManager : IFlightsManager {
9797
* default value for the flight config provided.
9898
*/
9999
private object DefaultValueFlightsManager : IFlightsManager {
100-
override fun getFlightsProvider(): IFlightsProvider {
100+
override fun getFlightsProvider(waitForConfigsWithTimeoutInMs: Long): IFlightsProvider {
101101
return DefaultValueFlightsProvider
102102
}
103103

104-
override fun getFlightsProviderForTenant(tenantId: String): IFlightsProvider {
104+
override fun getFlightsProviderForTenant(
105+
tenantId: String,
106+
waitForConfigsWithTimeoutInMs: Long
107+
): IFlightsProvider {
105108
return DefaultValueFlightsProvider
106109
}
107110
}

‎common4j/src/main/com/microsoft/identity/common/java/flighting/IFlightsManager.kt‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,28 @@ interface IFlightsManager {
3131
/**
3232
* Flights provider applicable by default. Features should always this
3333
* unless the feature behaviour is tenant specific one same device.
34+
* Calls [getFlightsProvider] with a default timeout of 0 milliseconds. 0 indicates no wait.
3435
*/
35-
fun getFlightsProvider(): IFlightsProvider
36+
fun getFlightsProvider(): IFlightsProvider = getFlightsProvider(0)
37+
38+
/**
39+
* Flights provider applicable by default. Features should always use this
40+
* unless the feature behaviour is tenant specific one same device.
41+
* @param waitForConfigsWithTimeoutInMs The timeout in milliseconds to wait for initial configurations from source.
42+
*/
43+
fun getFlightsProvider(waitForConfigsWithTimeoutInMs: Long = 0): IFlightsProvider
44+
45+
/**
46+
* Flights provider for the given tenant
47+
* @param tenantId The tenant ID for which the flights provider is requested.
48+
* Calls [getFlightsProviderForTenant] with a default timeout of 0 milliseconds. 0 indicates no wait.
49+
*/
50+
fun getFlightsProviderForTenant(tenantId: String): IFlightsProvider = getFlightsProviderForTenant(tenantId, 0)
3651

3752
/**
3853
* Flights provider for the given tenant
54+
* @param tenantId The tenant ID for which the flights provider is requested.
55+
* @param waitForConfigsWithTimeoutInMs The timeout in milliseconds to wait for initial configurations from source.
3956
*/
40-
fun getFlightsProviderForTenant(tenantId: String): IFlightsProvider
57+
fun getFlightsProviderForTenant(tenantId: String, waitForConfigsWithTimeoutInMs: Long = 0): IFlightsProvider
4158
}

‎common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/AzureActiveDirectory.java‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,31 +22,32 @@
2222
// THE SOFTWARE.
2323
package com.microsoft.identity.common.java.providers.microsoft.azureactivedirectory;
2424

25-
import lombok.NonNull;
25+
import static com.microsoft.identity.common.java.exception.ServiceException.OPENID_PROVIDER_CONFIGURATION_FAILED_TO_LOAD;
2626

2727
import com.google.gson.Gson;
2828
import com.google.gson.reflect.TypeToken;
2929
import com.microsoft.identity.common.java.authorities.Environment;
3030
import com.microsoft.identity.common.java.cache.HttpCache;
31+
import com.microsoft.identity.common.java.exception.ClientException;
32+
import com.microsoft.identity.common.java.exception.ServiceException;
3133
import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
3234
import com.microsoft.identity.common.java.logging.Logger;
33-
import com.microsoft.identity.common.java.exception.ClientException;
3435
import com.microsoft.identity.common.java.net.HttpClient;
3536
import com.microsoft.identity.common.java.net.HttpResponse;
3637
import com.microsoft.identity.common.java.net.UrlConnectionHttpClient;
3738
import com.microsoft.identity.common.java.providers.IdentityProvider;
3839
import com.microsoft.identity.common.java.providers.oauth2.OAuth2StrategyParameters;
40+
import com.microsoft.identity.common.java.providers.oauth2.OpenIdProviderConfiguration;
41+
import com.microsoft.identity.common.java.providers.oauth2.OpenIdProviderConfigurationClient;
42+
import com.microsoft.identity.common.java.util.CommonURIBuilder;
3943
import com.microsoft.identity.common.java.util.ObjectMapper;
4044
import com.microsoft.identity.common.java.util.StringUtil;
41-
import com.microsoft.identity.common.java.util.CommonURIBuilder;
4245
import com.microsoft.identity.common.java.util.UrlUtil;
4346

4447
import org.json.JSONException;
4548

46-
import java.io.IOException;
4749
import java.lang.reflect.Type;
4850
import java.net.HttpURLConnection;
49-
import java.net.MalformedURLException;
5051
import java.net.URI;
5152
import java.net.URISyntaxException;
5253
import java.net.URL;
@@ -59,6 +60,8 @@
5960
import java.util.concurrent.ConcurrentHashMap;
6061
import java.util.concurrent.ConcurrentMap;
6162

63+
import lombok.NonNull;
64+
6265
/**
6366
* Implements the IdentityProvider base class...
6467
*/
@@ -246,6 +249,34 @@ public static synchronized List<AzureActiveDirectoryCloud> getClouds() {
246249
return new ArrayList<>();
247250
}
248251

252+
/**
253+
* Loads the OpenID Provider Configuration metadata for the specified tenant by first constructing
254+
* AAD authority URL like https://login.microsoftonline.com/{tenant}/v2.0/.well-known/openid-configuration
255+
*
256+
* @param tenant The tenant identifier. Could be tenant id (guid) or tenant name (contoso.onmicrosoft.com).
257+
* @return The OpenID Provider Configuration for the specified tenant.
258+
* @throws ServiceException If there is an error loading the configuration.
259+
* @throws ClientException If there is a client-side error.
260+
*/
261+
public static OpenIdProviderConfiguration loadOpenIdProviderConfigurationMetadataForTenant(
262+
@NonNull final String tenant
263+
) throws ServiceException, ClientException {
264+
final OpenIdProviderConfigurationClient client =
265+
new OpenIdProviderConfigurationClient();
266+
try {
267+
final String tenantedAuthorityUrl = new CommonURIBuilder(getDefaultCloudUrl())
268+
.setPathSegments(tenant)
269+
.build()
270+
.toString();
271+
return client.loadOpenIdProviderConfigurationFromAuthority(tenantedAuthorityUrl);
272+
} catch (final URISyntaxException e) {
273+
throw new ClientException(
274+
OPENID_PROVIDER_CONFIGURATION_FAILED_TO_LOAD,
275+
"URISyntaxException while requesting metadata",
276+
e
277+
);
278+
}
279+
}
249280
/**
250281
* Deserializes the supplied JSONArray of cloud instances into a native List.
251282
*

‎common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OpenIdProviderConfigurationClient.java‎

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,10 @@
5353
public class OpenIdProviderConfigurationClient {
5454

5555
private static final String TAG = OpenIdProviderConfigurationClient.class.getSimpleName();
56-
private static final String HTTPS_SCHEME = "https";
57-
private static final String WELL_KNOWN_CONFIG_HOST = "login.microsoftonline.com";
5856
private static final String WELL_KNOWN_CONFIG_PATH = "/v2.0/.well-known/openid-configuration";
59-
private static final ExecutorService sBackgroundExecutor = Executors.newCachedThreadPool();
6057
private static final Map<URI, OpenIdProviderConfiguration> sConfigCache = new HashMap<>();
6158
private static final HttpClient httpClient = UrlConnectionHttpClient.getDefaultInstance();
6259

63-
public interface OpenIdProviderConfigurationCallback
64-
extends TaskCompletedCallbackWithError<OpenIdProviderConfiguration, Exception> {
65-
}
66-
6760
private static final Gson GSON = new Gson();
6861

6962
private String sanitize(@NonNull final String issuer) {
@@ -76,43 +69,6 @@ private String sanitize(@NonNull final String issuer) {
7669
return sanitizedIssuer;
7770
}
7871

79-
public void loadOpenIdProviderConfiguration(
80-
@NonNull final OpenIdProviderConfigurationCallback callback, @NonNull final String authority) {
81-
sBackgroundExecutor.submit(new Runnable() {
82-
@Override
83-
public void run() {
84-
try {
85-
callback.onTaskCompleted(loadOpenIdProviderConfigurationFromAuthority(authority));
86-
} catch (ServiceException e) {
87-
callback.onError(e);
88-
}
89-
}
90-
});
91-
}
92-
93-
/**
94-
* Get OpenID provider configuration.
95-
*
96-
* @return OpenIdProviderConfiguration
97-
*/
98-
public synchronized OpenIdProviderConfiguration loadOpenIdProviderConfigurationFromTenant(@NonNull final String tenantIdentifier)
99-
throws ServiceException {
100-
try {
101-
final String tenantedAuthorityUrl = new CommonURIBuilder()
102-
.setScheme(HTTPS_SCHEME)
103-
.setHost(WELL_KNOWN_CONFIG_HOST)
104-
.setPathSegments(tenantIdentifier)
105-
.build().toString();
106-
return loadOpenIdProviderConfigurationInternal(tenantedAuthorityUrl, null);
107-
} catch (final URISyntaxException e) {
108-
throw new ServiceException(
109-
OPENID_PROVIDER_CONFIGURATION_FAILED_TO_LOAD,
110-
"IOException while requesting metadata",
111-
e
112-
);
113-
}
114-
}
115-
11672
/**
11773
* Get OpenID provider configuration.
11874
*

‎common4j/src/test/com/microsoft/identity/common/java/flighting/MockFlightsManager.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@ public IFlightsProvider getFlightsProvider() {
4141
return mMockBrokerFlightsProvider;
4242
}
4343

44+
@NotNull
45+
@Override
46+
public IFlightsProvider getFlightsProviderForTenant(@NotNull String tenantId, long waitForConfigsWithTimeoutInMs) {
47+
return this.getFlightsProviderForTenant(tenantId);
48+
}
49+
50+
@NotNull
51+
@Override
52+
public IFlightsProvider getFlightsProvider(long waitForConfigsWithTimeoutInMs) {
53+
return this.getFlightsProvider();
54+
}
55+
4456
@NotNull
4557
@Override
4658
public IFlightsProvider getFlightsProviderForTenant(@NotNull String tenantId) {

0 commit comments

Comments
 (0)