[Freeipa-devel] [DESIGN][UPDATE] Time-Based HBAC Policies

[Standa Laznicka]

On 08/19/2016 04:06 PM, Martin Basti wrote:
> On 19.08.2016 12:37, Pavel Vomacka wrote:
>> On 08/16/2016 08:21 AM, Stanislav Laznicka wrote:
>>> On 08/12/2016 06:48 PM, Petr Spacek wrote:
>>>> On 11.8.2016 12:34, Stanislav Laznicka wrote:
>>>>> Hello,
>>>>>
>>>>> I updated the design of the Time-Based HBAC Policies according to the
>>>>> discussion we led here earlier. Please check the design page
>>>>> http://www.freeipa.org/page/V4/Time-Based_Account_Policies. The 
>>>>> biggest
>>>>> changes are in the Implementation and Feature Management sections. 
>>>>> I also
>>>>> added a short How to Use section.
>>> Thank you for the review! I will add some comments inline.
>>>> Nice page!
>>>>
>>>> On the high level it all makes sense.
>>>>
>>>> ad LDAP schema
>>>> ==============
>>>> 1) Why accessTime attribute is MAY in ipaTimeRule object class?
>>>> Does it make sense to have the object without accessTime? I do not 
>>>> think so.
>>> My idea was that we allow users prepare a few time rule objects 
>>> before filling them with the actual times.
>>>> Also, it could be good to add description attribute to the object 
>>>> class and
>>>> incorporate it into commands (including find).
>>>>
>>> Definitely a good idea, I will work that in.
>>>> 2) Besides all this, I spent few minutes in dark history of IPA. The
>>>> accessTime attribute was introduced back in 2009 in commit
>>>> "55ba300c7cb59cf05b16cc01281f51d93eb25acf" aka "Incorporate new 
>>>> schema for IPAv2".
>>>>
>>>> The commit does not contain any reasoning for the change but I can 
>>>> see that
>>>> the attribute is already used as MAY in old object classes 
>>>> ipaHBACRule and
>>>> ipaSELinuxUserMap.
>>>>
>>>> Is any of these a problem?
>>> I believe that the accessTime attribute was originally brought to 
>>> IPA when there was an implementation of time policies for HBAC 
>>> objects and it's been rotting there ever since those capabilities 
>>> were removed. We may eventually use a new attribute for storage of 
>>> the time strings as accessTime by definition is multi-valued which 
>>> is not what's currently desired (although we may end up with it some 
>>> day in the future). However, I don't think any other use of 
>>> accessTime should be a problem as it's been obsoleted for a long time.
>>>> Why is it even in ipaSELinuxUserMap object class?
>>> I'm sorry to say I have no idea. I used it for what it originally 
>>> was - a means for storing time strings at HBAC rules.
>>>> Commit
>>>> 55512dc938eb4a9a6655e473beab587e340af55c does not mention any 
>>>> reason for doing so.
>>>>
>>>> I cannot see any other problem so the low-level stuff is good and 
>>>> can be
>>>> implemented.
>>>>
>>>>
>>>> ad User interface
>>>> =================
>>>> We need to polish the user interface so it really usable.
>>>>
>>>> At least the web interface should contain some shortcuts. E.g. when 
>>>> I'm adding
>>>> a new HBAC rule, the "time" section should contain also "something" to
>>>> immediately add new time rule so I do not need to go to time rules 
>>>> first and
>>>> then go back to HBAC page.
>> I'm definitely for creating a field where admin can choose a existing 
>> time rule when creating a new HBAC. But I'm not sure about 
>> possibility to create and define new time rule in dialog for creating 
>> new HBAC. I think that mixing these two things together is like a 
>> possibility to create a new user when you are creating a user group. 
>> Which is mixing two different things together. I can imagine a button 
>> like "Create HBAC and add a new time rule to it" which could store 
>> new HBAC rule and immediately take admin to the page (or dialog) 
>> where admin can create a new time rule with prefilled HBAC rule. But 
>> as you write below it would be good to discuss it with some UX designer.
>
> I'm not UX guru, but if you add button there and show dialog window to 
> create new timerule and then automatically assign it to the HBACrule 
> it may work for me :)
>
>>>>
>>>> Similarly, dialog for rule modification should allow to easily 
>>>> change all the
>>>> values, warn if time rules is shared, and also have an easy way to
>>>> 'disconnect' the time rule, i.e. make a copy of it and edit only 
>>>> the new copy
>>>> (instead of the shared original).
>>>>
>> All of these points are really good.
>
>>>> All these are user interface things not affecting the low-level stuff.
>>>>
>>>>
>>>> Maybe you should sat down with some UX designer, talk about these 
>>>> cases and
>>>> draw some hand-made pictures.
>>> I will add Pavel V. to CC, we may want to discuss this.
>>>> I do not believe that this will require any changes in schema so 
>>>> you can
>>>> polish SSSD and framework implementation in meantime.
>>>>
>>>> On the link below is a PROTOTYPE-patched FreeIPA that covers most 
>>>> of the CLI
>>>> functionality (except for the creation of iCalendar strings from 
>>>> options) for
>>>> better illustration of the design.
>>>>
>>>> https://github.com/stlaz/freeipa/tree/timerules_2
>>>> Honestly I did not look at the code today :-)
>>>>
>>>> Overall, I'm glad to see current proposal. After so many iteration, 
>>>> we reached
>>>> something which does not have any glaring problem :-)
>>> It definitely felt better to be writing it with all the previous 
>>> knowledge. Thank you! :-)
>>
>
> LGTM with all previous comments
Thank you for the review, my comments are inline
>
>
> (Nitpick mode enabled: True)
> 1.
> It may not be clear from design that client is actually SSSD, and not 
> IPA CLI client. Please write explicitly there that HBAC time rules are 
> enforced by SSSD with libical in client side sections
Did not realize that, I will add the information.
>
> 2.
> Is there any design for SSSD part? If yes, it should be linked to this 
> (probably client section)
There's currently no design for the SSSD part. I think the 
implementation there might be straight forward enough - add caching of 
the new LDAP subtree, get the time rules from it, evaluate them at the 
given HBAC rules.
>
> 3.
> timerule-mod, timerule-show should show all HBAC rules that are using 
> it (Should be done by framework almost automatically, but explicit is 
> better than implicit, please make note there).
>
I believe framework should be already doing that but I'll check.
> 4.
> timerule-del should prevent deletion of timerule if it is used by any 
> HBAC rule (we can discuss this)
Perhaps just a prompt with warning showing all affected HBAC rules could 
do (could it?), although I am not sure if that is kosher within the 
framework.
>
> 5.
> WebUI: it may be nice to have warning shown when user is editing time 
> rule that is used by more than one HBACrule (even confirmation dialog 
> would be nice)
>
Should something like that be also in the CLI (similarly to previous point)?
> (Nitpick mode enable: False)
> 6.
> IMO we should add timerule-test (pick correct name) to test if given 
> time/date value matches timerule
That seems like a reasonable idea.
>
> 7.
> We should also extend hbac*-test with timerules (would be nice to 
> mention in design) 
Definitely, I will add that to the design, I completely forgot about 
hbac*-test there.