On 06/14/2016 06:42 AM, Fraser Tweedale wrote:
On Mon, Jun 13, 2016 at 07:48:58PM +0200, Pavel Vomacka wrote:

On 06/13/2016 06:55 AM, Fraser Tweedale wrote:
On Fri, Jun 10, 2016 at 04:34:33PM +0200, Pavel Vomacka wrote:
Hello,

please review these new patches which add WebUI for Sub-CAs.

https://fedorahosted.org/freeipa/ticket/5939

Hi Pavel, I have reviewed the functionality of the patches.
Functionality is good - a few minor comments below.
Hello, thank you for review.
Patch 45:

1) In the main `Certificate Authorities' table, `Subject DN' is
showing the DN of the IPA object, instead of the Subject DN.
Fixed.
2) In the `Certificate Authorities' detail table, there is an
unlabelled row showing the DN of the IPA object.  IMO we do not need
to show this value at all.
The field removed.
Patch 46:

3) I see a FIXME in certificate.js.  The behaviour (default to IPA
CA / 'ipa') is OK.  Alternatively, you could allow the user to not
specify a CA (this will allow the default - currently 'ipa' - to be
controlled by server).
FIXME comment removed.
Patch 47:

4) For backwards compatibility, a CA ACL without any specified CAs
(and not cacat=all) implies the 'ipa' CA.  It would be good to
indicate this in the UI somehow, or include a notice to explain.
I added tooltip next to the checkbox on adder dialog and also note above the
table with CAs in CA ACL details view.

The message has a small typo: "specificed" should be "specified"
(the typo occurrs in both places).

Once this is fixed, ACK.
Updated patches attached.

--
Pavel^3 Vomacka
From 3a41b336d83c4864f474b01a2afbd705a5279a4e Mon Sep 17 00:00:00 2001
From: Pavel Vomacka <pvoma...@redhat.com>
Date: Fri, 10 Jun 2016 16:12:45 +0200
Subject: [PATCH 1/3] Add new webui plugin - ca

Whole new entity for CAs.

https://fedorahosted.org/freeipa/ticket/5939
---
 install/ui/src/freeipa/app.js                  |  1 +
 install/ui/src/freeipa/navigation/menu_spec.js |  5 ++
 install/ui/src/freeipa/plugins/ca.js           | 91 ++++++++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 install/ui/src/freeipa/plugins/ca.js

diff --git a/install/ui/src/freeipa/app.js b/install/ui/src/freeipa/app.js
index daf17b7ba021d3db8288f2de89a8ae4814172a70..4eb045d7a989bdfee29cef670770d285701ae875 100644
--- a/install/ui/src/freeipa/app.js
+++ b/install/ui/src/freeipa/app.js
@@ -29,6 +29,7 @@ define([
     './aci',
     './automember',
     './automount',
+    './plugins/ca',
     './plugins/caacl',
     './plugins/certprofile',
     './dns',
diff --git a/install/ui/src/freeipa/navigation/menu_spec.js b/install/ui/src/freeipa/navigation/menu_spec.js
index 0afc7daceb725cee5677982c68c09d1666d42885..97a03527e94ba2ae21d70e0170b91efc81b155a4 100644
--- a/install/ui/src/freeipa/navigation/menu_spec.js
+++ b/install/ui/src/freeipa/navigation/menu_spec.js
@@ -142,6 +142,11 @@ var nav = {};
                             entity: 'caacl',
                             facet: 'search',
                             hidden: true
+                        },
+                        {
+                            entity: 'ca',
+                            facet: 'search',
+                            hidden: true
                         }
                     ]
                 },
diff --git a/install/ui/src/freeipa/plugins/ca.js b/install/ui/src/freeipa/plugins/ca.js
new file mode 100644
index 0000000000000000000000000000000000000000..8e2fb702fe3d220e7cae5d30dbb85e0ae5ad9cf7
--- /dev/null
+++ b/install/ui/src/freeipa/plugins/ca.js
@@ -0,0 +1,91 @@
+//
+// Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+//
+
+define([
+    '../ipa',
+    '../jquery',
+    '../phases',
+    '../reg',
+    '../certificate'
+],
+function(IPA, $, phases, reg, cert) {
+
+/**
+ * ca module
+ * @class plugins.ca
+ * @singleton
+ */
+ var ca = IPA.ca = {};
+
+ var make_ca_spec = function() {
+     var spec = {
+         name: 'ca',
+         facets: [
+             {
+                 $type: 'search',
+                 disable_facet_tabs: false,
+                 tabs_in_sidebar: true,
+                 tab_label: '@mo:ca.label',
+                 facet_groups: [cert.facet_group],
+                 facet_group: 'certificates',
+                 columns: [
+                     'cn',
+                     'ipacasubjectdn',
+                     'description'
+                 ]
+             },
+             {
+                 $type: 'details',
+                 disable_facet_tabs: true,
+                 fields: [
+                     'cn',
+                     {
+                         $type: 'textarea',
+                         name: 'description'
+                     },
+                     'ipacaid',
+                     'ipacaissuerdn',
+                     'ipacasubjectdn'
+                 ]
+             }
+         ],
+         adder_dialog: {
+             fields: [
+                 {
+                     $type: 'text',
+                     name: 'cn',
+                     required: true
+                 },
+                 'ipacasubjectdn',
+                 {
+                     $type: 'textarea',
+                     name: 'description'
+                 }
+             ]
+         }
+     };
+
+     return spec;
+ };
+
+ /**
+  * CA entity specification object
+  * @member plugins.ca
+  */
+ca.ca_spec = make_ca_spec();
+
+/**
+ * Register entity
+ * @member plugins.ca
+ */
+ca.register = function() {
+    var e = reg.entity;
+
+    e.register({type: 'ca', spec: ca.ca_spec});
+};
+
+phases.on('registration', ca.register);
+
+return ca;
+});
-- 
2.5.5

From 9665ebd57f07377d24bfc728ada6620caae0fac6 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka <pvoma...@redhat.com>
Date: Fri, 10 Jun 2016 16:15:07 +0200
Subject: [PATCH 2/3] Extend certificate entity page

Add field for choosing CA when issuing new certificate. Add new item to action menu
on cert details page which allows user to download the certificate as file.

Part of: https://fedorahosted.org/freeipa/ticket/5939
---
 install/ui/src/freeipa/certificate.js | 77 ++++++++++++++++++++++++++++++++---
 install/ui/test/data/ipa_init.json    |  2 +
 ipaserver/plugins/internal.py         |  2 +
 3 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index d03be205690b339e9ae56985ed0508df06ad1887..79f2582f8c38effae91ec440f2c7e58f27444f44 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -38,6 +38,8 @@ define([
 
 var exp = IPA.cert = {};
 
+IPA.cert.TOPLEVEL_CA = 'ipa';
+
 IPA.cert.BEGIN_CERTIFICATE = '-----BEGIN CERTIFICATE-----';
 IPA.cert.END_CERTIFICATE   = '-----END CERTIFICATE-----';
 
@@ -446,6 +448,14 @@ IPA.cert.request_dialog = function(spec) {
     section0.fields.push(
         {
             $type: 'entity_select',
+            name: 'cacn',
+            label: '@i18n:objects.cert.ca',
+            other_entity: 'ca',
+            other_field: 'cn',
+            required: true
+        },
+        {
+            $type: 'entity_select',
             name: 'profile_id',
             other_entity: 'certprofile',
             other_field: 'cn',
@@ -494,6 +504,11 @@ IPA.cert.request_dialog = function(spec) {
         }
     });
 
+    that.open = function() {
+        that.dialog_open();
+        that.get_field('cacn').set_value([IPA.cert.TOPLEVEL_CA]);
+    };
+
     return that;
 };
 
@@ -681,6 +696,51 @@ IPA.cert.get_action = function(spec) {
     return that;
 };
 
+IPA.cert.create_data_uri = function(certificate) {
+    if (typeof certificate !== 'string') return '';
+
+    var format = 'data:,';
+    var uri_new_line = '%0A';
+
+    var data_uri = IPA.cert.pem_format_base64(certificate);
+    data_uri = IPA.cert.pem_cert_format(data_uri);
+    data_uri = format + data_uri.replace(/\n/g, uri_new_line);
+
+    return data_uri;
+};
+
+IPA.cert.perform_download = function(data_uri) {
+    var a = document.createElement("a");
+    // Adding own click function as workaround for Firefox
+    a.click = function() {
+        var evt = this.ownerDocument.createEvent('MouseEvents');
+        evt.initMouseEvent('click', true, true, this.ownerDocument.defaultView,
+            1, 0, 0, 0, 0, false, false, false, false, 0, null);
+        this.dispatchEvent(evt);
+    };
+    a.download = 'cert.pem';
+    a.href = data_uri;
+
+    a.click();
+};
+
+IPA.cert.download_action = function(spec) {
+    spec = spec || {};
+    spec.name = spec.name || 'download_cert';
+    spec.label = spec.label || '@i18n:objects.cert.download';
+
+    var that = IPA.action(spec);
+
+    that.execute_action = function(facet) {
+        if (!facet.certificate) return;
+
+        var data_uri = IPA.cert.create_data_uri(facet.certificate.certificate);
+        IPA.cert.perform_download(data_uri);
+    };
+
+    return that;
+};
+
 IPA.cert.request_action = function(spec) {
 
     spec = spec || {};
@@ -736,7 +796,8 @@ IPA.cert.request_action = function(spec) {
             request: function(values) {
 
                 var options = {
-                    'principal': entity_principal
+                    'principal': entity_principal,
+                    'cacn': values.cacn[0]
                 };
                 if (values.profile_id) options.profile_id = values.profile_id[0];
                 if (values.principal) options.principal = values.principal[0];
@@ -1212,7 +1273,8 @@ exp.facet_group = {
     facets: {
         certificates: 'cert_search',
         profiles: 'certprofile_search',
-        acls: 'caacl_search'
+        acls: 'caacl_search',
+        ca_search: 'ca_search'
     }
 };
 
@@ -1345,14 +1407,15 @@ return {
             disable_facet_tabs: true,
             actions: [
                 'cert_revoke',
-                'cert_remove_hold'
+                'cert_remove_hold',
+                'download_cert'
             ],
             state: {
                 evaluators: [
                     IPA.cert.certificate_evaluator
                 ]
             },
-            header_actions: ['revoke_cert', 'remove_hold_cert'],
+            header_actions: ['revoke_cert', 'remove_hold_cert', 'download_cert'],
             sections: [
                 {
                     name: 'details',
@@ -1361,7 +1424,10 @@ return {
                         'serial_number',
                         'serial_number_hex',
                         'subject',
-                        'issuer',
+                        {
+                            name: 'issuer',
+                            read_only: true
+                        },
                         'valid_not_before',
                         'valid_not_after',
                         'sha1_fingerprint',
@@ -1539,6 +1605,7 @@ exp.register = function() {
     a.register('cert_view', IPA.cert.view_action);
     a.register('cert_get', IPA.cert.get_action);
     a.register('cert_request', IPA.cert.request_action);
+    a.register('download_cert', IPA.cert.download_action);
     a.register('cert_revoke', IPA.cert.revoke_action);
     a.register('cert_remove_hold', IPA.cert.remove_hold_action);
 
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 4ccf49f889f57eb28f11d8ebe701fac47d166879..053940d10779ad5286df55380ed5d370c6ba96f4 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -230,12 +230,14 @@
                             "aa_compromise": "AA Compromise",
                             "add_principal": "Add principal",
                             "affiliation_changed": "Affiliation Changed",
+                            "ca": "CA",
                             "ca_compromise": "CA Compromise",
                             "certificate": "Certificate",
                             "certificates": "Certificates",
                             "certificate_hold": "Certificate Hold",
                             "cessation_of_operation": "Cessation of Operation",
                             "common_name": "Common Name",
+                            "download": "Download",
                             "expires_on": "Expires On",
                             "fingerprints": "Fingerprints",
                             "find_issuedon_from": "Issued on from",
diff --git a/ipaserver/plugins/internal.py b/ipaserver/plugins/internal.py
index c44c260f542428a76ff81c7119efefef4b5da492..de0587fc9f3d6c1fc0b7e3a4ededeebb72706bf2 100644
--- a/ipaserver/plugins/internal.py
+++ b/ipaserver/plugins/internal.py
@@ -367,12 +367,14 @@ class i18n_messages(Command):
                 "aa_compromise": _("AA Compromise"),
                 "add_principal": _("Add principal"),
                 "affiliation_changed": _("Affiliation Changed"),
+                "ca": _("CA"),
                 "ca_compromise": _("CA Compromise"),
                 "certificate": _("Certificate"),
                 "certificates": _("Certificates"),
                 "certificate_hold": _("Certificate Hold"),
                 "cessation_of_operation": _("Cessation of Operation"),
                 "common_name": _("Common Name"),
+                "download": _("Download"),
                 "expires_on": _("Expires On"),
                 "find_issuedon_from": _("Issued on from"),
                 "find_issuedon_to": _("Issued on to"),
-- 
2.5.5

From 965779893708c064f39b78c903642ec897f9be92 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka <pvoma...@redhat.com>
Date: Fri, 10 Jun 2016 16:16:57 +0200
Subject: [PATCH 3/3] Extend caacl entity

There is new checkbox in adding new caacl which can set whether the ACL applies on all
CAs or not. Also there is a new table with CAs on which is current ACL applied. User
can add and remove CAs from this table.

Part of: https://fedorahosted.org/freeipa/ticket/5939
---
 install/ui/less/widgets.less            |  7 +++++
 install/ui/src/freeipa/plugins/caacl.js | 48 ++++++++++++++++++++++++++++++++-
 install/ui/src/freeipa/rule.js          | 21 ++++++++++++---
 install/ui/test/data/ipa_init.json      |  4 +++
 ipaserver/plugins/internal.py           |  4 +++
 5 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/install/ui/less/widgets.less b/install/ui/less/widgets.less
index 0f9bc8c171d5c35d955c6b55d2e95ee105c35159..56a3104624b489c3a1b7f829d7eadb6c44e90a3f 100644
--- a/install/ui/less/widgets.less
+++ b/install/ui/less/widgets.less
@@ -188,3 +188,10 @@ tbody:empty { display: none; }
       font-weight: bold;
     }
 }
+
+/**
+ * Moves message next to the rule radio button to the right in caacl plugin.
+ */
+.rule-radio-note {
+    float: right;
+}
diff --git a/install/ui/src/freeipa/plugins/caacl.js b/install/ui/src/freeipa/plugins/caacl.js
index 57343d1e45c2562e1492564c3394472ca9a0c06f..e101d25fb6a42273f8bf3d61d2692eaf69d4be0b 100644
--- a/install/ui/src/freeipa/plugins/caacl.js
+++ b/install/ui/src/freeipa/plugins/caacl.js
@@ -87,6 +87,17 @@ var spec = {
         fields: [
             'cn',
             {
+                $type: 'checkboxes',
+                name: 'ipacacategory',
+                options: [
+                    {
+                        label: '@i18n:objects.caacl.all',
+                        value: 'all'
+                    }
+                ],
+                tooltip: '@i18n:objects.caacl.no_ca_msg'
+            },
+            {
                 $type: 'textarea',
                 name: 'description'
             }
@@ -245,6 +256,18 @@ var add_caacl_details_facet_widgets = function (spec) {
             name: 'memberservice_service',
             widget: 'who.service.memberservice_service',
             priority: IPA.caacl.remove_method_priority
+        },
+        // ca
+        {
+            $type: 'radio',
+            name: 'ipacacategory',
+            widget: 'who.ipaca.ipacacategory'
+        },
+        {
+            $type: 'rule_association_table',
+            name: 'ipamemberca_ca',
+            widget: 'who.ipaca.ipamemberca_ca',
+            priority: IPA.caacl.remove_method_priority
         }
     );
 
@@ -350,13 +373,36 @@ var add_caacl_details_facet_widgets = function (spec) {
                             remove_title: '@i18n:association.remove.member'
                         }
                     ]
+                },
+                {
+                    $factory: IPA.rule_details_widget,
+                    name: 'ipaca',
+                    radio_name: 'ipacacategory',
+                    note: '@i18n:objects.caacl.no_ca_msg',
+                    options: [
+                        { 'value': 'all', 'label': '@i18n:objects.caacl.any_ca' },
+                        { 'value': '', 'label': '@i18n:objects.caacl.specified_cas' }
+                    ],
+                    tables: [
+                        { 'name': 'ipamemberca_ca' }
+                    ],
+                    widgets: [
+                        {
+                            $type: 'rule_association_table',
+                            id: 'caacl-ipamemberca_ca',
+                            name: 'ipamemberca_ca',
+                            add_method: 'add_ca',
+                            remove_method: 'remove_ca',
+                            add_title: '@i18n:association.add.member',
+                            remove_title: '@i18n:association.remove.member'
+                        }
+                    ]
                 }
             ]
         }
     );
 };
 
-
 /**
  * CAACL entity specification object
  * @member plugins.caacl
diff --git a/install/ui/src/freeipa/rule.js b/install/ui/src/freeipa/rule.js
index 706827190261efda136f6d1489bdb13543c00f7a..0f39d434f82e2f33c329551c14fa0ce7b248b8e4 100644
--- a/install/ui/src/freeipa/rule.js
+++ b/install/ui/src/freeipa/rule.js
@@ -24,11 +24,12 @@ define([
     './phases',
     './reg',
     './rpc',
+    './text',
     './details',
     './search',
     './association',
     './entity'],
-        function(IPA, $, phases, reg, rpc) {
+        function(IPA, $, phases, reg, rpc, text) {
 
 IPA.rule_details_widget = function(spec) {
 
@@ -40,6 +41,7 @@ IPA.rule_details_widget = function(spec) {
     that.options = spec.options || [];
     that.tables = spec.tables || [];
     that.columns = spec.columns;
+    that.note = spec.note;
 
     that.init = function() {
 
@@ -47,7 +49,8 @@ IPA.rule_details_widget = function(spec) {
             name: that.radio_name,
             options: that.options,
             entity: that.entity,
-            css_class: 'rule-enable-radio'
+            css_class: 'rule-enable-radio',
+            note: that.note
         });
 
         that.widgets.add_widget(that.enable_radio);
@@ -85,6 +88,11 @@ IPA.rule_radio_widget = function(spec) {
     spec = spec || {};
     var that = IPA.radio_widget(spec);
 
+    /**
+     * The text from note will be displayed after radio buttons.
+     */
+    that.note = spec.note || '';
+
     /** @inheritDoc */
     that.create = function(container) {
 
@@ -97,6 +105,13 @@ IPA.rule_radio_widget = function(spec) {
         if (that.undo) {
             that.create_undo(container);
         }
+
+        if (that.note) {
+            $('<div />', {
+                text: text.get(that.note),
+                'class': 'rule-radio-note'
+            }).appendTo(container);
+        }
     };
 
     return that;
@@ -274,4 +289,4 @@ phases.on('registration', function() {
 });
 
 return {};
-});
\ No newline at end of file
+});
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 053940d10779ad5286df55380ed5d370c6ba96f4..6fd75b826cdd37cbaecb9a458985e9ea4cfedfce 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -214,12 +214,16 @@
                             "map_type": "Map Type"
                         },
                         "caacl": {
+                            "all": "All",
+                            "any_ca": "Any CA",
                             "any_host": "Any Host",
                             "any_service": "Any Service",
                             "any_profile": "Any Profile",
                             "anyone": "Anyone",
                             "ipaenabledflag": "Rule status",
+                            "no_ca_msg": "If no CAs are specified, requests to the default CA are allowed.",
                             "profile": "Profiles",
+                            "specified_cas": "Specified CAs",
                             "specified_hosts": "Specified Hosts and Groups",
                             "specified_profiles": "Specified Profiles",
                             "specified_services": "Specified Services and Groups",
diff --git a/ipaserver/plugins/internal.py b/ipaserver/plugins/internal.py
index de0587fc9f3d6c1fc0b7e3a4ededeebb72706bf2..51b220598e24be7f027cb2659d3bedcf2c268203 100644
--- a/ipaserver/plugins/internal.py
+++ b/ipaserver/plugins/internal.py
@@ -351,12 +351,16 @@ class i18n_messages(Command):
                 "indirect": _("Indirect"),
             },
             "caacl": {
+                "all": _("All"),
+                "any_ca": _("Any CA"),
                 "any_host": _("Any Host"),
                 "any_service": _("Any Service"),
                 "any_profile": _("Any Profile"),
                 "anyone": _("Anyone"),
                 "ipaenabledflag": _("Rule status"),
+                "no_ca_msg": _("If no CAs are specified, requests to the default CA are allowed."),
                 "profile": _("Profiles"),
+                "specified_cas": _("Specified CAs"),
                 "specified_hosts": _("Specified Hosts and Groups"),
                 "specified_profiles": _("Specified Profiles"),
                 "specified_services": _("Specified Services and Groups"),
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to