Skip to content

Commit

Permalink
fix: ClaimStrategy bypass cookie principal validation (#475)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AndrewTriesToCode
AndrewTriesToCode authored Oct 7, 2021
1 parent d74ae41 commit cd38a7f
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 26 deletions.
43 changes: 38 additions & 5 deletions ...ckle.MultiTenant.AspNetCore/Extensions/FinbuckleMultiTenantAspNetCoreBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public static FinbuckleMultiTenantBuilder<TTenantInfo> WithPerTenantAuthenticati
/// <param name="builder">MultiTenantBuilder instance.</param>
/// <param name="config">Authentication options config.</param>
/// <returns>The same MultiTenantBuilder passed into the method.</returns>
// ReSharper disable once MemberCanBePrivate.Global
public static FinbuckleMultiTenantBuilder<TTenantInfo> WithPerTenantAuthentication<TTenantInfo>(this FinbuckleMultiTenantBuilder<TTenantInfo> builder, Action<MultiTenantAuthenticationOptions> config)
where TTenantInfo : class, ITenantInfo, new()
{
Expand Down Expand Up @@ -202,7 +203,7 @@ public static FinbuckleMultiTenantBuilder<TTenantInfo> WithRouteStrategy<TTenant
{
if (string.IsNullOrWhiteSpace(tenantParam))
{
throw new ArgumentException("Invalud value for \"tenantParam\"", nameof(tenantParam));
throw new ArgumentException("Invalid value for \"tenantParam\"", nameof(tenantParam));
}

return builder.WithStrategy<RouteStrategy>(ServiceLifetime.Singleton, tenantParam);
Expand Down Expand Up @@ -235,25 +236,57 @@ public static FinbuckleMultiTenantBuilder<TTenantInfo> WithHostStrategy<TTenantI
}

/// <summary>
/// Adds and configures a ClaimStrategy with tenantKey "__tenant__" to the application.
/// Adds and configures a ClaimStrategy for claim name "__tenant__" to the application. Uses the default authentication handler scheme.
/// </summary>
/// <returns>The same MultiTenantBuilder passed into the method.</returns>
public static FinbuckleMultiTenantBuilder<TTenantInfo> WithClaimStrategy<TTenantInfo>(this FinbuckleMultiTenantBuilder<TTenantInfo> builder) where TTenantInfo : class, ITenantInfo, new()
{
return builder.WithStrategy<ClaimStrategy>(ServiceLifetime.Singleton, Constants.TenantToken);
return builder.WithClaimStrategy(Constants.TenantToken);
}

/// <summary>
/// Adds and configures a ClaimStrategy to the application. Uses the default authentication handler scheme.
/// </summary>
/// <param name="builder">MultiTenantBuilder instance.</param>
/// <param name="tenantKey">Claim name for determining the tenant identifier.</param>
/// <returns>The same MultiTenantBuilder passed into the method.</returns>
public static FinbuckleMultiTenantBuilder<TTenantInfo> WithClaimStrategy<TTenantInfo>(this FinbuckleMultiTenantBuilder<TTenantInfo> builder, string tenantKey) where TTenantInfo : class, ITenantInfo, new()
{
BypassSessionPrincipalValidation(builder);
return builder.WithStrategy<ClaimStrategy>(ServiceLifetime.Singleton, tenantKey);
}

/// <summary>
/// Adds and configures a ClaimStrategy to the application.
/// </summary>
/// <param name="builder">MultiTenantBuilder instance.</param>
/// <param name="tenantKey">The template for determining the tenant identifier in the host.</param>
/// <param name="tenantKey">Claim name for determining the tenant identifier.</param>
/// <param name="authenticationScheme">The authentication scheme to check for claims.</param>
/// <returns>The same MultiTenantBuilder passed into the method.</returns>
public static FinbuckleMultiTenantBuilder<TTenantInfo> WithClaimStrategy<TTenantInfo>(this FinbuckleMultiTenantBuilder<TTenantInfo> builder, string tenantKey, string authenticationScheme = null) where TTenantInfo : class, ITenantInfo, new()
public static FinbuckleMultiTenantBuilder<TTenantInfo> WithClaimStrategy<TTenantInfo>(this FinbuckleMultiTenantBuilder<TTenantInfo> builder, string tenantKey, string authenticationScheme) where TTenantInfo : class, ITenantInfo, new()
{
BypassSessionPrincipalValidation(builder);
return builder.WithStrategy<ClaimStrategy>(ServiceLifetime.Singleton, tenantKey, authenticationScheme);
}

private static void BypassSessionPrincipalValidation<TTenantInfo>(FinbuckleMultiTenantBuilder<TTenantInfo> builder)
where TTenantInfo : class, ITenantInfo, new()
{
builder.Services.ConfigureAll<CookieAuthenticationOptions>(options =>
{
var origOnValidatePrincipal = options.Events.OnValidatePrincipal;
options.Events.OnValidatePrincipal = async context =>
{
// Skip if bypass set (e.g. ClaimStrategy in effect)
if (context.HttpContext.Items.Keys.Contains($"{Constants.TenantToken}__bypass_validate_principal__"))
return;

if (origOnValidatePrincipal != null)
await origOnValidatePrincipal(context);
};
});
}

/// <summary>
/// Adds and configures a HeaderStrategy with tenantKey "__tenant__" to the application.
/// </summary>
Expand Down
48 changes: 29 additions & 19 deletions src/Finbuckle.MultiTenant.AspNetCore/Strategies/ClaimStrategy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,20 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;

// ReSharper disable once CheckNamespace
namespace Finbuckle.MultiTenant.Strategies
{
// ReSharper disable once ClassNeverInstantiated.Global
public class ClaimStrategy : IMultiTenantStrategy
{
private readonly string _tenantKey;
private readonly string _authenticationScheme;
public ClaimStrategy(string template, string authenticationScheme = null)

public ClaimStrategy(string template) : this(template, null)
{
}

public ClaimStrategy(string template, string authenticationScheme)
{
if (string.IsNullOrWhiteSpace(template))
throw new ArgumentException(nameof(template));
Expand All @@ -29,27 +36,30 @@ public async Task<string> GetIdentifierAsync(object context)
if (!(context is HttpContext httpContext))
throw new MultiTenantException(null, new ArgumentException($@"""{nameof(context)}"" type must be of type HttpContext", nameof(context)));

if (!httpContext.User.Identity.IsAuthenticated)
if (httpContext.User.Identity is { IsAuthenticated: true })
return httpContext.User.FindFirst(_tenantKey)?.Value;

AuthenticationScheme authScheme;
var schemeProvider = httpContext.RequestServices.GetRequiredService<IAuthenticationSchemeProvider>();
if (_authenticationScheme is null)
{
AuthenticationScheme authScheme;
var schemeProvider = httpContext.RequestServices.GetRequiredService<IAuthenticationSchemeProvider>();
if (_authenticationScheme is null)
{
authScheme = await schemeProvider.GetDefaultAuthenticateSchemeAsync();
}
else
{
authScheme = (await schemeProvider.GetAllSchemesAsync()).FirstOrDefault(x => x.Name == _authenticationScheme);
}

var handler = (IAuthenticationHandler)ActivatorUtilities.CreateInstance(httpContext.RequestServices, authScheme.HandlerType);
await handler.InitializeAsync(authScheme, httpContext);
httpContext.Items[$"{Constants.TenantToken}__bypass_validate_principle__"] = "true"; // Value doesn't matter.
var handlerResult = await handler.AuthenticateAsync();
httpContext.Items.Remove($"{Constants.TenantToken}__bypass_validate_principle__");
authScheme = await schemeProvider.GetDefaultAuthenticateSchemeAsync();
}
else
{
authScheme = (await schemeProvider.GetAllSchemesAsync()).FirstOrDefault(x => x.Name == _authenticationScheme);
}

if (authScheme is null)
throw new NullReferenceException("No authentication scheme found.");

var handler = (IAuthenticationHandler)ActivatorUtilities.CreateInstance(httpContext.RequestServices, authScheme.HandlerType);
await handler.InitializeAsync(authScheme, httpContext);
httpContext.Items[$"{Constants.TenantToken}__bypass_validate_principle__"] = "true"; // Value doesn't matter.
var handlerResult = await handler.AuthenticateAsync();
httpContext.Items.Remove($"{Constants.TenantToken}__bypass_validate_principle__");

var identifier = httpContext.User.FindFirst(_tenantKey)?.Value;
var identifier = handlerResult.Principal?.FindFirst(_tenantKey)?.Value;
return await Task.FromResult(identifier);
}
}
Expand Down
1 change: 1 addition & 0 deletions ...ntityFrameworkCore/Extensions/FinbuckleMultiTenantBuilderExtensionsEntityFrameworkCore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public static class FinbuckleMultiTenantBuilderExtensionsEntityFrameworkCore
/// Adds an EFCore based multitenant store to the application. Will also add the database context service unless it is already added.
/// </summary>
/// <returns>The same MultiTenantBuilder passed into the method.</returns>
// ReSharper disable once InconsistentNaming
public static FinbuckleMultiTenantBuilder<TTenantInfo> WithEFCoreStore<TEFCoreStoreDbContext, TTenantInfo>(this FinbuckleMultiTenantBuilder<TTenantInfo> builder)
where TEFCoreStoreDbContext : EFCoreStoreDbContext<TTenantInfo>
where TTenantInfo : class, ITenantInfo, new()
Expand Down
45 changes: 43 additions & 2 deletions test/Finbuckle.MultiTenant.AspNetCore.Test/Extensions/MultiTenantBuilderExtensionsShould.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public void PassprincipalValidationIfTenantMatch()
}

[Fact]
public void SkipprincipalValidationIfBypassSet()
public void SkipPrincipalValidationIfBypassSet_WithPerTenantAuthentication()
{
var services = new ServiceCollection();
services.AddLogging();
Expand Down Expand Up @@ -184,9 +184,50 @@ public void SkipprincipalValidationIfBypassSet()
Assert.NotNull(cookieValidationContext.Principal);
Assert.False(called);
}

[Fact]
public void SkipPrincipalValidationIfBypassSet_WithClaimStrategy()
{
var services = new ServiceCollection();
services.AddLogging();
var called = false;
#pragma warning disable 1998
services.AddAuthentication().AddCookie(o => o.Events.OnValidatePrincipal = async _ => called = true);
#pragma warning restore 1998
services.AddMultiTenant<TenantInfo>()
.WithClaimStrategy();
var sp = services.BuildServiceProvider();

// Fake a resolved tenant
var mtc = new MultiTenantContext<TenantInfo>
{
TenantInfo = new TenantInfo { Identifier = "abc1" }
};
sp.GetRequiredService<IMultiTenantContextAccessor<TenantInfo>>().MultiTenantContext = mtc;

// Trigger the ValidatePrincipal event
var httpContextMock = new Mock<HttpContext>();
httpContextMock.Setup(c => c.RequestServices).Returns(sp);
var httpContextItems = new Dictionary<object, object>();
httpContextItems[$"{Constants.TenantToken}__bypass_validate_principal__"] = true;
httpContextMock.Setup(c => c.Items).Returns(httpContextItems);
var scheme = sp.GetRequiredService<IAuthenticationSchemeProvider>()
.GetSchemeAsync(CookieAuthenticationDefaults.AuthenticationScheme).Result;
var options = sp.GetRequiredService<IOptionsMonitor<CookieAuthenticationOptions>>().Get(CookieAuthenticationDefaults.AuthenticationScheme);
var principal = new ClaimsPrincipal(new ClaimsIdentity());
var authTicket = new AuthenticationTicket(principal, CookieAuthenticationDefaults.AuthenticationScheme);
authTicket.Properties.Items[Constants.TenantToken] = "abc2";
var cookieValidationContext =
new CookieValidatePrincipalContext(httpContextMock.Object, scheme, options, authTicket);

options.Events.ValidatePrincipal(cookieValidationContext).Wait();

Assert.NotNull(cookieValidationContext.Principal);
Assert.False(called);
}

[Fact]
public void RejectprincipalValidationIfTenantMatch()
public void RejectPrincipalValidationIfTenantMatch()
{
var services = new ServiceCollection();
services.AddLogging();
Expand Down

0 comments on commit cd38a7f

Please sign in to comment.