Merge pull request #197 from avantifellows/user-tests

Tests for Users App

.github/workflows/ci.yml

@@ -10,9 +10,9 @@ jobs:
     name: Pre-commit
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
-    - uses: actions/setup-python@v2
-    - uses: pre-commit/action@v2.0.0
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+      - uses: pre-commit/action@v2.0.0
 
   test:
     name: Test cases
@@ -58,11 +58,15 @@ jobs:
           DB_NAME: github_actions_testing
           DB_USER: postgres
           DB_PASSWORD: postgres
-          SECRET_KEY: wpurj&oym6m@kcp(m&z(q-g0bo-r*+!f_&j(94di8j&_j4m%2s  # random secret key
+          SECRET_KEY: wpurj&oym6m@kcp(m&z(q-g0bo-r*+!f_&j(94di8j&_j4m%2s # random secret key
           AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
           AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
           AWS_REGION: ${{ secrets.AWS_REGION }}
           AWS_STORAGE_BUCKET_NAME: ${{ secrets.AWS_STORAGE_BUCKET_NAME }}
+          ANALYTICS_IDP_TYPE: ${{ secrets.ANALYTICS_IDP_TYPE }}
+          ANALYTICS_IDP_TOKEN_URL: ${{ secrets.ANALYTICS_IDP_TOKEN_URL }}
+          ANALYTICS_IDP_CLIENT_ID: ${{ secrets.ANALYTICS_IDP_CLIENT_ID }}
+          ANALYTICS_IDP_CLIENT_SECRET: ${{ secrets.ANALYTICS_IDP_CLIENT_SECRET }}
           REDIS_HOSTNAME: 127.0.0.1
           REDIS_PORT: 6379
         # command to run tests and generate coverage metrics

.pep8speaks.yml

@@ -0,0 +1,27 @@
+scanner:
+  diff_only: True # If False, the entire file touched by the Pull Request is scanned for errors. If True, only the diff is scanned.
+  linter: pycodestyle # Other option is flake8
+
+pycodestyle: # Same as scanner.linter value. Other option is flake8
+  max-line-length: 100 # Default is 79 in PEP 8
+  # ignore:  # Errors and warnings to ignore
+  #     - W504  # line break after binary operator
+  #     - E402  # module level import not at top of file
+  #     - E731  # do not assign a lambda expression, use a def
+  #     - C406  # Unnecessary list literal - rewrite as a dict literal.
+  #     - E741  # ambiguous variable name
+
+no_blank_comment: True # If True, no comment is made on PR without any errors.
+descending_issues_order: False # If True, PEP 8 issues in message will be displayed in descending order of line numbers in the file
+
+message: # Customize the comment made by the bot
+  opened: # Messages when a new PR is submitted
+    header: "Hello @{name}! Thanks for opening this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:"
+    # The keyword {name} is converted into the author's username
+    footer:
+      "Do see the [Hitchhiker's guide to code style](https://goo.gl/hqbW4r)"
+      # The messages can be written as they would over GitHub
+  updated: # Messages when new commits are added to the PR
+    header: "Hello @{name}! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:"
+    footer: "" # Why to comment the link to the style guide everytime? :)
+  no_errors: "There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: "

docs/DEPLOYMENT.md

@@ -200,6 +200,12 @@ Follow the steps below to set up the staging environment on AWS.
        - AWS_ACCESS_KEY_ID
        - AWS_SECRET_ACCESS_KEY
        - AWS_REGION
+       - AWS_STORAGE_BUCKET_NAME
+       - ANALYTICS_IDP_TYPE
+       - ANALYTICS_IDP_TOKEN_URL
+       - ANALYTICS_IDP_CLIENT_ID
+       - ANALYTICS_IDP_CLIENT_SECRET
+       - ANALYTICS_IDP_AUDIENCE (optional)
 
 14. We are using Github Actions to trigger deployments. You can find the workflow defined in `.github/workflows/deploy_to_ecs_staging.yml`. It defines a target branch such that a deployment is initiated whenever a change is pushed to the target branch.
 

plio/urls.py

@@ -87,10 +87,14 @@
     # API routes
     path("api/v1/otp/request/", request_otp, name="request-otp"),
     path("api/v1/otp/verify/", verify_otp, name="verify-otp"),
-    path("api/v1/users/token/", get_by_access_token),
+    path("api/v1/users/token/", get_by_access_token, name="get-by-access-token"),
     path("api/v1/", include(api_router.urls)),
     path("api-auth/", include("rest_framework.urls", namespace="rest_framework")),
-    path("auth/cubejs-token/", retrieve_analytics_app_access_token),
+    path(
+        "auth/cubejs-token/",
+        retrieve_analytics_app_access_token,
+        name="get-analytics-token",
+    ),
     url(r"^auth/", include("rest_framework_social_oauth2.urls")),
     url(
         r"^api/v1/docs/$",

users/tests.py

@@ -1,6 +1,7 @@
+import json
 from rest_framework import status
 from django.urls import reverse
-from users.models import OneTimePassword, User, Role
+from users.models import OneTimePassword, Role, OrganizationUser
 from plio.tests import BaseTestCase
 from organizations.models import Organization
 
@@ -12,39 +13,138 @@ def setUp(self):
         super().setUp()
         # unset client credentials token so that the subsequent API calls goes as guest
         self.client.credentials()
-        self.user_mobile = "+919876543210"
 
     def test_guest_can_request_for_otp(self):
         response = self.client.post(
-            reverse("request-otp"), {"mobile": self.user_mobile}
+            reverse("request-otp"), {"mobile": self.user.mobile}
         )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
 
-        otp_exists = OneTimePassword.objects.filter(mobile=self.user_mobile).exists()
+        otp_exists = OneTimePassword.objects.filter(mobile=self.user.mobile).exists()
         self.assertTrue(otp_exists)
 
     def test_invalid_otp_should_fail(self):
         # request otp
-        self.client.post(reverse("request-otp"), {"mobile": self.user_mobile})
+        self.client.post(reverse("request-otp"), {"mobile": self.user.mobile})
 
         # invalid otp
         otp = "000000"
         response = self.client.post(
-            reverse("verify-otp"), {"mobile": self.user_mobile, "otp": otp}
+            reverse("verify-otp"), {"mobile": self.user.mobile, "otp": otp}
         )
         self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
 
     def test_valid_otp_should_pass(self):
         # request otp
-        self.client.post(reverse("request-otp"), {"mobile": self.user_mobile})
+        self.client.post(reverse("request-otp"), {"mobile": self.user.mobile})
 
         # verify valid otp
-        otp = OneTimePassword.objects.filter(mobile=self.user_mobile).first()
+        otp = OneTimePassword.objects.filter(mobile=self.user.mobile).first()
         response = self.client.post(
-            reverse("verify-otp"), {"mobile": self.user_mobile, "otp": otp.otp}
+            reverse("verify-otp"), {"mobile": self.user.mobile, "otp": otp.otp}
         )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
 
+    def test_valid_otp_should_pass_new_user(self):
+        # new user
+        new_user_mobile = "+919988776654"
+
+        # request otp
+        self.client.post(reverse("request-otp"), {"mobile": new_user_mobile})
+
+        # verify valid otp
+        otp = OneTimePassword.objects.filter(mobile=new_user_mobile).first()
+        response = self.client.post(
+            reverse("verify-otp"), {"mobile": new_user_mobile, "otp": otp.otp}
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+
+class UserTestCase(BaseTestCase):
+    def test_get_config(self):
+        config = {"test": True}
+        # set config
+        self.user.config = config
+        self.user.save()
+
+        # get config
+        response = self.client.get(f"/api/v1/users/{self.user.id}/config/")
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.json(), config)
+
+    def test_user_cannot_get_other_user_config(self):
+        # get config
+        response = self.client.get(f"/api/v1/users/{self.user_2.id}/config/")
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_update_config_no_config_provided(self):
+        """Updating config without passing the config to use should fail"""
+        # update config
+        response = self.client.patch(f"/api/v1/users/{self.user.id}/config/", {})
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+    def test_update_config_extra_params(self):
+        """Passing params other than config while updating config should fail"""
+        # update config
+        response = self.client.patch(
+            f"/api/v1/users/{self.user.id}/config/",
+            {"config": {}, "extra_param": True},
+            format="json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            json.loads(response.content)["detail"],
+            "extra keys apart from config are not allowed",
+        )
+
+    def test_update_config(self):
+        # update config
+        config = {"test": True}
+        response = self.client.patch(
+            f"/api/v1/users/{self.user.id}/config/",
+            {"config": config},
+            format="json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        # get the config to see if it is updated
+        response = self.client.get(f"/api/v1/users/{self.user.id}/config/")
+
+        self.assertEqual(
+            response.json(),
+            config,
+        )
+
+    def test_user_cannot_update_other_user_config(self):
+        # get config
+        response = self.client.patch(
+            f"/api/v1/users/{self.user_2.id}/config/",
+            {"config": {"test": True}},
+            format="json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_get_by_access_token_access_token_not_passed(self):
+        response = self.client.get(reverse("get-by-access-token"))
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+    def test_get_by_access_token_access_token_invalid(self):
+        response = self.client.get(reverse("get-by-access-token"), {"token": "1234"})
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    def test_get_by_access_token_valid_user(self):
+        response = self.client.get(
+            reverse("get-by-access-token"), {"token": self.access_token.token}
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.json()["id"], self.user.id)
+
+    def test_get_analytics_app_access_token(self):
+        response = self.client.post(reverse("get-analytics-token"))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertIn("access_token", response.json())
+
 
 class UserMetaTestCase(BaseTestCase):
     def setUp(self):
@@ -67,39 +167,86 @@ def test_for_role(self):
 class OrganizationUserTestCase(BaseTestCase):
     def setUp(self):
         super().setUp()
-        # set up an organization
-        self.organization = Organization.objects.create(name="Org 1", shortcode="org-1")
-        # set up a user that's supposed to be in the organization
-        self.organization_user = User.objects.create(mobile="+919988776655")
-
-    def test_normal_user_cannot_create_organization_user(self):
-        # get org-view role
-        role_org_view = Role.objects.filter(name="org-view").first()
+
+        # create another organization
+        self.organization_2 = Organization.objects.create(
+            name="Org 2", shortcode="org-2"
+        )
+
+        # seed some organization users
+        self.org_user_1 = OrganizationUser.objects.create(
+            organization=self.organization, user=self.user, role=self.org_view_role
+        )
+
+        self.org_user_2 = OrganizationUser.objects.create(
+            organization=self.organization_2, user=self.user, role=self.org_view_role
+        )
+
+    def test_superuser_can_list_all_org_users(self):
+        # make the current user as superuser
+        self.user.is_superuser = True
+        self.user.save()
+
+        # get organization users
+        response = self.client.get(reverse("organization-users-list"))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(len(response.json()), 2)
+        self.assertEqual(response.json()[0]["user"], self.user.id)
+        self.assertEqual(response.json()[1]["user"], self.user.id)
+        self.assertEqual(response.json()[0]["organization"], self.organization.id)
+        self.assertEqual(response.json()[1]["organization"], self.organization_2.id)
+
+    def test_normal_user_only_sees_empty_list_of_org_users(self):
+        # get organization users
+        response = self.client.get(reverse("organization-users-list"))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(len(response.json()), 0)
+
+    def test_org_admin_can_list_only_their_org_users(self):
+        # make user 2 org-admin for org 1
+        org_admin_role = Role.objects.filter(name="org-admin").first()
+        OrganizationUser.objects.create(
+            organization=self.organization, user=self.user_2, role=org_admin_role
+        )
+
+        # change user
+        self.client.credentials(
+            HTTP_AUTHORIZATION="Bearer " + self.access_token_2.token,
+        )
+
+        # get organization users
+        response = self.client.get(reverse("organization-users-list"))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(len(response.json()), 2)
+        self.assertEqual(response.json()[0]["user"], self.user.id)
+        self.assertEqual(response.json()[0]["organization"], self.organization.id)
+        self.assertEqual(response.json()[1]["user"], self.user_2.id)
+        self.assertEqual(response.json()[1]["organization"], self.organization.id)
+
+    def test_normal_user_cannot_create_org_user(self):
         # add organization_user to the organization
         response = self.client.post(
             reverse("organization-users-list"),
             {
-                "user": self.organization_user.id,
+                "user": self.user.id,
                 "organization": self.organization.id,
-                "role": role_org_view.id,
+                "role": self.org_view_role.id,
             },
         )
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
-    def test_superuser_can_create_organization_user(self):
+    def test_superuser_can_create_org_user(self):
         # make the current user as superuser
         self.user.is_superuser = True
         self.user.save()
 
-        # get org-view role
-        role_org_view = Role.objects.filter(name="org-view").first()
         # add organization_user to the organization
         response = self.client.post(
             reverse("organization-users-list"),
             {
-                "user": self.organization_user.id,
+                "user": self.user.id,
                 "organization": self.organization.id,
-                "role": role_org_view.id,
+                "role": self.org_view_role.id,
             },
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)

users/views.py

@@ -196,6 +196,12 @@ def verify_otp(request):
 
 @api_view(["GET"])
 def get_by_access_token(request):
+    if "token" not in request.query_params:
+        return Response(
+            {"detail": "token not provided"},
+            status=status.HTTP_400_BAD_REQUEST,
+        )
+
     token = request.query_params["token"]
     access_token = AccessToken.objects.filter(token=token).first()
     if access_token: