[Freeipa-devel] [PATCH] 0045-47: webui: Sub-CAs

[Petr Vobornik]

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

-- 
Petr Vobornik