vyommani commented on code in PR #1200:
URL: https://github.com/apache/ranger/pull/1200#discussion_r3902714788
##########
security-admin/src/test/java/org/apache/ranger/biz/TestSessionMgr.java:
##########
@@ -730,4 +732,209 @@ public void
testSetUserRoles_ConfigSuperUserGrantsKeyAdminForSysAdmin() {
PropertiesUtil.getPropertiesMap().remove(RangerConstants.RANGER_ADMIN_SUPER_USERS);
}
+
+ @Test
+ public void testProcessSuccessLogin_LimitZeroDoesNotExpireOldestSession() {
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"0");
+
+ setupAuthentication("limitUser");
+
+ XXPortalUser portalUser = portalUser("limitUser", 70L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(200L);
+
when(httpUtil.getDeviceType(anyString())).thenReturn(RangerCommonEnums.DEVICE_UNKNOWN);
+
+ HttpSession currentSession = mock(HttpSession.class);
+ when(currentSession.getAttribute("auditLoginId")).thenReturn(null);
+
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getSession()).thenReturn(currentSession);
+ when(request.getRequestURI()).thenReturn("/index.html");
+ when(request.getAttribute("spnegoEnabled")).thenReturn(null);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_PASSWORD, "UA", request);
+
+ assertNotNull(ret);
+ assertEquals(70L, ret.getUserId());
+ }
+
+ @Test
+ public void
testProcessSuccessLogin_LimitOneExpiresOldestFormLoginSession() {
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"1");
+
+ setupAuthentication("limitUser");
+
+ XXPortalUser portalUser = portalUser("limitUser", 71L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(201L);
+
when(httpUtil.getDeviceType(anyString())).thenReturn(RangerCommonEnums.DEVICE_UNKNOWN);
+
+ HttpSession currentSession = mock(HttpSession.class);
+ when(currentSession.getAttribute("auditLoginId")).thenReturn(null);
+
+ HttpSession oldestSession = mockUiSession("limitUser", 71L, false, 1L);
+
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getSession()).thenReturn(currentSession);
+ when(request.getRequestURI()).thenReturn("/index.html");
+ when(request.getAttribute("spnegoEnabled")).thenReturn(null);
+
+ try (MockedStatic<RangerHttpSessionListener> mocked =
Mockito.mockStatic(RangerHttpSessionListener.class)) {
+ CopyOnWriteArrayList<HttpSession> sessions = new
CopyOnWriteArrayList<>();
+ sessions.add(oldestSession);
+
mocked.when(RangerHttpSessionListener::getActiveSessionOnServer).thenReturn(sessions);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_PASSWORD, "UA", request);
+
+ assertNotNull(ret);
+
verify(oldestSession).setAttribute(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED,
Boolean.TRUE);
+
verify(oldestSession).setAttribute(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED_SSO,
false);
+ verify(oldestSession).invalidate();
+ }
+ }
+
+ @Test
+ public void
testProcessSuccessLogin_LimitOneMarksOldestSsoSessionExpiredWithoutInvalidate()
{
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"1");
+
+ setupAuthentication("ssoUser");
+
+ XXPortalUser portalUser = portalUser("ssoUser", 72L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(202L);
+
when(httpUtil.getDeviceType(anyString())).thenReturn(RangerCommonEnums.DEVICE_UNKNOWN);
+
+ HttpSession currentSession = mock(HttpSession.class);
+ when(currentSession.getAttribute("auditLoginId")).thenReturn(null);
+
+ HttpSession oldestSession = mockUiSession("ssoUser", 72L, true, 1L);
+
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getSession()).thenReturn(currentSession);
+ when(request.getRequestURI()).thenReturn("/index.html");
+ when(request.getAttribute("spnegoEnabled")).thenReturn(Boolean.TRUE);
+
+ try (MockedStatic<RangerHttpSessionListener> mocked =
Mockito.mockStatic(RangerHttpSessionListener.class)) {
+ CopyOnWriteArrayList<HttpSession> sessions = new
CopyOnWriteArrayList<>();
+ sessions.add(oldestSession);
+
mocked.when(RangerHttpSessionListener::getActiveSessionOnServer).thenReturn(sessions);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_TRUSTED_PROXY, "UA",
request);
+
+ assertNotNull(ret);
+ assertTrue(ret.isSSOEnabled());
+
verify(oldestSession).setAttribute(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED,
Boolean.TRUE);
+
verify(oldestSession).setAttribute(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED_SSO,
true);
+ verify(oldestSession, never()).invalidate();
+ }
+ }
+
+ @Test
+ public void
testProcessSuccessLogin_DownloadRequestDoesNotConsumeSessionQuota() {
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"1");
+
+ setupAuthentication("limitUser");
+
+ XXPortalUser portalUser = portalUser("limitUser", 73L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(203L);
+
when(httpUtil.getDeviceType(anyString())).thenReturn(RangerCommonEnums.DEVICE_UNKNOWN);
+
+ HttpSession currentSession = mock(HttpSession.class);
+ when(currentSession.getAttribute("auditLoginId")).thenReturn(null);
+
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getSession()).thenReturn(currentSession);
+
when(request.getRequestURI()).thenReturn("/service/plugins/policies/download/hadoopdev");
+ when(request.getAttribute("spnegoEnabled")).thenReturn(null);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_PASSWORD, "UA", request);
+
+ assertNotNull(ret);
+
verify(currentSession).setAttribute(SessionMgr.SESSION_ATTR_DOWNLOAD_ONLY,
Boolean.TRUE);
+ }
+
+ @Test
+ public void testIsPluginOrSecureDownloadRequest() {
Review Comment:
Nice coverage of the core cases. A few gaps worth adding given the logic
being tested:
- `limit=2` with three existing sessions for the user — verify only the
single oldest is expired (proves the `toExpire` count and sort-by-creation-time
ordering for N > 1, not just the N=1 case currently tested).
- Two different users at `limit=1` verify user B's login doesn't expire
user A's session (proves the `loginId` filter in `findActiveUiSessionsForUser`).
- A concurrent-login scenario (even a simple two-thread test) against
`limit=1`, since `enforceConcurrentSessionLimit` isn't currently synchronized
per `loginId`.
- If REST/API sessions end up excluded per the discussion elsewhere in this
PR, a test proving a `/service/public/v2/api/...`-style login doesn't consume a
UI quota slot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
