On Sun, Mar 9, 2008 at 2:56 PM, David Jencks <[EMAIL PROTECTED]> wrote:
On Mar 9, 2008, at 10:17 AM, Dave wrote:
> I don't understand why we need to pull this out into a separate
> interface. Why not just leave it in UserManager? That way, somebody
> only has to implement one interface to plugin their own
> User/Permissions manager.
>
My thinking, IIRC, is that the UserManager interface makes a number
of specific assumptions about the relationship between users and
permissions that may not hold with other security backends such as a
hierarchical rbac model. With a separate SecurityService interface
we can start to separate permission evaluation, which is pretty
universal, with user provisioning, which may not be so universal. I
don't recall looking into what the actual dependencies between the
front end and UserManager are so the prospect of using an entirely
different user manager that does not map well to the UserManager
interface may be harder than I dream of :-)
>> UserSource is the abstraction of what is tracking the current user.
>> Basically it attempts to avoid looking up the current User object
>> unless it's really necessary. For instance with a JACC based
>> authorization system the security service would already know the
>> current user from the container login and would not need to consult
>> the UserSource.
>
> Sounds good, but you've got UserSource in the front-end and you've got
> back-end classes depending on it. UserSource should be in the
> org.apache.roller.weblogger.business package.
ok.... <harp on sore point> if you just had a reasonably structured
maven build I would have realized this from the structure </harp on
sore point>
Is that Maven patch still up to date. I might like to have a second look at it.
>> Here's a brief description of what it can do now and what might be
>> simplified:
>>
>> - name. This is adapted from the URLPattern handling of
>> UserDataPermission. We don't need exclusions so there's only one
>> pattern, which acts like URL patterns in web security constraints.
>> Currently global permissions get "/*" and permissions specific to a
>> particular blog, say "foo", get "/foo". This could be simplified a
>> little bit more, but what is there now allows hierarchical
>> categorization of blogs. For instance one might organize blogs
>> under /internal and /external: it would then be possible to give
>> permissions to categories of blogs, say /internal/*. I thought it
>> would be worth asking if this sounded interesting before removing
>> the
>> code that lets you do this.
>
> From looking at the code, I cannot understand how "hierarchical
> categorization of blogs" works.
This is just an idea, supported by the current proposed permission
implementation: it would require some support in the rest of roller
to actually be usable. The idea is that blog handles could be
hierarchical like
/myco/mydept/myblog
Then you could give someone a permission for all myco blogs, or all
myco/mydept blogs, or just the myco/mydept/myblog itself.
That does sound useful, but as things stand in 4.1 object (e.g.
weblog) permissions are assigned by object ID and not by URL path. Are
you suggesting a mix of ID based and URL based?
Also, it sounds like something that would be useful to a blog site
administrator but I'd really like to allow users to control the
privacy settings of their blogs. Blogs should be as self-service as
possible.
Ideally, I'd like to allow users to set blog privacy based on who are
their friends and who belongs to their social groups. But to do that
we need more "social graph" support in Roller. There maybe some
synergies here with the Apache Shindig project.
>> - actions. This is adapted from the HTTPMethod handling of
>> UserDataPermission. This is probably significantly more complicated
>> that necessary, but my questions as to what is needed have so far
>> gone unanswered. The actions I've found in the existing code
>> ("admin", "post", "editdraft", "weblog", "login") are represented in
>> a bitmask. Any additional actions are stored as strings.
>> There's an
>> "isExcluded" flag that indicates whether the set of actions
>> explicitly listed (in the mask or as strings) is the set of granted
>> actions or the set of denied actions. Thus any finite set of
>> actions
>> or the complement of any finite set of actions can be
>> represented. I
>> strongly suspect that there is a known finite set of actions so a
>> bitmap would be sufficient. I'm hoping someone can explain whether
>> or not this is the case.
>
> I do not understand the need for a bitmask here. Why not store all
> actions as simple strings? This seem to overcomplicate things to an
> extent that I prefer the previous code.
I'm waiting for an answer to my question about whether all the
possible actions are known right now :-) If they are, I think the
argument for a bitmap is stronger. Having both a bitmap and a map
for extensions is a leftover from the implementation I was
simplifying, and I was hoping to remove the extensions when I found
out how many actions there might be.
I don't think we know all the possible actions right now. As new
features are added the list could grow and that's part of the reason
that I moved from a mask in Roller 4 to list of action strings.
>> Open questions:
>> - as already mentioned, I'd like to know what actions are possible.
>> - I don't really understand the thinking behind the ORM for
>> ObjectPermission. It doesn't look to me as if GlobalPermissions can
>> be persisted which I don't understand. In any case I suspect this
>> area might be possible to simplify.
>
> GlobalPermissions not persisted directly, they are implied by the
> roles that a user has.
> See the original proposal for details:
> http://cwiki.apache.org/confluence/x/PfM
I'll have to think some more about whether I think this is a reasonable model.
Please do.
> And some general comments:
>
> The mix of bug fixes and architectural changes is a little difficult
> to parse, can we separate those out?
I doubt it -- most of the bug fixes are a consequence of the new
permission implementation.
Fair enough.
> There are a fair number of formatting changes which make it difficult
> to evaluate the patch. Can we limit the patch to substantive changes
> only and leave formatting out of the patch?
I'll see what I can come up with. I already did remove some of the
formatting changes -- I'm afraid a lot of changes happened when I
made a bunch of changes and told my IDE to reformat the whole class.
Yep. That happens to the best of us ;-)
> For the reasons above, I don't feel comfortable about committing this
> patch. Generally, the changes here are complex and sweeping enough to
warrant a full proposal along the lines of
> http://cwiki.apache.org/confluence/x/PfM.
OK, I'll see what I can come up with. Your thoughts on whether
hierarchical blog names have any value and on whether the set of
actions is known and finite would help in simplifying the next
version of the patch.
- Dave
