[geary/wip/rework-password-prompting: 1/5] Don't prompt for passwords from deep within the engine



commit 085518a65ca8eb7d6de18363c2f8207d97d2ca81
Author: Michael Gratton <mike vee net>
Date:   Wed Mar 6 22:46:56 2019 +1100

    Don't prompt for passwords from deep within the engine
    
    The fix for !161 moved updating credentials to reasonably deep within
    the IMAP service to fix some poor behaviour for GOA accounts, however
    local accounts would still prompt for a password if not present in
    libsecret from there, and if cancelled would prompt as many times as the
    service attempted to re-open the pool.
    
    This removes the call to prompt for a password from the libsecret
    mediator, brings back some of the API removed by commit c8f5e029, so
    that services can tell if credentials weren't loaded, and hence notify
    appropriately so the controller can then do the prompting.

 src/client/application/secret-mediator.vala   |  4 ---
 src/engine/api/geary-account-information.vala | 36 +++++++++++++++++----------
 src/engine/imap/api/imap-client-service.vala  |  6 ++++-
 src/engine/smtp/smtp-client-service.vala      |  4 ++-
 4 files changed, 31 insertions(+), 19 deletions(-)
---
diff --git a/src/client/application/secret-mediator.vala b/src/client/application/secret-mediator.vala
index 2d541b96..ca380753 100644
--- a/src/client/application/secret-mediator.vala
+++ b/src/client/application/secret-mediator.vala
@@ -67,10 +67,6 @@ public class SecretMediator : Geary.CredentialsMediator, Object {
             }
         }
 
-        if (!loaded) {
-            loaded = yield prompt_token(account, service, cancellable);
-        }
-
         return loaded;
     }
 
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 2d0a8ff1..89b83363 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -468,35 +468,45 @@ public class Geary.AccountInformation : BaseObject {
     /**
      * Loads this account's outgoing service credentials, if needed.
      *
-     * Credentials are loaded from the mediator, which may cause the
-     * user to be prompted for the secret, thus it may yield for some
-     * time.
+     * Credentials are loaded from the mediator, thus it may yield for
+     * some time.
      *
-     * Returns true if the credentials were successfully loaded or had
-     * been previously loaded, or false the credentials could not be
-     * loaded and the service's credentials are invalid.
+     * Returns true if the credentials were successfully loaded, or
+     * false if the credentials could not be loaded and the service's
+     * credentials are invalid.
      */
-    public async void load_outgoing_credentials(GLib.Cancellable? cancellable)
+    public async bool load_outgoing_credentials(GLib.Cancellable? cancellable)
         throws GLib.Error {
         Credentials? creds = this.outgoing.credentials;
+        bool loaded = false;
         if (creds != null) {
-            yield this.mediator.load_token(this, this.outgoing, cancellable);
+            loaded = yield this.mediator.load_token(
+                this, this.outgoing, cancellable
+            );
         }
+        return loaded;
     }
 
     /**
      * Loads this account's incoming service credentials, if needed.
      *
-     * Credentials are loaded from the mediator, which may cause the
-     * user to be prompted for the secret, thus it may yield for some
-     * time.
+     * Credentials are loaded from the mediator, thus it may yield for
+     * some time.
+     *
+     * Returns true if the credentials were successfully loaded, or
+     * false if the credentials could not be loaded and the service's
+     * credentials are invalid.
      */
-    public async void load_incoming_credentials(GLib.Cancellable? cancellable)
+    public async bool load_incoming_credentials(GLib.Cancellable? cancellable)
         throws GLib.Error {
         Credentials? creds = this.incoming.credentials;
+        bool loaded = false;
         if (creds != null) {
-            yield this.mediator.load_token(this, this.incoming, cancellable);
+            loaded = yield this.mediator.load_token(
+                this, this.incoming, cancellable
+            );
         }
+        return loaded;
     }
 
     public bool equal_to(AccountInformation other) {
diff --git a/src/engine/imap/api/imap-client-service.vala b/src/engine/imap/api/imap-client-service.vala
index 3b36043c..24a1d0d2 100644
--- a/src/engine/imap/api/imap-client-service.vala
+++ b/src/engine/imap/api/imap-client-service.vala
@@ -270,9 +270,13 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
             // are up-to-date before attempting a connection, but
             // after we know we should be able to connect to it
             try {
-                yield this.account.load_incoming_credentials(
+                bool loaded = yield this.account.load_incoming_credentials(
                     this.pool_cancellable
                 );
+                if (!loaded) {
+                    notify_authentication_failed();
+                    return;
+                }
             } catch (GLib.Error err) {
                 notify_connection_failed(new ErrorContext(err));
                 return;
diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala
index bd22190a..59c41ba0 100644
--- a/src/engine/smtp/smtp-client-service.vala
+++ b/src/engine/smtp/smtp-client-service.vala
@@ -190,7 +190,9 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
         throws GLib.Error {
         // To prevent spurious connection failures, ensure tokens are
         // up-to-date before attempting to send the email
-        yield this.account.load_outgoing_credentials(cancellable);
+        if (!yield this.account.load_outgoing_credentials(cancellable)) {
+            throw new SmtpError.AUTHENTICATION_FAILED("Credentials not loaded");
+        }
 
         Email? email = null;
         try {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]