also, isn't it incorrect to use WD.canSave() to validate access to the
APP endpoint at all? for example, RollerAtomHandler.postEntry() calls
canEdit(), which calls WD.canSave(). this call should succeed if the
user has AUTHOR only rights, correct?
Jeff Blattman wrote:
okay ... then the code in RollerAtomHandler is incorrect then. the APP
code is rejecting GET access to the weblog if the user has only
authoring rights. i do not think this is correct. the impl of
RollerAtomHandler.canView() calls canEdit(), which calls WD.canSave()
... ?
i agree, a user shoud not be able to modify WD w/ only authoring
rights. however, consider the correctness of canSave() and
hasUserPermissions(). canSave() calls
hasUserPermissions(AUTHOR|ADMIN). if AUTHOR has nothing to do w/ the
check, then why is it being tested in the call? also, if the intent of:
hasUserPermissions(AUTHOR|ADMIN)
to check if the user has AUTHOR or ADMIN, or AUTHOR and ADMIN? if it's
the latter, then the code is correct. if it's the former, then the
code is wrong.
firstly, i would suggest that WD.canSave() should be modified to pass
in only the ADMIN flag, not AUTHOR. secondly, the implementation of
canView() in RollerAtomHandler needs to change. i am not exactly sure
what to check here. a blog can be viewed by anyone, right? so why do a
check at all in that case?
or, is it the case that you expect that the APP endpoint is only for
admin access, and clients should go through the atom endpoint for
read-only access? i hope not. that would require APP clients to be
configured w/ two endpoints.
thanks.
David M Johnson wrote:
On closer inspection, I think the existing code is correct.
Authors are not allowed to do any blog configuration, change
templates, etc. So authors should not be able to change the website
data object.
- Dave
On Feb 28, 2006, at 2:27 PM, Jeff Blattman wrote:
dave,
i've attached the patch file of what i think this should be. is
there a roller bug / issue tracker system that i can attach this to?
or can someone here make this change?
thanks.
David M Johnson wrote:
Yes, I think that is incorrect. All you should need is AUTHOR access.
- Dave
On Feb 27, 2006, at 6:10 PM, Jeff Blattman wrote:
we have a user with AUTHOR permissions on a weblog. to see if the
user can access the weblog, RollerAtomHandler calls
WebsiteData.canEdit(), passing in the user. this calls WD.canSave().
WD.canSave() calls hasUserPermissions(...,
PermissionsData.ADMIN|PermissionsData.AUTHOR).
so, in hasUserPermissions(), mask == ADMIN|AUTHOR == 0x01|0x03 ==
0001|0011 == 0011 == 0x03.
in hasUserPermissions(), we get to this block:
/ if (userPerms != null &&
(userPerms.getPermissionMask() & mask) == mask)
{
return true;
}/
the user's permission mask is 0x01 == AUTHOR. so,
userPerms.getPermissionMask() & mask == 0x01 & 0x03 == 0001 & 0011
== 0001 == 0x01 != mask. so, the check fails and the user is not
allowed to access the weblog.
this seems wrong, unless i am missing something. it seems like the
check should be:
/ if (userPerms != null &&
(userPerms.getPermissionMask() & mask) ==
userPerms.getPermissionMask()) .../
the important thing we want to check is that the user's permission
mask (bit) matches up with one of the bits in the mask. if it
does, the & result will be the same as the user's permission mask.
it looks like the present code is instead checking is the user has
ADMIN and AUTHOR permission for the weblog, which i do not think
is correct ...
?
Index: org/roller/pojos/WebsiteData.java
===================================================================
--- org/roller/pojos/WebsiteData.java (revision 373624)
+++ org/roller/pojos/WebsiteData.java (working copy)
@@ -1011,7 +1011,8 @@
// if we found one, does it satisfy the mask?
if (userPerms != null && !userPerms.isPending())
{
- if (userPerms != null && (userPerms.getPermissionMask()
& mask) == mask)
+ short userMask = userPerms.getPermissionMask();
+ if ((mask & userMask) == userMask)
{
return true;
}
