fix(): 404 if a trainer tries to login to a different gym

Rather than just returning "permission denied", we can interpret this scenario
as a "failure to look up a user with that gym/user ID combo".

This should give users the information they need to self-recover, while not
leaking any other sensitive details.

Fixes: wger-project#585

.github/contributing.md

@@ -28,9 +28,9 @@ describe a use-case or similar.
 ## Pull Requests
 You want to contribute code? Awesome! Here are some tips:
 
-* Make sure the tests are running (``python3 manage.py tests``)...
+* Make sure the tests are running (``python3 manage.py test``)...
 * ... and if you write new code, write new tests
-* Lint the code with flake8 (``flake8 --config .github/linters/.flake8``)
+* Lint the code with flake8 (``flake8 --config .github/linters/.flake8 ./wger``)
 and isort (``isort``) 
 * You can expect a response from a maintainer within a week, if you
 haven’t heard anything by then, feel free to ping the thread.

.github/pull_request_template.md

@@ -7,7 +7,7 @@
 ### Please check that the PR fulfills these requirements
 
 - [ ] Tests for the changes have been added (for bug fixes / features)
-- [ ] New python code has been linted with with flake8 (``flake8 --config .github/linters/.flake8``)
+- [ ] New python code has been linted with with flake8 (``flake8 --config .github/linters/.flake8 ./wger``)
 and isort (``isort``) 
 - [ ] Added yourself to AUTHORS.rst
 

.gitignore

@@ -65,6 +65,7 @@ node_modules
 
 # Virtual envs
 venv
+venv-wget
 
 # Dummy file for translations
 /wger/i18n.tpl

AUTHORS.rst

@@ -52,7 +52,7 @@ Developers
 * Aaron Bassett - https://github.com/aaronbassett
 * Rachael Wright-Munn - https://github.com/ChaelCodes
 * Will Pearson - https://github.com/qagoose
-
+* Brad Johnson - https://github.com/bradsk88
 
 Translators
 -----------
@@ -155,4 +155,4 @@ Translators
 Exercises
 ---------
 
-And of course many thanks as well to everyone that submited exercises!
+And of course many thanks as well to everyone that submitted exercises!

README.md

@@ -53,7 +53,9 @@ persists the database on a volume. From the root folder just call
 docker-compose -f extras/docker/compose/docker-compose.yml up
 ````
 
-For more info, check the README in wger/extras/compose.
+For more info, check the [README in wger/extras/docker/compose](
+ ./extras/docker/compose/README.md
+).
 
 #### Local installation (git)
 

wger/core/tests/test_user_login.py

@@ -0,0 +1,78 @@
+from unittest import mock
+
+from wger.core.tests.base_testcase import WgerTestCase
+from wger.core.views.user import trainer_login
+
+
+def _build_mock_request(user):
+    request = mock.Mock()
+    request.session = dict()
+    request.GET = dict()
+    request.user = user
+    return request
+
+
+def _build_mock_user(gym_name, is_trainer=False):
+    user = mock.Mock()
+    user.userprofile.gym = gym_name
+
+    def request_has_perm(perm):
+        if perm in ['gym.gym_trainer', 'gym.manage_gym', 'gym.manage_gyms']:
+            return is_trainer
+        return True  # Don't care about other permissions for these tests
+
+    user.has_perm.side_effect = request_has_perm
+    return user
+
+
+class TrainerLoginTestCase(WgerTestCase):
+
+    mock_django_login = None
+
+    @classmethod
+    def setUpClass(cls):
+        cls.mock_django_login = mock.patch('wger.core.views.user.django_login').start()
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.mock_django_login.stop()
+
+    def test_trainer_is_allowed_to_login_to_non_trainer_in_same_gym(self):
+        request_user = _build_mock_user('same-gym', is_trainer=True)
+        request = _build_mock_request(request_user)
+        user_from_db_lookup = _build_mock_user('same-gym', is_trainer=False)
+
+        with mock.patch('wger.core.views.user.get_object_or_404', return_value=user_from_db_lookup):
+            resp = trainer_login(request, 'primary-key-not-needed-because-get-object-is-mocked')
+
+        self.assertEqual(302, resp.status_code)
+
+    def test_trainer_is_denied_from_login_to_trainer_in_same_gym(self):
+        request_user = _build_mock_user('same-gym', is_trainer=True)
+        request = _build_mock_request(request_user)
+        user_from_db_lookup = _build_mock_user('same-gym', is_trainer=True)
+
+        with mock.patch('wger.core.views.user.get_object_or_404', return_value=user_from_db_lookup):
+            resp = trainer_login(request, 'primary-key-not-needed-because-of-mock')
+
+        self.assertEqual(403, resp.status_code)
+
+    def test_trainer_is_denied_from_login_to_trainer_at_different_gym(self):
+        request_user = _build_mock_user('trainer-gym', is_trainer=True)
+        request = _build_mock_request(request_user)
+        user_from_db_lookup = _build_mock_user('other-trainer-gym', is_trainer=True)
+
+        with mock.patch('wger.core.views.user.get_object_or_404', return_value=user_from_db_lookup):
+            resp = trainer_login(request, 'primary-key-not-needed-because-of-mock')
+
+        self.assertEqual(403, resp.status_code)
+
+    def test_trainer_gets_404_when_trying_to_login_to_non_trainer_in_different_gym(self):
+        request_user = _build_mock_user('trainer-gym', is_trainer=True)
+        request = _build_mock_request(request_user)
+        user_from_db_lookup = _build_mock_user('user-gym', is_trainer=False)
+
+        with mock.patch('wger.core.views.user.get_object_or_404', return_value=user_from_db_lookup):
+            resp = trainer_login(request, 'primary-key-not-needed-because-of-mock')
+
+        self.assertEqual(404, resp.status_code)

wger/core/views/user.py

@@ -39,7 +39,8 @@
 )
 from django.http import (
     HttpResponseForbidden,
-    HttpResponseRedirect
+    HttpResponseRedirect,
+    HttpResponseNotFound,
 )
 from django.shortcuts import (
     get_object_or_404,
@@ -175,10 +176,6 @@ def trainer_login(request, user_pk):
     user = get_object_or_404(User, pk=user_pk)
     orig_user_pk = request.user.pk
 
-    # Changing only between the same gym
-    if request.user.userprofile.gym != user.userprofile.gym:
-        return HttpResponseForbidden()
-
     # No changing if identity is not set
     if not request.user.has_perm('gym.gym_trainer') \
             and not request.session.get('trainer.identity'):
@@ -191,6 +188,13 @@ def trainer_login(request, user_pk):
                  or user.has_perm('gym.manage_gyms')):
         return HttpResponseForbidden()
 
+    # Changing is only allowed between the same gym
+    if request.user.userprofile.gym != user.userprofile.gym:
+        return HttpResponseNotFound(
+            'There are no users in gym "{}" with user ID "{}".'.format(
+                request.user.userprofile.gym, user_pk,
+            ))
+
     # Check if we're switching back to our original account
     own = False
     if (user.has_perm('gym.gym_trainer')