codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
24 stars 23 forks source link

[Bug Report] - Error on duplicate logins not reported to user #638

Closed AJSterner closed 3 weeks ago

AJSterner commented 1 month ago

Describe the bug:

Attempting to create a second pod with the same email address fails however this isn't reported to the user. The "Creating Pod..." message shows forever.

Expected behavior:

As a user I would expect an error to show telling me that the pod couldn't be created or that the email was already used (and potentially suggestion to reset password).

Actual Behavior:

The "Creating Pod..." message shows forever.

Screenshots:

If applicable, add screenshots to help explain your problem.

[1] 2024-05-22T01:58:44.806Z [HandlerServerConfigurator] {Primary} info: Received POST request for /.account/account/54752f10-9943-406c-9a68-c6639e98d788/login/password/
[1] 2024-05-22T01:58:44.843Z [BasePasswordStore] {Primary} warn: Trying to create duplicate login for email andrewjohnsterner@gmail.com
[1] 2024-05-22T01:58:44.855Z [HandlerServerConfigurator] {Primary} info: Received OPTIONS request for /.account/account/54752f10-9943-406c-9a68-c6639e98d788/pod/
[1] 2024-05-22T01:58:44.857Z [HandlerServerConfigurator] {Primary} info: Received POST request for /.account/account/54752f10-9943-406c-9a68-c6639e98d788/pod/
[1] 2024-05-22T01:58:44.887Z [BaseLoginAccountStorage] {Primary} warn: Trying to update account 54752f10-9943-406c-9a68-c6639e98d788 without login methods
[1] 2024-05-22T01:58:44.896Z [HandlerServerConfigurator] {Primary} info: Received OPTIONS request for /.account/login/password/
[1] 2024-05-22T01:58:44.897Z [HandlerServerConfigurator] {Primary} info: Received POST request for /.account/login/password/
[1] 2024-05-22T01:58:45.049Z [BasePasswordStore] {Primary} warn: Incorrect password for email andrewjohnsterner@gmail.com

image

To Reproduce:

Steps to reproduce the behavior:

  1. Go to the main page
  2. Try to create a pod with the same email as a pod that already exists
  3. See no error

Desktop (please complete the following information):

Possible Fix:

We know the account exists, we should return an error to the user.

Additional context:

Add any other context about the problem here.

JaeGif commented 1 month ago

👋. Are we looking to add a check for duplicate email addresses when the user is typing, or after the user attempts to submit the form (or both)? Either seems reasonable to me. Leaning toward just the notification on submission.

andycwilliams commented 1 month ago

👋. Are we looking to add a check for duplicate email addresses when the user is typing, or after the user attempts to submit the form (or both)? Either seems reasonable to me. Leaning toward just the notification on submission.

I vote for notification only after submitting.

JaeGif commented 1 month ago

@leekahung @andycwilliams

There's a lot of ways I can see this being implemented. I tried to implement it in a way that can be extended to other error messages, as duplicate username is likely not the only case available.

I added a quick fix to branch 638/error-on-login that uses a new function handleIncomingHTTPErrors to throw an error when the error is received. We can add other error codes and messages to the function to display a different message when a different error occurs.

Right now I've wrapped registerPod in a try/catch to receive any potential errors and update the state of the signup form with new errors - triggering a basic notification.

Code Below vvv

export default function handleIncomingHTTPErrors({ message, statusCode }) {
  // extensible with new error codes and messages
  if (statusCode !== 400) return;

  switch (message) {
    case 'There already is a login for this e-mail address.':
      throw new Error(message);
    default:
      break;
  }
}
...
  const createControlsResp = await fetch(createControls.password.create, {
    method: 'POST',
    headers: createHeaders,
    body: JSON.stringify({ email, password })
  });
  handleIncomingHTTPErrors(await createControlsResp.json());
...
...
  const registerAndInitialize = async (email, password, confirmPassword) => {
    setStep('loading');
    try {
      const registration = await registerPod(
        {
          email,
          password,
          confirmPassword
        },
        oidcIssuer
      );
      setRegistrationInfo(registration);
      const caseManagerNames = caseManagerName?.split(' ') || [];
      await initializePod(
        registration.webId,
        registration.podUrl,
        {
          caseManagerWebId,
          caseManagerFirstName: caseManagerNames[0],
          caseManagerLastName: caseManagerNames[caseManagerNames.length - 1]
        },
        registration.fetch
      );

      setStep('done');
      setHttpError(null);
    } catch (error) {
      setHttpError(error);
      setStep('begin');
    }
...

... {httpError && <BasicNotification message={httpError.message} id={5} severity="error" />} ...

I can't help but feel that there's a better system for catching errors here somewhere but I don't have time to work it out at the moment. If this solution is good with you guys I'll spin up some unit tests for it and leave it as is for now.

You can check out the current functionality on the branch and let me know what you think.