Bug 1453643 - Enable proper retry on oauth2 authenication failure. r=jorgk a=jorgk
authorGene Smith <gds@chartertn.net>
Sat, 07 Apr 2018 23:57:43 -0400
changeset 31403 417637b7cad86df52532cc7061f0cb683cbebb6a
parent 31402 76a7cea57a124d36f60e81aa2390057291ed0aba
child 31404 fef977d2e239a577890b3f6b410ca4d19cb9e3b4
push id383
push userclokep@gmail.com
push dateMon, 07 May 2018 21:52:48 +0000
reviewersjorgk, jorgk
bugs1453643
Bug 1453643 - Enable proper retry on oauth2 authenication failure. r=jorgk a=jorgk This prevents tb attempting to use an unauthenticated connection to mailbox(s) and avoid unexpected deletion of local mbox files and subsequent re-download of mailbox content over imap.
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapProtocol.h
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -5729,16 +5729,47 @@ void nsImapProtocol::MarkAuthMethodAsFai
  */
 void nsImapProtocol::ResetAuthMethods()
 {
   MOZ_LOG(IMAP, LogLevel::Debug, ("resetting (failed) auth methods"));
   m_currentAuthMethod = kCapabilityUndefined;
   m_failedAuthMethods = 0;
 }
 
+nsresult
+nsImapProtocol::SendDataParseIMAPandCheckForNewMail(const char *aData, const char *aCommand)
+{
+  nsresult rv;
+  bool isResend = false;
+  while (true)
+  {
+    // Send authentication string (true: suppress logging the string).
+    rv = SendData(aData, true);
+    if (NS_FAILED(rv))
+      break;
+    ParseIMAPandCheckForNewMail(aCommand);
+    if (!GetServerStateParser().WaitingForMoreClientInput())
+      break;
+
+    // The server is asking for the authentication string again. So we send
+    // the same string again although we know that it might be rejected again.
+    // We do that to get a firm authentication failure instead of a resend
+    // request. That keeps things in order before failing authentication and
+    // trying another method if capable.
+    if (isResend)
+    {
+      rv = NS_ERROR_FAILURE;
+      break;
+    }
+    isResend = true;
+  }
+
+  return rv;
+}
+
 nsresult nsImapProtocol::AuthLogin(const char *userName, const nsString &aPassword, eIMAPCapabilityFlag flag)
 {
   ProgressEventFunctionUsingName("imapStatusSendingAuthLogin");
   IncrementCommandTagNumber();
 
   char * currentCommand=nullptr;
   nsresult rv;
   NS_ConvertUTF16toUTF8 password(aPassword);
@@ -5895,39 +5926,17 @@ nsresult nsImapProtocol::AuthLogin(const
       len += PL_strlen(userName);
       len++;  // count for second <NUL> char
       PR_snprintf(&plainstr[len], 511-len, "%s", password.get());
       len += password.Length();
       char *base64Str = PL_Base64Encode(plainstr, len, nullptr);
       PR_snprintf(m_dataOutputBuf, OUTPUT_BUFFER_SIZE, "%s" CRLF, base64Str);
       PR_Free(base64Str);
 
-      bool isResend = false;
-      while (true)
-      {
-        // Send authentication string (true: suppress logging the string).
-        rv = SendData(m_dataOutputBuf, true);
-        if (NS_FAILED(rv))
-          break;
-        ParseIMAPandCheckForNewMail(currentCommand);
-        if (!GetServerStateParser().WaitingForMoreClientInput())
-          break;
-
-        // Server is asking for authentication string again. So we send the
-        // same string again although we already know that it will be
-        // rejected again. We do that to get a firm authentication failure
-        // instead of a resend request. That keeps things in order before
-        // failing "authenticate PLAIN" and trying another method if capable.
-        if (isResend)
-        {
-          rv = NS_ERROR_FAILURE;
-          break;
-        }
-        isResend = true;
-      }
+      rv = SendDataParseIMAPandCheckForNewMail(m_dataOutputBuf, currentCommand);
     } // if the last command succeeded
   } // if auth plain capability
   else if (flag & kHasAuthLoginCapability)
   {
     MOZ_LOG(IMAP, LogLevel::Debug, ("LOGIN auth"));
     PR_snprintf(m_dataOutputBuf, OUTPUT_BUFFER_SIZE, "%s authenticate LOGIN" CRLF, GetServerCommandTag());
     rv = SendData(m_dataOutputBuf);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -5997,19 +6006,18 @@ nsresult nsImapProtocol::AuthLogin(const
       return NS_ERROR_FAILURE;
     }
 
     // Send the data on the network.
     nsAutoCString command (GetServerCommandTag());
     command += " AUTHENTICATE XOAUTH2 ";
     command += base64Str;
     command += CRLF;
-    rv = SendData(command.get(), true /* suppress logging */);
-    NS_ENSURE_SUCCESS(rv, rv);
-    ParseIMAPandCheckForNewMail();
+
+    rv = SendDataParseIMAPandCheckForNewMail(command.get(), nullptr);
   }
   else if (flag & kHasAuthNoneCapability)
   {
     // TODO What to do? "login <username>" like POP?
     return NS_ERROR_NOT_IMPLEMENTED;
   }
   else
   {
--- a/mailnews/imap/src/nsImapProtocol.h
+++ b/mailnews/imap/src/nsImapProtocol.h
@@ -490,16 +490,17 @@ private:
   void ID(); // send RFC 2971 app info to server
   void EnableCondStore();
   void StartCompressDeflate();
   nsresult BeginCompressing();
   void Language(); // set the language on the server if it supports it
   void Namespace();
   void InsecureLogin(const char *userName, const nsCString &password);
   nsresult AuthLogin(const char *userName, const nsString &password, eIMAPCapabilityFlag flag);
+  nsresult SendDataParseIMAPandCheckForNewMail(const char *data, const char *command);
   void ProcessAuthenticatedStateURL();
   void ProcessAfterAuthenticated();
   void ProcessSelectedStateURL();
   bool TryToLogon();
 
   // Process Authenticated State Url used to be one giant if statement. I've broken out a set of actions
   // based on the imap action passed into the url. The following functions are imap protocol handlers for
   // each action. They are called by ProcessAuthenticatedStateUrl.