Changeset 19627 in josm

Timestamp:

2026-09-20T21:33:09+02:00 (40 hours ago)

Author:

stoecker

Message:

fix #24890 - handle OAuth 2 against a server without a built-in client id - fix #24889 - add OAuthID for OpenGeofiction - patches by wangi

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/oauth/OAuth20Authorization.java (modified) (3 diffs)
• data/oauth/OAuthParameters.java (modified) (4 diffs)
• gui/oauth/AdvancedOAuthPropertiesPanel.java (modified) (3 diffs)
• gui/preferences/server/OAuthAuthenticationPreferencesPanel.java (modified) (4 diffs)

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

@@ -28 +28 @@
 import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler;
 import org.openstreetmap.josm.tools.HttpClient;
+import org.openstreetmap.josm.tools.Utils;
 import org.openstreetmap.josm.tools.JosmRuntimeException;
 import org.openstreetmap.josm.tools.OpenBrowser;
@@ -126 +127 @@
             try {
                 HttpClient tradeCodeForToken = HttpClient.create(new URL(parameters.getAccessTokenUrl()), "POST");
+                // A confidential client (the default when registering an application on
+                // openstreetmap-website) must present its secret at the token endpoint;
+                // the built-in OSM application is a public client and has none.
+                String clientSecret = parameters.getClientSecret();
                 tradeCodeForToken.setRequestBody(("grant_type=authorization_code&client_id=" + parameters.getClientId()
+                        + (!Utils.isEmpty(clientSecret) ? "&client_secret=" + Utils.encodeUrl(clientSecret) : "")
                         + "&redirect_uri=" + parameters.getRedirectUri()
                         + "&code=" + code
@@ -135 +141 @@
                     tradeCodeForToken.connect();
                     HttpClient.Response response = tradeCodeForToken.getResponse();
+                    if (response.getResponseCode() >= 400) {
+                        // Doorkeeper answers a rejected exchange with a JSON error body, but an
+                        // authentication failure can come back with none - say what the server said
+                        String body = response.fetchContent();
+                        throw new OAuth20Exception(tr("The server refused the token request: HTTP {0} {1}",
+                                response.getResponseCode(), Utils.isEmpty(body) ? response.getResponseMessage() : body));
+                    }
                     OAuth20Token oAuth20Token = new OAuth20Token(parameters, response.getContentReader());
                     consumer.accept(Optional.of(oAuth20Token));

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

@@ -77 +77 @@
                 throw new IllegalArgumentException("Unknown OAuth version: " + oAuthVersion);
         }
+    }
+
+    /**
+     * Get the OAuth 2.0 parameters the user entered in the advanced OAuth settings and saved with
+     * {@link IOAuthParameters#rememberPreferences()}. Until a token exists, this is the only place a
+     * client id for a server not in {@link #createDefault(String, OAuthVersion)} can come from.
+     * {@link OAuth20Parameters#rememberPreferences()} keys by the API URL as given; callers may pass
+     * either the URL or its host, so both are tried.
+     * @param keys The preference key suffixes to try, in order
+     * @return The remembered parameters, or {@code null} if none are saved or they cannot be parsed
+     * @since 19627
+     */
+    private static IOAuthParameters createFromPreferences(String... keys) {
+        for (String key : keys) {
+            if (Utils.isEmpty(key)) {
+                continue;
+            }
+            final String json = Config.getPref().get("oauth.access-token.parameters." + OAuthVersion.OAuth20 + "." + key, null);
+            if (Utils.isEmpty(json)) {
+                continue;
+            }
+            try {
+                OAuth20Parameters parameters = new OAuth20Parameters(json);
+                if (!Utils.isEmpty(parameters.getClientId())) {
+                    return parameters;
+                }
+            } catch (IllegalArgumentException | NullPointerException | JsonParsingException e) {
+                // A preference written by hand, or by an older JOSM with different fields
+                Logging.trace(e);
+            }
+        }
+        return null;
     }
 
@@ -138 +170 @@
                 clientSecret = null;
                 break;
+            case "https://opengeofiction.net/api":
+            case "https://lugus.opengeofiction.net/api":
+                // clientId provided by wangi (Lee Kindness), OpenGeofiction administrator.
+                // lugus is the OpenGeofiction test server.
+                clientId = "dKB9Fb5H4MxrM3I23UFxNCh5VGiAEQ_20gC7WmXnaaY";
+                clientSecret = null;
+                break;
             default:
                 clientId = "";
@@ -200 +239 @@
      */
     public static IOAuthParameters createFromApiUrl(String apiUrl, OAuthVersion oAuthVersion) {
+        final String originalApiUrl = apiUrl;
         // We actually need the host
         if (apiUrl.startsWith("https://") || apiUrl.startsWith("http://")) {
@@ -219 +259 @@
                     Logging.trace(e);
                 }
+                IOAuthParameters remembered = createFromPreferences(originalApiUrl, apiUrl);
+                if (remembered != null) {
+                    return remembered;
+                }
                 return createDefault(apiUrl, oAuthVersion);
             default:

trunk/src/org/openstreetmap/josm/gui/oauth/AdvancedOAuthPropertiesPanel.java

@@ -15 +15 @@
 import javax.swing.JLabel;
 import javax.swing.JOptionPane;
+import javax.swing.event.DocumentEvent;
+import javax.swing.event.DocumentListener;
 
 import org.openstreetmap.josm.data.oauth.IOAuthParameters;
@@ -48 +50 @@
  */
 public class AdvancedOAuthPropertiesPanel extends VerticallyScrollablePanel {
+    /**
+     * Property fired whenever the parameters this panel would return change: the client id was edited,
+     * or "use default settings" was toggled. Listeners re-read {@link #getAdvancedParameters()}.
+     * @since 19627
+     */
+    public static final String PARAMETERS_CHANGED_PROP = AdvancedOAuthPropertiesPanel.class.getName() + ".parametersChanged";
 
     private final JCheckBox cbUseDefaults = new JCheckBox(tr("Use default settings"));
@@ -138 +146 @@
         cbUseDefaults.addItemListener(ilUseDefault);
         cbUseDefaults.setSelected(Config.getPref().getBoolean("oauth.settings.use-default", true));
+        cbUseDefaults.addItemListener(e -> fireParametersChanged());
+        tfConsumerKey.getDocument().addDocumentListener(new DocumentListener() {
+            @Override
+            public void insertUpdate(DocumentEvent e) {
+                fireParametersChanged();
+            }
+
+            @Override
+            public void removeUpdate(DocumentEvent e) {
+                fireParametersChanged();
+            }
+
+            @Override
+            public void changedUpdate(DocumentEvent e) {
+                fireParametersChanged();
+            }
+        });
+    }
+
+    private void fireParametersChanged() {
+        firePropertyChange(PARAMETERS_CHANGED_PROP, null, null);
+    }
+
+    /**
+     * Determines if the panel is set to use the default parameters for the API URL rather than
+     * the values entered in its fields.
+     * @return {@code true} if "use default settings" is selected
+     * @since 19627
+     */
+    public boolean isUseDefaultSettings() {
+        return cbUseDefaults.isSelected();
+    }
+
+    /**
+     * Get the client id as entered in the panel, without falling back to the defaults.
+     * @return the client id field's text
+     * @since 19627
+     */
+    public String getEnteredClientId() {
+        return tfConsumerKey.getText();
     }
 

trunk/src/org/openstreetmap/josm/gui/preferences/server/OAuthAuthenticationPreferencesPanel.java

@@ -110 +110 @@
         );
         pnlAdvancedProperties.setVisible(false);
+        pnlAdvancedProperties.addPropertyChangeListener(this);
         return pnl;
+    }
+
+    /**
+     * Re-evaluate whether the "Authorize now" buttons can be enabled, after the API URL or the
+     * advanced OAuth parameters changed.
+     */
+    private void updateAuthoriseNowActions() {
+        for (JPanel panel : Arrays.asList(this.pnlNotYetAuthorised, (JPanel) this.pnlAlreadyAuthorised.getComponent(6))) {
+            for (Component component : panel.getComponents()) {
+                if (component instanceof JButton && ((JButton) component).getAction() instanceof AuthoriseNowAction) {
+                    ((AuthoriseNowAction) ((JButton) component).getAction()).updateEnabledState();
+                }
+            }
+        }
     }
 
