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

[xenial] fix 2FA functionality in app code #4037

Closed
redshiftzero opened this issue Jan 16, 2019 · 4 comments · Fixed by #4069
Closed

[xenial] fix 2FA functionality in app code #4037

redshiftzero opened this issue Jan 16, 2019 · 4 comments · Fixed by #4069
Assignees

Comments

@redshiftzero
Copy link
Contributor

Description

2FA unit tests indicate that our 2FA functionality isn't working on xenial, this needs investigation.

Steps to Reproduce

make test-xenial

Expected Behavior

All 2FA related tests pass

Actual Behavior

many 2FA related tests are failing:

tests/test_2fa.py::test_totp_reuse_protections FAILED
tests/test_2fa.py::test_totp_reuse_protections2 FAILED
tests/test_2fa.py::test_bad_token_fails_to_verify_on_admin_new_user_two_factor_page FAILED
tests/test_2fa.py::test_bad_token_fails_to_verify_on_new_user_two_factor_page FAILED
...
tests/test_journalist_api.py::test_user_cannot_get_an_api_token_with_wrong_2fa_token FAILED
kushaldas added a commit that referenced this issue Jan 17, 2019
With the upgraded setuptools package, the errors
in both the mentioned issues get fixed.
kushaldas added a commit that referenced this issue Jan 17, 2019
With the upgraded setuptools package, the errors
in both the mentioned issues get fixed.
@redshiftzero
Copy link
Contributor Author

redshiftzero commented Jan 18, 2019

Let me know if I'm missing something, but #4041 doesn't solve this (2FA tests are still failing in the xenial app test job) no? (which is good in one sense because the workaround here will work)

@redshiftzero
Copy link
Contributor Author

Hmm - I'm on develop and now not seeing these failures...

@redshiftzero
Copy link
Contributor Author

Update from test run on ci-xenial-2fa-test - they pass locally, fail in CI, the best kind of test failure ;)

@redshiftzero redshiftzero self-assigned this Jan 24, 2019
kushaldas added a commit that referenced this issue Jan 24, 2019
With the upgraded setuptools package, the errors
in both the mentioned issues get fixed.
@redshiftzero
Copy link
Contributor Author

redshiftzero commented Jan 24, 2019

The reason why these failures are happening is because an exception is occurring during the functional test setup (specifically when setting up the driver) which is ticket #4039. When an exception occurs during setup, the teardown method will not run. This is important because the functional test setup patches the models.Journalist.verify_token method, which will stay patched for the remaining tests if the functional test teardown (which removes the patching) is not executed. Hence the 2FA test failures later in the test run. That's why there was deviation in behavior between CI and locally: in the CI run the functional test run occurred prior to the 2FA tests. The solution is to ensure that teardown is called if an exception occurs setting up the driver.

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

Successfully merging a pull request may close this issue.

1 participant