Try to improve the UI for decryption/signing keys by hiding

the details and hopefully providing buttons to do what 99% of
users will want to do (#1003).

diff --git a/ChangeLog b/ChangeLog
index c28fd9c7e9860635caadf5f2ec15ed8f8a18d07d..910a8f145ab39198f31c8e818fe9ccd94d4556b8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 2018-01-06  Carl Hetherington  <cth@carlh.net>
 
+       * Simplify user interface for managing keys.
+
        * Version 2.11.37 released.
 
 2018-01-06  Carl Hetherington  <cth@carlh.net>

diff --git a/src/wx/config_dialog.cc b/src/wx/config_dialog.cc
index 30585f4e341ece1af0cb9441a98dc9d0c98d3554..3871f738c9711ba02e477730b53211fd2e217540 100644 (file)
--- a/src/wx/config_dialog.cc
+++ b/src/wx/config_dialog.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2017 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -21,7 +21,10 @@
 #include "config_dialog.h"
 #include "nag_dialog.h"
 
+using std::string;
 using boost::bind;
+using boost::optional;
+using boost::shared_ptr;
 
 static
 void
@@ -323,7 +326,7 @@ CertificateChainEditor::CertificateChainEditor (
        boost::function<boost::shared_ptr<const dcp::CertificateChain> (void)> get,
        boost::function<void (void)> nag_remake
        )
-       : wxPanel (parent)
+       : wxDialog (parent, wxID_ANY, title)
        , _set (set)
        , _get (get)
        , _nag_remake (nag_remake)
@@ -388,14 +391,14 @@ CertificateChainEditor::CertificateChainEditor (
        font.SetFamily (wxFONTFAMILY_TELETYPE);
        _private_key->SetFont (font);
        table->Add (_private_key, wxGBPosition (r, 1), wxDefaultSpan, wxALIGN_CENTER_VERTICAL);
-       _load_private_key = new wxButton (this, wxID_ANY, _("Load..."));
-       table->Add (_load_private_key, wxGBPosition (r, 2));
+       _import_private_key = new wxButton (this, wxID_ANY, _("Import..."));
+       table->Add (_import_private_key, wxGBPosition (r, 2));
        _export_private_key = new wxButton (this, wxID_ANY, _("Export..."));
        table->Add (_export_private_key, wxGBPosition (r, 3));
        ++r;
 
        _button_sizer = new wxBoxSizer (wxHORIZONTAL);
-       _remake_certificates = new wxButton (this, wxID_ANY, _("Re-make certificates\nand key..."));
+       _remake_certificates = new wxButton (this, wxID_ANY, _("Re-make certificates and key..."));
        _button_sizer->Add (_remake_certificates, 1, wxRIGHT, border);
        table->Add (_button_sizer, wxGBPosition (r, 0), wxGBSpan (1, 4));
        ++r;
@@ -413,17 +416,15 @@ CertificateChainEditor::CertificateChainEditor (
        _certificates->Bind        (wxEVT_LIST_ITEM_SELECTED,   boost::bind (&CertificateChainEditor::update_sensitivity, this));
        _certificates->Bind        (wxEVT_LIST_ITEM_DESELECTED, boost::bind (&CertificateChainEditor::update_sensitivity, this));
        _remake_certificates->Bind (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::remake_certificates, this));
-       _load_private_key->Bind    (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::load_private_key, this));
+       _import_private_key->Bind  (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::import_private_key, this));
        _export_private_key->Bind  (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::export_private_key, this));
 
-       SetSizerAndFit (_sizer);
-}
-
+       wxSizer* buttons = CreateSeparatedButtonSizer (wxCLOSE);
+       if (buttons) {
+               _sizer->Add (buttons, wxSizerFlags().Expand().DoubleBorder());
+       }
 
-void
-CertificateChainEditor::config_changed ()
-{
-       _chain.reset (new dcp::CertificateChain (*_get().get ()));
+       SetSizerAndFit (_sizer);
 
        update_certificate_list ();
        update_private_key ();
@@ -449,7 +450,7 @@ CertificateChainEditor::add_certificate ()
                        try {
                                extra = c.read_string (dcp::file_to_string (wx_to_std (d->GetPath ())));
                        } catch (boost::filesystem::filesystem_error& e) {
-                               error_dialog (this, wxString::Format (_("Could not load certificate (%s)"), d->GetPath().data()));
+                               error_dialog (this, wxString::Format (_("Could not import certificate (%s)"), d->GetPath().data()));
                                d->Destroy ();
                                return;
                        }
@@ -461,16 +462,17 @@ CertificateChainEditor::add_certificate ()
                                          "Only the first certificate will be used.")
                                        );
                        }
