Merge pull request #903 from PasinduYeshan/fix/password-expiry-listener

Skip passwordExpiryEventListener in authentication flow
wso2-extensions · Jan 16, 2025 · 623e699 · 623e699
2 parents 84f94ab + c9d535b
commit 623e699
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 7 deletions.
diff --git a/...n/java/org/wso2/carbon/identity/password/expiry/listener/PasswordExpiryEventListener.java b/...n/java/org/wso2/carbon/identity/password/expiry/listener/PasswordExpiryEventListener.java
@@ -30,6 +30,7 @@
 import org.wso2.carbon.identity.password.expiry.util.PasswordPolicyUtils;
 import org.wso2.carbon.user.core.UserStoreException;
 import org.wso2.carbon.user.core.UserStoreManager;
+import org.wso2.carbon.user.core.common.AbstractUserStoreManager;
 import org.wso2.carbon.user.core.model.UserClaimSearchEntry;
 
 import java.util.Arrays;
@@ -60,15 +61,24 @@ public boolean doPostGetUserClaimValues(String username, String[] claims, String
                                                   Map<String, String> claimMap, UserStoreManager userStoreManager)
             throws UserStoreException {
 
-        if (!isEnable() || !Arrays.asList(claims).contains(PasswordPolicyConstants.PASSWORD_EXPIRY_TIME_CLAIM)) {
+        /*
+         * The passwordExpiryTime is a dynamically calculated value. It is only computed and added to the claims map
+         * if explicitly requested by the user via the claims list. This computation is also skipped during the
+         * authentication flow to avoid unnecessary processing.
+         */
+        if (!isEnable() || !Arrays.asList(claims).contains(PasswordPolicyConstants.PASSWORD_EXPIRY_TIME_CLAIM) ||
+                PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername() == null) {
             return true;
         }
         log.debug("post get user claim values with id is called in PasswordExpiryEventListener");
 
         try {
             String userTenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain();
+            String domainQualifiedUserName =
+                    ((AbstractUserStoreManager) userStoreManager).getUser(null, username)
+                            .getDomainQualifiedUsername();
             Optional<Long> passwordExpiryTime =
-                    PasswordPolicyUtils.getUserPasswordExpiryTime(userTenantDomain, username);
+                    PasswordPolicyUtils.getUserPasswordExpiryTime(userTenantDomain, domainQualifiedUserName);
             passwordExpiryTime.ifPresent(expiryTime -> claimMap.put(PasswordPolicyConstants.PASSWORD_EXPIRY_TIME_CLAIM,
                     String.valueOf(expiryTime)));
         } catch (ExpiredPasswordIdentificationException e) {

diff --git a/...piry/src/main/java/org/wso2/carbon/identity/password/expiry/util/PasswordPolicyUtils.java b/...piry/src/main/java/org/wso2/carbon/identity/password/expiry/util/PasswordPolicyUtils.java
@@ -265,8 +265,8 @@ private static Set<String> getUserAttributes(PasswordExpiryRuleAttributeEnum att
                     List<String> roleIdsOfGroups = getRoleIdsOfGroups(new ArrayList<>(userGroupIds), tenantDomain);
 
                     List<RoleBasicInfo> userRoles = getUserRoles(tenantDomain, userId);
-                    Set<String> userRoleIds =
-                            userRoles.stream().map(RoleBasicInfo::getId).collect(Collectors.toSet());
+                    Set<String> userRoleIds = userRoles.stream().map(RoleBasicInfo::getId).filter(Objects::nonNull)
+                                    .collect(Collectors.toSet());
                     userRoleIds.addAll(roleIdsOfGroups);
                     fetchedUserAttributes.put(PasswordExpiryRuleAttributeEnum.ROLES, userRoleIds);
                     break;

diff --git a/...va/org/wso2/carbon/identity/password/expiry/listener/PasswordExpiryEventListenerTest.java b/...va/org/wso2/carbon/identity/password/expiry/listener/PasswordExpiryEventListenerTest.java
@@ -32,7 +32,8 @@
 import org.testng.annotations.Test;
 import org.wso2.carbon.identity.common.testng.WithCarbonHome;
 import org.wso2.carbon.user.core.UserStoreException;
-import org.wso2.carbon.user.core.UserStoreManager;
+import org.wso2.carbon.user.core.common.User;
+import org.wso2.carbon.user.core.common.AbstractUserStoreManager;
 import org.wso2.carbon.user.core.model.UserClaimSearchEntry;
 
 import java.util.Collections;
@@ -58,13 +59,15 @@ public class PasswordExpiryEventListenerTest {
     @Mock
     PrivilegedCarbonContext privilegedCarbonContext;
     @Mock
-    UserStoreManager userStoreManager;
+    AbstractUserStoreManager userStoreManager;
+    @Mock
+    User user;
 
     private MockedStatic<PrivilegedCarbonContext> mockedPrivilegedCarbonContext;
     private MockedStatic<PasswordPolicyUtils> mockedPasswordPolicyUtils;
 
     @BeforeMethod
-    public void setUp() {
+    public void setUp() throws UserStoreException {
 
         MockitoAnnotations.openMocks(this);
         passwordExpiryEventListener = new PasswordExpiryEventListener();
@@ -73,6 +76,9 @@ public void setUp() {
                 .thenReturn(privilegedCarbonContext);
 
         when(privilegedCarbonContext.getTenantDomain()).thenReturn(TENANT_DOMAIN);
+        when(privilegedCarbonContext.getUsername()).thenReturn("USERNAME");
+
+        when(userStoreManager.getUser(any(), anyString())).thenReturn(user);
     }
 
     @BeforeClass
@@ -103,6 +109,8 @@ public void testDoPostGetUserClaimValuesWithPasswordExpiryClaim() throws UserSto
         Map<String, String> claimMap = new HashMap<>();
         String profileName = "default";
 
+        when(user.getDomainQualifiedUsername()).thenReturn(username);
+
         // Case 1: When claims contains PASSWORD_EXPIRY_TIME_CLAIM.
         claims = new String[]{PasswordPolicyConstants.PASSWORD_EXPIRY_TIME_CLAIM};
 
@@ -132,6 +140,23 @@ public void testDoPostGetUserClaimValuesWithoutPasswordExpiryClaim() throws User
         String profileName = "default";
         claims = new String[]{"claim1", "claim2"};
 
+        when(user.getDomainQualifiedUsername()).thenReturn(username);
+
+        passwordExpiryEventListener.doPostGetUserClaimValues(username, claims, profileName, claimMap, userStoreManager);
+        Assert.assertFalse(claimMap.containsKey(PasswordPolicyConstants.PASSWORD_EXPIRY_TIME_CLAIM));
+    }
+
+    @Test
+    public void testDoPostGetUserClaimValuesInAuthenticationFlow() throws UserStoreException {
+
+        String username = "testUser";
+        String[] claims;
+        Map<String, String> claimMap = new HashMap<>();
+        String profileName = "default";
+        claims = new String[]{"claim1", "claim2"};
+
+        when(privilegedCarbonContext.getUsername()).thenReturn(null);
+
         passwordExpiryEventListener.doPostGetUserClaimValues(username, claims, profileName, claimMap, userStoreManager);
         Assert.assertFalse(claimMap.containsKey(PasswordPolicyConstants.PASSWORD_EXPIRY_TIME_CLAIM));
     }