Changeset 19008 in josm

Timestamp:

2024-03-06T15:07:01+01:00 (2 months ago)

Author:

taylor.smock

Message:

Fix an issue with custom OAuth2 parameters where the custom parameters would be replaced by default parameters

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/oauth/OAuthParameters.java (modified) (1 diff)
• gui/oauth/AdvancedOAuthPropertiesPanel.java (modified) (2 diffs)
• gui/oauth/FullyAutomaticAuthorizationUI.java (modified) (1 diff)
• gui/oauth/OAuthAuthorizationWizard.java (modified) (6 diffs)
• gui/preferences/server/OAuthAuthenticationPreferencesPanel.java (modified) (3 diffs)

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

@@ -197 +197 @@
      */
     public static IOAuthParameters createFromApiUrl(String apiUrl, OAuthVersion oAuthVersion) {
-        IOAuthParameters parameters = createDefault(apiUrl, oAuthVersion);
+        // We actually need the host
+        if (apiUrl.startsWith("https://") || apiUrl.startsWith("http://")) {
+            try {
+                apiUrl = new URI(apiUrl).getHost();
+            } catch (URISyntaxException syntaxException) {
+                Logging.trace(apiUrl);
+            }
+        }
         switch (oAuthVersion) {
             case OAuth20:
             case OAuth21: // Right now, OAuth 2.1 will work with our OAuth 2.0 implementation
-                OAuth20Parameters oAuth20Parameters = (OAuth20Parameters) parameters;
                 try {
                     IOAuthToken storedToken = CredentialsManager.getInstance().lookupOAuthAccessToken(apiUrl);
-                    return storedToken != null ? storedToken.getParameters() : oAuth20Parameters;
+                    if (storedToken != null) {
+                        return storedToken.getParameters();
+                    }
                 } catch (CredentialsAgentException e) {
                     Logging.trace(e);
                 }
-                return oAuth20Parameters;
+                return createDefault(apiUrl, oAuthVersion);
             default:
                 throw new IllegalArgumentException("Unknown OAuth version: " + oAuthVersion);

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

@@ -215 +215 @@
                 tfConsumerKey.getText(),
                 tfConsumerSecret.getText(),
+                tfAccessTokenURL.getText(),
                 tfAuthoriseURL.getText(),
-                tfAccessTokenURL.getText(),
+                apiUrl,
                 tfRequestTokenURL.getText()
                 );
@@ -261 +262 @@
                 this.resetToDefaultSettings();
             } else {
-                setAdvancedParameters(OAuthParameters.createFromApiUrl(paramApiUrl, OAuthVersion.OAuth20));
+                // createFromApiUrl may make a network request
+                final IOAuthParameters parameters = OAuthParameters.createFromApiUrl(paramApiUrl, OAuthVersion.OAuth20);
+                GuiHelper.runInEDT(() -> setAdvancedParameters(parameters));
             }
             GuiHelper.runInEDT(() -> ilUseDefault.setEnabled(true));

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

@@ -381 +381 @@
                     });
                 }
-            }, getApiUrl(), getOAuthVersion());
+            }, getApiUrl(), getOAuthVersion(), getOAuthParameters());
             getProgressMonitor().worked(1);
         }

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

@@ -84 +84 @@
         if ((this.oAuthVersion == OAuthVersion.OAuth20 || this.oAuthVersion == OAuthVersion.OAuth21)
         && this.procedure == AuthorizationProcedure.FULLY_AUTOMATIC) {
-            authorize(true, callback, this.apiUrl, this.oAuthVersion);
+            authorize(true, callback, this.apiUrl, this.oAuthVersion, getOAuthParameters());
         } else {
             setVisible(true);
@@ -90 +90 @@
                 throw new UserCancelException();
             }