-                       _chain->add (c);
-                       if (!_chain->chain_valid ()) {
+                       shared_ptr<dcp::CertificateChain> chain(new dcp::CertificateChain(*_get().get()));
+                       chain->add (c);
+                       if (!chain->chain_valid ()) {
                                error_dialog (
                                        this,
                                        _("Adding this certificate would make the chain inconsistent, so it will not be added. "
                                          "Add certificates in order from root to intermediate to leaf.")
                                        );
-                               _chain->remove (c);
+                               chain->remove (c);
                        } else {
-                               _set (_chain);
+                               _set (chain);
                                update_certificate_list ();
                        }
                } catch (dcp::MiscError& e) {
@@ -492,8 +494,9 @@ CertificateChainEditor::remove_certificate ()
        }
 
        _certificates->DeleteItem (i);
-       _chain->remove (i);
-       _set (_chain);
+       shared_ptr<dcp::CertificateChain> chain(new dcp::CertificateChain(*_get().get()));
+       chain->remove (i);
+       _set (chain);
 
        update_sensitivity ();
 }
@@ -511,14 +514,14 @@ CertificateChainEditor::export_certificate ()
                wxFD_SAVE | wxFD_OVERWRITE_PROMPT
                );
 
-       dcp::CertificateChain::List all = _chain->root_to_leaf ();
+       dcp::CertificateChain::List all = _get()->root_to_leaf ();
        dcp::CertificateChain::List::iterator j = all.begin ();
        for (int k = 0; k < i; ++k) {
                ++j;
        }
 
        if (d->ShowModal () == wxID_OK) {
-               FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "w");
+               FILE* f = fopen_boost (path_from_file_dialog (d, "pem"), "w");
                if (!f) {
                        throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
                }
