From 67cd1efbc249809d314b0b0f052685b3a5ef3f31 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 30 May 2024 07:57:42 -0600 Subject: [PATCH] Better handle authorization problems Signed-off-by: Taylor Smock --- .../MapRouletteDownloadTaskBox.java | 23 ++-------------- .../josm/plugins/maproulette/api/TaskAPI.java | 1 + .../gui/task/list/LockUnlockTaskAction.java | 15 ----------- .../maproulette/util/ExceptionDialogUtil.java | 26 +++++++++++++++++- .../maproulette/util/OsmPreferenceUtils.java | 27 ++++++++++++------- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java index 1235499..7a6b9d3 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java +++ b/src/main/java/org/openstreetmap/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 index 93d1cc1..80d6c12 100644 --- 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/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java index a245248..5e1cd49 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java +++ b/src/main/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,7 +71,6 @@ private void lockTasks() { final var selected = this.table.getSelectionModel().getSelectedIndices(); final var model = (TaskTableModel) this.table.getModel(); final var selectedTasks = new ArrayList(); - final var messages = new HashSet(); for (int index : selected) { try { final var i = table.getRowSorter().convertRowIndexToModel(index); @@ -85,18 +78,10 @@ private void lockTasks() { 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(), - "" + messages.stream().sorted().collect(Collectors.joining("
")) + "", - 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 index f60eb09..10fe3a4 100644 --- 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("Failed to open a connection to the remote server
" + "''{0}''.
" + "Please check your internet connection.", socketException.getMessage()) + ""; 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 index c8fba86..3b33faf 100644 --- 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 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); + } + } }