Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.
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 @@ -69,6 +69,10 @@ export class DefaultSettingManager extends FileSettingManager {
maxImbalanceRatio: 10,
maxUtteranceAllowed: 15000,
},
skill: {
isSkill: false,
allowedCallers: ['*'],
},
defaultLanguage: 'en-us',
languages: ['en-us'],
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Security.Claims;
using System.Threading.Tasks;
using Microsoft.Bot.Connector.Authentication;
using Microsoft.BotFramework.Composer.Core.Settings;
using Microsoft.Extensions.Configuration;

namespace Microsoft.BotFramework.Composer.WebAppTemplates.Authorization
{
public class AllowedCallersClaimsValidator : ClaimsValidator
{
private readonly List<string> _allowedCallers;

public AllowedCallersClaimsValidator(BotSkillSettings settings)
{
// skill.allowedCallers is the setting in the appsettings.json file
// that consists of the list of parent bot IDs that are allowed to access the skill.
// To add a new parent bot, simply edit the AllowedCallers and add
// the parent bot's Microsoft app ID to the list.
// In this sample, we allow all callers if AllowedCallers contains an "*".
if (settings?.AllowedCallers == null || settings.AllowedCallers.Length == 0)
{
throw new ArgumentException($"skill.allowedCallers has to be defined, e.g. ['*'] or ['callerAppId']");
}

_allowedCallers = new List<string>(settings.AllowedCallers);
}

public override Task ValidateClaimsAsync(IList<Claim> claims)
{
// If _allowedCallers contains an "*", we allow all callers.
if (SkillValidation.IsSkillClaim(claims) &&
!_allowedCallers.Contains("*"))
{
// Check that the appId claim in the skill request is in the list of callers configured for this bot.
var appId = JwtTokenValidation.GetAppIdFromClaims(claims);
if (!_allowedCallers.Contains(appId))
{
throw new UnauthorizedAccessException(
$"Received a request from a bot with an app ID of \"{appId}\". To enable requests from this caller, add the app ID to your configuration file.");
}
}

return Task.CompletedTask;
}
}
}
15 changes: 12 additions & 3 deletions runtime/dotnet/azurewebapp/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
using Microsoft.Bot.Connector.Authentication;
using Microsoft.BotFramework.Composer.Core;
using Microsoft.BotFramework.Composer.Core.Settings;
using Microsoft.BotFramework.Composer.WebAppTemplates.Authorization;

//using Microsoft.BotFramework.Composer.CustomAction;
using Microsoft.Extensions.Configuration;
Expand Down Expand Up @@ -84,9 +85,14 @@ public IStorage ConfigureStorage(BotSettings settings)
return storage;
}

public bool IsSkill(BotSettings settings)
{
return settings?.Skill?.IsSkill == true;
}

