On Wed, Mar 28, 2012 at 11:24, Vincent Massol <vincent(a)massol.net> wrote:
Hi Denis,
On Mar 28, 2012, at 11:04 AM, Denis Gervalle wrote:
[snip]
> Denis, do you plan to refactor some existing
code to use this new API?
IMO
this
would be the best next step to prove that it works.
It works, I use it everyday ! Their minor differences only impact really
special usage.
We all know the "it works on my machine" syndrome but that doesn't prove
anything… :)
As I said in another mail we really need a very code unit test coverage.
Fo you
have any idea about what code could be migrated?
IMO, we may start using it for new components, or components that we are
refactoring, since it will allow being independent of the old core for
these component. This module may works for a while in parallel with the
existing service.
I was asking for an example of what we could start with. Do you have any
idea?
By merging it, and providing it as an option, it
will allows more user to
try it. There is a bridge between the old service and this new one that
you
may simply activate in xwiki.cfg. This new
service improve performance,
allow adding more rights on the fly and allow customizing the way it
works,
so it is an answer to those wanting these
improvements.
We need to be careful not to keep this new
security module not used for
too long as otherwise it could "rot".
It will not stay unused on my side, so be sure I will stay close to it,
since I have written it because I need those improvements. I really
encourage anyone to try it !
So IMO we need to start using it ASAP and then
have a plan for switching
to it.
IMO, the switch will be welcome when we have discussed the small
differences it has, and fix the few we do not tolerate. In particular,
the
startup of an empty wiki require login has
superadmin with this new
module.
I know this require more documentation to be
written, I am really aware
of
that, this just require time to be done... hopes
Andreas and others could
helps as well.
For me you are now responsible for this part (even though technically
we're all responsible of course since we use no code ownership ;) - but for
example Thomas and I know more the Rendering and are fixing stuff there,
while others have more knowledge in other parts).
Have I said the contrary ? All I said is that I will do my best supporting
it, but this is really a large part, and some help is also welcome. I have
taken the time to have it at the current state in the hope we will be able
to move out of the crappy service we use currently more that 20 times per
requests. It is now mature enough to start using it, since we do, and...
I understand you wanted this in the platform because
you're using it
already.
...no, I am not so concerned about having it in platform for using it. I
now trust my module more than the existing implementation. I would like to
have it in platform to encourage others to look at it, use it, adopt it and
contribute to it, since I am really confident it is really better than the
current implementation.
I just want to be sure we're going to maintain it
since I can see it'll
require a lot more work before being to full switch to this implementation.
IMO, there is even more work to do to use the current right service, since
its own test cover only 52% of the right service itself and the
documentation is really sparse and almost unwritable ! (BTW, these tests
cannot be used on our bridge since these use implementation method that are
not published in the interface.)
Thanks
-Vincent
> Thanks
> -Vincent
>
> On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
>
>> Hi all,
>>
>> As you have probably notice, I have recently committed an
>> feature-security-authorization branch on platform. I am working on this
> for
>> a while now and it was the first step to share the outcome of this
large
>> refactoring of the initial work done
early last year by Andreas. Since
> the
>> code was not quality compliant with platform but the general structure
>> Andreas has build seems to me well appropriate, I have progressively
>> refactor its code to better fit our real needs. Here is what I have
been
>> done:
>>
>> 1) Split in to module api and bridge to allow breaking the currently
>> unavoidable dependency on oldcore. Now only bridge depends on oldcore,
> and
>> the api does not depends on bridge. As mush as possible has been
written
> in
>> the api (still some code to migrate), and some temporary internal
bridge
>> are used to access oldcore stuffs since
augmenting the existing
>> document-bridge does not seems appropriate IMO.
>>
>> 2) The initial enumeration of rights as been replaced by a Right class,
>> which could be seen has a pseudo enum, but could be augmented with new
>> rights. To register a new Right, you have to provide a RightDescription
> to
>> the AuthorizationManager. The description will define the default
state,
>> the tie resolution policy, the
inheritance policy, the list of entity
> types
>> for which the right is applicable, the implied rights and if the right
>> could be allowed in read-only mode. So new defined Right will benefit
the
>> whole logic of the AuthorizationManager
and currently existing one
could
> be
>> declaratively defined.
>>
>> 3) Large renaming to better distinguish stuffs, clarify comments and
>> prepare for future. I have voluntarily not taken existing names to
> clearly
>> split the old and the new api. In brief, the new right service is now
> named
>> AuthorizationManager. Internally, it manipulates SecurityReference (as
> well
>> as UserSecurityReference and GroupSecurityReference, to represent
> entities,
>> user and group), SecurityRule (representing a right object) and
>> SecurityAccess (representing an access level in the old nomenclature),
>> which are store in a SecurityCache using SecurityRuleEntry (a set of
> rules)
>> and SecurityAccessEntry (the access of a given user). The
>> AuthorizationManager delegate cache management to a SecurityCacheLoader
>> which loads rules using a SecurityRuleLoader ; and delegate itself the
>> computing of the access for a given user and a set of rules to an
>> AuthorizationSettler. This last one could be overridden to provide
> specific
>> decision that could not be done in declarative mode.
>>
>> 4) Refactoring was necessary to improve consistency and reduce
> complexity,
>> and simplify as much as possible; while extending the limitations to
> allow
>> more rights to be registered. This work has been a little bit opposed
to
>> the optimization done by Andreas, in
particular on memory usage. But
>> optimization is often the enemy of clean code.
>>
>> 5) Improvement were necessary to better mimic the existing
implementation
>> in some peculiar but necessary rules to
stay compatible with current
>> working wiki. I tend to reduce as much as possible what is not done
>> declaratively, but there are still some special cases, like delete for
>> creators, deny for other user on explicit allow and admin for wiki
owner
>> that are settle by the authorization
settler. My implementation should
be
>> almost compatible with the old one,
except for groups that are
currently
>> not checked from the entity wiki, but
only from the user wiki. This
needs
>> some more refactoring for which I feel
inconfortable with, some I'd
like
> to
>> share first.
>>
>> 6) The AuthorizationManager interface has been simplified, providing 2
>> methods for either checking or verifying an access right (the checking
>> methods throws while the verifying one return a boolean), and one to
>> register a new right.
>>
>> The existing RightService could be bridged on the new implementation
> using
>> the XWikiCachingRightService class in xwiki.cfg and the new API could
be
>> used side-by-side with the old
implementation as well. What should
still
>> really need to be improved is the unit
testing, currently some tests
are
>> still awful and incomplete. I already
refactor some of them, to
provide a
>> better coverage of essential part of the
code: the security cache and
the
>> default authorization settler. Obviously,
any help is welcomed.
>>
>> Since I already have an existing wiki using this implementation
>> successfully and using it for creating new rights for extensions, I
would
>> like to merge this new implementation as
experimental in platform to
have
>> it available for anyone who need it or
want to test it, and for you to
> use
>> in your new experimental development as well. Providing it in platform
> will
>> encourage it to be finalized and replace the existing implementation.
>>
>> Here is my +1 for the merge on 4.x,
>>
>> WDYT ?
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Denis Gervalle
SOFTEC sa - CEO
eGuilde sarl - CTO