Skip to content

Commit

Permalink
SAK-47854: User whith Admin role should manage groups (sakaiproject#1…
Browse files Browse the repository at this point in the history 
…0884)

* SAK-47854: User whith Admin role cannot manage groups

* SAK-47854: Delete import

* SAK-47854: Add securityService and remove Cover

* SAK-47854: Codacy issues
  • Loading branch information
@alejandromf
alejandromf authored Sep 29, 2022
1 parent 7b92a03 commit 4df769e
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 58 deletions.
7 changes: 7 additions & 0 deletions ...ions-service/sections-api/src/java/org/sakaiproject/section/api/facade/manager/Authz.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,11 @@ public interface Authz {
* @return
*/
public String getRoleDescription(String userUid, String siteContext);

/**
* Returns true if current user is SuperUser
*
* @return
*/
public boolean isSuperUser();
}
31 changes: 19 additions & 12 deletions ...ions/sections-app/src/java/org/sakaiproject/tool/section/facade/sakai/AuthzSakaiImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import lombok.extern.slf4j.Slf4j;

import org.sakaiproject.authz.api.AuthzGroupService;
import org.sakaiproject.authz.cover.SecurityService;
import org.sakaiproject.authz.api.SecurityService;
import org.sakaiproject.section.api.SectionAwareness;
import org.sakaiproject.section.api.facade.manager.Authz;
import org.sakaiproject.site.cover.SiteService;
Expand All @@ -43,17 +43,23 @@ public class AuthzSakaiImpl implements Authz {

private AuthzGroupService authzGroupService;

protected SecurityService securityService;

public void setAuthzGroupService(AuthzGroupService authzGroupService) {
this.authzGroupService = authzGroupService;
}

public void setSecurityService(SecurityService securityService) {
this.securityService = securityService;
}

/**
* The user must have site.upd to update sections in the Section Info tool.
*/
public boolean isSectionManagementAllowed(String userUid, String siteContext) {
User sakaiUser = UserDirectoryService.getCurrentUser();
String siteRef = SiteService.siteReference(siteContext);
boolean canUpdateSite = SecurityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_SITE, siteRef);
boolean canUpdateSite = securityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_SITE, siteRef);

return canUpdateSite;
}
Expand All @@ -80,8 +86,8 @@ public boolean isSectionTaManagementAllowed(String userUid, String siteContext)
public boolean isSectionEnrollmentMangementAllowed(String userUid, String siteContext) {
User sakaiUser = UserDirectoryService.getCurrentUser();
String siteRef = SiteService.siteReference(siteContext);
boolean canUpdateSite = SecurityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_SITE, siteRef);
boolean canUpdateGroups = SecurityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_GROUP_MEMBERSHIP, siteRef);
boolean canUpdateSite = securityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_SITE, siteRef);
boolean canUpdateGroups = securityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_GROUP_MEMBERSHIP, siteRef);

return canUpdateSite || canUpdateGroups;
}
Expand All @@ -93,7 +99,7 @@ public boolean isSectionEnrollmentMangementAllowed(String userUid, String siteCo
public boolean isViewOwnSectionsAllowed(String userUid, String siteContext) {
User sakaiUser = UserDirectoryService.getCurrentUser();
String siteRef = SiteService.siteReference(siteContext);
boolean isStudent = SecurityService.unlock(sakaiUser, SectionAwareness.STUDENT_MARKER, siteRef);
boolean isStudent = securityService.unlock(sakaiUser, SectionAwareness.STUDENT_MARKER, siteRef);

return isStudent;
}
Expand All @@ -105,9 +111,9 @@ public boolean isViewOwnSectionsAllowed(String userUid, String siteContext) {
public boolean isViewAllSectionsAllowed(String userUid, String siteContext) {
User sakaiUser = UserDirectoryService.getCurrentUser();
String siteRef = SiteService.siteReference(siteContext);
return SecurityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_SITE, siteRef) ||
SecurityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_GROUP_MEMBERSHIP, siteRef) ||
SecurityService.unlock(sakaiUser, SectionAwareness.TA_MARKER, siteRef);
return securityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_SITE, siteRef) ||
securityService.unlock(sakaiUser, SiteService.SECURE_UPDATE_GROUP_MEMBERSHIP, siteRef) ||
securityService.unlock(sakaiUser, SectionAwareness.TA_MARKER, siteRef);
}

