diff --git a/teknologr/.env.example b/teknologr/.env.example index 3f3d16d7..b82700e5 100644 --- a/teknologr/.env.example +++ b/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 diff --git a/teknologr/api/templates/rest_framework/api.html b/teknologr/api/templates/rest_framework/api.html new file mode 100644 index 00000000..33c0e7fb --- /dev/null +++ b/teknologr/api/templates/rest_framework/api.html @@ -0,0 +1,8 @@ +{% extends "rest_framework/base.html" %} + +{% load static %} + +{% block style %} +{{ block.super }} + +{% endblock %} diff --git a/teknologr/api/tests.py b/teknologr/api/tests.py index 5e53cad5..d30ea368 100644 --- a/teknologr/api/tests.py +++ b/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())) diff --git a/teknologr/api/tests_dumps.py b/teknologr/api/tests_dumps.py index 441a04de..b2af1b02 100644 --- a/teknologr/api/tests_dumps.py +++ b/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 = [{ diff --git a/teknologr/api/tests_generikey.py b/teknologr/api/tests_members_by_mt.py similarity index 95% rename from teknologr/api/tests_generikey.py rename to teknologr/api/tests_members_by_mt.py index f674f988..30c8827f 100644 --- a/teknologr/api/tests_generikey.py +++ b/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] diff --git a/teknologr/api/tests_bill.py b/teknologr/api/tests_mt_by_member.py similarity index 87% rename from teknologr/api/tests_bill.py rename to teknologr/api/tests_mt_by_member.py index 0e80939f..fbe7927b 100644 --- a/teknologr/api/tests_bill.py +++ b/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 diff --git a/teknologr/api/urls.py b/teknologr/api/urls.py index 414b0005..b64ca5fd 100644 --- a/teknologr/api/urls.py +++ b/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/(?Pusername|studynumber)/(?P[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), ] diff --git a/teknologr/api/views.py b/teknologr/api/views.py index 3fba193f..44be39d5 100644 --- a/teknologr/api/views.py +++ b/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) diff --git a/teknologr/teknologr/settings.py b/teknologr/teknologr/settings.py index d86eb634..ab60bb8c 100644 --- a/teknologr/teknologr/settings.py +++ b/teknologr/teknologr/settings.py @@ -49,7 +49,24 @@ # SECURITY WARNING: don't run with debug turned on in production! DEBUG = env('DEBUG', True) + +''' +If a proxy (or similar) is used to handle requests to Teknologr, the request can be modified in some ways which can have unwanted side-effects in Django. + +One such problem occurs when an absoule uri (to for example /api/members/) is being built by Django, because the host name is taken from a request. If that request have been forwarded by some proxy it's 'HTTP_HOST' header will no longer be the orignal host name (let's say teknologr.com) but instead for example localhost:8000. The resulting uri is then 'http://localhost:8000/api/members/' while the correct uri is 'https://teknolog.com/api/members/'. + +The solution is to instead read the host name from the 'X-Forwarded-Host' (https://http.dev/x-forwarded-host) header in the request, which should be set by the proxy to the target host, as specified by the reqeust originator, if the request is forwarded. This is done in Django using the USE_X_FORWARDED_HOST setting. + +However, this will only resolve the problem with the host name, not the protocol part of the uri. Since the forwarded request can be done with HTTP, even if the original request was made with HTTPS, Django migth incorrectly deduce that a request is not secure even if the orignal request was (or vice verca). Using the SECURE_PROXY_SSL_HEADER setting Django will look a the specified header (usually the 'X-Forwarded-Proto' header: https://http.dev/x-forwarded-proto) rather than the request url to deduce if a request is secure or not. Note that the header needs to be in the format as used by request.META, i.e. all caps and (most likely) prefixed with 'HTTP_'. +''' ALLOWED_HOSTS = ['localhost'] +TEKNOLOGR_HOST = env('TEKNOLOGR_HOST', None) +if TEKNOLOGR_HOST: + ALLOWED_HOSTS.append(TEKNOLOGR_HOST) + +if env('IS_BEHIND_PROXY', False): + USE_X_FORWARDED_HOST = True + SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https') # Application definition @@ -85,7 +102,7 @@ TEMPLATES = [ { 'BACKEND': 'django.template.backends.django.DjangoTemplates', - 'DIRS': [], + 'DIRS': [os.path.join(BASE_DIR, 'api/templates')], 'APP_DIRS': True, 'OPTIONS': { 'context_processors': [