feat(backend): improve bad request errors

This commit is contained in:
sam 2024-07-14 16:44:41 +02:00
parent e7ec0e6661
commit fb34464199
Signed by: sam
GPG key ID: B4EF20DDE721CAA1
6 changed files with 114 additions and 45 deletions

View file

@ -48,7 +48,7 @@ public class DiscordAuthController(
public async Task<IActionResult> RegisterAsync([FromBody] AuthController.OauthRegisterRequest req)
{
var remoteUser = await keyCacheSvc.GetKeyAsync<RemoteAuthService.RemoteUser>($"discord:{req.Ticket}");
if (remoteUser == null) throw new ApiError.BadRequest("Invalid ticket", "ticket");
if (remoteUser == null) throw new ApiError.BadRequest("Invalid ticket", "ticket", req.Ticket);
if (await db.AuthMethods.AnyAsync(a => a.AuthType == AuthType.Discord && a.RemoteId == remoteUser.Id))
{
logger.Error("Discord user {Id} has valid ticket but is already linked to an existing account",

View file

@ -45,7 +45,7 @@ public class MembersController(
if (await db.Members.AnyAsync(m => m.UserId == CurrentUser!.Id && m.Name.ToLower() == req.Name.ToLower()))
#pragma warning restore CA1862
{
throw new ApiError.BadRequest("A member with that name already exists", "name");
throw new ApiError.BadRequest("A member with that name already exists", "name", req.Name);
}
var member = new Member

View file

@ -38,30 +38,35 @@ public class UsersController(DatabaseContext db, UserRendererService userRendere
{
await using var tx = await db.Database.BeginTransactionAsync();
var user = await db.Users.FirstAsync(u => u.Id == CurrentUser!.Id);
var errors = new List<(string, ValidationError?)>();
if (req.Username != null && req.Username != user.Username)
{
ValidationUtils.ValidateUsername(req.Username);
errors.Add(("username", ValidationUtils.ValidateUsername(req.Username)));
user.Username = req.Username;
}
if (req.HasProperty(nameof(req.DisplayName)))
{
ValidationUtils.ValidateDisplayName(req.DisplayName);
errors.Add(("display_name", ValidationUtils.ValidateDisplayName(req.DisplayName)));
user.DisplayName = req.DisplayName;
}
if (req.HasProperty(nameof(req.Bio)))
{
ValidationUtils.ValidateBio(req.Bio);
errors.Add(("bio", ValidationUtils.ValidateBio(req.Bio)));
user.Bio = req.Bio;
}
if (req.HasProperty(nameof(req.Avatar)))
{
ValidationUtils.ValidateAvatar(req.Avatar);
errors.Add(("avatar", ValidationUtils.ValidateAvatar(req.Avatar)));
ValidationUtils.Validate(errors);
// This is fired off regardless of whether the transaction is committed
// (atomic operations are hard when combined with background jobs)
// so it's in a separate block to the validation above.
if (req.HasProperty(nameof(req.Avatar)))
AvatarUpdateJob.QueueUpdateUserAvatar(CurrentUser!.Id, req.Avatar);
}
await db.SaveChangesAsync();
await tx.CommitAsync();

View file

@ -2,6 +2,7 @@ using System.Collections.ObjectModel;
using System.Net;
using Foxnouns.Backend.Middleware;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
namespace Foxnouns.Backend;
@ -31,11 +32,12 @@ public class ApiError(string message, HttpStatusCode? statusCode = null, ErrorCo
public readonly string[] Scopes = scopes?.ToArray() ?? [];
}
public class BadRequest(string message, IReadOnlyDictionary<string, string>? errors = null)
public class BadRequest(string message, IReadOnlyDictionary<string, IEnumerable<ValidationError>>? errors = null)
: ApiError(message, statusCode: HttpStatusCode.BadRequest)
{
public BadRequest(string message, string field) : this(message,
new Dictionary<string, string> { { field, message } })
public BadRequest(string message, string field, object actualValue) : this("Error validating input",
new Dictionary<string, IEnumerable<ValidationError>>
{ { field, [ValidationError.GenericValidationError(message, actualValue)] } })
{
}
@ -55,7 +57,7 @@ public class ApiError(string message, HttpStatusCode? statusCode = null, ErrorCo
var errorObj = new JObject
{
{ "key", error.Key },
{ "errors", new JArray(new JObject { { "message", error.Value } }) }
{ "errors", JArray.FromObject(error.Value) }
};
a.Add(errorObj);
}
@ -116,4 +118,55 @@ public enum ErrorCode
GenericApiError,
UserNotFound,
MemberNotFound,
}
public class ValidationError
{
public required string Message { get; init; }
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public int? MinLength { get; init; }
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public int? MaxLength { get; init; }
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public int? ActualLength { get; init; }
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public IEnumerable<object>? AllowedValues { get; init; }
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public object? ActualValue { get; init; }
public static ValidationError LengthError(string message, int minLength, int maxLength, int actualLength)
{
return new ValidationError
{
Message = message,
MinLength = minLength,
MaxLength = maxLength,
ActualLength = actualLength
};
}
public static ValidationError DisallowedValueError(string message, IEnumerable<object> allowedValues,
object actualValue)
{
return new ValidationError
{
Message = message,
AllowedValues = allowedValues,
ActualValue = actualValue
};
}
public static ValidationError GenericValidationError(string message, object? actualValue)
{
return new ValidationError
{
Message = message,
ActualValue = actualValue
};
}
}

View file

@ -47,7 +47,7 @@ public class AuthService(IClock clock, DatabaseContext db, ISnowflakeGenerator s
AssertValidAuthType(authType, instance);
if (await db.Users.AnyAsync(u => u.Username == username))
throw new ApiError.BadRequest("Username is already taken", "username");
throw new ApiError.BadRequest("Username is already taken", "username", username);
var user = new User
{
@ -124,7 +124,7 @@ public class AuthService(IClock clock, DatabaseContext db, ISnowflakeGenerator s
public (string, Token) GenerateToken(User user, Application application, string[] scopes, Instant expires)
{
if (!AuthUtils.ValidateScopes(application, scopes))
throw new ApiError.BadRequest("Invalid scopes requested for this token", "scopes");
throw new ApiError.BadRequest("Invalid scopes requested for this token", "scopes", scopes);
var (token, hash) = GenerateToken();
return (token, new Token

View file

@ -23,54 +23,65 @@ public static class ValidationUtils
"pronouns.cc",
"pronounscc"
];
/// <summary>
/// Validates whether a username is valid. If it is not valid, throws <see cref="Foxnouns.Backend.ApiError" />.
/// This does not check if the username is already taken.
/// </summary>
public static void ValidateUsername(string username)
public static ValidationError? ValidateUsername(string username)
{
if (!UsernameRegex.IsMatch(username))
throw username.Length switch
return username.Length switch
{
< 2 => new ApiError.BadRequest("Username is too short", "username"),
> 40 => new ApiError.BadRequest("Username is too long", "username"),
_ => new ApiError.BadRequest(
"Username is invalid, can only contain alphanumeric characters, dashes, underscores, and periods",
"username")
< 2 => ValidationError.LengthError("Username is too short", 2, 40, username.Length),
> 40 => ValidationError.LengthError("Username is too long", 2, 40, username.Length),
_ => ValidationError.GenericValidationError(
"Username is invalid, can only contain alphanumeric characters, dashes, underscores, and periods", username)
};
if (InvalidUsernames.Any(u => string.Equals(u, username, StringComparison.InvariantCultureIgnoreCase)))
throw new ApiError.BadRequest("Username is not allowed", "username");
return ValidationError.GenericValidationError("Username is not allowed", username);
return null;
}
public static void ValidateDisplayName(string? displayName)
public static void Validate(IEnumerable<(string, ValidationError?)> errors)
{
if (displayName == null) return;
switch (displayName.Length)
errors = errors.Where(e => e.Item2 != null).ToList();
if (!errors.Any()) return;
var errorDict = new Dictionary<string, IEnumerable<ValidationError>>();
foreach (var error in errors)
{
case 0:
throw new ApiError.BadRequest("Display name is too short", "display_name");
case > 100:
throw new ApiError.BadRequest("Display name is too long", "display_name");
if (errorDict.TryGetValue(error.Item1, out var value)) errorDict[error.Item1] = value.Append(error.Item2!);
errorDict.Add(error.Item1, [error.Item2!]);
}
throw new ApiError.BadRequest("Error validating input", errorDict);
}
public static void ValidateBio(string? bio)
public static ValidationError? ValidateDisplayName(string? displayName)
{
if (bio == null) return;
switch (bio.Length)
return displayName?.Length switch
{
case 0:
throw new ApiError.BadRequest("Bio is too short", "bio");
case > 1024:
throw new ApiError.BadRequest("Bio is too long", "bio");
}
0 => ValidationError.LengthError("Display name is too short", 1, 100, displayName.Length),
> 100 => ValidationError.LengthError("Display name is too long", 1, 100, displayName.Length),
_ => null
};
}
public static void ValidateAvatar(string? avatar)
public static ValidationError? ValidateBio(string? bio)
{
if (avatar == null) return;
if (avatar.Length > 1_500_000) throw new ApiError.BadRequest("Avatar is too big", "avatar");
return bio?.Length switch
{
0 => ValidationError.LengthError("Bio is too short", 1, 1024, bio.Length),
> 1024 => ValidationError.LengthError("Bio is too long", 1, 1024, bio.Length),
_ => null
};
}
public static ValidationError? ValidateAvatar(string? avatar)
{
return avatar?.Length switch
{
0 => ValidationError.GenericValidationError("Avatar cannot be empty", null),
> 1_500_00 => ValidationError.GenericValidationError("Avatar is too large", null),
_ => null
};
}
}