Skip to content
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

Email address with leading/trailing whitespace treated as different identifier #2158

Closed
3 of 6 tasks
meyfa opened this issue Jan 18, 2022 · 4 comments · Fixed by #2160
Closed
3 of 6 tasks

Email address with leading/trailing whitespace treated as different identifier #2158

meyfa opened this issue Jan 18, 2022 · 4 comments · Fixed by #2160
Labels
bug Something is not working.

Comments

@meyfa
Copy link
Contributor

meyfa commented Jan 18, 2022

Preflight checklist

Describe the bug

Suppose an account exists for "[email protected]", then it is not possible to log in using "[email protected] " (same string but with trailing whitespace, which often happens on mobile phones with auto-completion) or " [email protected]" (leading whitespace). In both cases, the error message reads "The provided credentials are invalid, check for spelling mistakes in your password or username, email address, or phone number.". I think whitespace around the identifier should be ignored.

Even worse, entirely new accounts can be registered for all of the following:

etc.

Interestingly, trying to register an account for "[email protected] " (trailing space instead of leading) will fail, as it should. The error message there reads "\"[email protected] \" is not valid \"email\"".

Reproducing the bug

Create an account for "[email protected]" and verify that logging into it works, then log out again.

At this point the following things will fail, although they should succeed:

The following will work, although it should fail:

The following will correctly fail:

(Note that the quotation marks are not part of user input. I had to add them to deal with Markdown formatting issues.)

Relevant log output

No response

Relevant configuration

{
  "$id": "https://schemas.ory.sh/presets/kratos/quickstart/email-password/identity.schema.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Person",
  "type": "object",
  "properties": {
    "traits": {
      "type": "object",
      "properties": {
        "email": {
          "type": "string",
          "format": "email",
          "title": "E-Mail",
          "minLength": 3,
          "ory.sh/kratos": {
            "credentials": {
              "password": {
                "identifier": true
              }
            },
            "verification": {
              "via": "email"
            }
          }
        },
        "name": {
          "type": "string",
          "title": "Full name"
        }
      },
      "required": [
        "email",
        "name"
      ],
      "additionalProperties": false
    }
  }
}

Version

v0.8.0-alpha.3

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes with Helm

Additional Context

No response

@meyfa meyfa added the bug Something is not working. label Jan 18, 2022
@aeneasr
Copy link
Member

aeneasr commented Jan 18, 2022

Nice find, we should probably add some type of sanitation (I think there's another similar issue tracked here already). A simple strings.TrimSpaces would be enough for email addresses. Question is if this should also be the case for non-emails?

@meyfa
Copy link
Contributor Author

meyfa commented Jan 18, 2022

There is #814, which dealt with capitalization (but that was already fixed). Personally, I think that whitespace should be ignored for regular usernames, too, i.e. the sanitation behavior for whitespace and capitalization should be the same.

@zepatrik
Copy link
Member

I can confirm that especially apple mobile devices add whitespace after word completion, which is not obvious to users. Experienced that once myself, took me quite some time to figure it out. That issue will also be true for usernames.

@meyfa
Copy link
Contributor Author

meyfa commented Jan 19, 2022

I just noticed a mistaken assumption in my initial comment:

Interestingly, trying to register an account for "[email protected] " (trailing space instead of leading) will fail, as it should. The error message there reads "\"[email protected] \" is not valid \"email\"".

While I think it is correct to fail in the described scenario (where "[email protected]" already exists) it is NOT correct to fail due to an invalid format. Word completion will also exist for registration forms and users should be able to register by entering "[email protected] ". This might be an entirely different issue more to do with JSON schema validation though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants