Skip to content

Commit 7ca1750

Browse files
authored
Require password when deleting account (#1054)
For security reasons, this PR makes requests to the user's own account deletion endpoint require a body with a SHA512 hash of the user's password, regardless of whether they are already authenticated. This does alter APIv3 spec, but it's likely not a widely used endpoint. A complementary PR for the refresh-web legacy branch will be opened alongside this one.
2 parents e33523e + 23181ed commit 7ca1750

4 files changed

Lines changed: 127 additions & 2 deletions

File tree

Refresh.Interfaces.APIv3/Endpoints/AuthenticationApiEndpoints.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,9 +425,15 @@ public ApiOkResponse ResendVerificationCode(RequestContext context, GameUser use
425425
}
426426

427427
[ApiV3Endpoint("users/me", HttpMethods.Delete), MinimumRole(GameUserRole.Restricted)]
428-
[DocSummary("Deletes your own account. This action is non-reversible.")]
429-
public ApiOkResponse DeleteMyAccount(RequestContext context, GameUser user, GameDatabaseContext database)
428+
[DocSummary("Deletes your own account. This action is non-reversible. This endpoint now requires you to include your own password while being authenticated.")]
429+
public ApiOkResponse DeleteMyAccount(RequestContext context, GameUser user, ApiOwnUserDeletionRequest body, GameDatabaseContext database)
430430
{
431+
if (string.IsNullOrWhiteSpace(body.PasswordSha512))
432+
return new ApiValidationError("You must enter your password to delete your account.");
433+
434+
if (!BC.Verify(body.PasswordSha512, user.PasswordBcrypt))
435+
return new ApiValidationError("The password was incorrect.");
436+
431437
database.DeleteUser(user);
432438
return new ApiOkResponse();
433439
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#nullable disable
2+
namespace Refresh.Interfaces.APIv3.Endpoints.DataTypes.Request.Authentication;
3+
4+
[JsonObject(NamingStrategyType = typeof(CamelCaseNamingStrategy))]
5+
public class ApiOwnUserDeletionRequest
6+
{
7+
public string PasswordSha512 { get; set; }
8+
}

RefreshTests.GameServer/Extensions/HttpClientExtensions.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,20 @@ public static class HttpClientExtensions
5858
Assert.That(response.StatusCode, Is.Not.EqualTo(OK));
5959
return ReadData<TData>(response.Content);
6060
}
61+
62+
public static ApiResponse<TData>? DeleteData<TData>(this HttpClient client, string endpoint, object data, bool ensureSuccessful = true, bool ensureFailure = false) where TData : class, IApiResponse
63+
{
64+
// HttpClient's DeleteAsync method does not allow us to include a body
65+
HttpRequestMessage request = new(HttpMethod.Delete, endpoint)
66+
{
67+
Content = new StringContent(data.AsJson())
68+
};
69+
HttpResponseMessage response = client.SendAsync(request).Result;
70+
71+
if (ensureSuccessful)
72+
Assert.That(response.StatusCode, Is.EqualTo(OK));
73+
else if (ensureFailure)
74+
Assert.That(response.StatusCode, Is.Not.EqualTo(OK));
75+
return ReadData<TData>(response.Content);
76+
}
6177
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
using Refresh.Database.Models.Authentication;
2+
using Refresh.Database.Models.Users;
3+
using Refresh.Interfaces.APIv3.Endpoints.ApiTypes;
4+
using Refresh.Interfaces.APIv3.Endpoints.DataTypes.Request.Authentication;
5+
using RefreshTests.GameServer.Extensions;
6+
7+
namespace RefreshTests.GameServer.Tests.ApiV3;
8+
9+
public class UserDeletionApiTests : GameServerTest
10+
{
11+
[Test]
12+
public void CanDeleteOwnUserWithPassword()
13+
{
14+
using TestContext context = this.GetServer();
15+
const string password = "password";
16+
17+
GameUser user = context.CreateUser();
18+
string passwordBcrypt = BCrypt.Net.BCrypt.HashPassword(password, 4);
19+
context.Database.SetUserPassword(user, passwordBcrypt);
20+
using HttpClient client = context.GetAuthenticatedClient(TokenType.Api, user);
21+
22+
Assert.That(context.Database.GetUserByObjectId(user.UserId), Is.Not.Null);
23+
24+
// Try to delete
25+
ApiOwnUserDeletionRequest request = new()
26+
{
27+
PasswordSha512 = password
28+
};
29+
ApiResponse<ApiEmptyResponse>? response = client.DeleteData<ApiEmptyResponse>($"/api/v3/users/me", request);
30+
Assert.That(response?.Data, Is.Not.Null);
31+
Assert.That(response!.Success, Is.True);
32+
33+
context.Database.Refresh();
34+
GameUser? fakeDeletedUser = context.Database.GetUserByObjectId(user.UserId);
35+
Assert.That(fakeDeletedUser, Is.Not.Null);
36+
Assert.That(fakeDeletedUser!.Role, Is.EqualTo(GameUserRole.Banned));
37+
Assert.That(fakeDeletedUser!.PasswordBcrypt, Is.EqualTo("deleted"));
38+
}
39+
40+
[Test]
41+
public async Task CannotDeleteOwnUserWithoutPassword()
42+
{
43+
using TestContext context = this.GetServer();
44+
const string password = "password";
45+
46+
GameUser user = context.CreateUser();
47+
string passwordBcrypt = BCrypt.Net.BCrypt.HashPassword(password, 4);
48+
context.Database.SetUserPassword(user, passwordBcrypt);
49+
using HttpClient client = context.GetAuthenticatedClient(TokenType.Api, user);
50+
51+
Assert.That(context.Database.GetUserByObjectId(user.UserId), Is.Not.Null);
52+
53+
// Try to delete
54+
HttpResponseMessage? response = await client.DeleteAsync($"/api/v3/users/me");
55+
Assert.That(response, Is.Not.Null);
56+
Assert.That(response!.IsSuccessStatusCode, Is.False);
57+
58+
context.Database.Refresh();
59+
GameUser? fakeDeletedUser = context.Database.GetUserByObjectId(user.UserId);
60+
Assert.That(fakeDeletedUser, Is.Not.Null);
61+
Assert.That(fakeDeletedUser!.Role, Is.EqualTo(GameUserRole.User));
62+
Assert.That(fakeDeletedUser!.PasswordBcrypt, Is.Not.EqualTo("deleted"));
63+
}
64+
65+
[Test]
66+
public void CannotDeleteOwnUserWithWrongPassword()
67+
{
68+
using TestContext context = this.GetServer();
69+
const string correctPassword = "password";
70+
const string wrongPassword = "assword";
71+
72+
GameUser user = context.CreateUser();
73+
string correctPasswordBcrypt = BCrypt.Net.BCrypt.HashPassword(correctPassword, 4);
74+
string wrongPasswordBcrypt = BCrypt.Net.BCrypt.HashPassword(wrongPassword, 4);
75+
context.Database.SetUserPassword(user, correctPasswordBcrypt);
76+
using HttpClient client = context.GetAuthenticatedClient(TokenType.Api, user);
77+
78+
Assert.That(context.Database.GetUserByObjectId(user.UserId), Is.Not.Null);
79+
80+
// Try to delete
81+
ApiOwnUserDeletionRequest request = new()
82+
{
83+
PasswordSha512 = wrongPassword
84+
};
85+
ApiResponse<ApiEmptyResponse>? response = client.DeleteData<ApiEmptyResponse>($"/api/v3/users/me", request, false, true);
86+
Assert.That(response?.Error, Is.Not.Null);
87+
Assert.That(response!.Success, Is.False);
88+
89+
context.Database.Refresh();
90+
GameUser? fakeDeletedUser = context.Database.GetUserByObjectId(user.UserId);
91+
Assert.That(fakeDeletedUser, Is.Not.Null);
92+
Assert.That(fakeDeletedUser!.Role, Is.EqualTo(GameUserRole.User));
93+
Assert.That(fakeDeletedUser!.PasswordBcrypt, Is.Not.EqualTo("deleted"));
94+
}
95+
}

0 commit comments

Comments
 (0)