fix: Proper exception handling of InvalidPasswordLength

[prateekj117]

Status

Ready for review

Description of Changes

When entering passphrase of more than 128 (max_limit) characters, the user lands to the Internal Server Error Page.

Fixes #5145

Changes proposed in this pull request:

Handle the exception properly so that the server returns with the flashed error message.

Testing

• check out this branch
• Run make dev - it completes successfully
• Attempt to log into the journalist interface on localhost:8081 with a valid username <userName> and TOTP code, and a password longer than 127 characters:
  • Login fails with Login failed. Please wait for a new code from your two-factor mobile app or security key before trying again message.
  • Error recorded in Docker output with contents: [<timestamp>] ERROR in utils: Login for '<userName>' failed: invalid password
• Attempt to log in to the JI with valid username, TOTP, and password:
  • Login succeeds.

Deployment

Checklist

If you made changes to the server application code:

• Linting (make lint) and tests (make test) pass in the development container

If you made non-trivial code changes:

• I have written a test plan and validated it for this PR

[zenmonkeykstop]

Hi @prateekj117 and thanks for this fix! I've added a simple test plan to the PR description, hope that's OK. In general, PRs to this project should include the sections laid out in the template - it helps speed the review process.

The change looks good at first glance, but if you have time, could you add a test to cover this behaviour? Take a look at this test for an example.

[prateekj117]

@zenmonkeykstop Ya, sure. I will keep that in mind. I will add the test as well. Thanks for the review !!!

[zenmonkeykstop]

Change looks good, a test covering the behavior change in this case would be great.

[rmol]

@prateekj117 Maybe I can save you some time. This could go in tests/test_journalist.py:

def test_login_password_too_long(journalist_app, test_journo, mocker):
    mocked_error_logger = mocker.patch('journalist.app.logger.error')
    with journalist_app.test_client() as app:
        resp = app.post(url_for('main.login'),
                        data=dict(username=test_journo['username'],
                                  password='a' * (Journalist.MAX_PASSWORD_LEN + 1),
                                  token=TOTP(test_journo['otp_secret']).now()))
    assert resp.status_code == 200
    text = resp.data.decode('utf-8')
    assert "Login failed" in text
    mocked_error_logger.assert_called_once_with(
        "Login for '{}' failed: Password too long (len={})".format(
            test_journo['username'], Journalist.MAX_PASSWORD_LEN + 1))

[prateekj117]

@rmol Thanks a lot. Though I would have definitely enjoyed writting this logic myself and learning a few more things along the way.

[rmol]

@prateekj117 Well, drat. I could delete the comment. 😉

That's good to know; I will not deprive you of the pleasure of writing tests for your next PR. 🙂

[prateekj117]

@zenmonkeykstop Error recorded in Docker output is ERROR in utils: Login for 'journalist' failed: Password too long (len=624), rather than the one you mentioned in PR template. Not an issue though I think.

@rmol Thanks for the test.

[zenmonkeykstop]

CI is passing with the new test, this is good to go!