Repository navigation
Fix auth token renewal: URL-encode requests, update tokens in place, don't start a login (or exit) from the background - #501
Open
leifrossau wants to merge 1 commit into
Open
leifrossau wants to merge 1 commit into
leifrossau wants to merge 1 commit into
Conversation
Auth.Refresh() built its form body by string concatenation, so refresh tokens or redirect URLs containing '&', '+' or '=' were sent broken. Use url.Values for both the refresh and the auth code requests. When renewal failed, Refresh() did 'a = newAuth(...)', which only reassigns the local receiver variable and so never updated the caller's tokens. Worse, newAuth() opens the interactive GTK login window and log.Fatal()s if it is closed or fails, which killed a mounted filesystem running as a background service on any error from the token endpoint (even a 5xx or a hiccup right after resuming from suspend). Now: - tokens are updated in place and saved to disk, - network errors, HTTP 5xx/429 and 'temporarily_unavailable' keep the current tokens and are retried later (at most every 10 seconds), - only a definitive rejection (e.g. invalid_grant) is treated as a lost login. In the background this logs an error pointing to 'onedriver --auth-only' and leaves the tokens alone, the filesystem keeps running, - the interactive login is only started from Authenticate() at startup, - the 401 handling in Request() renews the tokens in place instead of starting a login, unless another request already did so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Problem
Auth.Refreshbuilds its form body by string concatenation, so tokens or redirect URLs containing&,+or=break the request.a = newAuth(...), which only reassigns the local receiver, so callers never see the new tokens.newAuthopens an interactive GTK login window andlog.Fatal()s if it is closed. Reached from a renewal failure, this kills a mounted filesystem running as a systemd service: that happened to me — the service opened a login window by itself, I closed it, and the mount died.Fix
url.Valuesfor all token requests (RefreshandgetAuthTokens).temporarily_unavailable/server_error) keep the tokens and are retried later (at most every 10 s).invalid_grant) logs an error pointing atonedriver --auth-onlyand keeps the filesystem running from its cache. OnlyAuthenticate()at startup still starts an interactive login.Requestforce-renews the token in place instead of starting a login.Testing
Three new
httptest-based unit tests (no account needed): in-place update and form encoding; transient 503 / connection refused keep the tokens; a 400invalid_grantis treated as rejected without hammering the endpoint.go build ./...andgo vet ./...pass. Note: the tests needfs/graph's TestMain to not require a live login to run offline.Notes for reviewers
onedriver -a.getAuthTokens,getAuthCodeHeadlessandnewAuthstilllog.Fatal; they are now only reached at startup.🤖 Generated with Claude Code