Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

Removed references to TrustServiceUrl #1126

Merged
merged 1 commit into from
Apr 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -379,21 +379,11 @@ public CompletableFuture<Void> continueConversation(
context.getTurnState().add(BOT_IDENTITY_KEY, claimsIdentity);
context.getTurnState().add(OAUTH_SCOPE_KEY, audience);

String appIdFromClaims = JwtTokenValidation.getAppIdFromClaims(claimsIdentity.claims());
return credentialProvider.isValidAppId(appIdFromClaims).thenCompose(isValidAppId -> {
// If we receive a valid app id in the incoming token claims, add the
// channel service URL to the trusted services list so we can send messages
// back.
if (!StringUtils.isEmpty(appIdFromClaims) && isValidAppId) {
AppCredentials.trustServiceUrl(reference.getServiceUrl());
}

return createConnectorClient(reference.getServiceUrl(), claimsIdentity, audience)
.thenCompose(connectorClient -> {
context.getTurnState().add(CONNECTOR_CLIENT_KEY, connectorClient);
return runPipeline(context, callback);
});
});
return createConnectorClient(reference.getServiceUrl(), claimsIdentity, audience)
.thenCompose(connectorClient -> {
context.getTurnState().add(CONNECTOR_CLIENT_KEY, connectorClient);
return runPipeline(context, callback);
});
} catch (Exception e) {
pipelineResult.completeExceptionally(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,10 @@
import com.microsoft.bot.restclient.credentials.ServiceClientCredentials;
import okhttp3.OkHttpClient;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.LoggerFactory;

import java.net.MalformedURLException;
import java.net.URL;
import java.time.LocalDateTime;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

/**
* Base abstraction for AAD credentials for auth and caching.
Expand All @@ -24,16 +20,6 @@
* </p>
*/
public abstract class AppCredentials implements ServiceClientCredentials {
private static final int EXPIRATION_SLACK = 5;
private static final int EXPIRATION_DAYS = 1;
private static ConcurrentMap<String, LocalDateTime> trustHostNames = new ConcurrentHashMap<>();

static {
trustHostNames.put("api.botframework.com", LocalDateTime.MAX);
trustHostNames.put("token.botframework.com", LocalDateTime.MAX);
trustHostNames.put("api.botframework.azure.us", LocalDateTime.MAX);
trustHostNames.put("token.botframework.azure.us", LocalDateTime.MAX);
}

private String appId;
private String authTenant;
Expand Down Expand Up @@ -62,73 +48,6 @@ public AppCredentials(String withChannelAuthTenant, String withOAuthScope) {
: withOAuthScope;
}

/**
* Adds the host of service url to trusted hosts.
*
* @param serviceUrl The service URI.
*/
public static void trustServiceUrl(String serviceUrl) {
trustServiceUrl(serviceUrl, LocalDateTime.now().plusDays(EXPIRATION_DAYS));
}

/**
* Adds the host of service url to trusted hosts with the specified expiration.
*
* <p>
* Note: The will fail to add if the url is not valid.
* </p>
*
* @param serviceUrl The service URI.
* @param expirationTime The expiration time after which this service url is not
* trusted anymore.
*/
public static void trustServiceUrl(String serviceUrl, LocalDateTime expirationTime) {
try {
URL url = new URL(serviceUrl);
trustServiceUrl(url, expirationTime);
} catch (MalformedURLException e) {
LoggerFactory.getLogger(MicrosoftAppCredentials.class).error("trustServiceUrl", e);
}
}

/**
* Adds the host of service url to trusted hosts with the specified expiration.
*
* @param serviceUrl The service URI.
* @param expirationTime The expiration time after which this service url is not
* trusted anymore.
*/
public static void trustServiceUrl(URL serviceUrl, LocalDateTime expirationTime) {
trustHostNames.put(serviceUrl.getHost(), expirationTime);
}

/**
* Checks if the service url is for a trusted host or not.
*
* @param serviceUrl The service URI.
* @return true if the service is trusted.
*/
public static boolean isTrustedServiceUrl(String serviceUrl) {
try {
URL url = new URL(serviceUrl);
return isTrustedServiceUrl(url);
} catch (MalformedURLException e) {
LoggerFactory.getLogger(AppCredentials.class).error("trustServiceUrl", e);
return false;
}
}

/**
* Checks if the service url is for a trusted host or not.
*
* @param serviceUrl The service URI.
* @return true if the service is trusted.
*/
public static boolean isTrustedServiceUrl(URL serviceUrl) {
return !trustHostNames.getOrDefault(serviceUrl.getHost(), LocalDateTime.MIN)
.isBefore(LocalDateTime.now().minusMinutes(EXPIRATION_SLACK));
}

/**
* Gets the App ID for this credential.
*
Expand Down Expand Up @@ -245,7 +164,7 @@ boolean shouldSetToken(String url) {
if (StringUtils.isBlank(getAppId()) || getAppId().equals(AuthenticationConstants.ANONYMOUS_SKILL_APPID)) {
return false;
}
return isTrustedServiceUrl(url);
return true;
}

// lazy Authenticator create.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,7 @@ public static CompletableFuture<ClaimsIdentity> authenticateRequest(
return JwtTokenValidation.validateAuthHeader(
authHeader, credentials, channelProvider, activity.getChannelId(),
activity.getServiceUrl(), authConfig
)

.thenApply(identity -> {
// On the standard Auth path, we need to trust the URL that was incoming.
MicrosoftAppCredentials.trustServiceUrl(activity.getServiceUrl());
return identity;
});
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,24 +155,6 @@ public void Emulator_AuthHeader_CorrectAppIdAndServiceUrl_WithPrivateChannelServ
"TheChannel");
}

/**
* Tests with a valid Token and service url; and ensures that Service url is added to Trusted service url list.
*/
@Test
public void ChannelMsaHeaderValidServiceUrlShouldBeTrusted() throws IOException, ExecutionException, InterruptedException {
String header = getHeaderToken();
CredentialProvider credentials = new SimpleCredentialProvider(APPID, "");
Activity activity = new Activity(ActivityTypes.MESSAGE);
activity.setServiceUrl("https://smba.trafficmanager.net/amer-client-ss.msg/");
JwtTokenValidation.authenticateRequest(
activity,
header,
credentials,
new SimpleChannelProvider()).join();

Assert.assertTrue(MicrosoftAppCredentials.isTrustedServiceUrl("https://smba.trafficmanager.net/amer-client-ss.msg/"));
}

/**
* Tests with a valid Token and invalid service url; and ensures that Service url is NOT added to Trusted service url list.
*/
Expand All @@ -192,7 +174,6 @@ public void ChannelMsaHeaderInvalidServiceUrlShouldNotBeTrusted() throws IOExcep
Assert.fail("Should have thrown AuthenticationException");
} catch (CompletionException e) {
Assert.assertTrue(e.getCause() instanceof AuthenticationException);
Assert.assertFalse(MicrosoftAppCredentials.isTrustedServiceUrl("https://webchat.botframework.com/"));
}
}

Expand Down Expand Up @@ -255,26 +236,6 @@ public void ChannelNoHeaderAuthenticationEnabledShouldThrow() throws IOException
} catch (CompletionException e) {
Assert.assertTrue(e.getCause() instanceof AuthenticationException);
}

