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