@@ -535,7 +538,7 @@ CertificateChainEditor::update_certificate_list ()
 {
        _certificates->DeleteAllItems ();
        size_t n = 0;
-       dcp::CertificateChain::List certs = _chain->root_to_leaf ();
+       dcp::CertificateChain::List certs = _get()->root_to_leaf ();
        BOOST_FOREACH (dcp::Certificate const & i, certs) {
                wxListItem item;
                item.SetId (n);
@@ -555,7 +558,7 @@ CertificateChainEditor::update_certificate_list ()
 
        static wxColour normal = _private_key_bad->GetForegroundColour ();
 
-       if (_chain->private_key_valid ()) {
+       if (_get()->private_key_valid()) {
                _private_key_bad->Hide ();
                _private_key_bad->SetForegroundColour (normal);
        } else {
@@ -567,7 +570,7 @@ CertificateChainEditor::update_certificate_list ()
 void
 CertificateChainEditor::remake_certificates ()
 {
-       boost::shared_ptr<const dcp::CertificateChain> chain = _get ();
+       boost::shared_ptr<const dcp::CertificateChain> chain = _get();
 
        std::string subject_organization_name;
        std::string subject_organizational_unit_name;
@@ -608,18 +611,19 @@ CertificateChainEditor::remake_certificates ()
                );
 
        if (d->ShowModal () == wxID_OK) {
-               _chain.reset (
-                       new dcp::CertificateChain (
-                               openssl_path (),
-                               d->organisation (),
-                               d->organisational_unit (),
-                               d->root_common_name (),
-                               d->intermediate_common_name (),
-                               d->leaf_common_name ()
+               _set (
+                       shared_ptr<dcp::CertificateChain> (
+                               new dcp::CertificateChain (
+                                       openssl_path (),
+                                       d->organisation (),
+                                       d->organisational_unit (),
+                                       d->root_common_name (),
+                                       d->intermediate_common_name (),
+                                       d->leaf_common_name ()
+                                       )
                                )
                        );
 
-               _set (_chain);
                update_certificate_list ();
                update_private_key ();
        }
@@ -638,12 +642,12 @@ CertificateChainEditor::update_sensitivity ()
 void
 CertificateChainEditor::update_private_key ()
 {
-       checked_set (_private_key, dcp::private_key_fingerprint (_chain->key().get ()));
+       checked_set (_private_key, dcp::private_key_fingerprint (_get()->key().get()));
        _sizer->Layout ();
 }
 
 void
-CertificateChainEditor::load_private_key ()
+CertificateChainEditor::import_private_key ()
 {
        wxFileDialog* d = new wxFileDialog (this, _("Select Key File"));
 
@@ -658,8 +662,9 @@ CertificateChainEditor::load_private_key ()
                                return;
                        }
 
-                       _chain->set_key (dcp::file_to_string (p));
-                       _set (_chain);
+                       shared_ptr<dcp::CertificateChain> chain(new dcp::CertificateChain(*_get().get()));
+                       chain->set_key (dcp::file_to_string (p));
+                       _set (chain);
                        update_private_key ();
                } catch (dcp::MiscError& e) {
                        error_dialog (this, wxString::Format (_("Could not read certificate file (%s)"), e.what ()));
@@ -674,7 +679,7 @@ CertificateChainEditor::load_private_key ()
 void
 CertificateChainEditor::export_private_key ()
 {
-       boost::optional<std::string> key = _chain->key ();
+       boost::optional<std::string> key = _get()->key();
        if (!key) {
                return;
        }
@@ -685,12 +690,12 @@ CertificateChainEditor::export_private_key ()
                );
 
        if (d->ShowModal () == wxID_OK) {
-               FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "w");
+               FILE* f = fopen_boost (path_from_file_dialog (d, "pem"), "w");
                if (!f) {
                        throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
                }
 
-               std::string const s = _chain->key().get ();
+               std::string const s = _get()->key().get ();
                fwrite (s.c_str(), 1, s.length(), f);
                fclose (f);
        }
@@ -706,42 +711,136 @@ KeysPage::GetName () const
 void
 KeysPage::setup ()
 {
-       if (_sign) {
-               _signer = new CertificateChainEditor (
-                       _panel, _("Signing DCPs and KDMs"), _border,
-                       bind (&Config::set_signer_chain, Config::instance (), _1),
-                       bind (&Config::signer_chain, Config::instance ()),
-                       bind (&do_nothing)
-                       );
+       wxFont subheading_font (*wxNORMAL_FONT);
+       subheading_font.SetWeight (wxFONTWEIGHT_BOLD);
 
-               _panel->GetSizer()->Add (_signer);
+       wxSizer* sizer = _panel->GetSizer();
+
+       {
+               wxStaticText* m = new wxStaticText (_panel, wxID_ANY, _("Decrypting KDMs"));
+               m->SetFont (subheading_font);
+               sizer->Add (m, 0, wxALL, _border);
        }
 
-       _decryption = new CertificateChainEditor (
+       wxButton* export_decryption_certificate = new wxButton (_panel, wxID_ANY, _("Export KDM decryption certificate..."));
+       sizer->Add (export_decryption_certificate, 0, wxLEFT, _border);
+       wxButton* export_decryption_chain = new wxButton (_panel, wxID_ANY, _("Export KDM decryption chain..."));
+       sizer->Add (export_decryption_chain, 0, wxLEFT, _border);
+       wxButton* export_settings = new wxButton (_panel, wxID_ANY, _("Export all KDM decryption settings..."));
+       sizer->Add (export_settings, 0, wxLEFT, _border);
+       wxButton* import_settings = new wxButton (_panel, wxID_ANY, _("Import all KDM decryption settings..."));
+       sizer->Add (import_settings, 0, wxLEFT, _border);
+       wxButton* decryption_advanced = new wxButton (_panel, wxID_ANY, _("Advanced..."));
+       sizer->Add (decryption_advanced, 0, wxALL, _border);
+
+       export_decryption_certificate->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_certificate, this));
+       export_decryption_chain->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_chain, this));
+       export_settings->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_chain_and_key, this));
+       import_settings->Bind (wxEVT_BUTTON, bind (&KeysPage::import_decryption_chain_and_key, this));
+       decryption_advanced->Bind (wxEVT_BUTTON, bind (&KeysPage::decryption_advanced, this));
+
+       {
+               wxStaticText* m = new wxStaticText (_panel, wxID_ANY, _("Signing DCPs and KDMs"));
+               m->SetFont (subheading_font);
+               sizer->Add (m, 0, wxALL, _border);
+       }
+
+       wxButton* signing_advanced = new wxButton (_panel, wxID_ANY, _("Advanced..."));
+       sizer->Add (signing_advanced, 0, wxLEFT, _border);
+       signing_advanced->Bind (wxEVT_BUTTON, bind (&KeysPage::signing_advanced, this));
+}
+
+void
+KeysPage::decryption_advanced ()
+{
+       CertificateChainEditor* c = new CertificateChainEditor (
                _panel, _("Decrypting KDMs"), _border,
                bind (&Config::set_decryption_chain, Config::instance (), _1),
                bind (&Config::decryption_chain, Config::instance ()),
                bind (&KeysPage::nag_remake_decryption_chain, this)
                );
 
-       _panel->GetSizer()->Add (_decryption);
+       c->ShowModal();
+}
 
-       _export_decryption_certificate = new wxButton (_decryption, wxID_ANY, _("Export KDM decryption\ncertificate..."));
-       _decryption->add_button (_export_decryption_certificate);
-       _export_decryption_chain = new wxButton (_decryption, wxID_ANY, _("Export KDM decryption\nchain..."));
-       _decryption->add_button (_export_decryption_chain);
+void
+KeysPage::signing_advanced ()
+{
+       CertificateChainEditor* c = new CertificateChainEditor (
+               _panel, _("Signing DCPs and KDMs"), _border,
+               bind (&Config::set_signer_chain, Config::instance (), _1),
+               bind (&Config::signer_chain, Config::instance ()),
+               bind (&do_nothing)
+               );
 
-       _export_decryption_certificate->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_certificate, this));
-       _export_decryption_chain->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_chain, this));
+       c->ShowModal();
 }
 
 void
