feat(auth): refresh the session's GitHub token near/past its 8h expiry - #6129
Merged
Conversation
Closes #6115 Decision: refresh-token rotation, not disabling token expiration on the Loopover ORB App. GitHub Apps issue an 8h access token + a 6-month refresh_token by default (only omitted if the App owner explicitly opts out); given this App already has Contents/Pull requests/Issues/Checks/ Workflows at Read & write, a non-expiring token widens the blast radius of any future leak for no real implementation-effort savings over honoring GitHub's own default rotation contract. Confirmed via GitHub's docs (exact response shape, 8h/6mo defaults, and that the refresh_token is single-use-then-rotated on every refresh) before committing to this. getLiveSessionGitHubToken (src/auth/github-oauth.ts) resolves a currently- live token: returns the stored access token as-is when it's not near expiry or has no known expiry (a #6114-era row, or an exchange that never returned expires_in -- treated as never-expiring for backward compat), otherwise transparently refreshes via the stored refresh_token and persists the rotated pair. A failed refresh re-checks once before giving up, in case a concurrent request already consumed and rotated the same refresh token (GitHub invalidates both the old access AND refresh token on first use of either). POST /v1/auth/github/token now calls this instead of the bare decrypt, so AMS transparently gets a live token across an arbitrarily long-running session without ever needing a fresh login. auth_session_github_tokens gains expires_at + a second encrypted ciphertext/iv/salt/key_version set for the refresh token, plus its own expiry -- all nullable, since a #6114-era row or a caller-supplied-token exchange has no lifecycle info to offer.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6129 +/- ##
=======================================
Coverage 95.31% 95.32%
=======================================
Files 596 596
Lines 47154 47204 +50
Branches 15047 15069 +22
=======================================
+ Hits 44946 44996 +50
Misses 1477 1477
Partials 731 731
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6115
Decision (this issue's own first deliverable)
Refresh-token rotation, not disabling token expiration on the Loopover ORB App. Confirmed via GitHub's own docs before committing to this:
refresh_tokenissued alongside — this is only omitted if the App owner explicitly opts OUT of token expiration in the App's own settings.POST https://github.com/login/oauth/access_token,grant_type=refresh_token) is single-use-then-rotated: using a refresh token invalidates it and the prior access token, and issues a fresh 6-month refresh token in the response.Summary
getLiveSessionGitHubToken(src/auth/github-oauth.ts) is the new resolver: returns the stored access token as-is when it's not within 15 minutes of expiry, or when there's no known expiry at all (a Persist + expose the GitHub user-to-server token from login instead of discarding it #6114-era row, or an exchange whose response never includedexpires_in— treated as never-expiring for backward compatibility). Otherwise it transparently refreshes via the storedrefresh_tokenand persists the rotated pair.POST /v1/auth/github/tokennow calls this instead of the bare decrypt, so AMS gets a live token transparently across an arbitrarily long-running session without ever needing a fresh login.auth_session_github_tokens(migration 0155) gainsexpires_atplus a second full encrypted-envelope column set (refresh_ciphertext/refresh_iv/refresh_salt/refresh_key_version) andrefresh_expires_at— all nullable, since a Persist + expose the GitHub user-to-server token from login instead of discarding it #6114-era row or the caller-supplied-token exchange path (/v1/auth/github/session) has no lifecycle info to offer.pollGitHubDeviceFlow/completeGitHubWebOAuthnow captureexpires_in/refresh_token/refresh_token_expires_infrom GitHub's response (when present) and thread them throughcreateSessionFromGitHubToken→createSessionForGitHubUser→storeSessionGitHubToken.Test plan
test/unit/auth-github-token.test.ts: end-to-end capture of the lifecycle fields through the device-flow login; fast-path (no refresh call when fresh); refresh-triggers-and-persists-the-rotated-pair; falls back to a stale token when there's nothing to refresh with; refuses to even attempt a refresh when the refresh token is itself expired; treats an absent expiry as never-expiring; the concurrent-refresh race recovery (and its failure-with-no-concurrent-update sibling); a malformed/non-JSON refresh response degrades to failure, not a crash;GITHUB_OAUTH_CLIENT_ID/SECRETunconfigured;getDecryptedSessionGitHubTokenBundle's own no-key / access-decrypt-failure / refresh-decrypt-failure-degrades-independently paths.fetch's own side effect (the only way to genuinely land it between the initial read and the retry read in a single-threaded test).github-oauth.ts,security.ts, andrepositories.tsvia the raw v8 coverage JSON (cross-referenced against the exact diff hunks, not just the aggregate file-level percentage, which understates coverage for these large files when running a narrow test subset).npm run db:migrations:check/db:schema-drift:checkpass.npm run test:cigate green (before and after rebasing ontomainpost-Persist + expose the GitHub user-to-server token from login instead of discarding it #6114-merge).