-        }
-        OAuthAccessTokenHolder holder = OAuthAccessTokenHolder.getInstance();
-        holder.setAccessToken(apiUrl, getAccessToken());
-        holder.setSaveToPreferences(isSaveAccessTokenToPreferences());
+            OAuthAccessTokenHolder holder = OAuthAccessTokenHolder.getInstance();
+            holder.setAccessToken(apiUrl, getAccessToken());
+            holder.setSaveToPreferences(isSaveAccessTokenToPreferences());
+        }
     }
 
     /**
      * Perform the oauth dance
+     *
      * @param startRemoteControl {@code true} to start remote control if it is not already running
-     * @param callback The callback to use to notify that the OAuth dance succeeded
-     * @param apiUrl The API URL to get the token for
-     * @param oAuthVersion The OAuth version that the authorization dance is force
-     */
-    static void authorize(boolean startRemoteControl, Consumer<Optional<IOAuthToken>> callback, String apiUrl, OAuthVersion oAuthVersion) {
+     * @param callback           The callback to use to notify that the OAuth dance succeeded
+     * @param apiUrl             The API URL to get the token for
+     * @param oAuthVersion       The OAuth version that the authorization dance is force
+     * @param oAuthParameters    The OAuth parameters to use
+     */
+    static void authorize(boolean startRemoteControl, Consumer<Optional<IOAuthToken>> callback, String apiUrl,
+                          OAuthVersion oAuthVersion, IOAuthParameters oAuthParameters) {
         final boolean remoteControlIsRunning = Boolean.TRUE.equals(RemoteControl.PROP_REMOTECONTROL_ENABLED.get());
         // TODO: Ask user if they want to start remote control?
@@ -109 +112 @@
             RemoteControl.start();
         }
-        new OAuth20Authorization().authorize(OAuthParameters.createDefault(apiUrl, oAuthVersion), token -> {
+        new OAuth20Authorization().authorize(
+                Optional.ofNullable(oAuthParameters).orElseGet(() -> OAuthParameters.createDefault(apiUrl, oAuthVersion)),
+                token -> {
                     if (!remoteControlIsRunning) {
                         RemoteControl.stop();
@@ -253 +258 @@
      * Creates the wizard.
      *
-     * @param parent the component relative to which the dialog is displayed
-     * @param procedure the authorization procedure to use
-     * @param apiUrl the API URL. Must not be null.
-     * @param executor the executor used for running the HTTP requests for the authorization
-     * @param oAuthVersion The OAuth version this wizard is for
+     * @param parent             the component relative to which the dialog is displayed
+     * @param procedure          the authorization procedure to use
+     * @param apiUrl             the API URL. Must not be null.
+     * @param executor           the executor used for running the HTTP requests for the authorization
+     * @param oAuthVersion       The OAuth version this wizard is for
+     * @param advancedParameters The OAuth parameters to initialize the wizard with
      * @throws IllegalArgumentException if apiUrl is null
      */
     public OAuthAuthorizationWizard(Component parent, AuthorizationProcedure procedure, String apiUrl,
-                                    Executor executor, OAuthVersion oAuthVersion) {
+                                    Executor executor, OAuthVersion oAuthVersion, IOAuthParameters advancedParameters) {
         super(GuiHelper.getFrameForComponent(parent), ModalityType.DOCUMENT_MODAL);
         this.procedure = Objects.requireNonNull(procedure, "procedure");
@@ -268 +274 @@
         this.oAuthVersion = oAuthVersion;
         build();
+        if (advancedParameters != null) {
+            pnlFullyAutomaticAuthorisationUI.getAdvancedPropertiesPanel().setAdvancedParameters(advancedParameters);
+            pnlManualAuthorisationUI.getAdvancedPropertiesPanel().setAdvancedParameters(advancedParameters);
+        }
     }
 
@@ -362 +372 @@
                     AuthorizationProcedure.FULLY_AUTOMATIC,
                     serverUrl.toString(), Utils.newDirectExecutor(),
-                    OAuthVersion.OAuth20);
+                    OAuthVersion.OAuth20, null);
             wizard.showDialog(null);
             return wizard;

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

@@ -331 +331 @@
                     final String currentApiUrl = apiUrl;
                     MainApplication.worker.execute(() -> {
-                        final String clientId = OAuthParameters.createDefault(apiUrl, oAuthVersion).getClientId();
+                        final String clientId = OAuthParameters.createFromApiUrl(apiUrl, oAuthVersion).getClientId();
                         if (Objects.equals(apiUrl, currentApiUrl)) {
                             GuiHelper.runInEDT(() -> this.setEnabled(!Utils.isEmpty(clientId)));
@@ -347 +347 @@
                     apiUrl,
                     MainApplication.worker,
-                    oAuthVersion);
+                    oAuthVersion,
+                    pnlAdvancedProperties.getAdvancedParameters()
+                    );
             try {
                 wizard.showDialog(token -> GuiHelper.runInEDT(OAuthAuthenticationPreferencesPanel.this::refreshView));
@@ -407 +409 @@
         @Override
         public void actionPerformed(ActionEvent evt) {
-            IOAuthToken token = OAuthAccessTokenHolder.getInstance().getAccessToken(OsmApi.getOsmApi().getBaseUrl(), OAuthVersion.OAuth20);
+            IOAuthToken token = OAuthAccessTokenHolder.getInstance().getAccessToken(apiUrl, OAuthVersion.OAuth20);
             TestAccessTokenTask task = new TestAccessTokenTask(
                     OAuthAuthenticationPreferencesPanel.this,