From 4dfce57a58d201fdc5e5b7cea37cd929356d8510 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 9 Dec 2015 11:06:28 +0100 Subject: [PATCH] Creds: Forget password on explicit sign-out #4241 --- src/gui/accountsettings.cpp | 9 ++++----- src/gui/accountstate.cpp | 12 ++++++++---- src/gui/accountstate.h | 8 +++++++- src/gui/creds/shibbolethcredentials.cpp | 5 +++++ src/gui/creds/shibbolethcredentials.h | 1 + src/gui/owncloudgui.cpp | 15 +++------------ src/libsync/creds/abstractcredentials.h | 17 ++++++++++++++++- src/libsync/creds/dummycredentials.h | 1 + src/libsync/creds/httpcredentials.cpp | 6 ++++++ src/libsync/creds/httpcredentials.h | 1 + src/libsync/creds/tokencredentials.cpp | 5 +++++ src/libsync/creds/tokencredentials.h | 1 + 12 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index ee1d73cf6..c3c1bc4e9 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -176,14 +176,13 @@ void AccountSettings::slotOpenAccountWizard() OwncloudSetupWizard::runWizard(qApp, SLOT(slotownCloudWizardDone(int)), 0); } -// FIXME: Use same code path as ownCloudGui::slotLogout() void AccountSettings::slotToggleSignInState() { - bool signedOutState = _accountState->isSignedOut(); - if (!signedOutState) { - _accountState->account()->credentials()->invalidateToken(); + if (_accountState->isSignedOut()) { + _accountState->signIn(); + } else { + _accountState->signOutByUi(); } - _accountState->setSignedOut( !signedOutState ); } void AccountSettings::doExpand() diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp index b6035dbbf..26de4796a 100644 --- a/src/gui/accountstate.cpp +++ b/src/gui/accountstate.cpp @@ -118,11 +118,15 @@ bool AccountState::isSignedOut() const return _state == SignedOut; } -void AccountState::setSignedOut(bool signedOut) +void AccountState::signOutByUi() { - if (signedOut) { - setState(SignedOut); - } else if (_state == SignedOut) { + account()->credentials()->forgetSensitiveData(); + setState(SignedOut); +} + +void AccountState::signIn() +{ + if (_state == SignedOut) { setState(Disconnected); } } diff --git a/src/gui/accountstate.h b/src/gui/accountstate.h index dec0ca064..d75efd16d 100644 --- a/src/gui/accountstate.h +++ b/src/gui/accountstate.h @@ -78,7 +78,13 @@ public: static QString stateString(State state); bool isSignedOut() const; - void setSignedOut(bool signedOut); + + /** A user-triggered sign out which disconnects, stops syncs + * for the account and forgets the password. */ + void signOutByUi(); + + /// Move from SignedOut state to Disconnected (attempting to connect) + void signIn(); bool isConnected() const; bool isConnectedOrTemporarilyUnavailable() const; diff --git a/src/gui/creds/shibbolethcredentials.cpp b/src/gui/creds/shibbolethcredentials.cpp index c082a2bc1..2c09b7791 100644 --- a/src/gui/creds/shibbolethcredentials.cpp +++ b/src/gui/creds/shibbolethcredentials.cpp @@ -186,6 +186,11 @@ void ShibbolethCredentials::invalidateToken() _shibCookie = QNetworkCookie(); } +void ShibbolethCredentials::forgetSensitiveData() +{ + invalidateToken(); +} + void ShibbolethCredentials::onShibbolethCookieReceived(const QNetworkCookie& shibCookie) { storeShibCookie(shibCookie); diff --git a/src/gui/creds/shibbolethcredentials.h b/src/gui/creds/shibbolethcredentials.h index ecf66be93..06a70bcad 100644 --- a/src/gui/creds/shibbolethcredentials.h +++ b/src/gui/creds/shibbolethcredentials.h @@ -58,6 +58,7 @@ public: bool stillValid(QNetworkReply *reply) Q_DECL_OVERRIDE; void persist() Q_DECL_OVERRIDE; void invalidateToken() Q_DECL_OVERRIDE; + void forgetSensitiveData() Q_DECL_OVERRIDE; void showLoginWindow(); diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp index fd82315d7..5d0771e4b 100644 --- a/src/gui/owncloudgui.cpp +++ b/src/gui/owncloudgui.cpp @@ -629,15 +629,14 @@ void ownCloudGui::slotLogin() { auto list = AccountManager::instance()->accounts(); if (auto account = qvariant_cast(sender()->property(propertyAccountC))) { - account->setSignedOut(false); + account->signIn(); } else { foreach (const auto &a, list) { - a->setSignedOut(false); + a->signIn(); } } } -// FIXME: Unify codepath with AccountSettings::slotToggleSignInState() void ownCloudGui::slotLogout() { auto list = AccountManager::instance()->accounts(); @@ -647,15 +646,7 @@ void ownCloudGui::slotLogout() } foreach (const auto &ai, list) { - AccountPtr a = ai->account(); - // invalidate & forget token/password - a->credentials()->invalidateToken(); - // terminate all syncs and unload folders - FolderMan *folderMan = FolderMan::instance(); - folderMan->terminateSyncProcess(); - ai->setSignedOut(true); - // show result - slotComputeOverallSyncStatus(); + ai->signOutByUi(); } } diff --git a/src/libsync/creds/abstractcredentials.h b/src/libsync/creds/abstractcredentials.h index 10947b9ed..b45bd2c2a 100644 --- a/src/libsync/creds/abstractcredentials.h +++ b/src/libsync/creds/abstractcredentials.h @@ -50,9 +50,24 @@ public: virtual void askFromUser() = 0; virtual bool stillValid(QNetworkReply *reply) = 0; virtual void persist() = 0; - /** Invalidates auth token, or password for basic auth */ + + /** Invalidates token used to authorize requests, it will no longer be used. + * + * For http auth, this would be the session cookie. + * + * Note that sensitive data (like the password used to acquire the + * session cookie) may be retained. See forgetSensitiveData(). + */ virtual void invalidateToken() = 0; + /** Clears out all sensitive data; used for fully signing out users. + * + * This should always imply invalidateToken() but may go beyond it. + * + * For http auth, this would clear the session cookie and password. + */ + virtual void forgetSensitiveData() = 0; + static QString keychainKey(const QString &url, const QString &user); Q_SIGNALS: diff --git a/src/libsync/creds/dummycredentials.h b/src/libsync/creds/dummycredentials.h index b1cc3cf5b..7ee686267 100644 --- a/src/libsync/creds/dummycredentials.h +++ b/src/libsync/creds/dummycredentials.h @@ -37,6 +37,7 @@ public: void askFromUser() Q_DECL_OVERRIDE; void persist() Q_DECL_OVERRIDE; void invalidateToken() Q_DECL_OVERRIDE {} + void forgetSensitiveData() Q_DECL_OVERRIDE {}; }; } // namespace OCC diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index 4f29fb85e..6e8e7ba5f 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -238,6 +238,12 @@ void HttpCredentials::invalidateToken() #endif } +void HttpCredentials::forgetSensitiveData() +{ + invalidateToken(); + _previousPassword.clear(); +} + void HttpCredentials::persist() { if (_user.isEmpty()) { diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index d09fb1a2a..96467ff89 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -48,6 +48,7 @@ public: QString user() const Q_DECL_OVERRIDE; QString password() const; void invalidateToken() Q_DECL_OVERRIDE; + void forgetSensitiveData() Q_DECL_OVERRIDE; QString fetchUser(); virtual bool sslIsTrusted() { return false; } QString certificatePath() const; diff --git a/src/libsync/creds/tokencredentials.cpp b/src/libsync/creds/tokencredentials.cpp index 6187fda04..80be092cb 100644 --- a/src/libsync/creds/tokencredentials.cpp +++ b/src/libsync/creds/tokencredentials.cpp @@ -144,6 +144,11 @@ void TokenCredentials::invalidateToken() _password = QString(); } +void TokenCredentials::forgetSensitiveData() +{ + invalidateToken(); +} + void TokenCredentials::persist() { } diff --git a/src/libsync/creds/tokencredentials.h b/src/libsync/creds/tokencredentials.h index 1564a28a9..d107edf0a 100644 --- a/src/libsync/creds/tokencredentials.h +++ b/src/libsync/creds/tokencredentials.h @@ -49,6 +49,7 @@ public: void persist() Q_DECL_OVERRIDE; QString user() const Q_DECL_OVERRIDE; void invalidateToken() Q_DECL_OVERRIDE; + void forgetSensitiveData() Q_DECL_OVERRIDE; QString password() const; private Q_SLOTS: -- 2.30.2