Repository navigation
Conversation
ClearTokenFromStorage returned an error for any keyring failure other than ErrNotFound, so glean auth login failed in containers with no Secret Service daemon. If the keyring can't be read either, no token there can shadow the new credentials, so clear the config file and continue. It still fails when a token is readable from the keyring but can't be deleted. Fixes gleanwork#157
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.
Fixes #157.
glean auth logincallsconfig.ClearTokenFromStorage()before saving the OAuth tokens, and that function returned an error for any keyring failure other thanErrNotFound. In a devcontainer with no Secret Service daemon, the delete fails withThe name org.freedesktop.secrets was not provided by any .service files, so login aborts even though the OAuth tokens themselves are written to a file andSaveConfigalready falls back to file-only storage in the same situation.Change: when the keyring delete fails, check whether the token can still be read from the keyring. If it can't, no keyring token can shadow the new OAuth credentials, so log it and go on to clear the token from
~/.glean/config.json. If a token is still readable but couldn't be deleted (for example a keychain that allows reads but denies the delete), it still returns the error, so the stale-token protection frompersistLoginStatestays in place.Tests (in
TestClearTokenFromStorage):keyring.MockInitWithErrorwith the error from the issue): token is saved to the file only,ClearTokenFromStoragesucceeds, and the file token is cleared while the server URL is kept. This fails on main with the same error as the issue.error clearing token from keyring.Ran
go test ./...(all packages pass),go veton config and auth,gofmt, andgo build.golangci-lint run(v2.15.0 locally) reports the same 6 pre-existing goconst findings ininternal/configwith and without this change; CI's pinned version may differ. I didn't run a real OAuth login in a keyring-less container, only the unit tests.