From c043840cb19fca81a56e87e32465388011da6b55 Mon Sep 17 00:00:00 2001
From: Olivier Goffart <ogoffart@woboq.com>
Date: Tue, 18 Jul 2017 14:53:41 +0200
Subject: [PATCH] OAuth: Fix refresh of token after expiration

Before commit d3b00532b1dd9f44cc606e6738b53345c37582cf,
fetchFromKeychain was called everytime we detect that the creds are
invalid (in AccountState::slotInvalidCredentials)
But since that commit, AccountState was calling askFromUser directly,
breaking the refresh of the token.

So I made sure AccountState::slotInvalidCredentials still calls
refreshAccessToken.

Another change that was made was too be sure to clear the cookies
in HttpCredentials::invalidateToken even when we are only clearing the
access_token. That's because the session with a cookie may stay valid
longer than the access_token
---
 src/gui/accountstate.cpp              | 16 +++++++++++-----
 src/libsync/creds/httpcredentials.cpp | 12 ++++++++----
 src/libsync/creds/httpcredentials.h   |  9 ++++++---
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp
index dbe998ce1..09300b1ab 100644
--- a/src/gui/accountstate.cpp
+++ b/src/gui/accountstate.cpp
@@ -16,6 +16,7 @@
 #include "accountmanager.h"
 #include "account.h"
 #include "creds/abstractcredentials.h"
+#include "creds/httpcredentials.h"
 #include "logger.h"
 #include "configfile.h"
 
@@ -305,12 +306,17 @@ void AccountState::slotInvalidCredentials()
     qCInfo(lcAccountState) << "Invalid credentials for" << _account->url().toString()
                            << "asking user";
 
-    if (account()->credentials()->ready())
-        account()->credentials()->invalidateToken();
-    account()->credentials()->askFromUser();
-
-    setState(AskingCredentials);
     _waitingForNewCredentials = true;
+    setState(AskingCredentials);
+
+    if (account()->credentials()->ready()) {
+        account()->credentials()->invalidateToken();
+        if (auto creds = qobject_cast<HttpCredentials *>(account()->credentials())) {
+            if (creds->refreshAccessToken())
+                return;
+        }
+    }
+    account()->credentials()->askFromUser();
 }
 
 void AccountState::slotCredentialsFetched(AbstractCredentials *)
diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp
index bd1cc7aef..2ebb9f9a2 100644
--- a/src/libsync/creds/httpcredentials.cpp
+++ b/src/libsync/creds/httpcredentials.cpp
@@ -292,8 +292,11 @@ void HttpCredentials::slotReadJobDone(QKeychain::Job *incomingJob)
     }
 }
 
-void HttpCredentials::refreshAccessToken()
+bool HttpCredentials::refreshAccessToken()
 {
+    if (_refreshToken.isEmpty())
+        return false;
+
     QUrl requestToken(_account->url().toString()
         + QLatin1String("/index.php/apps/oauth2/api/v1/token?grant_type=refresh_token&refresh_token=")
         + _refreshToken);
@@ -324,6 +327,7 @@ void HttpCredentials::refreshAccessToken()
         }
         emit fetched();
     });
+    return true;
 }
 
 
@@ -344,6 +348,9 @@ void HttpCredentials::invalidateToken()
         return;
     }
 
+    // clear the session cookie.
+    _account->clearCookieJar();
+
     if (!_refreshToken.isEmpty()) {
         // Only invalidate the access_token (_password) but keep the _refreshToken in the keychain
         // (when coming from forgetSensitiveData, the _refreshToken is cleared)
@@ -364,9 +371,6 @@ void HttpCredentials::invalidateToken()
     job2->setKey(kck);
     job2->start();
 
-    // clear the session cookie.
-    _account->clearCookieJar();
-
     // let QNAM forget about the password
     // This needs to be done later in the event loop because we might be called (directly or
     // indirectly) from QNetworkAccessManagerPrivate::authenticationRequired, which itself
diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h
index 85e6d092a..df3bcdabc 100644
--- a/src/libsync/creds/httpcredentials.h
+++ b/src/libsync/creds/httpcredentials.h
@@ -46,8 +46,8 @@ namespace OCC {
 
    1) First, AccountState will attempt to load the certificate from the keychain
 
-   ---->  fetchFromKeychain  ------------------------> shortcut to refreshAccessToken if the cached
-                |                           }                            information is still valid
+   ---->  fetchFromKeychain
+                |                           }
                 v                            }
           slotReadClientCertPEMJobDone       }     There are first 3 QtKeychain jobs to fetch
                 |                             }   the TLS client keys, if any, and the password
@@ -92,7 +92,10 @@ public:
     QString fetchUser();
     virtual bool sslIsTrusted() { return false; }
 
-    void refreshAccessToken();
+    /* If we still have a valid refresh token, try to refresh it assynchronously and emit fetched()
+     * otherwise return false
+     */
+    bool refreshAccessToken();
 
     // To fetch the user name as early as possible
     void setAccount(Account *account) Q_DECL_OVERRIDE;