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.

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

feat: add version info API #31

Merged
merged 1 commit into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion futurex_openedx_extensions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""One-line description for README and other doc files."""

__version__ = '0.3.7'
__version__ = "0.3.8"
1 change: 1 addition & 0 deletions futurex_openedx_extensions/dashboard/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@
),
re_path(r'^api/fx/statistics/v1/course_statuses/$', views.CourseStatusesView.as_view(), name='course-statuses'),
re_path(r'^api/fx/statistics/v1/total_counts/$', views.TotalCountsView.as_view(), name='total-counts'),
re_path(r'^api/fx/version/v1/info/$', views.VersionInfoView.as_view(), name='version-info'),
]
16 changes: 15 additions & 1 deletion futurex_openedx_extensions/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from futurex_openedx_extensions.helpers.converters import error_details_to_dictionary
from futurex_openedx_extensions.helpers.filters import DefaultOrderingFilter
from futurex_openedx_extensions.helpers.pagination import DefaultPagination
from futurex_openedx_extensions.helpers.permissions import HasTenantAccess
from futurex_openedx_extensions.helpers.permissions import HasTenantAccess, IsSystemStaff
from futurex_openedx_extensions.helpers.tenants import get_selected_tenants, get_user_id_from_username_tenants


Expand Down Expand Up @@ -205,3 +205,17 @@ def get(self, request, username, *args, **kwargs): # pylint: disable=no-self-us
return Response(serializers.LearnerCoursesDetailsSerializer(
courses, context={'request': request}, many=True
).data)


class VersionInfoView(APIView):
"""View to get the version information"""
permission_classes = [IsSystemStaff]

def get(self, request, *args, **kwargs): # pylint: disable=no-self-use
"""
GET /api/fx/version/v1/info/
"""
import futurex_openedx_extensions # pylint: disable=import-outside-toplevel
return JsonResponse({
'version': futurex_openedx_extensions.__version__,
})
13 changes: 13 additions & 0 deletions futurex_openedx_extensions/helpers/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,16 @@ def has_permission(self, request, view):
raise PermissionDenied(detail=json.dumps(details))

return True


class IsSystemStaff(IsAuthenticated):
"""Permission class to check if the user is a staff member."""
def has_permission(self, request, view):
"""Check if the user is a staff member"""
if not super().has_permission(request, view):
raise NotAuthenticated()

if not request.user.is_staff and not request.user.is_superuser:
raise PermissionDenied(detail=json.dumps({"reason": "User is not a system staff member"}))

return True
6 changes: 3 additions & 3 deletions tests/base_test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"config_id": 8,
},
},
"users_count": 55,
"users_count": 60,
"user_signup_source__users": {
"s1.sample.com": [1, 2, 3, 4, 5],
"s2.sample.com": [4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15],
Expand Down Expand Up @@ -173,8 +173,8 @@
2: [23, 52, 53, 54],
},
},
"super_users": [1],
"staff_users": [2],
"super_users": [1, 60],
"staff_users": [2, 60],
"course_access_roles": { # roles, user ids per org
"staff": { # at least one role that is not in TENANT_LIMITED_ADMIN_ROLES
"ORG1": [3],
Expand Down
46 changes: 46 additions & 0 deletions tests/test_base_test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,49 @@ def test_certificate_must_be_enrolled():
assert user_id in _base_data['course_enrollments'].get(org, {}).get(course_id, []), (
f'User {user_id} is not enrolled in course {course_id} of org {org}'
)


def test_staff_only():
"""Verify the list of users having is_staff set to True, but not is_superuser"""
valid_list = [2]
for user_id in _base_data["staff_users"]:
if user_id in valid_list:
assert user_id not in _base_data["super_users"], \
f"User (user{user_id}) must be a staff user, but not a super user"
valid_list.remove(user_id)
else:
assert user_id in _base_data["super_users"], \
f"User (user{user_id}) must not be a staff user, or must be both a staff and a super user"

assert not valid_list, f"These users must be staff users, but not found in the staff_users list: {valid_list}"


def test_super_only():
"""Verify the list of users having is_superuser set to True"""
valid_list = [1]
for user_id in _base_data["super_users"]:
if user_id in valid_list:
assert user_id not in _base_data["staff_users"], \
f"User (user{user_id}) must be a super user, but not a staff user"
valid_list.remove(user_id)
else:
assert user_id in _base_data["staff_users"], \
f"User (user{user_id}) must not be a super user, or must be both a staff and a super user"

assert not valid_list, f"These users must be super users, but not found in the super_users list: ({valid_list})"


def test_both_staff_and_super():
"""Verify the list of users having both is_staff and is_superuser set to True"""
valid_list = [60]
for user_id in _base_data["super_users"]:
if user_id in valid_list:
assert user_id in _base_data["staff_users"], \
f"User (user{user_id}) must be both a staff and a super user"
valid_list.remove(user_id)
else:
assert user_id not in _base_data["staff_users"], \
f"User (user{user_id}) must not be both a staff and a super user"

assert not valid_list, \
f"These users must be both a staff and a super user, but not found in the super_users list: {valid_list}"
26 changes: 25 additions & 1 deletion tests/test_dashboard/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from futurex_openedx_extensions.dashboard import serializers
from futurex_openedx_extensions.helpers.constants import COURSE_STATUSES
from futurex_openedx_extensions.helpers.filters import DefaultOrderingFilter
from futurex_openedx_extensions.helpers.permissions import HasTenantAccess
from futurex_openedx_extensions.helpers.permissions import HasTenantAccess, IsSystemStaff
from tests.base_test_data import expected_statistics


Expand Down Expand Up @@ -357,3 +357,27 @@ def test_request_in_context(self, mock_serializer):
context={'request': request},
many=True,
)


class TestVersionInfoView(BaseTestViewMixin):
"""Tests for VersionInfoView"""
VIEW_NAME = 'fx_dashboard:version-info'

def test_permission_classes(self):
"""Verify that the view has the correct permission classes"""
view_func, _, _ = resolve(self.url)
view_class = view_func.view_class
self.assertEqual(view_class.permission_classes, [IsSystemStaff])

def test_unauthorized(self):
"""Verify that the view returns 403 when the user is not authenticated"""
response = self.client.get(self.url)
self.assertEqual(response.status_code, 403)

def test_success(self):
"""Verify that the view returns the correct response"""
self.login_user(self.staff_user)
with patch('futurex_openedx_extensions.__version__', new='0.1.dummy'):
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200)
self.assertEqual(json.loads(response.content), {'version': '0.1.dummy'})
34 changes: 33 additions & 1 deletion tests/test_helpers/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from rest_framework.exceptions import NotAuthenticated, PermissionDenied
from rest_framework.test import APIRequestFactory

from futurex_openedx_extensions.helpers.permissions import HasTenantAccess
from futurex_openedx_extensions.helpers.permissions import HasTenantAccess, IsSystemStaff


def set_user(request, user_id):
Expand Down Expand Up @@ -72,3 +72,35 @@ def test_has_tenant_access_not_authenticated(base_data, user_id): # pylint: dis
set_user(request, user_id)
with pytest.raises(NotAuthenticated):
permission.has_permission(request, None)


@pytest.mark.django_db
@pytest.mark.parametrize('user_id', [1, 2, 60])
def test_is_system_staff_ok(base_data, user_id): # pylint: disable=unused-argument
"""Verify that IsSystemStaff returns True when user is a system staff member."""
permission = IsSystemStaff()
request = APIRequestFactory().generic('GET', '/dummy/')
set_user(request, user_id)
assert permission.has_permission(request, None) is True


@pytest.mark.django_db
@pytest.mark.parametrize('user_id', [None, 0])
def test_is_system_staff_not_authenticated(base_data, user_id): # pylint: disable=unused-argument
"""Verify that NotAuthenticated is raised when user is not authenticated."""
permission = IsSystemStaff()
request = APIRequestFactory().generic('GET', '/dummy/')
set_user(request, user_id)
with pytest.raises(NotAuthenticated):
permission.has_permission(request, None)


@pytest.mark.django_db
@pytest.mark.parametrize('user_id', [3, 4])
def test_is_system_staff_not_staff(base_data, user_id): # pylint: disable=unused-argument
"""Verify that PermissionDenied is raised when user is not a system staff member."""
permission = IsSystemStaff()
request = APIRequestFactory().generic('GET', '/dummy/')
set_user(request, user_id)
with pytest.raises(PermissionDenied):
permission.has_permission(request, None)