public boolean isSectionAssignable(String userUid, String siteContext) {
Expand All @@ -118,13 +124,14 @@ public String getRoleDescription(String userUid, String siteContext) {
String siteRef = SiteService.siteReference(siteContext);
String role = authzGroupService.getUserRole(userUid, siteRef);
if(log.isDebugEnabled()) log.debug("User " + userUid + " has role " + role + " in site " + siteContext);
if(role == null) {
if ((role == null) && (isSuperUser())) {
// Is this a superuser?
if(SecurityService.isSuperUser()) {
return JsfUtil.getLocalizedMessage("admin_role");
}
return JsfUtil.getLocalizedMessage("admin_role");
}
return role;
}
public boolean isSuperUser() {
return securityService.isSuperUser();
}

}
9 changes: 9 additions & 0 deletions ...tions-app/src/java/org/sakaiproject/tool/section/jsf/backingbean/CourseDependentBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import javax.faces.context.FacesContext;

Expand Down Expand Up @@ -157,4 +158,12 @@ public String getSiteRole() {
return getCourseBean().authz.getRoleDescription(getUserUid(), getSiteContext());
}

protected boolean canManageAnySection() {
return ((getCourseBean().authz.isSuperUser()) || (getSiteInstructors().stream()
.map(s -> s.getUser().getUserUid()).collect(Collectors.toList()).contains(getUserUid())));
}
protected boolean canManageSection(String sectionUid) {
return ((canManageAnySection()) || (getSectionTeachingAssistantsMap().get(sectionUid).stream()
.anyMatch(s -> s.getUser().getUserUid().equals(getUserUid()))));
}
}
6 changes: 1 addition & 5 deletions ...s-app/src/java/org/sakaiproject/tool/section/jsf/backingbean/EditStudentSectionsBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ public void init() {
// Get the TAs for all groups
Map<String,List<ParticipationRecord>> sectionTAs = getSectionManager().getSectionTeachingAssistantsMap(sectionSet);

// Get the Instructors for the site
List<String> intructorUids = getSiteInstructors().stream().map(s -> s.getUser().getUserUid())
.collect(Collectors.toList());

for(Iterator sectionIter = sectionSet.iterator(); sectionIter.hasNext();) {
CourseSection section = (CourseSection)sectionIter.next();
String catName = getCategoryName(section.getCategory());
Expand Down Expand Up @@ -146,7 +142,7 @@ public void init() {

StudentSectionDecorator decoratedSection = new StudentSectionDecorator(section, catName, taNames,
totalEnrollments, member, memberOtherSection, showNegativeSpots,
(taUids.contains(getUserUid()) || (intructorUids.contains(getUserUid()))));
(taUids.contains(getUserUid()) || (canManageAnySection())));

if(!hideSectionInTable) {
sections.add(decoratedSection);
Expand Down
21 changes: 5 additions & 16 deletions ...sections-app/src/java/org/sakaiproject/tool/section/jsf/backingbean/EditStudentsBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,17 @@ public void init() {

List<CourseSection> all= getAllSiteSections();

// Get the Instructors for the site
List<String> intructorUids = getSiteInstructors().stream().map(s -> s.getUser().getUserUid())
.collect(Collectors.toList());

// Build the list of items for the left-side box
List available;
if ((StringUtils.trimToNull(availableSectionUuid) == null) && (intructorUids.contains(currentUserUuid))) {
if ((StringUtils.trimToNull(availableSectionUuid) == null) && (canManageAnySection())) {
available = getSectionManager().getUnsectionedEnrollments(currentSection.getCourse().getUuid(),
currentSection.getCategory());
} else if (StringUtils.trimToNull(availableSectionUuid) == null) {
// Add users from other TA's sections
List<String> sectionsCurrentUserIsTA = new ArrayList<String>();
for (CourseSection section : all) {
String sectionUid = section.getUuid();
if (sectionTAMap.get(sectionUid).stream()
.anyMatch(s -> s.getUser().getUserUid().equals(currentUserUuid))) {
sectionsCurrentUserIsTA.add(sectionUid);
if (canManageSection(section.getUuid())) {
sectionsCurrentUserIsTA.add(section.getUuid());
}
}
List<EnrollmentRecord> availableAll = getSectionManager()
Expand Down Expand Up @@ -162,10 +156,7 @@ public void init() {

List sectionsReadOnly = new ArrayList<CourseSection>();
for (CourseSection section : all) {
String sectionUid = section.getUuid();
if ((this.isFromCategoryReadOnly(sectionUid)) && ((sectionTAMap.get(sectionUid).stream()
.anyMatch(s -> s.getUser().getUserUid().equals(currentUserUuid)))
|| (intructorUids.contains(currentUserUuid)))) {
if ((this.isFromCategoryReadOnly(section.getUuid())) && (canManageSection(section.getUuid()))) {
sectionsReadOnly.add(section);
}
}
Expand All @@ -181,9 +172,7 @@ public void init() {
for (Iterator iter = sectionsInCategory.iterator(); iter.hasNext();) {
CourseSection section = (CourseSection) iter.next();
// Don't include the current section and section where currentUser is not a TA
if (!(section.getUuid().equals(currentSection.getUuid())) && ((sectionTAMap.get(section.getUuid()).stream()
.anyMatch(s -> s.getUser().getUserUid().equals(currentUserUuid)))
|| (intructorUids.contains(currentUserUuid)))) {
if (!(section.getUuid().equals(currentSection.getUuid())) && (canManageSection(currentSection.getUuid()))) {
if (section.getUuid().equals(availableSectionUuid)) {
availableSectionTitle = section.getTitle();
availableSectionMax = section.getMaxEnrollments();
Expand Down
6 changes: 1 addition & 5 deletions ...pp/src/java/org/sakaiproject/tool/section/jsf/backingbean/FilteredSectionListingBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ public void init() {
// Get the TAs for all groups
Map<String,List<ParticipationRecord>> sectionTAs = getSectionManager().getSectionTeachingAssistantsMap(sectionSet);

// Get the Instructors for the site
List<String> intructorUids = getSiteInstructors().stream().map(s -> s.getUser().getUserUid())
.collect(Collectors.toList());

for(Iterator sectionIter = sectionSet.iterator(); sectionIter.hasNext();) {
CourseSection section = (CourseSection)sectionIter.next();
String catName = getCategoryName(section.getCategory());
Expand Down Expand Up @@ -106,7 +102,7 @@ public void init() {
(Integer) sectionSize.get(section.getUuid()) : 0;

SectionDecorator decoratedSection = new SectionDecorator(section, catName, taNames, totalEnrollments, true,
(taUids.contains(getUserUid()) || (intructorUids.contains(getUserUid()))));
(taUids.contains(getUserUid()) || (canManageAnySection())));
sections.add(decoratedSection);
}

Expand Down
26 changes: 6 additions & 20 deletions sections/sections-app/src/java/org/sakaiproject/tool/section/jsf/backingbean/RosterBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ public class RosterBean extends CourseDependentBean implements Serializable {
private List<EnrollmentRecord> siteStudents;
private List<EnrollmentDecorator> unpagedEnrollments;
@Getter private List<SelectItem> sectionFilterSelectItems;
private boolean currentUserisTA;
private boolean currentUserisInstructor;
private boolean canManageSection;

public void init() {
// Determine whether this course is externally managed
Expand Down Expand Up @@ -151,18 +150,12 @@ public void init() {
private void decorateEnrollments(List<EnrollmentRecord> siteStudents, SectionEnrollments sectionEnrollments, List<CourseSection> assignedSections) {
unpagedEnrollments = new ArrayList<>();

// Check if current user is Instructor in site
currentUserisInstructor = getSiteInstructors().stream().map(s -> s.getUser().getUserUid())
.collect(Collectors.toList()).contains(getUserUid());

// Check if current user is TA in site
List<CourseSection> all= getAllSiteSections();
Map<String, List<ParticipationRecord>> sectionTAMap = getSectionTeachingAssistantsMap();
for (CourseSection section : all) {
String sectionUid = section.getUuid();
if (sectionTAMap.get(sectionUid).stream()
.anyMatch(s -> s.getUser().getUserUid().equals(getUserUid()))) {
currentUserisTA = true;
if (canManageSection(section.getUuid())) {
canManageSection = true;
}
}

Expand All @@ -174,16 +167,9 @@ private void decorateEnrollments(List<EnrollmentRecord> siteStudents, SectionEnr
boolean includeStudent = categories.stream().filter(category -> {
CourseSection section = sectionEnrollments.getSection(enrollment.getUser().getUserUid(), category);
boolean isValidSection = false;
boolean currentUserisTAinSection = false;

if (section != null) {
List<ParticipationRecord> tas = getSectionManager().getSectionTeachingAssistants(section.getUuid());
List<String> taUids = generateTaUids(tas);
if (taUids.contains(getUserUid())) {
currentUserisTAinSection = true;
}

if (currentUserisTAinSection || currentUserisInstructor) {
if (canManageSection(section.getUuid())) {
if (("MY".equals(getFilter()) || section.getCategory().equals(getFilter()))
&& assignedSections.contains(section)) {
isValidSection = true;
Expand All @@ -205,14 +191,14 @@ else if (StringUtils.isBlank(getFilter())) {
}).count() > 0; // If there are valid sections for the current filter, include the student in the view

// If there is no filter, the section is valid and the student shouldn't be excluded
if (StringUtils.isBlank(getFilter()) && currentUserisInstructor) {
if (StringUtils.isBlank(getFilter()) && canManageAnySection()) {
includeStudent = true;
}

// If the filter is set to "no sections" filter and the student sections map is
// empty, student should be included
if ((NO_SECTIONS_FILTER_KEY.equals(getFilter()) || (StringUtils.isBlank(getFilter())))
&& studentSectionsMap.isEmpty() && (currentUserisInstructor || currentUserisTA)) {
&& studentSectionsMap.isEmpty() && (canManageSection)) {
includeStudent = true;
}

Expand Down
1 change: 1 addition & 0 deletions sections/sections-app/src/webapp/WEB-INF/spring-services.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<bean id="org.sakaiproject.section.api.facade.manager.Authz"
class="org.sakaiproject.tool.section.facade.sakai.AuthzSakaiImpl">
<property name="authzGroupService" ref="org.sakaiproject.authz.api.AuthzGroupService"/>
<property name="securityService" ref="org.sakaiproject.authz.api.SecurityService" />
</bean>

<bean id="org.sakaiproject.section.api.facade.manager.Context"
Expand Down

0 comments on commit 4df769e

Please sign in to comment.