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

Login - Entering incorrect and short password will return wrong error message #4292

Closed
isagoico opened this issue Jul 29, 2021 · 13 comments
Closed
Assignees
Labels
Daily KSv2 Engineering Reviewing Has a PR in review

Comments

@isagoico
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open the app and enter e-mail to login
  2. Type a incorrect and short (3 or less characters) passwords, like aaa or 123

Expected Result:

User gets error saying password is incorrect or password is too short

Actual Result:

User gets 2FA error which is unrelated to the issue

Workaround:

N/A

Platform:

Where is this issue occurring?

  • Web ✔️
  • iOS✔️
  • Android✔️
  • Desktop App✔️
  • Mobile Web✔️

Version Number: 1.0.81-2

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

@MelvinBot
Copy link

Triggered auto assignment to @joelbettner (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@aman-atg
Copy link
Contributor

aman-atg commented Jul 29, 2021

The API's returning Status 402 for short passwords instead of 401 or 403 (preferred)
This needs to be fixed on the backend.

@joelbettner
Copy link
Contributor

@isagoico just to confirm, this happened when you were attempting to log in with gI3utest@gmail.com, correct?

(I can't tell if the second letter is a lower-case "L" or an upper-case "I")

@isagoico
Copy link
Author

isagoico commented Jul 29, 2021

Mm let me confirm with the tester. I was also able to reproduce this with my account isagoicotest@gmail.com

Edit: is a lowercase L

@joelbettner
Copy link
Contributor

I think I found the logs for this:

https://www.expensify.com/_devportal/tools/logSearch/#query=request_id:(%226761fa61de42601e-GRU%22)+AND+timestamp:[2021-07-28T22:44:54.625Z+TO+2021-07-29T00:44:54.625Z]&index=logs_expensify-011753

I'm confused, though. It is showing that we threw a 402 because there was no partnerUserSecret included in the request (and that means we threw from this check: https://github.com/Expensify/Auth/blob/master/auth/command/Authenticate.cpp#L84)

 Throw ExpException - a06cf65ac10bfccdfafc24d069935cfb ~~ message: '402 Missing partnerUserSecret' exceptionMessage: 'Auth Authenticate returned an error' 

That leads me to believe that, even though the password is being entered, it is not being passed to the backend for authentication. However, if that were the case, nobody would be able to enter a password and log in.

So...I'm really not sure what is causing this at the moment.

@isagoico
Copy link
Author

This is also reproducible when entering 1 or 2 characters in the password field. As a side note, I'm also able to log with with my account (isagoicotest@gmail.com) when entering the correct password.

Screenshot_20210729_153403_com expensify chat

@joelbettner
Copy link
Contributor

So, there is a threshold over which the correct error message is shown.

I'll keep digging, but something funky is happening here...

@joelbettner
Copy link
Contributor

joelbettner commented Jul 29, 2021

If I enter an incorrect password that is 3 characters or less I get the following message:
image

If I enter an incorrect password that is 4 characters or more I get the correct error message:
image

I would expect that I should not be getting that incorrect first message at all, because we have a minimum for the password length set in auth as 1 character:

Request::verifyAttributeSize(request, "partnerUserSecret", 1, Request::MAX_SIZE_SMALL);
void Request::verifyAttributeSize(const SData& request, const char* key, int64_t minSize, int64_t maxSize)
{
    if (request[key].size() < (size_t) minSize) {
        STHROW("402 Missing " + string(key));
    }
 Throw ExpException - a06cf65ac10bfccdfafc24d069935cfb ~~ message: '402 Missing partnerUserSecret'

@joelbettner
Copy link
Contributor

Ok...I've found out what is going on...

The WAF is preventing passwords that are less than 4 characters from being passed from a client to the API, and in that case simply passes an empty password:
https://github.com/Expensify/Web-Expensify/blame/master/_inputrules.php#L19

I've got a fix coming...

@joelbettner joelbettner added the Reviewing Has a PR in review label Jul 29, 2021
@Jag96 Jag96 added Reviewing Has a PR in review and removed Reviewing Has a PR in review labels Jul 30, 2021
@isagoico
Copy link
Author

isagoico commented Aug 4, 2021

@joelbettner issue is fixed! I think we are good to close this one 🎉

@MelvinBot
Copy link

@joelbettner Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@MelvinBot
Copy link

@joelbettner Still overdue 6 days?! Let's take care of this!

@MelvinBot
Copy link

@joelbettner 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

5 participants