Skip to content
Merged

Fixes #426

Show file tree
Hide file tree
Changes from all commits
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
27 changes: 9 additions & 18 deletions Jiten.Api/Controllers/AuthController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Google.Apis.Auth;
using Jiten.Api.Dtos;
using Jiten.Api.Dtos.Requests;
using Jiten.Api.Helpers;
using Jiten.Api.Services;
using Jiten.Core;
using Jiten.Core.Data.Authentication;
Expand Down Expand Up @@ -85,16 +86,16 @@ public async Task<IActionResult> Register([FromBody] RegisterRequest model)
var userName = model.Username.Trim();
var email = model.Email.Trim();

var usernameError = UsernameValidator.Validate(userName);
if (usernameError != null)
return BadRequest(new { message = usernameError });

var userExists = await _userManager.FindByNameAsync(userName);
if (userExists != null) return Conflict(new { message = "Username already exists." });

var emailExists = await _userManager.FindByEmailAsync(email);
if (emailExists != null) return Conflict(new { message = "Email already registered." });


if (userName.Length is < 3 or > 30)
return BadRequest(new { message = "Username must be between 3 and 30 characters." });

var user = new User
{
UserName = userName, Email = email, SecurityStamp = Guid.NewGuid().ToString(), TosAcceptedAt = DateTime.UtcNow,
Expand Down Expand Up @@ -512,25 +513,15 @@ public async Task<ActionResult<TokenResponse>> CompleteGoogleRegistration([FromB

var username = request.Username.Trim();

if (string.IsNullOrWhiteSpace(username))
var usernameError = UsernameValidator.Validate(username);
if (usernameError != null)
{
return BadRequest(new { message = "Username is required" });
}

if (username.Length < 3 || username.Length > 30)
{
return BadRequest(new { message = "Username must be between 3 and 30 characters" });
return BadRequest(new { message = usernameError });
}

var userExists = await _userManager.FindByNameAsync(username);
if (userExists != null) return Conflict(new { message = "Username already exists." });

var usernameExists = await _userManager.Users.AnyAsync(u => u.UserName == request.Username);
if (usernameExists)
{
return BadRequest(new { message = "Username is already taken" });
}

var emailExists = await _userManager.Users.AnyAsync(u => u.Email == registrationData!.Email);
if (emailExists)
{
Expand All @@ -540,7 +531,7 @@ public async Task<ActionResult<TokenResponse>> CompleteGoogleRegistration([FromB
// Create the user
var user = new User
{
UserName = request.Username, Email = registrationData.Email, EmailConfirmed = true, TosAcceptedAt = DateTime.UtcNow,
UserName = username, Email = registrationData!.Email, EmailConfirmed = true, TosAcceptedAt = DateTime.UtcNow,
ReceivesNewsletter = request.ReceiveNewsletter
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace Jiten.Api.Dtos.Requests;

public class CompleteGoogleRegistrationRequest
{
[Required, MaxLength(30)]
[Required, MinLength(2), MaxLength(30)]
public required string Username { get; set; }

public required string TempToken { get; set; }
Expand Down
2 changes: 1 addition & 1 deletion Jiten.Api/Dtos/Requests/RegisterRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace Jiten.Api.Dtos.Requests;

public class RegisterRequest
{
[Required, MaxLength(30)]
[Required, MinLength(2), MaxLength(30)]
public required string Username { get; set; }

[Required, EmailAddress, MaxLength(100)]
Expand Down
45 changes: 45 additions & 0 deletions Jiten.Api/Helpers/UsernameValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
using System.Text.RegularExpressions;

namespace Jiten.Api.Helpers;

/// <summary>
/// Central username validation shared by the email/password and Google registration paths so both
/// reject the same inputs with the same messages.
/// The allowed set is a subset of ASP.NET Identity's default <c>AllowedUserNameCharacters</c>, so a
/// name that passes here is always accepted by <c>UserManager.CreateAsync</c>.
/// </summary>
public static partial class UsernameValidator
{
public const int MinLength = 2;
public const int MaxLength = 30;

// Latin letters/digits plus the punctuation Identity's default AllowedUserNameCharacters permits
// (email-style names like tony@aol.com are allowed). This set is a subset of that default, so a
// name accepted here always passes UserManager.CreateAsync.
[GeneratedRegex(@"^[A-Za-z0-9._@+-]+$")]
private static partial Regex AllowedPattern();

/// <summary>
/// Validates a username. Returns null when valid, otherwise a user-facing error message.
/// Callers should pass the already-trimmed username.
/// </summary>
public static string? Validate(string? username)
{
if (string.IsNullOrWhiteSpace(username))
return "Username is required.";

if (username.Length < MinLength)
return $"Username must be at least {MinLength} characters.";

if (username.Length > MaxLength)
return $"Username must be at most {MaxLength} characters.";

if (!AllowedPattern().IsMatch(username))
return "Username can only contain Latin letters, digits and the characters . _ - @ +";

if (!username.Any(char.IsAsciiLetterOrDigit))
return "Username must contain at least one letter or digit.";

return null;
}
}
65 changes: 65 additions & 0 deletions Jiten.Tests/Integration/AccountTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,71 @@ async Task SetNewsletter(bool value)
(await GetNewsletter()).Should().BeFalse();
}

// ---- register username validation ----

private Task<HttpResponseMessage> RegisterAsync(string username, string email) =>
_client.SendAsync(new HttpRequestMessage(HttpMethod.Post, "/api/auth/register")
.WithJsonContent(new
{
username,
email,
password = DefaultPassword,
recaptchaResponse = "test",
tosAccepted = true,
receiveNewsletter = false
}));

[Theory]
[InlineData("valid_user1", "reg_valid@test.dev")]
[InlineData("tony@aol.com", "reg_email_uname@test.dev")] // email-style usernames are allowed
[InlineData("Benjamin_", "reg_trailing@test.dev")] // trailing separator is allowed
[InlineData("ab", "reg_two_char@test.dev")] // 2-char names are allowed
public async Task Register_ValidUsername_Returns200_AndCreatesUser(string username, string email)
{
await EnsureUserRoleAsync(); // role seeding is skipped in the Testing environment

var response = await RegisterAsync(username, email);
response.StatusCode.Should().Be(HttpStatusCode.OK, await response.Content.ReadAsStringAsync());

using var scope = factory.Services.CreateScope();
var userManager = scope.ServiceProvider.GetRequiredService<UserManager<User>>();
(await userManager.FindByNameAsync(username)).Should().NotBeNull();
}

private async Task EnsureUserRoleAsync()
{
using var scope = factory.Services.CreateScope();
var roleManager = scope.ServiceProvider.GetRequiredService<RoleManager<IdentityRole>>();
var roleName = nameof(UserRole.User);
if (!await roleManager.RoleExistsAsync(roleName))
await roleManager.CreateAsync(new IdentityRole(roleName));
}

[Theory]
[InlineData("a")] // too short (min 2)
[InlineData("たなか")] // non-latin (Japanese)
[InlineData("user name")] // space (disallowed char)
[InlineData("user#name")] // '#' disallowed
[InlineData("___")] // no letter or digit
[InlineData("...")] // no letter or digit
[InlineData("ааа")] // Cyrillic look-alikes
public async Task Register_InvalidUsername_Returns400_AndCreatesNoUser(string username)
{
var response = await RegisterAsync(username, "reg_invalid@test.dev");
response.StatusCode.Should().Be(HttpStatusCode.BadRequest, await response.Content.ReadAsStringAsync());

using var scope = factory.Services.CreateScope();
var userDb = scope.ServiceProvider.GetRequiredService<UserDbContext>();
(await userDb.Users.AnyAsync(u => u.Email == "reg_invalid@test.dev")).Should().BeFalse();
}

[Fact]
public async Task Register_TooLongUsername_Returns400()
{
var response = await RegisterAsync(new string('a', 31), "reg_long@test.dev");
response.StatusCode.Should().Be(HttpStatusCode.BadRequest, await response.Content.ReadAsStringAsync());
}

// ---- revoke-token keepCurrent ----

[Fact]
Expand Down
58 changes: 27 additions & 31 deletions Jiten.Web/app/components/CustomMeaning.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
const draft = ref('');
const saving = ref(false);
const deleting = ref(false);
const confirmingDelete = ref(false);

const canSave = computed(() => {
const t = draft.value.trim();
Expand All @@ -43,18 +44,22 @@
}
}

watch(() => props.wordId, () => {
meaning.value = null;
loaded.value = false;
editing.value = false;
load();
});
watch(
() => props.wordId,
() => {
meaning.value = null;
loaded.value = false;
editing.value = false;
load();
}
);

onMounted(load);

function startEditing() {
draft.value = meaning.value ?? '';
editing.value = true;
confirmingDelete.value = false;
}

async function save() {
Expand All @@ -78,6 +83,7 @@
await $api(`user/custom-meanings/${props.wordId}`, { method: 'DELETE' });
meaning.value = null;
editing.value = false;
confirmingDelete.value = false;
} finally {
deleting.value = false;
}
Expand Down Expand Up @@ -116,40 +122,30 @@
<label class="text-xs text-surface-400 block mb-1">Preview</label>
<div class="border-l-4 border-primary-500 pl-3 py-2 bg-primary-50 dark:bg-primary-950/40 rounded-r text-sm break-words" v-html="draftPreview" />
</div>
<div class="flex gap-2 justify-end">
<Button
v-if="meaning != null"
severity="danger"
text
size="small"
icon="pi pi-trash"
label="Delete"
:loading="deleting"
@click="remove"
/>
<Button text size="small" label="Cancel" @click="editing = false" />
<Button
size="small"
icon="pi pi-check"
label="Save"
:loading="saving"
:disabled="!canSave"
@click="save"
/>
<div class="flex gap-2 justify-end items-center">
<template v-if="meaning != null">
<template v-if="confirmingDelete">
<span class="text-xs text-surface-500 mr-auto">Delete this note?</span>
<Button severity="danger" size="small" icon="pi pi-trash" label="Delete" :loading="deleting" @click="remove" />
<Button text size="small" label="Keep" :disabled="deleting" @click="confirmingDelete = false" />
</template>
<Button v-else severity="danger" text size="small" icon="pi pi-trash" label="Delete" @click="confirmingDelete = true" />
</template>
<template v-if="!confirmingDelete">
<Button text size="small" label="Cancel" @click="editing = false" />
<Button size="small" icon="pi pi-check" label="Save" :loading="saving" :disabled="!canSave" @click="save" />
</template>
</div>
</div>

<!-- Display -->
<template v-else>
<div
v-if="meaning != null"
class="group relative border-l-4 border-primary-500 pl-3 pr-2 py-2 bg-primary-50 dark:bg-primary-950/40 rounded-r"
>
<div v-if="meaning != null" class="group relative border-l-4 border-primary-500 pl-3 pr-2 py-2 bg-primary-50 dark:bg-primary-950/40 rounded-r">
<div class="flex items-start gap-2">
<span class="text-xs tracking-wide text-primary-600 dark:text-primary-400 font-semibold mt-0.5">Notes</span>
<button
v-if="editable"
class="ml-auto inline-flex items-center justify-center text-surface-400 hover:text-primary-500 transition-colors shrink-0 cursor-pointer opacity-0 group-hover:opacity-100 focus:opacity-100"
class="ml-auto inline-flex items-center justify-center text-surface-400 hover:text-primary-500 transition-colors shrink-0 cursor-pointer"
title="Edit your notes"
@click.stop="startEditing"
@pointerdown.stop
Expand Down
13 changes: 9 additions & 4 deletions Jiten.Web/app/pages/register.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
return false;
}

if (username.length < 3) {
usernameError.value = 'Username must be at least 3 characters';
if (username.length < 2) {
usernameError.value = 'Username must be at least 2 characters';
return false;
}

Expand All @@ -44,8 +44,13 @@
return false;
}

if (username.includes(' ')) {
usernameError.value = 'Username cannot contain spaces';
if (!/^[A-Za-z0-9._@+-]+$/.test(username)) {
usernameError.value = 'Username can only contain Latin letters, digits and the characters . _ - @ +';
return false;
}

if (!/[A-Za-z0-9]/.test(username)) {
usernameError.value = 'Username must contain at least one letter or digit';
return false;
}

Expand Down