Commit 65b8515d authored by Michael Gratton's avatar Michael Gratton 🤞

Merge branch 'wip/rework-password-prompting' into 'master'

Fix some issues with password prompting

See merge request !166
parents aa275681 58110dd4
Pipeline #65775 passed with stages
in 28 minutes and 41 seconds
......@@ -333,7 +333,7 @@ public class GearyController : Geary.BaseObject {
SecretMediator? libsecret = null;
try {
libsecret = yield new SecretMediator(this.application, cancellable);
libsecret = yield new SecretMediator(cancellable);
} catch (GLib.Error err) {
error("Error opening libsecret: %s", err.message);
}
......@@ -770,29 +770,41 @@ public class GearyController : Geary.BaseObject {
PasswordDialog password_dialog = new PasswordDialog(
this.application.get_active_window(),
account,
service
service,
credentials
);
if (password_dialog.run()) {
service.credentials = service.credentials.copy_with_token(
password_dialog.password
);
service.remember_password = password_dialog.remember_password;
// The update the credentials for the service that the
// credentials actually came from
Geary.ServiceInformation creds_service =
credentials == account.incoming.credentials
(credentials == account.incoming.credentials)
? account.incoming
: account.outgoing;
creds_service.credentials = credentials.copy_with_token(
password_dialog.password
);
// Update the remember password pref if changed
bool remember = password_dialog.remember_password;
if (creds_service.remember_password != remember) {
creds_service.remember_password = remember;
account.changed();
}
SecretMediator libsecret = (SecretMediator) account.mediator;
try {
yield libsecret.update_token(
account, creds_service, context.cancellable
);
// Update the actual service in the engine though
yield this.application.engine.update_account_service(
account, service, context.cancellable
);
// Update the secret using the service where the
// credentials originated, since the service forms
// part of the key's identity
if (creds_service.remember_password) {
yield libsecret.update_token(
account, creds_service, context.cancellable
);
} else {
yield libsecret.clear_token(
account, creds_service, context.cancellable
);
}
} catch (GLib.IOError.CANCELLED err) {
// all good
} catch (GLib.Error err) {
......@@ -805,6 +817,7 @@ public class GearyController : Geary.BaseObject {
)
);
}
context.authentication_attempts++;
} else {
// User cancelled, bail out unconditionally
......@@ -813,7 +826,11 @@ public class GearyController : Geary.BaseObject {
context.authentication_prompting = false;
}
if (!handled) {
if (handled) {
yield this.application.engine.update_account_service(
account, service, context.cancellable
);
} else {
context.authentication_attempts = 0;
context.authentication_failed = true;
update_account_status();
......
......@@ -94,17 +94,6 @@ public class GoaMediator : Geary.CredentialsMediator, Object {
return loaded;
}
public virtual async bool prompt_token(Geary.AccountInformation account,
Geary.ServiceInformation service,
GLib.Cancellable? cancellable)
throws GLib.Error {
// XXX Open a dialog that says "Click here to change your GOA
// password" or "GOA credentials need renewing" or
// something. Connect to the GOA service and wait until we
// hear that needs attention is no longer true.
return false;
}
private Geary.Credentials.Method get_auth_method() throws GLib.Error {
if (this.handle.get_oauth2_based() != null) {
return Geary.Credentials.Method.OAUTH2;
......
......@@ -35,14 +35,9 @@ public class SecretMediator : Geary.CredentialsMediator, Object {
null
);
private GearyApplication application;
private Geary.Nonblocking.Mutex dialog_mutex = new Geary.Nonblocking.Mutex();
public async SecretMediator(GearyApplication application,
GLib.Cancellable? cancellable)
public async SecretMediator(GLib.Cancellable? cancellable)
throws GLib.Error {
this.application = application;
yield check_unlocked(cancellable);
}
......@@ -51,84 +46,37 @@ public class SecretMediator : Geary.CredentialsMediator, Object {
Cancellable? cancellable)
throws GLib.Error {
bool loaded = false;
if (service.credentials != null && service.remember_password) {
string? password = yield Secret.password_lookupv(
SecretMediator.schema, new_attrs(service), cancellable
);
if (service.credentials != null) {
if (service.remember_password) {
string? password = yield Secret.password_lookupv(
SecretMediator.schema, new_attrs(service), cancellable
);
if (password == null) {
password = yield migrate_old_password(service, cancellable);
}
if (password == null) {
password = yield migrate_old_password(service, cancellable);
}
if (password != null) {
service.credentials =
if (password != null) {
service.credentials =
service.credentials.copy_with_token(password);
loaded = true;
loaded = true;
}
} else {
// Not remembering the password, so just make sure it
// has been filled in
loaded = service.credentials.is_complete();
}
}
if (!loaded) {
loaded = yield prompt_token(account, service, cancellable);
}
return loaded;
}
public virtual async bool prompt_token(Geary.AccountInformation account,
Geary.ServiceInformation service,
GLib.Cancellable? cancellable)
throws GLib.Error {
if (service.credentials != null) {
// to prevent multiple dialogs from popping up at the same
// time, use a nonblocking mutex to serialize the code
int token = yield dialog_mutex.claim_async(null);
// Ensure main window present to the window
this.application.present();
PasswordDialog password_dialog = new PasswordDialog(
this.application.get_active_window(),
account,
service
);
bool result = password_dialog.run();
dialog_mutex.release(ref token);
if (result) {
// password_dialog.password should never be null at this
// point. It will only be null when password_dialog.run()
// returns false, in which case we have already returned.
service.credentials = service.credentials.copy_with_token(
password_dialog.password
);
service.remember_password = password_dialog.remember_password;
yield update_token(account, service, cancellable);
account.changed();
}
}
return true;
}
public async void update_token(Geary.AccountInformation account,
Geary.ServiceInformation service,
Cancellable? cancellable)
throws Error {
if (service.credentials != null && service.remember_password) {
try {
yield do_store(service, service.credentials.token, cancellable);
} catch (Error e) {
debug(
"Unable to store libsecret password for %s: %s %s",
account.id,
to_proto_value(service.protocol),
service.credentials.user
);
}
} else {
yield clear_token(account, service, cancellable);
throws GLib.Error {
if (service.credentials != null) {
yield do_store(service, service.credentials.token, cancellable);
}
}
......
......@@ -25,7 +25,8 @@ public class PasswordDialog {
public PasswordDialog(Gtk.Window? parent,
Geary.AccountInformation account,
Geary.ServiceInformation service) {
Geary.ServiceInformation service,
Geary.Credentials? credentials) {
Gtk.Builder builder = GioUtil.create_builder("password-dialog.glade");
dialog = (Gtk.Dialog) builder.get_object("PasswordDialog");
......@@ -43,18 +44,13 @@ public class PasswordDialog {
Gtk.Label primary_text_label = (Gtk.Label) builder.get_object("primary_text_label");
primary_text_label.set_markup(PRIMARY_TEXT_MARKUP.printf(PRIMARY_TEXT_FIRST_TRY));
bool is_smtp = service.protocol == Geary.Protocol.SMTP;
Geary.Credentials? credentials = (is_smtp)
? account.get_outgoing_credentials() : account.incoming.credentials;
if (credentials != null) {
label_username.set_text(credentials.user);
entry_password.set_text(credentials.token ?? "");
}
check_remember_password.active = service.remember_password;
if (is_smtp) {
if ((service.protocol == Geary.Protocol.SMTP)) {
label_smtp.show();
}
......
......@@ -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) {
......
......@@ -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;
......
......@@ -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 {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment