diff --git a/bff/hosts/Hosts.IdentityServer/Pages/Account/Login/Index.cshtml.cs b/bff/hosts/Hosts.IdentityServer/Pages/Account/Login/Index.cshtml.cs index 4714d951d..35ead6dbb 100644 --- a/bff/hosts/Hosts.IdentityServer/Pages/Account/Login/Index.cshtml.cs +++ b/bff/hosts/Hosts.IdentityServer/Pages/Account/Login/Index.cshtml.cs @@ -39,7 +39,7 @@ public class Index : PageModel TestUserStore users = null) { // this is where you would plug in your own custom identity management library (e.g. ASP.NET Identity) - _users = users ?? throw new Exception("Please call 'AddTestUsers(TestUsers.Users)' on the IIdentityServerBuilder in Startup or remove the TestUserStore from the AccountController."); + _users = users ?? throw new InvalidOperationException("Please call 'AddTestUsers(TestUsers.Users)' on the IIdentityServerBuilder in Startup or remove the TestUserStore from the AccountController."); _interaction = interaction; _clientStore = clientStore; @@ -146,7 +146,7 @@ public class Index : PageModel else { // user might have clicked on a malicious link - should be logged - throw new Exception("invalid return URL"); + throw new InvalidOperationException("invalid return URL"); } } diff --git a/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Callback.cshtml.cs b/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Callback.cshtml.cs index 5abd3f9f3..4329a5265 100644 --- a/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Callback.cshtml.cs +++ b/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Callback.cshtml.cs @@ -30,7 +30,7 @@ public class Callback : PageModel TestUserStore users = null) { // this is where you would plug in your own custom identity management library (e.g. ASP.NET Identity) - _users = users ?? throw new Exception("Please call 'AddTestUsers(TestUsers.Users)' on the IIdentityServerBuilder in Startup or remove the TestUserStore from the AccountController."); + _users = users ?? throw new InvalidOperationException("Please call 'AddTestUsers(TestUsers.Users)' on the IIdentityServerBuilder in Startup or remove the TestUserStore from the AccountController."); _interaction = interaction; _logger = logger; @@ -43,7 +43,7 @@ public class Callback : PageModel var result = await HttpContext.AuthenticateAsync(IdentityServerConstants.ExternalCookieAuthenticationScheme); if (result?.Succeeded != true) { - throw new Exception("External authentication error"); + throw new InvalidOperationException("External authentication error"); } var externalUser = result.Principal; @@ -60,7 +60,7 @@ public class Callback : PageModel // depending on the external provider, some other claim type might be used var userIdClaim = externalUser.FindFirst(JwtClaimTypes.Subject) ?? externalUser.FindFirst(ClaimTypes.NameIdentifier) ?? - throw new Exception("Unknown userid"); + throw new InvalidOperationException("Unknown userid"); var provider = result.Properties.Items["scheme"]; var providerUserId = userIdClaim.Value; diff --git a/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Challenge.cshtml.cs b/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Challenge.cshtml.cs index f8342aac7..25980a1a8 100644 --- a/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Challenge.cshtml.cs +++ b/bff/hosts/Hosts.IdentityServer/Pages/ExternalLogin/Challenge.cshtml.cs @@ -28,7 +28,7 @@ public class Challenge : PageModel if (Url.IsLocalUrl(returnUrl) == false && _interactionService.IsValidReturnUrl(returnUrl) == false) { // user might have clicked on a malicious link - should be logged - throw new Exception("invalid return URL"); + throw new InvalidOperationException("invalid return URL"); } // start challenge and roundtrip the return URL and scheme diff --git a/bff/hosts/RemoteApis/Hosts.RemoteApi.DPoP/DPoP/ConfigureJwtBearerOptions.cs b/bff/hosts/RemoteApis/Hosts.RemoteApi.DPoP/DPoP/ConfigureJwtBearerOptions.cs index fbf9f841e..8580e9024 100644 --- a/bff/hosts/RemoteApis/Hosts.RemoteApi.DPoP/DPoP/ConfigureJwtBearerOptions.cs +++ b/bff/hosts/RemoteApis/Hosts.RemoteApi.DPoP/DPoP/ConfigureJwtBearerOptions.cs @@ -18,11 +18,11 @@ public class ConfigureJwtBearerOptions : IPostConfigureOptions { if (options.EventsType != null && !typeof(DPoPJwtBearerEvents).IsAssignableFrom(options.EventsType)) { - throw new Exception("EventsType on JwtBearerOptions must derive from DPoPJwtBearerEvents to work with the DPoP support."); + throw new InvalidOperationException("EventsType on JwtBearerOptions must derive from DPoPJwtBearerEvents to work with the DPoP support."); } if (options.Events != null && !typeof(DPoPJwtBearerEvents).IsAssignableFrom(options.Events.GetType())) { - throw new Exception("Events on JwtBearerOptions must derive from DPoPJwtBearerEvents to work with the DPoP support."); + throw new InvalidOperationException("Events on JwtBearerOptions must derive from DPoPJwtBearerEvents to work with the DPoP support."); } if (options.Events == null && options.EventsType == null) diff --git a/bff/src/Bff.Blazor.Client/Internals/BffClientAuthenticationStateProvider.cs b/bff/src/Bff.Blazor.Client/Internals/BffClientAuthenticationStateProvider.cs index 0b08d4362..70ca9b6bc 100644 --- a/bff/src/Bff.Blazor.Client/Internals/BffClientAuthenticationStateProvider.cs +++ b/bff/src/Bff.Blazor.Client/Internals/BffClientAuthenticationStateProvider.cs @@ -8,7 +8,7 @@ using Microsoft.Extensions.Options; namespace Duende.Bff.Blazor.Client.Internals; -internal class BffClientAuthenticationStateProvider : AuthenticationStateProvider +internal class BffClientAuthenticationStateProvider : AuthenticationStateProvider, IDisposable { public const string HttpClientName = "Duende.Bff.Blazor.Client:StateProvider"; @@ -17,7 +17,7 @@ internal class BffClientAuthenticationStateProvider : AuthenticationStateProvide private ClaimsPrincipal? _user; private readonly ILogger _logger; - private SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1); + private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1); /// /// An intended for use in Blazor @@ -90,7 +90,9 @@ internal class BffClientAuthenticationStateProvider : AuthenticationStateProvide try { await _semaphore.WaitAsync(); +#pragma warning disable CA1508 // this is a false positive. It's a double locking pattern. if (_user == null) +#pragma warning restore CA1508 { _user = await RefreshUser(); } @@ -103,4 +105,10 @@ internal class BffClientAuthenticationStateProvider : AuthenticationStateProvide return new AuthenticationState(_user); } + + public void Dispose() + { + _timer?.Dispose(); + _semaphore.Dispose(); + } } diff --git a/bff/src/Bff.Blazor.Client/Internals/FetchUserService.cs b/bff/src/Bff.Blazor.Client/Internals/FetchUserService.cs index 48fe7b034..6bae70149 100644 --- a/bff/src/Bff.Blazor.Client/Internals/FetchUserService.cs +++ b/bff/src/Bff.Blazor.Client/Internals/FetchUserService.cs @@ -3,6 +3,7 @@ using System.Net.Http.Json; using System.Security.Claims; +using System.Text.Json; using Microsoft.Extensions.Logging; namespace Duende.Bff.Blazor.Client.Internals; @@ -10,7 +11,7 @@ namespace Duende.Bff.Blazor.Client.Internals; /// /// Internal service that retrieves user info from the /bff/user endpoint. /// -internal class FetchUserService +internal class FetchUserService : IDisposable { private readonly HttpClient _client; private readonly ILogger _logger; @@ -33,7 +34,9 @@ internal class FetchUserService internal FetchUserService() { _client = new HttpClient(); +#pragma warning disable CA2000 // This is a test-only ctor, so we don't want to dispose the client here. _logger = new Logger(new LoggerFactory()); +#pragma warning restore CA2000 } public virtual async ValueTask FetchUserAsync() @@ -58,10 +61,17 @@ internal class FetchUserService return new ClaimsPrincipal(identity); } - catch (Exception ex) + catch (HttpRequestException ex) + { + _logger.FetchingUserFailed(ex); + return new ClaimsPrincipal(new ClaimsIdentity()); + } + catch (JsonException ex) { _logger.FetchingUserFailed(ex); return new ClaimsPrincipal(new ClaimsIdentity()); } } + + public void Dispose() => _client.Dispose(); } diff --git a/bff/src/Bff.Blazor.Client/ServiceCollectionExtensions.cs b/bff/src/Bff.Blazor.Client/ServiceCollectionExtensions.cs index 06543ced6..b8d16b15b 100644 --- a/bff/src/Bff.Blazor.Client/ServiceCollectionExtensions.cs +++ b/bff/src/Bff.Blazor.Client/ServiceCollectionExtensions.cs @@ -83,24 +83,24 @@ public static class ServiceCollectionExtensions private static Action SetRemoteApiBaseAddress( Action? configureClient) => (sp, client) => - { - SetRemoteApiBaseAddress(sp, client); - configureClient?.Invoke(sp, client); - }; + { + SetRemoteApiBaseAddress(sp, client); + configureClient?.Invoke(sp, client); + }; private static Action SetRemoteApiBaseAddress( Action? configureClient) => (sp, client) => - { - SetRemoteApiBaseAddress(sp, client); - configureClient?.Invoke(client); - }; + { + SetRemoteApiBaseAddress(sp, client); + configureClient?.Invoke(client); + }; private static void SetLocalApiBaseAddress(IServiceProvider sp, HttpClient client) { var baseAddress = GetLocalBaseAddress(sp); - if (!baseAddress.EndsWith("/")) + if (!baseAddress.EndsWith('/')) { - baseAddress += "/"; + baseAddress += '/'; } client.BaseAddress = new Uri(baseAddress); @@ -108,37 +108,37 @@ public static class ServiceCollectionExtensions private static Action SetLocalApiBaseAddress( Action? configureClient) => (sp, client) => - { - SetLocalApiBaseAddress(sp, client); - configureClient?.Invoke(client); - }; + { + SetLocalApiBaseAddress(sp, client); + configureClient?.Invoke(client); + }; private static Action SetLocalApiBaseAddress( Action? configureClient) => (sp, client) => - { - SetLocalApiBaseAddress(sp, client); - configureClient?.Invoke(sp, client); - }; + { + SetLocalApiBaseAddress(sp, client); + configureClient?.Invoke(sp, client); + }; private static void SetRemoteApiBaseAddress(IServiceProvider sp, HttpClient client) { var baseAddress = GetRemoteBaseAddress(sp); - if (!baseAddress.EndsWith("/")) + if (!baseAddress.EndsWith('/')) { - baseAddress += "/"; + baseAddress += '/'; } var remoteApiPath = GetRemoteApiPath(sp); if (!string.IsNullOrEmpty(remoteApiPath)) { - if (remoteApiPath.StartsWith("/")) + if (remoteApiPath.StartsWith('/')) { remoteApiPath = remoteApiPath.Substring(1); } - if (!remoteApiPath.EndsWith("/")) + if (!remoteApiPath.EndsWith('/')) { - remoteApiPath += "/"; + remoteApiPath += '/'; } } @@ -159,8 +159,9 @@ public static class ServiceCollectionExtensions /// A configuration callback used to set up /// the . public static IHttpClientBuilder AddLocalApiHttpClient(this IServiceCollection services, string clientName, - Action configureClient) => services.AddHttpClient(clientName, SetLocalApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + Action configureClient) => services + .AddHttpClient(clientName, SetLocalApiBaseAddress(configureClient)) + .AddHttpMessageHandler(); /// /// Adds a named for use when invoking local APIs @@ -175,8 +176,9 @@ public static class ServiceCollectionExtensions /// A configuration callback used to set up /// the . public static IHttpClientBuilder AddLocalApiHttpClient(this IServiceCollection services, string clientName, - Action? configureClient = null) => services.AddHttpClient(clientName, SetLocalApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + Action? configureClient = null) => services + .AddHttpClient(clientName, SetLocalApiBaseAddress(configureClient)) + .AddHttpMessageHandler(); /// /// Adds a typed for use when invoking remote APIs @@ -189,7 +191,7 @@ public static class ServiceCollectionExtensions public static IHttpClientBuilder AddLocalApiHttpClient(this IServiceCollection services, Action? configureClient = null) where T : class => services.AddHttpClient(SetLocalApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + .AddHttpMessageHandler(); /// /// Adds a named for use when invoking remote APIs @@ -204,8 +206,9 @@ public static class ServiceCollectionExtensions /// A configuration callback used to set up /// the . public static IHttpClientBuilder AddRemoteApiHttpClient(this IServiceCollection services, string clientName, - Action configureClient) => services.AddHttpClient(clientName, SetRemoteApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + Action configureClient) => services + .AddHttpClient(clientName, SetRemoteApiBaseAddress(configureClient)) + .AddHttpMessageHandler(); /// /// Adds a named for use when invoking remote APIs @@ -221,8 +224,9 @@ public static class ServiceCollectionExtensions /// A configuration callback used to set up /// the . public static IHttpClientBuilder AddRemoteApiHttpClient(this IServiceCollection services, string clientName, - Action? configureClient = null) => services.AddHttpClient(clientName, SetRemoteApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + Action? configureClient = null) => services + .AddHttpClient(clientName, SetRemoteApiBaseAddress(configureClient)) + .AddHttpMessageHandler(); /// /// Adds a typed for use when invoking remote APIs @@ -234,7 +238,7 @@ public static class ServiceCollectionExtensions public static IHttpClientBuilder AddRemoteApiHttpClient(this IServiceCollection services, Action configureClient) where T : class => services.AddHttpClient(SetRemoteApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + .AddHttpMessageHandler(); /// /// Adds a typed for use when invoking remote APIs @@ -247,5 +251,5 @@ public static class ServiceCollectionExtensions public static IHttpClientBuilder AddRemoteApiHttpClient(this IServiceCollection services, Action? configureClient = null) where T : class => services.AddHttpClient(SetRemoteApiBaseAddress(configureClient)) - .AddHttpMessageHandler(); + .AddHttpMessageHandler(); } diff --git a/bff/src/Bff.Blazor/BffBuilderExtensions.cs b/bff/src/Bff.Blazor/BffBuilderExtensions.cs index 8da32826a..959a1cb57 100644 --- a/bff/src/Bff.Blazor/BffBuilderExtensions.cs +++ b/bff/src/Bff.Blazor/BffBuilderExtensions.cs @@ -11,8 +11,11 @@ namespace Duende.Bff.Blazor; public static class BffBuilderExtensions { - public static BffBuilder AddBlazorServer(this BffBuilder builder, Action? configureOptions = null) + public static BffBuilder AddBlazorServer(this BffBuilder builder, + Action? configureOptions = null) { + ArgumentNullException.ThrowIfNull(builder); + builder.Services .AddOpenIdConnectAccessTokenManagement() .AddBlazorServerAccessTokenManagement() diff --git a/bff/src/Bff.Blazor/BffServerAuthenticationStateProvider.cs b/bff/src/Bff.Blazor/BffServerAuthenticationStateProvider.cs index 9182cc113..4afb40be0 100644 --- a/bff/src/Bff.Blazor/BffServerAuthenticationStateProvider.cs +++ b/bff/src/Bff.Blazor/BffServerAuthenticationStateProvider.cs @@ -114,7 +114,7 @@ internal sealed class BffServerAuthenticationStateProvider : RevalidatingServerA Claims = claims.ToArray() }; - _logger.LogDebug("Persisting Authentication State"); + _logger.PersistingAuthenticationState(LogLevel.Debug); _state.PersistAsJson(nameof(ClaimsPrincipalRecord), principal); } @@ -143,7 +143,7 @@ internal sealed class BffServerAuthenticationStateProvider : RevalidatingServerA SessionId = sid, SubjectId = sub }, - ct); + ct); return sessions.Count != 0; } } diff --git a/bff/src/Bff.Blazor/LogMessages.cs b/bff/src/Bff.Blazor/LogMessages.cs new file mode 100644 index 000000000..53a25fbb7 --- /dev/null +++ b/bff/src/Bff.Blazor/LogMessages.cs @@ -0,0 +1,13 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using Microsoft.Extensions.Logging; + +namespace Duende.Bff.Blazor; + +internal static partial class LogMessages +{ + [LoggerMessage( + Message = "Persisting authentication state")] + public static partial void PersistingAuthenticationState(this ILogger logger, LogLevel logLevel); +} diff --git a/bff/src/Bff.Blazor/ServerSideTokenStore.cs b/bff/src/Bff.Blazor/ServerSideTokenStore.cs index f8b6aeec4..6d0c1f682 100644 --- a/bff/src/Bff.Blazor/ServerSideTokenStore.cs +++ b/bff/src/Bff.Blazor/ServerSideTokenStore.cs @@ -33,7 +33,7 @@ internal class ServerSideTokenStore( public async Task> GetTokenAsync(ClaimsPrincipal user, UserTokenRequestParameters? parameters = null, CancellationToken ct = default) { - logger.RetrievingTokenForUser(user.Identity?.Name); + logger.RetrievingTokenForUser(LogLevel.Debug, user.Identity?.Name); var session = await GetSession(user); if (session == null) { @@ -58,7 +58,7 @@ internal class ServerSideTokenStore( var sub = user.FindFirst("sub")?.Value ?? throw new InvalidOperationException("no sub claim"); var sid = user.FindFirst("sid")?.Value ?? throw new InvalidOperationException("no sid claim"); - logger.RetrievingSession(sid, sub); + logger.RetrievingSession(LogLevel.Debug, sid, sub); var sessions = await sessionStore.GetUserSessionsAsync(new UserSessionsFilter { @@ -82,7 +82,7 @@ internal class ServerSideTokenStore( public async Task StoreTokenAsync(ClaimsPrincipal user, UserToken token, UserTokenRequestParameters? parameters = null, CT ct = default) { - logger.StoringTokenForUser(user.Identity?.Name); + logger.StoringTokenForUser(LogLevel.Debug, user.Identity?.Name); await UpdateTicket(user, async ticket => { await tokensInAuthProperties.SetUserTokenAsync(token, ticket.Properties, parameters, ct); }); } @@ -90,7 +90,7 @@ internal class ServerSideTokenStore( public async Task ClearTokenAsync(ClaimsPrincipal user, UserTokenRequestParameters? parameters = null, CT ct = default) { - logger.RemovingTokenForUser(user.Identity?.Name); + logger.RemovingTokenForUser(LogLevel.Debug, user.Identity?.Name); await UpdateTicket(user, ticket => { tokensInAuthProperties.RemoveUserToken(ticket.Properties, parameters); @@ -103,7 +103,7 @@ internal class ServerSideTokenStore( var session = await GetSession(user); if (session == null) { - logger.FailedToFindSessionToUpdate(); + logger.FailedToFindSessionToUpdate(LogLevel.Debug); return; } diff --git a/bff/src/Bff.EntityFramework/BffBuilderExtensions.cs b/bff/src/Bff.EntityFramework/BffBuilderExtensions.cs index 8409be0fb..ed4eeaa77 100644 --- a/bff/src/Bff.EntityFramework/BffBuilderExtensions.cs +++ b/bff/src/Bff.EntityFramework/BffBuilderExtensions.cs @@ -37,6 +37,7 @@ public static class BffBuilderExtensions public static BffBuilder AddEntityFrameworkServerSideSessions(this BffBuilder bffBuilder, Action action) where TContext : DbContext, ISessionDbContext { + ArgumentNullException.ThrowIfNull(bffBuilder); bffBuilder.Services.AddDbContext(action); return bffBuilder.AddEntityFrameworkServerSideSessionsServices(); } @@ -50,6 +51,7 @@ public static class BffBuilderExtensions public static BffBuilder AddEntityFrameworkServerSideSessions(this BffBuilder bffBuilder, Action action) where TContext : DbContext, ISessionDbContext { + ArgumentNullException.ThrowIfNull(bffBuilder); bffBuilder.Services.AddDbContext(action); return bffBuilder.AddEntityFrameworkServerSideSessionsServices(); } @@ -64,6 +66,7 @@ public static class BffBuilderExtensions public static BffBuilder AddEntityFrameworkServerSideSessionsServices(this BffBuilder bffBuilder) where TContext : ISessionDbContext { + ArgumentNullException.ThrowIfNull(bffBuilder); bffBuilder.Services.AddTransient(); bffBuilder.Services.AddTransient(svcs => svcs.GetRequiredService()); return bffBuilder.AddServerSideSessions(); @@ -74,6 +77,7 @@ public static class BffBuilderExtensions /// public static BffBuilder ConfigureEntityFrameworkSessionStoreOptions(this BffBuilder bffBuilder, Action action) { + ArgumentNullException.ThrowIfNull(bffBuilder); bffBuilder.Services.Configure(action); return bffBuilder; } diff --git a/bff/src/Bff.EntityFramework/Internal/UserSessionStore.cs b/bff/src/Bff.EntityFramework/Internal/UserSessionStore.cs index 8a055f6c2..d70cf7c79 100644 --- a/bff/src/Bff.EntityFramework/Internal/UserSessionStore.cs +++ b/bff/src/Bff.EntityFramework/Internal/UserSessionStore.cs @@ -13,14 +13,16 @@ namespace Duende.Bff.EntityFramework; /// /// Entity framework core implementation of IUserSessionStore /// -internal class UserSessionStore(IOptions options, ISessionDbContext sessionDbContext, ILogger logger) : IUserSessionStore, IUserSessionStoreCleanup +#pragma warning disable CA1812 // internal class never instantiated? It is, but via DI +internal sealed class UserSessionStore(IOptions options, ISessionDbContext sessionDbContext, ILogger logger) : IUserSessionStore, IUserSessionStoreCleanup +#pragma warning restore CA1812 { private readonly string? _applicationDiscriminator = options.Value.ApplicationDiscriminator; /// public async Task CreateUserSessionAsync(UserSession session, CT ct) { - LogMessages.CreatingUserSession(logger, session.SubjectId, session.SessionId); + logger.CreatingUserSession(LogLevel.Debug, session.SubjectId, session.SessionId); var item = new UserSessionEntity() { @@ -46,13 +48,13 @@ internal class UserSessionStore(IOptions options, ISessio // SQL Server would send: ---> Microsoft.Data.SqlClient.SqlException (0x80131904): Cannot insert duplicate key row in object 'Session.UserSessions' with unique index 'IX_UserSessions_ApplicationName_SessionId'. The duplicate key value is (, ). // Postgres would send: ---> Npgsql.PostgresException (0x80004005): 23505: duplicate key value violates unique constraint "IX_UserSessions_ApplicationName_SessionId" // MySQL would send: ---> MySql.Data.MySqlClient.MySqlException (0x80004005): Duplicate entry '-' for key 'IX_UserSessions_ApplicationName_SessionId' - if (exception.Contains("UNIQUE", StringComparison.OrdinalIgnoreCase) || exception.Contains("IX_UserSessions_ApplicationName_SessionId")) + if (exception.Contains("UNIQUE", StringComparison.OrdinalIgnoreCase) || exception.Contains("IX_UserSessions_ApplicationName_SessionId", StringComparison.OrdinalIgnoreCase)) { - LogMessages.DuplicateSessionInsertDetected(logger, ex); + logger.DuplicateSessionInsertDetected(LogLevel.Debug, ex); } else { - LogMessages.ExceptionCreatingSession(logger, ex, ex.Message); + logger.ExceptionCreatingSession(LogLevel.Warning, ex, ex.Message); } } } @@ -65,11 +67,11 @@ internal class UserSessionStore(IOptions options, ISessio if (item == null) { - LogMessages.NoRecordFoundForKey(logger, key); + logger.NoRecordFoundForKey(LogLevel.Debug, key); return; } - LogMessages.DeletingUserSession(logger, item.SubjectId, item.SessionId); + logger.DeletingUserSession(LogLevel.Debug, item.SubjectId, item.SessionId); sessionDbContext.UserSessions.Remove(item); try @@ -80,7 +82,7 @@ internal class UserSessionStore(IOptions options, ISessio { // suppressing exception for concurrent deletes // https://github.com/DuendeSoftware/BFF/issues/63 - LogMessages.DbUpdateConcurrencyException(logger, ex.Message); + logger.DbUpdateConcurrencyException(LogLevel.Debug, ex.Message); foreach (var entry in ex.Entries) { @@ -117,7 +119,7 @@ internal class UserSessionStore(IOptions options, ISessio items = items.Where(x => x.SessionId == filter.SessionId).ToArray(); } - LogMessages.DeletingUserSessions(logger, items.Length, filter.SubjectId, filter.SessionId); + logger.DeletingUserSessions(LogLevel.Debug, items.Length, filter.SubjectId, filter.SessionId); sessionDbContext.UserSessions.RemoveRange(items); @@ -129,7 +131,7 @@ internal class UserSessionStore(IOptions options, ISessio { // suppressing exception for concurrent deletes // https://github.com/DuendeSoftware/BFF/issues/63 - LogMessages.DbUpdateConcurrencyException(logger, ex.Message); + logger.DbUpdateConcurrencyException(LogLevel.Debug, ex.Message); foreach (var entry in ex.Entries) { @@ -148,11 +150,11 @@ internal class UserSessionStore(IOptions options, ISessio UserSession? result = null; if (item == null) { - LogMessages.NoRecordFoundForKey(logger, key); + logger.NoRecordFoundForKey(LogLevel.Debug, key); return null; } - LogMessages.GettingUserSession(logger, item.SubjectId, item.SessionId); + logger.GettingUserSession(LogLevel.Debug, item.SubjectId, item.SessionId); result = new UserSession(); item.CopyTo(result); @@ -194,7 +196,7 @@ internal class UserSessionStore(IOptions options, ISessio return item; }).ToArray(); - LogMessages.GettingUserSessions(logger, results.Length, filter.SubjectId, filter.SessionId); + logger.GettingUserSessions(LogLevel.Debug, results.Length, filter.SubjectId, filter.SessionId); return results; } @@ -206,11 +208,11 @@ internal class UserSessionStore(IOptions options, ISessio var item = items.SingleOrDefault(x => x.Key == key && x.ApplicationName == _applicationDiscriminator); if (item == null) { - LogMessages.NoRecordFoundForKey(logger, key); + logger.NoRecordFoundForKey(LogLevel.Debug, key); return; } - LogMessages.UpdatingUserSession(logger, item.SubjectId, item.SessionId); + logger.UpdatingUserSession(LogLevel.Debug, item.SubjectId, item.SessionId); session.CopyTo(item); await sessionDbContext.SaveChangesAsync(ct); @@ -239,7 +241,7 @@ internal class UserSessionStore(IOptions options, ISessio continue; } - LogMessages.RemovingServerSideSessions(logger, found); + logger.RemovingServerSideSessions(LogLevel.Debug, found); sessionDbContext.UserSessions.RemoveRange(expired); removed += found; @@ -250,7 +252,7 @@ internal class UserSessionStore(IOptions options, ISessio catch (DbUpdateConcurrencyException ex) { // suppressing exception for concurrent deletes - LogMessages.DbUpdateConcurrencyException(logger, ex.Message); + logger.DbUpdateConcurrencyException(LogLevel.Debug, ex.Message); foreach (var entry in ex.Entries) { diff --git a/bff/src/Bff.EntityFramework/ModelBuilderExtensions.cs b/bff/src/Bff.EntityFramework/ModelBuilderExtensions.cs index b3df277f7..cdc0f4b39 100644 --- a/bff/src/Bff.EntityFramework/ModelBuilderExtensions.cs +++ b/bff/src/Bff.EntityFramework/ModelBuilderExtensions.cs @@ -24,6 +24,8 @@ public static class ModelBuilderExtensions /// The store options. public static void ConfigureSessionContext(this ModelBuilder modelBuilder, SessionStoreOptions storeOptions) { + ArgumentNullException.ThrowIfNull(modelBuilder); + ArgumentNullException.ThrowIfNull(storeOptions); if (!string.IsNullOrWhiteSpace(storeOptions.DefaultSchema)) { modelBuilder.HasDefaultSchema(storeOptions.DefaultSchema); diff --git a/bff/src/Bff.Yarp/BffBuilderExtensions.cs b/bff/src/Bff.Yarp/BffBuilderExtensions.cs index 23fd74906..7ddc0eb0f 100644 --- a/bff/src/Bff.Yarp/BffBuilderExtensions.cs +++ b/bff/src/Bff.Yarp/BffBuilderExtensions.cs @@ -22,6 +22,8 @@ public static class BffBuilderExtensions /// public static BffBuilder AddRemoteApis(this BffBuilder builder) { + ArgumentNullException.ThrowIfNull(builder); + builder.RegisterConfigurationLoader((services, config) => { services.Configure(config); @@ -45,6 +47,8 @@ public static class BffBuilderExtensions public static IReverseProxyBuilder AddYarpConfig(this BffBuilder builder, RouteConfig[] routes, ClusterConfig[] clusters) { + ArgumentNullException.ThrowIfNull(builder); + var yarpBuilder = builder.Services.AddReverseProxy() .AddBffExtensions(); @@ -55,6 +59,8 @@ public static class BffBuilderExtensions public static IReverseProxyBuilder AddYarpConfig(this BffBuilder builder, IConfiguration config) { + ArgumentNullException.ThrowIfNull(builder); + var yarpBuilder = builder.Services.AddReverseProxy() .AddBffExtensions(); diff --git a/bff/src/Bff.Yarp/DefaultBffYarpTransformerBuilders.cs b/bff/src/Bff.Yarp/DefaultBffYarpTransformerBuilders.cs index c33d5d943..f207a756e 100644 --- a/bff/src/Bff.Yarp/DefaultBffYarpTransformerBuilders.cs +++ b/bff/src/Bff.Yarp/DefaultBffYarpTransformerBuilders.cs @@ -15,7 +15,7 @@ public static class DefaultBffYarpTransformerBuilders /// Build a default 'direct proxy' transformer. This removes the 'cookie' header, removes the local path prefix, /// and adds an access token to the request. The type of access token is determined by the . /// - public static BffYarpTransformBuilder DirectProxyWithAccessToken = + public static readonly BffYarpTransformBuilder DirectProxyWithAccessToken = (localPath, context) => { context.AddRequestHeaderRemove("Cookie"); diff --git a/bff/src/Bff.Yarp/EventIds.cs b/bff/src/Bff.Yarp/EventIds.cs deleted file mode 100644 index 52b6eb477..000000000 --- a/bff/src/Bff.Yarp/EventIds.cs +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright (c) Duende Software. All rights reserved. -// See LICENSE in the project root for license information. - -using Microsoft.Extensions.Logging; - -namespace Duende.Bff.Yarp; - -internal static class EventIds -{ - public static readonly EventId ProxyError = new(5, "ProxyError"); -} diff --git a/bff/src/Bff.Yarp/Internal/AccessTokenRequestTransform.cs b/bff/src/Bff.Yarp/Internal/AccessTokenRequestTransform.cs index 29312b481..b30c5d4da 100644 --- a/bff/src/Bff.Yarp/Internal/AccessTokenRequestTransform.cs +++ b/bff/src/Bff.Yarp/Internal/AccessTokenRequestTransform.cs @@ -100,11 +100,11 @@ internal class AccessTokenRequestTransform( { if (!options.Value.RemoveSessionAfterRefreshTokenExpiration) { - Otel.LogMessages.FailedToRequestNewUserAccessToken(logger, tokenError.Error); + logger.FailedToRequestNewUserAccessToken(LogLevel.Warning, tokenError.Error); return false; } - Otel.LogMessages.UserSessionRevoked(logger, tokenError.Error); + logger.UserSessionRevoked(LogLevel.Warning, tokenError.Error); return true; } @@ -142,7 +142,7 @@ internal class AccessTokenRequestTransform( tokenError.Error); } - private void ApplyBearerToken(RequestTransformContext context, BearerTokenResult token) => context.ProxyRequest.Headers.Authorization = + private static void ApplyBearerToken(RequestTransformContext context, BearerTokenResult token) => context.ProxyRequest.Headers.Authorization = new AuthenticationHeaderValue(OidcConstants.AuthenticationSchemes.AuthorizationHeaderBearer, token.AccessToken.ToString()); private async Task ApplyDPoPToken(RequestTransformContext context, DPoPTokenResult token) diff --git a/bff/src/Bff.Yarp/Internal/RemoteRouteHandler.cs b/bff/src/Bff.Yarp/Internal/RemoteRouteHandler.cs index 4d7e0e515..b42537dfa 100644 --- a/bff/src/Bff.Yarp/Internal/RemoteRouteHandler.cs +++ b/bff/src/Bff.Yarp/Internal/RemoteRouteHandler.cs @@ -45,8 +45,8 @@ internal class RemoteRouteHandler( foreach (var route in frontend.GetRemoteApis()) { - - if (context.Request.Path.StartsWithSegments(route.LocalPath.ToString())) + // Path matching must be case insensitive + if (context.Request.Path.StartsWithSegments(route.LocalPath.ToString(), StringComparison.OrdinalIgnoreCase)) { var bffRemoteApiEndpointMetadata = new BffRemoteApiEndpointMetadata() { diff --git a/bff/src/Bff.Yarp/Log.cs b/bff/src/Bff.Yarp/Log.cs deleted file mode 100644 index de345033b..000000000 --- a/bff/src/Bff.Yarp/Log.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Duende Software. All rights reserved. -// See LICENSE in the project root for license information. - -using Microsoft.Extensions.Logging; - -namespace Duende.Bff.Yarp; - -internal static class Log -{ - private static readonly Action ProxyResponseErrorMessage = LoggerMessage.Define( - LogLevel.Information, - EventIds.ProxyError, - "Proxy response error. local path: '{localPath}', error: '{error}'"); - - public static void ProxyResponseError(this ILogger logger, string localPath, string error) => ProxyResponseErrorMessage(logger, localPath, error, null); -} diff --git a/bff/src/Bff.Yarp/MapRemoteRoutesMiddleware.cs b/bff/src/Bff.Yarp/MapRemoteRoutesMiddleware.cs index 247c94356..03ec8344d 100644 --- a/bff/src/Bff.Yarp/MapRemoteRoutesMiddleware.cs +++ b/bff/src/Bff.Yarp/MapRemoteRoutesMiddleware.cs @@ -22,7 +22,7 @@ internal class MapRemoteRoutesMiddleware(RequestDelegate next, RemoteRouteHandle await next(context); } - private bool ShouldMapRemoteRoutes(HttpContext context) + private static bool ShouldMapRemoteRoutes(HttpContext context) { var selectedFrontend = context.RequestServices.GetRequiredService(); @@ -31,6 +31,6 @@ internal class MapRemoteRoutesMiddleware(RequestDelegate next, RemoteRouteHandle return false; } - return frontend.GetRemoteApis().Any(); + return frontend.GetRemoteApis().Length != 0; } } diff --git a/bff/src/Bff.Yarp/ProxyConfigExtensions.cs b/bff/src/Bff.Yarp/ProxyConfigExtensions.cs index a9bb320a8..b05c89714 100644 --- a/bff/src/Bff.Yarp/ProxyConfigExtensions.cs +++ b/bff/src/Bff.Yarp/ProxyConfigExtensions.cs @@ -17,7 +17,11 @@ public static class ProxyConfigExtensions /// /// /// - public static RouteConfig WithAccessToken(this RouteConfig config, RequiredTokenType requiredTokenType) => config.WithMetadata(Constants.Yarp.TokenTypeMetadata, requiredTokenType.ToString()); + public static RouteConfig WithAccessToken(this RouteConfig config, RequiredTokenType requiredTokenType) + { + ArgumentNullException.ThrowIfNull(config); + return config.WithMetadata(Constants.Yarp.TokenTypeMetadata, requiredTokenType.ToString()); + } /// /// Adds BFF access token metadata to a route configuration, indicating that @@ -27,17 +31,27 @@ public static class ProxyConfigExtensions /// /// [Obsolete("Use TokenRequirement.OptionalUserOrNone")] - public static RouteConfig WithOptionalUserAccessToken(this RouteConfig config) => WithAccessToken(config, RequiredTokenType.UserOrNone); + public static RouteConfig WithOptionalUserAccessToken(this RouteConfig config) + { + ArgumentNullException.ThrowIfNull(config); + return WithAccessToken(config, RequiredTokenType.UserOrNone); + } /// /// Adds anti-forgery metadata to a route configuration /// /// /// - public static RouteConfig WithAntiforgeryCheck(this RouteConfig config) => config.WithMetadata(Constants.Yarp.AntiforgeryCheckMetadata, "true"); + public static RouteConfig WithAntiforgeryCheck(this RouteConfig config) + { + ArgumentNullException.ThrowIfNull(config); + return config.WithMetadata(Constants.Yarp.AntiforgeryCheckMetadata, "true"); + } private static RouteConfig WithMetadata(this RouteConfig config, string key, string value) { + ArgumentNullException.ThrowIfNull(config); + Dictionary metadata; if (config.Metadata != null) @@ -63,6 +77,7 @@ public static class ProxyConfigExtensions /// public static ClusterConfig WithAccessToken(this ClusterConfig config, RequiredTokenType requiredTokenType) { + ArgumentNullException.ThrowIfNull(config); Dictionary metadata; if (config.Metadata != null) diff --git a/bff/src/Bff.Yarp/ProxyFrontendExtensionExtensions.cs b/bff/src/Bff.Yarp/ProxyFrontendExtensionExtensions.cs index 13ccbbb58..45da07fe9 100644 --- a/bff/src/Bff.Yarp/ProxyFrontendExtensionExtensions.cs +++ b/bff/src/Bff.Yarp/ProxyFrontendExtensionExtensions.cs @@ -10,6 +10,8 @@ public static class ProxyFrontendExtensionExtensions { public static BffFrontend WithRemoteApis(this BffFrontend frontend, params RemoteApi[] config) { + ArgumentNullException.ThrowIfNull(frontend); + // Remove existing ProxyFrontendExtension if present var newExtensions = frontend.DataExtensions .Where(e => e is not ProxyBffPlugin) @@ -29,8 +31,13 @@ public static class ProxyFrontendExtensionExtensions /// /// /// - public static RemoteApi[] GetRemoteApis(this BffFrontend frontend) => frontend.DataExtensions - .OfType() - .SelectMany(e => e.RemoteApis) - .ToArray(); + public static RemoteApi[] GetRemoteApis(this BffFrontend frontend) + { + ArgumentNullException.ThrowIfNull(frontend); + + return frontend.DataExtensions + .OfType() + .SelectMany(e => e.RemoteApis) + .ToArray(); + } } diff --git a/bff/src/Bff.Yarp/RouteBuilderExtensions.cs b/bff/src/Bff.Yarp/RouteBuilderExtensions.cs index 297ed1022..cb4d1e672 100644 --- a/bff/src/Bff.Yarp/RouteBuilderExtensions.cs +++ b/bff/src/Bff.Yarp/RouteBuilderExtensions.cs @@ -39,6 +39,8 @@ public static class RouteBuilderExtensions ForwarderRequestConfig? requestConfig = null ) { + ArgumentNullException.ThrowIfNull(endpoints); + ArgumentNullException.ThrowIfNull(apiAddress); endpoints.CheckLicense(); // See if a default request config is registered in DI, otherwise use an empty one diff --git a/bff/src/Bff.Yarp/YarpTransformExtensions.cs b/bff/src/Bff.Yarp/YarpTransformExtensions.cs index 088a8284c..a5c112da8 100644 --- a/bff/src/Bff.Yarp/YarpTransformExtensions.cs +++ b/bff/src/Bff.Yarp/YarpTransformExtensions.cs @@ -22,6 +22,7 @@ public static class YarpTransformExtensions /// public static TransformBuilderContext AddBffAccessToken(this TransformBuilderContext context, PathString localPath) { + ArgumentNullException.ThrowIfNull(context); var proofService = context.Services.GetRequiredService(); var logger = context.Services.GetRequiredService>(); diff --git a/bff/src/Bff/AccessTokenManagement/AccessToken.cs b/bff/src/Bff/AccessTokenManagement/AccessToken.cs index f1862be63..4c1212816 100644 --- a/bff/src/Bff/AccessTokenManagement/AccessToken.cs +++ b/bff/src/Bff/AccessTokenManagement/AccessToken.cs @@ -4,7 +4,6 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using Duende.Bff.Internal; - using AtmAccessToken = Duende.AccessTokenManagement.AccessToken; namespace Duende.Bff.AccessTokenManagement; @@ -20,7 +19,8 @@ public readonly record struct AccessToken : IStronglyTypedValue // Officially, there's no max length for JWTs, but 32k is a good limit public const int MaxLength = 32 * 1024; // 32k - private static readonly ValidationRule[] Validators = [ + private static readonly ValidationRule[] Validators = + [ ValidationRules.MaxLength(MaxLength) ]; @@ -29,13 +29,17 @@ public readonly record struct AccessToken : IStronglyTypedValue /// /// public static implicit operator string(AccessToken value) => value.ToString(); + +#pragma warning disable CA2225 // (OperatorOverloadsHaveNamedAlternates) Intentionally not using named alternative for this, becuase we don't want to expose the ATM type in the BFF API. public static implicit operator AtmAccessToken(AccessToken value) => AtmAccessToken.Parse(value.ToString()); +#pragma warning restore CA2225 /// /// You can't directly create this type. /// /// public AccessToken() => throw new InvalidOperationException("Can't create null value"); + private AccessToken(string value) => Value = value; private string Value { get; } diff --git a/bff/src/Bff/AccessTokenManagement/DPoPProofKey.cs b/bff/src/Bff/AccessTokenManagement/DPoPProofKey.cs index c19e3ec6e..02d23f342 100644 --- a/bff/src/Bff/AccessTokenManagement/DPoPProofKey.cs +++ b/bff/src/Bff/AccessTokenManagement/DPoPProofKey.cs @@ -3,6 +3,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; +using System.Text.Json; using System.Text.Json.Serialization; using Duende.Bff.Internal; using Microsoft.IdentityModel.Tokens; @@ -16,13 +17,15 @@ public readonly record struct DPoPProofKey : IStronglyTypedValue { public bool Equals(DPoPProofKey other) => Value == other.Value; - public override int GetHashCode() => Value.GetHashCode(); + public override int GetHashCode() => Value.GetHashCode(StringComparison.InvariantCulture); /// /// Implicitly converts DPOPProofKey to same type from access token management. /// /// +#pragma warning disable CA2225 // (OperatorOverloadsHaveNamedAlternates) Intentionally not using named alternative for this, becuase we don't want to expose the ATM type in the BFF API. public static implicit operator AtmDPoPProofKey(DPoPProofKey value) => AtmDPoPProofKey.Parse(value.ToString()); +#pragma warning restore CA2225 /// /// Convenience method for converting a into a string. @@ -34,7 +37,8 @@ public readonly record struct DPoPProofKey : IStronglyTypedValue public override string ToString() => Value; - private static readonly ValidationRule[] Validators = [ + private static readonly ValidationRule[] Validators = + [ // Officially, there's no max length for JWTs, but 32k is a good limit ValidationRules.MaxLength(32 * 1024), IsValidJsonWebKey() @@ -49,7 +53,7 @@ public readonly record struct DPoPProofKey : IStronglyTypedValue JsonWebKey.Create(value); return true; } - catch (Exception e) + catch (JsonException e) { message = "String is not a valid json web key: " + e.Message; return false; @@ -61,6 +65,7 @@ public readonly record struct DPoPProofKey : IStronglyTypedValue /// /// public DPoPProofKey() => throw new InvalidOperationException("Can't create null value"); + private DPoPProofKey(string value) { Value = value; @@ -95,5 +100,4 @@ public readonly record struct DPoPProofKey : IStronglyTypedValue /// contain null or whitespace strings. /// public static DPoPProofKey? ParseOrDefault(string? value) => StringParsers.ParseOrDefault(value); - } diff --git a/bff/src/Bff/AccessTokenManagement/Scheme.cs b/bff/src/Bff/AccessTokenManagement/Scheme.cs index ff6d6a3bc..292239136 100644 --- a/bff/src/Bff/AccessTokenManagement/Scheme.cs +++ b/bff/src/Bff/AccessTokenManagement/Scheme.cs @@ -6,7 +6,6 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using Duende.Bff.Internal; using Duende.IdentityModel; - using AtmScheme = Duende.AccessTokenManagement.Scheme; namespace Duende.Bff.AccessTokenManagement; @@ -27,11 +26,15 @@ public readonly record struct Scheme : IStronglyTypedValue /// /// public static implicit operator string(Scheme value) => value.ToString(); + +#pragma warning disable CA2225 // (OperatorOverloadsHaveNamedAlternates) Intentionally not using named alternative for this, becuase we don't want to expose the ATM type in the BFF API. public static implicit operator AtmScheme(Scheme value) => AtmScheme.Parse(value.ToString()); +#pragma warning restore CA2225 public override string ToString() => Value; - private static readonly ValidationRule[] Validators = [ + private static readonly ValidationRule[] Validators = + [ ValidationRules.MaxLength(MaxLength), ]; @@ -46,7 +49,6 @@ public readonly record struct Scheme : IStronglyTypedValue private Scheme(string value) { - // Some target systems are case-sensitive in their scheme handling. This code normalizes // the casing. @@ -56,14 +58,17 @@ public readonly record struct Scheme : IStronglyTypedValue //IE: if Scheme == BeAReR => "Bearer" //IE: if Scheme == DpoP => "DPoP" - if (value.Equals(OidcConstants.AuthenticationSchemes.AuthorizationHeaderBearer, StringComparison.OrdinalIgnoreCase)) + if (value.Equals(OidcConstants.AuthenticationSchemes.AuthorizationHeaderBearer, + StringComparison.OrdinalIgnoreCase)) { value = OidcConstants.AuthenticationSchemes.AuthorizationHeaderBearer; } - else if (value.Equals(OidcConstants.AuthenticationSchemes.AuthorizationHeaderDPoP, StringComparison.OrdinalIgnoreCase)) + else if (value.Equals(OidcConstants.AuthenticationSchemes.AuthorizationHeaderDPoP, + StringComparison.OrdinalIgnoreCase)) { value = OidcConstants.AuthenticationSchemes.AuthorizationHeaderDPoP; } + Value = value; } @@ -87,6 +92,6 @@ public readonly record struct Scheme : IStronglyTypedValue /// Parses a value to a . This will throw an exception if the string is not valid. /// public static Scheme Parse(string value) => StringParsers.Parse(value); - public static Scheme? ParseOrDefault(string? value) => StringParsers.ParseOrDefault(value); + public static Scheme? ParseOrDefault(string? value) => StringParsers.ParseOrDefault(value); } diff --git a/bff/src/Bff/AuthenticationPropertiesExtensions.cs b/bff/src/Bff/AuthenticationPropertiesExtensions.cs index be64a3c4b..ec85f3d8e 100644 --- a/bff/src/Bff/AuthenticationPropertiesExtensions.cs +++ b/bff/src/Bff/AuthenticationPropertiesExtensions.cs @@ -15,7 +15,12 @@ public static class AuthenticationPropertiesExtensions /// /// Determines if this AuthenticationProperties represents a BFF silent login. /// - public static bool IsSilentLogin(this AuthenticationProperties props) => props.TryGetPrompt(out var prompt) && prompt == "none"; + public static bool IsSilentLogin(this AuthenticationProperties props) => + props.TryGetPrompt(out var prompt) && prompt == "none"; - public static bool TryGetPrompt(this AuthenticationProperties props, [NotNullWhen(true)] out string? prompt) => props.Items.TryGetValue(Constants.BffFlags.Prompt, out prompt); + public static bool TryGetPrompt(this AuthenticationProperties props, [NotNullWhen(true)] out string? prompt) + { + ArgumentNullException.ThrowIfNull(props); + return props.Items.TryGetValue(Constants.BffFlags.Prompt, out prompt); + } } diff --git a/bff/src/Bff/AuthenticationTicketExtensions.cs b/bff/src/Bff/AuthenticationTicketExtensions.cs index 2c35eba31..94c07f32a 100644 --- a/bff/src/Bff/AuthenticationTicketExtensions.cs +++ b/bff/src/Bff/AuthenticationTicketExtensions.cs @@ -2,9 +2,11 @@ // See LICENSE in the project root for license information. using System.Security.Claims; +using System.Security.Cryptography; using System.Text.Json; using System.Text.Json.Serialization; using Duende.Bff.Internal; +using Duende.Bff.Otel; using Duende.Bff.SessionManagement.SessionStore; using Duende.IdentityModel; using Microsoft.AspNetCore.Authentication; @@ -29,6 +31,7 @@ public static class AuthenticationTicketExtensions /// public static string GetSubjectId(this AuthenticationTicket ticket) { + ArgumentNullException.ThrowIfNull(ticket); var subjectId = ticket.Principal.FindFirst(JwtClaimTypes.Subject)?.Value ?? ticket.Principal.FindFirst(ClaimTypes.NameIdentifier)?.Value ?? // for the mfa remember me cookie, ASP.NET Identity uses the 'name' claim for the subject id (for some reason) @@ -41,23 +44,37 @@ public static class AuthenticationTicketExtensions /// /// Extracts the session ID /// - public static string? GetSessionId(this AuthenticationTicket ticket) => ticket.Principal.FindFirst(JwtClaimTypes.SessionId)?.Value; + public static string? GetSessionId(this AuthenticationTicket ticket) + { + ArgumentNullException.ThrowIfNull(ticket); + return ticket.Principal.FindFirst(JwtClaimTypes.SessionId)?.Value; + } /// /// Extracts the issuance time /// - public static DateTime GetIssued(this AuthenticationTicket ticket) => ticket.Properties.IssuedUtc?.UtcDateTime ?? DateTime.UtcNow; + public static DateTime GetIssued(this AuthenticationTicket ticket) + { + ArgumentNullException.ThrowIfNull(ticket); + return ticket.Properties.IssuedUtc?.UtcDateTime ?? DateTime.UtcNow; + } /// /// Extracts the expiration time /// - public static DateTime? GetExpiration(this AuthenticationTicket ticket) => ticket.Properties.ExpiresUtc?.UtcDateTime; + public static DateTime? GetExpiration(this AuthenticationTicket ticket) + { + ArgumentNullException.ThrowIfNull(ticket); + return ticket.Properties.ExpiresUtc?.UtcDateTime; + } /// /// Serializes and AuthenticationTicket to a string /// public static string Serialize(this AuthenticationTicket ticket, IDataProtector protector) { + ArgumentNullException.ThrowIfNull(ticket); + ArgumentNullException.ThrowIfNull(protector); var data = new AuthenticationTicketLite { Scheme = ticket.AuthenticationScheme, @@ -79,12 +96,16 @@ public static class AuthenticationTicketExtensions /// public static AuthenticationTicket? Deserialize(this UserSession session, IDataProtector protector, ILogger logger) { + ArgumentNullException.ThrowIfNull(session); + ArgumentNullException.ThrowIfNull(protector); try { var envelope = JsonSerializer.Deserialize(session.Ticket, JsonOptions); if (envelope == null || envelope.Version != 1) { - logger.LogDebug("Deserializing AuthenticationTicket envelope failed or found incorrect version for key {key}.", session.Key); + logger.AuthenticationTicketEnvelopeVersionInvalid( + LogLevel.Debug, + session.Key); return null; } @@ -93,16 +114,16 @@ public static class AuthenticationTicketExtensions { payload = protector.Unprotect(envelope.Payload); } - catch (Exception ex) + catch (CryptographicException ex) { - logger.LogDebug(ex, "Failed to unprotect AuthenticationTicket payload for key {key}", session.Key); + logger.AuthenticationTicketPayloadInvalid(ex, LogLevel.Warning, session.Key); return null; } var ticket = JsonSerializer.Deserialize(payload, JsonOptions); if (ticket == null) { - logger.LogDebug("Deserializing AuthenticationTicket failed for key {key}.", session.Key); + logger.AuthenticationTicketPayloadInvalid(ex: null, LogLevel.Warning, session.Key); return null; } @@ -121,10 +142,10 @@ public static class AuthenticationTicketExtensions return new AuthenticationTicket(user, properties, ticket.Scheme); } - catch (Exception ex) + catch (JsonException ex) { // failed deserialize - logger.LogError(ex, "Failed to deserialize UserSession payload for key {key}", session.Key); + logger.AuthenticationTicketFailedToDeserialize(ex, LogLevel.Warning, session.Key); } return null; diff --git a/bff/src/Bff/BffBuilder.cs b/bff/src/Bff/BffBuilder.cs index bc885b3c8..bc461daa1 100644 --- a/bff/src/Bff/BffBuilder.cs +++ b/bff/src/Bff/BffBuilder.cs @@ -93,10 +93,10 @@ public sealed class BffBuilder(IServiceCollection services) // Add 'default' configure methods that would have been added by // .AddAuthentication().AddCookie().AddOpenIdConnect() - Services.TryAddEnumerable(ServiceDescriptor.Singleton, OpenIdConnectPostConfigureOptions>()); - Services.TryAddEnumerable(ServiceDescriptor.Singleton, PostConfigureCookieAuthenticationOptions>()); - - Services.AddTransient(); + Services.TryAddEnumerable(ServiceDescriptor + .Singleton, OpenIdConnectPostConfigureOptions>()); + Services.TryAddEnumerable(ServiceDescriptor + .Singleton, PostConfigureCookieAuthenticationOptions>()); Services.TryAddSingleton(); @@ -124,7 +124,9 @@ public sealed class BffBuilder(IServiceCollection services) /// public BffBuilder AddServerSideSessions() { - Services.AddSingleton, PostConfigureApplicationCookieTicketStore>(); + Services + .AddSingleton, + PostConfigureApplicationCookieTicketStore>(); Services.AddTransient(); Services.AddTransient(); Services.AddSingleton(); @@ -162,6 +164,7 @@ public sealed class BffBuilder(IServiceCollection services) { configLoader(Services, section); } + // We no longer need them. _pluginConfigurationLoaders.Clear(); @@ -170,6 +173,8 @@ public sealed class BffBuilder(IServiceCollection services) public BffBuilder AddFrontends(params BffFrontend[] frontends) { + ArgumentNullException.ThrowIfNull(frontends); + // Check for duplicate frontend names var duplicateNames = frontends .GroupBy(f => f.Name) @@ -177,9 +182,10 @@ public sealed class BffBuilder(IServiceCollection services) .Select(g => g.Key) .ToList(); - if (duplicateNames.Any()) + if (duplicateNames.Count > 0) { - throw new InvalidOperationException($"Duplicate frontend names detected: {string.Join(", ", duplicateNames.Select(n => n))}"); + throw new InvalidOperationException( + $"Duplicate frontend names detected: {string.Join(", ", duplicateNames.Select(n => n))}"); } foreach (var frontend in frontends) diff --git a/bff/src/Bff/BffEndpointRouteBuilderExtensions.cs b/bff/src/Bff/BffEndpointRouteBuilderExtensions.cs index 329da6e01..5038ecaf9 100644 --- a/bff/src/Bff/BffEndpointRouteBuilderExtensions.cs +++ b/bff/src/Bff/BffEndpointRouteBuilderExtensions.cs @@ -4,6 +4,7 @@ using Duende.Bff.Configuration; using Duende.Bff.Endpoints; using Duende.Bff.Endpoints.Internal; +using Duende.Bff.Otel; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; @@ -35,6 +36,7 @@ public static class BffEndpointRouteBuilderExtensions /// public static void MapBffManagementEndpoints(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); var options = endpoints.ServiceProvider.GetRequiredService>().Value; if (endpoints.AlreadyMappedManagementEndpoint(options.LoginPath, "Login")) { @@ -57,6 +59,7 @@ public static class BffEndpointRouteBuilderExtensions /// public static void MapBffManagementLoginEndpoint(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); endpoints.CheckLicense(); var options = endpoints.ServiceProvider.GetRequiredService>().Value; @@ -66,12 +69,16 @@ public static class BffEndpointRouteBuilderExtensions .AllowAnonymous(); } - internal static bool AlreadyMappedManagementEndpoint(this IEndpointRouteBuilder endpoints, PathString route, string name) + internal static bool AlreadyMappedManagementEndpoint( + this IEndpointRouteBuilder endpoints, + PathString route, + string name) { if (endpoints.DataSources.Any(x => x.Endpoints.OfType().Any(x => x.RoutePattern.RawText == route.ToString()))) { - endpoints.ServiceProvider.GetRequiredService>().LogWarning("Already mapped {name} endpoint, so the call to MapBffManagementEndpoints will be ignored. If you're using BffOptions.AutomaticallyRegisterBffMiddleware, you don't need to call endpoints.MapBffManagementEndpoints()", name); + var logger = endpoints.ServiceProvider.GetRequiredService>(); + logger.AlreadyMappedManagementEndpoint(LogLevel.Warning, name); return true; } @@ -82,9 +89,11 @@ public static class BffEndpointRouteBuilderExtensions /// Adds the silent login BFF management endpoints /// /// - [Obsolete("The silent login endpoint will be removed in a future version. Silent login is now handled by passing the prompt=none parameter to the login endpoint.")] + [Obsolete( + "The silent login endpoint will be removed in a future version. Silent login is now handled by passing the prompt=none parameter to the login endpoint.")] public static void MapBffManagementSilentLoginEndpoints(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); endpoints.CheckLicense(); var options = endpoints.ServiceProvider.GetRequiredService>().Value; @@ -105,6 +114,7 @@ public static class BffEndpointRouteBuilderExtensions /// public static void MapBffManagementLogoutEndpoint(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); endpoints.CheckLicense(); var options = endpoints.ServiceProvider.GetRequiredService>().Value; @@ -121,6 +131,7 @@ public static class BffEndpointRouteBuilderExtensions /// public static void MapBffManagementUserEndpoint(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); endpoints.CheckLicense(); var options = endpoints.ServiceProvider.GetRequiredService>().Value; @@ -136,6 +147,7 @@ public static class BffEndpointRouteBuilderExtensions /// public static void MapBffManagementBackchannelEndpoint(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); endpoints.CheckLicense(); var options = endpoints.ServiceProvider.GetRequiredService>().Value; @@ -150,6 +162,7 @@ public static class BffEndpointRouteBuilderExtensions /// public static void MapBffDiagnosticsEndpoint(this IEndpointRouteBuilder endpoints) { + ArgumentNullException.ThrowIfNull(endpoints); endpoints.CheckLicense(); var options = endpoints.ServiceProvider.GetRequiredService>().Value; diff --git a/bff/src/Bff/BffRemoteApiEndpointExtensions.cs b/bff/src/Bff/BffRemoteApiEndpointExtensions.cs index 2ffa93def..f012623b7 100644 --- a/bff/src/Bff/BffRemoteApiEndpointExtensions.cs +++ b/bff/src/Bff/BffRemoteApiEndpointExtensions.cs @@ -49,6 +49,7 @@ public static class BffRemoteApiEndpointExtensions public static IEndpointConventionBuilder WithAccessTokenRetriever(this IEndpointConventionBuilder builder) where TRetriever : IAccessTokenRetriever { + ArgumentNullException.ThrowIfNull(builder); builder.Add(endpointBuilder => { var metadata = endpointBuilder.GetBffRemoteApiEndpointMetadata(); diff --git a/bff/src/Bff/Configuration/BffOptions.cs b/bff/src/Bff/Configuration/BffOptions.cs index ae3655aae..1dd9d1952 100644 --- a/bff/src/Bff/Configuration/BffOptions.cs +++ b/bff/src/Bff/Configuration/BffOptions.cs @@ -126,7 +126,7 @@ public sealed class BffOptions /// The ASP.NET environment names that enable the diagnostics endpoint. /// Defaults to "Development". /// - public ICollection DiagnosticsEnvironments { get; set; } = new HashSet() + public ICollection DiagnosticsEnvironments { get; } = new HashSet() { Environments.Development }; diff --git a/bff/src/Bff/Configuration/BffRemoteApiEndpointMetadata.cs b/bff/src/Bff/Configuration/BffRemoteApiEndpointMetadata.cs index a7e478da4..451f51edc 100644 --- a/bff/src/Bff/Configuration/BffRemoteApiEndpointMetadata.cs +++ b/bff/src/Bff/Configuration/BffRemoteApiEndpointMetadata.cs @@ -14,7 +14,7 @@ public sealed class BffRemoteApiEndpointMetadata : IBffApiMetadata /// /// Required token type (if any) /// - public RequiredTokenType? TokenType; + public RequiredTokenType? TokenType { get; set; } /// /// Maps to UserAccessTokenParameters and included if set @@ -31,13 +31,15 @@ public sealed class BffRemoteApiEndpointMetadata : IBffApiMetadata get => _accessTokenRetriever; set { + ArgumentNullException.ThrowIfNull(value); if (value.IsAssignableTo(typeof(IAccessTokenRetriever))) { _accessTokenRetriever = value; } else { - throw new Exception("Attempt to assign a AccessTokenRetriever type that cannot be assigned to IAccessTokenTokenRetriever"); + throw new InvalidOperationException( + "Attempt to assign a AccessTokenRetriever type that cannot be assigned to IAccessTokenTokenRetriever"); } } } diff --git a/bff/src/Bff/Configuration/OidcConfiguration.cs b/bff/src/Bff/Configuration/OidcConfiguration.cs index 227da799f..cb6e59d6a 100644 --- a/bff/src/Bff/Configuration/OidcConfiguration.cs +++ b/bff/src/Bff/Configuration/OidcConfiguration.cs @@ -61,7 +61,7 @@ internal sealed record OidcConfiguration { if (Authority != null) { - options.Authority = Authority?.ToString(); + options.Authority = Authority.ToString(); } if (ClientId != null) diff --git a/bff/src/Bff/Constants.cs b/bff/src/Bff/Constants.cs index 2c5d55864..015940fb8 100644 --- a/bff/src/Bff/Constants.cs +++ b/bff/src/Bff/Constants.cs @@ -29,7 +29,9 @@ public static class Constants public const string AntiforgeryCheckMetadata = "Duende.Bff.Yarp.AntiforgeryCheck"; } +#pragma warning disable CA1724 // CA1724: Type names should not match namespaces public static class Cookies +#pragma warning restore CA1724 { public const string HostPrefix = "__Host"; public const string SecurePrefix = "__Secure"; @@ -129,7 +131,7 @@ public static class Constants public const string Prompt = "bff-prompt"; } - public class HttpClientNames + public static class HttpClientNames { public const string IndexHtmlHttpClient = "Duende.Bff.IndexHtmlClient"; diff --git a/bff/src/Bff/DynamicFrontends/BffFrontendExtensions.cs b/bff/src/Bff/DynamicFrontends/BffFrontendExtensions.cs index ed4ba3d79..06dfba889 100644 --- a/bff/src/Bff/DynamicFrontends/BffFrontendExtensions.cs +++ b/bff/src/Bff/DynamicFrontends/BffFrontendExtensions.cs @@ -8,33 +8,54 @@ namespace Duende.Bff.DynamicFrontends; public static class BffFrontendExtensions { - public static BffFrontend WithIndexHtmlUrl(this BffFrontend frontend, Uri? url) => frontend with + public static BffFrontend WithIndexHtmlUrl(this BffFrontend frontend, Uri? url) { - IndexHtmlUrl = url - }; - public static BffFrontend WithOpenIdConnectOptions(this BffFrontend frontend, Action options) => frontend with - { - ConfigureOpenIdConnectOptions = options - }; - - public static BffFrontend WithCookieOptions(this BffFrontend frontend, Action options) => frontend with - { - ConfigureCookieOptions = options - }; - - public static BffFrontend MappedToOrigin(this BffFrontend frontend, Origin origin) => frontend with - { - SelectionCriteria = frontend.SelectionCriteria with + ArgumentNullException.ThrowIfNull(frontend); + return frontend with { - MatchingOrigin = origin - } - }; + IndexHtmlUrl = url + }; + } - public static BffFrontend MappedToPath(this BffFrontend frontend, LocalPath path) => frontend with + public static BffFrontend WithOpenIdConnectOptions(this BffFrontend frontend, Action options) { - SelectionCriteria = frontend.SelectionCriteria with + ArgumentNullException.ThrowIfNull(frontend); + return frontend with { - MatchingPath = path - } - }; + ConfigureOpenIdConnectOptions = options + }; + } + + public static BffFrontend WithCookieOptions(this BffFrontend frontend, Action options) + { + ArgumentNullException.ThrowIfNull(frontend); + return frontend with + { + ConfigureCookieOptions = options + }; + } + + public static BffFrontend MappedToOrigin(this BffFrontend frontend, Origin origin) + { + ArgumentNullException.ThrowIfNull(frontend); + return frontend with + { + SelectionCriteria = frontend.SelectionCriteria with + { + MatchingOrigin = origin + } + }; + } + + public static BffFrontend MappedToPath(this BffFrontend frontend, LocalPath path) + { + ArgumentNullException.ThrowIfNull(frontend); + return frontend with + { + SelectionCriteria = frontend.SelectionCriteria with + { + MatchingPath = path + } + }; + } } diff --git a/bff/src/Bff/DynamicFrontends/FrontendSelectionCriteria.cs b/bff/src/Bff/DynamicFrontends/FrontendSelectionCriteria.cs index 8361a3fac..61d3c2a19 100644 --- a/bff/src/Bff/DynamicFrontends/FrontendSelectionCriteria.cs +++ b/bff/src/Bff/DynamicFrontends/FrontendSelectionCriteria.cs @@ -18,7 +18,7 @@ public sealed record FrontendSelectionCriteria get => _matchingPath; init { - if (value == string.Empty) + if (string.IsNullOrEmpty(value)) { _matchingPath = null; } diff --git a/bff/src/Bff/DynamicFrontends/IFrontendCollection.cs b/bff/src/Bff/DynamicFrontends/IFrontendCollection.cs index 15d7025db..ed4431a5b 100644 --- a/bff/src/Bff/DynamicFrontends/IFrontendCollection.cs +++ b/bff/src/Bff/DynamicFrontends/IFrontendCollection.cs @@ -12,9 +12,10 @@ namespace Duende.Bff.DynamicFrontends; /// - Any changes are not expected to be persisted across application restarts; they are transient and in-memory. /// - This is not an extensibility point and implementors of this library cannot replace it with a different implementation. /// -public interface IFrontendCollection +public interface IFrontendCollection : IEnumerable { void AddOrUpdate(BffFrontend frontend); void Remove(BffFrontendName frontendName); - IReadOnlyList GetAll(); + + public int Count { get; } } diff --git a/bff/src/Bff/DynamicFrontends/Internal/BffAuthenticationSchemeProvider.cs b/bff/src/Bff/DynamicFrontends/Internal/BffAuthenticationSchemeProvider.cs index a93af0e7c..770ec27b6 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/BffAuthenticationSchemeProvider.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/BffAuthenticationSchemeProvider.cs @@ -34,7 +34,7 @@ internal class BffAuthenticationSchemeProvider( return scheme ?? GetBffAuthenticationScheme(name); } - private AuthenticationScheme? GetBffAuthenticationScheme(string name) + private BffAuthenticationScheme? GetBffAuthenticationScheme(string name) { selectedFrontend.TryGet(out var frontend); diff --git a/bff/src/Bff/DynamicFrontends/Internal/FrontendCollection.cs b/bff/src/Bff/DynamicFrontends/Internal/FrontendCollection.cs index c8d128fc8..78d3acbfd 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/FrontendCollection.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/FrontendCollection.cs @@ -1,6 +1,7 @@ // Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. +using System.Collections; using Duende.Bff.Configuration; using Microsoft.Extensions.Options; @@ -26,7 +27,7 @@ internal class FrontendCollection : IDisposable, IFrontendCollection IOptionsMonitor bffConfiguration, IEnumerable plugins, IEnumerable? frontendsConfiguredDuringStartup = null - ) + ) { _plugins = plugins.ToArray(); _frontends = ReadFrontends(bffConfiguration.CurrentValue, frontendsConfiguredDuringStartup ?? []); @@ -83,7 +84,6 @@ internal class FrontendCollection : IDisposable, IFrontendCollection IndexHtmlUrl = frontend.IndexHtmlUrl, ConfigureOpenIdConnectOptions = opt => { - // Then apply any explicitly configured options for the frontend frontend.Oidc?.ApplyTo(opt); }, ConfigureCookieOptions = opt => @@ -93,7 +93,9 @@ internal class FrontendCollection : IDisposable, IFrontendCollection SelectionCriteria = new FrontendSelectionCriteria() { // todo: parse or default - MatchingOrigin = string.IsNullOrEmpty(frontend.MatchingOrigin) ? null : Origin.Parse(frontend.MatchingOrigin), + MatchingOrigin = string.IsNullOrEmpty(frontend.MatchingOrigin) + ? null + : Origin.Parse(frontend.MatchingOrigin), MatchingPath = string.IsNullOrEmpty(frontend.MatchingPath) ? null : frontend.MatchingPath, }, DataExtensions = extensions @@ -161,7 +163,6 @@ internal class FrontendCollection : IDisposable, IFrontendCollection .Where(x => x.Name != frontend.Name) .Append(frontend) .ToArray()); - } // Notify subscribers that a frontend has changed. @@ -192,9 +193,10 @@ internal class FrontendCollection : IDisposable, IFrontendCollection OnFrontendChanged(existing); } - // ReSharper disable once InconsistentlySynchronizedField - // The _frontends array is completely replaced on add/update, so we don't need to lock here. - public IReadOnlyList GetAll() => _frontends.AsReadOnly(); + public int Count => _frontends.Length; void IDisposable.Dispose() => _stopSubscription?.Dispose(); + public IEnumerator GetEnumerator() => ((IEnumerable)_frontends).GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => _frontends.GetEnumerator(); } diff --git a/bff/src/Bff/DynamicFrontends/Internal/FrontendSelector.cs b/bff/src/Bff/DynamicFrontends/Internal/FrontendSelector.cs index 0a7af3fcb..52fb46564 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/FrontendSelector.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/FrontendSelector.cs @@ -8,12 +8,11 @@ using Microsoft.Extensions.Logging; namespace Duende.Bff.DynamicFrontends.Internal; -internal class FrontendSelector(FrontendCollection frontendCollection, ILogger logger) +internal class FrontendSelector(FrontendCollection frontends, ILogger logger) { public bool TrySelectFrontend(HttpRequest request, [NotNullWhen(true)] out BffFrontend? selectedFrontend) { selectedFrontend = null; - var frontends = frontendCollection.GetAll(); if (frontends.Count == 0) { @@ -23,12 +22,14 @@ internal class FrontendSelector(FrontendCollection frontendCollection, ILogger x.SelectionCriteria.MatchingOrigin == null || x.SelectionCriteria.MatchingOrigin.Equals(request)) + .Where(x => x.SelectionCriteria.MatchingOrigin == null || + x.SelectionCriteria.MatchingOrigin.Equals(request)) .ToList(); // First, look for a match by origin and path (if specified) selectedFrontend = matchingByOrigin - .OrderByDescending(x => x.SelectionCriteria.MatchingOrigin != null) // Prefer frontends with a specific origin + .OrderByDescending(x => + x.SelectionCriteria.MatchingOrigin != null) // Prefer frontends with a specific origin .ThenByDescending(x => PathOrder(x.SelectionCriteria.MatchingPath)) .FirstOrDefault(x => x.SelectionCriteria.MatchingPath == null || @@ -42,7 +43,8 @@ internal class FrontendSelector(FrontendCollection frontendCollection, ILogger + private static bool PathMatches(HttpRequest request, PathString path) => request.Path.StartsWithSegments(path, StringComparison.OrdinalIgnoreCase); - private int PathOrder(PathString? path) => + private static int PathOrder(PathString? path) => path?.Value?.Length ?? 0; } diff --git a/bff/src/Bff/DynamicFrontends/Internal/IndexHtmlHttpClient.cs b/bff/src/Bff/DynamicFrontends/Internal/IndexHtmlHttpClient.cs index 76c1afb64..6990f83cf 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/IndexHtmlHttpClient.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/IndexHtmlHttpClient.cs @@ -38,7 +38,6 @@ internal class IndexHtmlHttpClient : IIndexHtmlClient, IAsyncDisposable try { await cache.RemoveAsync(BuildCacheKey(changedFrontend), _stopping.Token); - } catch (OperationCanceledException) { @@ -50,12 +49,10 @@ internal class IndexHtmlHttpClient : IIndexHtmlClient, IAsyncDisposable throw; } }; - } public async Task GetIndexHtmlAsync(CT ct = default) { - if (!_selectedFrontend.TryGet(out var frontend)) { // Todo: log @@ -67,46 +64,50 @@ internal class IndexHtmlHttpClient : IIndexHtmlClient, IAsyncDisposable try { return await _cache.GetOrCreateAsync(cacheKey, async (ct1) => - { - var client = _clientFactory.CreateClient(_options.Value.IndexHtmlClientName ?? Constants.HttpClientNames.IndexHtmlHttpClient); - - var response = await client.GetAsync(frontend.IndexHtmlUrl, ct1); - if (response.StatusCode != HttpStatusCode.OK) { + var client = _clientFactory.CreateClient(_options.Value.IndexHtmlClientName ?? + Constants.HttpClientNames.IndexHtmlHttpClient); + + var response = await client.GetAsync(frontend.IndexHtmlUrl, ct1); + if (response.StatusCode != HttpStatusCode.OK) + { + // Todo: log + throw new PreventCacheException(); + } + // Todo: log - throw new PreventCacheException(); - } - // Todo: log + var html = await response.Content.ReadAsStringAsync(ct1); - var html = await response.Content.ReadAsStringAsync(ct1); + if (_transformer == null) + { + return html; + } - if (_transformer == null) - { - return html; - } - - var transformed = await _transformer.Transform(html, ct1); - return transformed; - - }, + var transformed = await _transformer.Transform(html, ct1); + return transformed; + }, options: new HybridCacheEntryOptions() { Expiration = TimeSpan.FromMinutes(5) }, cancellationToken: ct); - } catch (PreventCacheException) { return null; } - } private static string BuildCacheKey(BffFrontend frontend) => "Duende.Bff.IndexHtml:" + frontend.Name; - private class PreventCacheException : Exception; +#pragma warning disable CA1032 // Do not use a custom message for this exception, as it is used to prevent caching +#pragma warning disable CA1064 // do not make this exception public as it's purely internal + private class PreventCacheException : Exception +#pragma warning restore CA1064 +#pragma warning restore CA1032 + { + } public async ValueTask DisposeAsync() { @@ -114,4 +115,3 @@ internal class IndexHtmlHttpClient : IIndexHtmlClient, IAsyncDisposable _stopping.Dispose(); } } - diff --git a/bff/src/Bff/DynamicFrontends/Internal/OpenIdConnectCallbackMiddleware.cs b/bff/src/Bff/DynamicFrontends/Internal/OpenIdConnectCallbackMiddleware.cs index 45e306762..9d2b8f6ad 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/OpenIdConnectCallbackMiddleware.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/OpenIdConnectCallbackMiddleware.cs @@ -23,7 +23,7 @@ internal class OpenIdConnectCallbackMiddleware(RequestDelegate next, var oidcOptionsFactory = context.RequestServices.GetRequiredService>(); var options = oidcOptionsFactory.Create(frontend.OidcSchemeName); - if (context.Request.Path.StartsWithSegments(options.CallbackPath)) + if (context.Request.Path.StartsWithSegments(options.CallbackPath, StringComparison.OrdinalIgnoreCase)) { var handlers = context.RequestServices.GetRequiredService(); if (await handlers.GetHandlerAsync(context, frontend.OidcSchemeName) is IAuthenticationRequestHandler handler) @@ -32,7 +32,7 @@ internal class OpenIdConnectCallbackMiddleware(RequestDelegate next, return; } } - if (context.Request.Path.StartsWithSegments(options.SignedOutCallbackPath)) + if (context.Request.Path.StartsWithSegments(options.SignedOutCallbackPath, StringComparison.OrdinalIgnoreCase)) { var handlers = context.RequestServices.GetRequiredService(); if (await handlers.GetHandlerAsync(context, frontend.OidcSchemeName) is IAuthenticationRequestHandler handler) diff --git a/bff/src/Bff/DynamicFrontends/Internal/PathMapper.cs b/bff/src/Bff/DynamicFrontends/Internal/PathMapper.cs index c8b88f2bd..b2fe03ec1 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/PathMapper.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/PathMapper.cs @@ -5,9 +5,9 @@ using Microsoft.AspNetCore.Http; namespace Duende.Bff.DynamicFrontends.Internal; -internal class PathMapper +internal static class PathMapper { - public void MapPath(HttpContext context, BffFrontend frontend) + public static void MapPath(HttpContext context, BffFrontend frontend) { var path = frontend.SelectionCriteria.MatchingPath; @@ -16,7 +16,7 @@ internal class PathMapper return; } - if (!context.Request.Path.StartsWithSegments(path)) + if (!context.Request.Path.StartsWithSegments(path, StringComparison.OrdinalIgnoreCase)) { context.Response.StatusCode = 404; return; diff --git a/bff/src/Bff/DynamicFrontends/Internal/PathMappingMiddleware.cs b/bff/src/Bff/DynamicFrontends/Internal/PathMappingMiddleware.cs index 63de6c53b..baa110369 100644 --- a/bff/src/Bff/DynamicFrontends/Internal/PathMappingMiddleware.cs +++ b/bff/src/Bff/DynamicFrontends/Internal/PathMappingMiddleware.cs @@ -5,13 +5,13 @@ using Microsoft.AspNetCore.Http; namespace Duende.Bff.DynamicFrontends.Internal; -internal class PathMappingMiddleware(RequestDelegate next, SelectedFrontend selectedFrontend, PathMapper pathMapper) +internal class PathMappingMiddleware(RequestDelegate next, SelectedFrontend selectedFrontend) { public async Task InvokeAsync(HttpContext context) { if (selectedFrontend.TryGet(out var frontend)) { - pathMapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); } await next(context); diff --git a/bff/src/Bff/DynamicFrontends/Origin.cs b/bff/src/Bff/DynamicFrontends/Origin.cs index acac0edda..616af5b21 100644 --- a/bff/src/Bff/DynamicFrontends/Origin.cs +++ b/bff/src/Bff/DynamicFrontends/Origin.cs @@ -20,12 +20,16 @@ public sealed record Origin : IEquatable return Parse(uri); } - public static Origin Parse(Uri uri) => new() + public static Origin Parse(Uri uri) { - Scheme = uri.Scheme, - Host = uri.Host, - Port = uri.Port - }; + ArgumentNullException.ThrowIfNull(uri); + return new() + { + Scheme = uri.Scheme, + Host = uri.Host, + Port = uri.Port + }; + } public required string Scheme { get; init; } diff --git a/bff/src/Bff/Endpoints/IBffApiMetadata.cs b/bff/src/Bff/Endpoints/IBffApiMetadata.cs index 298ecb3b6..321b2802c 100644 --- a/bff/src/Bff/Endpoints/IBffApiMetadata.cs +++ b/bff/src/Bff/Endpoints/IBffApiMetadata.cs @@ -7,4 +7,6 @@ namespace Duende.Bff.Endpoints; /// Marks an endpoint as BFF API endpoint. /// By default, this provides anti-forgery protection and response handling. /// +#pragma warning disable CA1040 public interface IBffApiMetadata; +#pragma warning restore CA1040 diff --git a/bff/src/Bff/Endpoints/IBffApiSkipAntiforgery.cs b/bff/src/Bff/Endpoints/IBffApiSkipAntiforgery.cs index 6aa9c4690..3487eada4 100644 --- a/bff/src/Bff/Endpoints/IBffApiSkipAntiforgery.cs +++ b/bff/src/Bff/Endpoints/IBffApiSkipAntiforgery.cs @@ -7,4 +7,6 @@ namespace Duende.Bff.Endpoints; /// /// Indicates that the BFF middleware will ignore the antiforgery header checks. /// +#pragma warning disable CA1040 public interface IBffApiSkipAntiForgery; +#pragma warning restore CA1040 diff --git a/bff/src/Bff/Endpoints/IBffApiSkipResponseHandling.cs b/bff/src/Bff/Endpoints/IBffApiSkipResponseHandling.cs index 63bb402b3..7ae1d855f 100644 --- a/bff/src/Bff/Endpoints/IBffApiSkipResponseHandling.cs +++ b/bff/src/Bff/Endpoints/IBffApiSkipResponseHandling.cs @@ -7,4 +7,6 @@ namespace Duende.Bff.Endpoints; /// /// Indicates that the BFF middleware will not override the HTTP response status code. /// +#pragma warning disable CA1040 public interface IBffApiSkipResponseHandling; +#pragma warning restore CA1040 diff --git a/bff/src/Bff/Endpoints/IReturnUrlValidator.cs b/bff/src/Bff/Endpoints/IReturnUrlValidator.cs index 7597279bb..88fff0ead 100644 --- a/bff/src/Bff/Endpoints/IReturnUrlValidator.cs +++ b/bff/src/Bff/Endpoints/IReturnUrlValidator.cs @@ -14,5 +14,5 @@ public interface IReturnUrlValidator /// /// /// - public bool IsValidAsync(string returnUrl); + public bool IsValidAsync(Uri returnUrl); } diff --git a/bff/src/Bff/Endpoints/Internal/BffAntiForgeryMiddleware.cs b/bff/src/Bff/Endpoints/Internal/BffAntiForgeryMiddleware.cs index 2cf49cf0e..665c6bd11 100644 --- a/bff/src/Bff/Endpoints/Internal/BffAntiForgeryMiddleware.cs +++ b/bff/src/Bff/Endpoints/Internal/BffAntiForgeryMiddleware.cs @@ -55,7 +55,7 @@ internal class BffAntiForgeryMiddleware( var isUiEndpoint = endpoint.Metadata.GetMetadata() != null; if (isUiEndpoint && context.IsAjaxRequest()) { - logger.ManagementEndpointAccessedViaAjax(context.Request.Path); + logger.ManagementEndpointAccessedViaAjax(LogLevel.Debug, context.Request.Path.Sanitize()); } await next(context); diff --git a/bff/src/Bff/Endpoints/Internal/BffAuthenticationService.cs b/bff/src/Bff/Endpoints/Internal/BffAuthenticationService.cs index eac23faf5..6807fe29e 100644 --- a/bff/src/Bff/Endpoints/Internal/BffAuthenticationService.cs +++ b/bff/src/Bff/Endpoints/Internal/BffAuthenticationService.cs @@ -32,8 +32,7 @@ internal class BffAuthenticationService(Decorator decora return; } - // Todo: EV: not sure. - logger.LogWarning("Authentiating scheme: {scheme}", scheme); + logger.AuthenticatingScheme(LogLevel.Warning, scheme); await _inner.SignOutAsync(context, frontend.OidcSchemeName, properties); return; } @@ -50,10 +49,7 @@ internal class BffAuthenticationService(Decorator decora return await _inner.AuthenticateAsync(context, scheme); } - // Todo: EV: not sure. - // It looks like all schemes are authentiated, even if we only want the frontend scheme to be triggered. - // Force the cookie scheme to be authentiated. LIkely this means it happens twice - logger.LogWarning("Authentiating scheme: {scheme}", scheme); + logger.AuthenticatingScheme(LogLevel.Warning, scheme); return await _inner.AuthenticateAsync(context, frontend.CookieSchemeName); } @@ -80,7 +76,7 @@ internal class BffAuthenticationService(Decorator decora var requireResponseHandling = endpoint?.Metadata.GetMetadata() == null; if (requireResponseHandling) { - logger.ChallengeForBffApiEndpoint(); + logger.ChallengeForBffApiEndpoint(LogLevel.Debug); context.Response.StatusCode = 401; context.Response.Headers.Remove("Location"); context.Response.Headers.Remove("Set-Cookie"); @@ -107,7 +103,7 @@ internal class BffAuthenticationService(Decorator decora var requireResponseHandling = endpoint?.Metadata.GetMetadata() == null; if (requireResponseHandling) { - logger.ForbidForBffApiEndpoint(); + logger.ForbidForBffApiEndpoint(LogLevel.Debug); context.Response.StatusCode = 403; context.Response.Headers.Remove("Location"); context.Response.Headers.Remove("Set-Cookie"); diff --git a/bff/src/Bff/Endpoints/Internal/BffOpenIdConnectEvents.cs b/bff/src/Bff/Endpoints/Internal/BffOpenIdConnectEvents.cs index 0bd1ec3a7..5f2797008 100644 --- a/bff/src/Bff/Endpoints/Internal/BffOpenIdConnectEvents.cs +++ b/bff/src/Bff/Endpoints/Internal/BffOpenIdConnectEvents.cs @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. using Duende.Bff.Configuration; +using Duende.Bff.Otel; using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -35,12 +36,12 @@ internal class BffOpenIdConnectEvents(IOptions options, ILogger options, ILogger options, ILogger options, ILogger [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] -internal class BffUiEndpointAttribute : Attribute, IBffUIApiEndpoint +internal sealed class BffUiEndpointAttribute : Attribute, IBffUIApiEndpoint { } diff --git a/bff/src/Bff/Endpoints/Internal/DefaultBackchannelLogoutEndpoint.cs b/bff/src/Bff/Endpoints/Internal/DefaultBackchannelLogoutEndpoint.cs index 42e352330..56447292b 100644 --- a/bff/src/Bff/Endpoints/Internal/DefaultBackchannelLogoutEndpoint.cs +++ b/bff/src/Bff/Endpoints/Internal/DefaultBackchannelLogoutEndpoint.cs @@ -29,49 +29,42 @@ internal class DefaultBackchannelLogoutEndpoint( /// public async Task ProcessRequestAsync(HttpContext context, CT ct = default) { - logger.LogDebug("Processing back-channel logout request"); + logger.ProcessingBackChannelLogoutRequest(LogLevel.Debug); context.Response.Headers.Append("Cache-Control", "no-cache, no-store"); context.Response.Headers.Append("Pragma", "no-cache"); - try + if (context.Request.HasFormContentType) { - if (context.Request.HasFormContentType) + var logoutToken = context.Request.Form[OidcConstants.BackChannelLogoutRequest.LogoutToken].FirstOrDefault(); + + if (!string.IsNullOrWhiteSpace(logoutToken)) { - var logoutToken = context.Request.Form[OidcConstants.BackChannelLogoutRequest.LogoutToken].FirstOrDefault(); - - if (!string.IsNullOrWhiteSpace(logoutToken)) + var user = await ValidateLogoutTokenAsync(logoutToken); + if (user != null) { - var user = await ValidateLogoutTokenAsync(logoutToken); - if (user != null) + // these are the sub & sid to signout + var sub = user.FindFirst("sub")?.Value; + var sid = user.FindFirst("sid")?.Value; + + logger.BackChannelLogout(LogLevel.Debug, sub ?? "missing", sid ?? "missing"); + + await userSession.RevokeSessionsAsync(new UserSessionsFilter { - // these are the sub & sid to signout - var sub = user.FindFirst("sub")?.Value; - var sid = user.FindFirst("sid")?.Value; + SubjectId = sub, + SessionId = sid + }, ct); - logger.BackChannelLogout(sub ?? "missing", sid ?? "missing"); - - await userSession.RevokeSessionsAsync(new UserSessionsFilter - { - SubjectId = sub, - SessionId = sid - }, ct); - - return; - } - } - else - { - logger.BackChannelLogoutError($"Failed to process backchannel logout request. 'Logout token is missing'"); + return; } } - } - catch (Exception ex) - { - logger.BackChannelLogoutError($"Failed to process backchannel logout request. '{ex.Message}'"); + else + { + logger.FailedToProcessBackchannelLogoutRequestMissingToken(LogLevel.Information); + } } - logger.BackChannelLogoutError($"Failed to process backchannel logout request."); + logger.FailedToProcessBackchannelLogoutRequest(LogLevel.Information); context.Response.StatusCode = 400; } @@ -85,31 +78,31 @@ internal class DefaultBackchannelLogoutEndpoint( var claims = await ValidateJwt(logoutToken); if (claims == null) { - logger.LogDebug("No claims in back-channel JWT"); + logger.NoClaimsInBackChannelJwt(LogLevel.Debug); return null; } else { - logger.LogTrace("Claims found in back-channel JWT {claims}", claims.Claims); + logger.ClaimsFoundInBackChannelJwt(LogLevel.Trace, string.Join(',', claims.Claims)); } if (claims.FindFirst("sub") == null && claims.FindFirst("sid") == null) { - logger.BackChannelLogoutError("Logout token missing sub and sid claims."); + logger.LogoutTokenMissingSubAndSidClaims(LogLevel.Information); return null; } var nonce = claims.FindFirst("nonce")?.Value; if (!string.IsNullOrWhiteSpace(nonce)) { - logger.BackChannelLogoutError("Logout token should not contain nonce claim."); + logger.LogoutTokenShouldNotContainNonceClaim(LogLevel.Information); return null; } var eventsJson = claims.FindFirst("events")?.Value; if (string.IsNullOrWhiteSpace(eventsJson)) { - logger.BackChannelLogoutError("Logout token missing events claim."); + logger.LogoutTokenMissingEventsClaim(LogLevel.Information); return null; } @@ -118,13 +111,13 @@ internal class DefaultBackchannelLogoutEndpoint( var events = JsonDocument.Parse(eventsJson); if (!events.RootElement.TryGetProperty("http://schemas.openid.net/event/backchannel-logout", out _)) { - logger.BackChannelLogoutError("Logout token contains missing http://schemas.openid.net/event/backchannel-logout value."); + logger.LogoutTokenMissingBackchannelLogoutValue(LogLevel.Information); return null; } } - catch (Exception ex) + catch (JsonException ex) { - logger.BackChannelLogoutError($"Logout token contains invalid JSON in events claim value. '{ex.Message}'"); + logger.LogoutTokenContainsInvalidJsonInEventsClaim(LogLevel.Information, ex); return null; } @@ -144,11 +137,11 @@ internal class DefaultBackchannelLogoutEndpoint( var result = await handler.ValidateTokenAsync(jwt, parameters); if (result.IsValid) { - logger.LogDebug("Back-channel JWT validation successful"); + logger.BackChannelJwtValidationSuccessful(LogLevel.Debug); return result.ClaimsIdentity; } - logger.BackChannelLogoutError($"Error validating logout token. '{result.Exception.ToString()}'"); + logger.ErrorValidatingLogoutToken(LogLevel.Information, result.Exception); return null; } @@ -162,13 +155,13 @@ internal class DefaultBackchannelLogoutEndpoint( var scheme = await authenticationSchemeProvider.GetDefaultChallengeSchemeAsync(); if (scheme == null) { - throw new Exception("Failed to obtain default challenge scheme"); + throw new InvalidOperationException("Failed to obtain default challenge scheme"); } var options = optionsMonitor.Get(scheme.Name); if (options == null) { - throw new Exception("Failed to obtain OIDC options for default challenge scheme"); + throw new InvalidOperationException("Failed to obtain OIDC options for default challenge scheme"); } var config = options.Configuration; @@ -179,7 +172,7 @@ internal class DefaultBackchannelLogoutEndpoint( if (config == null) { - throw new Exception("Failed to obtain OIDC configuration"); + throw new InvalidOperationException("Failed to obtain OIDC configuration"); } var parameters = new TokenValidationParameters diff --git a/bff/src/Bff/Endpoints/Internal/DefaultLoginEndpoint.cs b/bff/src/Bff/Endpoints/Internal/DefaultLoginEndpoint.cs index 881f2cf3e..f62148825 100644 --- a/bff/src/Bff/Endpoints/Internal/DefaultLoginEndpoint.cs +++ b/bff/src/Bff/Endpoints/Internal/DefaultLoginEndpoint.cs @@ -29,7 +29,7 @@ internal class DefaultLoginEndpoint( /// public async Task ProcessRequestAsync(HttpContext context, CT ct = default) { - logger.LogDebug("Processing login request"); + logger.ProcessingLoginRequest(LogLevel.Debug); context.CheckForBffMiddleware(bffOptions.Value); @@ -54,7 +54,8 @@ internal class DefaultLoginEndpoint( if (!string.IsNullOrWhiteSpace(returnUrl)) { - if (!returnUrlValidator.IsValidAsync(returnUrl)) + if (!Uri.TryCreate(returnUrl, UriKind.RelativeOrAbsolute, out var returnUri) + || !returnUrlValidator.IsValidAsync(returnUri)) { logger.InvalidReturnUrl(LogLevel.Information, returnUrl.Sanitize()); context.ReturnHttpProblem("Invalid return url", (Constants.RequestParameters.ReturnUrl, [$"ReturnUrl '{returnUrl}' was invalid"])); @@ -79,7 +80,7 @@ internal class DefaultLoginEndpoint( props.Items.Add(Constants.BffFlags.Prompt, prompt); } - logger.LogDebug("Login endpoint triggering Challenge with returnUrl {returnUrl}", returnUrl.Sanitize()); + logger.LoginEndpointTriggeringChallenge(LogLevel.Debug, returnUrl.Sanitize()); await context.ChallengeAsync(props); } @@ -106,7 +107,7 @@ internal class DefaultLoginEndpoint( var openIdConnectOptions = openIdConnectOptionsMonitor.Get(scheme); if (openIdConnectOptions == null) { - throw new Exception("Failed to obtain OIDC options for scheme: " + scheme); + throw new InvalidOperationException("Failed to obtain OIDC options for scheme: " + scheme); } var config = openIdConnectOptions.Configuration; diff --git a/bff/src/Bff/Endpoints/Internal/DefaultLogoutEndpoint.cs b/bff/src/Bff/Endpoints/Internal/DefaultLogoutEndpoint.cs index b0e15fc62..05a2496af 100644 --- a/bff/src/Bff/Endpoints/Internal/DefaultLogoutEndpoint.cs +++ b/bff/src/Bff/Endpoints/Internal/DefaultLogoutEndpoint.cs @@ -24,7 +24,7 @@ internal class DefaultLogoutEndpoint(IOptions options, /// public async Task ProcessRequestAsync(HttpContext context, CT ct = default) { - logger.LogDebug("Processing logout request"); + logger.ProcessingLogoutRequest(LogLevel.Debug); context.CheckForBffMiddleware(options.Value); @@ -50,7 +50,8 @@ internal class DefaultLogoutEndpoint(IOptions options, var returnUrl = context.Request.Query[Constants.RequestParameters.ReturnUrl].FirstOrDefault(); if (!string.IsNullOrWhiteSpace(returnUrl)) { - if (!returnUrlValidator.IsValidAsync(returnUrl)) + if (!Uri.TryCreate(returnUrl, UriKind.RelativeOrAbsolute, out var returnUri) || + !returnUrlValidator.IsValidAsync(returnUri)) { logger.InvalidReturnUrl(LogLevel.Information, returnUrl.Sanitize()); context.ReturnHttpProblem("Invalid return url", (Constants.RequestParameters.ReturnUrl, [$"ReturnUrl '{returnUrl}' was invalid"])); @@ -79,7 +80,7 @@ internal class DefaultLogoutEndpoint(IOptions options, RedirectUri = returnUrl }; - logger.LogDebug("Logout endpoint triggering SignOut with returnUrl {returnUrl}", returnUrl.Sanitize()); + logger.LogoutEndpointTriggeringSignOut(LogLevel.Debug, returnUrl.Sanitize()); // trigger idp logout await context.SignOutAsync(props); diff --git a/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginCallbackEndpoint.cs b/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginCallbackEndpoint.cs index b7ddb299f..87db18b57 100644 --- a/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginCallbackEndpoint.cs +++ b/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginCallbackEndpoint.cs @@ -23,7 +23,7 @@ internal class DefaultSilentLoginCallbackEndpoint( /// public async Task ProcessRequestAsync(HttpContext context, CancellationToken cancellationToken = default) { - logger.LogDebug("Processing silent login callback request"); + logger.ProcessingSilentLoginCallbackRequest(LogLevel.Debug); context.CheckForBffMiddleware(options.Value); @@ -42,7 +42,7 @@ internal class DefaultSilentLoginCallbackEndpoint( context.Response.Headers["Cache-Control"] = "no-store, no-cache, max-age=0"; context.Response.Headers["Pragma"] = "no-cache"; - logger.LogDebug("Silent login endpoint rendering HTML with JS postMessage to origin {origin} with isLoggedIn {isLoggedIn}", origin.Sanitize(), result); + logger.SilentLoginEndpointRenderingHtml(LogLevel.Debug, origin.Sanitize(), result); await context.Response.WriteAsync(html, Encoding.UTF8, cancellationToken); } diff --git a/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginEndpoint.cs b/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginEndpoint.cs index 2481bee8a..9284d1038 100644 --- a/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginEndpoint.cs +++ b/bff/src/Bff/Endpoints/Internal/DefaultSilentLoginEndpoint.cs @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. using Duende.Bff.Configuration; +using Duende.Bff.Otel; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -22,7 +23,7 @@ internal class DefaultSilentLoginEndpoint(IOptions options, ILogger< /// public async Task ProcessRequestAsync(HttpContext context, CT ct = default) { - logger.LogDebug("Processing silent login request"); + logger.ProcessingSilentLoginRequest(LogLevel.Debug); context.CheckForBffMiddleware(_options); @@ -34,7 +35,7 @@ internal class DefaultSilentLoginEndpoint(IOptions options, ILogger< }, }; - logger.LogWarning("Using deprecated silentlogin endpoint. This endpoint will be removed in future versions. Consider calling the BFF Login endpoint with prompt=none."); + logger.UsingDeprecatedSilentLoginEndpoint(LogLevel.Warning); await context.ChallengeAsync(props); } diff --git a/bff/src/Bff/Endpoints/Internal/DefaultUserEndpoint.cs b/bff/src/Bff/Endpoints/Internal/DefaultUserEndpoint.cs index efe51b3fb..3b6f70a04 100644 --- a/bff/src/Bff/Endpoints/Internal/DefaultUserEndpoint.cs +++ b/bff/src/Bff/Endpoints/Internal/DefaultUserEndpoint.cs @@ -5,6 +5,7 @@ using System.Text; using System.Text.Json; using Duende.Bff.Configuration; using Duende.Bff.Internal; +using Duende.Bff.Otel; using Duende.IdentityModel; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; @@ -24,11 +25,10 @@ internal class DefaultUserEndpoint(IOptions options, ILogger private readonly BffOptions _options = options.Value; - /// public async Task ProcessRequestAsync(HttpContext context, CT ct = default) { - logger.LogDebug("Processing user request"); + logger.ProcessingUserRequest(LogLevel.Debug); context.CheckForBffMiddleware(_options); @@ -47,7 +47,7 @@ internal class DefaultUserEndpoint(IOptions options, ILogger options, ILogger options, ILogger /// - private Task> GetUserClaimsAsync(AuthenticateResult authenticateResult, CT ct = default) => + private static Task> GetUserClaimsAsync(AuthenticateResult authenticateResult, CT ct = default) => Task.FromResult(authenticateResult.Principal?.Claims.Select(x => new ClaimRecord(x.Type, x.Value)) ?? Enumerable.Empty()); /// diff --git a/bff/src/Bff/Internal/IStronglyTypedValue.cs b/bff/src/Bff/Internal/IStronglyTypedValue.cs index 4f7a3904b..5437d21d5 100644 --- a/bff/src/Bff/Internal/IStronglyTypedValue.cs +++ b/bff/src/Bff/Internal/IStronglyTypedValue.cs @@ -58,13 +58,13 @@ internal interface IStronglyTypedValue : IParsableType } } - if (!errors.Any()) + if (errors.Count == 0) { parsed = TSelf.Create(value); } foundErrors = errors.ToArray(); - return !foundErrors.Any(); + return foundErrors.Length == 0; } } @@ -118,13 +118,13 @@ internal interface IStronglyTypedValue : IParsableType } } - if (!errors.Any()) + if (errors.Count == 0) { parsed = TSelf.Create(converted); } foundErrors = errors.ToArray(); - return !foundErrors.Any(); + return foundErrors.Length == 0; } } diff --git a/bff/src/Bff/Internal/LocalUrlReturnUrlValidator.cs b/bff/src/Bff/Internal/LocalUrlReturnUrlValidator.cs index 976b2b52d..39faa477f 100644 --- a/bff/src/Bff/Internal/LocalUrlReturnUrlValidator.cs +++ b/bff/src/Bff/Internal/LocalUrlReturnUrlValidator.cs @@ -8,7 +8,9 @@ namespace Duende.Bff.Internal; internal class LocalUrlReturnUrlValidator : IReturnUrlValidator { /// - public bool IsValidAsync(string returnUrl) => IsLocalUrl(returnUrl); +#pragma warning disable CA1822 // Can't be marked as static, because it implements an interface method. + public bool IsValidAsync(Uri returnUrl) => IsLocalUrl(returnUrl.ToString()); +#pragma warning restore CA1822 internal static bool IsLocalUrl(string url) { diff --git a/bff/src/Bff/Licensing/License.cs b/bff/src/Bff/Licensing/License.cs index 0f099ae24..414147ad9 100644 --- a/bff/src/Bff/Licensing/License.cs +++ b/bff/src/Bff/Licensing/License.cs @@ -34,7 +34,7 @@ internal class License var edition = claims.FindFirst("edition")?.Value; if (!Enum.TryParse(edition, true, out var editionValue)) { - throw new Exception($"Invalid edition in license: '{edition}'"); + throw new InvalidOperationException($"Invalid edition in license: '{edition}'"); } Edition = editionValue; @@ -43,12 +43,12 @@ internal class License if (IsCommunityEdition && RedistributionFeature) { - throw new Exception("Invalid License: Redistribution is not valid for community edition."); + throw new InvalidOperationException("Invalid License: Redistribution is not valid for community edition."); } if (IsBffEdition && RedistributionFeature) { - throw new Exception("Invalid License: Redistribution is not valid for BFF edition."); + throw new InvalidOperationException("Invalid License: Redistribution is not valid for BFF edition."); } if (IsBffEdition) diff --git a/bff/src/Bff/Licensing/LicenseValidator.cs b/bff/src/Bff/Licensing/LicenseValidator.cs index 8d3abc09b..ddebfb475 100644 --- a/bff/src/Bff/Licensing/LicenseValidator.cs +++ b/bff/src/Bff/Licensing/LicenseValidator.cs @@ -6,10 +6,15 @@ using System.Security.Claims; using System.Security.Cryptography; +using Duende.Bff.Otel; using Microsoft.Extensions.Logging; using Microsoft.IdentityModel.JsonWebTokens; using Microsoft.IdentityModel.Tokens; +// Logging APIs used by Duende license validation +#pragma warning disable CA1848 +#pragma warning disable CA2254 + namespace Duende.Bff.Licensing; // shared APIs needed for Duende license validation @@ -68,20 +73,15 @@ internal partial class LicenseValidator { if (Logger == null) { - throw new Exception("LicenseValidator.Initalize has not yet been called."); + throw new InvalidOperationException("LicenseValidator.Initalize has not yet been called."); } var errors = new List(); if (License == null) { - var message = "You do not have a valid license key for the Duende software. " + - "This is allowed for development and testing scenarios. " + - "If you are running in production you are required to have a licensed version. " + - "Please start a conversation with us: https://duendesoftware.com/contact"; - // we're not using our _warningLog because we always want this emitted regardless of the context - Logger.LogWarning(message); + Logger.NoValidLicense(LogLevel.Warning); LicenseValidator.WarnForProductFeaturesWhenMissingLicense(); return; } @@ -149,7 +149,11 @@ internal partial class LicenseValidator ValidIssuer = "https://duendesoftware.com", ValidAudience = "IdentityServer", IssuerSigningKey = key, + // CA5404: Do not set ValidateLifetime to false in production code + // this is OK for the license key validation, as we do not want to enforce expiration on the license key itself +#pragma warning disable CA5404 ValidateLifetime = false +#pragma warning restore CA5404 }; var validateResult = handler.ValidateTokenAsync(licenseKey, parms).Result; if (validateResult.IsValid) @@ -158,13 +162,14 @@ internal partial class LicenseValidator } else { - Logger.LogCritical(validateResult.Exception, "Error validating the Duende software license key"); + Logger.ErrorValidatingLicenseKey(LogLevel.Critical, validateResult.Exception); } } return null; } + private static void LogToTrace(string message, params object[] args) { if (Logger.IsEnabled(LogLevel.Trace)) @@ -204,4 +209,5 @@ internal partial class LicenseValidator Logger.LogError(message, args); } } + } diff --git a/bff/src/Bff/LocalPath.cs b/bff/src/Bff/LocalPath.cs index 95654fb99..11cbdfa1d 100644 --- a/bff/src/Bff/LocalPath.cs +++ b/bff/src/Bff/LocalPath.cs @@ -20,7 +20,7 @@ public readonly record struct LocalPath : IStronglyTypedValue /// over the conversion process, please use or . /// /// - public static implicit operator LocalPath(PathString value) => Parse(value); + public static implicit operator LocalPath(PathString value) => ToLocalPath(value); /// /// Convenience method for converting a into a string. @@ -50,11 +50,14 @@ public readonly record struct LocalPath : IStronglyTypedValue public static bool TryParse(string value, [NotNullWhen(true)] out LocalPath? parsed, out string[] errors) => IStronglyTypedValue.TryBuildValidatedObject(value, Validators, out parsed, out errors); + public static LocalPath ToLocalPath(PathString pathString) => Parse(pathString); + /// /// Parses a value to a . This will throw an exception if the string is not valid. /// public static LocalPath Parse(string value) => StringParsers.Parse(value); static LocalPath IStronglyTypedValue.Create(string result) => new(result); + } diff --git a/bff/src/Bff/Otel/BffMetrics.cs b/bff/src/Bff/Otel/BffMetrics.cs index 039b5a189..c5e6dbadb 100644 --- a/bff/src/Bff/Otel/BffMetrics.cs +++ b/bff/src/Bff/Otel/BffMetrics.cs @@ -5,19 +5,20 @@ using System.Diagnostics.Metrics; namespace Duende.Bff.Otel; -public sealed class BffMetrics +public sealed class BffMetrics : IDisposable { public const string MeterName = "Duende.Bff"; private readonly Counter _sessionStarted; private readonly Counter _sessionEnded; + private Meter _meter; public BffMetrics(IMeterFactory meterFactory) { - var meter = meterFactory.Create(MeterName); + _meter = meterFactory.Create(MeterName); - _sessionStarted = meter.CreateCounter("session.started", "count", "Number of sessions started"); - _sessionEnded = meter.CreateCounter("session.ended", "count", "Number of sessions ended"); + _sessionStarted = _meter.CreateCounter("session.started", "count", "Number of sessions started"); + _sessionEnded = _meter.CreateCounter("session.ended", "count", "Number of sessions ended"); } public void SessionStarted() => _sessionStarted.Add(1); @@ -25,4 +26,6 @@ public sealed class BffMetrics public void SessionEnded() => _sessionEnded.Add(1); public void SessionsEnded(int count) => _sessionEnded.Add(count); + + public void Dispose() => _meter.Dispose(); } diff --git a/bff/src/Bff/Otel/LogMessages.cs b/bff/src/Bff/Otel/LogMessages.cs index 04cb88684..3c9145367 100644 --- a/bff/src/Bff/Otel/LogMessages.cs +++ b/bff/src/Bff/Otel/LogMessages.cs @@ -10,9 +10,32 @@ namespace Duende.Bff.Otel; internal static partial class LogMessages { + [LoggerMessage( + Message = $"Proxy response error. local path: '{{{OTelParameters.LocalPath}}}', error: '{{{OTelParameters.Error}}}'")] + public static partial void ProxyResponseError(this ILogger logger, LogLevel level, string localPath, string error); + + [LoggerMessage( + message: + $"Deserializing AuthenticationTicket envelope failed or found incorrect version for key {{{OTelParameters.Key}}}")] + public static partial void AuthenticationTicketEnvelopeVersionInvalid(this ILogger logger, LogLevel logLevel, + string key); + + [LoggerMessage( + message: + $"Failed to unprotect AuthenticationTicket payload for key {{{OTelParameters.Key}}}")] + public static partial void AuthenticationTicketPayloadInvalid(this ILogger logger, Exception? ex, LogLevel logLevel, + string key); + + [LoggerMessage( + message: + $"Failed to deserialize AuthenticationTicket payload for key {{{OTelParameters.Key}}}")] + public static partial void AuthenticationTicketFailedToDeserialize(this ILogger logger, Exception? ex, + LogLevel logLevel, + string key); + [LoggerMessage( Message = "FrontendSelection: No frontends registered in the store.")] - public static partial void NoFrontendsRegistered(ILogger logger, LogLevel logLevel); + public static partial void NoFrontendsRegistered(this ILogger logger, LogLevel logLevel); [LoggerMessage( $"Invalid prompt value {{{OTelParameters.Prompt}}}.")] @@ -29,11 +52,13 @@ internal static partial class LogMessages [LoggerMessage( $"Failed To clear IndexHtmlCache for BFF Frontend {{{OTelParameters.Frontend}}}")] - public static partial void FailedToClearIndexHtmlCacheForFrontend(this ILogger logger, LogLevel logLevel, Exception ex, BffFrontendName frontend); + public static partial void FailedToClearIndexHtmlCacheForFrontend(this ILogger logger, LogLevel logLevel, + Exception ex, BffFrontendName frontend); [LoggerMessage( $"No OpenID Configuration found for scheme {{{OTelParameters.Scheme}}}")] - public static partial void NoOpenIdConfigurationFoundForScheme(this ILogger logger, LogLevel logLevel, Scheme scheme); + public static partial void NoOpenIdConfigurationFoundForScheme(this ILogger logger, LogLevel logLevel, + Scheme scheme); [LoggerMessage( $"No frontend selected.")] @@ -49,166 +74,325 @@ internal static partial class LogMessages public static partial void AntiForgeryValidationFailed(this ILogger logger, string localPath); [LoggerMessage( - level: LogLevel.Debug, message: $"Back-channel logout. sub: '{{{OTelParameters.Sub}}}', sid: '{{{OTelParameters.Sid}}}'")] - public static partial void BackChannelLogout(this ILogger logger, string sub, string sid); + public static partial void BackChannelLogout(this ILogger logger, LogLevel logLevel, string sub, string sid); + [LoggerMessage( - level: LogLevel.Warning, - message: $"Back-channel logout error. error: '{{{OTelParameters.Error}}}'")] - public static partial void BackChannelLogoutError(this ILogger logger, string error); + message: + $"Access token is missing. token type: '{{{OTelParameters.TokenType}}}', local path: '{{{OTelParameters.LocalPath}}}', detail: '{{{OTelParameters.Detail}}}'")] + public static partial void AccessTokenMissing(this ILogger logger, LogLevel logLevel, string tokenType, + string localPath, string detail); [LoggerMessage( - message: $"Access token is missing. token type: '{{{OTelParameters.TokenType}}}', local path: '{{{OTelParameters.LocalPath}}}', detail: '{{{OTelParameters.Detail}}}'")] - public static partial void AccessTokenMissing(this ILogger logger, LogLevel logLevel, string tokenType, string localPath, string detail); + message: + $"Invalid route configuration. Cannot combine a required access token (a call to WithAccessToken) and an optional access token (a call to WithOptionalUserAccessToken). clusterId: '{{{OTelParameters.ClusterId}}}', routeId: '{{{OTelParameters.RouteId}}}'")] + public static partial void InvalidRouteConfiguration(this ILogger logger, LogLevel logLevel, string? clusterId, string routeId); [LoggerMessage( - level: LogLevel.Warning, - message: $"Invalid route configuration. Cannot combine a required access token (a call to WithAccessToken) and an optional access token (a call to WithOptionalUserAccessToken). clusterId: '{{{OTelParameters.ClusterId}}}', routeId: '{{{OTelParameters.RouteId}}}'")] - public static partial void InvalidRouteConfiguration(this ILogger logger, string? clusterId, string routeId); + message: + $"Failed to request new User Access Token due to: {{{OTelParameters.Error}}}. This can mean that the refresh token is expired or revoked but the cookie session is still active. If the session was not revoked, ensure that the session cookie lifetime is smaller than the refresh token lifetime.")] + public static partial void FailedToRequestNewUserAccessToken(this ILogger logger, LogLevel logLevel, string error); [LoggerMessage( - level: LogLevel.Warning, - message: $"Failed to request new User Access Token due to: {{{OTelParameters.Error}}}. This can mean that the refresh token is expired or revoked but the cookie session is still active. If the session was not revoked, ensure that the session cookie lifetime is smaller than the refresh token lifetime.")] - public static partial void FailedToRequestNewUserAccessToken(this ILogger logger, string error); + message: + $"Failed to request new User Access Token due to: {{{OTelParameters.Error}}}. This likely means that the user's refresh token is expired or revoked. The user's session will be ended, which will force the user to log in.")] + public static partial void UserSessionRevoked(this ILogger logger, LogLevel logLevel, string error); [LoggerMessage( - level: LogLevel.Warning, - message: $"Failed to request new User Access Token due to: {{{OTelParameters.Error}}}. This likely means that the user's refresh token is expired or revoked. The user's session will be ended, which will force the user to log in.")] - public static partial void UserSessionRevoked(this ILogger logger, string error); + message: + $"BFF management endpoint {{endpoint}} is only intended for a browser window to request and load. It is not intended to be accessed with Ajax or fetch requests.")] + public static partial void ManagementEndpointAccessedViaAjax(this ILogger logger, LogLevel logLevel, string endpoint); [LoggerMessage( - level: LogLevel.Debug, - message: $"BFF management endpoint {{endpoint}} is only intended for a browser window to request and load. It is not intended to be accessed with Ajax or fetch requests.")] - public static partial void ManagementEndpointAccessedViaAjax(this ILogger logger, string endpoint); - - [LoggerMessage( - level: LogLevel.Debug, message: $"Challenge was called for a BFF API endpoint, BFF response handling changing status code to 401.")] - public static partial void ChallengeForBffApiEndpoint(this ILogger logger); + public static partial void ChallengeForBffApiEndpoint(this ILogger logger, LogLevel logLevel); [LoggerMessage( - level: LogLevel.Debug, message: $"Forbid was called for a BFF API endpoint, BFF response handling changing status code to 403.")] - public static partial void ForbidForBffApiEndpoint(this ILogger logger); + public static partial void ForbidForBffApiEndpoint(this ILogger logger, LogLevel logLevel); [LoggerMessage( - level: LogLevel.Debug, - message: $"Creating user session record in store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void CreatingUserSession(this ILogger logger, string sub, string? sid); + message: + $"Creating user session record in store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void CreatingUserSession(this ILogger logger, LogLevel logLevel, string sub, string? sid); [LoggerMessage( - level: LogLevel.Debug, - message: $"Detected a duplicate insert of the same session. This can happen when multiple browser tabs are open and can safely be ignored.")] - public static partial void DuplicateSessionInsertDetected(this ILogger logger, Exception ex); + message: + $"Detected a duplicate insert of the same session. This can happen when multiple browser tabs are open and can safely be ignored.")] + public static partial void DuplicateSessionInsertDetected(this ILogger logger, LogLevel logLevel, Exception ex); [LoggerMessage( - level: LogLevel.Warning, - message: $"Exception creating new server-side session in database: {{{OTelParameters.Error}}}. If this is a duplicate key error, it's safe to ignore. This can happen (for example) when two identical tabs are open.")] - public static partial void ExceptionCreatingSession(this ILogger logger, Exception ex, string error); + message: + $"Exception creating new server-side session in database: {{{OTelParameters.Error}}}. If this is a duplicate key error, it's safe to ignore. This can happen (for example) when two identical tabs are open.")] + public static partial void ExceptionCreatingSession(this ILogger logger, LogLevel logLevel, Exception ex, string error); [LoggerMessage( - level: LogLevel.Debug, - message: $"No record found in user session store when trying to delete user session for key {{{OTelParameters.Key}}}")] - public static partial void NoRecordFoundForKey(this ILogger logger, string key); + message: + $"No record found in user session store when trying to delete user session for key {{{OTelParameters.Key}}}")] + public static partial void NoRecordFoundForKey(this ILogger logger, LogLevel logLevel, string key); [LoggerMessage( - level: LogLevel.Debug, - message: $"Deleting user session record in store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void DeletingUserSession(this ILogger logger, string sub, string? sid); + message: + $"Deleting user session record in store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void DeletingUserSession(this ILogger logger, LogLevel logLevel, string sub, string? sid); [LoggerMessage( - level: LogLevel.Debug, message: $"DbUpdateConcurrencyException: {{{OTelParameters.Error}}}")] - public static partial void DbUpdateConcurrencyException(this ILogger logger, string error); + public static partial void DbUpdateConcurrencyException(this ILogger logger, LogLevel logLevel, string error); [LoggerMessage( - level: LogLevel.Debug, - message: $"Getting user session record from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void GettingUserSession(this ILogger logger, string sub, string? sid); + message: + $"Getting user session record from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void GettingUserSession(this ILogger logger, LogLevel logLevel, string sub, string? sid); [LoggerMessage( - level: LogLevel.Debug, - message: $"Getting {{{OTelParameters.Count}}} user session(s) from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void GettingUserSessions(this ILogger logger, int count, string? sub, string? sid); + message: + $"Getting {{{OTelParameters.Count}}} user session(s) from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void GettingUserSessions(this ILogger logger, LogLevel logLevel, int count, string? sub, string? sid); [LoggerMessage( - level: LogLevel.Debug, - message: $"Deleting {{{OTelParameters.Count}}} user session(s) from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void DeletingUserSessions(this ILogger logger, int count, string? sub, string? sid); + message: + $"Deleting {{{OTelParameters.Count}}} user session(s) from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void DeletingUserSessions(this ILogger logger, LogLevel logLevel, int count, string? sub, string? sid); [LoggerMessage( - level: LogLevel.Debug, - message: $"Updating user session record in store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void UpdatingUserSession(this ILogger logger, string? sub, string? sid); + message: + $"Updating user session record in store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void UpdatingUserSession(this ILogger logger, LogLevel logLevel, string? sub, string? sid); [LoggerMessage( - level: LogLevel.Debug, message: $"Removing {{{OTelParameters.Count}}} server side sessions")] - public static partial void RemovingServerSideSessions(this ILogger logger, int count); + public static partial void RemovingServerSideSessions(this ILogger logger, LogLevel logLevel, int count); [LoggerMessage( - level: LogLevel.Debug, message: $"Retrieving token for user {{{OTelParameters.User}}}")] - public static partial void RetrievingTokenForUser(this ILogger logger, string? user); + public static partial void RetrievingTokenForUser(this ILogger logger, LogLevel logLevel, string? user); [LoggerMessage( - level: LogLevel.Debug, message: $"Retrieving session {{{OTelParameters.Sid}}} for sub {{{OTelParameters.Sub}}}")] - public static partial void RetrievingSession(this ILogger logger, string sid, string sub); + public static partial void RetrievingSession(this ILogger logger, LogLevel logLevel, string sid, string sub); [LoggerMessage( - level: LogLevel.Debug, message: $"Storing token for user {{{OTelParameters.User}}}")] - public static partial void StoringTokenForUser(this ILogger logger, string? user); + public static partial void StoringTokenForUser(this ILogger logger, LogLevel logLevel, string? user); [LoggerMessage( - level: LogLevel.Debug, message: $"Removing token for user {{{OTelParameters.User}}}")] - public static partial void RemovingTokenForUser(this ILogger logger, string? user); + public static partial void RemovingTokenForUser(this ILogger logger, LogLevel logLevel, string? user); [LoggerMessage( - level: LogLevel.Debug, message: $"Failed to find a session to update, bailing out")] - public static partial void FailedToFindSessionToUpdate(this ILogger logger); + public static partial void FailedToFindSessionToUpdate(this ILogger logger, LogLevel logLevel); [LoggerMessage( - level: LogLevel.Debug, - message: $"Creating entry in store for AuthenticationTicket, key {{{OTelParameters.Key}}}, with expiration: {{{OTelParameters.Expiration}}}")] - public static partial void CreatingAuthenticationTicketEntry(this ILogger logger, string key, DateTime? expiration); + message: + $"Creating entry in store for AuthenticationTicket, key {{{OTelParameters.Key}}}, with expiration: {{{OTelParameters.Expiration}}}")] + public static partial void CreatingAuthenticationTicketEntry(this ILogger logger, LogLevel logLevel, string key, DateTime? expiration); [LoggerMessage( - level: LogLevel.Debug, message: $"Retrieve AuthenticationTicket for key {{{OTelParameters.Key}}}")] - public static partial void RetrieveAuthenticationTicket(this ILogger logger, string key); + public static partial void RetrieveAuthenticationTicket(this ILogger logger, LogLevel logLevel, string key); + + [LoggerMessage( + message: $"Ticket loaded for key: {{{OTelParameters.Key}}}, with expiration: {{{OTelParameters.Expiration}}}")] + public static partial void TicketLoaded(this ILogger logger, LogLevel logLevel, string key, DateTime? expiration); [LoggerMessage( - level: LogLevel.Debug, message: $"No AuthenticationTicket found in store for {{{OTelParameters.Key}}}")] - public static partial void NoAuthenticationTicketFoundForKey(this ILogger logger, string key); + public static partial void NoAuthenticationTicketFoundForKey(this ILogger logger, LogLevel logLevel, string key); [LoggerMessage( - level: LogLevel.Warning, - message: $"Failed to deserialize authentication ticket from store, deleting record for key {{{OTelParameters.Key}}}")] - public static partial void FailedToDeserializeAuthenticationTicket(this ILogger logger, string key); + message: + $"Failed to deserialize authentication ticket from store, deleting record for key {{{OTelParameters.Key}}}")] + public static partial void FailedToDeserializeAuthenticationTicket(this ILogger logger, LogLevel logLevel, string key); [LoggerMessage( - level: LogLevel.Debug, - message: $"Renewing AuthenticationTicket for key {{{OTelParameters.Key}}}, with expiration: {{{OTelParameters.Expiration}}}")] - public static partial void RenewingAuthenticationTicket(this ILogger logger, string key, DateTime? expiration); + message: + $"Renewing AuthenticationTicket for key {{{OTelParameters.Key}}}, with expiration: {{{OTelParameters.Expiration}}}")] + public static partial void RenewingAuthenticationTicket(this ILogger logger, LogLevel logLevel, string key, DateTime? expiration); [LoggerMessage( - level: LogLevel.Debug, message: $"Removing AuthenticationTicket from store for key {{{OTelParameters.Key}}}")] - public static partial void RemovingAuthenticationTicket(this ILogger logger, string key); + public static partial void RemovingAuthenticationTicket(this ILogger logger, LogLevel logLevel, string key); [LoggerMessage( - level: LogLevel.Debug, - message: $"Getting AuthenticationTickets from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] - public static partial void GettingAuthenticationTickets(this ILogger logger, string? sub, string? sid); + message: + $"Getting AuthenticationTickets from store for sub {{{OTelParameters.Sub}}} sid {{{OTelParameters.Sid}}}")] + public static partial void GettingAuthenticationTickets(this ILogger logger, LogLevel logLevel, string? sub, string? sid); + [LoggerMessage( - message: $"Frontend selected via path mapping '{{{OTelParameters.PathMapping}}}', but request path '{{{OTelParameters.LocalPath}}}' has different case. Cookie path names are case sensitive, so the cookie likely doesn't work.")] - public static partial void FrontendSelectedWithPathCasingIssue(this ILogger logger, LogLevel level, string pathMapping, LocalPath localPath); + message: + $"Frontend selected via path mapping '{{{OTelParameters.PathMapping}}}', but request path '{{{OTelParameters.LocalPath}}}' has different case. Cookie path names are case sensitive, so the cookie likely doesn't work.")] + public static partial void FrontendSelectedWithPathCasingIssue(this ILogger logger, LogLevel logLevel, + string pathMapping, LocalPath localPath); + + [LoggerMessage( + message: + $"Already mapped {{{OTelParameters.Name}}} endpoint, so the call to MapBffManagementEndpoints will be ignored. If you're using BffOptions.AutomaticallyRegisterBffMiddleware, you don't need to call endpoints.MapBffManagementEndpoints()")] + public static partial void AlreadyMappedManagementEndpoint(this ILogger logger, LogLevel logLevel, string name); + + [LoggerMessage( + message: "Authenticating scheme: {Scheme}")] + public static partial void AuthenticatingScheme(this ILogger logger, LogLevel logLevel, string? scheme); + + [LoggerMessage( + message: "Setting OIDC ProtocolMessage.Prompt to 'none' for BFF silent login")] + public static partial void SettingOidcPromptNoneForSilentLogin(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Setting OIDC ProtocolMessage.Prompt to {Prompt} for BFF silent login")] + public static partial void SettingOidcPromptForSilentLogin(this ILogger logger, LogLevel logLevel, string prompt); + + [LoggerMessage( + message: "Handling error response from OIDC provider for BFF silent login.")] + public static partial void HandlingErrorResponseFromOidcProviderForSilentLogin(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Handling failed response from OIDC provider for BFF silent login.")] + public static partial void HandlingFailedResponseFromOidcProviderForSilentLogin(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Processing back-channel logout request")] + public static partial void ProcessingBackChannelLogoutRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "No claims in back-channel JWT")] + public static partial void NoClaimsInBackChannelJwt(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Claims found in back-channel JWT {Claims}")] + public static partial void ClaimsFoundInBackChannelJwt(this ILogger logger, LogLevel logLevel, string claims); + + [LoggerMessage( + message: "Back-channel JWT validation successful")] + public static partial void BackChannelJwtValidationSuccessful(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Processing login request")] + public static partial void ProcessingLoginRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Login endpoint triggering Challenge with returnUrl {ReturnUrl}")] + public static partial void LoginEndpointTriggeringChallenge(this ILogger logger, LogLevel logLevel, string returnUrl); + + [LoggerMessage( + message: "Processing logout request")] + public static partial void ProcessingLogoutRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Logout endpoint triggering SignOut with returnUrl {ReturnUrl}")] + public static partial void LogoutEndpointTriggeringSignOut(this ILogger logger, LogLevel logLevel, string returnUrl); + + [LoggerMessage( + message: "Processing silent login callback request")] + public static partial void ProcessingSilentLoginCallbackRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Silent login endpoint rendering HTML with JS postMessage to origin {Origin} with isLoggedIn {IsLoggedIn}")] + public static partial void SilentLoginEndpointRenderingHtml(this ILogger logger, LogLevel logLevel, string origin, string isLoggedIn); + + [LoggerMessage( + message: "Processing silent login request")] + public static partial void ProcessingSilentLoginRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Using deprecated silentlogin endpoint. This endpoint will be removed in future versions. Consider calling the BFF Login endpoint with prompt=none.")] + public static partial void UsingDeprecatedSilentLoginEndpoint(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Processing user request")] + public static partial void ProcessingUserRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "User endpoint indicates the user is not logged in, using status code {StatusCode}")] + public static partial void UserEndpointNotLoggedIn(this ILogger logger, LogLevel logLevel, int statusCode); + + [LoggerMessage( + message: "User endpoint indicates the user is logged in with claims {Claims}")] + public static partial void UserEndpointLoggedInWithClaims(this ILogger logger, LogLevel logLevel, string claims); + + [LoggerMessage( + message: "Nop implementation of session revocation for sub: {Sub}, and sid: {Sid}. Implement ISessionRevocationService to provide your own implementation.")] + public static partial void NopSessionRevocation(this ILogger logger, LogLevel logLevel, string? sub, string? sid); + + [LoggerMessage( + message: "Revoking sessions for sub {Sub} and sid {Sid}")] + public static partial void RevokingSessions(this ILogger logger, LogLevel logLevel, string? sub, string? sid); + + [LoggerMessage( + message: "Refresh token revoked for sub {Sub} and sid {Sid}")] + public static partial void RefreshTokenRevoked(this ILogger logger, LogLevel logLevel, string sub, string? sid); + + [LoggerMessage( + message: "BFF session cleanup is enabled, but no IUserSessionStoreCleanup is registered in DI. BFF session cleanup will not run.")] + public static partial void SessionCleanupNotRegistered(this ILogger logger, LogLevel logLevel); + + + [LoggerMessage( + message: "Failed to cleanup session")] + public static partial void FailedToCleanupSession(this ILogger logger, LogLevel logLevel, Exception ex); + + [LoggerMessage( + message: "Failed to cleanup expired sessions")] + public static partial void FailedToCleanupExpiredSessions(this ILogger logger, LogLevel logLevel, Exception ex); + + [LoggerMessage( + message: "Revoking user's refresh tokens in OnSigningOut for subject id: {Sub}")] + public static partial void RevokingUserRefreshTokensOnSigningOut(this ILogger logger, LogLevel logLevel, string? sub); + + [LoggerMessage( + message: "Explicitly setting ShouldRenew=false in OnValidatePrincipal due to query param suppressing slide behavior.")] + public static partial void SuppressingSlideBehaviorOnValidatePrincipal(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Explicitly setting ShouldRenew=false in OnCheckSlidingExpiration due to query param suppressing slide behavior.")] + public static partial void SuppressingSlideBehaviorOnCheckSlidingExpiration(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Failed to process backchannel logout request. 'Logout token is missing'")] + public static partial void FailedToProcessBackchannelLogoutRequestMissingToken(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Failed to process backchannel logout request.")] + public static partial void FailedToProcessBackchannelLogoutRequest(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Logout token missing sub and sid claims.")] + public static partial void LogoutTokenMissingSubAndSidClaims(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Logout token should not contain nonce claim.")] + public static partial void LogoutTokenShouldNotContainNonceClaim(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Logout token missing events claim.")] + public static partial void LogoutTokenMissingEventsClaim(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Logout token contains missing http://schemas.openid.net/event/backchannel-logout value.")] + public static partial void LogoutTokenMissingBackchannelLogoutValue(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Logout token contains invalid JSON in events claim value.")] + public static partial void LogoutTokenContainsInvalidJsonInEventsClaim(this ILogger logger, LogLevel logLevel, Exception ex); + + [LoggerMessage( + message: "Error validating logout token.")] + public static partial void ErrorValidatingLogoutToken(this ILogger logger, LogLevel logLevel, Exception ex); + + [LoggerMessage( + message: "You do not have a valid license key for the Duende software. " + + "This is allowed for development and testing scenarios. " + + "If you are running in production you are required to have a licensed version. " + + "Please start a conversation with us: https://duendesoftware.com/contact")] + public static partial void NoValidLicense(this ILogger logger, LogLevel logLevel); + + [LoggerMessage( + message: "Error validating the license key" + + "If you are running in production you are required to have a licensed version. " + + "Please start a conversation with us: https://duendesoftware.com/contact")] + public static partial void ErrorValidatingLicenseKey(this ILogger logger, LogLevel logLevel, Exception ex); public static string Sanitize(this string toSanitize) => toSanitize.ReplaceLineEndings(string.Empty); diff --git a/bff/src/Bff/Otel/OTelParameters.cs b/bff/src/Bff/Otel/OTelParameters.cs index c9a976d6b..a3c3e93d3 100644 --- a/bff/src/Bff/Otel/OTelParameters.cs +++ b/bff/src/Bff/Otel/OTelParameters.cs @@ -23,4 +23,8 @@ internal class OTelParameters public const string Scheme = "Scheme"; public const string Prompt = "Prompt"; public const string PathMapping = "PathMapping"; + public const string Name = "Name"; + public const string Claims = "Claims"; + public const string Origin = "Origin"; + public const string IsLoggedIn = "IsLoggedIn"; } diff --git a/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationCookieRevokeRefreshToken.cs b/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationCookieRevokeRefreshToken.cs index ad4784cf3..d3e55ee3f 100644 --- a/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationCookieRevokeRefreshToken.cs +++ b/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationCookieRevokeRefreshToken.cs @@ -5,6 +5,7 @@ using Duende.AccessTokenManagement.OpenIdConnect; using Duende.Bff.AccessTokenManagement; using Duende.Bff.Configuration; using Duende.Bff.Internal; +using Duende.Bff.Otel; using Duende.IdentityModel; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.Extensions.Logging; @@ -43,7 +44,7 @@ internal class PostConfigureApplicationCookieRevokeRefreshToken( { // Todo: Ev: logging with sourcegens // todo: ev: should we have userparameters here? - logger.LogDebug("Revoking user's refresh tokens in OnSigningOut for subject id: {subjectId}", ctx.HttpContext.User.FindFirst(JwtClaimTypes.Subject)?.Value); + logger.RevokingUserRefreshTokensOnSigningOut(LogLevel.Debug, ctx.HttpContext.User.FindFirst(JwtClaimTypes.Subject)?.Value); await ctx.HttpContext.RevokeRefreshTokenAsync(ct: ctx.HttpContext.RequestAborted); await inner.Invoke(ctx); diff --git a/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationValidatePrincipal.cs b/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationValidatePrincipal.cs index 0bc043e1e..ecf3bfae1 100644 --- a/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationValidatePrincipal.cs +++ b/bff/src/Bff/SessionManagement/Configuration/PostConfigureApplicationValidatePrincipal.cs @@ -4,6 +4,7 @@ using Duende.Bff.AccessTokenManagement; using Duende.Bff.Configuration; using Duende.Bff.Internal; +using Duende.Bff.Otel; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -44,7 +45,7 @@ internal class PostConfigureApplicationValidatePrincipal( var slide = ctx.Request.Query[Constants.RequestParameters.SlideCookie]; if (slide == "false") { - logger.LogDebug("Explicitly setting ShouldRenew=false in OnValidatePrincipal due to query param suppressing slide behavior."); + logger.SuppressingSlideBehaviorOnValidatePrincipal(LogLevel.Debug); ctx.ShouldRenew = false; } } diff --git a/bff/src/Bff/SessionManagement/Configuration/PostConfigureSlidingExpirationCheck.cs b/bff/src/Bff/SessionManagement/Configuration/PostConfigureSlidingExpirationCheck.cs index 26f6c6d06..305f44c0e 100644 --- a/bff/src/Bff/SessionManagement/Configuration/PostConfigureSlidingExpirationCheck.cs +++ b/bff/src/Bff/SessionManagement/Configuration/PostConfigureSlidingExpirationCheck.cs @@ -4,7 +4,7 @@ using Duende.Bff.AccessTokenManagement; using Duende.Bff.Configuration; using Duende.Bff.Internal; -using Microsoft.AspNetCore.Authentication; +using Duende.Bff.Otel; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -17,12 +17,10 @@ namespace Duende.Bff.SessionManagement.Configuration; internal class PostConfigureSlidingExpirationCheck( ActiveCookieAuthenticationScheme activeCookieScheme, IOptions bffOptions, - IOptions authOptions, ILogger logger) : IPostConfigureOptions { private readonly BffOptions _options = bffOptions.Value; - private readonly string? _scheme = authOptions.Value.DefaultAuthenticateScheme ?? authOptions.Value.DefaultScheme; /// public void PostConfigure(string? name, CookieAuthenticationOptions options) @@ -47,7 +45,7 @@ internal class PostConfigureSlidingExpirationCheck( var slide = ctx.Request.Query[Constants.RequestParameters.SlideCookie]; if (slide == "false") { - logger.LogDebug("Explicitly setting ShouldRenew=false in OnCheckSlidingExpiration due to query param suppressing slide behavior."); + logger.SuppressingSlideBehaviorOnCheckSlidingExpiration(LogLevel.Debug); ctx.ShouldRenew = false; } } diff --git a/bff/src/Bff/SessionManagement/Revocation/NopSessionRevocationService.cs b/bff/src/Bff/SessionManagement/Revocation/NopSessionRevocationService.cs index 62bbe998a..87a632758 100644 --- a/bff/src/Bff/SessionManagement/Revocation/NopSessionRevocationService.cs +++ b/bff/src/Bff/SessionManagement/Revocation/NopSessionRevocationService.cs @@ -1,6 +1,7 @@ // Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. +using Duende.Bff.Otel; using Duende.Bff.SessionManagement.SessionStore; using Microsoft.Extensions.Logging; @@ -14,7 +15,7 @@ internal class NopSessionRevocationService(ILogger /// public Task RevokeSessionsAsync(UserSessionsFilter filter, CT ct = default) { - logger.LogDebug("Nop implementation of session revocation for sub: {sub}, and sid: {sid}. Implement ISessionRevocationService to provide your own implementation.", filter.SubjectId, filter.SessionId); + logger.NopSessionRevocation(LogLevel.Debug, filter.SubjectId, filter.SessionId); return Task.CompletedTask; } } diff --git a/bff/src/Bff/SessionManagement/Revocation/SessionRevocationService.cs b/bff/src/Bff/SessionManagement/Revocation/SessionRevocationService.cs index 56e1254b5..b87a00034 100644 --- a/bff/src/Bff/SessionManagement/Revocation/SessionRevocationService.cs +++ b/bff/src/Bff/SessionManagement/Revocation/SessionRevocationService.cs @@ -4,6 +4,7 @@ using Duende.AccessTokenManagement; using Duende.AccessTokenManagement.OpenIdConnect; using Duende.Bff.Configuration; +using Duende.Bff.Otel; using Duende.Bff.SessionManagement.SessionStore; using Duende.Bff.SessionManagement.TicketStore; using Microsoft.AspNetCore.Authentication; @@ -32,7 +33,7 @@ internal class SessionRevocationService( filter.SessionId = null; } - logger.LogDebug("Revoking sessions for sub {sub} and sid {sid}", filter.SubjectId, filter.SessionId); + logger.RevokingSessions(LogLevel.Debug, filter.SubjectId, filter.SessionId); if (_options.RevokeRefreshTokenOnLogout) { @@ -46,7 +47,7 @@ internal class SessionRevocationService( new UserRefreshToken(RefreshToken.Parse(refreshToken), options.Value.DPoPJsonWebKey), new UserTokenRequestParameters(), ct); - logger.LogDebug("Refresh token revoked for sub {sub} and sid {sid}", ticket.GetSubjectId(), ticket.GetSessionId()); + logger.RefreshTokenRevoked(LogLevel.Debug, ticket.GetSubjectId(), ticket.GetSessionId()); } } } diff --git a/bff/src/Bff/SessionManagement/SessionStore/InMemoryUserSessionStore.cs b/bff/src/Bff/SessionManagement/SessionStore/InMemoryUserSessionStore.cs index 17b055b5c..17186d6fd 100644 --- a/bff/src/Bff/SessionManagement/SessionStore/InMemoryUserSessionStore.cs +++ b/bff/src/Bff/SessionManagement/SessionStore/InMemoryUserSessionStore.cs @@ -17,7 +17,7 @@ internal class InMemoryUserSessionStore : IUserSessionStore { if (!_store.TryAdd(session.Key, session.Clone())) { - throw new Exception("Key already exists"); + throw new InvalidOperationException("Key already exists"); } return Task.CompletedTask; diff --git a/bff/src/Bff/SessionManagement/SessionStore/SessionCleanupHost.cs b/bff/src/Bff/SessionManagement/SessionStore/SessionCleanupHost.cs index 4aa5ab28f..cdf78ba79 100644 --- a/bff/src/Bff/SessionManagement/SessionStore/SessionCleanupHost.cs +++ b/bff/src/Bff/SessionManagement/SessionStore/SessionCleanupHost.cs @@ -17,66 +17,29 @@ internal class SessionCleanupHost( BffMetrics metrics, IServiceProvider serviceProvider, IOptions options, - ILogger logger) : IHostedService + ILogger logger) : BackgroundService { private readonly BffOptions _options = options.Value; private TimeSpan CleanupInterval => _options.SessionCleanupInterval; - private CancellationTokenSource? _source; - - /// - /// Starts the token cleanup polling. - /// - public Task StartAsync(CT ct) + protected override async Task ExecuteAsync(CT ct) { - if (_options.EnableSessionCleanup) + if (!_options.EnableSessionCleanup) { - if (_source != null) - { - throw new InvalidOperationException("Already started. Call Stop first."); - } - - if (IsIUserSessionStoreCleanupRegistered()) - { - logger.LogDebug("Starting BFF session cleanup"); - - _source = CancellationTokenSource.CreateLinkedTokenSource(ct); - - Task.Factory.StartNew(() => StartInternalAsync(_source.Token)); - } - else - { - logger.LogWarning("BFF session cleanup is enabled, but no IUserSessionStoreCleanup is registered in DI. BFF session cleanup will not run."); - } + return; } - return Task.CompletedTask; - } - - /// - /// Stops the token cleanup polling. - /// - public Task StopAsync(CT ct) - { - if (_options.EnableSessionCleanup && _source != null) + if (!IsIUserSessionStoreCleanupRegistered()) { - logger.LogDebug("Stopping BFF session cleanup"); - - _source.Cancel(); - _source = null; + logger.SessionCleanupNotRegistered(LogLevel.Warning); + return; } - return Task.CompletedTask; - } - - private async Task StartInternalAsync(CT ct) - { while (true) { if (ct.IsCancellationRequested) { - logger.LogDebug("CancellationRequested. Exiting."); break; } @@ -86,18 +49,20 @@ internal class SessionCleanupHost( } catch (TaskCanceledException) { - logger.LogDebug("TaskCanceledException. Exiting."); break; } + +#pragma warning disable CA1031// Do not catch general exception types + // Catching general exceptions here to prevent the host from crashing if an exception occurs during the delay. catch (Exception ex) +#pragma warning restore CA1031 { - logger.LogError("Task.Delay exception: {0}. Exiting.", ex.Message); + logger.FailedToCleanupSession(LogLevel.Error, ex); break; } if (ct.IsCancellationRequested) { - logger.LogDebug("CancellationRequested. Exiting."); break; } @@ -114,9 +79,12 @@ internal class SessionCleanupHost( var removed = await tokenCleanupService.DeleteExpiredSessionsAsync(ct); metrics.SessionsEnded(removed); } +#pragma warning disable CA1031// Do not catch general exception types + // Catching general exceptions here to prevent the host from crashing if an exception occurs during the cleanup. catch (Exception ex) +#pragma warning restore CA1031 { - logger.LogError("Exception deleting expired sessions: {exception}", ex.Message); + logger.FailedToCleanupExpiredSessions(LogLevel.Error, ex); } } diff --git a/bff/src/Bff/SessionManagement/SessionStore/UserSession.cs b/bff/src/Bff/SessionManagement/SessionStore/UserSession.cs index 6edb9ef61..16a2bc56b 100644 --- a/bff/src/Bff/SessionManagement/SessionStore/UserSession.cs +++ b/bff/src/Bff/SessionManagement/SessionStore/UserSession.cs @@ -32,6 +32,7 @@ public class UserSession : UserSessionUpdate /// public void CopyTo(UserSession other) { + ArgumentNullException.ThrowIfNull(other); other.Key = Key; base.CopyTo(other); } diff --git a/bff/src/Bff/SessionManagement/SessionStore/UserSessionUpdate.cs b/bff/src/Bff/SessionManagement/SessionStore/UserSessionUpdate.cs index 8b4577fa0..0dcd7c580 100644 --- a/bff/src/Bff/SessionManagement/SessionStore/UserSessionUpdate.cs +++ b/bff/src/Bff/SessionManagement/SessionStore/UserSessionUpdate.cs @@ -46,6 +46,7 @@ public class UserSessionUpdate /// public void CopyTo(UserSessionUpdate other) { + ArgumentNullException.ThrowIfNull(other); other.SubjectId = SubjectId; other.SessionId = SessionId; other.Created = Created; diff --git a/bff/src/Bff/SessionManagement/TicketStore/ServerSideTicketStore.cs b/bff/src/Bff/SessionManagement/TicketStore/ServerSideTicketStore.cs index 8b4fd7047..488c9f9d9 100644 --- a/bff/src/Bff/SessionManagement/TicketStore/ServerSideTicketStore.cs +++ b/bff/src/Bff/SessionManagement/TicketStore/ServerSideTicketStore.cs @@ -48,7 +48,7 @@ internal class ServerSideTicketStore( private async Task CreateNewSessionAsync(string key, AuthenticationTicket ticket) { - LogMessages.CreatingAuthenticationTicketEntry(logger, key, ticket.GetExpiration()); + logger.CreatingAuthenticationTicketEntry(LogLevel.Debug, key, ticket.GetExpiration()); var session = new UserSession { @@ -68,24 +68,24 @@ internal class ServerSideTicketStore( /// public async Task RetrieveAsync(string key) { - LogMessages.RetrieveAuthenticationTicket(logger, key); + logger.RetrieveAuthenticationTicket(LogLevel.Debug, key); var session = await store.GetUserSessionAsync(key); if (session == null) { - LogMessages.NoAuthenticationTicketFoundForKey(logger, key); + logger.NoAuthenticationTicketFoundForKey(LogLevel.Debug, key); return null; } var ticket = session.Deserialize(_protector, logger); if (ticket != null) { - logger.LogDebug("Ticket loaded for key: {key}, with expiration: {expiration}", key, ticket.GetExpiration()); + logger.TicketLoaded(LogLevel.Debug, key, ticket.GetExpiration()); return ticket; } // if we failed to get a ticket, then remove DB record - LogMessages.FailedToDeserializeAuthenticationTicket(logger, key); + logger.FailedToDeserializeAuthenticationTicket(LogLevel.Information, key); await RemoveAsync(key); return ticket; } @@ -101,7 +101,7 @@ internal class ServerSideTicketStore( return; } - LogMessages.RenewingAuthenticationTicket(logger, key, ticket.GetExpiration()); + logger.RenewingAuthenticationTicket(LogLevel.Debug, key, ticket.GetExpiration()); var sub = ticket.GetSubjectId(); var sid = ticket.GetSessionId(); @@ -122,7 +122,7 @@ internal class ServerSideTicketStore( /// public Task RemoveAsync(string key) { - LogMessages.RemovingAuthenticationTicket(logger, key); + logger.RemovingAuthenticationTicket(LogLevel.Debug, key); metrics.SessionEnded(); return store.DeleteUserSessionAsync(key); @@ -131,7 +131,7 @@ internal class ServerSideTicketStore( /// public async Task> GetUserTicketsAsync(UserSessionsFilter filter, CT ct) { - LogMessages.GettingAuthenticationTickets(logger, filter.SubjectId, filter.SessionId); + logger.GettingAuthenticationTickets(LogLevel.Debug, filter.SubjectId, filter.SessionId); var list = new List(); @@ -146,7 +146,7 @@ internal class ServerSideTicketStore( else { // if we failed to get a ticket, then remove DB record - LogMessages.FailedToDeserializeAuthenticationTicket(logger, session.Key); + logger.FailedToDeserializeAuthenticationTicket(LogLevel.Debug, session.Key); await RemoveAsync(session.Key); } } diff --git a/bff/src/Directory.Build.props b/bff/src/Directory.Build.props index 4e92caa26..33fd4a123 100644 --- a/bff/src/Directory.Build.props +++ b/bff/src/Directory.Build.props @@ -4,6 +4,7 @@ + All enable OAuth 2.0;OpenID Connect;Security;BFF;IdentityServer;ASP.NET Core;SPA;Blazor Duende BFF @@ -14,4 +15,12 @@ true + + + $(NoWarn);CA1034; CA2007; + + diff --git a/bff/test/Bff.Tests/BffFrontendSigninTests.cs b/bff/test/Bff.Tests/BffFrontendSigninTests.cs index 54237e9c4..8aab9f010 100644 --- a/bff/test/Bff.Tests/BffFrontendSigninTests.cs +++ b/bff/test/Bff.Tests/BffFrontendSigninTests.cs @@ -14,18 +14,18 @@ public class BffFrontendSigninTests : BffTestBase { public BffFrontendSigninTests(ITestOutputHelper output) : base(output) => Bff.OnConfigureEndpoints += endpoints => + { + endpoints.MapGet("/secret", (HttpContext c) => { - endpoints.MapGet("/secret", (HttpContext c) => + if (!c.User.IsAuthenticated()) { - if (!c.User.IsAuthenticated()) - { - c.Response.StatusCode = 401; - return ""; - } - + c.Response.StatusCode = 401; return ""; - }); - }; + } + + return ""; + }); + }; [Fact] @@ -121,14 +121,12 @@ public class BffFrontendSigninTests : BffTestBase } [Fact] - public async Task given_path_based_frontend_login_endpoint_should_challenge_and_redirect_to_root_with_custom_prefix() + public async Task + given_path_based_frontend_login_endpoint_should_challenge_and_redirect_to_root_with_custom_prefix() { Bff.OnConfigureServices += svcs => { - svcs.Configure(options => - { - options.ManagementBasePath = "/custom/bff"; - }); + svcs.Configure(options => { options.ManagementBasePath = "/custom/bff"; }); }; await InitializeAsync(); @@ -149,7 +147,6 @@ public class BffFrontendSigninTests : BffTestBase var response = await Bff.BrowserClient.Login("/somepath/custom"); response.RequestMessage!.RequestUri.ShouldBe(Bff.Url("/somepath")); - } [Fact] @@ -245,18 +242,15 @@ public class BffFrontendSigninTests : BffTestBase // it to a wrong value and see if it throws. AddOrUpdateFrontend(bffFrontend with { - ConfigureCookieOptions = opt => - { - opt.Cookie.Name = "my_custom_cookie_name"; - } + ConfigureCookieOptions = opt => { opt.Cookie.Name = "my_custom_cookie_name"; } }); await Bff.BrowserClient.Login(); Bff.BrowserClient.Cookies.GetCookies(Bff.Url()) .ShouldContain(c => c.Name == "my_custom_cookie_name" && c.HttpOnly && c.Secure && c.Path == "/"); - } + [Fact] public async Task Default_settings_augment_frontend_settings() { @@ -299,31 +293,29 @@ public class BffFrontendSigninTests : BffTestBase var onTokenValidatedInvoked = false; Bff.EnableBackChannelHandler = false; - Bff.SetBffOptions += options => - { - options.ConfigureOpenIdConnectDefaults = (oidc => - { - oidc.Authority = IdentityServer.Url().ToString(); - - oidc.ClientId = "some_frontend"; - oidc.ClientSecret = DefaultOidcClient.ClientSecret; - oidc.ResponseType = DefaultOidcClient.ResponseType; - oidc.ResponseMode = DefaultOidcClient.ResponseMode; - - oidc.MapInboundClaims = false; - oidc.GetClaimsFromUserInfoEndpoint = true; - oidc.SaveTokens = true; - oidc.BackchannelHttpHandler = Internet; - oidc.Events.OnTokenValidated += c => - { - onTokenValidatedInvoked = true; - return Task.CompletedTask; - }; - }); - }; await InitializeAsync(); + Bff.BffOptions.ConfigureOpenIdConnectDefaults = (oidc => + { + oidc.Authority = IdentityServer.Url().ToString(); + + oidc.ClientId = "some_frontend"; + oidc.ClientSecret = DefaultOidcClient.ClientSecret; + oidc.ResponseType = DefaultOidcClient.ResponseType; + oidc.ResponseMode = DefaultOidcClient.ResponseMode; + + oidc.MapInboundClaims = false; + oidc.GetClaimsFromUserInfoEndpoint = true; + oidc.SaveTokens = true; + oidc.BackchannelHttpHandler = Internet; + oidc.Events.OnTokenValidated += c => + { + onTokenValidatedInvoked = true; + return Task.CompletedTask; + }; + }); + AddOrUpdateFrontend(new BffFrontend() { Name = BffFrontendName.Parse("some_frontend") @@ -392,16 +384,9 @@ public class BffFrontendSigninTests : BffTestBase AddOrUpdateFrontend(new BffFrontend() { Name = BffFrontendName.Parse("some_frontend"), - ConfigureOpenIdConnectOptions = opt => - { - - The.DefaultOpenIdConnectConfiguration(opt); - } + ConfigureOpenIdConnectOptions = opt => { The.DefaultOpenIdConnectConfiguration(opt); } }); onTokenValidatedInvoked.ShouldBeFalse(); } - } - - diff --git a/bff/test/Bff.Tests/BffWithoutExplicitFrontendTests.cs b/bff/test/Bff.Tests/BffWithoutExplicitFrontendTests.cs index c08e1be97..2df7c70a8 100644 --- a/bff/test/Bff.Tests/BffWithoutExplicitFrontendTests.cs +++ b/bff/test/Bff.Tests/BffWithoutExplicitFrontendTests.cs @@ -24,23 +24,21 @@ public class BffWithoutExplicitFrontendTests(ITestOutputHelper output) : BffTest return ""; }); }; - Bff.SetBffOptions += options => - { - options.ConfigureOpenIdConnectDefaults = oidc => - { - oidc.Authority = IdentityServer.Url().ToString(); - oidc.ClientId = DefaultOidcClient.ClientId; - oidc.ClientSecret = DefaultOidcClient.ClientSecret; - oidc.ResponseType = DefaultOidcClient.ResponseType; - oidc.ResponseMode = DefaultOidcClient.ResponseMode; - oidc.MapInboundClaims = false; - oidc.GetClaimsFromUserInfoEndpoint = true; - oidc.SaveTokens = true; - oidc.BackchannelHttpHandler = Internet; - }; - }; await base.InitializeAsync(); + + Bff.BffOptions.ConfigureOpenIdConnectDefaults = oidc => + { + oidc.Authority = IdentityServer.Url().ToString(); + oidc.ClientId = DefaultOidcClient.ClientId; + oidc.ClientSecret = DefaultOidcClient.ClientSecret; + oidc.ResponseType = DefaultOidcClient.ResponseType; + oidc.ResponseMode = DefaultOidcClient.ResponseMode; + oidc.MapInboundClaims = false; + oidc.GetClaimsFromUserInfoEndpoint = true; + oidc.SaveTokens = true; + oidc.BackchannelHttpHandler = Internet; + }; } [Fact] diff --git a/bff/test/Bff.Tests/Blazor/BffBlazorTests.cs b/bff/test/Bff.Tests/Blazor/BffBlazorTests.cs index d9e93ca00..05ed62264 100644 --- a/bff/test/Bff.Tests/Blazor/BffBlazorTests.cs +++ b/bff/test/Bff.Tests/Blazor/BffBlazorTests.cs @@ -11,7 +11,6 @@ namespace Bff.Tests.Blazor; public class BffBlazorTests : BffTestBase { - public BffBlazorTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) { Bff.MapGetForRoot = false; @@ -44,8 +43,7 @@ public class BffBlazorTests : BffTestBase [Theory, MemberData(nameof(AllSetups))] public async Task Can_get_home(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync("/"); response.StatusCode.ShouldBe(HttpStatusCode.OK); } @@ -53,8 +51,7 @@ public class BffBlazorTests : BffTestBase [Theory, MemberData(nameof(AllSetups))] public async Task Cannot_get_secure_without_loggin_in(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); Bff.BrowserClient.RedirectHandler.AutoFollowRedirects = false; var response = await Bff.BrowserClient.GetAsync("/secure"); @@ -64,8 +61,7 @@ public class BffBlazorTests : BffTestBase [Theory, MemberData(nameof(AllSetups))] public async Task Can_get_secure_when_logged_in(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); Bff.BrowserClient.RedirectHandler.AutoFollowRedirects = false; diff --git a/bff/test/Bff.Tests/Configuration/BffBuilderTests.cs b/bff/test/Bff.Tests/Configuration/BffBuilderTests.cs index 313720e4c..1d56d26f9 100644 --- a/bff/test/Bff.Tests/Configuration/BffBuilderTests.cs +++ b/bff/test/Bff.Tests/Configuration/BffBuilderTests.cs @@ -31,7 +31,7 @@ public class BffBuilderTests .AddFrontends(frontend1, frontend2); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(2); frontends.ShouldContain(frontend1); frontends.ShouldContain(frontend2); @@ -51,7 +51,7 @@ public class BffBuilderTests .AddFrontends(frontend2); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(2); frontends.ShouldContain(frontend1); frontends.ShouldContain(frontend2); @@ -74,7 +74,7 @@ public class BffBuilderTests services.AddBff() .LoadConfiguration(configuration); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(1); var found = frontends.First(x => x.Name == The.FrontendName); @@ -124,7 +124,7 @@ public class BffBuilderTests .AddRemoteApis() .LoadConfiguration(configuration); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(1); var found = frontends.First(x => x.Name == The.FrontendName); @@ -145,7 +145,6 @@ public class BffBuilderTests Frontends = new Dictionary() { [The.FrontendName] = new(), - } }) .Build(); @@ -153,7 +152,7 @@ public class BffBuilderTests var services = new ServiceCollection(); services.AddBff().LoadConfiguration(configuration); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(1); var found = frontends.First(x => x.Name == The.FrontendName); @@ -181,9 +180,7 @@ public class BffBuilderTests Cookies = Some.CookieConfiguration() // ev: todo //CallbackPath = The.Path - }, - } }) .Build(); @@ -191,10 +188,11 @@ public class BffBuilderTests // Wire up the BFF var services = new ServiceCollection(); - services.AddSingleton(); // We need the http context to set the scope + services + .AddSingleton(); // We need the http context to set the scope services.AddBff().LoadConfiguration(configuration); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(1); var found = frontends.First(x => x.Name == The.FrontendName); @@ -232,7 +230,6 @@ public class BffBuilderTests [The.FrontendName] = new() { }, - } }) .Build(); @@ -240,10 +237,11 @@ public class BffBuilderTests // Wire up the BFF var services = new ServiceCollection(); - services.AddSingleton(); // We need the http context to set the scope + services + .AddSingleton(); // We need the http context to set the scope services.AddBff().LoadConfiguration(configuration); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(1); var found = frontends.First(x => x.Name == The.FrontendName); @@ -277,7 +275,6 @@ public class BffBuilderTests IConfiguration configuration = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary() { - ["frontends:_FrontendName_:matchingPath"] = The.Path, ["frontends:_FrontendName_:matchingOrigin"] = The.Origin.ToString(), ["frontends:_FrontendName_:indexHtmlUrl"] = The.Url.ToString(), @@ -294,7 +291,8 @@ public class BffBuilderTests ["frontends:_FrontendName_:RemoteApis:0:localPath"] = The.Path, ["frontends:_FrontendName_:RemoteApis:0:targetUri"] = The.Url.ToString(), ["frontends:_FrontendName_:RemoteApis:0:requiredTokenType"] = The.RequiredTokenType.ToString(), - ["frontends:_FrontendName_:RemoteApis:0:tokenRetrieverTypeName"] = The.TokenRetrieverType.AssemblyQualifiedName, + ["frontends:_FrontendName_:RemoteApis:0:tokenRetrieverTypeName"] = + The.TokenRetrieverType.AssemblyQualifiedName, ["frontends:_FrontendName_:RemoteApis:0:userAccessTokenParameters:signinScheme"] = The.Scheme, ["frontends:_FrontendName_:RemoteApis:0:userAccessTokenParameters:challengeScheme"] = The.Scheme, ["frontends:_FrontendName_:RemoteApis:0:userAccessTokenParameters:forceRenewal"] = true.ToString(), @@ -307,7 +305,7 @@ public class BffBuilderTests .LoadConfiguration(configuration) .AddRemoteApis(); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(1); var found = frontends.First(x => x.Name == The.FrontendName); @@ -347,7 +345,7 @@ public class BffBuilderTests var services = new ServiceCollection(); services.AddBff().LoadConfiguration(configFile.Configuration); var provider = services.BuildServiceProvider(); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(2); configFile.Save(new BffConfiguration() @@ -359,7 +357,7 @@ public class BffBuilderTests } }); - frontends = provider.GetRequiredService().GetAll(); + frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(2); var found = frontends.ToArray(); @@ -398,7 +396,7 @@ public class BffBuilderTests optionsCache.TryAdd("to_be_removed", new OpenIdConnectOptions()); optionsCache.TryAdd("to_be_updated", new OpenIdConnectOptions()); - var frontends = provider.GetRequiredService().GetAll(); + var frontends = provider.GetRequiredService(); configFile.Save(new BffConfiguration() { @@ -409,11 +407,12 @@ public class BffBuilderTests } }); - frontends = provider.GetRequiredService().GetAll(); + frontends = provider.GetRequiredService(); frontends.Count.ShouldBe(2); optionsCache.TryRemove("to_be_removed") - .ShouldBeTrue("The frontend 'to_be_removed' is no longer in the config and should be removed from oidc config"); + .ShouldBeTrue( + "The frontend 'to_be_removed' is no longer in the config and should be removed from oidc config"); optionsCache.TryRemove("to_be_updated") .ShouldBeTrue("The frontend 'to_be_updated' is changed. We need to clear it from the oidc cache."); @@ -440,7 +439,6 @@ public class BffBuilderTests var options = provider.GetRequiredService>(); ValidateOpenIdConnectOptions(options.Value, The.CallbackPath.ToString()); - } [Fact] @@ -491,8 +489,7 @@ public class BffBuilderTests ClientId = "not-null", ClientSecret = null } - } - , + }, } }) .Build(); @@ -510,7 +507,7 @@ public class BffBuilderTests var services = new ServiceCollection(); Should.Throw(() => services.AddBff() - .AddFrontends(Some.BffFrontend(), Some.BffFrontend())) + .AddFrontends(Some.BffFrontend(), Some.BffFrontend())) ; } } diff --git a/bff/test/Bff.Tests/ConventionTests.cs b/bff/test/Bff.Tests/ConventionTests.cs index 417d8aaf4..a32653e1d 100644 --- a/bff/test/Bff.Tests/ConventionTests.cs +++ b/bff/test/Bff.Tests/ConventionTests.cs @@ -177,6 +177,7 @@ public class ConventionTests(ITestOutputHelper output) typeof(AddServerManagementClaimsTransform), typeof(BffServerAuthenticationStateProvider), typeof(BffClientAuthenticationStateProvider), + typeof(SessionCleanupHost), typeof(SessionDbContext), typeof(SessionDbContext<>), typeof(ServerSideTokenStore), // This one needs to be removed after move to ATM 4.0 diff --git a/bff/test/Bff.Tests/Endpoints/DPoPTestsWithManualAuthentication.cs b/bff/test/Bff.Tests/Endpoints/DPoPTestsWithManualAuthentication.cs index 7c321ab8b..cc12c127b 100644 --- a/bff/test/Bff.Tests/Endpoints/DPoPTestsWithManualAuthentication.cs +++ b/bff/test/Bff.Tests/Endpoints/DPoPTestsWithManualAuthentication.cs @@ -11,20 +11,15 @@ using Xunit.Abstractions; namespace Duende.Bff.Tests.Endpoints; -public class DPoPTestsWithManualAuthentication : BffTestBase, IAsyncLifetime +public class DPoPTestsWithManualAuthentication(ITestOutputHelper output) : BffTestBase(output), IAsyncLifetime { - public DPoPTestsWithManualAuthentication(ITestOutputHelper output) : base(output) + public override async Task InitializeAsync() { Bff.EnableBackChannelHandler = false; var idSrvClient = IdentityServer.AddClient(The.ClientId, Bff.Url()); idSrvClient.RequireDPoP = true; - Bff.SetBffOptions += options => - { - options.DPoPJsonWebKey = The.DPoPJsonWebKey; - }; - Bff.OnConfigureServices += services => { services.AddAuthentication(opt => @@ -49,6 +44,8 @@ public class DPoPTestsWithManualAuthentication : BffTestBase, IAsyncLifetime ; }; + await base.InitializeAsync(); + Bff.BffOptions.DPoPJsonWebKey = The.DPoPJsonWebKey; } [Fact] @@ -60,7 +57,6 @@ public class DPoPTestsWithManualAuthentication : BffTestBase, IAsyncLifetime [Fact] public async Task When_calling_api_endpoint_with_dpop_enabled_then_dpop_headers_are_sent() { - ApiCallDetails callToApi = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.PathAndSubPath) ); diff --git a/bff/test/Bff.Tests/Endpoints/DpopRemoteEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/DpopRemoteEndpointTests.cs index a2eb3d035..588805295 100644 --- a/bff/test/Bff.Tests/Endpoints/DpopRemoteEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/DpopRemoteEndpointTests.cs @@ -8,24 +8,15 @@ using Duende.Bff.Yarp; using Xunit.Abstractions; namespace Duende.Bff.Tests.Endpoints; -public class DpopRemoteEndpointTests : BffTestBase, IAsyncLifetime + +public class DpopRemoteEndpointTests(ITestOutputHelper output) : BffTestBase(output), IAsyncLifetime { - public DpopRemoteEndpointTests(ITestOutputHelper output) : base(output) + public override async Task InitializeAsync() { var idSrvClient = IdentityServer.AddClient(The.ClientId, Bff.Url()); idSrvClient.RequireDPoP = true; - Bff.SetBffOptions += options => - { - options.DPoPJsonWebKey = The.DPoPJsonWebKey; - options.ConfigureOpenIdConnectDefaults = opt => - { - opt.BackchannelHttpHandler = Internet; - The.DefaultOpenIdConnectConfiguration(opt); - }; - }; - Bff.OnConfigureBff += bff => bff.AddRemoteApis(); Bff.OnConfigureEndpoints += endpoints => { @@ -34,16 +25,22 @@ public class DpopRemoteEndpointTests : BffTestBase, IAsyncLifetime ; }; + await base.InitializeAsync(); + Bff.BffOptions.DPoPJsonWebKey = The.DPoPJsonWebKey; + Bff.BffOptions.ConfigureOpenIdConnectDefaults = opt => + { + opt.BackchannelHttpHandler = Internet; + The.DefaultOpenIdConnectConfiguration(opt); + }; } [Fact] public async Task Can_login_with_dpop_enabled() => await Bff.BrowserClient.Login() - .CheckHttpStatusCode(); + .CheckHttpStatusCode(); [Fact] public async Task When_calling_api_endpoint_with_dpop_enabled_then_dpop_headers_are_sent() { - ApiCallDetails callToApi = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.PathAndSubPath) ); diff --git a/bff/test/Bff.Tests/Endpoints/LocalEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/LocalEndpointTests.cs index 9e4120125..3f6503cce 100644 --- a/bff/test/Bff.Tests/Endpoints/LocalEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/LocalEndpointTests.cs @@ -24,8 +24,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -49,8 +48,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .SkipAntiforgery() .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -74,8 +72,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .RequireAuthorization() .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); @@ -96,8 +93,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -117,8 +113,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .SkipAntiforgery() .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); @@ -139,8 +134,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) endpoints.Map(The.Path, c => ApiHost.ReturnApiCallDetails(c, () => LocalApiResponseStatus)) .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); @@ -160,8 +154,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) endpoints.Map(The.Path, c => ApiHost.ReturnApiCallDetails(c, () => LocalApiResponseStatus)) .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); @@ -189,8 +182,8 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) endpoints.Map(The.Path, c => ApiHost.ReturnApiCallDetails(c, () => LocalApiResponseStatus)) .RequireAuthorization(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + Bff.BrowserClient.RedirectHandler.AutoFollowRedirects = false; // we want to see the redirect @@ -216,8 +209,8 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .RequireAuthorization() .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + LocalApiResponseStatus = HttpStatusCode.Unauthorized; @@ -239,9 +232,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); - + await ConfigureBff(setup); LocalApiResponseStatus = HttpStatusCode.Forbidden; await Bff.BrowserClient.Login(); @@ -263,8 +254,8 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .AsBffApiEndpoint(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); @@ -287,8 +278,9 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .SkipResponseHandling(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + + await Bff.BrowserClient.Login(); Bff.BrowserClient.RedirectHandler.AutoFollowRedirects = false; @@ -313,8 +305,7 @@ public class LocalEndpointTests(ITestOutputHelper output) : BffTestBase(output) .Build(); }); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync(Bff.Url("/not-found")); diff --git a/bff/test/Bff.Tests/Endpoints/Management/BackchannelLogoutEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/Management/BackchannelLogoutEndpointTests.cs index d9295706d..230ed98b4 100644 --- a/bff/test/Bff.Tests/Endpoints/Management/BackchannelLogoutEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/Management/BackchannelLogoutEndpointTests.cs @@ -11,9 +11,9 @@ namespace Duende.Bff.Tests.Endpoints.Management; public class BackchannelLogoutEndpointTests : BffTestBase { public BackchannelLogoutEndpointTests(ITestOutputHelper output) : base(output) => Bff.OnConfigureBff += bff => - { - bff.AddServerSideSessions(); - }; + { + bff.AddServerSideSessions(); + }; public override async Task InitializeAsync() { @@ -30,8 +30,6 @@ public class BackchannelLogoutEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task backchannel_logout_should_allow_anonymous(BffSetupType setup) { - ConfigureBff(setup); - Bff.OnConfigureServices += svcs => { svcs.AddAuthorization(opts => @@ -42,7 +40,7 @@ public class BackchannelLogoutEndpointTests : BffTestBase .Build(); }); }; - await InitializeAsync(); + await ConfigureBff(setup); // if you call the endpoint without a token, it should return 400 await Bff.BrowserClient.PostAsync(Bff.Url("/bff/backchannel"), null) @@ -53,8 +51,7 @@ public class BackchannelLogoutEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task backchannel_logout_endpoint_should_signout(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -67,8 +64,7 @@ public class BackchannelLogoutEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task backchannel_logout_endpoint_for_incorrect_sub_should_not_logout_user(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.CreateIdentityServerSessionCookieAsync(IdentityServer, The.Sub, The.Sid); @@ -85,8 +81,7 @@ public class BackchannelLogoutEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task backchannel_logout_endpoint_for_incorrect_sid_should_not_logout_user(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -100,15 +95,11 @@ public class BackchannelLogoutEndpointTests : BffTestBase [Theory] [MemberData(nameof(AllSetups))] - public async Task when_BackchannelLogoutAllUserSessions_is_false_backchannel_logout_should_only_logout_one_session(BffSetupType setup) + public async Task when_BackchannelLogoutAllUserSessions_is_false_backchannel_logout_should_only_logout_one_session( + BffSetupType setup) { - ConfigureBff(setup); - Bff.SetBffOptions += options => - { - options.BackchannelLogoutAllUserSessions = false; - }; - - await InitializeAsync(); + await ConfigureBff(setup); + Bff.BffOptions.BackchannelLogoutAllUserSessions = false; await Bff.BrowserClient.CreateIdentityServerSessionCookieAsync(IdentityServer, The.Sub, The.Sid); @@ -139,15 +130,11 @@ public class BackchannelLogoutEndpointTests : BffTestBase [Theory] [MemberData(nameof(AllSetups))] - public async Task when_BackchannelLogoutAllUserSessions_is_false_backchannel_logout_should_logout_all_sessions(BffSetupType setup) + public async Task when_BackchannelLogoutAllUserSessions_is_false_backchannel_logout_should_logout_all_sessions( + BffSetupType setup) { - ConfigureBff(setup); - Bff.SetBffOptions += options => - { - options.BackchannelLogoutAllUserSessions = true; - }; - - await InitializeAsync(); + await ConfigureBff(setup); + Bff.BffOptions.BackchannelLogoutAllUserSessions = true; await Bff.BrowserClient.CreateIdentityServerSessionCookieAsync(IdentityServer, The.Sub, The.Sid); @@ -175,4 +162,3 @@ public class BackchannelLogoutEndpointTests : BffTestBase } } } - diff --git a/bff/test/Bff.Tests/Endpoints/Management/LoginEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/Management/LoginEndpointTests.cs index 536956224..aea70ad16 100644 --- a/bff/test/Bff.Tests/Endpoints/Management/LoginEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/Management/LoginEndpointTests.cs @@ -8,21 +8,19 @@ using Xunit.Abstractions; namespace Duende.Bff.Tests.Endpoints.Management; -public class LoginEndpointTests : BffTestBase +public class LoginEndpointTests(ITestOutputHelper output) : BffTestBase(output) { - public LoginEndpointTests(ITestOutputHelper output) : base(output) => Bff.SetBffOptions += options => - { - options.ConfigureOpenIdConnectDefaults = opt => - { - The.DefaultOpenIdConnectConfiguration(opt); - }; - }; + public override async Task InitializeAsync() + { + await base.InitializeAsync(); + Bff.BffOptions.ConfigureOpenIdConnectDefaults = opt => { The.DefaultOpenIdConnectConfiguration(opt); }; + } [Theory] [MemberData(nameof(AllSetups))] public async Task login_should_allow_anonymous(BffSetupType setup) { - ConfigureBff(setup); + await ConfigureBff(setup); Bff.OnConfigureServices += svcs => { @@ -34,7 +32,6 @@ public class LoginEndpointTests : BffTestBase .Build(); }); }; - await InitializeAsync(); var response = await Bff.BrowserClient.Login(); response.StatusCode.ShouldNotBe(HttpStatusCode.Unauthorized); @@ -44,8 +41,7 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task when_unauthenticated_silent_login_should_return_isLoggedIn_false(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/silent-login?redirectUri=/")) .CheckHttpStatusCode(); @@ -60,8 +56,7 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task silent_login_should_challenge_and_return_silent_login_html(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -81,8 +76,7 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task can_issue_silent_login_with_prompt_none(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -102,8 +96,7 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task login_with_unsupported_prompt_is_rejected(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/login?prompt=not_supported_prompt")); response.StatusCode.ShouldBe(HttpStatusCode.BadRequest); @@ -117,8 +110,7 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task can_use_prompt_supported_by_IdentityServer(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); // Prompt=create is enabled in identity server configuration: // https://docs.duendesoftware.com/identityserver/reference/options#userinteraction @@ -127,7 +119,8 @@ public class LoginEndpointTests : BffTestBase var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/login?prompt=create")) .CheckHttpStatusCode(); - response.RequestMessage!.RequestUri!.ToString().ShouldStartWith(IdentityServer.Url("/account/create").ToString()); + response.RequestMessage!.RequestUri!.ToString() + .ShouldStartWith(IdentityServer.Url("/account/create").ToString()); response.RequestMessage!.RequestUri!.ToString().ShouldNotContain("error"); } @@ -135,48 +128,41 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task login_endpoint_should_authenticatre_and_redirect_to_root(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.Login(); response.RequestMessage!.RequestUri.ShouldBe(Bff.Url("/")); - } [Theory] [MemberData(nameof(AllSetups))] public async Task login_endpoint_should_challenge_and_redirect_to_root_with_custom_prefix(BffSetupType setup) { - ConfigureBff(setup); Bff.OnConfigureServices += svcs => { - svcs.Configure(options => - { - options.ManagementBasePath = "/custom/bff"; - }); + svcs.Configure(options => { options.ManagementBasePath = "/custom/bff"; }); }; - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(expectedStatusCode: HttpStatusCode.NotFound); var response = await Bff.BrowserClient.Login("/custom"); response.RequestMessage!.RequestUri.ShouldBe(Bff.Url("/")); - } [Theory] [MemberData(nameof(AllSetups))] - public async Task login_endpoint_should_challenge_and_redirect_to_root_with_custom_prefix_trailing_slash(BffSetupType setup) + public async Task login_endpoint_should_challenge_and_redirect_to_root_with_custom_prefix_trailing_slash( + BffSetupType setup) { - ConfigureBff(setup); Bff.OnConfigureServices += svcs => { - svcs.Configure(options => - { - options.ManagementBasePath = "/custom/bff/"; - }); + svcs.Configure(options => { options.ManagementBasePath = "/custom/bff/"; }); }; - await InitializeAsync(); + + await ConfigureBff(setup); + await Bff.BrowserClient.Login(expectedStatusCode: HttpStatusCode.NotFound); @@ -188,29 +174,24 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task login_endpoint_should_challenge_and_redirect_to_root_with_root_prefix(BffSetupType setup) { - ConfigureBff(setup); Bff.OnConfigureServices += svcs => { - svcs.Configure(options => - { - options.ManagementBasePath = "/"; - }); + svcs.Configure(options => { options.ManagementBasePath = "/"; }); }; - await InitializeAsync(); + + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync(Bff.Url("/login")) .CheckHttpStatusCode(); response.RequestMessage!.RequestUri.ShouldBe(Bff.Url("/")); - } [Theory] [MemberData(nameof(AllSetups))] public async Task login_endpoint_with_existing_session_should_challenge(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -225,10 +206,8 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task login_endpoint_should_accept_returnUrl(BffSetupType setup) { - ConfigureBff(setup); Bff.OnConfigureEndpoints += endpoints => endpoints.MapGet("/foo", () => "foo'd you"); - - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/login") + "?returnUrl=/foo") .CheckHttpStatusCode(); @@ -241,8 +220,7 @@ public class LoginEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task login_endpoint_should_not_accept_non_local_returnUrl(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var problem = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/login") + "?returnUrl=https://foo") .ShouldBeProblem(); diff --git a/bff/test/Bff.Tests/Endpoints/Management/LogoutEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/Management/LogoutEndpointTests.cs index dc0ded94f..cf39c915e 100644 --- a/bff/test/Bff.Tests/Endpoints/Management/LogoutEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/Management/LogoutEndpointTests.cs @@ -16,7 +16,6 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) [MemberData(nameof(AllSetups))] public async Task logout_endpoint_should_allow_anonymous(BffSetupType setup) { - ConfigureBff(setup); Bff.OnConfigureServices += svcs => { svcs.AddAuthorization(opts => @@ -28,7 +27,7 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) }); }; - await InitializeAsync(); + await ConfigureBff(setup); var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/logout")); response.StatusCode.ShouldNotBe(HttpStatusCode.Unauthorized); @@ -38,8 +37,7 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) [MemberData(nameof(AllSetups))] public async Task logout_endpoint_should_signout(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); @@ -54,8 +52,7 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) [MemberData(nameof(AllSetups))] public async Task logout_endpoint_for_authenticated_should_require_sid(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); var problem = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/logout")) @@ -68,10 +65,10 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) [Theory] [MemberData(nameof(AllSetups))] - public async Task logout_endpoint_for_authenticated_when_require_option_is_false_should_not_require_sid(BffSetupType setup) + public async Task logout_endpoint_for_authenticated_when_require_option_is_false_should_not_require_sid( + BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); Bff.BffOptions.RequireLogoutSessionId = false; @@ -79,28 +76,29 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/logout")); response.StatusCode.ShouldBe(HttpStatusCode.Redirect); // endsession - response.Headers.Location!.ToString().ToLowerInvariant().ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); + response.Headers.Location!.ToString().ToLowerInvariant() + .ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); } [Theory] [MemberData(nameof(AllSetups))] public async Task logout_endpoint_for_authenticated_user_without_sid_should_succeed(BffSetupType setup) { - // Workaround to place a session cookie in the BFF without a session id claim. Bff.OnConfigureEndpoints += endpoints => { endpoints.MapGet("/__signin", async ctx => { var props = new AuthenticationProperties(); - await ctx.SignInAsync(new ClaimsPrincipal(new ClaimsIdentity([new Claim(JwtClaimTypes.Subject, The.Sub)], "test", "name", "role")), props); + await ctx.SignInAsync( + new ClaimsPrincipal(new ClaimsIdentity([new Claim(JwtClaimTypes.Subject, The.Sub)], "test", "name", + "role")), props); ctx.Response.StatusCode = 204; }); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.GetAsync("__signin"); // workaround for RevokeUserRefreshTokenAsync throwing when no RT in session @@ -110,29 +108,29 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/logout")); response.StatusCode.ShouldBe(HttpStatusCode.Redirect); // endsession - response.Headers.Location!.ToString().ToLowerInvariant().ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); + response.Headers.Location!.ToString().ToLowerInvariant() + .ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); } [Theory] [MemberData(nameof(AllSetups))] public async Task logout_endpoint_for_anonymous_user_without_sid_should_succeed(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); Bff.BrowserClient.RedirectHandler.AutoFollowRedirects = false; var response = await Bff.BrowserClient.GetAsync(Bff.Url("/bff/logout")); response.StatusCode.ShouldBe(HttpStatusCode.Redirect); // endsession - response.Headers.Location!.ToString().ToLowerInvariant().ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); + response.Headers.Location!.ToString().ToLowerInvariant() + .ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); } [Theory] [MemberData(nameof(AllSetups))] public async Task can_logout_twice(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); var sid = await Bff.BrowserClient.GetSid(); @@ -143,7 +141,8 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) var response = await Bff.BrowserClient.Logout(sid); response.StatusCode.ShouldBe(HttpStatusCode.Redirect); // endsession - response.Headers.Location!.ToString().ToLowerInvariant().ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); + response.Headers.Location!.ToString().ToLowerInvariant() + .ShouldStartWith(IdentityServer.Url("/connect/endsession").ToString()); (await Bff.BrowserClient.GetIsUserLoggedInAsync()).ShouldBeFalse(); @@ -156,23 +155,20 @@ public class LogoutEndpointTests(ITestOutputHelper output) : BffTestBase(output) { Bff.OnConfigureEndpoints += endpoints => endpoints.MapGet("/foo", () => "foo'd you"); - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); var response = await Bff.BrowserClient.Logout(returnUrl: new Uri("/foo", UriKind.Relative)) .CheckHttpStatusCode(); response.RequestMessage!.RequestUri.ShouldBe(Bff.Url("/foo")); - } [Theory] [MemberData(nameof(AllSetups))] public async Task logout_endpoint_should_reject_non_local_returnUrl(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); var problem = await Bff.BrowserClient.Logout(returnUrl: new Uri("https://foo")) diff --git a/bff/test/Bff.Tests/Endpoints/Management/ManagementBasePathTests.cs b/bff/test/Bff.Tests/Endpoints/Management/ManagementBasePathTests.cs index 1787b6d32..fbdc1ac7e 100644 --- a/bff/test/Bff.Tests/Endpoints/Management/ManagementBasePathTests.cs +++ b/bff/test/Bff.Tests/Endpoints/Management/ManagementBasePathTests.cs @@ -20,7 +20,6 @@ public class ManagementBasePathTests(ITestOutputHelper output) : BffTestBase(out [InlineData(Constants.ManagementEndpoints.User, HttpStatusCode.Unauthorized)] public async Task custom_ManagementBasePath_should_affect_basepath(string path, HttpStatusCode expectedStatusCode) { - ConfigureBff(BffSetupType.V4Bff); Bff.OnConfigureServices += svcs => { svcs.Configure(options => @@ -28,7 +27,7 @@ public class ManagementBasePathTests(ITestOutputHelper output) : BffTestBase(out options.ManagementBasePath = new PathString("/{path:regex(^[a-zA-Z\\d-]+$)}/bff"); }); }; - await InitializeAsync(); + await ConfigureBff(BffSetupType.V4Bff); // Don't follow the redirects, becuase otherwise we might folow a redirect flow that ends up in a 404 Bff.BrowserClient.RedirectHandler.AutoFollowRedirects = false; @@ -48,6 +47,5 @@ public class ManagementBasePathTests(ITestOutputHelper output) : BffTestBase(out var response = await Bff.BrowserClient.SendAsync(req); response.StatusCode.ShouldBe(expectedStatusCode); - } } diff --git a/bff/test/Bff.Tests/Endpoints/Management/UserEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/Management/UserEndpointTests.cs index c549cdda5..ecd5107d5 100644 --- a/bff/test/Bff.Tests/Endpoints/Management/UserEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/Management/UserEndpointTests.cs @@ -33,8 +33,7 @@ public class UserEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task user_endpoint_for_authenticated_user_should_return_claims(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); AddCustomUserClaims(new Claim("foo", "foo1"), new Claim("foo", "foo2")); await Bff.BrowserClient.Login(); @@ -56,8 +55,7 @@ public class UserEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task user_endpoint_for_authenticated_user_with_sid_should_return_claims_including_logout(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); UserToSignIn = new ClaimsPrincipal(new ClaimsIdentity([ new Claim("sub", "alice"), new Claim("sid", "123"), @@ -78,8 +76,7 @@ public class UserEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task user_endpoint_for_authenticated_user_without_csrf_header_should_fail(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.IssueSessionCookieAsync(new Claim("sub", "alice"), new Claim("foo", "foo1"), new Claim("foo", "foo2")); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url("/bff/user")); @@ -92,8 +89,7 @@ public class UserEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task user_endpoint_for_unauthenticated_user_should_fail(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url("/bff/user")); req.Headers.Add("x-csrf", "1"); var response = await Bff.BrowserClient.SendAsync(req); @@ -105,8 +101,7 @@ public class UserEndpointTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task when_configured_user_endpoint_for_unauthenticated_user_should_return_200_and_empty(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var options = Bff.Resolve>(); options.Value.AnonymousSessionResponse = AnonymousSessionResponse.Response200; diff --git a/bff/test/Bff.Tests/Endpoints/RemoteEndpointTests.cs b/bff/test/Bff.Tests/Endpoints/RemoteEndpointTests.cs index 251cfeadc..90d02d0ed 100644 --- a/bff/test/Bff.Tests/Endpoints/RemoteEndpointTests.cs +++ b/bff/test/Bff.Tests/Endpoints/RemoteEndpointTests.cs @@ -30,7 +30,6 @@ public class RemoteEndpointTests : BffTestBase { // Add a custom default transform that adds a header to the request services.AddSingleton(CustomDefaultBffTransformBuilder); - }; Bff.OnConfigureBff += bff => bff.AddRemoteApis(); @@ -51,8 +50,7 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path), @@ -69,9 +67,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(); }; - ConfigureBff(setup); + await ConfigureBff(setup); - await InitializeAsync(); await Bff.BrowserClient.Login(); var (response, apiResult) = await Bff.BrowserClient.CallBffHostApi( @@ -111,7 +108,7 @@ public class RemoteEndpointTests : BffTestBase .WithAccessToken(); }; - ConfigureBff(setup, configureOpenIdConnect: options => + await ConfigureBff(setup, configureOpenIdConnect: options => { options.Events.OnUserInformationReceived = context => { @@ -130,7 +127,6 @@ public class RemoteEndpointTests : BffTestBase }; The.DefaultOpenIdConnectConfiguration(options); }); - await InitializeAsync(); await Bff.BrowserClient.Login(); var (response, apiResult) = await Bff.BrowserClient.CallBffHostApi( @@ -167,7 +163,7 @@ public class RemoteEndpointTests : BffTestBase .WithAccessToken(); }; - ConfigureBff(setup, configureOpenIdConnect: options => + await ConfigureBff(setup, configureOpenIdConnect: options => { options.Events.OnUserInformationReceived = context => { @@ -186,7 +182,6 @@ public class RemoteEndpointTests : BffTestBase }; The.DefaultOpenIdConnectConfiguration(options); }); - await InitializeAsync(); await Bff.BrowserClient.Login(); var (response, apiResult) = await Bff.BrowserClient.CallBffHostApi( @@ -203,9 +198,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(); }; - ConfigureBff(setup); + await ConfigureBff(setup); - await InitializeAsync(); await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path), @@ -227,9 +221,9 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path), @@ -251,9 +245,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.UserOrNone); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); { ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path) @@ -288,9 +281,9 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.Client); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path) ); @@ -317,9 +310,9 @@ public class RemoteEndpointTests : BffTestBase .WithAccessTokenRetriever() .WithAccessToken(RequiredTokenType.UserOrClient); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path), expectedStatusCode: HttpStatusCode.Unauthorized @@ -339,8 +332,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path), @@ -357,8 +350,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path), @@ -382,8 +375,8 @@ public class RemoteEndpointTests : BffTestBase .WithAccessTokenRetriever() .WithAccessToken(); }; - ConfigureBff(setup); - await InitializeAsync(); + + await ConfigureBff(setup); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path) @@ -428,9 +421,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.UserOrClient); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); { ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path) @@ -465,9 +457,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); { ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( url: Bff.Url(The.Path) @@ -506,9 +497,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint("/none", Api.Url()) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); foreach (var client in IdentityServer.Clients) { client.Enabled = false; @@ -537,9 +527,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); Api.ApiStatusCodeToReturn = HttpStatusCode.Unauthorized; @@ -558,9 +547,8 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); Api.ApiStatusCodeToReturn = HttpStatusCode.Forbidden; @@ -579,15 +567,13 @@ public class RemoteEndpointTests : BffTestBase app.MapRemoteBffApiEndpoint(The.Path, Api.Url(The.Path)) .WithAccessToken(RequiredTokenType.None); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url(The.Path)); var response = await Bff.BrowserClient.SendAsync(req); response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized, "The endpoint requires CSRF protection, so it should return 403 Forbidden when no CSRF header is present."); - } [Theory, MemberData(nameof(AllSetups))] @@ -599,15 +585,13 @@ public class RemoteEndpointTests : BffTestBase .WithAccessToken(RequiredTokenType.UserOrClient) .SkipAntiforgery(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url(The.Path)); var response = await Bff.BrowserClient.SendAsync(req); response.StatusCode.ShouldBe(HttpStatusCode.OK); - } [Theory, MemberData(nameof(AllSetups))] @@ -623,18 +607,19 @@ public class RemoteEndpointTests : BffTestBase c.AddRequestTransform(async context => { // One of our customers asked how to access the catch-all route value in subsequent transforms - if (context.HttpContext.Request.RouteValues.TryGetValue("catch-all", out var value) && value is string s) + if (context.HttpContext.Request.RouteValues.TryGetValue("catch-all", out var value) && + value is string s) { context.ProxyRequest.Headers.Add("catch-all", "/" + s); } + await Task.CompletedTask; }); }) .WithAccessToken(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); ApiCallDetails result = await Bff.BrowserClient.CallBffHostApi( @@ -665,6 +650,7 @@ public class RemoteEndpointTests : BffTestBase { await Task.Delay(TimeSpan.FromSeconds(5)); } + await n(); }); }; @@ -686,8 +672,8 @@ public class RemoteEndpointTests : BffTestBase .WithAccessToken(); }; - ConfigureBff(setup); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.Login(); // first, ensure that the 'normal' process works, becuase delay's are turned off. @@ -718,6 +704,7 @@ public class RemoteEndpointTests : BffTestBase { await Task.Delay(TimeSpan.FromSeconds(5)); } + await n(); }); }; @@ -736,8 +723,8 @@ public class RemoteEndpointTests : BffTestBase .WithAccessToken(); }; - ConfigureBff(setup); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.Login(); // first, ensure that the 'normal' process works, becuase delay's are turned off. diff --git a/bff/test/Bff.Tests/Endpoints/YarpTests.cs b/bff/test/Bff.Tests/Endpoints/YarpTests.cs index 8a9e1478d..ef5559e3e 100644 --- a/bff/test/Bff.Tests/Endpoints/YarpTests.cs +++ b/bff/test/Bff.Tests/Endpoints/YarpTests.cs @@ -15,26 +15,22 @@ namespace Duende.Bff.Tests.Endpoints; public class YarpTests : BffTestBase { public YarpTests(ITestOutputHelper output) : base(output) => Bff.OnConfigureEndpoints += endpoints => - { - endpoints.MapReverseProxy(proxyApp => - { - proxyApp.UseAntiforgeryCheck(); - }); - }; + { + endpoints.MapReverseProxy(proxyApp => { proxyApp.UseAntiforgeryCheck(); }); + }; private void ConfigureYarp(RouteConfig routeConfig) => - Bff.OnConfigureBff += bff => - { - bff.AddYarpConfig([routeConfig], [Some.ClusterConfig(Api)]); - }; + Bff.OnConfigureBff += bff => { bff.AddYarpConfig([routeConfig], [Some.ClusterConfig(Api)]); }; [Theory] [MemberData(nameof(AllSetups))] - public async Task anonymous_call_with_no_csrf_header_to_no_token_requirement_no_csrf_route_should_succeed(BffSetupType setup) + public async Task anonymous_call_with_no_csrf_header_to_no_token_requirement_no_csrf_route_should_succeed( + BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig()); - await InitializeAsync(); + + await ConfigureBff(setup); + await Bff.BrowserClient.CallBffHostApi( path: The.PathAndSubPath, @@ -46,9 +42,8 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task anonymous_call_with_no_csrf_header_to_csrf_route_should_fail(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAntiforgeryCheck()); - await InitializeAsync(); + await ConfigureBff(setup); var req = new HttpRequestMessage(HttpMethod.Get, The.PathAndSubPath); var response = await Bff.BrowserClient.SendAsync(req); @@ -60,15 +55,11 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task can_disable_anti_forgery_check(BffSetupType setup) { - ConfigureBff(setup); - - Bff.SetBffOptions += options => - { - options.DisableAntiForgeryCheck = (c) => true; - }; - ConfigureYarp(Some.RouteConfig().WithAntiforgeryCheck()); - await InitializeAsync(); + + await ConfigureBff(setup); + + Bff.BffOptions.DisableAntiForgeryCheck = (c) => true; var req = new HttpRequestMessage(HttpMethod.Get, The.PathAndSubPath); var response = await Bff.BrowserClient.SendAsync(req); @@ -80,9 +71,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task anonymous_call_to_no_token_requirement_route_should_succeed(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig()); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.CallBffHostApi( path: The.PathAndSubPath, @@ -94,9 +85,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task anonymous_call_to_user_token_requirement_route_should_fail(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.User)); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.CallBffHostApi( path: The.PathAndSubPath, @@ -108,9 +99,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task anonymous_call_to_optional_user_token_route_should_succeed(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.UserOrNone)); - await InitializeAsync(); + + await ConfigureBff(setup); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( path: The.PathAndSubPath, @@ -127,13 +118,11 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task old_anonymous_call_to_optional_user_token_route_should_succeed(BffSetupType setup) { - ConfigureBff(setup); - #pragma warning disable CS0618 // Type or member is obsolete ConfigureYarp(Some.RouteConfig().WithOptionalUserAccessToken()); #pragma warning restore CS0618 // Type or member is obsolete - await InitializeAsync(); + await ConfigureBff(setup); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( path: The.PathAndSubPath, @@ -150,9 +139,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task authenticated_GET_should_forward_user_to_api_for_user(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.User)); - await InitializeAsync(); + + await ConfigureBff(setup); await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -169,12 +158,12 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task authenticated_PUT_should_forward_user_to_api_for_UserOrNone(BffSetupType setup) { - ConfigureBff(setup); - ConfigureYarp(Some.RouteConfig() .WithAccessToken(RequiredTokenType.UserOrNone) ); - await InitializeAsync(); + + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -192,9 +181,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task authenticated_Post_should_forward_user_to_api_for_User(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.User)); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -212,9 +201,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task authenticated_Post_should_forward_user_to_api_for_UserOrNone(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.UserOrNone)); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -232,9 +221,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task call_to_client_token_route_should_forward_client_token_to_api(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.Client)); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -251,9 +240,9 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task call_to_user_or_client_token_route_should_forward_user_or_client_token_to_api(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.UserOrClient)); - await InitializeAsync(); + await ConfigureBff(setup); + await Bff.BrowserClient.Login(); ApiCallDetails apiResult = await Bff.BrowserClient.CallBffHostApi( @@ -301,10 +290,10 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task response_status_401_from_remote_endpoint_should_return_401_from_bff(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.User)); + + await ConfigureBff(setup); Api.ApiStatusCodeToReturn = HttpStatusCode.Unauthorized; - await InitializeAsync(); await Bff.BrowserClient.Login(); await Bff.BrowserClient.CallBffHostApi( @@ -317,10 +306,10 @@ public class YarpTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task response_status_403_from_remote_endpoint_should_return_403_from_bff(BffSetupType setup) { - ConfigureBff(setup); ConfigureYarp(Some.RouteConfig().WithAccessToken(RequiredTokenType.User)); + + await ConfigureBff(setup); Api.ApiStatusCodeToReturn = HttpStatusCode.Forbidden; - await InitializeAsync(); await Bff.BrowserClient.Login(); await Bff.BrowserClient.CallBffHostApi( diff --git a/bff/test/Bff.Tests/EntityFramework/UserSessionStoreTests.cs b/bff/test/Bff.Tests/EntityFramework/UserSessionStoreTests.cs index 6200b9c1f..1af361631 100644 --- a/bff/test/Bff.Tests/EntityFramework/UserSessionStoreTests.cs +++ b/bff/test/Bff.Tests/EntityFramework/UserSessionStoreTests.cs @@ -169,7 +169,8 @@ public class UserSessionStoreTests } [Fact] - public async Task DeleteUserSessionAsync_for_invalid_key_should_succeed() => await _subject.DeleteUserSessionAsync("invalid"); + public async Task DeleteUserSessionAsync_for_invalid_key_should_succeed() => + await _subject.DeleteUserSessionAsync("invalid"); [Fact] public async Task GetUserSessionsAsync_for_valid_sub_should_succeed() @@ -421,7 +422,8 @@ public class UserSessionStoreTests SessionId = "sid3_1", }); - var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter { SubjectId = "sub2", SessionId = "sid2_2" }); + var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter + { SubjectId = "sub2", SessionId = "sid2_2" }); items.Count().ShouldBe(1); items.Select(x => x.SubjectId).ToArray().ShouldBeEquivalentTo(new[] { "sub2" }); items.Select(x => x.SessionId).ToArray().ShouldBeEquivalentTo(new[] { "sid2_2" }); @@ -474,15 +476,18 @@ public class UserSessionStoreTests }); { - var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter { SubjectId = "invalid", SessionId = "invalid" }); + var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter + { SubjectId = "invalid", SessionId = "invalid" }); items.Count().ShouldBe(0); } { - var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter { SubjectId = "sub1", SessionId = "invalid" }); + var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter + { SubjectId = "sub1", SessionId = "invalid" }); items.Count().ShouldBe(0); } { - var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter { SubjectId = "invalid", SessionId = "sid1_1" }); + var items = await _subject.GetUserSessionsAsync(new UserSessionsFilter + { SubjectId = "invalid", SessionId = "sid1_1" }); items.Count().ShouldBe(0); } } @@ -795,15 +800,18 @@ public class UserSessionStoreTests }); { - await _subject.DeleteUserSessionsAsync(new UserSessionsFilter { SubjectId = "invalid", SessionId = "invalid" }); + await _subject.DeleteUserSessionsAsync(new UserSessionsFilter + { SubjectId = "invalid", SessionId = "invalid" }); _database.UserSessions.Count().ShouldBe(6); } { - await _subject.DeleteUserSessionsAsync(new UserSessionsFilter { SubjectId = "sub1", SessionId = "invalid" }); + await _subject.DeleteUserSessionsAsync(new UserSessionsFilter + { SubjectId = "sub1", SessionId = "invalid" }); _database.UserSessions.Count().ShouldBe(6); } { - await _subject.DeleteUserSessionsAsync(new UserSessionsFilter { SubjectId = "invalid", SessionId = "sid1_1" }); + await _subject.DeleteUserSessionsAsync(new UserSessionsFilter + { SubjectId = "invalid", SessionId = "sid1_1" }); _database.UserSessions.Count().ShouldBe(6); } } diff --git a/bff/test/Bff.Tests/Headers/GeneralTests.cs b/bff/test/Bff.Tests/Headers/GeneralTests.cs index a4e00e827..46a8312fb 100644 --- a/bff/test/Bff.Tests/Headers/GeneralTests.cs +++ b/bff/test/Bff.Tests/Headers/GeneralTests.cs @@ -21,8 +21,7 @@ public class GeneralTests(ITestOutputHelper output) : BffTestBase(output) endpoints.Map(The.Path, c => ApiHost.ReturnApiCallDetails(c)) .RequireAuthorization(); }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url(The.Path)); req.Headers.Add("x-csrf", "1"); @@ -45,10 +44,8 @@ public class GeneralTests(ITestOutputHelper output) : BffTestBase(output) { endpoints.MapRemoteBffApiEndpoint(The.Path, Api.Url()) .WithAccessToken(RequiredTokenType.None); - }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url(The.Path)); req.Headers.Add("x-csrf", "1"); @@ -71,10 +68,8 @@ public class GeneralTests(ITestOutputHelper output) : BffTestBase(output) { endpoints.MapRemoteBffApiEndpoint(The.Path, Api.Url()) .WithAccessToken(RequiredTokenType.None); - }; - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); var req = new HttpRequestMessage(HttpMethod.Get, Bff.Url(The.Path)); req.Headers.Add("x-csrf", "1"); @@ -94,8 +89,7 @@ public class GeneralTests(ITestOutputHelper output) : BffTestBase(output) [Theory, MemberData(nameof(AllSetups))] public async Task Will_auto_register_login_endpoints(BffSetupType setup) { - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); Context.LogMessages.ToString().ShouldNotContain("Already mapped Login endpoint"); @@ -107,14 +101,11 @@ public class GeneralTests(ITestOutputHelper output) : BffTestBase(output) public async Task If_management_endpoints_are_mapped_manually_then_warning_is_written(BffSetupType setup) { Bff.OnConfigureEndpoints += endpoints => endpoints.MapBffManagementEndpoints(); - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); // And we can log in, which means the login endpoint was registered await Bff.BrowserClient.Login(); Context.LogMessages.ToString().ShouldContain("Already mapped Login endpoint"); } - - } diff --git a/bff/test/Bff.Tests/MultiFrontend/FrontendCollectionTests.cs b/bff/test/Bff.Tests/MultiFrontend/FrontendCollectionTests.cs index 8ba86d726..fa0d633ee 100644 --- a/bff/test/Bff.Tests/MultiFrontend/FrontendCollectionTests.cs +++ b/bff/test/Bff.Tests/MultiFrontend/FrontendCollectionTests.cs @@ -22,19 +22,20 @@ public class FrontendCollectionTests [Fact] public void Can_load_frontends_from_constructor() { - _frontendsConfiguredDuringStartup = [ + _frontendsConfiguredDuringStartup = + [ Some.BffFrontendWithSelectionCriteria(), Some.BffFrontendWithSelectionCriteria() with { Name = BffFrontendName.Parse("different") } ]; - var cache = BuildCache(); + var cache = BuildFrontendCollection(); - var result = cache.GetAll(); + var result = cache; result.Count.ShouldBe(2); - result.ShouldBeEquivalentTo(_frontendsConfiguredDuringStartup.AsReadOnly()); + result.ToArray().ShouldBeEquivalentTo(_frontendsConfiguredDuringStartup); } - private FrontendCollection BuildCache() + private FrontendCollection BuildFrontendCollection() { // No longer inject OptionsCache var cache = new FrontendCollection(_bffConfigurationOptionsMonitor, [], _frontendsConfiguredDuringStartup); @@ -53,8 +54,8 @@ public class FrontendCollectionTests } }; - var cache = BuildCache(); - var result = cache.GetAll(); + var cache = BuildFrontendCollection(); + var result = cache; result.Count.ShouldBe(2); result.ShouldBe(new[] @@ -93,14 +94,14 @@ public class FrontendCollectionTests } }; - var cache = BuildCache(); - var result = cache.GetAll(); + var cache = BuildFrontendCollection(); var openIdConnectOptions = new OpenIdConnectOptions(); - result.First().ConfigureOpenIdConnectOptions!.Invoke(openIdConnectOptions); + cache.First().ConfigureOpenIdConnectOptions!.Invoke(openIdConnectOptions); openIdConnectOptions.ClientId.ShouldBe("clientid from programmatic defaults"); openIdConnectOptions.ClientSecret.ShouldBe("clientsecret from configured defaults"); openIdConnectOptions.ResponseMode.ShouldBe("responsemode from frontend"); } + [Fact(Skip = "find other way of testing this")] public void Cookie_Config_precedence_is_programmatic_defaults_then_configured_defaults_then_frontend_specific() { @@ -130,10 +131,9 @@ public class FrontendCollectionTests } }; - var cache = BuildCache(); - var result = cache.GetAll(); + var cache = BuildFrontendCollection(); var cookieOptions = new CookieAuthenticationOptions(); - result.First().ConfigureCookieOptions!.Invoke(cookieOptions); + cache.First().ConfigureCookieOptions!.Invoke(cookieOptions); cookieOptions.Cookie.Name.ShouldBe("Name from programmatic defaults"); cookieOptions.Cookie.Path.ShouldBe("Path from configured defaults"); cookieOptions.Cookie.Domain.ShouldBe("Domain from frontend"); @@ -143,13 +143,17 @@ public class FrontendCollectionTests [Fact] public void When_frontend_is_updated_then_event_is_raised() { - var cache = BuildCache(); + var cache = BuildFrontendCollection(); var bffFrontend = Some.BffFrontendWithSelectionCriteria(); // Track event invocations BffFrontend? eventArg = null; var eventCount = 0; - cache.OnFrontendChanged += f => { eventArg = f; eventCount++; }; + cache.OnFrontendChanged += f => + { + eventArg = f; + eventCount++; + }; // Add a new frontend (should not raise event, as it's an add) cache.AddOrUpdate(bffFrontend); @@ -167,13 +171,17 @@ public class FrontendCollectionTests [Fact] public void When_frontend_is_removed_then_event_is_raised() { - var cache = BuildCache(); + var cache = BuildFrontendCollection(); var bffFrontend = Some.BffFrontendWithSelectionCriteria(); // Track event invocations BffFrontend? eventArg = null; var eventCount = 0; - cache.OnFrontendChanged += f => { eventArg = f; eventCount++; }; + cache.OnFrontendChanged += f => + { + eventArg = f; + eventCount++; + }; // Add a new frontend (should not raise event) cache.AddOrUpdate(bffFrontend); diff --git a/bff/test/Bff.Tests/MultiFrontend/PathMapperTests.cs b/bff/test/Bff.Tests/MultiFrontend/PathMapperTests.cs index 730cc09c0..a5da46a3b 100644 --- a/bff/test/Bff.Tests/MultiFrontend/PathMapperTests.cs +++ b/bff/test/Bff.Tests/MultiFrontend/PathMapperTests.cs @@ -8,7 +8,6 @@ namespace Duende.Bff.Tests.MultiFrontend; public class PathMapperTests { - private readonly PathMapper _mapper = new(); [Fact] public void MapPath_WithNoMatchingPaths_DoesNotModifyPathBaseOrPath() @@ -18,7 +17,7 @@ public class PathMapperTests var frontend = CreateFrontendWithPath(); // Act - _mapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); // Assert context.Request.PathBase.Value.ShouldBe("/app"); @@ -33,7 +32,7 @@ public class PathMapperTests var frontend = CreateFrontendWithPath("/api"); // Act - _mapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); // Assert context.Request.PathBase.Value.ShouldBe("/app/api"); @@ -48,7 +47,7 @@ public class PathMapperTests var frontend = CreateFrontendWithPath("/api"); // Act - _mapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); // Assert context.Request.PathBase.Value.ShouldBe("/app/api"); @@ -63,7 +62,7 @@ public class PathMapperTests var frontend = CreateFrontendWithPath("/api"); // Act - _mapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); // Assert context.Response.StatusCode.ShouldBe(404); @@ -77,7 +76,7 @@ public class PathMapperTests var frontend = CreateFrontendWithPath("/api"); // Act - _mapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); // Assert context.Request.PathBase.Value.ShouldBe("/api"); @@ -92,7 +91,7 @@ public class PathMapperTests var frontend = CreateFrontendWithPath("/api"); // Act - _mapper.MapPath(context, frontend); + PathMapper.MapPath(context, frontend); // Assert context.Request.PathBase.Value.ShouldBe("/app/api"); diff --git a/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff.verified.txt b/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff.verified.txt index f320bb352..11136b06b 100644 --- a/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff.verified.txt +++ b/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff.verified.txt @@ -192,11 +192,6 @@ namespace Duende.Bff } public static class Constants { - public class HttpClientNames - { - public const string IndexHtmlHttpClient = "Duende.Bff.IndexHtmlClient"; - public HttpClientNames() { } - } public static class BffFlags { public const string Prompt = "bff-prompt"; @@ -213,6 +208,10 @@ namespace Duende.Bff public const string HostPrefix = "__Host"; public const string SecurePrefix = "__Secure"; } + public static class HttpClientNames + { + public const string IndexHtmlHttpClient = "Duende.Bff.IndexHtmlClient"; + } public static class ManagementEndpoints { public const string BackChannelLogout = "/backchannel"; @@ -249,6 +248,7 @@ namespace Duende.Bff public LocalPath() { } public override string ToString() { } public static Duende.Bff.LocalPath Parse(string value) { } + public static Duende.Bff.LocalPath ToLocalPath(Microsoft.AspNetCore.Http.PathString pathString) { } public static bool TryParse(string value, [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out Duende.Bff.LocalPath? parsed, out string[] errors) { } public static string op_Implicit(Duende.Bff.LocalPath value) { } public static Duende.Bff.LocalPath op_Implicit(Microsoft.AspNetCore.Http.PathString value) { } @@ -291,7 +291,7 @@ namespace Duende.Bff.Configuration public bool BackchannelLogoutAllUserSessions { get; set; } public System.Net.Http.HttpMessageHandler? BackchannelMessageHandler { get; set; } public Duende.Bff.AccessTokenManagement.DPoPProofKey? DPoPJsonWebKey { get; set; } - public System.Collections.Generic.ICollection DiagnosticsEnvironments { get; set; } + public System.Collections.Generic.ICollection DiagnosticsEnvironments { get; } public Microsoft.AspNetCore.Http.PathString DiagnosticsPath { get; } public Duende.Bff.Configuration.DisableAntiForgeryCheck DisableAntiForgeryCheck { get; set; } public bool EnableSessionCleanup { get; set; } @@ -313,10 +313,10 @@ namespace Duende.Bff.Configuration } public sealed class BffRemoteApiEndpointMetadata : Duende.Bff.Endpoints.IBffApiMetadata { - public Duende.Bff.AccessTokenManagement.RequiredTokenType? TokenType; public BffRemoteApiEndpointMetadata() { } public System.Type AccessTokenRetriever { get; set; } public Duende.Bff.Configuration.BffUserAccessTokenParameters? BffUserAccessTokenParameters { get; set; } + public Duende.Bff.AccessTokenManagement.RequiredTokenType? TokenType { get; set; } } public sealed class BffUserAccessTokenParameters : System.IEquatable { @@ -378,10 +378,10 @@ namespace Duende.Bff.DynamicFrontends public Duende.Bff.DynamicFrontends.Origin? MatchingOrigin { get; init; } public string? MatchingPath { get; init; } } - public interface IFrontendCollection + public interface IFrontendCollection : System.Collections.Generic.IEnumerable, System.Collections.IEnumerable { + int Count { get; } void AddOrUpdate(Duende.Bff.DynamicFrontends.BffFrontend frontend); - System.Collections.Generic.IReadOnlyList GetAll(); void Remove(Duende.Bff.DynamicFrontends.BffFrontendName frontendName); } public interface IIndexHtmlClient @@ -431,7 +431,7 @@ namespace Duende.Bff.Endpoints public interface ILogoutEndpoint : Duende.Bff.Endpoints.IBffEndpoint { } public interface IReturnUrlValidator { - bool IsValidAsync(string returnUrl); + bool IsValidAsync(System.Uri returnUrl); } public interface ISilentLoginCallbackEndpoint : Duende.Bff.Endpoints.IBffEndpoint { } public interface ISilentLoginEndpoint : Duende.Bff.Endpoints.IBffEndpoint { } @@ -439,10 +439,11 @@ namespace Duende.Bff.Endpoints } namespace Duende.Bff.Otel { - public sealed class BffMetrics + public sealed class BffMetrics : System.IDisposable { public const string MeterName = "Duende.Bff"; public BffMetrics(System.Diagnostics.Metrics.IMeterFactory meterFactory) { } + public void Dispose() { } public void SessionEnded() { } public void SessionStarted() { } public void SessionsEnded(int count) { } diff --git a/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff_Yarp.verified.txt b/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff_Yarp.verified.txt index 803ce1d5f..33259cdc6 100644 --- a/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff_Yarp.verified.txt +++ b/bff/test/Bff.Tests/PublicApiVerificationTests.VerifyPublicApi_Bff_Yarp.verified.txt @@ -10,7 +10,7 @@ public delegate void BffYarpTransformBuilder(string localPath, Yarp.ReverseProxy.Transforms.Builder.TransformBuilderContext context); public static class DefaultBffYarpTransformerBuilders { - public static Duende.Bff.Yarp.BffYarpTransformBuilder DirectProxyWithAccessToken; + public static readonly Duende.Bff.Yarp.BffYarpTransformBuilder DirectProxyWithAccessToken; } public static class ProxyAppBuilderExtensions { diff --git a/bff/test/Bff.Tests/SessionManagement/CookieSlidingTests.cs b/bff/test/Bff.Tests/SessionManagement/CookieSlidingTests.cs index 90b3bc9e3..3f9875159 100644 --- a/bff/test/Bff.Tests/SessionManagement/CookieSlidingTests.cs +++ b/bff/test/Bff.Tests/SessionManagement/CookieSlidingTests.cs @@ -14,10 +14,9 @@ public class CookieSlidingTests : BffTestBase { private InMemoryUserSessionStore _sessionStore => (InMemoryUserSessionStore)Bff.Resolve(); - public CookieSlidingTests(ITestOutputHelper output) : base(output) => Bff.OnConfigureBff += bff => - { - bff.AddServerSideSessions(); - }; + public CookieSlidingTests(ITestOutputHelper output) : base(output) => + Bff.OnConfigureBff += bff => + bff.AddServerSideSessions(); private void AdvanceClock(TimeSpan by) => The.Clock.SetUtcNow(The.Clock.GetUtcNow().Add(by)); @@ -25,8 +24,8 @@ public class CookieSlidingTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task user_endpoint_cookie_should_slide(BffSetupType setup) { - ConfigureBff(setup, UseSlidingCookieExpiration); - await InitializeAsync(); + await ConfigureBff(setup, UseSlidingCookieExpiration); + await Bff.BrowserClient.Login(); var sessions = await _sessionStore.GetUserSessionsAsync(Some.UserSessionsFilter()); @@ -53,8 +52,8 @@ public class CookieSlidingTests : BffTestBase [MemberData(nameof(AllSetups))] public async Task user_endpoint_when_sliding_flag_is_passed_cookie_should_not_slide(BffSetupType setup) { - ConfigureBff(setup, UseSlidingCookieExpiration); - await InitializeAsync(); + await ConfigureBff(setup, UseSlidingCookieExpiration); + await Bff.BrowserClient.Login(); var sessions = await _sessionStore.GetUserSessionsAsync(Some.UserSessionsFilter()); @@ -82,7 +81,7 @@ public class CookieSlidingTests : BffTestBase { var shouldRenew = false; - ConfigureBff(setup, cookieOptions => + await ConfigureBff(setup, cookieOptions => { UseSlidingCookieExpiration(cookieOptions); @@ -95,7 +94,7 @@ public class CookieSlidingTests : BffTestBase }; }); - await InitializeAsync(); + await Bff.BrowserClient.Login(); var sessions = await _sessionStore.GetUserSessionsAsync(Some.UserSessionsFilter()); @@ -121,11 +120,12 @@ public class CookieSlidingTests : BffTestBase [Theory] [MemberData(nameof(AllSetups))] - public async Task user_endpoint_when_uservalidate_renews_and_sliding_flag_is_passed_cookie_should_not_slide(BffSetupType setup) + public async Task user_endpoint_when_uservalidate_renews_and_sliding_flag_is_passed_cookie_should_not_slide( + BffSetupType setup) { var shouldRenew = false; - ConfigureBff(setup, cookieOptions => + await ConfigureBff(setup, cookieOptions => { UseSlidingCookieExpiration(cookieOptions); @@ -136,7 +136,7 @@ public class CookieSlidingTests : BffTestBase }; }); - await InitializeAsync(); + await Bff.BrowserClient.Login(); var sessions = await _sessionStore.GetUserSessionsAsync(Some.UserSessionsFilter()); @@ -164,5 +164,4 @@ public class CookieSlidingTests : BffTestBase options.SlidingExpiration = true; options.ExpireTimeSpan = TimeSpan.FromMinutes(10); } - } diff --git a/bff/test/Bff.Tests/SessionManagement/RevokeRefreshTokenTests.cs b/bff/test/Bff.Tests/SessionManagement/RevokeRefreshTokenTests.cs index c9161ee80..d28b8a052 100644 --- a/bff/test/Bff.Tests/SessionManagement/RevokeRefreshTokenTests.cs +++ b/bff/test/Bff.Tests/SessionManagement/RevokeRefreshTokenTests.cs @@ -12,12 +12,12 @@ public class RevokeRefreshTokenTests(ITestOutputHelper output) : BffTestBase(out [Theory, MemberData(nameof(AllSetups))] public async Task logout_should_revoke_refreshtoken(BffSetupType setup) { - ConfigureBff(setup, configureOpenIdConnect: options => + await ConfigureBff(setup, configureOpenIdConnect: options => { The.DefaultOpenIdConnectConfiguration(options); options.Scope.Add("offline_access"); }); - await InitializeAsync(); + await Bff.BrowserClient.Login(); { @@ -45,13 +45,12 @@ public class RevokeRefreshTokenTests(ITestOutputHelper output) : BffTestBase(out [Theory, MemberData(nameof(AllSetups))] public async Task when_setting_disabled_logout_should_not_revoke_refreshtoken(BffSetupType setup) { - - ConfigureBff(setup, configureOpenIdConnect: options => + await ConfigureBff(setup, configureOpenIdConnect: options => { The.DefaultOpenIdConnectConfiguration(options); options.Scope.Add("offline_access"); }); - await InitializeAsync(); + Bff.BffOptions.RevokeRefreshTokenOnLogout = false; await Bff.BrowserClient.Login(); @@ -82,16 +81,14 @@ public class RevokeRefreshTokenTests(ITestOutputHelper output) : BffTestBase(out [Theory, MemberData(nameof(AllSetups))] public async Task backchannel_logout_endpoint_should_revoke_refreshtoken(BffSetupType setup) { - ConfigureBff(setup, configureOpenIdConnect: options => + Bff.OnConfigureBff += bff => bff.AddServerSideSessions(); + + await ConfigureBff(setup, configureOpenIdConnect: options => { The.DefaultOpenIdConnectConfiguration(options); options.Scope.Add("offline_access"); }); - Bff.OnConfigureBff += bff => bff.AddServerSideSessions(); - - await InitializeAsync(); - foreach (var client in IdentityServer.Clients) { client.BackChannelLogoutUri = Bff.Url("/bff/backchannel").ToString(); @@ -123,9 +120,10 @@ public class RevokeRefreshTokenTests(ITestOutputHelper output) : BffTestBase(out } [Theory, MemberData(nameof(AllSetups))] - public async Task when_setting_disabled_backchannel_logout_endpoint_should_not_revoke_refreshtoken(BffSetupType setup) + public async Task when_setting_disabled_backchannel_logout_endpoint_should_not_revoke_refreshtoken( + BffSetupType setup) { - ConfigureBff(setup, configureOpenIdConnect: options => + await ConfigureBff(setup, configureOpenIdConnect: options => { The.DefaultOpenIdConnectConfiguration(options); options.Scope.Add("offline_access"); @@ -133,7 +131,7 @@ public class RevokeRefreshTokenTests(ITestOutputHelper output) : BffTestBase(out Bff.OnConfigureBff += bff => bff.AddServerSideSessions(); - await InitializeAsync(); + Bff.BffOptions.RevokeRefreshTokenOnLogout = false; foreach (var client in IdentityServer.Clients) diff --git a/bff/test/Bff.Tests/SessionManagement/ServerSideTicketStoreTests.cs b/bff/test/Bff.Tests/SessionManagement/ServerSideTicketStoreTests.cs index 021bc1027..e2be4890a 100644 --- a/bff/test/Bff.Tests/SessionManagement/ServerSideTicketStoreTests.cs +++ b/bff/test/Bff.Tests/SessionManagement/ServerSideTicketStoreTests.cs @@ -12,16 +12,15 @@ public class ServerSideTicketStoreTests : BffTestBase private readonly InMemoryUserSessionStore _sessionStore = new(); public ServerSideTicketStoreTests(ITestOutputHelper output) : base(output) => Bff.OnConfigureServices += services => - { - services.AddSingleton(_sessionStore); - }; + { + services.AddSingleton(_sessionStore); + }; [Theory, MemberData(nameof(AllSetups))] public async Task StoreAsync_should_remove_conflicting_entries_prior_to_creating_new_entry(BffSetupType setup) { Bff.OnConfigureBff += bff => bff.AddServerSideSessions(); - ConfigureBff(setup); - await InitializeAsync(); + await ConfigureBff(setup); await Bff.BrowserClient.Login(); Bff.BrowserClient.Cookies.Clear(Bff.Url()); diff --git a/bff/test/Bff.Tests/TestInfra/ApiHost.cs b/bff/test/Bff.Tests/TestInfra/ApiHost.cs index dd97bc730..d11bcbe9e 100644 --- a/bff/test/Bff.Tests/TestInfra/ApiHost.cs +++ b/bff/test/Bff.Tests/TestInfra/ApiHost.cs @@ -94,7 +94,7 @@ public class ApiHost : TestHost } else { - throw new Exception("Invalid LocalApiResponseStatus"); + throw new InvalidOperationException("Invalid LocalApiResponseStatus"); } } } diff --git a/bff/test/Bff.Tests/TestInfra/BffTestBase.cs b/bff/test/Bff.Tests/TestInfra/BffTestBase.cs index 1ed732211..823370d95 100644 --- a/bff/test/Bff.Tests/TestInfra/BffTestBase.cs +++ b/bff/test/Bff.Tests/TestInfra/BffTestBase.cs @@ -46,10 +46,10 @@ public abstract class BffTestBase : IAsyncDisposable Some = Context.Some; } - protected void ConfigureBff(BffSetupType setup, - Action? configureCookies = null, - Action? configureOpenIdConnect = null - ) + protected virtual async Task ConfigureBff( + BffSetupType setup, + Action? configureCookie = null, + Action? configureOpenIdConnect = null) { // This method is used to configure the BFF in different ways depending on the setup type. Action openIdConfiguration = opt => @@ -71,7 +71,7 @@ public abstract class BffTestBase : IAsyncDisposable { ConfigureCookieOptions = options => { - configureCookies?.Invoke(options); + configureCookie?.Invoke(options); }, ConfigureOpenIdConnectOptions = openIdConfiguration }); @@ -84,7 +84,7 @@ public abstract class BffTestBase : IAsyncDisposable bff.WithDefaultOpenIdConnectOptions(openIdConfiguration); bff.WithDefaultCookieOptions(options => { - configureCookies?.Invoke(options); + configureCookie?.Invoke(options); }); }; } @@ -103,7 +103,7 @@ public abstract class BffTestBase : IAsyncDisposable }) .AddCookie("cookie", options => { - configureCookies?.Invoke(options); + configureCookie?.Invoke(options); }) .AddOpenIdConnect("oidc", openIdConfiguration); }; @@ -113,6 +113,8 @@ public abstract class BffTestBase : IAsyncDisposable throw new ArgumentOutOfRangeException(nameof(setup), setup, null); } + + await InitializeAsync(); } protected virtual void Initialize() diff --git a/bff/test/Bff.Tests/TestInfra/BffTestHost.cs b/bff/test/Bff.Tests/TestInfra/BffTestHost.cs index e653b7254..43cae6b61 100644 --- a/bff/test/Bff.Tests/TestInfra/BffTestHost.cs +++ b/bff/test/Bff.Tests/TestInfra/BffTestHost.cs @@ -10,7 +10,8 @@ using Yarp.ReverseProxy.Forwarder; namespace Duende.Bff.Tests.TestInfra; -public class BffTestHost(TestHostContext context, IdentityServerTestHost identityServer) : TestHost(context, new Uri("https://bff")) +public class BffTestHost(TestHostContext context, IdentityServerTestHost identityServer) + : TestHost(context, new Uri("https://bff")) { public readonly string DefaultRootResponse = "Default response from root"; private BffHttpClient _browserClient = null!; @@ -23,7 +24,6 @@ public class BffTestHost(TestHostContext context, IdentityServerTestHost identit public bool MapGetForRoot { get; set; } = true; public bool EnableBackChannelHandler { get; set; } = true; - public event Action SetBffOptions = _ => { }; public event Action OnConfigureBff = _ => { }; public override void Initialize() @@ -41,20 +41,16 @@ public class BffTestHost(TestHostContext context, IdentityServerTestHost identit OnConfigureServices += services => { - if (EnableBackChannelHandler) + services.AddSingleton( + new CallbackForwarderHttpClientFactory(context => new HttpMessageInvoker(Internet))); + + var builder = services.AddBff(options => { - SetBffOptions += options => + if (EnableBackChannelHandler) { options.BackchannelMessageHandler = Internet; - }; - } - - services.AddSingleton( - new CallbackForwarderHttpClientFactory( - context => new HttpMessageInvoker(Internet))); - - - var builder = services.AddBff(SetBffOptions); + } + }); OnConfigureBff(builder); }; @@ -65,7 +61,6 @@ public class BffTestHost(TestHostContext context, IdentityServerTestHost identit { endpoints.MapGet("/", () => DefaultRootResponse); } - }; } diff --git a/bff/test/Bff.Tests/TestInfra/ResponseExtensions.cs b/bff/test/Bff.Tests/TestInfra/ResponseExtensions.cs index 4a49ff7a5..d4c5bacf6 100644 --- a/bff/test/Bff.Tests/TestInfra/ResponseExtensions.cs +++ b/bff/test/Bff.Tests/TestInfra/ResponseExtensions.cs @@ -14,7 +14,7 @@ public static class ResponseExtensions if (response.StatusCode != statusCode) { var content = await response.Content.ReadAsStringAsync(); - throw new Exception($"Expected {statusCode} but got {response.StatusCode}. Content: {content}"); + throw new InvalidOperationException($"Expected {statusCode} but got {response.StatusCode}. Content: {content}"); } return response;