david at kineticode
Feb 26, 2009, 12:42 PM
Post #6 of 22
On Feb 26, 2009, at 12:16 PM, Adrian Yee wrote:
> David E. Wheeler wrote:
>> On Feb 25, 2009, at 3:26 PM, Adrian Yee wrote:
>>> The reason I don't think it's complete is that I think users
>>> should have read access to even add keywords that are already in
>>> the database. Not really a big change, but I'm sure there are
>>> some other quirks with how the permissions work that would need to
>>> be ironed out before it would be a commitable change.
>> Hrm. Why? If I grant READ access to All Users on the All Keywords
>> group, it covers that case, no?
> Sure, but my point is that when we're adding keywords to a story, it
> currently doesn't perform a check. My patch only makes the check
> when we're creating a new keyword.
Well, if it's in a story, they should be able to see it's part of the
story, and even delete it, if they have EDIT permission to the story.
That's how it works with Elements and Categories.
> Just stick another check for READ perms after the lookup (if we have
> a keyword) and that should do it.
If they're creating it, I don't see the point of that. I'm probably
just missing something, though.
> What about deletes? Should there be any permissions checking on
> deleting a keyword from a story/media/category? I guess you're not
> really changing a keyword, so that's out of the scope of keyword
Correct. Right now, if you're editing a story, you can edit or delete
any of the categories or elements associated with that story, even if
you don't have READ access to the underlying element types or
categories. That might be something to change at some point, but it's
a separate issue. You're just bringing keyword association into
alignment with element and category association.
Oh, but you should probably disable the inputs for adding keywords if
users don't have permission to create keywords, FWIW. That way it
shouldn't really even get to the point where you're checking the
permission now (though you still should, of course).
Also, the same checks should be put into the SOAP interface, for
> The nice thing about using the created keyword object is that you
> get a nicer error:
> You have not been granted access to the “foo bar” Keyword
> instead of:
> You have not been granted access to the “” Keyword
> But perhaps we should be returning a less cryptic error?
Yes. If it's a CREATE error, it should be "You have not been granted
permission to create keywords." Or something like that.
>> Are permissions already checked in the keyword profile?
> Yep. Assuming by keyword profile, you're referring to the Admin =>
> Publishing => Keywords interface (sorry, I'm not up on the Bricolage
> lingo yet).
Yes, exactly (/admin/profile/keyword is the URI, IIRC).