Skip to content
Merged
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
3 changes: 2 additions & 1 deletion src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,8 @@ async Task<AuthResult> DiscoverWorkOSAsync(
ct: ct, progress: progress, keys: KeyWatcher),
orgSwitch: (refreshToken, organizationId) =>
workos.SwitchOrganizationAsync(clientId, refreshToken, organizationId, ct),
orglessRefresh: (refreshToken, refreshCt) => workos.RefreshAsync(clientId, refreshToken, refreshCt),
orglessRefresh: async (refreshToken, refreshCt) =>
(await workos.RefreshAsync(clientId, refreshToken, refreshCt)).Response,
provisioner: provisioner,
ct: ct,
progress: progress,
Expand Down
40 changes: 31 additions & 9 deletions src/Capacitor.Cli.Core/Auth/TokenStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,15 @@ public sealed record TokenResolution(
// Outcome of a proactive-refresh tick (<see cref="TokenStore.RefreshIfExpiringAsync"/>).
// NotDue = no-op (no tokens, the None provider, or the token still comfortably valid);
// Refreshed = a valid token is now persisted (we refreshed, a peer did, or it was already
// fresh under the lock); Failed = a refresh was attempted but failed (network / 4xx) and the
// token is unchanged; Contended = we couldn't acquire the cross-process lock before its
// fresh under the lock); Failed = a refresh was attempted but the endpoint never answered
// (transport failure) — the refresh credential may still be live, so a later retry can succeed;
// Rejected = WorkOS refused the refresh token (a non-success response, a 400 invalid_grant the
// usual case — the token is consumed or revoked), which only a fresh `kcap login` repairs, so
// re-sending it is pointless; Contended = we couldn't acquire the cross-process lock before its
// deadline (a peer holds it, presumably refreshing) — no endpoint call was made, so it is NOT a
// failure. The daemon loop rate-limits on Refreshed and Failed (to bound endpoint traffic) but
// failure. The daemon loop rate-limits on Refreshed and Failed, backs off hard on Rejected, and
// treats Contended quietly — no warning, no backoff.
public enum ProactiveRefreshOutcome { NotDue, Refreshed, Failed, Contended }
public enum ProactiveRefreshOutcome { NotDue, Refreshed, Failed, Contended, Rejected }

public sealed class TokenStore(ConfigRoot config, ProfileOverrides env, IHttpClientFactory httpFactory, WorkOSClient workos) {
string LegacyTokenPath { get; } = config.Path("tokens.json");
Expand Down Expand Up @@ -483,8 +486,14 @@ public async Task<ProactiveRefreshOutcome> RefreshIfExpiringAsync(string profile
// between our read here and our acquiring the lock, leaving the re-read token fresh.
bool ExpiringWithinWindow(StoredTokens t) => DateTimeOffset.UtcNow >= t.ExpiresAt - window;

// Capture the raw WorkOS classification the same way `contended` captures lock contention:
// the delegate records it, the code after the lock reads it. Only set when the WorkOS
// refresh delegate actually runs (a peer-refresh or still-fresh re-read returns non-null
// without calling it), so a null result plus a Rejected outcome is a genuine refusal.
WorkOSRefreshOutcome? workosOutcome = null;

var refresh = decision == RefreshDecision.RefreshWorkOS
? (Func<StoredTokens, Task<StoredTokens?>>)RefreshWorkOSAsync
? (Func<StoredTokens, Task<StoredTokens?>>)(t => RefreshWorkOSAsync(t, CancellationToken.None, o => workosOutcome = o))
: t => RefreshGitHubAsync(profile, t, CancellationToken.None);

var contended = false;
Expand All @@ -495,8 +504,13 @@ public async Task<ProactiveRefreshOutcome> RefreshIfExpiringAsync(string profile
// don't let the daemon warn/back off as though the endpoint rejected us. Otherwise:
// non-null = a valid token is now persisted; null = the refresh call actually failed.
if (contended) return ProactiveRefreshOutcome.Contended;
if (result is not null) return ProactiveRefreshOutcome.Refreshed;

return result is not null ? ProactiveRefreshOutcome.Refreshed : ProactiveRefreshOutcome.Failed;
// A refused WorkOS refresh token is terminal — re-sending it is pointless, so the daemon
// backs off hard on Rejected rather than the ordinary Failed retry cadence.
return workosOutcome is WorkOSRefreshOutcome.Rejected
? ProactiveRefreshOutcome.Rejected
: ProactiveRefreshOutcome.Failed;
}

// Profile-scoped cross-process lock. Acquire it, re-read the token under it (a peer
Expand Down Expand Up @@ -682,13 +696,21 @@ public static DateTimeOffset JwtExpiry(string accessToken) {
Task<StoredTokens?> RefreshWorkOSAsync(StoredTokens tokens) =>
RefreshWorkOSAsync(tokens, CancellationToken.None);

async Task<StoredTokens?> RefreshWorkOSAsync(StoredTokens tokens, CancellationToken ct) {
var json = await workos.RefreshAsync(tokens.ClientId!, tokens.RefreshToken!, ct);
// `onOutcome` reports the raw WorkOS classification so a caller (the proactive tick) can tell a
// refused refresh token — terminal, `kcap login` repairs it — from a transport failure it may
// retry. A non-Rotated outcome maps to null; only the proactive tick reads onOutcome.
async Task<StoredTokens?> RefreshWorkOSAsync(
StoredTokens tokens, CancellationToken ct, Action<WorkOSRefreshOutcome>? onOutcome = null) {
var result = await workos.RefreshAsync(tokens.ClientId!, tokens.RefreshToken!, ct);

onOutcome?.Invoke(result.Outcome);

if (json is null) {
if (result.Outcome is not WorkOSRefreshOutcome.Rotated) {
return null;
}

var json = result.Response!;

// Persistence is the caller's responsibility (RefreshWithCrossProcessLockAsync saves under the
// locked profile) — see RefreshGitHubAsync. WorkOS rotates the refresh token on use, so writing
// under the wrong profile would be especially damaging.
Expand Down
77 changes: 57 additions & 20 deletions src/Capacitor.Cli.Core/Auth/WorkOSClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,80 @@ namespace Capacitor.Cli.Core.Auth;
/// authenticate exits quietly rather than stack-tracing into a transcript. The sign-in legs do not,
/// because a sign-in is interactive and a transport failure there is worth saying out loud.</para>
/// </summary>
public sealed class WorkOSClient(IHttpClientFactory httpFactory) {
public sealed class WorkOSClient(IHttpClientFactory httpFactory, TimeSpan? refreshTimeout = null) {
/// <summary>AuthKit's API host. The machine mint posts elsewhere — see <see cref="MachineAuth.DefaultTokenUrl"/>.</summary>
public const string ApiBase = "https://api.workos.com";

// Short, so a hook never blocks for the default budget when WorkOS is unreachable.
static readonly TimeSpan RetryBudget = TimeSpan.FromSeconds(5);
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved
// A hard deadline on the single refresh attempt: the shared HttpClient carries the 100 s default,
// and a refresh runs under the cross-process lock, so a stalled WorkOS would otherwise hold auth
// and every peer's refresh for that long. Short — a hook must not block on an unreachable WorkOS.
readonly TimeSpan _refreshTimeout = refreshTimeout ?? TimeSpan.FromSeconds(5);

/// <summary>
/// Exchanges a rotating refresh token for a fresh access token, or null when WorkOS refused it or
/// answered unreadably — the repair is a fresh login either way.
/// Exchanges a rotating refresh token for a fresh access token, classifying the outcome so a caller
/// can tell a token WorkOS refused apart from a request that never landed.
///
/// <para>The retry covers transport failures only, never a non-success response. WorkOS rotates the
/// refresh token on every successful use, so a retry after a response was lost in transit does
/// re-send a token WorkOS already consumed — but that window exists without the retry too, since
/// the next refresh re-reads the same unrotated token from disk. What it buys is the common case:
/// a request that never arrived.</para>
/// <para>Sent once — never retried. WorkOS consumes the refresh token on every use, so re-sending
/// one after a lost response presents a token it has already spent, which trips reuse detection and
/// revokes the whole family. A <see cref="WorkOSRefreshOutcome.Rejected"/> is any non-success status
/// (an <c>invalid_grant</c> among them); <see cref="WorkOSRefreshOutcome.TransportFailed"/> is an
/// exception or an unreadable body, where the token was not spent. Reports rather than throws,
/// cancellation aside.</para>
/// </summary>
public async Task<WorkOSAuthResponse?> RefreshAsync(
public async Task<WorkOSRefreshResult> RefreshAsync(
string clientId, string refreshToken, CancellationToken ct) {
// The deadline cancels only the linked token, not the caller's ct, so a timeout is caught below
// as a TransportFailed (the token was not spent — retry is safe) while a real caller cancel
// still propagates.
using var deadline = CancellationTokenSource.CreateLinkedTokenSource(ct);
deadline.CancelAfter(_refreshTimeout);

HttpResponseMessage response;
try {
using var http = httpFactory.CreateClient(CapacitorClients.WorkOS);
using var form = new FormUrlEncodedContent(new Dictionary<string, string> {
response = await PostFormAsync(AuthenticateUrl, new() {
["grant_type"] = "refresh_token",
["client_id"] = clientId,
["refresh_token"] = refreshToken
});
}, deadline.Token);
} catch when (!ct.IsCancellationRequested) {
// No response headers arrived — the request may never have reached WorkOS, so the token is
// most likely still live and a later attempt re-reads the same on-disk one. The residual
// ambiguity (a reply lost after WorkOS processed it) needs a durable "attempted" marker to
// close fully — a deliberate follow-up, since marking every transient blip terminal would
// force a re-login far more often than a lost reply actually occurs.
return new(WorkOSRefreshOutcome.TransportFailed, null);
}

using (response) {
if (!response.IsSuccessStatusCode) {
// A 4xx means WorkOS understood and refused the refresh token (a 400 invalid_grant on
// a consumed/revoked token is the usual one) — terminal, only `kcap login` repairs it.
// A 5xx/408/429 is the server faltering, not the token being bad, so it reads as a
// transport failure the caller may retry with the same still-live token.
return IsTransientStatus((int)response.StatusCode)
? new(WorkOSRefreshOutcome.TransportFailed, null)
: new(WorkOSRefreshOutcome.Rejected, null);
}

using var response = await http.PostWithRetryAsync(AuthenticateUrl, form, RetryBudget, ct);
// A success status means WorkOS has consumed the old token and rotated. If the new one is
// unreadable it is lost and the old token is spent — Rejected (re-login), never a
// retryable failure that would re-send the consumed token.
WorkOSAuthResponse? body;
try {
body = await response.Content.ReadFromJsonAsync(
CapacitorJsonContext.Default.WorkOSAuthResponse, deadline.Token);
} catch when (!ct.IsCancellationRequested) {
return new(WorkOSRefreshOutcome.Rejected, null);
}

return response.IsSuccessStatusCode
? await response.Content.ReadFromJsonAsync(CapacitorJsonContext.Default.WorkOSAuthResponse, ct)
: null;
} catch {
return null;
return body is null
? new(WorkOSRefreshOutcome.Rejected, null)
: new(WorkOSRefreshOutcome.Rotated, body);
}
}

static bool IsTransientStatus(int status) => status >= 500 || status is 408 or 429;

/// <summary>
/// Opens a device grant. It takes no organization: the human picks one at the AuthKit screen, so
/// a caller that needs a particular one corrects it afterwards with
Expand Down
15 changes: 15 additions & 0 deletions src/Capacitor.Cli.Core/Auth/WorkOSRefreshResult.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
namespace Capacitor.Cli.Core.Auth;

/// <summary>
/// How a WorkOS refresh resolved. WorkOS rotates the refresh token single-use, so the outcome must
/// separate a token it <see cref="Rejected"/> — consumed or refused, repair is a fresh login — from a
/// <see cref="TransportFailed"/> that never reached it, where the same token is still live to try again.
/// </summary>
public enum WorkOSRefreshOutcome {
Rotated,
Rejected,
TransportFailed
}

/// <summary>The result of <see cref="WorkOSClient.RefreshAsync"/>; <see cref="Response"/> is set only when <see cref="WorkOSRefreshOutcome.Rotated"/>.</summary>
public readonly record struct WorkOSRefreshResult(WorkOSRefreshOutcome Outcome, WorkOSAuthResponse? Response);
7 changes: 4 additions & 3 deletions src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -598,9 +598,10 @@ internal partial class AgentOrchestrator : IAsyncDisposable {
// scan) or explicitly rate-limited by TitleResolveLoop itself.
readonly PeriodicTimer _titleResolve = new(TimeSpan.FromSeconds(60));

// Refresh once the token is within this much of its expiry. Comfortably above the 60 s tick
// so the window is never stepped over.
static readonly TimeSpan ProactiveRefreshWindow = TimeSpan.FromMinutes(5);
// Refresh once the token is within this much of its expiry. Kept above the 60 s tick plus the
// reactive 30 s IsExpired margin, so proactive refresh still fires before a hook would hit the
// margin — but no wider, so a token minted seconds ago at login isn't refreshed on the spot.
static readonly TimeSpan ProactiveRefreshWindow = TimeSpan.FromMinutes(2);

// Hit the refresh endpoint at most once per this interval (see TokenRefreshLoop). Small
// enough that a healthy token issued with a short lifetime is still renewed before it
Expand Down
14 changes: 14 additions & 0 deletions src/Capacitor.Cli.Daemon/Services/TokenRefreshLoop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ internal sealed class TokenStoreRefreshPort(TokenStore store, string profile, Ti
/// the moment it enters the window.</para>
/// </summary>
internal sealed class TokenRefreshLoop {
// A rejected refresh means the refresh token itself is dead — WorkOS refused it, and only
// `kcap login` mints a new one. Backing off this far (rather than the ordinary
// minAttemptInterval) stops the loop re-sending a doomed token every few minutes, while still
// picking a re-login up eventually without a daemon restart.
static readonly TimeSpan RejectedBackoff = TimeSpan.FromHours(1);

readonly IProactiveTokenRefreshPort _port;
readonly ILogger _logger;
readonly TimeSpan _minAttemptInterval;
Expand Down Expand Up @@ -96,6 +102,14 @@ public async Task TickAsync(CancellationToken ct) {

break;

case ProactiveRefreshOutcome.Rejected:
_nextAttemptAllowedAt = _utcNow() + RejectedBackoff;
_logger.LogWarning(
"Proactive token refresh was rejected — the stored credential is no longer valid; run `kcap login` to re-authenticate"
);

break;

case ProactiveRefreshOutcome.Contended:
// A peer (hook/watcher/MCP) held the cross-process refresh lock — it is
// presumably refreshing. No endpoint call was made, so this is not a failure:
Expand Down
8 changes: 4 additions & 4 deletions test/Capacitor.Cli.Core.Tests.Unit/Auth/OAuthFlowTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ public async Task RefreshWorkOSToken_posts_org_less_refresh_grant_and_returns_to
"""{"user":{"id":"user_x"},"access_token":"acc","refresh_token":"rt2"}"""));
using var stub = new StubHost(server.Urls[0]);

var auth = await new WorkOSClient(new PlainHttpClientFactory(stub))
.RefreshAsync("client_d", "rt1", CancellationToken.None);
var auth = (await new WorkOSClient(new PlainHttpClientFactory(stub))
.RefreshAsync("client_d", "rt1", CancellationToken.None)).Response;

await Assert.That(auth!.AccessToken).IsEqualTo("acc");
await Assert.That(auth.RefreshToken).IsEqualTo("rt2");
Expand All @@ -165,8 +165,8 @@ public async Task RefreshWorkOSToken_returns_null_on_transport_failure() {

using var stub = new StubHost(url);

var auth = await new WorkOSClient(new PlainHttpClientFactory(stub))
.RefreshAsync("client_d", "rt1", CancellationToken.None);
var auth = (await new WorkOSClient(new PlainHttpClientFactory(stub))
.RefreshAsync("client_d", "rt1", CancellationToken.None)).Response;

await Assert.That(auth).IsNull();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,28 @@ public async Task Token_exactly_at_window_boundary_refreshes() {
await Assert.That(decision).IsEqualTo(TokenStore.RefreshDecision.RefreshWorkOS);
}

[Test]
public async Task Two_minute_window_leaves_a_token_beyond_it_not_due() {
// The daemon's 2-minute window: a token 2m30s out is still comfortably valid, so a
// freshly-issued token is not refreshed the instant after login.
var window = TimeSpan.FromMinutes(2);
var tokens = Tokens(AuthProvider.WorkOS, Now.AddSeconds(150));
var decision = TokenStore.DecideProactiveRefresh(tokens, Now, window);

await Assert.That(decision).IsEqualTo(TokenStore.RefreshDecision.NotDueYet);
}

[Test]
public async Task Two_minute_window_refreshes_a_token_inside_it() {
// 1m30s out is inside the 2-minute window — proactive refresh fires before the reactive
// 30s IsExpired margin, keeping the window above the 60s tick plus that margin.
var window = TimeSpan.FromMinutes(2);
var tokens = Tokens(AuthProvider.WorkOS, Now.AddSeconds(90));
var decision = TokenStore.DecideProactiveRefresh(tokens, Now, window);

await Assert.That(decision).IsEqualTo(TokenStore.RefreshDecision.RefreshWorkOS);
}

[Test]
public async Task Already_expired_workos_token_still_refreshes() {
var tokens = Tokens(AuthProvider.WorkOS, Now.AddMinutes(-10));
Expand Down
Loading