Merge pull request #176 from Teknologforeningen/feature/fix-broken-ap…

…i-urls-when-using-proxy

Fix broken api urls when using proxy

teknologr/.env.example

@@ -1,5 +1,11 @@
 # This file contains configuration and settings variables that should be different in production and development environments.
 
+# The host name used for teknologr.io, for example teknologr.mysite.com
+TEKNOLOGR_HOST=
+
+# Whether or not requests to teknologr.io are forwarded by a proxy or not
+IS_BEHIND_PROXY=True
+
 
 # Secret key, change it maybe?
 SECRET_KEY=SomeRandomSecretKey

teknologr/api/templates/rest_framework/api.html

@@ -0,0 +1,8 @@
+{% extends "rest_framework/base.html" %}
+
+{% load static %}
+
+{% block style %}
+{{ block.super }}
+<link rel="icon" href="{% static 'favicon.ico' %}">
+{% endblock %}

teknologr/api/tests.py

@@ -693,10 +693,10 @@ def test_get_for_user(self):
         self.login_user()
         response = self.get_all()
         self.check_status_code(response, status.HTTP_200_OK)
-        self.assertEqual(10, len(response.json()))
+        self.assertEqual(8, len(response.json()))
 
     def test_get_for_superuser(self):
         self.login_superuser()
         response = self.get_all()
         self.check_status_code(response, status.HTTP_200_OK)
-        self.assertEqual(10, len(response.json()))
+        self.assertEqual(16, len(response.json()))

teknologr/api/tests_dumps.py

@@ -67,7 +67,7 @@ def test_get_for_superuser(self):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(self.response, response.json())
 
-class HTK(BaseClass, DumpsTestCases):
+class HTK_Full(BaseClass, DumpsTestCases):
     path = f'/api/dump-htk/'
     response = [{
         'id': 1,
@@ -78,6 +78,17 @@ class HTK(BaseClass, DumpsTestCases):
         'decorations': ['Hedersmedlem: 1999-01-01'],
     }]
 
+class HTK_One(BaseClass, DumpsTestCases):
+    path = f'/api/dump-htk/1/'
+    response = {
+        'id': 1,
+        'name': 'Sverker Svakar von Teknolog',
+        'functionaries': [f'Funkkis: {today} > 2999-01-01'],
+        'groups': [],
+        'membertypes': ['Ordinarie Medlem: 1999-01-01 > None'],
+        'decorations': ['Hedersmedlem: 1999-01-01'],
+    }
+
 class Modulen(BaseClass, DumpsTestCases):
     path = f'/api/dump-modulen/'
     response = [{

teknologr/api/tests_generikey.py → teknologr/api/tests_members_by_mt.py

@@ -45,7 +45,7 @@ def setUp(self):
     def login_superuser(self):
         self.client.login(username='superuser', password='teknolog')
 
-class GenerikeyTestCases():
+class TestCases():
     def test_get_for_anonymous_users(self):
         response = self.get('PH')
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
@@ -91,7 +91,7 @@ def test_get_double(self):
         self.assertEqual(response.json(), self.double)
 
 
-class GenerikeyStudynumbersTestCases(BaseClass, GenerikeyTestCases):
+class StudynumbersByMTTests(BaseClass, TestCases):
     normal = ['123456', '654321']
     null = ['123456', None]
     double = ['654321', None]
@@ -100,7 +100,7 @@ def get(self, type):
         return self.client.get(f'/api/membersByMemberType/{type}/')
 
 
-class GenerikeyUsernamesTestCases(BaseClass, GenerikeyTestCases):
+class UsernamesByMTTests(BaseClass, TestCases):
     normal = ['vonteks1', 'vonteks2']
     null = ['vonteks1', None]
     double = ['vonteks2', None]

teknologr/api/tests_bill.py → teknologr/api/tests_mt_by_member.py

@@ -29,7 +29,7 @@ def login_superuser(self):
         self.client.login(username='superuser', password='teknolog')
 
 
-class BILLTestCases:
+class TestCases:
     def test_get_for_anonymous_users(self):
         response = self.get()
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
@@ -48,11 +48,11 @@ def test_get_for_superuser(self):
         else:
             self.assertEqual(self.response, response.json())
 
-class BILLByUsernameTestCases(BILLTestCases):
+class ByUsernameTestCases(TestCases):
     def get(self):
         return self.client.get(f'/api/memberTypesForMember/username/{self.username}/')
 
-class BILLByStudynumberTestCases(BILLTestCases):
+class ByStudynumberTestCases(TestCases):
     def get(self):
         return self.client.get(f'/api/memberTypesForMember/studynumber/{self.studynumber}/')
 
@@ -77,26 +77,26 @@ def get(self):
     }
 }
 