public BotFrameworkHttpAdapter GetBotAdapter(IStorage storage, BotSettings settings, UserState userState, ConversationState conversationState, IServiceProvider s, TelemetryInitializerMiddleware telemetryInitializerMiddleware)
{
var adapter = new BotFrameworkHttpAdapter(new ConfigurationCredentialProvider(this.Configuration));
var adapter = IsSkill(settings) ? new BotFrameworkHttpAdapter(new ConfigurationCredentialProvider(this.Configuration), s.GetService<AuthenticationConfiguration>()) : new BotFrameworkHttpAdapter(new ConfigurationCredentialProvider(this.Configuration));
Copy link
Member

@carlosscastro carlosscastro Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general we want to move away from doing GetService in startup.cs. If we could optionally do a services.AddSingleton<AuthenticationConfiguration> and have DI wire it into the adapter's constructor , and also do services.AddSingleton<ICredentialProvider>(new ConfigurationCredentialProvider...), then we could avoid the getService. Not 100% sure that would work because of the numerous overloads of BotFrameworkHttpAdapter constructor. If that doesn't work, then as-is is good. We are currently refactoring the adapter and will soon refactor this startup.cs, so we should clean this up soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a way to inject the Authorization. That's why I used GetService.


adapter
.UseStorage(storage)
Expand Down Expand Up @@ -123,8 +129,11 @@ public void ConfigureServices(IServiceCollection services)
services.AddSingleton<ICredentialProvider, ConfigurationCredentialProvider>();
services.AddSingleton<BotAdapter>(sp => (BotFrameworkHttpAdapter)sp.GetService<IBotFrameworkHttpAdapter>());

// Register AuthConfiguration to enable custom claim validation.
services.AddSingleton<AuthenticationConfiguration>();
// Register AuthConfiguration to enable custom claim validation for skills.
if (IsSkill(settings))
{
services.AddSingleton(sp => new AuthenticationConfiguration { ClaimsValidator = new AllowedCallersClaimsValidator(settings.Skill) });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

settings could be null, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If yes, we cannot throw NullRef

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsSkill returns false when null:

        public bool IsSkill(BotSettings settings)
        {
            return settings?.Skill?.IsSkill == true;
        }

So it should be fine

}

// register components.
ComponentRegistration.Add(new DialogsComponentRegistration());
Expand Down
2 changes: 2 additions & 0 deletions runtime/dotnet/core/Settings/BotSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public class BotSettings

public string Bot { get; set; }

public BotSkillSettings Skill { get; set; }

public class BlobStorageConfiguration
{
public string ConnectionString { get; set; }
Expand Down
12 changes: 12 additions & 0 deletions runtime/dotnet/core/Settings/BotSkillSettings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

namespace Microsoft.BotFramework.Composer.Core.Settings
{
public class BotSkillSettings
{
// Is skill
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, optional: Ideally we strive for either xml docs (/// format) or no docs.

public bool IsSkill { get; set; }
public string[] AllowedCallers { get; set; }
}
}
101 changes: 101 additions & 0 deletions runtime/dotnet/tests/AllowedCallersClaimsValidationTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Security.Claims;
using System.Threading.Tasks;
using Microsoft.Bot.Connector.Authentication;
using Microsoft.BotFramework.Composer.Core.Settings;
using Microsoft.BotFramework.Composer.WebAppTemplates.Authorization;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Tests
{
[TestClass]
public class AllowedCallersClaimsValidationTests
{
[TestMethod]
[ExpectedException(typeof(ArgumentException))]
public void WhenAllowedCallersIsNullThrowException()
{
var unused = new AllowedCallersClaimsValidator(new BotSkillSettings());
}

[TestMethod]
[ExpectedException(typeof(ArgumentException))]
public void WhenAllowedCallersIsEmptyThrowException()
{
var unused = new AllowedCallersClaimsValidator(new BotSkillSettings()
{
AllowedCallers = new string[0]
});
}

[TestMethod]
public async Task AllowAnyCaller()
{
var validator = new AllowedCallersClaimsValidator(new BotSkillSettings()
{
AllowedCallers = new string[]{"*"}
});
var callerAppId = "BE3F9920-D42D-4D3A-9BDF-DBA62DAE3A00";
var claims = CreateCallerClaims(callerAppId);

await validator.ValidateClaimsAsync(claims);
}

[TestMethod]
public async Task AllowedCaller()
{
const string callerAppId = "BE3F9920-D42D-4D3A-9BDF-DBA62DAE3A00";
var validator = new AllowedCallersClaimsValidator(new BotSkillSettings()
{
AllowedCallers = new string[]{callerAppId}
});

var claims = CreateCallerClaims(callerAppId);

await validator.ValidateClaimsAsync(claims);
}

[TestMethod]
public async Task AllowedCallers()
{
const string callerAppId = "BE3F9920-D42D-4D3A-9BDF-DBA62DAE3A00";
var validator = new AllowedCallersClaimsValidator(new BotSkillSettings()
{
AllowedCallers = new string[]{"anotherId", callerAppId}
});

var claims = CreateCallerClaims(callerAppId);

await validator.ValidateClaimsAsync(claims);
}

[TestMethod]
[ExpectedException(typeof(UnauthorizedAccessException))]
public async Task NonAllowedCallerShouldThrowException()
{
var callerAppId = "BE3F9920-D42D-4D3A-9BDF-DBA62DAE3A00";
var validator = new AllowedCallersClaimsValidator(new BotSkillSettings()
{
AllowedCallers = new string[]{callerAppId}
});

var claims = CreateCallerClaims("I'm not allowed");

await validator.ValidateClaimsAsync(claims);
}

private List<Claim> CreateCallerClaims(string appId)
{
return new List<Claim>()
{
new Claim(AuthenticationConstants.AppIdClaim, appId),
new Claim(AuthenticationConstants.VersionClaim, "1.0"),
new Claim(AuthenticationConstants.AudienceClaim, "5BA599BD-F9E9-48D3-B98D-377BB2A0EAE9"),
};
}
}
}
5 changes: 5 additions & 0 deletions runtime/node/src/shared/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ export interface BotFeatureSettings {
removeRecipientMention: boolean;
}

export interface BotSkillSettings {
isSkill: boolean;
alllowedCallers: string[];
}

export interface BlobStorageConfiguration {
connectionString: string;
container: string;
Expand Down