-KeysPage::config_changed ()
+KeysPage::export_decryption_chain_and_key ()
 {
-       if (_sign) {
-               _signer->config_changed ();
+       wxFileDialog* d = new wxFileDialog (
+               _panel, _("Select Export File"), wxEmptyString, wxEmptyString, wxT ("DOM files (*.dom)|*.dom"),
+               wxFD_SAVE | wxFD_OVERWRITE_PROMPT
+               );
+
+       if (d->ShowModal () == wxID_OK) {
+               FILE* f = fopen_boost (path_from_file_dialog (d, "dom"), "w");
+               if (!f) {
+                       throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
+               }
+
+               string const chain = Config::instance()->decryption_chain()->chain();
+               fwrite (chain.c_str(), 1, chain.length(), f);
+               optional<string> const key = Config::instance()->decryption_chain()->key();
+               DCPOMATIC_ASSERT (key);
+               fwrite (key->c_str(), 1, key->length(), f);
+               fclose (f);
+       }
+       d->Destroy ();
+
+}
+
+void
+KeysPage::import_decryption_chain_and_key ()
+{
+       wxFileDialog* d = new wxFileDialog (
+               _panel, _("Select File To Import"), wxEmptyString, wxEmptyString, wxT ("DOM files (*.dom)|*.dom")
+               );
+
+       if (d->ShowModal () == wxID_OK) {
+               shared_ptr<dcp::CertificateChain> new_chain(new dcp::CertificateChain());
+
+               FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "r");
+               if (!f) {
+                       throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
+               }
+
+               string current;
+               while (!feof (f)) {
+                       char buffer[128];
+                       if (fgets (buffer, 128, f) == 0) {
+                               break;
+                       }
+                       current += buffer;
+                       if (strncmp (buffer, "-----END CERTIFICATE-----", 25) == 0) {
+                               new_chain->add (dcp::Certificate (current));
+                               current = "";
+                       } else if (strncmp (buffer, "-----END RSA PRIVATE KEY-----", 29) == 0) {
+                               std::cout << "the key is " << current << "\n";
+                               new_chain->set_key (current);
+                               current = "";
+                       }
+               }
+               fclose (f);
+
+               if (new_chain->chain_valid() && new_chain->private_key_valid()) {
+                       Config::instance()->set_decryption_chain (new_chain);
+               } else {
+                       error_dialog (_panel, _("Invalid DCP-o-matic export file"));
+               }
        }
-       _decryption->config_changed ();
+       d->Destroy ();
 }
 
 void
