Students are removed from groups when added by official rosters
Description
Attachments
depends on
relates to
Activity
Hudson CI Server June 20, 2014 at 6:28 PM
Integrated in sakai-trunk-java-1.7 #291 (See http://builds.sakaiproject.org:8080/job/sakai-trunk-java-1.7/291/)
set authz.synchWithContainingRealm=false because of issues found in KNL1250 (Revision 310333)
Result = SUCCESS
Hudson CI Server June 20, 2014 at 4:23 PM
Integrated in sakai-10-java-1.7 #88 (See http://builds.sakaiproject.org:8080/job/sakai-10-java-1.7/88/)
merge 310333 to 10.x (Revision 310335)
Result = SUCCESS
Sam Ottenhoff June 20, 2014 at 11:07 AM
authz.synchWithContainingRealm=false in trunk with commit r310333
has been reverted... commit was done against ticket.
Please open new JIRAs for new patches.
Paul Lukasewych June 20, 2014 at 8:05 AMEdited
I think now that there are more problems with 1250. There are implicit conditionals in the original code that cause it not to run on realms that have no provider id, and I am not accounting for them in 1250.
For example, in the original code, when they loop over the existing realm grants that are provided, I think there won't be any such grants if the realm does not have a provider. So any code in that loop doesn't even run on realms with no provider. I definitely missed this one.
There are also implicit conditional when iterating over target and in the promotion to provided code because it only runs if existingRole == targetRole, and targetRole is always null if the realm has no provider.
In 1250 I tried to solve all the problems described in KNL-800. I think this can be done for syncing role and inactive state, but provided state is more complex and needs to take into account whether or not the realm is provided. I also think there might be other bugs in 1250 so I think the safe course of action is to revert it. If you want 10.0 out the door, disabling the property as Matt suggests makes sense to me.
I can work on a different patch that just focuses on fixing the problems with the insert statements in the patch and does not try to fix all the problems described in the ticket..
Matthew Buckett June 20, 2014 at 5:09 AMEdited
Or to get 10 out the door could we just disable the feature introduce in with authz.synchWithContainingRealm=false being the default and add a warning that there is a know bug with this feature and it shouldn't be used until fixed?
If I set authz.synchWithContainingRealm=false and repeat the same test plan my test student remains in the group after logging in.
To reproduce:
Part A
Create a site with provided users from a roster (with demo=true or on a nightly using a course site)
Add one of thsee provided user to the group in the class (like student0001)
Login as admin, clear the cache (The student still will show up in the group and work temporarily from the cache)
Login as the provided user, when refreshUser is called their grants are updated
Logout from that user, login as instructor again, go to look at the group
Observed: User is removed from that group
Expected: User is still in the group