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.

Sign up for GitHub

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

tests: Ensure authenticate returns newly created user #380

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

tests: Ensure authenticate returns newly created user #380

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions tests/test_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,9 +447,9 @@ def test_successful_authentication_new_user(self, token_mock, request_mock, algo
'redirect_uri': 'http://testserver/callback/',
}
self.assertEqual(User.objects.all().count(), 0)
self.backend.authenticate(request=auth_request)
user = self.backend.authenticate(request=auth_request)
self.assertEqual(User.objects.all().count(), 1)
user = User.objects.all()[0]
self.assertEqual(user, User.objects.get())
self.assertEqual(user.email, '[email protected]')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cfra for this PR.
If the authenticate method returned None here the test would fail. This test is about authenticating and creating a new user. If authenticate was failing somewhere in the flow the user creation wouldn't have happened. In that case the assertEqual would fail because the queryset would have returned an empty set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test doesn't necessarily fail if it is just the return value that is omitted:

[nihilus@hermes mozilla-django-oidc]$ TOXENV=py38-django220 tox -- tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_new_user
GLOB sdist-make: /home/nihilus/coding/mozilla-django-oidc/setup.py
py38-django220 inst-nodeps: /home/nihilus/coding/mozilla-django-oidc/.tox/.tmp/package/1/mozilla-django-oidc-1.2.4.zip
py38-django220 installed: certifi==2020.6.20,cffi==1.14.3,chardet==3.0.4,cryptography==3.1.1,Django==2.2.16,djangorestframework==3.12.1,idna==2.10,josepy==1.4.0,mock==2.0.0,mozilla-django-oidc==1.2.4,pbr==5.5.0,pycparser==2.20,pyOpenSSL==19.1.0,pytz==2020.1,requests==2.24.0,six==1.15.0,sqlparse==0.4.1,urllib3==1.25.10
py38-django220 run-test-pre: PYTHONHASHSEED='1952038455'
py38-django220 run-test: commands[0] | django-admin.py test tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_new_user
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.
----------------------------------------------------------------------
Ran 1 test in 0.008s

OK
Destroying test database for alias 'default'...
_________________________________________________________________________________________________________________________________________________________________________________ summary __________________________________________________________________________________________________________________________________________________________________________________
  py38-django220: commands succeeded
  congratulations :)
[nihilus@hermes mozilla-django-oidc]$ git apply
diff --git a/mozilla_django_oidc/auth.py b/mozilla_django_oidc/auth.py
index b211571..a275693 100644
--- a/mozilla_django_oidc/auth.py
+++ b/mozilla_django_oidc/auth.py
@@ -327,7 +327,6 @@ class OIDCAuthenticationBackend(ModelBackend):
             raise SuspiciousOperation(msg)
         elif self.get_settings('OIDC_CREATE_USER', True):
             user = self.create_user(user_info)
-            return user
         else:
             LOGGER.debug('Login failed: No user with %s found, and '
                          'OIDC_CREATE_USER is False',
[nihilus@hermes mozilla-django-oidc]$ TOXENV=py38-django220 tox -- tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_new_user
GLOB sdist-make: /home/nihilus/coding/mozilla-django-oidc/setup.py
py38-django220 inst-nodeps: /home/nihilus/coding/mozilla-django-oidc/.tox/.tmp/package/1/mozilla-django-oidc-1.2.4.zip
py38-django220 installed: certifi==2020.6.20,cffi==1.14.3,chardet==3.0.4,cryptography==3.1.1,Django==2.2.16,djangorestframework==3.12.1,idna==2.10,josepy==1.4.0,mock==2.0.0,mozilla-django-oidc==1.2.4,pbr==5.5.0,pycparser==2.20,pyOpenSSL==19.1.0,pytz==2020.1,requests==2.24.0,six==1.15.0,sqlparse==0.4.1,urllib3==1.25.10
py38-django220 run-test-pre: PYTHONHASHSEED='2719431438'
py38-django220 run-test: commands[0] | django-admin.py test tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_newRevert "tests: Ensure authenticate returns newly created user"
_user
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
F
======================================================================
FAIL: test_successful_authentication_new_user (tests.test_auth.OIDCAuthenticationBackendTestCase)
Test successful authentication and user creation.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/nihilus/coding/mozilla-django-oidc/.tox/py38-django220/lib/python3.8/site-packages/mock/mock.py", line 1305, in patched
    return func(*args, **keywargs)
  File "/home/nihilus/coding/mozilla-django-oidc/tests/test_auth.py", line 452, in test_successful_authentication_new_user
    self.assertEqual(user, User.objects.get())
AssertionError: None != <User: username_algo>

----------------------------------------------------------------------
Ran 1 test in 0.009s

FAILED (failures=1)
Destroying test database for alias 'default'...
ERROR: InvocationError for command /home/nihilus/coding/mozilla-django-oidc/.tox/py38-django220/bin/django-admin.py test tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_new_user (exited with code 1)
_________________________________________________________________________________________________________________________________________________________________________________ summary __________________________________________________________________________________________________________________________________________________________________________________
ERROR:   py38-django220: commands failed
[nihilus@hermes mozilla-django-oidc]$ git revert 45d546cbc26a565d7f8fae58667c21ea421610dc
Auto-merging tests/test_auth.py
[fix/test-backend-authenticate 45ec08d] Revert "tests: Ensure authenticate returns newly created user"
 1 file changed, 2 insertions(+), 2 deletions(-)
[nihilus@hermes mozilla-django-oidc]$ TOXENV=py38-django220 tox -- tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_new_user
GLOB sdist-make: /home/nihilus/coding/mozilla-django-oidc/setup.py
py38-django220 inst-nodeps: /home/nihilus/coding/mozilla-django-oidc/.tox/.tmp/package/1/mozilla-django-oidc-1.2.4.zip
py38-django220 installed: certifi==2020.6.20,cffi==1.14.3,chardet==3.0.4,cryptography==3.1.1,Django==2.2.16,djangorestframework==3.12.1,idna==2.10,josepy==1.4.0,mock==2.0.0,mozilla-django-oidc==1.2.4,pbr==5.5.0,pycparser==2.20,pyOpenSSL==19.1.0,pytz==2020.1,requests==2.24.0,six==1.15.0,sqlparse==0.4.1,urllib3==1.25.10
py38-django220 run-test-pre: PYTHONHASHSEED='2072510053'
py38-django220 run-test: commands[0] | django-admin.py test tests.test_auth.OIDCAuthenticationBackendTestCase.test_successful_authentication_new_user
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.
----------------------------------------------------------------------
Ran 1 test in 0.008s

OK
Destroying test database for alias 'default'...
_________________________________________________________________________________________________________________________________________________________________________________ summary __________________________________________________________________________________________________________________________________________________________________________________
  py38-django220: commands succeeded
  congratulations :)

However, I have to admit that this error is also caught by other tests.

If you want to argue that verification of the return value is out of scope for this particular test, and should only be validated elsewhere, feel free to close this PR.

self.assertEqual(user.username, 'username_algo')

Expand Down