-class BILLByInvalidUsernameTests(BaseClass, BILLByUsernameTestCases):
+class ByInvalidUsernameTests(BaseClass, ByUsernameTestCases):
     def setUp(self):
         super().setUp()
         self.username = 'invalid'
         self.response = None
 
-class BILLByValidUsernameTests(BaseClass, BILLByUsernameTestCases):
+class ByValidUsernameTests(BaseClass, ByUsernameTestCases):
     def setUp(self):
         super().setUp()
         self.username = self.member.username
         self.response = RESPONSE
 
 
-class BILLByInvalidStudynumberTests(BaseClass, BILLByStudynumberTestCases):
+class ByInvalidStudynumberTests(BaseClass, ByStudynumberTestCases):
     def setUp(self):
         super().setUp()
         self.studynumber = '123321'
         self.response = None
 
-class BILLByValidStudynumberTests(BaseClass, BILLByStudynumberTestCases):
+class ByValidStudynumberTests(BaseClass, ByStudynumberTestCases):
     def setUp(self):
         super().setUp()
         self.studynumber = self.member.student_id

teknologr/api/urls.py

@@ -1,16 +1,49 @@
 from django.conf.urls import url, include
-from rest_framework import routers, permissions
+from rest_framework.routers import APIRootView, DefaultRouter
+from rest_framework.permissions import IsAuthenticated, IsAdminUser
 from api.views import *
 
-class RootView(routers.APIRootView):
-    permission_classes = (permissions.IsAuthenticated, )
+class TeknologrRootView(APIRootView):
+    permission_classes = (IsAuthenticated, )
     name = 'Katalogen root API'
     description = ''
+    router_list_name = None
+    router_registry = None
+
+    @property
+    def api_root_dict(self):
+        '''
+        Overriding the parent attribute to be able to filter views based on permissions.
+
+        This is accessed by the get() method and is by default provided by the router instead.
+        '''
+        d = {}
+        is_staff = self.request.user.is_staff
+        for prefix, viewset, basename in self.router_registry:
+            # Include the route unless it is staff only
+            if not is_staff and IsAdminUser in viewset.permission_classes:
+                continue
+            d[prefix] = self.router_list_name.format(basename=basename)
+
+        # Also add the other named API endpoints to the list (mainly the dumps)
+        # XXX: Is there a way to check the permissions agains the user for these too?
+        if is_staff:
+            for url in urlpatterns:
+                if hasattr(url, 'name') and url.name:
+                    d[url.name] = url.name
+
+        return d
+
+class TeknologrRouter(DefaultRouter):
+    APIRootView = TeknologrRootView
+
+    def get_api_root_view(self, api_urls=None):
+        return self.APIRootView.as_view(router_list_name=self.routes[0].name, router_registry=self.registry)
+
 
 # Routers provide an easy way of automatically determining the URL conf.
 # NOTE: Use for example {% url 'api:member-list' %} to access these urls
