From 466fd90b8078db71bb6bb873fb763b1f83818bf3 Mon Sep 17 00:00:00 2001 From: Lee Parrish <30470292+LeeParrishMSFT@users.noreply.github.com> Date: Wed, 17 Mar 2021 10:41:03 -0500 Subject: [PATCH] Fixes for Issue 809 (#1060) --- .../bot/builder/ChannelServiceHandler.java | 6 +- .../builder/ChannelServiceHandlerTests.java | 73 ++++++++++++++ .../authentication/AppCredentials.java | 3 + .../authentication/JwtTokenValidation.java | 31 ++++-- .../bot/connector/AppCredentialsTests.java | 95 +++++++++++++++++++ .../connector/JwtTokenValidationTests.java | 27 ++++++ 6 files changed, 224 insertions(+), 11 deletions(-) create mode 100644 libraries/bot-builder/src/test/java/com/microsoft/bot/builder/ChannelServiceHandlerTests.java create mode 100644 libraries/bot-connector/src/test/java/com/microsoft/bot/connector/AppCredentialsTests.java diff --git a/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/ChannelServiceHandler.java b/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/ChannelServiceHandler.java index cb3e74dd..14a3cde5 100644 --- a/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/ChannelServiceHandler.java +++ b/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/ChannelServiceHandler.java @@ -48,11 +48,11 @@ public class ChannelServiceHandler { ChannelProvider channelProvider) { if (credentialProvider == null) { - throw new IllegalArgumentException("credentialprovider cannot be nul"); + throw new IllegalArgumentException("credentialprovider cannot be null"); } if (authConfiguration == null) { - throw new IllegalArgumentException("authConfiguration cannot be nul"); + throw new IllegalArgumentException("authConfiguration cannot be null"); } this.credentialProvider = credentialProvider; @@ -603,7 +603,7 @@ public class ChannelServiceHandler { return credentialProvider.isAuthenticationDisabled().thenCompose(isAuthDisabled -> { if (!isAuthDisabled) { return Async.completeExceptionally( - // No auth header. Auth is required. Request is not authorized. + // No auth header. Auth is required. Request is not authorized. new AuthenticationException("No auth header, Auth is required. Request is not authorized") ); } diff --git a/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/ChannelServiceHandlerTests.java b/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/ChannelServiceHandlerTests.java new file mode 100644 index 00000000..77f201fe --- /dev/null +++ b/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/ChannelServiceHandlerTests.java @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MT License. + +package com.microsoft.bot.builder; + +import java.util.concurrent.CompletableFuture; + +import com.microsoft.bot.connector.authentication.AuthenticationConfiguration; +import com.microsoft.bot.connector.authentication.AuthenticationConstants; +import com.microsoft.bot.connector.authentication.ClaimsIdentity; +import com.microsoft.bot.connector.authentication.JwtTokenValidation; +import com.microsoft.bot.connector.authentication.SimpleCredentialProvider; +import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ActivityTypes; +import com.microsoft.bot.schema.ResourceResponse; + +import org.junit.Assert; +import org.junit.Test; + +public class ChannelServiceHandlerTests { + + @Test + public void AuthenticateSetsAnonymousSkillClaim() { + TestChannelServiceHandler sut = new TestChannelServiceHandler(); + sut.handleReplyToActivity(null, "123", "456", new Activity(ActivityTypes.MESSAGE)); + + Assert.assertEquals(AuthenticationConstants.ANONYMOUS_AUTH_TYPE, + sut.getClaimsIdentity().getType()); + Assert.assertEquals(AuthenticationConstants.ANONYMOUS_SKILL_APPID, + JwtTokenValidation.getAppIdFromClaims(sut.getClaimsIdentity().claims())); + } + + /** + * A {@link ChannelServiceHandler} with overrides for testings. + */ + private class TestChannelServiceHandler extends ChannelServiceHandler { + TestChannelServiceHandler() { + super(new SimpleCredentialProvider(), new AuthenticationConfiguration(), null); + } + + private ClaimsIdentity claimsIdentity; + + @Override + protected CompletableFuture onReplyToActivity( + ClaimsIdentity claimsIdentity, + String conversationId, + String activityId, + Activity activity + ) { + this.claimsIdentity = claimsIdentity; + return CompletableFuture.completedFuture(new ResourceResponse()); + } + /** + * Gets the {@link ClaimsIdentity} sent to the different methods after + * auth is done. + * @return the ClaimsIdentity value as a getClaimsIdentity(). + */ + public ClaimsIdentity getClaimsIdentity() { + return this.claimsIdentity; + } + + /** + * Gets the {@link ClaimsIdentity} sent to the different methods after + * auth is done. + * @param withClaimsIdentity The ClaimsIdentity value. + */ + private void setClaimsIdentity(ClaimsIdentity withClaimsIdentity) { + this.claimsIdentity = withClaimsIdentity; + } + + } +} + diff --git a/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/AppCredentials.java b/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/AppCredentials.java index 000f3db4..497d9412 100644 --- a/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/AppCredentials.java +++ b/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/AppCredentials.java @@ -242,6 +242,9 @@ public abstract class AppCredentials implements ServiceClientCredentials { * @return true if the auth token should be added to the request. */ boolean shouldSetToken(String url) { + if (StringUtils.isBlank(getAppId()) || getAppId().equals(AuthenticationConstants.ANONYMOUS_SKILL_APPID)) { + return false; + } return isTrustedServiceUrl(url); } diff --git a/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/JwtTokenValidation.java b/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/JwtTokenValidation.java index f81a70ca..286eab79 100644 --- a/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/JwtTokenValidation.java +++ b/libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/JwtTokenValidation.java @@ -4,7 +4,10 @@ package com.microsoft.bot.connector.authentication; import com.microsoft.bot.connector.Async; +import com.microsoft.bot.connector.Channels; import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.RoleTypes; + import java.util.Map; import org.apache.commons.lang3.StringUtils; @@ -61,19 +64,31 @@ public final class JwtTokenValidation { ChannelProvider channelProvider, AuthenticationConfiguration authConfig ) { + if (authConfig == null) { + return Async.completeExceptionally( + new IllegalArgumentException("authConfig cannot be null") + ); + } - if (StringUtils.isEmpty(authHeader)) { + if (StringUtils.isBlank(authHeader)) { // No auth header was sent. We might be on the anonymous code path. return credentials.isAuthenticationDisabled().thenApply(isAuthDisable -> { - if (isAuthDisable) { - // In the scenario where Auth is disabled, we still want to have the - // IsAuthenticated flag set in the ClaimsIdentity. To do this requires - // adding in an empty claim. - return new ClaimsIdentity("anonymous"); + if (!isAuthDisable) { + // No Auth Header. Auth is required. Request is not authorized. + throw new AuthenticationException("No Auth Header. Auth is required."); } - // No Auth Header. Auth is required. Request is not authorized. - throw new AuthenticationException("No Auth Header. Auth is required."); + if (activity.getChannelId() != null + && activity.getChannelId().equals(Channels.EMULATOR) + && activity.getRecipient() != null + && activity.getRecipient().getRole().equals(RoleTypes.SKILL)) { + return SkillValidation.createAnonymousSkillClaim(); + } + + // In the scenario where Auth is disabled, we still want to have the + // IsAuthenticated flag set in the ClaimsIdentity. To do this requires + // adding in an empty claim. + return new ClaimsIdentity(AuthenticationConstants.ANONYMOUS_AUTH_TYPE); }); } diff --git a/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/AppCredentialsTests.java b/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/AppCredentialsTests.java new file mode 100644 index 00000000..6310b659 --- /dev/null +++ b/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/AppCredentialsTests.java @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MT License. + +package com.microsoft.bot.connector; + +import java.io.IOException; +import java.net.MalformedURLException; + +import com.microsoft.bot.connector.authentication.AppCredentials; +import com.microsoft.bot.connector.authentication.AppCredentialsInterceptor; +import com.microsoft.bot.connector.authentication.AuthenticationConstants; +import com.microsoft.bot.connector.authentication.Authenticator; +import com.microsoft.bot.restclient.ServiceClient; + +import org.junit.Assert; +import org.junit.Test; + +import okhttp3.Interceptor; +import okhttp3.MediaType; +import okhttp3.OkHttpClient; +import okhttp3.Protocol; +import okhttp3.Request; +import okhttp3.Response; +import okhttp3.ResponseBody; +import retrofit2.Retrofit; + +public class AppCredentialsTests { + + @Test + public void ConstructorTests() { + TestAppCredentials shouldDefaultToChannelScope = new TestAppCredentials("irrelevant"); + Assert.assertEquals(AuthenticationConstants.TO_CHANNEL_FROM_BOT_OAUTH_SCOPE, + shouldDefaultToChannelScope.oAuthScope()); + + TestAppCredentials shouldDefaultToCustomScope = new TestAppCredentials("irrelevant", "customScope"); + Assert.assertEquals("customScope", shouldDefaultToCustomScope.oAuthScope()); + } + + @Test + public void basicCredentialsTest() throws Exception { + TestAppCredentials credentials = new TestAppCredentials("irrelevant", "pass"); + OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); + credentials.applyCredentialsFilter(clientBuilder); + clientBuilder.addInterceptor( + new Interceptor() { + @Override + public Response intercept(Chain chain) throws IOException { + String header = chain.request().header("Authorization"); + Assert.assertNull(header); + return new Response.Builder() + .request(chain.request()) + .code(200) + .message("OK") + .protocol(Protocol.HTTP_1_1) + .body(ResponseBody.create(MediaType.parse("text/plain"), "azure rocks")) + .build(); + } + }); + ServiceClient serviceClient = new ServiceClient("http://localhost", clientBuilder, new Retrofit.Builder()) { }; + Response response = serviceClient.httpClient().newCall( + new Request.Builder().url("http://localhost").build()).execute(); + Assert.assertEquals(200, response.code()); + } + + private class TestAppCredentials extends AppCredentials { + TestAppCredentials(String channelAuthTenant) { + super(channelAuthTenant); + } + + TestAppCredentials(String channelAuthTenant, String oAuthScope) { + super(channelAuthTenant, oAuthScope); + } + + @Override + protected Authenticator buildAuthenticator() throws MalformedURLException { + return null; + } + + /** + * Apply the credentials to the HTTP request. + * + *

+ * Note: Provides the same functionality as dotnet ProcessHttpRequestAsync + *

+ * + * @param clientBuilder the builder for building up an {@link OkHttpClient} + */ + @Override + public void applyCredentialsFilter(OkHttpClient.Builder clientBuilder) { + clientBuilder.interceptors().add(new AppCredentialsInterceptor(this)); + } + + } +} + diff --git a/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/JwtTokenValidationTests.java b/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/JwtTokenValidationTests.java index ce755c5a..90b5485d 100644 --- a/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/JwtTokenValidationTests.java +++ b/libraries/bot-connector/src/test/java/com/microsoft/bot/connector/JwtTokenValidationTests.java @@ -5,6 +5,10 @@ package com.microsoft.bot.connector; import com.microsoft.bot.connector.authentication.*; import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ChannelAccount; +import com.microsoft.bot.schema.ConversationReference; +import com.microsoft.bot.schema.RoleTypes; + import org.junit.Assert; import org.junit.Test; @@ -209,6 +213,29 @@ public class JwtTokenValidationTests { Assert.assertEquals("anonymous", identity.getIssuer()); } + /** + * Tests with no authentication header and makes sure the service URL is not added to the trusted list. + */ + @Test + public void ChannelAuthenticationDisabledAndSkillShouldBeAnonymous() throws ExecutionException, InterruptedException { + String header = ""; + CredentialProvider credentials = new SimpleCredentialProvider("", ""); + + ClaimsIdentity identity = JwtTokenValidation.authenticateRequest( + new Activity() {{ + setServiceUrl("https://webchat.botframework.com/"); + setChannelId(Channels.EMULATOR); + setRelatesTo(new ConversationReference()); + setRecipient(new ChannelAccount() { { setRole(RoleTypes.SKILL); } }); + }}, + header, + credentials, + new SimpleChannelProvider()).join(); + Assert.assertEquals(AuthenticationConstants.ANONYMOUS_AUTH_TYPE, identity.getType()); + Assert.assertEquals(AuthenticationConstants.ANONYMOUS_SKILL_APPID, JwtTokenValidation.getAppIdFromClaims(identity.claims())); + } + + @Test public void ChannelNoHeaderAuthenticationEnabledShouldThrow() throws IOException, ExecutionException, InterruptedException { try {