Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 22, 2025

What do these changes do?

Resolves an unhandled exception in the registration flow . Instead of returning a generic 500 error (see below), the system now responds with an appropriate 4XX status and a clear, descriptive message.

image

Auto-description

This pull request improves the robustness and user experience of the registration and authentication flows in the login service. It introduces centralized exception handling for several registration endpoints and refines user-facing messages to be clearer and more helpful.

Error handling improvements:

  • Added the @handle_rest_requests_exceptions decorator to the check_registration_invitation, register, and register_phone endpoints in registration.py to ensure consistent and centralized error handling for registration-related REST requests. [1] [2] [3] [4]

User message enhancements:

  • Updated several user-facing messages in constants.py to improve clarity and guidance, including messages related to two-factor authentication availability, authentication failures, expired or incorrect verification codes, password errors, and password strength requirements. [1] [2] [3]

Related issue/s

  • Detected by @giancarloromeo during registration in a different produce within a deploy that already had his account registered.

How to test

  • No reviewer will ask for this. So I am checking here if someone reads this line ;-)

Dev-ops

None

@pcrespov pcrespov self-assigned this Oct 22, 2025
@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Oct 22, 2025
@pcrespov pcrespov added this to the Imparable milestone Oct 22, 2025
@pcrespov pcrespov changed the title WIP: 🐛 Fix/handler wrong password WIP: 🐛 [BugFix] Properly handle incorrect passwords for existing accounts during product registration Oct 22, 2025
@pcrespov pcrespov changed the title WIP: 🐛 [BugFix] Properly handle incorrect passwords for existing accounts during product registration 🐛 [BugFix] Properly handle incorrect passwords for existing accounts during product registration Oct 22, 2025
@pcrespov pcrespov marked this pull request as ready for review October 22, 2025 16:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an unhandled exception during product registration when users provide incorrect passwords for existing accounts. Instead of a generic 500 error, the system now returns appropriate 4XX responses with clear error messages.

Key changes:

  • Added centralized exception handling to registration endpoints to properly catch and handle authentication errors
  • Improved clarity and consistency of user-facing error messages across authentication flows

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
constants.py Updated error messages for 2FA, authentication failures, verification codes, and password requirements to be more user-friendly and descriptive
registration.py Added @handle_rest_requests_exceptions decorator to three registration endpoints for consistent error handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@pcrespov pcrespov requested a review from odeimaiz October 22, 2025 16:33
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.28%. Comparing base (13d4661) to head (6ef7188).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8548      +/-   ##
==========================================
- Coverage   87.50%   84.28%   -3.22%     
==========================================
  Files        2011      784    -1227     
  Lines       78601    35656   -42945     
  Branches     1349      175    -1174     
==========================================
- Hits        68776    30054   -38722     
+ Misses       9421     5545    -3876     
+ Partials      404       57     -347     
Flag Coverage Δ
integrationtests 64.00% <100.00%> (+0.14%) ⬆️
unittests 86.32% <100.00%> (+0.09%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 76.69% <ø> (-8.21%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.33% <ø> (-12.56%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 81.89% <ø> (-8.55%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 87.07% <100.00%> (+0.02%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13d4661...6ef7188. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Oct 22, 2025

🧪 CI Insights

Here's what we observed from your CI run for 6ef7188.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI integration-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
unit-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Oct 22, 2025
@pcrespov pcrespov enabled auto-merge (squash) October 22, 2025 17:17
@pcrespov
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Oct 22, 2025

queue

🛑 Configuration not compatible with a branch protection setting

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.

Copy link
Contributor

@giancarloromeo giancarloromeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks 🙏

I've read the message in the "How to Test" section.✋😁

@odeimaiz
Copy link
Member

Many thanks 🙏

I've read the message in the "How to Test" section.✋😁

☝️

@sonarqubecloud
Copy link

@pcrespov pcrespov merged commit f7f861b into ITISFoundation:master Oct 23, 2025
92 of 95 checks passed
@pcrespov pcrespov deleted the fix/handler-wrong-password branch October 23, 2025 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants