diff --git a/docs/online-inbox-api-contract.md b/docs/online-inbox-api-contract.md index 7f5ff1c..48c34ff 100644 --- a/docs/online-inbox-api-contract.md +++ b/docs/online-inbox-api-contract.md @@ -24,7 +24,28 @@ Sync directions (each one-way per entity → no conflict resolution needed): - **Idle tasks**: desktop mirrors its Idle backlog up; the web can create new ones, which the desktop pulls down and then owns. -Single user. Both the desktop and the web client authenticate as the **same Zitadel user**. +Single user today. Both the desktop and the web client authenticate as the **same Zitadel +user**. + +**Multi-user readiness (`ownerId`).** Each resource is owned by a Zitadel subject (`sub`). +`RemoteList`, `RemoteTask`, and `MirrorTask` carry an optional `ownerId` field. The desktop +stamps its own `sub` (decoded from the access token) onto everything it pushes, and +defensively ignores any pulled task whose `ownerId` is set to a *different* user; an absent +`ownerId` is treated as unowned/legacy and still syncs. This keeps the contract ready for +multiple users **without enforcing isolation client-side** — the server remains the +authority that scopes every request by the token's `sub`. When the server goes multi-user it +should partition all rows by owner and ignore (or validate) the client-supplied `ownerId`. + +**Access control (as of 2026-06-10).** Access is granted by assigning the **"user" project +role** in the Zitadel project "ClaudeDo" (id `376787351902355727`, issuer +`https://auth.kuns.dev`) — there is no app-side allowlist (the former `ALLOWED_USER_IDS` +env var is gone). The access token carries the role in the claim +`urn:zitadel:iam:org:project:roles` (or the project-scoped variant +`urn:zitadel:iam:org:project:376787351902355727:roles`), an object keyed by role key, e.g. +`{ "user": { "": "" } }`. The desktop OIDC client +(id `376787352137302287`) has `accessTokenRoleAssertion` enabled, so any token issued +after login/refresh includes the claim automatically — no extra scopes are needed. +Granting/revoking access is purely a Zitadel role grant, nothing app-side. ## 2. Idle backlog definition (desktop side) @@ -68,9 +89,12 @@ All task writes are idempotent upserts keyed on id. ## 4. Endpoints -All endpoints require a valid Zitadel access token (`Authorization: Bearer `). -Missing/invalid/expired → `401`. No anonymous access (imported tasks can trigger code -execution on the user's machine). +All endpoints require a valid Zitadel access token (`Authorization: Bearer `) that +carries the **"user" project role** (see §1). Missing/invalid/expired token, or a valid +token without the role → `401`. No anonymous access (imported tasks can trigger code +execution on the user's machine). The desktop client treats a `401` as: force a +refresh-token exchange and retry once; if a freshly issued token is still rejected, it +surfaces "missing 'user' role in Zitadel" and pauses sync until the user signs in again. > **Auth (VPS/.NET):** use the in-house `KunsZitadel` nuget package (feed > `https://git.kuns.dev/api/packages/kuns/nuget/index.json`) — call `AddKunsZitadel(...)` @@ -80,13 +104,17 @@ execution on the user's machine). | Method & path | Caller | Body | Response | |---|---|---|---| -| `PUT /lists` | desktop | `[{ "id", "name" }]` — the FULL catalog | `200` | -| `GET /lists` | web | — | `200 [{ "id", "name" }]` | +| `PUT /lists` | desktop | `[{ "id", "name", "ownerId"? }]` — the FULL catalog | `200` | +| `GET /lists` | web | — | `200 [{ "id", "name", "ownerId"? }]` | | `GET /lists/{id}/tasks` | web | — | `200` tasks in that list (`404` if list unknown) | | `POST /tasks` | web | `{ "title", "description"?, "listId" }` | `201` created task incl. `id` | -| `GET /tasks?imported=false` | desktop | — | `200 [{ "id","listId","title","description","createdAt" }]` | +| `GET /tasks?imported=false` | desktop | — | `200 [{ "id","listId","title","description","createdAt","ownerId"? }]` | | `POST /tasks/{id}/imported` | desktop | — | `200` (`404` if unknown) | -| `PUT /tasks/mirror` | desktop | `[{ "id","listId","title","description" }]` — full Idle set | `200` | +| `PUT /tasks/mirror` | desktop | `[{ "id","listId","title","description","ownerId"? }]` — full Idle set | `200` | + +`ownerId` (optional, see §1) is the Zitadel `sub` of the owner. The desktop sends it on push +and ignores pulled tasks owned by a different user; the server should derive/validate it from +the token rather than trust the client value. Semantics: diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index 1cc0161..9db1802 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -447,7 +447,7 @@ "merge": { "commitMessage": "Merge-Aufgabe: {0}", "workerOfflineBranches": "Worker offline — Branches können nicht aufgelistet werden.", "loadBranchesFailed": "Branches konnten nicht geladen werden: {0}", "merged": "Zusammengeführt.", "conflict": "Merge-Konflikt — Ziel-Branch wiederhergestellt. Manuell oder über Fortsetzen lösen, dann erneut versuchen.", "blocked": "Blockiert: {0}", "unknownStatus": "Unbekannter Status: {0}", "mergeFailed": "Merge fehlgeschlagen: {0}" }, "conflictResolution": { "vsCodeError": "VS Code konnte nicht gestartet werden: {0}. Die Pfade sind oben aufgeführt — kopiere sie manuell.", "subtaskPrefix": "Konflikte in Teilaufgabe: {0}", "targetPrefix": "Zusammenführen in: {0}" }, "settingsModal": { "workerOffline": "Worker offline — Einstellungen schreibgeschützt.", "saveFailed": "Speichern fehlgeschlagen: {0}" }, - "onlineInbox": { "workerOffline": "Worker offline — Konfiguration kann nicht geladen werden.", "saved": "Konfiguration gespeichert.", "saveFailed": "Speichern fehlgeschlagen: {0}", "signedIn": "Erfolgreich angemeldet.", "signInFailed": "Anmeldung fehlgeschlagen: {0}", "signedOut": "Abgemeldet.", "signOutFailed": "Abmeldung fehlgeschlagen: {0}" }, + "onlineInbox": { "workerOffline": "Worker offline — Konfiguration kann nicht geladen werden.", "saved": "Konfiguration gespeichert.", "saveFailed": "Speichern fehlgeschlagen: {0}", "signedIn": "Erfolgreich angemeldet.", "signedInNoRole": "Angemeldet, aber diesem Konto fehlt die Rolle 'user' in Zitadel — die Online-Synchronisierung wird abgelehnt, bis die Rolle im ClaudeDo-Projekt zugewiesen wird.", "signInFailed": "Anmeldung fehlgeschlagen: {0}", "signedOut": "Abgemeldet.", "signOutFailed": "Abmeldung fehlgeschlagen: {0}" }, "weeklyReport": { "invalidRange": "Ungültiger Datumsbereich.", "generating": "Bericht wird erstellt…", "error": "Fehler: {0}" }, "filesTab": { "workerOffline": "Worker offline.", "noneBundled": "Keine Standard-Agenten mitgeliefert.", "allPresent": "Alle Standard-Agenten bereits vorhanden.", "restored": "{0} Standard-Agent(en) wiederhergestellt.", "restoreFailed": "Wiederherstellung fehlgeschlagen: {0}", "openFailed": "Öffnen fehlgeschlagen: {0}" }, "worktreesTab": { "workerOffline": "Worker offline.", "removed": "{0} Worktree(s) entfernt.", "blocked": "Zwangsentfernung nicht möglich: {0} Aufgabe(n) laufen noch. Brich sie zuerst ab.", "removedFrom": "{0} Worktree(s) von {1} Aufgabe(n) entfernt." }, diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index cb6e1ea..b732dc2 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -447,7 +447,7 @@ "merge": { "commitMessage": "Merge task: {0}", "workerOfflineBranches": "Worker offline — cannot list branches.", "loadBranchesFailed": "Failed to load branches: {0}", "merged": "Merged.", "conflict": "Merge conflict — target branch restored. Resolve manually or via Continue, then retry.", "blocked": "Blocked: {0}", "unknownStatus": "Unknown status: {0}", "mergeFailed": "Merge failed: {0}" }, "conflictResolution": { "vsCodeError": "Could not launch VS Code: {0}. Paths are listed above — copy them manually.", "subtaskPrefix": "Conflicts in subtask: {0}", "targetPrefix": "Merging into: {0}" }, "settingsModal": { "workerOffline": "Worker offline — settings read-only.", "saveFailed": "Save failed: {0}" }, - "onlineInbox": { "workerOffline": "Worker offline — cannot load config.", "saved": "Config saved.", "saveFailed": "Save failed: {0}", "signedIn": "Signed in successfully.", "signInFailed": "Sign-in failed: {0}", "signedOut": "Signed out.", "signOutFailed": "Sign-out failed: {0}" }, + "onlineInbox": { "workerOffline": "Worker offline — cannot load config.", "saved": "Config saved.", "saveFailed": "Save failed: {0}", "signedIn": "Signed in successfully.", "signedInNoRole": "Signed in, but this account is missing the 'user' role in Zitadel — online sync will be rejected until the role is granted in the ClaudeDo project.", "signInFailed": "Sign-in failed: {0}", "signedOut": "Signed out.", "signOutFailed": "Sign-out failed: {0}" }, "weeklyReport": { "invalidRange": "Invalid date range.", "generating": "Generating report…", "error": "Error: {0}" }, "filesTab": { "workerOffline": "Worker offline.", "noneBundled": "No default agents bundled.", "allPresent": "All default agents already present.", "restored": "Restored {0} default agent(s).", "restoreFailed": "Restore failed: {0}", "openFailed": "Open failed: {0}" }, "worktreesTab": { "workerOffline": "Worker offline.", "removed": "Removed {0} worktree(s).", "blocked": "Cannot force-remove: {0} task(s) still running. Cancel them first.", "removedFrom": "Removed {0} worktree(s) from {1} task(s)." }, diff --git a/src/ClaudeDo.Ui/Services/Interfaces/IOnlineLoginService.cs b/src/ClaudeDo.Ui/Services/Interfaces/IOnlineLoginService.cs index ac620a1..4658847 100644 --- a/src/ClaudeDo.Ui/Services/Interfaces/IOnlineLoginService.cs +++ b/src/ClaudeDo.Ui/Services/Interfaces/IOnlineLoginService.cs @@ -1,6 +1,6 @@ namespace ClaudeDo.Ui.Services; -public sealed record OnlineLoginResult(bool Success, string? RefreshToken, string? Error); +public sealed record OnlineLoginResult(bool Success, string? RefreshToken, string? Error, string? Warning = null); public interface IOnlineLoginService { diff --git a/src/ClaudeDo.Ui/Services/OnlineLoginService.cs b/src/ClaudeDo.Ui/Services/OnlineLoginService.cs index 8943e3b..51fe90f 100644 --- a/src/ClaudeDo.Ui/Services/OnlineLoginService.cs +++ b/src/ClaudeDo.Ui/Services/OnlineLoginService.cs @@ -34,7 +34,13 @@ public sealed class OnlineLoginService : IOnlineLoginService return new OnlineLoginResult(false, null, "No refresh token returned. Ensure 'offline_access' is in scope and the client allows it."); - return new OnlineLoginResult(true, result.RefreshToken, null); + // Early heads-up: if the access token lacks the "user" project role the server will + // reject sync with a 401. Login still succeeds; surface this as a warning, not an error. + var warning = ZitadelTokenInspector.HasUserRole(result.AccessToken) + ? null + : "missing-user-role"; + + return new OnlineLoginResult(true, result.RefreshToken, null, warning); } catch (OperationCanceledException) { diff --git a/src/ClaudeDo.Ui/Services/ZitadelTokenInspector.cs b/src/ClaudeDo.Ui/Services/ZitadelTokenInspector.cs new file mode 100644 index 0000000..0fd4d27 --- /dev/null +++ b/src/ClaudeDo.Ui/Services/ZitadelTokenInspector.cs @@ -0,0 +1,64 @@ +using System; +using System.Text.Json; + +namespace ClaudeDo.Ui.Services; + +/// +/// Minimal, dependency-free inspection of a Zitadel JWT access token. Used to warn early when +/// a freshly issued token lacks the "user" project role (the server otherwise rejects sync +/// with a 401). The server remains the source of truth — this check fails open. +/// +public static class ZitadelTokenInspector +{ + private const string ProjectRolesClaim = "urn:zitadel:iam:org:project:roles"; + private const string ProjectRolesClaimPrefix = "urn:zitadel:iam:org:project:"; + private const string ProjectRolesClaimSuffix = ":roles"; + private const string UserRole = "user"; + + /// + /// Returns true if the access token carries the "user" role in either the generic or + /// project-scoped Zitadel roles claim. Returns true (fail-open) if the token is absent or + /// cannot be parsed — never block login on a decode hiccup. + /// + public static bool HasUserRole(string? accessToken) + { + if (string.IsNullOrWhiteSpace(accessToken)) + return true; + + var parts = accessToken.Split('.'); + if (parts.Length < 2) + return true; + + try + { + using var doc = JsonDocument.Parse(Base64UrlDecode(parts[1])); + foreach (var claim in doc.RootElement.EnumerateObject()) + { + if (claim.Name != ProjectRolesClaim && + !(claim.Name.StartsWith(ProjectRolesClaimPrefix, StringComparison.Ordinal) && + claim.Name.EndsWith(ProjectRolesClaimSuffix, StringComparison.Ordinal))) + continue; + + if (claim.Value.ValueKind == JsonValueKind.Object && + claim.Value.TryGetProperty(UserRole, out _)) + return true; + } + return false; + } + catch + { + return true; + } + } + + private static byte[] Base64UrlDecode(string input) + { + var s = input.Replace('-', '+').Replace('_', '/'); + switch (s.Length % 4) + { + case 2: s += "=="; break; + case 3: s += "="; break; + } + return Convert.FromBase64String(s); + } +} diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/Settings/OnlineInboxSettingsViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/Settings/OnlineInboxSettingsViewModel.cs index a3b54c7..abb2e97 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/Settings/OnlineInboxSettingsViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/Settings/OnlineInboxSettingsViewModel.cs @@ -92,7 +92,9 @@ public sealed partial class OnlineInboxSettingsViewModel : ViewModelBase await _worker.SetOnlineInboxAuthAsync(result.RefreshToken!); SignedIn = true; - StatusMessage = Loc.T("vm.onlineInbox.signedIn"); + StatusMessage = result.Warning == "missing-user-role" + ? Loc.T("vm.onlineInbox.signedInNoRole") + : Loc.T("vm.onlineInbox.signedIn"); } catch (Exception ex) { diff --git a/src/ClaudeDo.Ui/Views/MainWindow.axaml b/src/ClaudeDo.Ui/Views/MainWindow.axaml index 6ef8f65..c6b0cd0 100644 --- a/src/ClaudeDo.Ui/Views/MainWindow.axaml +++ b/src/ClaudeDo.Ui/Views/MainWindow.axaml @@ -22,7 +22,7 @@ - + GetAccessTokenAsync(CancellationToken ct = default); + + /// + /// Gets an access token, optionally dropping any cached token first so a fresh + /// (role-bearing) token is minted via the refresh-token grant. Used to recover from a + /// 401 caused by a stale token issued before role assertion was enabled. + /// + Task GetAccessTokenAsync(bool forceRefresh, CancellationToken ct = default); } diff --git a/src/ClaudeDo.Worker/Online/JwtClaims.cs b/src/ClaudeDo.Worker/Online/JwtClaims.cs new file mode 100644 index 0000000..ab9f8ef --- /dev/null +++ b/src/ClaudeDo.Worker/Online/JwtClaims.cs @@ -0,0 +1,49 @@ +using System.Text.Json; + +namespace ClaudeDo.Worker.Online; + +/// +/// Minimal, dependency-free reader for a JWT access token's payload claims. Used to resolve the +/// current user's Zitadel subject (sub) so sync payloads can be stamped with an owner. +/// Never throws — returns null when the token is absent or cannot be parsed. +/// +public static class JwtClaims +{ + /// + /// Returns the sub claim of the JWT, or null if the token is absent/unparseable or + /// carries no subject. + /// + public static string? GetSubject(string? jwt) + { + if (string.IsNullOrWhiteSpace(jwt)) + return null; + + var parts = jwt.Split('.'); + if (parts.Length < 2) + return null; + + try + { + using var doc = JsonDocument.Parse(Base64UrlDecode(parts[1])); + if (doc.RootElement.TryGetProperty("sub", out var sub) && + sub.ValueKind == JsonValueKind.String) + return sub.GetString(); + return null; + } + catch + { + return null; + } + } + + private static byte[] Base64UrlDecode(string input) + { + var s = input.Replace('-', '+').Replace('_', '/'); + switch (s.Length % 4) + { + case 2: s += "=="; break; + case 3: s += "="; break; + } + return Convert.FromBase64String(s); + } +} diff --git a/src/ClaudeDo.Worker/Online/OnlineInboxApiClient.cs b/src/ClaudeDo.Worker/Online/OnlineInboxApiClient.cs index 1444288..cb62681 100644 --- a/src/ClaudeDo.Worker/Online/OnlineInboxApiClient.cs +++ b/src/ClaudeDo.Worker/Online/OnlineInboxApiClient.cs @@ -1,3 +1,5 @@ +using System.Net; +using System.Net.Http.Headers; using System.Net.Http.Json; using ClaudeDo.Worker.Online.Interfaces; @@ -5,6 +7,10 @@ namespace ClaudeDo.Worker.Online; public sealed class OnlineInboxApiClient : IOnlineInboxApi { + internal const string MissingRoleMessage = + "Account has no access (missing 'user' role in Zitadel). " + + "Grant the 'user' role for this account in the ClaudeDo project, then sign in again."; + private readonly HttpClient _http; private readonly IOnlineAuthProvider _auth; @@ -31,54 +37,68 @@ public sealed class OnlineInboxApiClient : IOnlineInboxApi public async Task PutListsAsync(IReadOnlyList lists, CancellationToken ct = default) { - using var req = await BuildAsync(HttpMethod.Put, "lists", lists, ct); - using var resp = await _http.SendAsync(req, ct); - await EnsureSuccessAsync(resp, ct); + using var resp = await SendAsync(HttpMethod.Put, "lists", lists, ct); } public async Task> GetUnimportedTasksAsync(CancellationToken ct = default) { - using var req = await BuildAsync(HttpMethod.Get, "tasks?imported=false", null, ct); - using var resp = await _http.SendAsync(req, ct); - await EnsureSuccessAsync(resp, ct); + using var resp = await SendAsync(HttpMethod.Get, "tasks?imported=false", null, ct); var result = await resp.Content.ReadFromJsonAsync>(ct); return result ?? []; } public async Task MarkImportedAsync(string id, CancellationToken ct = default) { - using var req = await BuildAsync(HttpMethod.Post, $"tasks/{Uri.EscapeDataString(id)}/imported", null, ct); - using var resp = await _http.SendAsync(req, ct); - await EnsureSuccessAsync(resp, ct); + using var resp = await SendAsync(HttpMethod.Post, $"tasks/{Uri.EscapeDataString(id)}/imported", null, ct); } public async Task PutMirrorAsync(IReadOnlyList tasks, CancellationToken ct = default) { - using var req = await BuildAsync(HttpMethod.Put, "tasks/mirror", tasks, ct); - using var resp = await _http.SendAsync(req, ct); - await EnsureSuccessAsync(resp, ct); + using var resp = await SendAsync(HttpMethod.Put, "tasks/mirror", tasks, ct); } - private async Task BuildAsync( - HttpMethod method, - string path, - object? body, - CancellationToken ct) + /// + /// Sends an authenticated request. On a 401, forces a fresh (role-bearing) token via the + /// refresh-token grant and retries once; if a fresh token is still rejected, throws an + /// with . + /// + private async Task SendAsync( + HttpMethod method, string path, object? body, CancellationToken ct) { - var token = await _auth.GetAccessTokenAsync(ct); - var req = new HttpRequestMessage(method, path); + var resp = await SendOnceAsync(method, path, body, forceRefresh: false, ct); + + if (resp.StatusCode == HttpStatusCode.Unauthorized) + { + resp.Dispose(); + resp = await SendOnceAsync(method, path, body, forceRefresh: true, ct); + } + + if (resp.StatusCode == HttpStatusCode.Unauthorized) + { + resp.Dispose(); + throw new OnlineInboxException(401, MissingRoleMessage); + } + + if (!resp.IsSuccessStatusCode) + { + var status = (int)resp.StatusCode; + var errBody = await resp.Content.ReadAsStringAsync(ct); + resp.Dispose(); + throw new OnlineInboxException(status, $"Online Inbox API error {status}: {errBody}"); + } + + return resp; + } + + private async Task SendOnceAsync( + HttpMethod method, string path, object? body, bool forceRefresh, CancellationToken ct) + { + var token = await _auth.GetAccessTokenAsync(forceRefresh, ct); + using var req = new HttpRequestMessage(method, path); if (token is not null) - req.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token); + req.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); if (body is not null) req.Content = JsonContent.Create(body); - return req; - } - - private static async Task EnsureSuccessAsync(HttpResponseMessage resp, CancellationToken ct) - { - if (resp.IsSuccessStatusCode) return; - var body = await resp.Content.ReadAsStringAsync(ct); - throw new OnlineInboxException((int)resp.StatusCode, - $"Online Inbox API error {(int)resp.StatusCode}: {body}"); + return await _http.SendAsync(req, ct); } } diff --git a/src/ClaudeDo.Worker/Online/OnlineSyncService.cs b/src/ClaudeDo.Worker/Online/OnlineSyncService.cs index 92ac8e6..091765a 100644 --- a/src/ClaudeDo.Worker/Online/OnlineSyncService.cs +++ b/src/ClaudeDo.Worker/Online/OnlineSyncService.cs @@ -42,6 +42,12 @@ public sealed class OnlineSyncService : BackgroundService { return; } + catch (OnlineInboxException ex) when (ex.StatusCode == 401) + { + _logger.LogError( + "OnlineSyncService: {Message} Sync is paused until you sign in again with an authorized account.", + ex.Message); + } catch (Exception ex) { _logger.LogWarning(ex, "OnlineSyncService cycle failed; backing off to next interval"); @@ -67,6 +73,10 @@ public sealed class OnlineSyncService : BackgroundService return; } + // Resolve the current user's Zitadel subject so sync payloads carry an owner and pulls + // can be guarded. Null today (single user / server derives it from the token). + var ownerId = JwtClaims.GetSubject(token); + await using var ctx = await _dbFactory.CreateDbContextAsync(ct); var tasks = new TaskRepository(ctx); var lists = new ListRepository(ctx); @@ -75,6 +85,15 @@ public sealed class OnlineSyncService : BackgroundService var unimported = await _api.GetUnimportedTasksAsync(ct); foreach (var remote in unimported) { + // Multi-user guard: never import a task explicitly owned by a different user. + // Unowned tasks (ownerId == null) stay importable so single-user behavior is intact. + if (ownerId is not null && remote.OwnerId is not null && remote.OwnerId != ownerId) + { + _logger.LogWarning( + "OnlineSyncService: remote task {Id} is owned by another user; skipping", remote.Id); + continue; + } + var existing = await tasks.GetByIdAsync(remote.Id, ct); if (existing is not null) { @@ -109,13 +128,15 @@ public sealed class OnlineSyncService : BackgroundService _logger.LogInformation("OnlineSyncService: imported task {Id} ('{Title}')", remote.Id, remote.Title); } - // Step 2: push full list catalog. + // Step 2: push full list catalog, stamped with the owner. var allLists = await lists.GetAllAsync(ct); - var remoteLists = allLists.Select(l => new RemoteList(l.Id, l.Name)).ToList(); + var remoteLists = allLists.Select(l => new RemoteList(l.Id, l.Name, ownerId)).ToList(); await _api.PutListsAsync(remoteLists, ct); - // Step 3: push current Idle backlog mirror. - var mirror = await OnlineBacklog.CurrentAsync(tasks, ct); + // Step 3: push current Idle backlog mirror, stamped with the owner. + var mirror = (await OnlineBacklog.CurrentAsync(tasks, ct)) + .Select(m => m with { OwnerId = ownerId }) + .ToList(); await _api.PutMirrorAsync(mirror, ct); } } diff --git a/src/ClaudeDo.Worker/Online/StaticTokenAuthProvider.cs b/src/ClaudeDo.Worker/Online/StaticTokenAuthProvider.cs index bbb7906..6d41a38 100644 --- a/src/ClaudeDo.Worker/Online/StaticTokenAuthProvider.cs +++ b/src/ClaudeDo.Worker/Online/StaticTokenAuthProvider.cs @@ -18,4 +18,7 @@ public sealed class StaticTokenAuthProvider : IOnlineAuthProvider public Task GetAccessTokenAsync(CancellationToken ct = default) => Task.FromResult(_token); + + public Task GetAccessTokenAsync(bool forceRefresh, CancellationToken ct = default) + => Task.FromResult(_token); } diff --git a/src/ClaudeDo.Worker/Online/ZitadelAuthProvider.cs b/src/ClaudeDo.Worker/Online/ZitadelAuthProvider.cs index f17ec49..3cee47e 100644 --- a/src/ClaudeDo.Worker/Online/ZitadelAuthProvider.cs +++ b/src/ClaudeDo.Worker/Online/ZitadelAuthProvider.cs @@ -20,6 +20,9 @@ public sealed class ZitadelAuthProvider : IOnlineAuthProvider // Cached access token state. private string? _cachedAccessToken; private DateTimeOffset _cacheExpiry; + // The refresh token that minted the cached access token. When the stored refresh token + // changes (sign-out, or signing in as a different user), the cache is no longer valid. + private string? _refreshTokenUsed; // Cached token endpoint URL (discovered once). private string? _tokenEndpoint; @@ -36,22 +39,33 @@ public sealed class ZitadelAuthProvider : IOnlineAuthProvider _logger = logger; } - public async Task GetAccessTokenAsync(CancellationToken ct = default) + public Task GetAccessTokenAsync(CancellationToken ct = default) + => GetAccessTokenAsync(false, ct); + + public async Task GetAccessTokenAsync(bool forceRefresh, CancellationToken ct = default) { - // Fast path: check cache without acquiring the lock. - if (_cachedAccessToken is not null && DateTimeOffset.UtcNow < _cacheExpiry) + var refreshToken = _tokenStore.Read(); + + // Fast path: cached token is valid, not forced, and was minted from the still-current + // refresh token (i.e. the signed-in user hasn't changed). + if (IsCacheUsable(forceRefresh, refreshToken)) return _cachedAccessToken; await _lock.WaitAsync(ct); try { - // Re-check inside the lock (double-checked locking). - if (_cachedAccessToken is not null && DateTimeOffset.UtcNow < _cacheExpiry) + // Re-read + re-check inside the lock (double-checked locking). + refreshToken = _tokenStore.Read(); + if (IsCacheUsable(forceRefresh, refreshToken)) return _cachedAccessToken; - var refreshToken = _tokenStore.Read(); + // Drop any stale access token so a fresh one is minted for the current user. + _cachedAccessToken = null; + _cacheExpiry = default; + if (refreshToken is null) { + _refreshTokenUsed = null; _logger.LogDebug("No refresh token stored; skipping token refresh."); return null; } @@ -64,6 +78,12 @@ public sealed class ZitadelAuthProvider : IOnlineAuthProvider } } + private bool IsCacheUsable(bool forceRefresh, string? storedRefreshToken) => + !forceRefresh + && _cachedAccessToken is not null + && DateTimeOffset.UtcNow < _cacheExpiry + && storedRefreshToken == _refreshTokenUsed; + private async Task RefreshAsync(string refreshToken, CancellationToken ct) { var tokenEndpoint = await GetTokenEndpointAsync(ct); @@ -113,15 +133,19 @@ public sealed class ZitadelAuthProvider : IOnlineAuthProvider } // If Zitadel rotated the refresh token, persist the new one. + var persistedRefreshToken = refreshToken; if (tokenResponse.RefreshToken is not null && tokenResponse.RefreshToken != refreshToken) { _logger.LogDebug("Refresh token rotated; persisting new token."); _tokenStore.Save(tokenResponse.RefreshToken); + persistedRefreshToken = tokenResponse.RefreshToken; } // Cache the access token (subtract 60 s safety margin; minimum 0 to avoid far-future expiry on zero). + // Remember which refresh token it was minted from so the cache invalidates on a user switch. _cachedAccessToken = tokenResponse.AccessToken; _cacheExpiry = DateTimeOffset.UtcNow.AddSeconds(tokenResponse.ExpiresIn - 60); + _refreshTokenUsed = persistedRefreshToken; return _cachedAccessToken; } diff --git a/tests/ClaudeDo.Ui.Tests/Services/ZitadelTokenInspectorTests.cs b/tests/ClaudeDo.Ui.Tests/Services/ZitadelTokenInspectorTests.cs new file mode 100644 index 0000000..c48e54d --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/Services/ZitadelTokenInspectorTests.cs @@ -0,0 +1,62 @@ +using System; +using System.Text; +using ClaudeDo.Ui.Services; +using Xunit; + +namespace ClaudeDo.Ui.Tests.Services; + +public class ZitadelTokenInspectorTests +{ + // Builds a fake JWT (header.payload.signature) carrying the given JSON payload. + private static string MakeToken(string payloadJson) + { + static string B64Url(string s) + { + var bytes = Encoding.UTF8.GetBytes(s); + return Convert.ToBase64String(bytes).TrimEnd('=').Replace('+', '-').Replace('/', '_'); + } + return $"{B64Url("{\"alg\":\"RS256\"}")}.{B64Url(payloadJson)}.sig"; + } + + [Fact] + public void HasUserRole_True_ForGenericRolesClaim() + { + var token = MakeToken( + "{\"urn:zitadel:iam:org:project:roles\":{\"user\":{\"org1\":\"example.com\"}}}"); + Assert.True(ZitadelTokenInspector.HasUserRole(token)); + } + + [Fact] + public void HasUserRole_True_ForProjectScopedRolesClaim() + { + var token = MakeToken( + "{\"urn:zitadel:iam:org:project:376787351902355727:roles\":{\"user\":{\"org1\":\"example.com\"}}}"); + Assert.True(ZitadelTokenInspector.HasUserRole(token)); + } + + [Fact] + public void HasUserRole_False_WhenRoleMissing() + { + var token = MakeToken( + "{\"urn:zitadel:iam:org:project:roles\":{\"admin\":{\"org1\":\"example.com\"}}}"); + Assert.False(ZitadelTokenInspector.HasUserRole(token)); + } + + [Fact] + public void HasUserRole_False_WhenNoRolesClaim() + { + var token = MakeToken("{\"sub\":\"123\",\"email\":\"a@b.c\"}"); + Assert.False(ZitadelTokenInspector.HasUserRole(token)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not-a-jwt")] + [InlineData("only.two")] + public void HasUserRole_FailsOpen_ForUnparseableInput(string? token) + { + // Cannot decide -> fail open (server remains the source of truth). + Assert.True(ZitadelTokenInspector.HasUserRole(token)); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Online/JwtClaimsTests.cs b/tests/ClaudeDo.Worker.Tests/Online/JwtClaimsTests.cs new file mode 100644 index 0000000..d769b76 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Online/JwtClaimsTests.cs @@ -0,0 +1,40 @@ +using System.Text; +using ClaudeDo.Worker.Online; + +namespace ClaudeDo.Worker.Tests.Online; + +public sealed class JwtClaimsTests +{ + // Builds a fake JWT (header.payload.signature) carrying the given JSON payload. + internal static string MakeToken(string payloadJson) + { + static string B64Url(string s) => + Convert.ToBase64String(Encoding.UTF8.GetBytes(s)) + .TrimEnd('=').Replace('+', '-').Replace('/', '_'); + return $"{B64Url("{\"alg\":\"RS256\"}")}.{B64Url(payloadJson)}.sig"; + } + + [Fact] + public void GetSubject_ReturnsSub() + { + var token = MakeToken("{\"sub\":\"user-123\",\"email\":\"a@b.c\"}"); + Assert.Equal("user-123", JwtClaims.GetSubject(token)); + } + + [Fact] + public void GetSubject_Null_WhenNoSub() + { + var token = MakeToken("{\"email\":\"a@b.c\"}"); + Assert.Null(JwtClaims.GetSubject(token)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not-a-jwt")] + [InlineData("opaque.token")] + public void GetSubject_Null_ForUnparseableInput(string? token) + { + Assert.Null(JwtClaims.GetSubject(token)); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Online/OnlineInboxApiClientTests.cs b/tests/ClaudeDo.Worker.Tests/Online/OnlineInboxApiClientTests.cs index b53cb05..b6efbb6 100644 --- a/tests/ClaudeDo.Worker.Tests/Online/OnlineInboxApiClientTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Online/OnlineInboxApiClientTests.cs @@ -130,6 +130,52 @@ public sealed class OnlineInboxApiClientTests Assert.Equal(401, ex.StatusCode); } + [Fact] + public async Task Unauthorized_RetriesOnceWithForcedRefresh_ThenThrowsMissingRole() + { + var (client, handler) = Build(); + handler.ResponseStatus = HttpStatusCode.Unauthorized; + handler.ResponseBody = "Unauthorized"; + + var ex = await Assert.ThrowsAsync( + () => client.PutListsAsync([])); + + Assert.Equal(401, ex.StatusCode); + Assert.Equal(OnlineInboxApiClient.MissingRoleMessage, ex.Message); + // Original attempt + one forced-refresh retry. + Assert.Equal(2, handler.Requests.Count); + } + + [Fact] + public async Task Unauthorized_ThenSuccessOnRetry_Succeeds() + { + var handler = new SequenceHandler(HttpStatusCode.Unauthorized, HttpStatusCode.OK); + var http = new HttpClient(handler) { BaseAddress = new Uri("https://inbox.example.com/api/") }; + var client = new OnlineInboxApiClient(http, new StaticTokenAuthProvider("test-token")); + + await client.PutListsAsync([]); + + Assert.Equal(2, handler.Requests.Count); + } + + private sealed class SequenceHandler : HttpMessageHandler + { + private readonly Queue _statuses; + public List Requests { get; } = new(); + + public SequenceHandler(params HttpStatusCode[] statuses) => _statuses = new(statuses); + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken ct) + { + Requests.Add(request); + var status = _statuses.Count > 0 ? _statuses.Dequeue() : HttpStatusCode.OK; + return Task.FromResult(new HttpResponseMessage(status) + { + Content = new StringContent("[]", Encoding.UTF8, "application/json"), + }); + } + } + [Fact] public async Task ServerError_Throws_OnlineInboxException_WithStatusCode() { diff --git a/tests/ClaudeDo.Worker.Tests/Online/OnlineSyncServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Online/OnlineSyncServiceTests.cs index a5c2535..15288d6 100644 --- a/tests/ClaudeDo.Worker.Tests/Online/OnlineSyncServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Online/OnlineSyncServiceTests.cs @@ -209,6 +209,76 @@ public sealed class OnlineSyncServiceTests : IDisposable Assert.Equal(0, api.CallCount); } + // ---- multi-user: owner stamping + guard ---- + + [Fact] + public async Task Tick_StampsOwnerId_OnPushedListsAndMirror() + { + var (listId, ctx, tasks, _) = await SeedAsync(); + using var _ = ctx; + await tasks.AddAsync(new TaskEntity + { + Id = Guid.NewGuid().ToString(), ListId = listId, Title = "Idle", + Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow, + }); + + var token = JwtClaimsTests.MakeToken("{\"sub\":\"owner-1\"}"); + var api = new FakeApi(); + var svc = BuildService(api, token); + + await svc.TickAsync(CancellationToken.None); + + Assert.All(api.ReceivedLists, l => Assert.Equal("owner-1", l.OwnerId)); + Assert.NotEmpty(api.ReceivedMirror); + Assert.All(api.ReceivedMirror, m => Assert.Equal("owner-1", m.OwnerId)); + } + + [Fact] + public async Task Tick_SkipsRemoteTask_OwnedByAnotherUser() + { + var (listId, ctx, tasks, _) = await SeedAsync(); + using var _ = ctx; + + var foreignId = Guid.NewGuid().ToString(); + var api = new FakeApi + { + UnimportedTasks = + [ + new RemoteTask(foreignId, listId, "Theirs", null, DateTimeOffset.UtcNow, OwnerId: "owner-2"), + ], + }; + var svc = BuildService(api, JwtClaimsTests.MakeToken("{\"sub\":\"owner-1\"}")); + + await svc.TickAsync(CancellationToken.None); + + Assert.Null(await tasks.GetByIdAsync(foreignId)); + Assert.DoesNotContain(foreignId, api.MarkedImported); + } + + [Fact] + public async Task Tick_Imports_OwnTask_And_UnownedTask() + { + var (listId, ctx, tasks, _) = await SeedAsync(); + using var _ = ctx; + + var mine = Guid.NewGuid().ToString(); + var unowned = Guid.NewGuid().ToString(); + var api = new FakeApi + { + UnimportedTasks = + [ + new RemoteTask(mine, listId, "Mine", null, DateTimeOffset.UtcNow, OwnerId: "owner-1"), + new RemoteTask(unowned, listId, "Unowned", null, DateTimeOffset.UtcNow), + ], + }; + var svc = BuildService(api, JwtClaimsTests.MakeToken("{\"sub\":\"owner-1\"}")); + + await svc.TickAsync(CancellationToken.None); + + Assert.NotNull(await tasks.GetByIdAsync(mine)); + Assert.NotNull(await tasks.GetByIdAsync(unowned)); + } + // ---- already-imported task on server ---- [Fact] diff --git a/tests/ClaudeDo.Worker.Tests/Online/ZitadelAuthProviderTests.cs b/tests/ClaudeDo.Worker.Tests/Online/ZitadelAuthProviderTests.cs index 6f8987c..c8c502d 100644 --- a/tests/ClaudeDo.Worker.Tests/Online/ZitadelAuthProviderTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Online/ZitadelAuthProviderTests.cs @@ -152,6 +152,37 @@ public sealed class ZitadelAuthProviderTests : IDisposable Assert.Equal(2, handler.Requests.Count); } + [Fact] + public async Task ChangedRefreshToken_InvalidatesCache_AndRefreshesForNewUser() + { + if (!OperatingSystem.IsWindows()) return; + + var (provider, handler, store) = Build(); + store.Save("admin-refresh"); + + // First user (admin): discovery + token. + handler.Enqueue(".well-known", HttpStatusCode.OK, + DiscoveryJson("https://auth.example.com/oauth/token")); + handler.Enqueue("oauth/token", HttpStatusCode.OK, + TokenJson("admin-access", expiresIn: 3600)); + + var adminToken = await provider.GetAccessTokenAsync(); + Assert.Equal("admin-access", adminToken); + + // Re-sign-in as a different user writes a new refresh token to the store. + store.Save("normal-refresh"); + + // Even though the cached admin token is still within its expiry window, the changed + // refresh token must force a new exchange (no second discovery — it's cached). + handler.Enqueue("oauth/token", HttpStatusCode.OK, + TokenJson("normal-access", expiresIn: 3600)); + + var normalToken = await provider.GetAccessTokenAsync(); + + Assert.Equal("normal-access", normalToken); + Assert.Equal(3, handler.Requests.Count); // discovery + admin token + normal token + } + [Fact] public async Task RotatedRefreshToken_IsPersisted() {