diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json b/src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json
index 47da6004a7..3506c9f76c 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json
@@ -541,6 +541,9 @@
"src/Tests/Features/Identity/TwoFactorAuthTests.cs",
"src/Tests/Features/Identity/PrivilegedSessionTests.cs",
"src/Tests/Features/Identity/RefreshTokenRotationTests.cs",
+ "src/Tests/Features/Identity/ReturnUrlHardeningTests.cs",
+ "src/Tests/Features/Identity/OneTimeTokenTests.cs",
+ "src/Tests/Features/Identity/ConfirmPageTwoFactorTests.cs",
"src/Tests/Features/Identity/UserGroupFeatureManagementUITests.cs",
"src/Tests/Features/Seo/**",
"src/Tests/Features/Tenants/TenantInvitationUITests.cs",
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/ConfirmPage.razor.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/ConfirmPage.razor.cs
index f8e334db6c..bdcbbf53ba 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/ConfirmPage.razor.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/ConfirmPage.razor.cs
@@ -31,6 +31,8 @@ public partial class ConfirmPage
[Parameter, SupplyParameterFromQuery(Name = "phoneToken")]
public string? PhoneTokenQueryString { get; set; }
+ private string GetSafeReturnUrl() => Uri.IsAppRelativeUrl(ReturnUrlQueryString, requireLeadingSlash: false) ? ReturnUrlQueryString : PageUrls.Home;
+
protected override async Task OnInitAsync()
{
@@ -84,9 +86,18 @@ await WrapRequest(async () =>
Token = emailModel.Token
}, CurrentCancellationToken);
+ if (signInResponse.RequiresTwoFactor)
+ {
+ // The e-mail is confirmed, but the automatic sign-in needs a second factor this page cannot collect
+ // (ConfirmEmailRequestDto carries no code). Storing the response would overwrite the caller's tokens
+ // with nulls, so show the "confirmed, now sign in" panel instead.
+ isEmailConfirmed = true;
+ return;
+ }
+
await AuthManager.StoreTokens(signInResponse, true);
- NavigationManager.NavigateTo(ReturnUrlQueryString ?? PageUrls.Home, replace: true);
+ NavigationManager.NavigateTo(GetSafeReturnUrl(), replace: true);
isEmailConfirmed = true;
});
@@ -98,7 +109,7 @@ private async Task ResendEmailToken()
await WrapRequest(async () =>
{
- await identityController.SendConfirmEmailToken(new() { Email = emailModel.Email, ReturnUrl = ReturnUrlQueryString }, CurrentCancellationToken);
+ await identityController.SendConfirmEmailToken(new() { Email = emailModel.Email, ReturnUrl = GetSafeReturnUrl() }, CurrentCancellationToken);
});
}
@@ -114,9 +125,16 @@ await WrapRequest(async () =>
PhoneNumber = phoneModel.PhoneNumber
}, CurrentCancellationToken);
+ if (signInResponse.RequiresTwoFactor)
+ {
+ // See the note in ConfirmEmail above.
+ isPhoneConfirmed = true;
+ return;
+ }
+
await AuthManager.StoreTokens(signInResponse, true);
- NavigationManager.NavigateTo(ReturnUrlQueryString ?? PageUrls.Home, replace: true);
+ NavigationManager.NavigateTo(GetSafeReturnUrl(), replace: true);
isPhoneConfirmed = true;
});
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor
index 7b70823bd2..04b0a0f84a 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor
@@ -68,7 +68,7 @@
@Localizer[nameof(AppStrings.Password)]
- @Localizer[nameof(AppStrings.ForgotPasswordLink)]
+ @Localizer[nameof(AppStrings.ForgotPasswordLink)]
@@ -138,7 +138,7 @@
{
@Localizer[nameof(AppStrings.DontHaveAccountMessage)]
- @Localizer[nameof(AppStrings.SignUp)]
+ @Localizer[nameof(AppStrings.SignUp)]
}
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs
index e3c8afe3c6..63ba077fc6 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs
@@ -18,7 +18,12 @@ public partial class SignInPanel
private SignInPanelType internalSignInPanelType;
private readonly SignInRequestDto model = new();
private AppDataAnnotationsValidator? validatorRef;
- private string GetReturnUrl() => ReturnUrl ?? ReturnUrlQueryString ?? PageUrls.Home;
+ private string GetSafeReturnUrl()
+ {
+ var returnUrl = ReturnUrl ?? ReturnUrlQueryString;
+
+ return Uri.IsAppRelativeUrl(returnUrl, requireLeadingSlash: false) ? returnUrl : PageUrls.Home;
+ }
[Parameter]
public string? ReturnUrl { get; set; }
@@ -143,7 +148,7 @@ private async Task DoSignIn()
if (isNewUser is false)
{
- model.ReturnUrl = GetReturnUrl();
+ model.ReturnUrl = GetSafeReturnUrl();
requiresTwoFactor = await AuthManager.SignIn(model, CurrentCancellationToken);
@@ -187,7 +192,7 @@ private async Task DoSignIn()
}
else
{
- NavigationManager.NavigateTo(GetReturnUrl(), replace: true);
+ NavigationManager.NavigateTo(GetSafeReturnUrl(), replace: true);
}
}
}
@@ -231,7 +236,7 @@ private async Task ExternalSignIn(string provider)
queryParams.TryGetValue("return-url", out var returnUrl);
var returnUrlValue = GetValue(returnUrl);
- ReturnUrlQueryString = Uri.IsAppRelativeUrl(returnUrlValue) ? returnUrlValue : PageUrls.Home;
+ ReturnUrlQueryString = Uri.IsAppRelativeUrl(returnUrlValue, requireLeadingSlash: false) ? returnUrlValue : PageUrls.Home;
queryParams.TryGetValue("userName", out var userName);
UserNameQueryString = GetValue(userName);
queryParams.TryGetValue("email", out var email);
@@ -248,7 +253,7 @@ private async Task ExternalSignIn(string provider)
var port = localHttpServer.EnsureStarted();
- var redirectUrl = await identityController.GetExternalSignInUri(provider, GetReturnUrl(), port is -1 ? null : port, CurrentCancellationToken);
+ var redirectUrl = await identityController.GetExternalSignInUri(provider, GetSafeReturnUrl(), port is -1 ? null : port, CurrentCancellationToken);
await externalNavigationService.NavigateTo(redirectUrl);
}
@@ -324,7 +329,7 @@ private async Task SendOtp(bool resend)
var request = new IdentityRequestDto { UserName = model.UserName, Email = model.Email, PhoneNumber = model.PhoneNumber };
- await identityController.SendOtp(request, GetReturnUrl(), CurrentCancellationToken);
+ await identityController.SendOtp(request, GetSafeReturnUrl(), CurrentCancellationToken);
isOtpSent = true;
}
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/NotAuthorizedPage.razor.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/NotAuthorizedPage.razor.cs
index f53e3a21b3..0ba7646731 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/NotAuthorizedPage.razor.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/NotAuthorizedPage.razor.cs
@@ -9,6 +9,7 @@ public partial class NotAuthorizedPage
[SupplyParameterFromQuery(Name = "return-url"), Parameter]
public string? ReturnUrl { get; set; }
+ private string GetSafeReturnUrl() => Uri.IsAppRelativeUrl(ReturnUrl, requireLeadingSlash: false) ? ReturnUrl : PageUrls.Home;
[AutoInject] private SignInModalService signInModalService = default!;
@@ -28,9 +29,10 @@ protected override async Task OnAfterFirstRenderAsync()
var accessToken = await AuthManager.RefreshToken(requestedBy: nameof(NotAuthorizedPage));
if (string.IsNullOrEmpty(accessToken) is false && ReturnUrl is not null && ReturnUrl.Contains("try_refreshing_token=false", StringComparison.InvariantCulture) is false)
{
+ var returnUrl = GetSafeReturnUrl();
// To prevent infinities redirect loop, let's append try_refreshing_token=false to the url, so we only redirect in case no try_refreshing_token=false is present
- var @char = ReturnUrl.Contains('?') ? '&' : '?'; // The RedirectUrl may already include a query string.
- NavigationManager.NavigateTo($"{ReturnUrl}{@char}try_refreshing_token=false", replace: true);
+ var @char = returnUrl.Contains('?') ? '&' : '?'; // The RedirectUrl may already include a query string.
+ NavigationManager.NavigateTo($"{returnUrl}{@char}try_refreshing_token=false", replace: true);
}
}
@@ -48,7 +50,7 @@ protected override async Task OnAfterFirstRenderAsync()
private async Task SignIn()
{
await AuthManager.SignOut(CurrentCancellationToken);
- var returnUrl = ReturnUrl ?? NavigationManager.GetRelativePath();
+ var returnUrl = ReturnUrl is null ? NavigationManager.GetRelativePath() : GetSafeReturnUrl();
await signInModalService.SignIn(returnUrl);
// Alternatively, you can redirect the user to the sign-in page.
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.EmailConfirmation.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.EmailConfirmation.cs
index 63e888d3a7..426776d2a3 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.EmailConfirmation.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.EmailConfirmation.cs
@@ -28,7 +28,8 @@ public async Task ConfirmEmail(ConfirmEmailRequestDto request, CancellationToken
var user = await userManager.FindByEmailAsync(request.Email!)
?? throw new BadRequestException(Localizer[nameof(AppStrings.UserNotFound)]).WithData("Email", request.Email);
- var expired = (TimeProvider.GetUtcNow() - user.EmailTokenRequestedOn) > AppSettings.Identity.EmailTokenLifetime;
+ var expired = user.EmailTokenRequestedOn is null ||
+ (TimeProvider.GetUtcNow() - user.EmailTokenRequestedOn.Value) > AppSettings.Identity.EmailTokenLifetime;
if (expired)
throw new BadRequestException(nameof(AppStrings.ExpiredToken)).WithData("UserId", user.Id);
@@ -39,7 +40,7 @@ public async Task ConfirmEmail(ConfirmEmailRequestDto request, CancellationToken
throw new BadRequestException(Localizer[nameof(AppStrings.UserLockedOut), (TimeProvider.GetUtcNow() - user.LockoutEnd!).Value.Humanize(culture: CultureInfo.CurrentUICulture)]).WithData("UserId", user.Id).WithExtensionData("TryAgainIn", tryAgainIn);
}
- var tokenIsValid = await userManager.VerifyUserTokenAsync(user, TokenOptions.DefaultPhoneProvider, FormattableString.Invariant($"VerifyEmail:{request.Email},{user.EmailTokenRequestedOn?.ToUniversalTime()}"), request.Token!);
+ var tokenIsValid = await userManager.VerifyUserTokenAsync(user, TokenOptions.DefaultPhoneProvider, FormattableString.Invariant($"VerifyEmail:{user.Email},{user.EmailTokenRequestedOn?.ToUniversalTime()}"), request.Token!);
if (tokenIsValid is false)
{
@@ -66,7 +67,10 @@ public async Task ConfirmEmail(ConfirmEmailRequestDto request, CancellationToken
private async Task SendConfirmEmailToken(User user, string? returnUrl, CancellationToken cancellationToken)
{
- returnUrl ??= PageUrls.Home;
+ if (Uri.IsAppRelativeUrl(returnUrl, requireLeadingSlash: false) is false)
+ {
+ returnUrl = PageUrls.Home;
+ }
var resendDelay = (TimeProvider.GetUtcNow() - user.EmailTokenRequestedOn) - AppSettings.Identity.EmailTokenLifetime;
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ExternalSignIn.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ExternalSignIn.cs
index 4486095c65..3a4e09c32c 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ExternalSignIn.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ExternalSignIn.cs
@@ -11,8 +11,9 @@ public partial class IdentityController
///
/// Deliberately not cached: the returned URL embeds the caller's origin, which GetWebAppUrl resolves from the X-Origin
- /// header. That header is part of neither the output cache's vary rules nor a CDN's cache key, so a cached response would
- /// hand one origin's sign-in URL to a caller coming from another, sending the resulting sign-in link to the wrong origin.
+ /// header. The output cache does vary by that header (See AppResponseCachePolicy.CacheRequestAsync), but a CDN edge
+ /// does not unless it is configured to, so a shared-cached response would hand one origin's sign-in URL to a caller
+ /// coming from another, sending the resulting sign-in link to the wrong origin.
///
[HttpGet]
public async Task GetExternalSignInUri(string provider, string? returnUrl = null, int? localHttpPort = null, CancellationToken cancellationToken = default)
@@ -38,6 +39,9 @@ public async Task ExternalSignInCallback(string? returnUrl = null,
string? signInPageUri;
ExternalLoginInfo? info = null;
+ if (localHttpPort is not null and (< 1 or > 65535))
+ throw new BadRequestException().WithData("localHttpPort", localHttpPort);
+
try
{
info = await signInManager.GetExternalLoginInfoAsync() ?? throw new BadRequestException().WithData("Reason", "External login info is missing.");
@@ -46,11 +50,15 @@ public async Task ExternalSignInCallback(string? returnUrl = null,
var user = await userManager.FindByLoginAsync(info.LoginProvider, info.ProviderKey);
+ var isLinked = user is not null;
+
if (user is null && (string.IsNullOrEmpty(email) is false || string.IsNullOrEmpty(phoneNumber) is false))
{
user = await userManager.FindUser(new() { Email = email, PhoneNumber = phoneNumber });
}
+ var isNewUser = user is null;
+
if (user is null)
{
var name = info.Principal.FindFirstValue("preferred_username") ?? info.Principal.FindFirstValue(ClaimTypes.Name) ?? info.Principal.FindFirstValue(ClaimTypes.NameIdentifier) ?? info.Principal.FindFirstValue("name");
@@ -86,8 +94,24 @@ public async Task ExternalSignInCallback(string? returnUrl = null,
// Therefore, we assign a role to these users by default.
await userManager.CreateUserWithDemoRole(user);
}
+ }
- await userManager.AddLoginAsync(user, info);
+ if (isLinked is false)
+ {
+ var addLoginResult = await userManager.AddLoginAsync(user, info);
+
+ if (addLoginResult.Succeeded is false)
+ {
+ if (isNewUser)
+ {
+ // Only this request could reach that row: a provider asserting neither an email nor a phone
+ // number leaves the identifier fallback above nothing to match it by, so without the login it
+ // would be unreachable forever and every retry would add another one.
+ await userManager.DeleteAsync(user);
+ }
+
+ throw new ResourceValidationException(addLoginResult.Errors.Select(e => new LocalizedString(e.Code, e.Description)).ToArray()).WithData("UserId", user.Id);
+ }
}
// Confirmation is only as good as the provider's own verification of the identifier
@@ -130,7 +154,11 @@ public async Task ExternalSignInCallback(string? returnUrl = null,
}
catch (Exception exp)
{
- serverExceptionHandler.Handle(exp, new() { { "LoginProvider", info?.LoginProvider }, { "Principal", info?.Principal?.GetDisplayName() } });
+ var principalName = info?.Principal?.FindFirstValue("preferred_username")
+ ?? info?.Principal?.GetEmail()
+ ?? info?.Principal?.FindFirstValue(ClaimTypes.NameIdentifier);
+
+ serverExceptionHandler.Handle(exp, new() { { "LoginProvider", info?.LoginProvider }, { "Principal", principalName } });
signInPageUri = $"{PageUrls.SignIn}?error={Uri.EscapeDataString(exp is KnownException ? Localizer[exp.Message] : Localizer[nameof(AppStrings.UnknownException)])}";
}
finally
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.PhoneConfirmation.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.PhoneConfirmation.cs
index 28ef99bf5f..5119abfa44 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.PhoneConfirmation.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.PhoneConfirmation.cs
@@ -30,7 +30,8 @@ public async Task ConfirmPhone(ConfirmPhoneRequestDto request, CancellationToken
var user = await userManager.FindByPhoneNumber(request.PhoneNumber!)
?? throw new BadRequestException(Localizer[nameof(AppStrings.UserNotFound)]).WithData("PhoneNumber", request.PhoneNumber);
- var expired = (TimeProvider.GetUtcNow() - user.PhoneNumberTokenRequestedOn) > AppSettings.Identity.PhoneNumberTokenLifetime;
+ var expired = user.PhoneNumberTokenRequestedOn is null ||
+ (TimeProvider.GetUtcNow() - user.PhoneNumberTokenRequestedOn.Value) > AppSettings.Identity.PhoneNumberTokenLifetime;
if (expired)
throw new BadRequestException(nameof(AppStrings.ExpiredToken)).WithData("UserId", user.Id);
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ResetPassword.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ResetPassword.cs
index c09ad99671..9e4736d6f3 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ResetPassword.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ResetPassword.cs
@@ -39,7 +39,8 @@ public async Task SendResetPasswordToken(SendResetPasswordTokenRequestDto reques
var token = await userManager.GenerateUserTokenAsync(user, TokenOptions.DefaultPhoneProvider, FormattableString.Invariant($"ResetPassword,{user.ResetPasswordTokenRequestedOn?.ToUniversalTime()}"));
var isEmail = string.IsNullOrEmpty(request.Email) is false;
var qs = $"{(isEmail ? "email" : "phoneNumber")}={Uri.EscapeDataString(isEmail ? request.Email! : request.PhoneNumber!)}";
- var url = $"{PageUrls.ResetPassword}?token={Uri.EscapeDataString(token)}&{qs}&culture={CultureInfo.CurrentUICulture.Name}&return-url={Uri.EscapeDataString(request.ReturnUrl ?? PageUrls.Home)}";
+ var returnUrl = Uri.IsAppRelativeUrl(request.ReturnUrl, requireLeadingSlash: false) ? request.ReturnUrl : PageUrls.Home;
+ var url = $"{PageUrls.ResetPassword}?token={Uri.EscapeDataString(token)}&{qs}&culture={CultureInfo.CurrentUICulture.Name}&return-url={Uri.EscapeDataString(returnUrl)}";
var link = new Uri(HttpContext.Request.GetWebAppUrl(), url);
List sendMessagesTasks = [];
@@ -82,7 +83,8 @@ public async Task ResetPassword(ResetPasswordRequestDto request, CancellationTok
request.PhoneNumber = phoneService.NormalizePhoneNumber(request.PhoneNumber);
var user = await userManager.FindUser(request) ?? throw new ResourceNotFoundException(Localizer[nameof(AppStrings.UserNotFound)]).WithData("Identifier", request);
- var expired = (TimeProvider.GetUtcNow() - user.ResetPasswordTokenRequestedOn) > AppSettings.Identity.ResetPasswordTokenLifetime;
+ var expired = user.ResetPasswordTokenRequestedOn is null ||
+ (TimeProvider.GetUtcNow() - user.ResetPasswordTokenRequestedOn.Value) > AppSettings.Identity.ResetPasswordTokenLifetime;
if (expired)
throw new BadRequestException(nameof(AppStrings.ExpiredToken)).WithData("UserId", user.Id);
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/Services/RemoteAuthenticationOptionsConfigurator.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/Services/RemoteAuthenticationOptionsConfigurator.cs
index d5fa3ddbc3..641ccaffea 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/Services/RemoteAuthenticationOptionsConfigurator.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/Services/RemoteAuthenticationOptionsConfigurator.cs
@@ -10,15 +10,25 @@
namespace Boilerplate.Server.Api.Features.Identity.Services;
///
-/// Configures External Identity Providers BackchannelHttpHandler to use Microsoft.Extensions.HttpClient Factory
+/// Routes every external identity provider's backchannel through Microsoft.Extensions.Http's client factory, so the
+/// named-client configuration (See ConfigureHttpClientFactoryForExternalIdentityProviders) and everything
+/// ConfigureHttpClientDefaults applies - resilience, service discovery, the SocketsHttpHandler settings - reach it.
+///
+/// OpenIdConnect and Apple are configured through rather than
+/// on purpose. Their own framework post-configure builds a
+/// ConfigurationManager around whatever Backchannel it finds and keeps that instance for the process lifetime,
+/// so it has to already be the factory's client by then; assigning it afterwards leaves the discovery-document and
+/// JWKS fetches on a bare HttpClient while only the token/userinfo calls get the configured one. The OAuth-family
+/// handlers capture nothing, so they stay on post-configure.
+///
///
public class RemoteAuthenticationOptionsConfigurator(IHttpClientFactory httpClientFactory)
- : IPostConfigureOptions,
+ : IConfigureNamedOptions,
+ IConfigureNamedOptions,
IPostConfigureOptions,
IPostConfigureOptions,
IPostConfigureOptions,
IPostConfigureOptions,
- IPostConfigureOptions,
IPostConfigureOptions
{
public void PostConfigure(string? name, RemoteAuthenticationOptions options)
@@ -26,32 +36,42 @@ public void PostConfigure(string? name, RemoteAuthenticationOptions options)
options.Backchannel = httpClientFactory.CreateClient(name!);
}
- public void PostConfigure(string? name, OpenIdConnectOptions options)
+ public void Configure(string? name, OpenIdConnectOptions options)
{
PostConfigure(name, (RemoteAuthenticationOptions)options);
}
- public void PostConfigure(string? name, GitHubAuthenticationOptions options)
+ public void Configure(OpenIdConnectOptions options)
+ {
+ Configure(Options.DefaultName, options);
+ }
+
+ public void Configure(string? name, AppleAuthenticationOptions options)
{
PostConfigure(name, (RemoteAuthenticationOptions)options);
}
- public void PostConfigure(string? name, TwitterOptions options)
+ public void Configure(AppleAuthenticationOptions options)
+ {
+ Configure(Options.DefaultName, options);
+ }
+
+ public void PostConfigure(string? name, GitHubAuthenticationOptions options)
{
PostConfigure(name, (RemoteAuthenticationOptions)options);
}
- public void PostConfigure(string? name, FacebookOptions options)
+ public void PostConfigure(string? name, TwitterOptions options)
{
PostConfigure(name, (RemoteAuthenticationOptions)options);
}
- public void PostConfigure(string? name, MicrosoftIdentityOptions options)
+ public void PostConfigure(string? name, FacebookOptions options)
{
PostConfigure(name, (RemoteAuthenticationOptions)options);
}
- public void PostConfigure(string? name, AppleAuthenticationOptions options)
+ public void PostConfigure(string? name, MicrosoftIdentityOptions options)
{
PostConfigure(name, (RemoteAuthenticationOptions)options);
}
@@ -96,6 +116,9 @@ public void ConfigureHttpClientFactoryForExternalIdentityProviders()
});
+ // Program.Services.cs also registers a "Keycloak" client (with its BaseAddress, which
+ // AppUserClaimsPrincipalFactory's relative POST depends on). AddHttpClient accumulates rather than
+ // replaces, so both configure actions run; this one exists so the scheme always has a named client.
services.AddHttpClient("Keycloak", httpClient =>
{
@@ -109,11 +132,14 @@ public void ConfigureHttpClientFactoryForExternalIdentityProviders()
services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
- services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
- services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
+ // Configure, not PostConfigure - see the note on RemoteAuthenticationOptionsConfigurator. The framework's
+ // own post-configure for these two captures Backchannel into a ConfigurationManager it keeps forever, so
+ // ours has to run first or the metadata/JWKS channel never sees the factory's client.
+ services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
+ services.TryAddEnumerable(ServiceDescriptor.Singleton, RemoteAuthenticationOptionsConfigurator>());
}
}
}
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Infrastructure/Services/PhoneService.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Infrastructure/Services/PhoneService.cs
index 53c82bdb59..e366604cda 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Infrastructure/Services/PhoneService.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Infrastructure/Services/PhoneService.cs
@@ -5,6 +5,7 @@ namespace Boilerplate.Server.Api.Infrastructure.Services;
public partial class PhoneService
{
[AutoInject] private readonly ServerApiSettings appSettings = default!;
+ [AutoInject] private readonly IStringLocalizer localizer = default!;
[AutoInject] private readonly PhoneNumberUtil phoneNumberUtil = default!;
[AutoInject] private readonly IHostEnvironment hostEnvironment = default!;
[AutoInject] private readonly ILogger phoneLogger = default!;
@@ -21,9 +22,23 @@ public partial class PhoneService
? value.ToString()
: new RegionInfo((CultureInfoManager.InvariantGlobalization is false ? CultureInfo.CurrentUICulture : CultureInfoManager.DefaultCulture).Name).TwoLetterISORegionName;
- var parsedPhoneNumber = phoneNumberUtil.Parse(phoneNumber, region);
+ try
+ {
+ var parsedPhoneNumber = phoneNumberUtil.Parse(phoneNumber, region);
- return phoneNumberUtil.Format(parsedPhoneNumber, PhoneNumberFormat.E164);
+ return phoneNumberUtil.Format(parsedPhoneNumber, PhoneNumberFormat.E164);
+ }
+ catch (NumberParseException exp)
+ {
+ // libphonenumber is far stricter than the [Phone] data annotation the DTOs carry, so a value that passed
+ // model validation can still land here - "+1" does. The region is caller-influenced too: Cloudflare sends
+ // XX for unknown geography and T1 for Tor, and neither is a region libphonenumber accepts, which makes any
+ // national-format number throw for a perfectly legitimate visitor. Unhandled this surfaces as a 500 logged
+ // at Critical on an anonymous endpoint; it is a bad request.
+ throw new BadRequestException(localizer[nameof(AppStrings.PhoneAttribute_ValidationError), localizer[nameof(AppStrings.PhoneNumber)]])
+ .WithData("Reason", exp.Message)
+ .WithData("Region", region);
+ }
}
public virtual async Task SendSms(string messageText, string phoneNumber)
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Infrastructure/Extensions/UriExtensions.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Infrastructure/Extensions/UriExtensions.cs
index 5611100894..ea75b352b7 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Infrastructure/Extensions/UriExtensions.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Infrastructure/Extensions/UriExtensions.cs
@@ -4,28 +4,6 @@ namespace System;
public static partial class UriExtensions
{
- extension(Uri)
- {
- ///
- /// True only for an app-rooted relative url such as /sign-in?otp=123.
- ///
- /// with is NOT enough on its own:
- /// a network-path reference like //attacker.com/path is a well-formed RELATIVE reference, and both
- /// browsers and NavigationManager.NavigateTo resolve it to https://attacker.com/path.
- /// Browsers treat /\host the same way, so the second character is rejected too.
- ///
- /// Use this wherever a url arrives from outside the app and is then navigated to - the Blazor Hybrid local
- /// HTTP server's external-sign-in callback and the sign-in / sign-up pages that consume it.
- ///
- public static bool IsAppRelativeUrl([NotNullWhen(true)] string? url)
- {
- return string.IsNullOrEmpty(url) is false
- && url.StartsWith('/')
- && (url.Length is 1 || url[1] is not ('/' or '\\'))
- && Uri.IsWellFormedUriString(url, UriKind.Relative);
- }
- }
-
extension(Uri uri)
{
public string GetUrlWithoutQueryParameter(string key)
@@ -96,5 +74,43 @@ public string GetPath()
var uriBuilder = new UriBuilder(uri.GetUrlWithoutCulture()) { Query = string.Empty, Fragment = string.Empty };
return uriBuilder.Path;
}
+
+ ///
+ /// True only for a relative url that cannot leave this app's origin, such as /sign-in?otp=123.
+ ///
+ /// with is NOT enough on its own:
+ /// a network-path reference like //attacker.com/path is a well-formed RELATIVE reference, and both
+ /// browsers and NavigationManager.NavigateTo resolve it to https://attacker.com/path.
+ /// Browsers treat /\host and a leading backslash the same way, so those are rejected too.
+ ///
+ /// Use this wherever a url arrives from outside the app and is then navigated to - the Blazor Hybrid local
+ /// HTTP server's external-sign-in callback, and every return-url that reaches
+ /// NavigationManager.NavigateTo.
+ ///
+ ///
+ /// Keep the default when the url is expected to be app-rooted (/products/1). Pass false for a
+ /// return-url, because the app itself produces base-relative values without the leading slash - see
+ /// NavigationManagerExtensions.GetRelativePath, which AppShell and SignInModalService
+ /// feed straight into a return-url. Rootless values are equally origin-bound: a well-formed relative
+ /// reference carries no scheme and no authority, so it always resolves against the app's own base.
+ ///
+ public static bool IsAppRelativeUrl([NotNullWhen(true)] string? url, bool requireLeadingSlash = true)
+ {
+ if (string.IsNullOrEmpty(url))
+ return false;
+
+ if (url[0] is '\\')
+ return false;
+
+ if (url[0] is '/')
+ {
+ if (url.Length > 1 && url[1] is '/' or '\\')
+ return false;
+ }
+ else if (requireLeadingSlash)
+ return false;
+
+ return Uri.IsWellFormedUriString(url, UriKind.Relative);
+ }
}
}
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/BunitUITests.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/BunitUITests.cs
index cec257cbde..50189ea243 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/BunitUITests.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/BunitUITests.cs
@@ -52,9 +52,9 @@ public async Task SignIn_Should_WorkAsExpected()
// user is the seeded default account.
var authenticationStateProvider = ctx.Services.GetRequiredService();
- cut.WaitForAssertion(() =>
+ await cut.WaitForAssertionAsync(async () =>
{
- var user = authenticationStateProvider.GetAuthenticationStateAsync().GetAwaiter().GetResult().User;
+ var user = (await authenticationStateProvider.GetAuthenticationStateAsync()).User;
Assert.IsTrue(user.IsAuthenticated());
Assert.AreEqual(Guid.Parse("8ff71671-a1d6-4f97-abb9-d87d7b47d6e7"), user.GetUserId());
}, timeout: TimeSpan.FromSeconds(30));
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/ConfirmPageTwoFactorTests.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/ConfirmPageTwoFactorTests.cs
new file mode 100644
index 0000000000..c1f896e80f
--- /dev/null
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/ConfirmPageTwoFactorTests.cs
@@ -0,0 +1,105 @@
+using Bunit;
+using Microsoft.EntityFrameworkCore;
+using Boilerplate.Shared.Features.Identity;
+using Boilerplate.Server.Api.Infrastructure.Data;
+using Boilerplate.Tests.Infrastructure.Services;
+using Boilerplate.Server.Api.Features.Identity.Models;
+using Microsoft.AspNetCore.Components.Authorization;
+using Boilerplate.Client.Core.Components.Pages.Identity;
+using Boilerplate.Client.Core.Infrastructure.Services.Contracts;
+
+namespace Boilerplate.Tests.Features.Identity;
+
+///
+/// Confirming an identifier ends with an automatic sign-in, and that sign-in answers
+/// { requiresTwoFactor: true } - HTTP 200, with null tokens and no session row - whenever the account has
+/// two-factor enabled. ConfirmEmailRequestDto carries no second-factor code, so the flow cannot be completed
+/// from this page at all; the only correct behaviour is to confirm and stop.
+///
+/// Storing that response instead is the failure this pins, and it is worse than it looks: on the Web client the null
+/// crosses JS interop and Storage.setItem coerces it, so access_token becomes the literal string
+/// "null", which is non-empty enough to pass the token provider's guard and then throws while being parsed - an
+/// interrupting error dialog on every auth-state evaluation until the tokens are cleared, on top of the sign-out.
+///
+///
+[TestClass, TestCategory("UITest")]
+public class ConfirmPageTwoFactorTests
+{
+ [TestMethod]
+ public async Task ConfirmEmail_Should_NotStoreTokens_When_TheAccountRequiresASecondFactor()
+ {
+ await using var server = new AppTestServer();
+ await server.Build().Start(TestContext.CancellationToken);
+
+ var (email, token) = await CreateUnconfirmedTwoFactorAccount(server);
+
+ await using var ctx = server.CreateBunitContext();
+
+ var storageService = ctx.Services.GetRequiredService();
+ await storageService.SetItem("access_token", ExistingSessionMarker);
+
+ // [SupplyParameterFromQuery] values cannot be passed as bUnit parameters - the query string has to be real, so
+ // navigate to the confirmation link first. That is also exactly what clicking the e-mailed link does, and
+ // ConfirmPage.OnInitAsync then auto-submits ConfirmEmail.
+ var navigationManager = ctx.Services.GetRequiredService();
+ var confirmUrl = $"{PageUrls.Confirm}?email={Uri.EscapeDataString(email)}&emailToken={Uri.EscapeDataString(token)}";
+ navigationManager.NavigateTo(confirmUrl);
+
+ var cut = ctx.Render(parameters => parameters.AddChildContent());
+
+ // The confirmation itself must still happen - this is not "reject the request", it is "do not pretend it
+ // signed you in".
+ await cut.WaitForAssertionAsync(async () =>
+ {
+ Assert.IsTrue(await IsEmailConfirmed(server, email), "The e-mail must still be confirmed.");
+ }, timeout: TimeSpan.FromSeconds(30));
+
+ Assert.AreEqual(ExistingSessionMarker, await storageService.GetItem("access_token"),
+ "A requiresTwoFactor response carries null tokens; storing it would overwrite the caller's live session.");
+
+ Assert.AreEqual(navigationManager.ToAbsoluteUri(confirmUrl).ToString(), navigationManager.Uri,
+ "Without tokens the user is not signed in, so the page must not navigate away as if it had succeeded.");
+ }
+
+ ///
+ /// Creates an account whose e-mail is still unconfirmed while two-factor authentication is on, and returns the
+ /// confirmation code that was e-mailed to it. TwoFactorEnabled is set directly on the row because the
+ /// shipped flow for turning it on (UserController) needs an authenticator app.
+ ///
+ private async Task<(string email, string token)> CreateUnconfirmedTwoFactorAccount(AppTestServer server)
+ {
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ var email = MagicLinkSignInUtils.NewTestEmail();
+
+ // A brand-new address auto-provisions an unconfirmed account and mails it a confirmation code.
+ await Assert.ThrowsExactlyAsync(
+ () => identityController.SendOtp(new() { Email = email }, null, TestContext.CancellationToken));
+
+ var captured = await server.WaitForCapturedEmail(email, e => e.Kind is CapturedEmailKind.EmailToken, TestContext.CancellationToken);
+
+ await using var dbScope = server.WebApp.Services.CreateAsyncScope();
+ var dbContext = dbScope.ServiceProvider.GetRequiredService();
+ var normalizedEmail = email.ToUpperInvariant();
+ await dbContext.Set()
+ .Where(u => u.NormalizedEmail == normalizedEmail)
+ .ExecuteUpdateAsync(setters => setters.SetProperty(u => u.TwoFactorEnabled, true), TestContext.CancellationToken);
+
+ return (email, captured.Token!);
+ }
+
+ private static async Task IsEmailConfirmed(AppTestServer server, string email)
+ {
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var dbContext = scope.ServiceProvider.GetRequiredService();
+ var normalizedEmail = email.ToUpperInvariant();
+
+ return await dbContext.Set().AnyAsync(u => u.NormalizedEmail == normalizedEmail && u.EmailConfirmed);
+ }
+
+ /// A recognisable stand-in for the token of a session that already exists in this browser.
+ private const string ExistingSessionMarker = "existing-session-access-token";
+
+ public Microsoft.VisualStudio.TestTools.UnitTesting.TestContext TestContext { get; set; } = default!;
+}
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/OneTimeTokenTests.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/OneTimeTokenTests.cs
new file mode 100644
index 0000000000..5e21ebf77f
--- /dev/null
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/OneTimeTokenTests.cs
@@ -0,0 +1,162 @@
+using Microsoft.EntityFrameworkCore;
+using Boilerplate.Shared.Features.Identity;
+using Boilerplate.Server.Api.Infrastructure.Data;
+using Boilerplate.Tests.Infrastructure.Services;
+using Boilerplate.Server.Api.Features.Identity.Models;
+
+namespace Boilerplate.Tests.Features.Identity;
+
+///
+/// The one-time token flows (e-mail / phone confirmation and password reset) share one shape: a nullable
+/// *TokenRequestedOn column, an expiry gate that subtracts it from now, and a token whose purpose string
+/// embeds that same timestamp. Two things about that shape fail quietly, and both are pinned here.
+///
+/// 1. The expiry gate is a lifted comparison.DateTimeOffset - DateTimeOffset? is TimeSpan?, and a
+/// lifted > is false when the column is null - which is exactly the state a consumed token
+/// leaves behind. Without an explicit null check the gate is skipped and the request runs on into a verification that
+/// can never succeed, charging the user an AccessFailedAsync strike on the way.
+///
+///
+/// 2. The purpose string must be built from the same value on both sides. The account lookup is
+/// case-insensitive (it matches on NormalizedEmail) but the purpose is an HMAC input, so verifying against the
+/// caller's casing while minting from the stored casing rejects the user's own valid code.
+///
+/// Neither failure is visible from the code alone - both look like an ordinary "Invalid token." - which is why they
+/// are worth the cost of a test.
+///
+[TestClass, TestCategory("IntegrationTest")]
+public class OneTimeTokenTests
+{
+ ///
+ /// Re-opening a confirmation link that was already used. ConfirmEmail nulls EmailTokenRequestedOn on
+ /// success, and ConfirmPage auto-submits whenever the link carries an emailToken, so a second device,
+ /// a second tab or a reload replays it. The replay must be rejected as expired without touching the
+ /// account's failed-access count: that count does not decay, so five such replays over an account's life would
+ /// lock the user out of password sign-in for something they did not do.
+ ///
+ [TestMethod]
+ public async Task ReplayingAConsumedConfirmationToken_Should_NotChargeAFailedAccessAttempt()
+ {
+ await using var server = new AppTestServer();
+ await server.Build(services => services.AddIntegrationApiOnlyTestsServices()).Start(TestContext.CancellationToken);
+
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ var email = MagicLinkSignInUtils.NewTestEmail();
+
+ await Assert.ThrowsExactlyAsync(
+ () => identityController.SendOtp(new() { Email = email }, null, TestContext.CancellationToken));
+
+ var captured = await server.WaitForCapturedEmail(email, e => e.Kind is CapturedEmailKind.EmailToken, TestContext.CancellationToken);
+
+ // First use succeeds and clears EmailTokenRequestedOn.
+ await identityController.ConfirmEmail(new() { Email = email, Token = captured.Token }, TestContext.CancellationToken);
+
+ var failedCountBeforeReplay = await ReadAccessFailedCount(server, email);
+
+ var replay = await Assert.ThrowsExactlyAsync(
+ () => identityController.ConfirmEmail(new() { Email = email, Token = captured.Token }, TestContext.CancellationToken),
+ "Replaying a consumed confirmation token must be rejected.");
+
+ Assert.AreEqual(nameof(AppStrings.ExpiredToken), replay.Key,
+ "A consumed token has no outstanding request, so the expiry gate must reject it before the token check - " +
+ "reporting InvalidToken instead means the lifted null comparison let it through.");
+
+ Assert.AreEqual(failedCountBeforeReplay, await ReadAccessFailedCount(server, email),
+ "Rejecting a provably unverifiable replay must not spend the user's lockout budget.");
+ }
+
+ ///
+ /// The same gate on the phone flow, which the shipped seed data reaches directly: UserConfiguration seeds
+ /// PhoneNumberConfirmed = true while leaving PhoneNumberTokenRequestedOn null.
+ ///
+ [TestMethod]
+ public async Task ConfirmingAPhoneWithNoOutstandingToken_Should_ReportExpiredRatherThanInvalid()
+ {
+ await using var server = new AppTestServer();
+ await server.Build(services => services.AddIntegrationApiOnlyTestsServices()).Start(TestContext.CancellationToken);
+
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ var exception = await Assert.ThrowsExactlyAsync(
+ () => identityController.ConfirmPhone(new() { PhoneNumber = SeededTestPhoneNumber, Token = "123456" }, TestContext.CancellationToken),
+ "There is no outstanding phone token for the seeded account, so this must not be accepted.");
+
+ Assert.AreEqual(nameof(AppStrings.ExpiredToken), exception.Key,
+ "PhoneNumberTokenRequestedOn is null for the seeded account, so the expiry gate must fire first.");
+ }
+
+ ///
+ /// A user who registered as John.Doe@Example.com and later types john.doe@example.com resolves to the
+ /// same account, because Identity looks accounts up by NormalizedEmail. The 6-digit code they were sent must
+ /// therefore still work - it is minted from the stored address, so verifying against the typed one would reject a
+ /// correctly delivered code and charge a strike for it.
+ ///
+ [TestMethod]
+ public async Task ConfirmEmail_Should_AcceptACaseVariantOfTheRegisteredAddress()
+ {
+ await using var server = new AppTestServer();
+ await server.Build(services => services.AddIntegrationApiOnlyTestsServices()).Start(TestContext.CancellationToken);
+
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ var email = $"Mixed.Case.{Guid.NewGuid()}@BitPlatform.dev";
+
+ await Assert.ThrowsExactlyAsync(
+ () => identityController.SendOtp(new() { Email = email }, null, TestContext.CancellationToken));
+
+ var captured = await server.WaitForCapturedEmail(email, e => e.Kind is CapturedEmailKind.EmailToken, TestContext.CancellationToken);
+
+ // The user types their address in a different case than they registered with.
+ await identityController.ConfirmEmail(
+ new() { Email = email.ToLowerInvariant(), Token = captured.Token }, TestContext.CancellationToken);
+
+ await using var dbScope = server.WebApp.Services.CreateAsyncScope();
+ var dbContext = dbScope.ServiceProvider.GetRequiredService();
+ var normalizedEmail = email.ToUpperInvariant();
+
+ Assert.IsTrue(await dbContext.Set().AnyAsync(u => u.NormalizedEmail == normalizedEmail && u.EmailConfirmed, TestContext.CancellationToken),
+ "The code was correct and was delivered to that address, so a difference in casing must not reject it.");
+ }
+
+ ///
+ /// [Phone] (the only validation the DTOs carry) is far more permissive than libphonenumber, which is what
+ /// actually parses the value: "+1" passes the attribute and throws inside PhoneService. On an
+ /// anonymous endpoint that difference is the difference between a 400 and a 500 logged at Critical.
+ ///
+ [TestMethod]
+ [DataRow("+1")]
+ [DataRow("+999999999999999999")]
+ public async Task AnUnparsablePhoneNumber_Should_BeABadRequestRatherThanAServerFault(string phoneNumber)
+ {
+ await using var server = new AppTestServer();
+ await server.Build(services => services.AddIntegrationApiOnlyTestsServices()).Start(TestContext.CancellationToken);
+
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ await Assert.ThrowsExactlyAsync(
+ () => identityController.SendConfirmPhoneToken(new() { PhoneNumber = phoneNumber }, TestContext.CancellationToken),
+ $"'{phoneNumber}' passes the [Phone] attribute but libphonenumber cannot parse it; that is a bad request, not a server fault.");
+ }
+
+ /// The phone number UserConfiguration seeds for the default test account.
+ private const string SeededTestPhoneNumber = "+31684207362";
+
+ private static async Task ReadAccessFailedCount(AppTestServer server, string email)
+ {
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var dbContext = scope.ServiceProvider.GetRequiredService();
+ var normalizedEmail = email.ToUpperInvariant();
+
+ return await dbContext.Set()
+ .Where(u => u.NormalizedEmail == normalizedEmail)
+ .Select(u => u.AccessFailedCount)
+ .SingleAsync();
+ }
+
+ public TestContext TestContext { get; set; } = default!;
+}
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/PhoneNumberNormalizationUITests.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/PhoneNumberNormalizationUITests.cs
index f0126053fa..bf159cbb85 100644
--- a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/PhoneNumberNormalizationUITests.cs
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/PhoneNumberNormalizationUITests.cs
@@ -68,7 +68,7 @@ await server.Build(services =>
// proves the number was normalized (not merely passed through).
var e164 = new Regex(@"^\+[1-9]\d{6,14}$");
- foreach (var attempt in attempts)
+ foreach (var (Typed, Normalized) in attempts)
{
// Each request starts from a fresh Sign in page. A hard reload is the simplest way back to the phone form:
// the OTP panel is treated as a modal, but its NavigationLock only intercepts internal Blazor navigation
@@ -82,17 +82,17 @@ await Page.GotoAsync(new Uri(serverAddress, PageUrls.SignIn).ToString(),
// Type the de-normalized number into the BitPhoneInput's number box (its carries the
// placeholder) and ask for the code. Send OTP stays disabled until the debounced value commits, so
// Playwright's actionability wait covers the 500ms debounce.
- await Page.GetByPlaceholder(AppStrings.PhoneNumberPlaceholder).FillAsync(attempt.Typed);
+ await Page.GetByPlaceholder(AppStrings.PhoneNumberPlaceholder).FillAsync(Typed);
await Page.GetByRole(AriaRole.Button, new() { Name = AppStrings.SendOtpButtonText }).ClickAsync();
// The server registers the new account, texts the confirmation code (captured synchronously by the fake) and
// answers "not confirmed", which reveals the OTP panel - a reliable signal that SendSms has already run.
await Page.Locator(".bit-otp-inp").First.WaitForAsync();
- var sms = await WaitForSmsTo(attempt.Normalized, TestContext.CancellationToken);
+ var sms = await WaitForSmsTo(Normalized, TestContext.CancellationToken);
- Assert.AreEqual(attempt.Normalized, sms.PhoneNumber,
- $"The server should have normalized '{attempt.Typed}' to E.164 before calling SendSms.");
+ Assert.AreEqual(Normalized, sms.PhoneNumber,
+ $"The server should have normalized '{Typed}' to E.164 before calling SendSms.");
Assert.MatchesRegex(e164, sms.PhoneNumber,
"SendSms must receive a canonical E.164 number (leading '+', digits only, no formatting).");
}
@@ -146,8 +146,8 @@ await Page.GotoAsync(new Uri(serverAddress, PageUrls.SignIn).ToString(),
/// is exactly what the server normalized - which is the behavior under test. Registered per-test via
/// configureTestServices (RemoveAll<PhoneService> then AddScoped<PhoneService, CapturingPhoneService>).
///
-public partial class CapturingPhoneService(ServerApiSettings appSettings, IBackgroundJobClient backgroundJobClient, IHostEnvironment hostEnvironment, IHttpContextAccessor httpContextAccessor, ILogger phoneLogger, PhoneNumberUtil phoneNumberUtil) :
- PhoneService(appSettings, backgroundJobClient, hostEnvironment, httpContextAccessor, phoneLogger, phoneNumberUtil)
+public partial class CapturingPhoneService(ServerApiSettings appSettings, IBackgroundJobClient backgroundJobClient, IHostEnvironment hostEnvironment, IHttpContextAccessor httpContextAccessor, IStringLocalizer localizer, ILogger phoneLogger, PhoneNumberUtil phoneNumberUtil) :
+ PhoneService(appSettings, backgroundJobClient, hostEnvironment, httpContextAccessor, localizer, phoneLogger, phoneNumberUtil)
{
///
/// Every SendSms call as (message body, phone number). Static because DI owns the resolved instance's lifetime, so a
diff --git a/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/ReturnUrlHardeningTests.cs b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/ReturnUrlHardeningTests.cs
new file mode 100644
index 0000000000..ad1cd48b41
--- /dev/null
+++ b/src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/Features/Identity/ReturnUrlHardeningTests.cs
@@ -0,0 +1,170 @@
+using System.Web;
+using Boilerplate.Shared.Features.Identity;
+using Boilerplate.Tests.Infrastructure.Services;
+
+namespace Boilerplate.Tests.Features.Identity;
+
+///
+/// Guards the return-url trust boundary. The value travels a long way from an anonymous request body, through
+/// a link the app itself e-mails, into NavigationManager.NavigateTo on the client - so anything that can leave
+/// this origin has to be rejected somewhere, and is that somewhere.
+///
+/// The predicate has two subtleties worth pinning: a network-path reference such as //attacker.com is a
+/// perfectly well-formed relative reference that browsers resolve off-origin, and the app itself produces
+/// base-relative return urls without a leading slash (See NavigationManagerExtensions.GetRelativePath,
+/// which AppShell feeds straight into a return-url), so a rooted-only predicate would break the app's
+/// own sign-in links.
+///
+///
+[TestClass, TestCategory("UnitTest")]
+public class ReturnUrlPredicateTests
+{
+ ///
+ /// Values the app itself generates, plus the rooted form the e-mailed links carry. All must survive.
+ ///
+ [TestMethod]
+ [DataRow("/")]
+ [DataRow("/products/1")]
+ [DataRow("/sign-in?otp=123456")]
+ [DataRow("/confirm?email=a%40b.c&emailToken=123456")]
+ // GetRelativePath() returns a BASE-relative path, i.e. no leading slash - this is what AppShell and
+ // SignInModalService put into a return-url, so rejecting it would break ordinary sign-in.
+ [DataRow("products/1")]
+ [DataRow("sign-in")]
+ [DataRow("products/1?a=b")]
+ public void IsAppRelativeUrl_Should_AcceptTheUrlsTheAppItselfProduces(string url)
+ {
+ Assert.IsTrue(Uri.IsAppRelativeUrl(url, requireLeadingSlash: false),
+ $"'{url}' is produced by the app itself and must not be rejected.");
+ }
+
+ ///
+ /// Everything that can leave the origin. The // and /\ forms are the interesting ones: both are
+ /// well-formed RELATIVE references, so Uri.IsWellFormedUriString(_, UriKind.Relative) alone would let them
+ /// through and the browser would then resolve them to the attacker's host.
+ ///
+ [TestMethod]
+ [DataRow("//evil.example")]
+ [DataRow("//evil.example/path")]
+ [DataRow("///evil.example")]
+ [DataRow("/\\evil.example")]
+ [DataRow("\\\\evil.example")]
+ [DataRow("\\/evil.example")]
+ [DataRow("https://evil.example")]
+ [DataRow("http://evil.example/x")]
+ [DataRow("app://localhost/x")]
+ [DataRow("javascript:alert(1)")]
+ [DataRow("")]
+ public void IsAppRelativeUrl_Should_RejectAnythingThatCanLeaveTheOrigin(string url)
+ {
+ Assert.IsFalse(Uri.IsAppRelativeUrl(url, requireLeadingSlash: false),
+ $"'{url}' can escape this app's origin and must be rejected.");
+ Assert.IsFalse(Uri.IsAppRelativeUrl(url),
+ $"'{url}' must also be rejected by the stricter, rooted-only form.");
+ }
+
+ ///
+ /// The default (rooted-only) form is what the Blazor Hybrid local HTTP server's callback uses, and it must stay
+ /// strict: a rootless value is origin-bound but is not a valid app-rooted url.
+ ///
+ [TestMethod]
+ public void IsAppRelativeUrl_Should_StillRequireALeadingSlashByDefault()
+ {
+ Assert.IsFalse(Uri.IsAppRelativeUrl("products/1"), "The default overload must keep requiring a leading slash.");
+ Assert.IsTrue(Uri.IsAppRelativeUrl("/products/1"), "A rooted url must pass the default overload.");
+ }
+}
+
+///
+/// The server half of the same boundary: returnUrl arrives in an anonymous request body and is spliced
+/// into a link the application e-mails from its own domain, so an off-origin value would turn the app's own
+/// transactional mail into a redirector. These tests read the captured e-mail (See )
+/// and assert what actually left the building.
+///
+[TestClass, TestCategory("IntegrationTest")]
+public class ReturnUrlHardeningTests
+{
+ ///
+ /// SendOtp needs no pre-existing account - it auto-provisions - so this is reachable by an anonymous caller
+ /// against any address they choose, which is what makes it worth a test rather than a comment.
+ ///
+ [TestMethod]
+ public async Task SendOtp_Should_NotPutAnOffOriginReturnUrlIntoTheConfirmationEmail()
+ {
+ await using var server = new AppTestServer();
+ await server.Build(services => services.AddIntegrationApiOnlyTestsServices()).Start(TestContext.CancellationToken);
+
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ var email = MagicLinkSignInUtils.NewTestEmail();
+
+ // A brand-new address: the account is created unconfirmed, so SendOtp sends the CONFIRMATION mail, and the
+ // attacker-chosen returnUrl is what would be embedded in its link.
+ await Assert.ThrowsExactlyAsync(
+ () => identityController.SendOtp(new() { Email = email, ReturnUrl = "https://evil.example/login" }, null, TestContext.CancellationToken),
+ "A brand-new account is unconfirmed, so SendOtp must answer UserIsNotConfirmed after mailing the confirmation link.");
+
+ var captured = await server.WaitForCapturedEmail(email, e => e.Kind is CapturedEmailKind.EmailToken, TestContext.CancellationToken);
+
+ var returnUrl = HttpUtility.ParseQueryString(captured.Link!.Query)["return-url"];
+
+ Assert.AreEqual(PageUrls.Home, returnUrl,
+ $"An off-origin return url must be replaced by the home page, but the e-mailed link carried '{returnUrl}'.");
+ }
+
+ ///
+ /// The same boundary on the reset-password flow, which takes its return url from a different DTO and builds its
+ /// link in a different method - so it can (and did) regress independently. A network-path reference is the
+ /// interesting payload here: it is a well-formed RELATIVE url, so it slips past the obvious check.
+ ///
+ [TestMethod]
+ [DataRow("//evil.example/login", PageUrls.Home, "A network-path reference is a well-formed RELATIVE url, so it must be caught explicitly.")]
+ [DataRow("https://evil.example/login", PageUrls.Home, "An absolute url must never survive into an e-mailed link.")]
+ // The guard must not become "replace everything": an ordinary in-app return url has to survive end to end, or
+ // resetting a password from a product page would drop the user on the home page instead of bringing them back.
+ [DataRow("/settings/sessions", "/settings/sessions", "A legitimate app-relative return url must be preserved.")]
+ public async Task SendResetPasswordToken_Should_OnlyEmailAnAppRelativeReturnUrl(string requested, string expected, string because)
+ {
+ await using var server = new AppTestServer();
+ await server.Build(services => services.AddIntegrationApiOnlyTestsServices()).Start(TestContext.CancellationToken);
+
+ await using var scope = server.WebApp.Services.CreateAsyncScope();
+ var identityController = scope.ServiceProvider.GetRequiredService();
+
+ // A per-run account, not the seeded one: SendResetPasswordToken is throttled per user for the token's whole
+ // lifetime, so sharing an account across these rows (they run in parallel) would fail on the resend delay.
+ var email = await CreateConfirmedAccount(server, identityController);
+
+ await identityController.SendResetPasswordToken(
+ new() { Email = email, ReturnUrl = requested }, TestContext.CancellationToken);
+
+ var captured = await server.WaitForCapturedEmail(email, e => e.Kind is CapturedEmailKind.ResetPassword, TestContext.CancellationToken);
+
+ var returnUrl = HttpUtility.ParseQueryString(captured.Link!.Query)["return-url"];
+
+ Assert.AreEqual(expected, returnUrl, $"{because} The e-mailed link carried '{returnUrl}'.");
+ }
+
+ ///
+ /// Registers and confirms a brand-new account, so the test owns its own fixture instead of mutating the seeded
+ /// one. SendOtp auto-provisions the (unconfirmed) account and mails the confirmation code; consuming that
+ /// code confirms it, which is what SendResetPasswordToken requires.
+ ///
+ private async Task CreateConfirmedAccount(AppTestServer server, IIdentityController identityController)
+ {
+ var email = MagicLinkSignInUtils.NewTestEmail();
+
+ await Assert.ThrowsExactlyAsync(
+ () => identityController.SendOtp(new() { Email = email }, null, TestContext.CancellationToken),
+ "A brand-new account is unconfirmed, so SendOtp answers UserIsNotConfirmed after mailing the confirmation link.");
+
+ var captured = await server.WaitForCapturedEmail(email, e => e.Kind is CapturedEmailKind.EmailToken, TestContext.CancellationToken);
+
+ await identityController.ConfirmEmail(new() { Email = email, Token = captured.Token }, TestContext.CancellationToken);
+
+ return email;
+ }
+
+ public TestContext TestContext { get; set; } = default!;
+}