eoinmcdonnell113 commented on code in PR #1200:
URL: https://github.com/apache/ranger/pull/1200#discussion_r3981175258
##########
security-admin/src/test/java/org/apache/ranger/biz/TestSessionMgr.java:
##########
@@ -730,4 +737,372 @@ 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, "Mozilla/5.0",
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, "Mozilla/5.0",
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,
"Mozilla/5.0", 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);
+
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, "Mozilla/5.0",
request);
+
+ assertNotNull(ret);
+
verify(currentSession).setAttribute(SessionMgr.SESSION_ATTR_DOWNLOAD_ONLY,
Boolean.TRUE);
+ }
+
+ @Test
+ public void testProcessSuccessLogin_ApiRequestDoesNotConsumeSessionQuota()
{
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"1");
+
+ setupAuthentication("limitUser");
+
+ XXPortalUser portalUser = portalUser("limitUser", 74L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(204L);
+
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/public/v2/api/policies");
+ when(request.getAttribute("spnegoEnabled")).thenReturn(null);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_PASSWORD, "curl/8.0",
request);
+
+ assertNotNull(ret);
+ verify(currentSession).setAttribute(SessionMgr.SESSION_ATTR_NON_UI,
Boolean.TRUE);
+ verify(currentSession,
never()).setAttribute(eq(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED), any());
+ }
+
+ @Test
+ public void
testProcessSuccessLogin_LimitTwoExpiresOnlyOldestOfTwoExistingSessions() {
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"2");
+
+ setupAuthentication("limitUser");
+
+ XXPortalUser portalUser = portalUser("limitUser", 75L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(205L);
+
when(httpUtil.getDeviceType(anyString())).thenReturn(RangerCommonEnums.DEVICE_UNKNOWN);
+
+ HttpSession currentSession = mock(HttpSession.class);
+ when(currentSession.getAttribute("auditLoginId")).thenReturn(null);
+
+ HttpSession oldestSession = mockUiSession("limitUser", 75L, false, 1L);
+ HttpSession newerSession = mockUiSession("limitUser", 75L, false, 2L);
+
+ 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(newerSession);
+ sessions.add(oldestSession);
+
mocked.when(RangerHttpSessionListener::getActiveSessionOnServer).thenReturn(sessions);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_PASSWORD, "Mozilla/5.0",
request);
+
+ assertNotNull(ret);
+
verify(oldestSession).setAttribute(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED,
Boolean.TRUE);
+ verify(oldestSession).invalidate();
+ verify(newerSession,
never()).setAttribute(eq(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED), any());
+ verify(newerSession, never()).invalidate();
+ }
+ }
+
+ @Test
+ public void testProcessSuccessLogin_DoesNotExpireOtherUsersSessions() {
+ RangerContextHolder.setSecurityContext(null);
+
PropertiesUtil.getPropertiesMap().put(SessionMgr.PROP_SESSION_LIMIT_CONCURRENCY,
"1");
+
+ setupAuthentication("userB");
+
+ XXPortalUser portalUser = portalUser("userB", 76L);
+ stubPortalUserLookup(portalUser);
+ stubRolesAndPermissions(portalUser);
+ stubAuthSessionCreate(206L);
+
when(httpUtil.getDeviceType(anyString())).thenReturn(RangerCommonEnums.DEVICE_UNKNOWN);
+
+ HttpSession currentSession = mock(HttpSession.class);
+ when(currentSession.getAttribute("auditLoginId")).thenReturn(null);
+
+ HttpSession otherUserSession = mockUiSession("userA", 77L, 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(otherUserSession);
+
mocked.when(RangerHttpSessionListener::getActiveSessionOnServer).thenReturn(sessions);
+
+ UserSessionBase ret =
sessionMgr.processSuccessLogin(XXAuthSession.AUTH_TYPE_PASSWORD, "Mozilla/5.0",
request);
+
+ assertNotNull(ret);
+ assertEquals(76L, ret.getUserId());
+ verify(otherUserSession,
never()).setAttribute(eq(SessionMgr.SESSION_ATTR_CONCURRENT_EXPIRED), any());
+ verify(otherUserSession, never()).invalidate();
+ }
+ }
+
+ @Test
+ public void
testEnforceConcurrentSessionLimit_ConcurrentSameUserExpiresOldest() throws
Exception {
Review Comment:
Thank you. The previous mock did not retain attributes written via
setAttribute(), so atLeastOnce() didn’t really prove the per-user lock
prevented duplicate expiration.
I updated the test so the session mock is stateful, coordinated both threads
to exercise the race, and now assert that expiration happens exactly once. That
better shows the lock matters.
--
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]
