[SAK-27501] Roster 2 require roster.viewallmembers or roster.viewgroup to work Created: 17-Jul-2014  Updated: 11-Dec-2014  Resolved: 30-Oct-2014

Status: CLOSED
Project: Sakai
Component/s: Roster
Affects Version/s: 10.2, 11.0
Fix Version/s: 10.3, 11.0

Type: Bug Priority: Blocker
Reporter: Juan José Meroño Sánchez Assignee: Adrian Fish
Resolution: Fixed Votes: 0
Labels: PATCH_ADDED, UMUtoSAK10, s2u-work
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File SAK-27501-code.patch     Text File SAK-27501.patch     JPEG File emptyroster.JPG    
Issue Links:
Relate
relates to SAK-28886 Roster2 permission checks need to be ... RESOLVED
is related to SAK-26265 Roster2 shows empty list in first att... CLOSED
10 status: Resolved

 Description   

Steps to reproduce:

1. Create a course site with at least 2 sections.
2. Add roster tool.
3. Login as an student of one section.
4. You won't be able to see roster.

If you grant roster.viewgroup permission to student role then you see the list. I think this permission should be granted by default in Sakai OOTB.
It has no sense grant roster.viewallmembers to student or even instructor.



 Comments   
Comment by Juan José Meroño Sánchez [ 02-Oct-2014 ]

This patch adds roster.viewgroup permission to Student role in !site.template.course. It also be desirable to add something in conversion scripts to modify this template, and check if nothing is affected by this grant.

Other solution could be change the way roster2 is checking permissions, what do you think Adrian?

Comment by Matthew Buckett [ 09-Oct-2014 ]

The permissions Roster2 currently register are:

roster.viewallmembers
roster.viewhidden
roster.export
roster.viewgroup
roster.viewenrollmentstatus
roster.viewprofile

The code uses roster.viewallmembers to see if the user should be allowed to see the members of the site. org.sakaiproject.roster.impl.SakaiProxyImpl#getRosterSite() but roster also uses the same permissions to assume that you should be able to see the members of all groups.

it seems like there should be a way to allow people to see all the members of the site without having to grant them access to all groups.

Comment by Juan José Meroño Sánchez [ 23-Oct-2014 ]

When a student is enrolled in just one section within a site, Roster1 did not need any permission in the site realm to show the members of his group, it was only checking the group realm looking for the roster.viewgroup permission in order to show the members of the group. This usecase is not working with roster2.

For users with 'roster.viewallmembers' in the site realm, roster1 and roster2 act the same way. The user is able to see all members of the site.

Matthew, do you think that this should be solved by fixing the code rather than adding a new permission to some roles that right now are not able to see anything in roster2 (like students in an OOTB SAKAI)?

Thanks in advance.

Comment by Matthew Jones [ 23-Oct-2014 ]

I feel like the change in RSTR-57 is breaking this. After RSTR-57, it does a precheck in getRosterSite which requires these two permissions in order to work.

Otherwise it goes through and when it calls getViewAbleSiteGroups it actually returns the correct groups based on the old roster permissions. Removing this pre-check makes it work as expected for this issue, even removing the permissions that are added to the group realm(s).

I can't figure out any reason why it's here, but if needed it should be trusting the check in getViewableSiteGroups. as that also does this same security check. (SAK-27501-code.patch)

Comment by Juan José Meroño Sánchez [ 29-Oct-2014 ]

Thanks Matthew !! I'm going to test your patch and apply if no more comments are added here.

Comment by Juan José Meroño Sánchez [ 30-Oct-2014 ]

Committed revision 315038

Comment by Hudson CI Server [ 30-Oct-2014 ]

SUCCESS: Integrated in sakai-trunk-java-1.7 #511 (See http://builds.sakaiproject.org:8080/job/sakai-trunk-java-1.7/511/)
SAK-27501 Roster 2 require roster.viewallmembers or roster.viewgroup to work (jjmerono@um.es: rev 315038)

Comment by Juan José Meroño Sánchez [ 31-Oct-2014 ]

Tested in trunk.

Comment by Juan José Meroño Sánchez [ 27-Nov-2014 ]

After SAK-28028 this is not working on trunk.

Comment by Hudson CI Server [ 27-Nov-2014 ]

SUCCESS: Integrated in sakai-trunk-java-1.7 #529 (See http://builds.sakaiproject.org:8080/job/sakai-trunk-java-1.7/529/)
SAK-27501

Reapplied Juanjo's patch (a.fish@lancaster.ac.uk: rev 315716)

Comment by Neal Caidin [ 01-Dec-2014 ]

Does this need an updated patch? Are we still trying to get this into 10.3 ?

Comment by Juan José Meroño Sánchez [ 01-Dec-2014 ]

This has to be merged again (r315716) only if SAK-28028 (new roster2 UI) is merged into 10.x, but not for 10.3, is it right?
I think we need to link both jiras to keep this in mind.

Comment by Neal Caidin [ 01-Dec-2014 ]

I removed the 10.3Triage label.

Comment by Juan José Meroño Sánchez [ 02-Dec-2014 ]

This was fixed for 10.3, later, it was broken in trunk (due to SAK-28028), and then fixed again for trunk (probably this should have been done in another jira but it was done in this one that was already closed). This will be broken in 10.x only if SAK-28028 is merged into 10.x, so I've closed the jira and changed fix versions to 11 and 10.3. Now is working in trunk and 10.x.

Comment by Neal Caidin [ 02-Dec-2014 ]

Thank you! That was mighty confusing.

Generated at Tue May 21 00:17:43 CDT 2019 using JIRA 7.5.0#75005-sha1:fd8c849d4e278dd8bbaccc61e707a716ad697024.