-router = routers.DefaultRouter()
-router.APIRootView = RootView
+router = TeknologrRouter()
 router.register(r'members', MemberViewSet)
 router.register(r'grouptypes', GroupTypeViewSet)
 router.register(r'groups', GroupViewSet)
@@ -30,19 +63,19 @@ class RootView(routers.APIRootView):
     url(r'^multi-groupmemberships/$', multi_group_memberships_save),
     url(r'^multi-functionaries/$', multi_functionaries_save),
     url(r'^multi-decorationownerships/$', multi_decoration_ownerships_save),
-    url(r'^multi-applicantsubmissions/$', multi_applicant_submissions, name='multi_applicant_submissions'),
+    url(r'^multi-applicantsubmissions/$', multi_applicant_submissions),
     url(r'^accounts/ldap/(\d+)/$', LDAPAccountView.as_view()),
     url(r'^accounts/ldap/change_pw/(\d+)/$', change_ldap_password),
     url(r'^accounts/bill/(\d+)/$', BILLAccountView.as_view()),
     url(r'^applicants/make-member/(\d+)/$', ApplicantMembershipView.as_view()),
-    url(r'^dump-htk/(\d+)?$', dump_htk, name='dump_htk'),
+    url(r'^dump-htk/(?:(\d+)/)?$', dump_htk, name='dump_htk'),
     url(r'^dump-modulen/$', dump_modulen, name='dump_modulen'),
     url(r'^dump-active/$', dump_active, name='dump_active'),
     url(r'^dump-arsk/$', dump_arsk, name='dump_arsk'),
     url(r'^dump-regemails/$', dump_reg_emails, name='dump_reg_emails'),
     url(r'^dump-studentbladet/$', dump_studentbladet, name='dump_studentbladet'),
-    # Used by BILL
+    # Used by BILL (?)
     url(r'^memberTypesForMember/(?P<mode>username|studynumber)/(?P<query>[A-Za-z0-9]+)/$', member_types_for_member),
-    # Used by Generikey
-    url(r'^membersByMemberType/([A-Z]{2})/(\w+)?$', members_by_member_type),
+    # Used by BILL and Generikey
+    url(r'^membersByMemberType/([A-Z]{2})/(?:(\w+)/?)?$', members_by_member_type),
 ]

teknologr/api/views.py

@@ -24,7 +24,7 @@
 
 # ViewSets define the view behavior.
 
-class APIPermissions(permissions.BasePermission):
+class IsStaffOrReadOnly(permissions.BasePermission):
     def has_permission(self, request, view):
         # Do not allow anything for un-authenticated users
         if not request.user.is_authenticated:
@@ -39,7 +39,7 @@ def has_permission(self, request, view):
 
 class BaseModelViewSet(viewsets.ModelViewSet):
     # Use custom permissions
-    permission_classes = (APIPermissions, )
+    permission_classes = (IsStaffOrReadOnly, )
 
     def get_serializer(self, *args, **kwargs):
         serializer_class = self.get_serializer_class()
@@ -260,11 +260,11 @@ class LDAPAccountView(APIView):
     def get(self, request, member_id):
         member = get_object_or_404(Member, id=member_id)
         result = {}
-        with LDAPAccountManager() as lm:
-            try:
+        try:
+            with LDAPAccountManager() as lm:
                 result = {'username': member.username, 'groups': lm.get_ldap_groups(member.username)}
-            except LDAPError as e:
-                return Response(str(e), status=400)
+        except LDAPError as e:
+            return Response(str(e), status=400)
 
         return Response(result, status=200)
 
@@ -280,11 +280,11 @@ def post(self, request, member_id):
         if member.username:
             return Response("Member already has LDAP account", status=400)
 
-        with LDAPAccountManager() as lm:
-            try:
+        try:
+            with LDAPAccountManager() as lm:
                 lm.add_account(member, username, password)
-            except LDAPError as e:
-                return Response(str(e), status=400)
+        except LDAPError as e:
+            return Response(str(e), status=400)
 
         # Store account details
         member.username = username