@@ -149 +164 @@
         this.apiUrl = apiUrl;
         pnlAdvancedProperties.setApiUrl(apiUrl);
-        for (JPanel panel : Arrays.asList(this.pnlNotYetAuthorised, (JPanel) this.pnlAlreadyAuthorised.getComponent(6))) {
-            for (Component component : panel.getComponents()) {
-                if (component instanceof JButton && ((JButton) component).getAction() instanceof AuthoriseNowAction) {
-                    ((AuthoriseNowAction) ((JButton) component).getAction()).updateEnabledState();
-                }
-            }
-        }
+        updateAuthoriseNowActions();
     }
 
@@ -319 +328 @@
             if (procedure == AuthorizationProcedure.MANUALLY) {
                 this.setEnabled(true);
+            } else if (!pnlAdvancedProperties.isUseDefaultSettings()) {
+                // The user supplied the parameters - the client id of an OAuth application they
+                // registered on the server themselves. The wizard is given exactly these parameters
+                // when the button is pressed, so they decide whether it can be pressed.
+                this.setEnabled(!Utils.isEmpty(pnlAdvancedProperties.getEnteredClientId()));
             } else if (Utils.isValidUrl(apiUrl)) {
                 final URI apiURI;
@@ -421 +435 @@
     @Override
     public void propertyChange(PropertyChangeEvent evt) {
-        if (!evt.getPropertyName().equals(OsmApiUrlInputPanel.API_URL_PROP))
-            return;
-        setApiUrl((String) evt.getNewValue());
+        if (AdvancedOAuthPropertiesPanel.PARAMETERS_CHANGED_PROP.equals(evt.getPropertyName())) {
+            updateAuthoriseNowActions();
+        } else if (OsmApiUrlInputPanel.API_URL_PROP.equals(evt.getPropertyName())) {
+            setApiUrl((String) evt.getNewValue());
+        }
     }
 }