Skip to content

Commit

Permalink
Route Passwordless API errors in PasswordlessService (#84)
Browse files Browse the repository at this point in the history 
* Route Passwordless API errors in `PasswordlessService`

* Remove test for an aspect that no longer exists
  • Loading branch information
@Tyrrrz
Tyrrrz authored Nov 28, 2023
1 parent a11fc99 commit f37c27b
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 116 deletions.
275 changes: 174 additions & 101 deletions src/Passwordless.AspNetCore/Services/Implementations/PasswordlessService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ namespace Passwordless.AspNetCore.Services.Implementations;
public class PasswordlessService<TUser> : PasswordlessService<TUser, PasswordlessRegisterRequest>
where TUser : class, new()
{
/// <summary>
/// Initializes an instance of <see cref="PasswordlessService{TUser}" />.
/// </summary>
public PasswordlessService(
IPasswordlessClient passwordlessClient,
IUserStore<TUser> userStore,
Expand All @@ -37,6 +40,9 @@ public class PasswordlessService<TUser, TRegisterRequest>
{
private readonly ILogger<PasswordlessService<TUser, TRegisterRequest>> _logger;

/// <summary>
/// Initializes an instance of <see cref="PasswordlessService{TUser, TRegisterRequest}" />.
/// </summary>
public PasswordlessService(
IPasswordlessClient passwordlessClient,
IUserStore<TUser> userStore,
Expand Down Expand Up @@ -66,70 +72,95 @@ public PasswordlessService(
protected AuthenticationOptions? AuthenticationOptions { get; }
protected UserManager<TUser>? UserManager { get; }

public virtual async Task<IResult> AddCredentialAsync(PasswordlessAddCredentialRequest request, ClaimsPrincipal claimsPrincipal, CancellationToken cancellationToken)
/// <inheritdoc />
public virtual async Task<IResult> AddCredentialAsync(
PasswordlessAddCredentialRequest request,
ClaimsPrincipal claimsPrincipal,
CancellationToken cancellationToken)
{
var userId = await GetUserIdAsync(claimsPrincipal, cancellationToken);

if (string.IsNullOrEmpty(userId))
try
{
return Unauthorized();
}


var user = await UserStore.FindByIdAsync(userId, cancellationToken);
var userId = await GetUserIdAsync(claimsPrincipal, cancellationToken);

if (user is null)
{
_logger.LogDebug("Could not find user with id {UserId} while attempting to add credential", userId);
return Unauthorized();
}
if (string.IsNullOrEmpty(userId))
{
return Unauthorized();
}

_logger.LogInformation("Found user {UserId} while attempting to add credential", userId);
var user = await UserStore.FindByIdAsync(userId, cancellationToken);

var username = await UserStore.GetUserNameAsync(user, cancellationToken);
if (user is null)
{
_logger.LogDebug("Could not find user with id {UserId} while attempting to add credential", userId);
return Unauthorized();
}

if (string.IsNullOrEmpty(username))
{
return Unauthorized();
}
_logger.LogInformation("Found user {UserId} while attempting to add credential", userId);

UserInformation userInformation;
if (IdentityOptions?.User.RequireUniqueEmail is true && UserStore is IUserEmailStore<TUser> emailStore)
{
var email = await emailStore.GetEmailAsync(user, cancellationToken);
var username = await UserStore.GetUserNameAsync(user, cancellationToken);

if (string.IsNullOrEmpty(email))
if (string.IsNullOrEmpty(username))
{
return Unauthorized();
}

userInformation = new UserInformation(username, request.DisplayName, new HashSet<string> { email });
}
else
{
userInformation = new UserInformation(username, request.DisplayName, null);
}
UserInformation userInformation;
if (IdentityOptions?.User.RequireUniqueEmail is true && UserStore is IUserEmailStore<TUser> emailStore)
{
var email = await emailStore.GetEmailAsync(user, cancellationToken);

var registerOptions = CreateRegisterOptions(userId, userInformation);
if (string.IsNullOrEmpty(email))
{
return Unauthorized();
}

// I could check if the customize service is the noop one and not allocate this context class
// which would make this a bit more pay-for-play.
var customizeContext = new CustomizeRegisterOptionsContext(false, registerOptions);
await CustomizeRegisterOptions.CustomizeAsync(customizeContext, cancellationToken);
userInformation = new UserInformation(username, request.DisplayName, new HashSet<string> { email });
}
else
{
userInformation = new UserInformation(username, request.DisplayName, null);
}

if (customizeContext.Options is null)
{
return Unauthorized();
}
var registerOptions = CreateRegisterOptions(userId, userInformation);

// I could check if the customize service is the noop one and not allocate this context class
// which would make this a bit more pay-for-play.
var customizeContext = new CustomizeRegisterOptionsContext(false, registerOptions);
await CustomizeRegisterOptions.CustomizeAsync(customizeContext, cancellationToken);

if (customizeContext.Options is null)
{
return Unauthorized();
}

var registerTokenResponse = await PasswordlessClient.CreateRegisterTokenAsync(customizeContext.Options, cancellationToken);
var registerTokenResponse =
await PasswordlessClient.CreateRegisterTokenAsync(customizeContext.Options, cancellationToken);

_logger.LogDebug("Successfully created a register token for user {UserId}", userId);
_logger.LogDebug("Successfully created a register token for user {UserId}", userId);

return Ok(registerTokenResponse);
return Ok(registerTokenResponse);
}
// Route Passwordless API errors to the corresponding result
catch (PasswordlessApiException ex)
{
_logger.LogDebug(
"Passwordless API responded with an error while attempting to add credential: {Error}",
ex.Details
);

return Problem(
ex.Details.Detail,
ex.Details.Instance,
ex.Details.Status,
ex.Details.Title,
ex.Details.Type
);
}
}

protected virtual ValueTask<string?> GetUserIdAsync(ClaimsPrincipal claimsPrincipal, CancellationToken cancellationToken)
protected virtual ValueTask<string?> GetUserIdAsync(
ClaimsPrincipal claimsPrincipal,
CancellationToken cancellationToken)
{
// Do we want to check if the Identity is authenticated? They could have multiple identities and the first one isn't authenticated
if (claimsPrincipal.Identity?.IsAuthenticated is not true)
Expand All @@ -150,89 +181,131 @@ public virtual async Task<IResult> AddCredentialAsync(PasswordlessAddCredentialR
return ValueTask.FromResult(userId);
}

public virtual async Task<IResult> RegisterUserAsync(TRegisterRequest request, CancellationToken cancellationToken)
/// <inheritdoc />
public virtual async Task<IResult> RegisterUserAsync(
TRegisterRequest request,
CancellationToken cancellationToken)
{
var user = new TUser();
var result = await CreateUserAsync(user, request, cancellationToken);

if (!result.Succeeded)
try
{
return ValidationProblem(result.Errors.ToDictionary(e => e.Code, e => new[] { e.Description })); ;
}
var user = new TUser();
var result = await CreateUserAsync(user, request, cancellationToken);

var userId = await UserStore.GetUserIdAsync(user, cancellationToken);
_logger.LogDebug("Registering user with id: {Id}", userId);
if (!result.Succeeded)
{
return ValidationProblem(result.Errors.ToDictionary(e => e.Code, e => new[] { e.Description }));
}

var aliases = request.Aliases ?? new HashSet<string>();
var userId = await UserStore.GetUserIdAsync(user, cancellationToken);
_logger.LogDebug("Registering user with id: {Id}", userId);

if (IdentityOptions?.User.RequireUniqueEmail is true && UserStore is IUserEmailStore<TUser> emailStore)
{
if (string.IsNullOrEmpty(request.Email))
var aliases = request.Aliases ?? new HashSet<string>();

if (IdentityOptions?.User.RequireUniqueEmail is true && UserStore is IUserEmailStore<TUser> emailStore)
{
return ValidationProblem(new Dictionary<string, string[]>
if (string.IsNullOrEmpty(request.Email))
{
{ "invalid_email", new [] { "Email cannot be null or empty." }}
});
return ValidationProblem(new Dictionary<string, string[]>
{
{ "invalid_email", new[] { "Email cannot be null or empty." } }
});
}

aliases.Add(request.Email);
}
else if (!string.IsNullOrEmpty(request.Email))
{
_logger.LogWarning(
"An email was provided for {UserId}, but IdentityOptions.User.RequireUniqueEmail was not set to true so the email will not be used as an alias for the passkey.",
userId);
}

aliases.Add(request.Email);
}
else if (!string.IsNullOrEmpty(request.Email))
{
_logger.LogWarning("An email was provided for {UserId}, but IdentityOptions.User.RequireUniqueEmail was not set to true so the email will not be used as an alias for the passkey.",
userId);
}
var registerOptions = CreateRegisterOptions(userId,
new UserInformation(request.Username, request.DisplayName, aliases));

var registerOptions = CreateRegisterOptions(userId,
new UserInformation(request.Username, request.DisplayName, aliases));
// Customize register options
var customizeContext = new CustomizeRegisterOptionsContext(true, registerOptions);
await CustomizeRegisterOptions.CustomizeAsync(customizeContext, cancellationToken);

// Customize register options
var customizeContext = new CustomizeRegisterOptionsContext(true, registerOptions);
await CustomizeRegisterOptions.CustomizeAsync(customizeContext, cancellationToken);
if (customizeContext.Options is null)
{
// Is this the best result?
return Unauthorized();
}

if (customizeContext.Options is null)
var token = await PasswordlessClient.CreateRegisterTokenAsync(customizeContext.Options, cancellationToken);
return Ok(token);
}
// Route Passwordless API errors to the corresponding result
catch (PasswordlessApiException ex)
{
// Is this the best result?
return Unauthorized();
_logger.LogDebug(
"Passwordless API responded with an error while attempting to register user: {Error}",
ex.Details
);

return Problem(
ex.Details.Detail,
ex.Details.Instance,
ex.Details.Status,
ex.Details.Title,
ex.Details.Type
);
}

var token = await PasswordlessClient.CreateRegisterTokenAsync(customizeContext.Options, cancellationToken);
return Ok(token);
}

public virtual async Task<IResult> LoginUserAsync(PasswordlessLoginRequest loginRequest, CancellationToken cancellationToken)
/// <inheritdoc />
public virtual async Task<IResult> LoginUserAsync(
PasswordlessLoginRequest loginRequest,
CancellationToken cancellationToken)
{
var verifiedUser = await PasswordlessClient.VerifyTokenAsync(loginRequest.Token, cancellationToken);

if (verifiedUser is null)
try
{
_logger.LogDebug("User could not be verified with token {Token}", loginRequest.Token);
return Unauthorized();
}
var verifiedUser = await PasswordlessClient.VerifyTokenAsync(loginRequest.Token, cancellationToken);

_logger.LogDebug("Attempting to find user in store by id {UserId}.", verifiedUser.UserId);
var user = await UserStore.FindByIdAsync(verifiedUser.UserId, cancellationToken);
_logger.LogDebug("Attempting to find user in store by id {UserId}.", verifiedUser.UserId);
var user = await UserStore.FindByIdAsync(verifiedUser.UserId, cancellationToken);

if (user is null)
{
_logger.LogDebug("Could not find user.");
return Unauthorized();
}
if (user is null)
{
_logger.LogDebug("Could not find user.");
return Unauthorized();
}

var claimsPrincipal = await UserClaimsPrincipalFactory.CreateAsync(user);
var claimsPrincipal = await UserClaimsPrincipalFactory.CreateAsync(user);

// First try our own scheme, then optionally try built in options but null is still allowed because it
// will then fallback to the default scheme.
var scheme = Options.SignInScheme
?? AuthenticationOptions?.DefaultSignInScheme;
// First try our own scheme, then optionally try built in options but null is still allowed because it
// will then fallback to the default scheme.
var scheme = Options.SignInScheme
?? AuthenticationOptions?.DefaultSignInScheme;

_logger.LogInformation("Signing in user with scheme {Scheme} and {NumberOfClaims} claims",
scheme, claimsPrincipal.Claims.Count());
_logger.LogInformation("Signing in user with scheme {Scheme} and {NumberOfClaims} claims",
scheme, claimsPrincipal.Claims.Count());

return SignIn(claimsPrincipal, authenticationScheme: scheme);
return SignIn(claimsPrincipal, authenticationScheme: scheme);
}
// Route Passwordless API errors to the corresponding result
catch (PasswordlessApiException ex)
{
_logger.LogDebug(
"Passwordless API responded with an error while attempting to login user: {Error}",
ex.Details
);

return Problem(
ex.Details.Detail,
ex.Details.Instance,
ex.Details.Status,
ex.Details.Title,
ex.Details.Type
);
}
}

protected virtual async Task<IdentityResult> CreateUserAsync(TUser user, TRegisterRequest request, CancellationToken cancellationToken)
protected virtual async Task<IdentityResult> CreateUserAsync(
TUser user,
TRegisterRequest request,
CancellationToken cancellationToken)
{
await UserStore.SetUserNameAsync(user, request.Username, cancellationToken);

Expand Down
1 change: 0 additions & 1 deletion tests/Passwordless.AspNetCore.Tests.Dummy/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Microsoft.AspNetCore.Identity.EntityFrameworkCore;
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;

namespace Passwordless.AspNetCore.Tests.Dummy;
Expand Down
2 changes: 1 addition & 1 deletion tests/Passwordless.AspNetCore.Tests/EndpointTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public async Task I_can_define_a_register_endpoint_and_it_will_reject_duplicate_
responses.Skip(1).Should().OnlyContain(r => r.StatusCode == HttpStatusCode.BadRequest);
}

[Fact(Skip = "Bug: this currently does not return 400 status code. Task: PAS-260")]
[Fact]
public async Task I_can_define_a_signin_endpoint_and_it_will_reject_invalid_signin_attempts()
{
// Arrange
Expand Down
13 changes: 0 additions & 13 deletions tests/Passwordless.AspNetCore.Tests/OldTests/PasswordlessServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,19 +312,6 @@ public async Task LoginUserAsync_TriesAuthenticationOptionsIfOursIsNull()
Assert.Equal("auth_options_scheme", signInResult.AuthenticationScheme);
}

[Fact]
public async Task LoginUserAsync_PasswordlessClientReturnsNull_ReturnsUnauthorized()
{
_mockPasswordlessClient
.Setup(s => s.VerifyTokenAsync("test_token", default))
.Returns(Task.FromResult<VerifiedUser>(null!));

var sut = CreateSut();

var result = await sut.LoginUserAsync(new PasswordlessLoginRequest("test_token"), CancellationToken.None);
Assert.IsAssignableFrom<UnauthorizedHttpResult>(result);
}

[Fact]
public async Task LoginUserAsync_UserDoesNotExist_ReturnsUnauthorized()
{
Expand Down

0 comments on commit f37c27b

Please sign in to comment.