@@ -763,7 +862,7 @@ KeysPage::export_decryption_chain ()
                );
 
        if (d->ShowModal () == wxID_OK) {
-               FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "w");
+               FILE* f = fopen_boost (path_from_file_dialog (d, "pem"), "w");
                if (!f) {
                        throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
                }
@@ -784,7 +883,7 @@ KeysPage::export_decryption_certificate ()
                );
 
        if (d->ShowModal () == wxID_OK) {
-               FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "w");
+               FILE* f = fopen_boost (path_from_file_dialog (d, "pem"), "w");
                if (!f) {
                        throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
                }

diff --git a/src/wx/config_dialog.h b/src/wx/config_dialog.h
index 77389c375ef3d8c94bfdf0d2c67d6a5ee1b5329f..9a73d4e614722445c87e6ea11bad1e351493b5c8 100644 (file)
--- a/src/wx/config_dialog.h
+++ b/src/wx/config_dialog.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2017 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -111,7 +111,7 @@ private:
        wxCheckBox* _check_for_test_updates;
 };
 
-class CertificateChainEditor : public wxPanel
+class CertificateChainEditor : public wxDialog
 {
 public:
        CertificateChainEditor (
@@ -123,7 +123,6 @@ public:
                boost::function<void (void)> nag_remake
                );
 
-       void config_changed ();
        void add_button (wxWindow* button);
 
 private:
@@ -134,7 +133,7 @@ private:
        void remake_certificates ();
        void update_sensitivity ();
        void update_private_key ();
-       void load_private_key ();
+       void import_private_key ();
        void export_private_key ();
 
        wxListCtrl* _certificates;
@@ -143,12 +142,11 @@ private:
        wxButton* _remove_certificate;
        wxButton* _remake_certificates;
        wxStaticText* _private_key;
-       wxButton* _load_private_key;
+       wxButton* _import_private_key;
        wxButton* _export_private_key;
        wxStaticText* _private_key_bad;
        wxSizer* _sizer;
        wxBoxSizer* _button_sizer;
-       boost::shared_ptr<dcp::CertificateChain> _chain;
        boost::function<void (boost::shared_ptr<dcp::CertificateChain>)> _set;
        boost::function<boost::shared_ptr<const dcp::CertificateChain> (void)> _get;
        boost::function<void (void)> _nag_remake;
@@ -177,13 +175,13 @@ private:
 
        void export_decryption_certificate ();
        void export_decryption_chain ();
-       void config_changed ();
+       void config_changed () {}
        void nag_remake_decryption_chain ();
+       void decryption_advanced ();
+       void signing_advanced ();
+       void export_decryption_chain_and_key ();
+       void import_decryption_chain_and_key ();
 
-       CertificateChainEditor* _signer;
-       CertificateChainEditor* _decryption;
-       wxButton* _export_decryption_certificate;
-       wxButton* _export_decryption_chain;
        bool _sign;
 };
 

diff --git a/src/wx/wx_util.cc b/src/wx/wx_util.cc
index dd4ec3948765bbcdbf9946c8d0a923c3a669f24e..d732e03d305259d37d71b55291afb22146a53da4 100644 (file)
--- a/src/wx/wx_util.cc
+++ b/src/wx/wx_util.cc
@@ -404,3 +404,9 @@ maybe_show_splash ()
 
        return splash;
 }
+
+boost::filesystem::path
+path_from_file_dialog (wxFileDialog* dialog, string extension)
+{
+       return boost::filesystem::path(wx_to_std(dialog->GetPath())).replace_extension(extension);
+}

diff --git a/src/wx/wx_util.h b/src/wx/wx_util.h
index 7011d35d32559c54f7f6d992b0726be5347ffde2..dbd8e117a476b290a80ec2354490f852aab5532b 100644 (file)
--- a/src/wx/wx_util.h
+++ b/src/wx/wx_util.h
@@ -82,6 +82,7 @@ extern std::string string_client_data (wxClientData* o);
 extern wxString time_to_timecode (DCPTime t, double fps);
 extern void setup_audio_channels_choice (wxChoice* choice, int minimum);
 extern wxSplashScreen* maybe_show_splash ();
+extern boost::filesystem::path path_from_file_dialog (wxFileDialog* dialog, std::string extension);
 
 extern void checked_set (FilePickerCtrl* widget, boost::filesystem::path value);
 extern void checked_set (wxSpinCtrl* widget, int value);