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?
- 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). - On the Authentication tab, try to authenticate with OAuth 2.
- 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 aswitchon 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/apiwith the regular expression/api$, which does not match when the URL ends with a slash. View > History (web) then openshttps://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'/'.nullis 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 cachedOsmApiinstance. - 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. ApiUrlTestTaskandTestAccessTokenTaskeach 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, soApiUrlTestTask.getNormalizedApiUrl()andTestAccessTokenTask.normalizeApiUrl()are removed. While replacing them, the parameter ofApiUrlTestTask.alertInvalidCapabilitiesUrl()is renamed tocapabilitiesUrl: 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()andgetDefaultOAuth20Parameters()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()togetApiUrl(): "stripped" now describes only half of what it does, andgetApiUrl()is the name used for the same thing inIOAuthParameters,OAuth20ParametersandAbstractAuthorizationUI.
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.


