From e2f8a250279d9a79a03d82995ef191d773627bfa Mon Sep 17 00:00:00 2001 From: Dan Breslau Date: Tue, 23 Aug 2016 17:54:28 -0400 Subject: [PATCH] Fixed regressions and new bugs in importing --- src/moonshot-identities-manager.vala | 50 ++++++++++++++++-------------- src/moonshot-identity-management-view.vala | 26 +++++++++++++--- src/moonshot-identity-manager-app.vala | 13 ++++---- src/moonshot-local-flat-file-store.vala | 1 + src/moonshot-provisioning-common.vala | 8 +---- src/moonshot-server.vala | 32 +++++++++++++++++-- src/moonshot-webp-parser.vala | 2 +- 7 files changed, 88 insertions(+), 44 deletions(-) diff --git a/src/moonshot-identities-manager.vala b/src/moonshot-identities-manager.vala index f3d1b76..43c549d 100644 --- a/src/moonshot-identities-manager.vala +++ b/src/moonshot-identities-manager.vala @@ -142,30 +142,32 @@ public class IdentityManagerModel : Object { return true; } - private bool remove_duplicates(IdCard new_card) + private bool remove_duplicates(IdCard new_card, out ArrayList? old_duplicates) { - bool duplicate_found = false; - bool found = false; - do { - var cards = this.store.get_card_list(); - found = false; - foreach (IdCard id_card in cards) { - if ((new_card != id_card) && (id_card.nai == new_card.nai)) { - stdout.printf("removing duplicate id for '%s'\n", new_card.nai); - logger.trace("removing duplicate id for '%s'\n".printf(new_card.nai)); - remove_card_internal(id_card); - found = duplicate_found = true; - - if (new_card.trust_anchor.Compare(id_card.trust_anchor) == 0) { - logger.trace("Old and new cards have same trust anchor. Re-using the datetime_added and user_verified fields from the old card."); - new_card.trust_anchor.set_datetime_added(id_card.trust_anchor.datetime_added); - new_card.trust_anchor.user_verified = id_card.trust_anchor.user_verified; - } - break; - } + ArrayList dups = new ArrayList(); + var cards = this.store.get_card_list(); + foreach (IdCard id_card in cards) { + if ((new_card != id_card) && (id_card.nai == new_card.nai)) { + dups.add(id_card); + } + } + + foreach (IdCard id_card in dups) { + logger.trace("removing duplicate id for '%s'\n".printf(new_card.nai)); + remove_card_internal(id_card); + + if (new_card.trust_anchor.Compare(id_card.trust_anchor) == 0) { + logger.trace("Old and new cards have same trust anchor. Re-using the datetime_added and user_verified fields from the old card."); + new_card.trust_anchor.set_datetime_added(id_card.trust_anchor.datetime_added); + new_card.trust_anchor.user_verified = id_card.trust_anchor.user_verified; } - } while (found); - return duplicate_found; + } + + if (&old_duplicates != null) { + old_duplicates = dups; + } + + return (dups.size > 0); } public IdCard? find_id_card(string nai, bool force_flat_file_store) { @@ -187,7 +189,7 @@ public class IdentityManagerModel : Object { return retval; } - public void add_card(IdCard card, bool force_flat_file_store) { + public void add_card(IdCard card, bool force_flat_file_store, out ArrayList? old_duplicates=null) { if (card.temporary) { logger.trace("add_card: card is temporary; returning."); return; @@ -199,7 +201,7 @@ public class IdentityManagerModel : Object { if (force_flat_file_store) set_store_type(IIdentityCardStore.StoreType.FLAT_FILE); - remove_duplicates(card); + remove_duplicates(card, out old_duplicates); if (!display_name_is_valid(card.display_name, out candidate)) { diff --git a/src/moonshot-identity-management-view.vala b/src/moonshot-identity-management-view.vala index 9f4e6bc..c91d0e1 100644 --- a/src/moonshot-identity-management-view.vala +++ b/src/moonshot-identity-management-view.vala @@ -299,7 +299,7 @@ public class IdentityManagerView : Window { this.send_button.set_sensitive(false); } - public bool add_identity(IdCard id_card, bool force_flat_file_store) + public bool add_identity(IdCard id_card, bool force_flat_file_store, out ArrayList? old_duplicates=null) { #if OS_MACOS /* @@ -316,6 +316,10 @@ public class IdentityManagerView : Window { int flags = prev_id.Compare(id_card); logger.trace("add_identity: compare returned " + flags.to_string()); if (flags == 0) { + if (&old_duplicates != null) { + old_duplicates = new ArrayList(); + } + return false; // no changes, no need to update } else if ((flags & (1 << IdCard.DiffFlags.DISPLAY_NAME)) != 0) { dialog = new Gtk.MessageDialog(this, @@ -348,10 +352,15 @@ public class IdentityManagerView : Window { #endif if (ret == Gtk.ResponseType.YES) { - this.identities_manager.add_card(id_card, force_flat_file_store); + this.identities_manager.add_card(id_card, force_flat_file_store, out old_duplicates); return true; } - return false; + else { + if (&old_duplicates != null) { + old_duplicates = new ArrayList(); + } + return false; + } } private void add_identity_cb() @@ -887,7 +896,7 @@ SUCH DAMAGE. this, FileChooserAction.OPEN, _("Cancel"),ResponseType.CANCEL, - _("Save"), ResponseType.ACCEPT, + _("Open"), ResponseType.ACCEPT, null); if (import_directory != null) { @@ -915,6 +924,14 @@ SUCH DAMAGE. continue; } + if (!card.trust_anchor.is_empty()) { + string ta_datetime_added = TrustAnchor.format_datetime_now(); + card.trust_anchor.set_datetime_added(ta_datetime_added); + logger.trace("import_identities_cb : Set ta_datetime_added for '%s' to '%s'; ca_cert='%s'; server_cert='%s'" + .printf(card.display_name, ta_datetime_added, card.trust_anchor.ca_cert, card.trust_anchor.server_cert)); + } + + bool result = add_identity(card, use_flat_file_store); if (result) { logger.trace(@"import_identities_cb: Added or updated '$(card.display_name)'"); @@ -933,6 +950,7 @@ SUCH DAMAGE. msg_dialog.run(); msg_dialog.destroy(); } + dialog.destroy(); } } diff --git a/src/moonshot-identity-manager-app.vala b/src/moonshot-identity-manager-app.vala index 507ca8c..cb109e1 100644 --- a/src/moonshot-identity-manager-app.vala +++ b/src/moonshot-identity-manager-app.vala @@ -124,16 +124,17 @@ public class IdentityManagerApp { #endif } - public bool add_identity(IdCard id, bool force_flat_file_store) { + public bool add_identity(IdCard id, bool force_flat_file_store, out ArrayList? old_duplicates=null) { if (view != null) { logger.trace("add_identity: calling view.add_identity"); - return view.add_identity(id, force_flat_file_store); + return view.add_identity(id, force_flat_file_store, out old_duplicates); + } + else { + logger.trace("add_identity: calling model.add_card"); + model.add_card(id, force_flat_file_store, out old_duplicates); + return true; } - - logger.trace("add_identity: calling model.add_card"); - model.add_card(id, force_flat_file_store); - return true; } public void select_identity(IdentityRequest request) { diff --git a/src/moonshot-local-flat-file-store.vala b/src/moonshot-local-flat-file-store.vala index e4c303c..d1d7127 100644 --- a/src/moonshot-local-flat-file-store.vala +++ b/src/moonshot-local-flat-file-store.vala @@ -131,6 +131,7 @@ public class LocalFlatFileStore : Object, IIdentityCardStore { id_card_list.add(id_card); } catch (Error e) { + logger.error("load_id_cards: Error while loading keyfile: %s\n".printf(e.message)); stdout.printf("Error: %s\n", e.message); } } diff --git a/src/moonshot-provisioning-common.vala b/src/moonshot-provisioning-common.vala index 98b52df..29160dd 100644 --- a/src/moonshot-provisioning-common.vala +++ b/src/moonshot-provisioning-common.vala @@ -155,13 +155,7 @@ namespace WebProvisioning ta_subject, ta_subject_alt, false); - if (!ta.is_empty()) { - string ta_datetime_added = TrustAnchor.format_datetime_now(); - ta.set_datetime_added(ta_datetime_added); - logger.trace("end_element_func : Set ta_datetime_added for '%s' to '%s'".printf(card.display_name, ta_datetime_added)); - card.set_trust_anchor_from_store(ta); - } - + // Set the datetime_added in moonshot-server.vala, since it doesn't get sent via IPC card.set_trust_anchor_from_store(ta); } } diff --git a/src/moonshot-server.vala b/src/moonshot-server.vala index 54a5bff..ea3bd0c 100644 --- a/src/moonshot-server.vala +++ b/src/moonshot-server.vala @@ -29,6 +29,9 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ + +using Gee; + #if IPC_DBUS [DBus (name = "org.janet.Moonshot")] @@ -200,9 +203,20 @@ public class MoonshotServer : Object { idcard.update_services(services); var ta = new TrustAnchor(ca_cert, server_cert, subject, subject_alt, false); + if (!ta.is_empty()) { + // We have to set the datetime_added here, because it isn't delivered via IPC. + string ta_datetime_added = TrustAnchor.format_datetime_now(); + ta.set_datetime_added(ta_datetime_added); + logger.trace("install_id_card : Set ta_datetime_added for '%s' to '%s'; ca_cert='%s'; server_cert='%s'".printf(idcard.display_name, ta.datetime_added, ta.ca_cert, ta.server_cert)); + } + idcard.set_trust_anchor_from_store(ta); + logger.trace("install_id_card: Card '%s' has services: '%s'" .printf(idcard.display_name, idcard.get_services_string("; "))); + logger.trace(@"Installing IdCard named '$(idcard.display_name)'; ca_cert='$(idcard.trust_anchor.ca_cert)'; server_cert='$(idcard.trust_anchor.server_cert)'"); + + if (rules_patterns.length == rules_always_confirm.length) { /* workaround Centos vala array property bug: use temp array */ @@ -216,7 +230,17 @@ public class MoonshotServer : Object { idcard.rules = rules; } - return parent_app.add_identity(idcard, force_flat_file_store!=0); + ArrayList? old_duplicates = null; + var ret = parent_app.add_identity(idcard, (force_flat_file_store != 0), out old_duplicates); + + if (old_duplicates != null) { + // Printing to stdout here is ugly behavior; but it's old behavior that + // may be expected. (TODO: Do we need to keep this?) + foreach (IdCard id_card in old_duplicates) { + stdout.printf("removed duplicate id for '%s'\n", id_card.nai); + } + } + return ret; } @@ -493,10 +517,14 @@ public class MoonshotServer : Object { mutex.lock(); + ArrayList? old_duplicates = null; // Defer addition to the main loop thread. Idle.add(() => { mutex.lock(); - success = parent_app.add_identity(idcard, force_flat_file_store); + success = parent_app.add_identity(idcard, force_flat_file_store, out old_duplicates); + foreach (IdCard id_card in old_duplicates) { + stdout.printf("removing duplicate id for '%s'\n", new_card.nai); + } cond.signal(); mutex.unlock(); return false; diff --git a/src/moonshot-webp-parser.vala b/src/moonshot-webp-parser.vala index 46ba9c6..c5106b7 100644 --- a/src/moonshot-webp-parser.vala +++ b/src/moonshot-webp-parser.vala @@ -37,7 +37,7 @@ namespace WebProvisioning public static int main(string[] args) { - logger = new MoonshotLogger("WebProvisioning"); + logger = new MoonshotLogger("WebProvisioning (WebpParser)"); int arg_index = -1; int force_flat_file_store = 0; -- 2.1.4