[Building Sakai] BaseSiteService: is this a bug?

[Stephen Marquard]

Generally speaking I'd say this is a dubious approach.

In particular, it's not a good idea to introduce dependence on current tool context into the service implementation, as it can break use from non-tool contexts like web services, quartz jobs, etc. s a dubious approach.

If the only context you want people to be able to create a site is from within a specific tool, rather add a security advisor there.

Regards
Stephen
 
>>> "Zach A. Thomas" <zach at aeroplanesoftware.com> 03/17/09 9:36 PM >>> 
Hello, all. I'm working on some pseudo-hierarchical features of Site  
Info, and I ran into a snag which may or may not be BaseSiteService's  
fault.

The basic idea is that from within Site Info, the site maintainer can  
click to create a child site of that site. For permission, we give  
this role "site.add" within that site, but not globally. In other  
words, they do not have permission to create sites from Worksite Setup  
in "My Workspace."

Here's where the problem comes in: in BaseSiteService, public Site  
addSite(String id, Site other) performs the following permission check:
unlock(SECURE_ADD_SITE, siteReference(siteReference(id)));

"id" in this case is the site ID of the Site we're just about to  
create. Since it doesn't exist yet, there's no way for my user to have  
"site.add" on it. The only reason this works at all in Sakai is that  
the typical User has been granted "site.add" by their user type (e.g.  
"registered"). To prove to myself that the id passed into this check  
is irrelevant, I changed it to "foobar" and sure enough, the  
"registered" user passes the check, while my poor site maintainer is  
held to an impossible standard.

It does what I want if I change it to check the id of the site _from  
which_ I am making this call:
String currentSiteId = ToolManager.getCurrentPlacement().getContext();
unlock(SECURE_ADD_SITE, siteReference(siteReference(currentSiteId)));

This form of permission check also takes place in the  
allowAddSite(String id) and addSite(String id, String type) methods.

Would it be reasonable to patch this and merge it to the 2.5.x branch?  
I would add a check that getCurrentPlacement() is not null, but other  
than that, any concerns?

cheers,
Zach Thomas
Owner, Aeroplane Software
_______________________________________________
sakai-dev mailing list
sakai-dev at collab.sakaiproject.org
http://collab.sakaiproject.org/mailman/listinfo/sakai-dev

TO UNSUBSCRIBE: send email to sakai-dev-unsubscribe at collab.sakaiproject.org with a subject of "unsubscribe"