Merge commit from fork

https://sakaiproject.atlassian.net/browse/SAK-50571
sakaiproject · Oct 11, 2024 · a9aadd9 · a9aadd9
1 parent 2e37c64
commit a9aadd9
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 14 deletions.
diff --git a/kernel/api/src/main/java/org/sakaiproject/user/api/UserDirectoryService.java b/kernel/api/src/main/java/org/sakaiproject/user/api/UserDirectoryService.java
@@ -507,7 +507,9 @@ User addUser(String id, String eid, String firstName, String lastName, String em
 	String idFromReference(String reference);
 
 	/**
-	 * Check if the user is a fake one to simulate role views
+	 * Check if the id or eid is a user type roleview.
+	 * This check only looks in local storage (non-provided) which is the only place
+	 * these types of user should exist.
 	 *
 	 * @param id the user id to check
 	 * @return true if the user is only for role view purposes

diff --git a/kernel/kernel-impl/src/main/java/org/sakaiproject/site/impl/BaseSiteService.java b/kernel/kernel-impl/src/main/java/org/sakaiproject/site/impl/BaseSiteService.java
@@ -3006,7 +3006,7 @@ public User addMockUserInSite(String siteReference, String eid, String role) thr
 		if (StringUtils.isNoneBlank(siteReference, eid, role)) {
 			try {
 				String mockUserEmail = eid + "@" + serverConfigurationService().getServerName();
-				newUser = userDirectoryService().addUser(null, eid, role, role, mockUserEmail, eid, UserDirectoryService.ROLEVIEW_USER_TYPE, null);
+				newUser = userDirectoryService().addUser(null, eid, role, role, mockUserEmail, null, UserDirectoryService.ROLEVIEW_USER_TYPE, null);
 				AuthzGroup realmEdit = authzGroupService().getAuthzGroup(siteReference);
 				if (authzGroupService().allowUpdate(siteReference) || allowUpdateSiteMembership(siteId(siteReference))) {
 					realmEdit.addMember(newUser.getId(), role, true, false);

diff --git a/kernel/kernel-impl/src/main/java/org/sakaiproject/user/impl/BaseUserDirectoryService.java b/kernel/kernel-impl/src/main/java/org/sakaiproject/user/impl/BaseUserDirectoryService.java
@@ -241,13 +241,14 @@ public String idFromReference(String reference) {
 		return reference.substring(lastSeparatorIndex + 1);
 	}
 
+	@Override
 	public boolean isRoleViewType(String id) {
-		try {
-			User user = getUser(id);
-			return ROLEVIEW_USER_TYPE.equals(user.getType());
-		} catch (UserNotDefinedException e) {
-			return false;
-		}
+		if (id != null) {
+			String userId = Optional.ofNullable(m_storage.checkMapForId(id)).orElse(id);
+			User user = m_storage.getById(userId);
+			if (user != null) return ROLEVIEW_USER_TYPE.equals(user.getType());
+        }
+		return false;
 	}
 
 	/**
@@ -1691,6 +1692,12 @@ public User authenticate(String loginId, String password)
 		loginId = cleanEid(loginId);
 		if (loginId == null) return null;
 
+		// never authenticate user type roleview
+		if (isRoleViewType(loginId)) {
+			log.warn("SECURITY: attempted to authenticate a user with type roleview [{}]", loginId);
+			return null;
+		}
+
 		UserEdit user = null;
 		boolean authenticateWithProviderFirst = (m_provider != null) && m_provider.authenticateWithProviderFirst(loginId);
 

diff --git a/.../portal-impl/impl/src/java/org/sakaiproject/portal/charon/handlers/RoleSwitchHandler.java b/.../portal-impl/impl/src/java/org/sakaiproject/portal/charon/handlers/RoleSwitchHandler.java
@@ -22,7 +22,7 @@
 import javax.servlet.http.HttpServletResponse;
 
 import org.sakaiproject.authz.api.Role;
-import org.sakaiproject.component.cover.ServerConfigurationService;
+import org.sakaiproject.component.api.ServerConfigurationService;
 import org.sakaiproject.event.api.EventTrackingService;
 import org.sakaiproject.event.api.NotificationService;
 import org.sakaiproject.event.api.UsageSessionService;
@@ -47,12 +47,20 @@ public class RoleSwitchHandler extends BasePortalHandler
 
 	@Autowired @Qualifier("org.sakaiproject.event.api.EventTrackingService")
 	private EventTrackingService eventTrackingService;
+	@Autowired @Qualifier("org.sakaiproject.component.api.ServerConfigurationService")
+	private ServerConfigurationService serverConfigurationService;
 	@Autowired @Qualifier("org.sakaiproject.site.api.SiteService")
 	private SiteService siteService;
 
+	private String portalUrl;
+	private String externalRoles;
+
 	public RoleSwitchHandler() {
 		SpringBeanAutowiringSupport.processInjectionBasedOnCurrentContext(this);
 		setUrlFragment(RoleSwitchHandler.URL_FRAGMENT);
+		portalUrl = serverConfigurationService.getPortalUrl();
+		// get the roles that can be swapped to from sakai.properties
+		externalRoles = serverConfigurationService.getString("studentview.roles");
 	}
 
 	@Override
@@ -86,8 +94,6 @@ public int doGet(String[] parts, HttpServletRequest req, HttpServletResponse res
             }
 
             Set<Role> roles = activeSite.getRoles(); // all the roles in our site
-
-        	String externalRoles = ServerConfigurationService.getString("studentview.roles"); // get the roles that can be swapped to from sakai.properties
         	String[] svRoles = externalRoles.split(",");
         	boolean isRoleLegit = false;
 
@@ -114,8 +120,7 @@ public int doGet(String[] parts, HttpServletRequest req, HttpServletResponse res
 
 			try
 			{
-				String siteUrl = ServerConfigurationService.getPortalUrl() + "/site/" + parts[2] + "/tool/" + parts[4] + "/";
-
+				String url = portalUrl + "/site/" + parts[2] + "/tool/" + parts[4] + "/";
 
 				activeSite.getPages().stream() // get all pages in site
 					.map(SitePage::getTools) // tools for each page
@@ -132,7 +137,7 @@ public int doGet(String[] parts, HttpServletRequest req, HttpServletResponse res
 				eventTrackingService.post(eventTrackingService
 						.newEvent(UsageSessionService.EVENT_ROLEVIEW_START, parts[5], parts[2], false, NotificationService.NOTI_NONE));
 
-				res.sendRedirect(URLUtils.sanitisePath(siteUrl));
+				res.sendRedirect(URLUtils.sanitisePath(url));
 				return RESET_DONE;
 			}
 			catch(Exception ex)