Skip to content

Commit c2b9002

Browse files
authored
Merge commit from fork
Lock down API permissions for unprivileged users
2 parents e091528 + 26ffe04 commit c2b9002

File tree

3 files changed

+20
-14
lines changed

3 files changed

+20
-14
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Lock down API access for unprivileged users
2+
3+
By default, NAV granted full API access to logged-in users, regardless of their configured privilege level. This would give unprivileged users access to manipulate NAV configuration and even elevate their own user privileges to administrator level. [Read the full security advisory here.](https://github.com/Uninett/nav/security/advisories/GHSA-gprr-5vvf-582g)

python/nav/web/api/v1/auth.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import logging
1919
from datetime import datetime
2020
from urllib.parse import urlparse
21+
2122
from rest_framework.permissions import BasePermission, SAFE_METHODS
2223
from rest_framework.exceptions import AuthenticationFailed
2324
from rest_framework.authentication import TokenAuthentication, BaseAuthentication
@@ -67,6 +68,14 @@ def has_permission(self, request, _view):
6768
return not request.account.is_default_account()
6869

6970

71+
class AdminPermission(BasePermission):
72+
"""Checks if the user is a NAV administrator"""
73+
74+
def has_permission(self, request, _view):
75+
"""If user is an admin, it is authorized"""
76+
return request.account.is_admin()
77+
78+
7079
class TokenPermission(BasePermission):
7180
"""Checks if the token has correct permissions"""
7281

@@ -157,14 +166,6 @@ def has_permission(self, request, _view):
157166
return True
158167

159168

160-
class APIPermission(BasePermission):
161-
"""Checks for correct permissions when accessing the API"""
162-
163-
def has_permission(self, request, view):
164-
"""Checks if request is permissable
165-
:type request: rest_framework.request.Request
166-
"""
167-
return any(
168-
permission().has_permission(request, view)
169-
for permission in (LoggedInPermission, TokenPermission, JWTPermission)
170-
)
169+
APITokensPermission = TokenPermission | JWTPermission
170+
DefaultPermission = AdminPermission | APITokensPermission
171+
RelaxedPermission = LoggedInPermission | APITokensPermission

python/nav/web/api/v1/views.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@
5454
from nav.web.status2 import STATELESS_THRESHOLD
5555
from nav.macaddress import MacPrefix
5656
from .auth import (
57-
APIPermission,
5857
APIAuthentication,
58+
DefaultPermission,
5959
NavBaseAuthentication,
60+
RelaxedPermission,
6061
)
6162
from .helpers import prefix_collector
6263
from .filter_backends import (
@@ -68,7 +69,6 @@
6869

6970
EXPIRE_DELTA = timedelta(days=365)
7071
MINIMUMPREFIXLENGTH = 4
71-
7272
_logger = logging.getLogger(__name__)
7373

7474

@@ -209,7 +209,7 @@ class NAVAPIMixin(APIView):
209209
APIAuthentication,
210210
JSONWebTokenAuthentication,
211211
)
212-
permission_classes = (APIPermission,)
212+
permission_classes = (DefaultPermission,)
213213
renderer_classes = (JSONRenderer, BrowsableAPIRenderer)
214214
filter_backends = (filters.SearchFilter, DjangoFilterBackend, RelatedOrderingFilter)
215215
ordering_fields = '__all__'
@@ -1028,6 +1028,8 @@ class AlertHistoryViewSet(NAVAPIMixin, viewsets.ReadOnlyModelViewSet):
10281028
"""
10291029

10301030
filter_backends = (AlertHistoryFilterBackend,)
1031+
# Logged-in users must be able to access this API to use the status tool
1032+
permission_classes = (RelaxedPermission,)
10311033
model = event.AlertHistory
10321034
serializer_class = alert_serializers.AlertHistorySerializer
10331035
base_queryset = base = event.AlertHistory.objects.prefetch_related(

0 commit comments

Comments
 (0)