Index: site-manage-participant-helper/src/java/org/sakaiproject/site/tool/helper/participant/impl/SiteAddParticipantHandler.java =================================================================== --- site-manage-participant-helper/src/java/org/sakaiproject/site/tool/helper/participant/impl/SiteAddParticipantHandler.java (revision 130237) +++ site-manage-participant-helper/src/java/org/sakaiproject/site/tool/helper/participant/impl/SiteAddParticipantHandler.java (working copy) @@ -110,9 +110,8 @@ this.securityService = securityService; } - private boolean isAdmin = false; - private boolean propertiesNotFound = false; - private List allowedRoles = new ArrayList(); + // SAK-23257 + private static final String SAK_PROP_RESTRICTED_ROLES = "sitemanage.addParticipants.restrictedRoles"; private UserDirectoryService userDirectoryService; public void setUserDirectoryService(UserDirectoryService userDirectoryService) { @@ -269,20 +268,10 @@ try { site = siteService.getSite(siteId); realm = authzGroupService.getAuthzGroup(siteService.siteReference(siteId)); + + // bjones86 - SAK-23257 + roles = getAllowedRoles( site.getType() ); - // SAK-23257 - isAdmin = securityService.isSuperUser(); - propertiesNotFound = false; - allowedRoles = Arrays.asList( - ArrayUtils.nullToEmpty( serverConfigurationService.getStrings( "sitemanage.addParticipants.allowedRoles" ) ) ); - if( allowedRoles.isEmpty() ) - propertiesNotFound = true; - if( propertiesNotFound ) - for( Iterator itr = realm.getRoles().iterator(); itr.hasNext(); ) - roles.add( (Role) itr.next() ); - else - roles = getAllowedRoles(); - } catch (IdUnusedException e) { // The siteId we were given was bogus e.printStackTrace(); @@ -295,34 +284,47 @@ } /** - * Get a list of the 'allowed roles' as defined in sakai.properties. + * Get a list of the 'allowed roles', taking into account the current site type + * and the list of restricted roles defined in sakai.properties. * If the properties are not found, just return all the roles. - * If the user is an admin, return all the roles + * If the user is an admin, return all the roles. * * @author bjones86 - SAK-23257 * - * @param state - * @return A list of 'allowed' role objects + * @param siteType + * the current site's type + * @return A list of 'allowed' role objects for the given site type */ - private List getAllowedRoles() + private List getAllowedRoles( String siteType ) { List retVal = new ArrayList(); - for( Iterator i = realm.getRoles().iterator(); i.hasNext(); ) - { - Role r = (Role) i.next(); + // Get all the restricted roles for this site type, as well as all restricted roles at the top level (restricted for all site types) + Set restrictedRoles = new HashSet(); + restrictedRoles.addAll( Arrays.asList( ArrayUtils.nullToEmpty( serverConfigurationService.getStrings( SAK_PROP_RESTRICTED_ROLES + "." + siteType ) ) ) ); + restrictedRoles.addAll( Arrays.asList( ArrayUtils.nullToEmpty( serverConfigurationService.getStrings( SAK_PROP_RESTRICTED_ROLES ) ) ) ); + + // Loop through all the roles for this realm + for( Iterator itr = realm.getRoles().iterator(); itr.hasNext(); ) + { + Role role = (Role) itr.next(); + + // If the user is an admin, or if the properties weren't found (empty set), just add the role to the list + if( securityService.isSuperUser() || restrictedRoles.isEmpty() ) + { + retVal.add( role ); + } - // If the user is an admin, or if the properties weren't found, just add the role to the list - if( isAdmin || propertiesNotFound ) - retVal.add( r ); - - // Otherwise, only add the role if it's in the list of allowed roles - else - for( String role : allowedRoles ) - if( role.equalsIgnoreCase( r.getId() ) ) - retVal.add( r ); + // Otherwise, only add the role to the list of 'allowed' roles if it's not present in the set of 'restricted' roles + else + { + if( !restrictedRoles.contains( role.getId().toLowerCase() ) ) + { + retVal.add( role ); + } + } } - + return retVal; } @@ -601,16 +603,13 @@ okRoles.add(role); } - // SAK-23257 - if( !propertiesNotFound ) + // SAK-23257 - display an error message if the new role is in the restricted role list + String siteType = site.getType(); + Role r = realmEdit.getRole( role ); + if( !getAllowedRoles( siteType ).contains( r ) ) { - Role r = realmEdit.getRole( role ); - if( !getAllowedRoles().contains( r ) ) - { - targettedMessageList.addMessage( new TargettedMessage( "java.roleperm", new Object[] { role }, - TargettedMessage.SEVERITY_ERROR ) ); - continue; - } + targettedMessageList.addMessage( new TargettedMessage( "java.roleperm", new Object[] { role }, TargettedMessage.SEVERITY_ERROR ) ); + continue; } try { Index: site-manage-tool/tool/src/java/org/sakaiproject/site/tool/SiteAction.java =================================================================== --- site-manage-tool/tool/src/java/org/sakaiproject/site/tool/SiteAction.java (revision 130237) +++ site-manage-tool/tool/src/java/org/sakaiproject/site/tool/SiteAction.java (working copy) @@ -736,11 +736,9 @@ private List prefLocales = new ArrayList(); - private boolean isAdmin = false; - private boolean propertiesNotFound = false; - private List allowedRoles = new ArrayList(); - private static final String VM_ALLOWED_ROLES_DROP_DOWN = "allowedRoles"; - private static final String SAK_PROP_ALLOWED_ROLES_FOR_ADD = "sitemanage.addParticipants.allowedRoles"; + // SAK-23257 + private static final String SAK_PROP_RESTRICTED_ROLES = "sitemanage.addParticipants.restrictedRoles"; + private static final String VM_ALLOWED_ROLES_DROP_DOWN = "allowedRoles"; // state variable for whether any multiple instance tool has been selected private String STATE_MULTIPLE_TOOL_INSTANCE_SELECTED = "state_multiple_tool_instance_selected"; @@ -2195,7 +2193,8 @@ roles = getRoles(state); context.put("roles", roles); - context.put( VM_ALLOWED_ROLES_DROP_DOWN, getAllowedRoles( state ) ); + // SAK-23257 - add the allowed roles to the context for UI rendering + context.put( VM_ALLOWED_ROLES_DROP_DOWN, getAllowedRoles( site.getType(), roles ) ); // will have the choice to active/inactive user or not String activeInactiveUser = ServerConfigurationService.getString( @@ -7569,14 +7568,6 @@ int siteTitleMaxLength = ServerConfigurationService.getInt("site.title.maxlength", 25); state.setAttribute(STATE_SITE_TITLE_MAX, siteTitleMaxLength); } - - // SAK-23257 - isAdmin = securityService.isSuperUser(); - propertiesNotFound = false; - allowedRoles = Arrays.asList( - ArrayUtils.nullToEmpty( ServerConfigurationService.getStrings( SAK_PROP_ALLOWED_ROLES_FOR_ADD ) ) ); - if( allowedRoles.isEmpty() ) - propertiesNotFound = true; } // init @@ -7774,17 +7765,16 @@ roles.add(oldRoleId); } - // SAK-23257 - if( !propertiesNotFound ) + // SAK-23257 - display an error message if the new role is in the restricted role list + String siteType = s.getType(); + List allowedRoles = getAllowedRoles( siteType, getRoles( state ) ); + for( String roleName : roles ) { - for( String roleName : roles ) + Role r = realmEdit.getRole( roleName ); + if( !allowedRoles.contains( r ) ) { - Role role = realmEdit.getRole( roleName ); - if( !getAllowedRoles( state ).contains( role ) ) - { - addAlert( state, rb.getFormattedMessage( "java.roleperm", new Object[] { roleName } ) ); - return; - } + addAlert( state, rb.getFormattedMessage( "java.roleperm", new Object[] { roleName } ) ); + return; } } @@ -9644,36 +9634,44 @@ } // getRoles /** - * Get a list of the 'allowed roles' as defined in sakai.properties. + * Get a list of the 'allowed roles', taking into account the current site type + * and the list of restricted roles defined in sakai.properties. * If the properties are not found, just return all the roles. - * If the user is an admin, return all the roles + * If the user is an admin, return all the roles. * * @author bjones86 - SAK-23257 * - * @param state - * @return A list of 'allowed' role objects + * @param siteType + * the current site's type + * @return A list of 'allowed' role objects for the given site type */ - private List getAllowedRoles( SessionState state ) + private List getAllowedRoles( String siteType, List allRolesForSiteType ) { - // Get all the roles available - List roles = getRoles( state ); List retVal = new ArrayList(); - // Loop through them - for( Object obj : roles ) + // Get all the restricted roles for this site type, as well as all restricted roles at the top level (restricted for all site types) + Set restrictedRoles = new HashSet(); + restrictedRoles.addAll( Arrays.asList( ArrayUtils.nullToEmpty( ServerConfigurationService.getStrings( SAK_PROP_RESTRICTED_ROLES + "." + siteType ) ) ) ); + restrictedRoles.addAll( Arrays.asList( ArrayUtils.nullToEmpty( ServerConfigurationService.getStrings( SAK_PROP_RESTRICTED_ROLES ) ) ) ); + + // Loop through all the roles for this site type + for( Role role : allRolesForSiteType ) { - Role r = (Role) obj; - - // If the user is an admin, or if the sakai.properties were not found, just add the role to the list - if( isAdmin || propertiesNotFound ) - retVal.add( r ); - - // Otherwise, only add the role if it's in the list of allowed roles - else - for( String role : allowedRoles ) - if( role.equalsIgnoreCase( r.getId() ) ) - retVal.add( r ); - } + // If the user is an admin, or if the properties weren't found (empty set), just add the role to the list + if( securityService.isSuperUser() || restrictedRoles.isEmpty() ) + { + retVal.add( role ); + } + + // Otherwise, only add the role to the list of 'allowed' roles if it's not present in the set of 'restricted' roles + else + { + if( !restrictedRoles.contains( role.getId().toLowerCase() ) ) + { + retVal.add( role ); + } + } + } return retVal; }