Better handle authorization problems

Signed-off-by: Taylor Smock <[email protected]>
JOSM · May 30, 2024 · 67cd1ef · 67cd1ef
1 parent 188c58f
commit 67cd1ef
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 46 deletions.
diff --git a/...nstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java b/...nstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java
@@ -1,7 +1,6 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.maproulette.actions.downloadtasks;
 
-import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.io.IOException;
@@ -12,21 +11,14 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import javax.swing.JOptionPane;
-import javax.swing.SwingUtilities;
-
 import org.openstreetmap.josm.actions.downloadtasks.AbstractDownloadTask;
 import org.openstreetmap.josm.actions.downloadtasks.DownloadParams;
 import org.openstreetmap.josm.data.Bounds;
-import org.openstreetmap.josm.data.UserIdentityManager;
-import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
+import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
-import org.openstreetmap.josm.gui.preferences.PreferenceDialog;
-import org.openstreetmap.josm.gui.preferences.server.ServerAccessPreference;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressTaskId;
-import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.OsmApiException;
 import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI;
@@ -96,18 +88,7 @@ protected void realRun() throws IOException, OsmTransferException {
                     TaskCache.isHidden(task);
                 }
             } catch (UnauthorizedException unauthorizedException) {
-                final var message = UserIdentityManager.getInstance().isAnonymous()
-                        ? tr("Please log in to OpenStreetMap in JOSM")
-                        : tr("Please log in to the MapRoulette instance at least once: {0}", getBaseUrl());
-                GuiHelper.runInEDT(() -> {
-                    ConditionalOptionPaneUtil.showMessageDialog("maproulette.user.not.logged.in",
-                            MainApplication.getMainFrame(), message, message, JOptionPane.ERROR_MESSAGE);
-                    if (UserIdentityManager.getInstance().isAnonymous()) {
-                        final var p = new PreferenceDialog(MainApplication.getMainFrame());
-                        SwingUtilities.invokeLater(() -> p.selectPreferencesTabByClass(ServerAccessPreference.class));
-                        p.setVisible(true);
-                    }
-                });
+                ExceptionDialogUtil.explainException(unauthorizedException);
                 // This is specifically so that user's don't get a bug report message
                 final var transferException = new OsmApiException(unauthorizedException);
                 transferException.setUrl(MapRouletteConfig.getBaseUrl());

diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java
@@ -119,6 +119,7 @@ public static Task get(long task) throws IOException {
      * @param task The task to start
      * @return The updated task
      * @throws IOException if there was a problem communicating with the server
+     * @throws UnauthorizedException If we aren't authorized for the server
      */
     public static Task start(long task) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task + "/start");

diff --git a/...n/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java b/...n/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java
@@ -8,29 +8,23 @@
 import java.io.IOException;
 import java.io.Serial;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
 
-import javax.swing.JOptionPane;
 import javax.swing.JTable;
 
 import org.openstreetmap.josm.actions.AutoScaleAction;
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.data.osm.IPrimitive;
-import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI;
-import org.openstreetmap.josm.plugins.maproulette.api.UnauthorizedException;
 import org.openstreetmap.josm.plugins.maproulette.api.enums.TaskStatus;
 import org.openstreetmap.josm.plugins.maproulette.api.model.Task;
 import org.openstreetmap.josm.plugins.maproulette.gui.ModifiedObjects;
 import org.openstreetmap.josm.plugins.maproulette.gui.layer.MapRouletteClusteredPointLayer;
 import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil;
 import org.openstreetmap.josm.tools.ImageProvider;
-import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Shortcut;
 
 /**
@@ -77,26 +71,17 @@ private void lockTasks() {
         final var selected = this.table.getSelectionModel().getSelectedIndices();
         final var model = (TaskTableModel) this.table.getModel();
         final var selectedTasks = new ArrayList<Task>();
-        final var messages = new HashSet<String>();
         for (int index : selected) {
             try {
                 final var i = table.getRowSorter().convertRowIndexToModel(index);
                 final var task = model.get(i);
                 final var lockedTask = TaskAPI.start(task.id());
                 ModifiedObjects.addLockedTask(lockedTask);
                 selectedTasks.add(lockedTask);
-            } catch (UnauthorizedException unauthorizedException) {
-                Logging.trace(unauthorizedException);
-                messages.add(unauthorizedException.getMessage());
             } catch (IOException e) {
                 ExceptionDialogUtil.explainException(e);
             }
         }
-        if (!messages.isEmpty()) {
-            ConditionalOptionPaneUtil.showMessageDialog("maproulette.tasks.locked", MainApplication.getMainFrame(),
-                    "<html>" + messages.stream().sorted().collect(Collectors.joining("<br>")) + "</html>",
-                    tr("Could not lock tasks"), JOptionPane.INFORMATION_MESSAGE);
-        }
 
         ((TaskTableModel) this.table.getModel()).fireTableDataChanged();
         reselect(selected);

diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java
@@ -2,6 +2,7 @@
 package org.openstreetmap.josm.plugins.maproulette.util;
 
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
+import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.Component;
@@ -10,9 +11,16 @@
 import java.net.UnknownHostException;
 
 import javax.swing.JOptionPane;
+import javax.swing.SwingUtilities;
 
+import org.openstreetmap.josm.data.UserIdentityManager;
+import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.preferences.PreferenceDialog;
+import org.openstreetmap.josm.gui.preferences.server.ServerAccessPreference;
+import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.plugins.maproulette.api.UnauthorizedException;
 import org.openstreetmap.josm.tools.ExceptionUtil;
 
 /**
@@ -28,7 +36,23 @@ private ExceptionDialogUtil() {
      * @param exception The exception to explain to the user
      */
     public static void explainException(Exception exception) {
-        if (exception instanceof SocketException socketException) {
+        if (exception instanceof UnauthorizedException) {
+            OsmPreferenceUtils.clearCachedKey();
+            final var message = UserIdentityManager.getInstance().isAnonymous()
+                    ? tr("Please log in to OpenStreetMap in JOSM")
+                    : tr("Please log in to the MapRoulette instance at least once: {0}\n NOTE: you may need to reset your MapRoulette API key",
+                            getBaseUrl());
+            GuiHelper.runInEDT(() -> {
+                ConditionalOptionPaneUtil.showMessageDialog("maproulette.user.not.logged.in",
+                        MainApplication.getMainFrame(), message, message, JOptionPane.ERROR_MESSAGE);
+                if (UserIdentityManager.getInstance().isAnonymous()) {
+                    final var p = new PreferenceDialog(MainApplication.getMainFrame());
+                    SwingUtilities.invokeLater(() -> p.selectPreferencesTabByClass(ServerAccessPreference.class));
+                    p.setVisible(true);
+                }
+            });
+            showErrorDialog(message, tr("Unauthorized"), null);
+        } else if (exception instanceof SocketException socketException) {
             final var message = tr("<html>Failed to open a connection to the remote server<br>" + "''{0}''.<br>"
                     + "Please check your internet connection.", socketException.getMessage()) + "</html>";
             showErrorDialog(message, tr("Network exception"), ht("/ErrorMessages#NestedSocketException"));

diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java
@@ -37,16 +37,11 @@ private OsmPreferenceUtils() {
      */
     static String getMapRouletteApiKey() throws UnauthorizedException {
         final var user = UserIdentityManager.getInstance().getUserInfo();
-        final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList());
         if (user == null) {
-            // Right now JOSM doesn't support multiple users, so just wipe everything. If JOSM ever supports multiple users
-            for (var userId : userList.get()) {
-                final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + userId;
-                Config.getPref().put(preferenceKey, null);
-            }
+            clearCachedKey();
             throw new UnauthorizedException("User is not logged in");
         }
-        final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + "." + user.getId();
+        final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + '.' + user.getId();
         final var possibleApiKey = Config.getPref().get(preferenceKey);
         if (!Utils.isStripEmpty(possibleApiKey) && !"Couldn't authenticate you".equals(possibleApiKey)) {
             return possibleApiKey;
@@ -55,12 +50,14 @@ static String getMapRouletteApiKey() throws UnauthorizedException {
                 "maproulette_apikey_v2");
         final var reader = new OsmServerUserPreferencesReader();
         final var monitor = new PleaseWaitProgressMonitor(tr("Fetching OpenStreetMap User Preferences"));
+        final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList());
         try {
             final var key = reader.fetchUserPreferences(monitor, tr("Getting MapRoulette API Key"))
                     .getOrDefault(osmServerKey, null);
+            final String userId = String.valueOf(user.getId());
             List<String> userIds = new ArrayList<>(userList.get());
-            if (!userIds.contains(String.valueOf(user.getId()))) {
-                userIds.add(String.valueOf(user.getId()));
+            if (!userIds.contains(userId)) {
+                userIds.add(userId);
                 userList.put(userIds);
             }
             Config.getPref().put(preferenceKey, key);
@@ -71,4 +68,16 @@ static String getMapRouletteApiKey() throws UnauthorizedException {
             monitor.close();
         }
     }
+
+    /**
+     * Remove all cached keys (this can happen due to auth failure)
+     */
+    static void clearCachedKey() {
+        final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList());
+        // Right now JOSM doesn't support multiple users, so just wipe everything. If JOSM ever supports multiple users
+        for (var userId : userList.get()) {
+            final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + '.' + userId;
+            Config.getPref().put(preferenceKey, null);
+        }
+    }
 }