Changeset 19638 in josm

Timestamp:

2026-10-06T09:00:18+02:00 (2 hours ago)

Author:

stoecker

Message:

oauth2 for a non-OSM fails setup on upload, patch by wangi, fix #24925

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/oauth/OAuthParameters.java (modified) (1 diff)
• src/org/openstreetmap/josm/io/OsmConnection.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/data/oauth/OAuthParametersTest.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/io/OsmConnectionTest.java (added)

trunk/src/org/openstreetmap/josm/data/oauth/OAuthParameters.java

@@ -263 +263 @@
                     return remembered;
                 }
-                return createDefault(apiUrl, oAuthVersion);
+                // Use the URL as it was given: apiUrl is the host by now, which is never a valid URL, so
+                // createDefault() would silently fall back to the API currently configured in the preferences.
+                return createDefault(originalApiUrl, oAuthVersion);
             default:
                 throw new IllegalArgumentException("Unknown OAuth version: " + oAuthVersion);

trunk/src/org/openstreetmap/josm/io/OsmConnection.java

@@ -34 +34 @@
 import org.openstreetmap.josm.tools.JosmRuntimeException;
 import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -147 +148 @@
      */
     private void obtainOAuth20Token() throws MissingOAuthAccessTokenException {
+        if (Utils.isEmpty(this.oAuth20Parameters.getClientId())) {
+            // Without a client id the authorization request is rejected by the server with "Missing required
+            // parameter: client_id", so do not send the user to the browser at all. The message of the exception
+            // is for the log only; what the user is shown already points at the preferences, which is where the
+            // client id of an OAuth application registered on this server has to be entered.
+            throw new MissingOAuthAccessTokenException(
+                    "No OAuth client id for " + OsmApi.getOsmApi().getServerUrl() + ", cannot request a token");
+        }
         if (!Boolean.TRUE.equals(GuiHelper.runInEDTAndWaitAndReturn(() ->
                 ConditionalOptionPaneUtil.showConfirmationDialog("oauth.oauth20.obtain.automatically",
@@ -201 +210 @@
     protected void addOAuth20AuthorizationHeader(HttpClient connection) throws OsmTransferException {
         if (this.oAuth20Parameters == null) {
-            this.oAuth20Parameters = OAuthParameters.createFromApiUrl(connection.getURL().getHost(), OAuthVersion.OAuth20);
+            // The parameters are stored under the API URL, so look them up with the API URL and not with the
+            // host of the request: OAuthParameters.createFromApiUrl() reduces it to the host itself where that
+            // is what it needs.
+            this.oAuth20Parameters = OAuthParameters.createFromApiUrl(OsmApi.getOsmApi().getServerUrl(), OAuthVersion.OAuth20);
         }
         OAuthAccessTokenHolder holder = OAuthAccessTokenHolder.getInstance();

trunk/test/unit/org/openstreetmap/josm/data/oauth/OAuthParametersTest.java

@@ -6 +6 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.TestUtils;
+import org.openstreetmap.josm.io.NetworkManager;
+import org.openstreetmap.josm.io.OnlineResource;
 import org.openstreetmap.josm.spi.preferences.Config;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 import org.openstreetmap.josm.tools.Logging;
 
@@ -17 +21 @@
  * Unit tests for class {@link OAuthParameters}.
  */
+@BasicPreferences
 class OAuthParametersTest {
+
+    @AfterEach
+    void tearDown() {
+        NetworkManager.setOnline(OnlineResource.ALL);
+        // the API URL is static state shared with the other tests of this class
+        Config.getPref().put("osm-server.url", null);
+    }
+
+    /**
+     * {@link OAuthParameters#createFromApiUrl} answers for the server it is asked about, whichever server is
+     * configured in the preferences, and finds the parameters the user entered in the advanced OAuth settings.
+     * <p>
+     * Non-regression test for #24925: the URL was reduced to the host before the fallback to
+     * {@link OAuthParameters#createDefault}, and a host is never a valid URL, so the defaults of the configured
+     * API were returned instead of those of the server which was asked about.
+     */
+    @Test
+    void testCreateFromApiUrlUsesTheRequestedServer() {
+        // the RFC 8414 lookup must not make a network request
+        NetworkManager.setOffline(OnlineResource.ALL);
+        final String thirdParty = "https://nonprod-mapops.example.org/api";
+        Config.getPref().put("osm-server.url", thirdParty);
+
+        // a server JOSM has a client id for is answered with that client id, not with the configured server's
+        assertEquals("Hl5yIhFS-Egj6aY7A35ouLOuZl0EHjj8JJQQ46IO96E",
+                OAuthParameters.createFromApiUrl("https://api.openhistoricalmap.org/api", OAuthVersion.OAuth20).getClientId());
+        // a server JOSM knows nothing about has no client id until the user enters one
+        assertEquals("", OAuthParameters.createFromApiUrl(thirdParty, OAuthVersion.OAuth20).getClientId());
+
+        // once the parameters are remembered - as the authentication preferences do, under the API URL - they
+        // are found both by the API URL and by the host, which is what a connection has at hand
+        new OAuth20Parameters("entered-client-id", null, thirdParty, thirdParty, "http://127.0.0.1:8111/oauth_authorization")
+                .rememberPreferences();
+        assertEquals("entered-client-id",
+                OAuthParameters.createFromApiUrl(thirdParty, OAuthVersion.OAuth20).getClientId());
+        assertEquals("https://api.openhistoricalmap.org/api",
+                OAuthParameters.createFromApiUrl("https://api.openhistoricalmap.org/api", OAuthVersion.OAuth20).getApiUrl());
+    }
+
     /**
      * Unit test of method {@link OAuthParameters#createDefault}.