Opened 9 hours ago
#24932 new defect
[PATCH] Remove token leaves the OAuth access token in the preferences
| Reported by: | wangi | Owned by: | team |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Core | Version: | |
| Keywords: | oauth token preferences security | Cc: |
Description
"Remove token" leaves the OAuth access token in the preferences.
What steps will reproduce the problem?
- Preferences > Connection Settings > Authentication: authorize with OAuth 2, so that JOSM has an access token.
- Press "Remove token" and close the preferences.
- Look at
preferences.xml(or Advanced Preferences, searching foroauth.access-token).
What is the expected result?
The access token is gone from the preferences, as the authentication panel says.
What happens instead?
The authentication panel shows that there is no token, but the token is still stored in clear text under oauth.access-token.object.OAuth20.<host>. Only the parameters stored alongside it, under oauth.access-token.parameters.OAuth20.<host>, have been removed.
Please provide any additional information below. Attach a screenshot if possible.
JosmPreferencesCredentialAgent.storeOAuthAccessToken(host, null) builds both keys it deletes with the same prefix:
final String hostKey = "oauth.access-token.parameters." + oauthType + "." + host; final String parametersKey = "oauth.access-token.parameters." + oauthType + "." + host;
The first one should be oauth.access-token.object., which is what the store and lookup branches of the same class use. JOSM still believes the token is gone because lookupOAuthAccessToken() needs both keys to be present, and the parameters were deleted.
The attached patch (against r19636) fixes the prefix. CredentialsAgentTest.testLookupAndStoreOAuthTokens() only checks that a lookup fails after removal, which is why this was not noticed; a new test in JosmPreferencesCredentialAgentTest checks that both keys are gone from the preferences and from the sensitive key list. It fails without the patch and passes with it. All tests in org.openstreetmap.josm.io.auth, org.openstreetmap.josm.data.oauth, org.openstreetmap.josm.gui.oauth, org.openstreetmap.josm.gui.preferences.server and UserIdentityManagerTest pass, and checkstyle is clean.
Tokens that were "removed" before this fix are still in the preferences of the users concerned. The patch does not clean those up: nothing can tell an orphaned token from one whose parameters were lost some other way, and authorizing again overwrites it anyway.
Found while looking at #24928.


