On 14.06.2016 18:30, Petr Vobornik wrote:
On 06/14/2016 10:17 AM, Pavel Vomacka wrote:

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.
Also ACK from Web UI internals perspective.

Patch 0045-2: ACK

Patch 0046-2: ACK, could be split into two patches but don't bother...

Patch 0047-3: ACK

sub-CA patches were pushed, we can push this
pushed to master:
* 6e78169e3bc71c5ff3824e91cce8a0d6d9580d7a Add new webui plugin - ca
* f4dd2446cd8b2c2a814c7bcb95415eb186b0a70f Extend certificate entity page
* 5e5df4abf037161d9c9d9fd5e6051f861dff4bd1 Extend caacl entity

--
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