Modify

Opened 35 hours ago

#24907 new defect

[PATCH] Trailing slash in API breaks OAuth and links

Reported by: wangi Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: api url oauth Cc:

Description

A trailing slash on the OSM API URL breaks OAuth authentication and the links to the server.

Component: Core
Keywords: api url oauth trailing slash

What steps will reproduce the problem?

  1. Preferences > Connection Settings > OSM Server, uncheck "Use the default OSM server URL" and enter the API URL of a server with a trailing slash, for example https://api06.dev.openstreetmap.org/api/ (the field accepts it, and "Validate" reports the URL as valid).
  2. On the Authentication tab, try to authenticate with OAuth 2.
  3. Select an object and use View > History (web), or any other link to the web site of the server.

What is the expected result?

The trailing slash makes no difference: OAuth authenticates, and the web links point at the web site of the server, e.g. https://api06.dev.openstreetmap.org/node/1/history.

What happens instead?

  • OAuth 2 does not authenticate. OAuthParameters.getDefaultOAuth20Parameters() selects the client id with a switch on the API URL. https://…/api/ matches none of the known servers, so the client id stays empty and the authorization request is rejected.
  • Links to the server are built from the API path. JosmUrls.getOSMWebsiteDependingOnSelectedApi() derives the web site by stripping /api with the regular expression /api$, which does not match when the URL ends with a slash. View > History (web) then opens https://api06.dev.openstreetmap.org/api/node/1/history, and the same applies to the browse and user URLs.
  • The default API URL is no longer recognized as the default, since it is compared with equals, so even for the standard server the links are built from the API URL rather than from the OSM web site.

Ordinary API calls keep working, which is what makes this confusing: OsmApi.getBaseUrl() already collapses the resulting double slash.

Please provide any additional information below. Attach a screenshot if possible.

The attached patch (against r19627) normalizes the URL once instead of working around it at each use.

  • New OsmApi.normalizeApiUrl(String): removes leading and trailing white space and any trailing '/'. null is returned unchanged.
  • It is applied when the URL is read (OsmApi.getOsmApi(String) and the constructor), so an URL that is already stored in the preferences is fixed without the user having to edit it, and two URLs differing only by a trailing slash resolve to the same cached OsmApi instance.
  • It is applied when the URL is entered (OsmApiUrlInputPanel.getStrippedApiUrl()), so the value that is saved, validated and propagated to the OAuth panel is clean. The panel also displays the normalized URL, so the stored preference is rewritten without the slash the next time the settings are saved.
  • ApiUrlTestTask and TestAccessTokenTask each carried their own copy of exactly this normalization. Both now normalize once when the URL is stored in the constructor and use the field directly, so ApiUrlTestTask.getNormalizedApiUrl() and TestAccessTokenTask.normalizeApiUrl() are removed. While replacing them, the parameter of ApiUrlTestTask.alertInvalidCapabilitiesUrl() is renamed to capabilitiesUrl: it shadowed the field of the same name, and the two placeholders of that message are meant to show different URLs (the capabilities URL that could not be built, and the API URL).
  • OAuthParameters.createFromApiUrl() and getDefaultOAuth20Parameters() normalize their argument as well, so a caller which passes the URL as the user typed it also finds the client id.
  • Renames OsmApiUrlInputPanel.getStrippedApiUrl() to getApiUrl(): "stripped" now describes only half of what it does, and getApiUrl() is the name used for the same thing in IOAuthParameters, OAuth20Parameters and AbstractAuthorizationUI.

Two further places compare the API URL with equals and are fixed by the same normalization without being touched: Preferences clears the stored basic authentication credentials when the server is the default one (which no longer allows basic authentication), and NotesDialog decides by the same comparison whether short note and changeset links may be matched. OAuth20Token.sign() is not affected, since it tests the host with contains rather than comparing the whole URL.

Unit tests: OsmApiTest covers the normalization itself (trailing slash, several trailing slashes, surrounding white space, null, the https:// edge case), that getOsmApi() returns the same instance for both spellings, and that an API URL stored with a slash is used without one. JosmUrlsTest covers the "History (web)" case and the default URL entered with a slash. All five new tests fail without the patch and pass with it. The rest of org.openstreetmap.josm.io, org.openstreetmap.josm.data.oauth, org.openstreetmap.josm.gui.oauth and org.openstreetmap.josm.gui.preferences.server is unchanged (the two failures in OsmReaderTest.testMissingVersion and OAuth20AuthorizationTest are present on an unmodified checkout as well). Checkstyle is clean.

Attachments (1)

api-strip-slash.patch (17.5 KB ) - added by wangi 35 hours ago.

Download all attachments as: .zip

Change History (1)

by wangi, 35 hours ago

Attachment: api-strip-slash.patch added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to wangi.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


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