From efa8f054c064faaf551d42cc8917bf6a0a08cb2f Mon Sep 17 00:00:00 2001 From: Rashmini Date: Fri, 6 Oct 2023 17:34:29 +0530 Subject: [PATCH 1/3] Fix recovery V2 /confirm API for EXTERNAL channel --- .../password/PasswordRecoveryManagerImpl.java | 36 +++++++++++------- .../NotificationPasswordRecoveryManager.java | 37 ++++++++++++------- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java index 0ae64b3092..f12522f9a8 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java @@ -295,8 +295,9 @@ public PasswordResetCodeDTO confirm(String otp, String confirmationCode, String IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_CODE.getCode(), IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_CODE.getMessage(), code); } catch (IdentityRecoveryException e) { - /* This is a fallback logic to support already initiated email link based recovery flows using the - recovery V1 API, which do not have recovery flow ids. */ + /* This method is to support already initiated email link based recovery flows using the recovery V1 API, + which do not have recovery flow ids, and handle recovery flows when the notifications are externally + managed. */ return validateConfirmationCode(userAccountRecoveryManager, recoveryFlowId, tenantDomain); } } @@ -892,10 +893,9 @@ private boolean isMinNoOfRecoveryQuestionsAnswered(String username, String tenan } /** - * This method is to validate the confirmation code when there's no recovery flow id. This is added as a fallback - * logic to handle the already initiated email link based recovery flows which do not have recovery flow ids, - * which were initiated before moving to the Recovery V2 API. This shouldn't be used for any other purpose and - * should be kept for sometime. + * This method is added to handle the already initiated email link based recovery flows which do not have + * recovery flow ids, which were initiated before moving to the Recovery V2 API. This also handles the + * recovery flows when the notifications are externally managed. * * @param userAccountRecoveryManager UserAccountRecoveryManager. * @param confirmationCode Confirmation code. @@ -910,17 +910,27 @@ private PasswordResetCodeDTO validateConfirmationCode(UserAccountRecoveryManager UserRecoveryData userRecoveryData; try { - userRecoveryData = userAccountRecoveryManager.getUserRecoveryData(confirmationCode, + String hashedConfirmationCode = Utils.hashCode(confirmationCode); + userRecoveryData = userAccountRecoveryManager.getUserRecoveryData(hashedConfirmationCode, RecoverySteps.UPDATE_PASSWORD); + } catch (NoSuchAlgorithmException e) { + throw Utils.handleServerException( + IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_NO_HASHING_ALGO_FOR_CODE, null); } catch (IdentityRecoveryException e) { - if (IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_CODE.getCode().equals( - e.getErrorCode())) { - e.setErrorCode(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID.getCode()); + try { + userRecoveryData = userAccountRecoveryManager.getUserRecoveryData(confirmationCode, + RecoverySteps.UPDATE_PASSWORD); + } catch (IdentityRecoveryException ex) { + if (IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_CODE.getCode().equals( + ex.getErrorCode())) { + ex.setErrorCode(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID.getCode()); + } + throw ex; } - throw e; } - if (!StringUtils.equals(userRecoveryData.getRemainingSetIds(), - NotificationChannels.EMAIL_CHANNEL.getChannelType())) { + if (!(StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels.EMAIL_CHANNEL. + getChannelType()) || StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels. + EXTERNAL_CHANNEL.getChannelType()))) { throw Utils.handleClientException( IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID, confirmationCode); } diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java index 196811355a..aa19c6a60a 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java @@ -715,8 +715,9 @@ public User updateUserPassword(String code, String confirmationCode, String pass IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_CODE.getMessage(), code); } } catch (IdentityRecoveryException e) { - /* This is a fallback logic to support already initiated email link based recovery flows using the - recovery V1 API, which do not have recovery flow ids. */ + /* This method is to support already initiated email link based recovery flows using the recovery V1 API, + which do not have recovery flow ids, and handle recovery flows when the notifications are externally + managed. */ userRecoveryData = validateUserRecoveryDataFromCode(code, confirmationCode, password, properties); } @@ -784,10 +785,9 @@ public User updateUserPassword(String code, String confirmationCode, String pass } /** - * This method is to validate user recovery data using the reset code when there's no recovery flow id. - * This is added as a fallback logic to handle the already initiated email link based recovery flows which do not - * have recovery flow ids, which were initiated before moving to the Recovery V2 API. - * This shouldn't be used for any other purpose and should be kept for sometime. + * This method is added to handle the already initiated email link based recovery flows which do not have + * recovery flow ids, which were initiated before moving to the Recovery V2 API. This also handles the + * recovery flows when the notifications are externally managed. * * @param code Password Reset code. * @param confirmationCode Confirmation code. @@ -803,16 +803,27 @@ private UserRecoveryData validateUserRecoveryDataFromCode(String code, String co UserRecoveryData userRecoveryData; UserAccountRecoveryManager userAccountRecoveryManager = UserAccountRecoveryManager.getInstance(); try { - userRecoveryData = userAccountRecoveryManager.getUserRecoveryData(code, RecoverySteps.UPDATE_PASSWORD); + String hashedCode = Utils.hashCode(code); + userRecoveryData = userAccountRecoveryManager.getUserRecoveryData(hashedCode, + RecoverySteps.UPDATE_PASSWORD); + } catch (NoSuchAlgorithmException e) { + throw Utils.handleServerException( + IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_NO_HASHING_ALGO_FOR_CODE, null); } catch (IdentityRecoveryException e) { - if (IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_CODE.getCode().equals( - e.getErrorCode())) { - e.setErrorCode(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID.getCode()); + try { + userRecoveryData = userAccountRecoveryManager.getUserRecoveryData(code, + RecoverySteps.UPDATE_PASSWORD); + } catch (IdentityRecoveryException ex) { + if (IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_CODE.getCode().equals( + ex.getErrorCode())) { + ex.setErrorCode(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID.getCode()); + } + throw ex; } - throw e; } - if (!StringUtils.equals(userRecoveryData.getRemainingSetIds(), - NotificationChannels.EMAIL_CHANNEL.getChannelType())) { + if (!(StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels.EMAIL_CHANNEL. + getChannelType()) || StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels. + EXTERNAL_CHANNEL.getChannelType()))) { throw Utils.handleClientException( IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID, confirmationCode); } From 4544a6c2c9ba28b4b2fa8911ab07a426c969552a Mon Sep 17 00:00:00 2001 From: Rashmini Date: Mon, 9 Oct 2023 13:21:22 +0530 Subject: [PATCH 2/3] Concat recovery flow id with the secret for EXTERNAL channel --- .../impl/password/PasswordRecoveryManagerImpl.java | 12 ++++++------ .../NotificationPasswordRecoveryManager.java | 12 ++++++------ .../wso2/carbon/identity/recovery/util/Utils.java | 5 +++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java index f12522f9a8..7dde3f6d02 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java @@ -295,9 +295,8 @@ public PasswordResetCodeDTO confirm(String otp, String confirmationCode, String IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_CODE.getCode(), IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_CODE.getMessage(), code); } catch (IdentityRecoveryException e) { - /* This method is to support already initiated email link based recovery flows using the recovery V1 API, - which do not have recovery flow ids, and handle recovery flows when the notifications are externally - managed. */ + /* This is a fallback logic to support already initiated email link based recovery flows and EXTERNAL + channel based recovery flows using the recovery V1 API, which do not have recovery flow ids. */ return validateConfirmationCode(userAccountRecoveryManager, recoveryFlowId, tenantDomain); } } @@ -893,9 +892,10 @@ private boolean isMinNoOfRecoveryQuestionsAnswered(String username, String tenan } /** - * This method is added to handle the already initiated email link based recovery flows which do not have - * recovery flow ids, which were initiated before moving to the Recovery V2 API. This also handles the - * recovery flows when the notifications are externally managed. + * This method is to validate the confirmation code when there's no recovery flow id. This is added as a fallback + * logic to handle the already initiated email link based recovery flows and EXTERNAL channel based recovery flows + * which do not have recovery flow ids, which were initiated before moving to the Recovery V2 API. + * This shouldn't be used for any other purpose and should be kept for sometime. * * @param userAccountRecoveryManager UserAccountRecoveryManager. * @param confirmationCode Confirmation code. diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java index aa19c6a60a..85faad57ec 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java @@ -715,9 +715,8 @@ public User updateUserPassword(String code, String confirmationCode, String pass IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_CODE.getMessage(), code); } } catch (IdentityRecoveryException e) { - /* This method is to support already initiated email link based recovery flows using the recovery V1 API, - which do not have recovery flow ids, and handle recovery flows when the notifications are externally - managed. */ + /* This is a fallback logic to support already initiated email link based recovery flows and EXTERNAL + channel based recovery flows using the recovery V1 API, which do not have recovery flow ids. */ userRecoveryData = validateUserRecoveryDataFromCode(code, confirmationCode, password, properties); } @@ -785,9 +784,10 @@ public User updateUserPassword(String code, String confirmationCode, String pass } /** - * This method is added to handle the already initiated email link based recovery flows which do not have - * recovery flow ids, which were initiated before moving to the Recovery V2 API. This also handles the - * recovery flows when the notifications are externally managed. + * This method is to validate user recovery data using the reset code when there's no recovery flow id. + * This is added as a fallback logic to handle the already initiated email link based recovery flows and EXTERNAL + * channel based recovery flows which do not have recovery flow ids, which were initiated before moving to the + * Recovery V2 API. This shouldn't be used for any other purpose and should be kept for sometime. * * @param code Password Reset code. * @param confirmationCode Confirmation code. diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java index 36692ba562..cdde41b271 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java @@ -1204,8 +1204,9 @@ public static String generateSecretKey(String channel, String tenantDomain, Stri */ public static String concatRecoveryFlowIdWithSecretKey(String recoveryFlowId, String notificationChannel, String secretKey) { - if (recoveryFlowId != null && StringUtils.equals(notificationChannel, - NotificationChannels.EMAIL_CHANNEL.getChannelType())) { + if (recoveryFlowId != null && (StringUtils.equals(notificationChannel, NotificationChannels.EMAIL_CHANNEL. + getChannelType()) || StringUtils.equals(notificationChannel, NotificationChannels.EXTERNAL_CHANNEL. + getChannelType()))) { secretKey = recoveryFlowId + IdentityRecoveryConstants.CONFIRMATION_CODE_SEPARATOR + secretKey; } return secretKey; From 6e6aa3151db682c8b69adc0ed7e8f068a8e713e2 Mon Sep 17 00:00:00 2001 From: Rashmini Date: Tue, 10 Oct 2023 09:47:58 +0530 Subject: [PATCH 3/3] Fix comments --- .../service/impl/password/PasswordRecoveryManagerImpl.java | 5 ++--- .../password/NotificationPasswordRecoveryManager.java | 5 ++--- .../java/org/wso2/carbon/identity/recovery/util/Utils.java | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java index 7dde3f6d02..0b1eb93bad 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/internal/service/impl/password/PasswordRecoveryManagerImpl.java @@ -928,9 +928,8 @@ private PasswordResetCodeDTO validateConfirmationCode(UserAccountRecoveryManager throw ex; } } - if (!(StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels.EMAIL_CHANNEL. - getChannelType()) || StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels. - EXTERNAL_CHANNEL.getChannelType()))) { + if (!(NotificationChannels.EMAIL_CHANNEL.getChannelType().equals(userRecoveryData.getRemainingSetIds()) || + NotificationChannels.EXTERNAL_CHANNEL.getChannelType().equals(userRecoveryData.getRemainingSetIds()))) { throw Utils.handleClientException( IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID, confirmationCode); } diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java index 85faad57ec..8ff31ae33d 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/password/NotificationPasswordRecoveryManager.java @@ -821,9 +821,8 @@ private UserRecoveryData validateUserRecoveryDataFromCode(String code, String co throw ex; } } - if (!(StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels.EMAIL_CHANNEL. - getChannelType()) || StringUtils.equals(userRecoveryData.getRemainingSetIds(),NotificationChannels. - EXTERNAL_CHANNEL.getChannelType()))) { + if (!(NotificationChannels.EMAIL_CHANNEL.getChannelType().equals(userRecoveryData.getRemainingSetIds()) || + NotificationChannels.EXTERNAL_CHANNEL.getChannelType().equals(userRecoveryData.getRemainingSetIds()))) { throw Utils.handleClientException( IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_RECOVERY_FLOW_ID, confirmationCode); } diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java index cdde41b271..43a4d57e6e 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java @@ -1204,9 +1204,8 @@ public static String generateSecretKey(String channel, String tenantDomain, Stri */ public static String concatRecoveryFlowIdWithSecretKey(String recoveryFlowId, String notificationChannel, String secretKey) { - if (recoveryFlowId != null && (StringUtils.equals(notificationChannel, NotificationChannels.EMAIL_CHANNEL. - getChannelType()) || StringUtils.equals(notificationChannel, NotificationChannels.EXTERNAL_CHANNEL. - getChannelType()))) { + if (recoveryFlowId != null && (NotificationChannels.EMAIL_CHANNEL.getChannelType().equals(notificationChannel) + || NotificationChannels.EXTERNAL_CHANNEL.getChannelType().equals(notificationChannel))) { secretKey = recoveryFlowId + IdentityRecoveryConstants.CONFIRMATION_CODE_SEPARATOR + secretKey; } return secretKey;