Fix #105 and #107 for release 0.7.7

CHANGES.md

@@ -1,5 +1,18 @@
 # IDEA Community VCS Integration for Perforce
 
+## ::v0.7.7::
+
+### Overview
+
+* Bug fixes.
+
+### Details
+
+* Bug fixes.
+    * Fixed an issue where the user may be
+      told repeatedly that their password is wrong,
+      even though it's correct. (#105, #107)
+
 
 ## ::v0.7.6::
 

plugin/META-INF/plugin.xml

@@ -1,11 +1,18 @@
 <idea-plugin version="2">
     <name>Perforce IDEA Community Integration</name>
     <id>PerforceIC</id>
-    <version>0.7.6</version>
+    <version>0.7.7</version>
     <idea-version since-build="IC-135.1286"/>
     <category>VCS Integration</category>
     <change-notes><![CDATA[
         <ol>
+            <li><em>0.7.7</em>
+                <ol>
+                  <li>Fixed an issue where the user may be
+                  told repeatedly that their password is wrong,
+                  even though it's correct.</li>
+                </ol>
+            </li>
             <li><em>0.7.6</em>
                 <ol>
                     <li>Switched to using MD5 hash codes when comparing if a file is actually
@@ -29,30 +36,6 @@
                     <li>Fixed ignored tick marks (') in messages.</li>
                 </ol>
             </li>
-            <li><em>0.7.5</em>
-                <ol>
-                  <li>Switched to JDK 1.8 compiler for IDEA 16
-                  support.  The primary components of the plugin are
-                  still built with 1.6 compatibility, so older IDEs
-                  should continue to work.</li>
-                  <li>Trimmed whitespace on job IDs in the submit dialog.</li>
-                  <li>Annotation set before / after popup now shows correct list of
-                  revisions.</li>
-                  <li>Allow for pushing the open-for-edit on keystroke to be pushed
-                  out to a background thread.  Some users were encountering
-                  performance issues when opening a file for edit (#99),
-                  and this new option will allow for moving as much of that
-                  processing to away from the event thread.  The option is
-                  in the **User Preferences** of the Perforce VCS panel, titled
-                  "Open for edit in background."</li>
-                  <li>Removed the obsolete user preference for setting the maximum
-                  number of concurrent connections.  This is now limited to one
-                  per workspace.</li>
-                  <li>Fixed an IllegalStateException that could occur when refreshing
-                  the changelist view, when the P4 changelist was submitted or deleted on the
-                  server.</li>
-                </ol>
-            </li>
         </ol>
     ]]></change-notes>
     <description><![CDATA[

plugin/src/net/groboclown/idea/p4ic/server/connection/ClientPasswordConnectionHandler.java

@@ -17,7 +17,9 @@
 import com.intellij.openapi.project.Project;
 import com.perforce.p4java.PropertyDefs;
 import com.perforce.p4java.exception.AccessException;
+import com.perforce.p4java.exception.ConnectionException;
 import com.perforce.p4java.exception.P4JavaException;
+import com.perforce.p4java.exception.RequestException;
 import com.perforce.p4java.option.server.LoginOptions;
 import com.perforce.p4java.server.IOptionsServer;
 import net.groboclown.idea.p4ic.config.ServerConfig;
@@ -30,9 +32,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.Collections;
-import java.util.List;
-import java.util.Properties;
+import java.util.*;
 
 /**
  * Does not look at the P4CONFIG.  Uses a password for authentication.
@@ -42,6 +42,23 @@ public class ClientPasswordConnectionHandler extends ConnectionHandler {
 
     public static ClientPasswordConnectionHandler INSTANCE = new ClientPasswordConnectionHandler();
 
+    // TODO this is a poor work-around for bugs #105 and #107
+    // Essentially, a loop starts occurring where the initial
+    // login attempt triggers a false authentication request.
+    // This work-around keeps track of the servers that have
+    // already had their authentication called (in a weak
+    // map, to hopefully keep the memory leaks low), and will
+    // make an attempt to validate whether it really is
+    // in need of authentication or not.
+
+    // There is a gap in functionality here, when a
+    // user who isn't authorized to act upon a part of the
+    // registry gets a correct unauthorized error, but the
+    // check that we make ("getUser") passes without issue.
+    // That could be alleviated, possibly, by keeping a record
+    // of the number of invalid requests.
+    private Map<IOptionsServer, Throwable> authenticatedServers = new WeakHashMap<IOptionsServer, Throwable>();
+
     ClientPasswordConnectionHandler() {
         // stateless utility class
     }
@@ -94,6 +111,18 @@ public List<ConfigurationProblem> getConfigProblems(@NotNull final ServerConfig
     private AccessException authenticate(@Nullable Project project, @NotNull IOptionsServer server,
             @NotNull ServerConfig config, boolean force)
             throws P4JavaException {
+        if (authenticatedServers.containsKey(server))
+        {
+            LOG.info(authenticatedServers.get(server));
+            AccessException ex = testLogin(server, config.getUsername());
+            if (ex == null)
+            {
+                LOG.warn("Already authenticated server correctly!");
+                return null;
+            }
+            LOG.warn("login attempts have been wrong; try again", ex);
+        }
+
         // Default login - use the simple password
         final String password;
         try {
@@ -111,7 +140,7 @@ private AccessException authenticate(@Nullable Project project, @NotNull IOption
             try {
                 server.login(password, new LoginOptions(false, true));
                 LOG.debug("No issue logging in with stored password");
-                return null;
+                return testLogin(server, config.getUsername());
             } catch (AccessException ex) {
                 LOG.info("Stored password was bad; forgetting it", ex);
                 PasswordManager.getInstance().forgetPassword(project, config);
@@ -124,17 +153,33 @@ private AccessException authenticate(@Nullable Project project, @NotNull IOption
                 }
             }
         } else {
-            try {
-                // Perform an operation that should succeed, and only fail if the
-                // login is wrong.
-                server.getUser(config.getUsername());
-                return null;
-            } catch (AccessException ex) {
+            AccessException ex = testLogin(server, config.getUsername());
+            if (ex != null)
+            {
                 LOG.info("Stored password was bad; forgetting it", ex);
                 PasswordManager.getInstance().forgetPassword(project, config);
                 return new LoginRequiresPasswordException(ex);
             }
+            return null;
+        }
+    }
+
+
+    private AccessException testLogin(final IOptionsServer server, final String username)
+            throws ConnectionException, RequestException {
+        try {
+            // Perform an operation that should succeed, and only fail if the
+            // login is wrong.
+            server.getUser(username);
+            if (!authenticatedServers.containsKey(server)) {
+                authenticatedServers.put(server, new Throwable());
+            }
+            return null;
+        } catch (AccessException ex) {
+            authenticatedServers.remove(server);
+            return ex;
         }
+
     }
 
 }