Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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;
});
Expand All @@ -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);
});
}

Expand All @@ -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;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
<BitStack Horizontal VerticalAlign="BitAlignment.Center">
<BitText>@Localizer[nameof(AppStrings.Password)]</BitText>
<BitSpacer />
<BitLink Href="@($"{PageUrls.ForgotPassword}?return-url={Uri.EscapeDataString(GetReturnUrl())}")">@Localizer[nameof(AppStrings.ForgotPasswordLink)]</BitLink>
<BitLink Href="@($"{PageUrls.ForgotPassword}?return-url={Uri.EscapeDataString(GetSafeReturnUrl())}")">@Localizer[nameof(AppStrings.ForgotPasswordLink)]</BitLink>
</BitStack>
</LabelTemplate>
</BitTextField>
Expand Down Expand Up @@ -138,7 +138,7 @@
{
<BitText Align="BitTextAlign.Center" Typography="BitTypography.Body2">
@Localizer[nameof(AppStrings.DontHaveAccountMessage)]
<BitLink Href="@($"{PageUrls.SignUp}?return-url={Uri.EscapeDataString(GetReturnUrl())}")">@Localizer[nameof(AppStrings.SignUp)]</BitLink>
<BitLink Href="@($"{PageUrls.SignUp}?return-url={Uri.EscapeDataString(GetSafeReturnUrl())}")">@Localizer[nameof(AppStrings.SignUp)]</BitLink>
</BitText>
}
</BitStack>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
yasmoradi marked this conversation as resolved.
}

[Parameter]
public string? ReturnUrl { get; set; }
Expand Down Expand Up @@ -143,7 +148,7 @@ private async Task DoSignIn()

if (isNewUser is false)
{
model.ReturnUrl = GetReturnUrl();
model.ReturnUrl = GetSafeReturnUrl();

requiresTwoFactor = await AuthManager.SignIn(model, CurrentCancellationToken);

Expand Down Expand Up @@ -187,7 +192,7 @@ private async Task DoSignIn()
}
else
{
NavigationManager.NavigateTo(GetReturnUrl(), replace: true);
NavigationManager.NavigateTo(GetSafeReturnUrl(), replace: true);
}
}
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!;

Expand All @@ -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);
}
}

Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
{
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ public partial class IdentityController

/// <summary>
/// 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.
/// </summary>
[HttpGet]
public async Task<string> GetExternalSignInUri(string provider, string? returnUrl = null, int? localHttpPort = null, CancellationToken cancellationToken = default)
Expand All @@ -38,6 +39,9 @@ public async Task<ActionResult> 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.");
Expand All @@ -46,6 +50,8 @@ public async Task<ActionResult> 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 });
Expand Down Expand Up @@ -86,8 +92,14 @@ public async Task<ActionResult> 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)
throw new ResourceValidationException(addLoginResult.Errors.Select(e => new LocalizedString(e.Code, e.Description)).ToArray()).WithData("UserId", user.Id);
Comment thread
yasmoradi marked this conversation as resolved.
}

// Confirmation is only as good as the provider's own verification of the identifier
Expand Down Expand Up @@ -130,7 +142,11 @@ public async Task<ActionResult> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Task> sendMessagesTasks = [];
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading