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

diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp
index dbe998ce1aeb52b997f143a6a747c6ccfa0cbcf1..09300b1abd241926358bda210e06948ca27f1c0d 100644 (file)
--- 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())
+    _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();
-
-    setState(AskingCredentials);
-    _waitingForNewCredentials = true;
 }
 
 void AccountState::slotCredentialsFetched(AbstractCredentials *)

diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp
index bd1cc7aef75a1ae3967860b83e67ee4398b9a8e0..2ebb9f9a204dc25951b77b62f2979eacb31a78b3 100644 (file)
--- 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 85e6d092af1cf5bff4c565c6177408e8615f91da..df3bcdabc4fba82234fa2e6201b38a81d09875e2 100644 (file)
--- 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;