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.

Patch 45:

1) In the main `Certificate Authorities' table, `Subject DN' is
showing the DN of the IPA object, instead of the Subject DN.

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.

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

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.

Cheers,
Fraser

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