Fixes for Issue 809 (#1060)

libraries/bot-builder/src/main/java/com/microsoft/bot/builder/ChannelServiceHandler.java

@@ -48,11 +48,11 @@ public 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 @@ private CompletableFuture<ClaimsIdentity> authenticate(String authHeader) {
             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")
                     );
                 }

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<ResourceResponse> 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;
+        }
+
+    }
+}
+

libraries/bot-connector/src/main/java/com/microsoft/bot/connector/authentication/AppCredentials.java

@@ -242,6 +242,9 @@ public CompletableFuture<String> getToken() {
      * @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);
     }
 

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 static CompletableFuture<ClaimsIdentity> authenticateRequest(
         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.");
+                }
+
+                if (activity.getChannelId() != null
+                    && activity.getChannelId().equals(Channels.EMULATOR)
+                    && activity.getRecipient() != null
+                    && activity.getRecipient().getRole().equals(RoleTypes.SKILL)) {
+                    return SkillValidation.createAnonymousSkillClaim();
                 }
 
-                // No Auth Header. Auth is required. Request is not authorized.
-                throw new AuthenticationException("No Auth Header. Auth is required.");
+                // 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);
             });
         }
 

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.
+         *
+         * <p>
+         * Note: Provides the same functionality as dotnet ProcessHttpRequestAsync
+         * </p>
+         *
+         * @param clientBuilder the builder for building up an {@link OkHttpClient}
+         */
+        @Override
+        public void applyCredentialsFilter(OkHttpClient.Builder clientBuilder) {
+            clientBuilder.interceptors().add(new AppCredentialsInterceptor(this));
+        }
+
+    }
+}
+

libraries/bot-connector/src/test/java/com/microsoft/bot/connector/JwtTokenValidationTests.java

@@ -5,6 +5,10 @@
 
 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 void ChannelAuthenticationDisabledShouldBeAnonymous() throws ExecutionExc
         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 {