From a3078853109d1719e08033940f470da2e76e73d9 Mon Sep 17 00:00:00 2001 From: Paul Schneider Date: Fri, 25 Sep 2026 20:50:58 +0100 Subject: [PATCH] Blogs: list personal files under the user's login, not the sub claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET api/v1/fs passed User.GetUserId() (the "sub" claim — a GUID in production) to GetUserFiles, so the listing looked under {Blog}/{sub-GUID}. But the personal-storage tree is keyed by the user's login everywhere else: EnsureDestinationDirectory writes under {Blog}/{name-claim}, the download endpoint and avatar paths use user.UserName. So once the upload path was fixed (previous commit), files landed under {Blog}/{login} while the listing still looked under {Blog}/{GUID} — an empty tree — hence the PostIt "Mes fichiers" page listing nothing. GetDir now resolves the user from the DB (as Post already does) and lists under user.UserName. UploadedFile.OwnerId stays "sub", so EnrichWithFileIds still queries by uid. Tests: - TestTokenIssuer.Issue gains an optional `name` param (defaults to subject, preserving existing behavior) so a test token can mirror production where sub (GUID) ≠ name (login). - FileSystemApiTests.Get_fs_lists_files_stored_under_the_user_login decouples sub from the login, uploads under the login, and asserts GET /api/v1/fs returns the file (and the subject-named directory does NOT exist). Verified to fail when the controller uses uid for the listing. Co-Authored-By: Claude Code --- src/Yavsc.Blogs.Tests/FileSystemApiTests.cs | 76 ++++++++++++++++++- .../Controllers/FileSystemApiController.cs | 13 +++- src/Yavsc.Tests.Shared/TestTokenIssuer.cs | 9 ++- 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/src/Yavsc.Blogs.Tests/FileSystemApiTests.cs b/src/Yavsc.Blogs.Tests/FileSystemApiTests.cs index b5eeb8e00..e67f235aa 100644 --- a/src/Yavsc.Blogs.Tests/FileSystemApiTests.cs +++ b/src/Yavsc.Blogs.Tests/FileSystemApiTests.cs @@ -44,7 +44,7 @@ public sealed class FileSystemApiTests : IClassFixture _fixture = fixture; } - private HttpClient NewClient(string subject = "alice") + private HttpClient NewClient(string subject = "alice", string? name = null) { var handler = new HttpClientHandler { @@ -54,8 +54,11 @@ public sealed class FileSystemApiTests : IClassFixture { BaseAddress = new Uri(_fixture.Addresses.First(a => a.StartsWith("https://"))) }; + // Pass a distinct name to mirror production (sub = GUID, name = + // login); defaults to subject for the existing tests where the + // two are equal. http.DefaultRequestHeaders.Authorization = - new AuthenticationHeaderValue("Bearer", TestTokenIssuer.Issue(subject)); + new AuthenticationHeaderValue("Bearer", TestTokenIssuer.Issue(subject, name: name)); return http; } @@ -129,6 +132,75 @@ public sealed class FileSystemApiTests : IClassFixture "Expected the uploaded file on disk under the owner's personal root"); } + /// + /// Regression for the "Mes fichiers" page listing nothing: the + /// personal-storage tree is keyed by the user's login + /// (UserName) — EnsureDestinationDirectory writes + /// under {Blog}/{name-claim}, the download endpoint and + /// avatar paths use user.UserName — but GET api/v1/fs + /// passed User.GetUserId() (the sub claim, a GUID in + /// production) to GetUserFiles, so the listing looked under + /// {Blog}/{sub-GUID}, an empty tree. This test decouples + /// sub from the login (mirroring production) and asserts the + /// listing returns the file stored under the login. + /// + [Fact] + public async Task Get_fs_lists_files_stored_under_the_user_login() + { + _fixture.ResetDatabase(); + // sub = "alice-sub" (GUID-like), login = "alice" — the shape + // ProfileService mints in production. DiskQuota non-zero so the + // upload persists the UploadedFile row (see Post_fs test). + _fixture.SeedUser("alice", u => + { + u.Id = "alice-sub"; + u.DiskQuota = 10_000_000; + }); + + var payload = System.Text.Encoding.UTF8.GetBytes("list me"); + + using var form = new MultipartFormDataContent(); + var fileContent = new ByteArrayContent(payload); + fileContent.Headers.ContentType = new MediaTypeHeaderValue("text/plain"); + form.Add(fileContent, "file", "note.txt"); + + // Upload with a token whose sub is the GUID-like id and whose + // name is the login — EnsureDestinationDirectory keys off "name". + using (var http = NewClient("alice-sub", name: "alice")) + { + var post = await http.PostAsync("/api/v1/fs", form, TestContext.Current.CancellationToken); + var postBody = await post.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(post.StatusCode == HttpStatusCode.OK, + $"Upload failed: {(int)post.StatusCode} ({post.StatusCode}): {postBody}"); + } + + // The file landed under the login, not the subject. + Assert.True(File.Exists(PhysicalFile(_fixture, "alice", "note.txt")), + "Expected the uploaded file under {BlogFilesRoot}/alice/note.txt (the login)"); + Assert.False(Directory.Exists(Path.Combine(_fixture.BlogFilesRoot, "alice-sub")), + "The subject-named directory must NOT exist — storage is keyed by the login"); + + // GET /api/v1/fs must list under the login and find the file. + using var get = NewClient("alice-sub", name: "alice"); + var response = await get.GetAsync("/api/v1/fs", TestContext.Current.CancellationToken); + var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(response.StatusCode == HttpStatusCode.OK, + $"List failed: {(int)response.StatusCode} ({response.StatusCode}): {body}"); + + using var doc = JsonDocument.Parse(body); + Assert.True(doc.RootElement.TryGetProperty("files", out var filesEl), + $"Response has no 'files' property. Body: {body}"); + var names = filesEl.EnumerateArray().Select(f => f.GetProperty("name").GetString()).ToArray(); + Assert.Contains("note.txt", names); + + // The UploadedFile row (OwnerId = sub) is enriched onto the + // entry, so the client can attach the file by reference. + var noteEntry = filesEl.EnumerateArray() + .First(f => f.GetProperty("name").GetString() == "note.txt"); + Assert.True(noteEntry.TryGetProperty("id", out var idEl) && idEl.GetInt64() > 0, + "Expected the enriched UploadedFile id on the listed entry"); + } + [Fact] public async Task GetFile_owner_can_download_their_own_file() { diff --git a/src/Yavsc.Blogs/Controllers/FileSystemApiController.cs b/src/Yavsc.Blogs/Controllers/FileSystemApiController.cs index 1b1038f57..2ca4250c0 100644 --- a/src/Yavsc.Blogs/Controllers/FileSystemApiController.cs +++ b/src/Yavsc.Blogs/Controllers/FileSystemApiController.cs @@ -54,7 +54,18 @@ namespace Yavsc.Blogs.Controllers { if (!ModelState.IsValid) return new BadRequestObjectResult(ModelState); var uid = User.GetUserId(); - var files = FileSystemHelpers.GetUserFiles(siteSettings, uid, subdir); + // The personal-storage tree is keyed by the user's login + // (UserName): EnsureDestinationDirectory writes under + // {Blog}/{UserName}, the download endpoint and avatar paths + // use user.UserName too. GetUserId() returns the "sub" claim + // (a GUID in production), so passing uid here made the listing + // look under {Blog}/{sub-GUID} — an empty tree, hence the + // "Mes fichiers" page listing nothing. Resolve the login and + // list under it. OwnerId on UploadedFile stays "sub" (uid), + // so EnrichWithFileIds still queries by uid. + var user = dbContext.Users.SingleOrDefault(u => u.Id == uid); + if (user == null) return NotFound(); + var files = FileSystemHelpers.GetUserFiles(siteSettings, user.UserName, subdir); EnrichWithFileIds(files, uid, subdir); return Ok(files); } diff --git a/src/Yavsc.Tests.Shared/TestTokenIssuer.cs b/src/Yavsc.Tests.Shared/TestTokenIssuer.cs index 0046fc346..b1e0fc005 100644 --- a/src/Yavsc.Tests.Shared/TestTokenIssuer.cs +++ b/src/Yavsc.Tests.Shared/TestTokenIssuer.cs @@ -63,11 +63,18 @@ public static class TestTokenIssuer /// Value of the scope claim. The /// production BlogScope policy requires /// RequireClaim("scope", "blogs"). + /// Value of the name claim. Defaults to + /// (the test convention where the login + /// equals the subject). Pass a distinct login to mirror production, + /// where sub is a GUID and name is the user's + /// UserName — the file-system list/upload paths key the + /// personal-storage tree off the login, not the subject. /// Optional additional claims /// (e.g. a role for an admin-bypass test). public static string Issue( string subject, string scope = "blogs", + string? name = null, IEnumerable? extraClaims = null) { var now = DateTime.UtcNow; @@ -81,7 +88,7 @@ public static class TestTokenIssuer // — not via User.Identity.Name. A test token without "name" would // make ReceiveUserFile / EnsureDestinationDirectory compute a // null owner root and 500 on upload, so stamp it here. - new("name", subject), + new("name", name ?? subject), new("scope", scope), }; if (extraClaims is not null) claims.AddRange(extraClaims);