@@ -306,11 +306,11 @@ def delete(self, request, member_id):
         if member.bill_code:
             return Response("BILL account must be deleted first", status=400)
 
-        with LDAPAccountManager() as lm:
-            try:
+        try:
+            with LDAPAccountManager() as lm:
                 lm.delete_account(member.username)
-            except LDAPError as e:
-                return Response(str(e), status=400)
+        except LDAPError as e:
+            return Response(str(e), status=400)
 
         # Delete account information from user in db
         member.username = None
@@ -327,15 +327,15 @@ def change_ldap_password(request, member_id):
     if not password:
         return Response("password field missing", status=400)
 
-    with LDAPAccountManager() as lm:
-        try:
+    try:
+        with LDAPAccountManager() as lm:
             lm.change_password(member.username, password)
             if mailToUser:
                 status = mailNewPassword(member, password)
                 if not status:
                     return Response("Password changed, failed to send mail", status=500)
-        except LDAPError as e:
-            return Response(str(e), status=400)
+    except LDAPError as e:
+        return Response(str(e), status=400)
 
     return Response(status=200)
 
@@ -464,8 +464,8 @@ def post(self, request, applicant_id):
 
         # Create an LDAP account if the application included a username and the username is not taken
         if username and not Member.objects.filter(username=username).exists():
-            with LDAPAccountManager() as lm:
-                try:
+            try:
+                with LDAPAccountManager() as lm:
                     import secrets
                     password = secrets.token_urlsafe(16)
                     lm.add_account(new_member, username, password)
@@ -478,14 +478,14 @@ def post(self, request, applicant_id):
                     if not status:
                         return Response(f'LDAP account created, failed to send mail to {new_member}', status=400)
 
-                # LDAP account creation failed (e.g. if the account already exists)
-                except LDAPError as e:
-                    return Response(f'Error creating LDAP account for {new_member}: {str(e)}', status=400)
-                # Updating the username field failed, remove the created LDAP account
-                # as it is not currently referenced by any member.
-                except IntegrityError as e:
-                    lm.delete_account(username)
-                    return Response(f'Error creating LDAP account for {new_member}: {str(e)}', status=400)
+            # LDAP account creation failed (e.g. if the account already exists)
+            except LDAPError as e:
+                return Response(f'Error creating LDAP account for {new_member}: {str(e)}', status=400)
+            # Updating the username field failed, remove the created LDAP account
+            # as it is not currently referenced by any member.
+            except IntegrityError as e:
+                lm.delete_account(username)
+                return Response(f'Error creating LDAP account for {new_member}: {str(e)}', status=400)
 
         return Response(status=200)
 
@@ -519,7 +519,7 @@ def multi_applicant_submissions(request):
 
 # JSON API:s
 
-# Used by BILL
+# Used by BILL (?)
 @api_view(['GET'])
 def member_types_for_member(request, mode, query):
     try:
@@ -548,7 +548,7 @@ def member_types_for_member(request, mode, query):
     return Response(data, status=200)
 
 
-# Used by GeneriKey
+# Used by BILL and GeneriKey
 @api_view(['GET'])
 def members_by_member_type(request, membertype, field=None):
     member_pks = MemberType.objects.filter(type=membertype, end_date=None).values_list("member", flat=True)
@@ -678,14 +678,11 @@ def dump_active(request):
         if membership.group.begin_date < now and membership.group.end_date > now:
             grouped_by_group[membership.group].append(membership.member)
     for group, members in grouped_by_group.items():
-        content.append({
-            'position': str(group.grouptype),
-            'member': ''
-        })
-        content.extend([{
-            'position': '',
-            'member': m.common_name
-        } for m in members])
+        for m in members:
+            content.append({
+                'position': str(group.grouptype),
+                'member': m.full_name,
+            })
 
     return Response(content, status=200)