Modify ↓

Opened 4 weeks ago

Closed 11 days ago

Last modified 11 days ago

#24932 closed defect (fixed)

[PATCH] Remove token leaves the OAuth access token in the preferences

Reported by: wangi Owned by: team
Priority: normal Milestone: 26.09
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?

  1. Preferences > Connection Settings > Authentication: authorize with OAuth 2, so that JOSM has an access token.
  2. Press "Remove token" and close the preferences.
  3. Look at preferences.xml (or Advanced Preferences, searching for oauth.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.

Attachments (1)

oauth-remove-token.patch​ (4.2 KB ) - added by wangi 4 weeks ago.

Download all attachments as: .zip

Change History (4)

by wangi, 4 weeks ago

Attachment: oauth-remove-token.patch​ added

comment:1 by wangi, 11 days ago

Any thoughts on this one? Good to get it merged ahead of starting #24928 authentication tidy up.

comment:2 by stoecker, 11 days ago

Milestone: → 26.09

Hmm. "a new test in JosmPreferencesCredentialAgentTest checks that both keys are gone from the preferences and from the sensitive key list." That part is missing in the patch, isn't it?

Forget that. Looked at wrong file :-(

Last edited 11 days ago by stoecker (previous) (diff)

comment:3 by stoecker, 11 days ago

Resolution: → fixed
Status: new → closed

In 19652/josm:

fix #24932 - patch by wangi - Remove token leaves the OAuth access token in the preferences

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.