Assert.assertFalse(MicrosoftAppCredentials.isTrustedServiceUrl("https://smba.trafficmanager.net/amer-client-ss.msg/"));
}

/**
* Tests with no authentication header and makes sure the service URL is not added to the trusted list.
*/
@Test
public void ChannelAuthenticationDisabledServiceUrlShouldNotBeTrusted() throws ExecutionException, InterruptedException {
String header = "";
CredentialProvider credentials = new SimpleCredentialProvider("", "");

Activity activity = new Activity(ActivityTypes.MESSAGE);
activity.setServiceUrl("https://webchat.botframework.com/");
ClaimsIdentity identity = JwtTokenValidation.authenticateRequest(
activity,
header,
credentials,
new SimpleChannelProvider()).join();
Assert.assertFalse(MicrosoftAppCredentials.isTrustedServiceUrl("https://webchat.botframework.com/"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,6 @@
import java.time.LocalDateTime;

public class MicrosoftAppCredentialsTests {
@Test
public void ValidUrlTrusted() {
MicrosoftAppCredentials.trustServiceUrl("https://goodurl.com");
Assert.assertTrue(MicrosoftAppCredentials.isTrustedServiceUrl("https://goodurl.com"));
}

@Test
public void InvalidUrlTrusted() {
MicrosoftAppCredentials.trustServiceUrl("badurl");
Assert.assertFalse(MicrosoftAppCredentials.isTrustedServiceUrl("badurl"));
}

@Test
public void TrustedUrlExpiration() throws InterruptedException {
// There is a +5 minute window for an expired url
MicrosoftAppCredentials.trustServiceUrl("https://goodurl.com", LocalDateTime.now().minusMinutes(6));
Assert.assertFalse(MicrosoftAppCredentials.isTrustedServiceUrl("https://goodurl.com"));

MicrosoftAppCredentials.trustServiceUrl("https://goodurl.com", LocalDateTime.now().minusMinutes(4));
Assert.assertTrue(MicrosoftAppCredentials.isTrustedServiceUrl("https://goodurl.com"));
}

@Test
public void ValidateAuthEndpoint() {
Expand Down