Skip to content

fix(auth): device-flow poll audits routine polling as denied, a real user decline as error #8378

Description

@JSONbored

Context

pollGitHubDeviceFlow in src/auth/github-oauth.ts audits every non-success response from GitHub's device-flow token endpoint:

if ("error" in tokenPayload) {
  await recordAuditEvent(env, {
    eventType: "auth.github_device_poll",
    outcome: tokenPayload.error === "authorization_pending" || tokenPayload.error === "slow_down" ? "denied" : "error",
    detail: tokenPayload.error,
  });
  return {
    status: tokenPayload.error,
    message: tokenPayload.error_description,
  };
}

Per RFC 8628 (the OAuth Device Authorization Grant this flow implements) and this codebase's own startGitHubDeviceFlow/pollGitHubDeviceFlow comments, authorization_pending and slow_down are not denials — they are the expected, routine response on nearly every poll of a login that is still in progress (the CLI/browser client polls every ~5s for up to the device code's 900s/15-minute expiry window, per routeClassForPath's own comment on /v1/auth/github/device/poll). A single successful device-flow login can legitimately poll dozens of times before the user finishes the browser step, so this line currently writes a denied audit_events row on every one of those routine polls.

Meanwhile, GitHub's actual signal for "the user explicitly declined the authorization" — error: "access_denied" — is not authorization_pending or slow_down, so it falls into the : "error" branch instead, identical to a genuine failure like expired_token, bad_verification_code, or incorrect_client_credentials (the last three are already exercised this way in test/unit/auth.test.ts's pollGitHubDeviceFlow coverage).

This is backwards relative to how outcome: "denied" is used everywhere else in this codebase — it consistently means "an action was actively rejected," never "still in progress, try again":

  • src/api/routes.ts's GET /v1/auth/github/callback (the sibling web OAuth flow, same file as this device flow) treats GitHub's ?error= query param — which for the web flow includes the user's real decline — as outcome: "denied".
  • src/auth/rate-limit.ts's enforceRateLimit uses outcome: "denied" for an actual rate-limit rejection.
  • Every outcome: "denied" call site in src/queue/processors.ts (command-authorization denials, review-command denials, etc.) represents an actual access/action rejection, never a "keep waiting" state.

Net effect: an operator searching audit_events for auth.github_device_poll rows with outcome = 'denied' to find real declined logins would instead find a flood of routine, successful-login-in-progress noise (up to ~180 rows for one successful login at the default 5s interval), while the one row that actually represents a user declining the login (access_denied) is filed under the generic "error" outcome alongside unrelated failures like an expired/malformed device code.

This is purely an audit-log categorization fix — it does not change what token is issued, what scopes are granted, or which callers can authenticate. pollGitHubDeviceFlow's returned { status, message } shape (consumed by the /v1/auth/github/device/poll route and the polling client) is unchanged.

Requirements

  • authorization_pending and slow_down are routine, expected, non-terminal polling states, not audit-worthy events — do not call recordAuditEvent for them at all (mirrors how RFC 8628 treats them: they carry no new information beyond "keep polling").
  • Every other GitHub device-token-exchange error remains audited via the existing auth.github_device_poll event type, with detail: tokenPayload.error unchanged.
  • access_denied specifically must be recorded with outcome: "denied" (it is the one genuine user-rejection signal in this flow), matching how the sibling web-OAuth callback route treats an OAuth error query parameter.
  • Every other terminal error code (expired_token, bad_verification_code, incorrect_client_credentials, unsupported_grant_type, or any other value GitHub might return) keeps outcome: "error", unchanged from today.
  • The function's return value for every case ({ status: tokenPayload.error, message: tokenPayload.error_description }) must not change — this is an audit-outcome-only fix.

Deliverables

  • pollGitHubDeviceFlow in src/auth/github-oauth.ts no longer calls recordAuditEvent for authorization_pending or slow_down.
  • pollGitHubDeviceFlow records outcome: "denied" specifically for access_denied, and outcome: "error" for every other terminal error code.

Test Coverage Requirements

src/auth/github-oauth.ts is in src/** and is measured by Codecov at 99% patch coverage, branch-counted — the changed conditional needs explicit coverage of every arm, not just the pre-existing bad_verification_code"error" case already in test/unit/auth.test.ts. Add/extend tests (likely in test/unit/auth.test.ts, alongside the existing "polls GitHub device flow and creates a session only after authorization" test):

  • authorization_pending and slow_down responses produce no new audit_events row for eventType = 'auth.github_device_poll' (query the count before/after, or assert no row matches, rather than asserting an absence of any specific outcome value).
  • access_denied produces exactly one auth.github_device_poll row with outcome = 'denied' and detail = 'access_denied'.
  • A genuine terminal error (e.g. bad_verification_code, already covered) still produces outcome = 'error' — this is a regression guard so the existing behavior for real errors doesn't get accidentally folded into the new denied branch.
  • The returned { status, message } shape is unchanged for all of the above (extend the existing toMatchObject assertions rather than replace them).

Expected Outcome

audit_events rows for auth.github_device_poll only exist for outcomes that actually happened (a real denial or a real error), not for the routine "still waiting for the user" polling cadence — an operator reviewing outcome = 'denied' rows for this event type sees genuine user declines, not login-in-progress noise, and a genuine decline is no longer indistinguishable from an unrelated device-code error.

Links & Resources

  • src/auth/github-oauth.tspollGitHubDeviceFlow (the function to change) and startGitHubDeviceFlow (unchanged, for context on the flow's polling cadence).
  • src/api/routes.tsGET /v1/auth/github/callback's existing outcome: "denied" handling of GitHub's ?error= query parameter (the sibling convention this brings the device flow in line with).
  • test/unit/auth.test.ts — existing pollGitHubDeviceFlow coverage (authorization_pending, slow_down, bad_verification_code cases) to extend.
  • src/auth/rate-limit.ts's routeClassForPath comment on /v1/auth/github/device/poll's polling cadence (RFC 8628, ~5s interval, up to 900s expiry) — background on why routine polls can be numerous.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions