-
Notifications
You must be signed in to change notification settings - Fork 220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change execution order to ensure the account is locked in case of a notification sending failure #895
Conversation
…otification error
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #895 +/- ##
============================================
- Coverage 50.45% 48.74% -1.71%
- Complexity 2380 2532 +152
============================================
Files 297 300 +3
Lines 17767 19104 +1337
Branches 2516 2878 +362
============================================
+ Hits 8965 9313 +348
- Misses 7662 8618 +956
- Partials 1140 1173 +33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
PR builder started |
PR builder completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12351339821
PR builder started |
PR builder completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12647435299
throw new IdentityEventException("Error while lock user account :" + user.getUserName(), e); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a return from here https://github.com/wso2-extensions/identity-governance/pull/895/files#diff-8a0d606a915a355053e3ac3ab16564269874a60d46fa5bf2cee7591fceca8658R162. We may have to consider that logic before locking the account. Only the notification sending part is required to be moved down.
PR builder started |
PR builder completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12860789150
Proposed changes in this pull request
This PR resolves an issue in the self-registration flow where user accounts would remain unlocked if the account confirmation notification failed to send. When account confirmation is enabled, the user's account should be locked until they verify it. However, if a server error occurs during notification delivery (e.g., missing account confirmation email template), the account remains unlocked, leading to unintended behavior.
Changes Made
Issue
wso2/product-is#21290