Merge pull request #2352 from ryanpetrello/fix-csrf-sessions

fix a few remaining CSRF nits
This commit is contained in:
Ryan Petrello
2018-06-28 13:57:13 -04:00
committed by GitHub
2 changed files with 31 additions and 30 deletions

View File

@@ -23,7 +23,6 @@ from django.utils.translation import ugettext_lazy as _
from django.contrib.auth import views as auth_views from django.contrib.auth import views as auth_views
# Django REST Framework # Django REST Framework
from rest_framework.authentication import get_authorization_header
from rest_framework.exceptions import PermissionDenied, AuthenticationFailed, ParseError, NotAcceptable from rest_framework.exceptions import PermissionDenied, AuthenticationFailed, ParseError, NotAcceptable
from rest_framework import generics from rest_framework import generics
from rest_framework.response import Response from rest_framework.response import Response
@@ -195,7 +194,7 @@ class APIView(views.APIView):
request.drf_request_user = getattr(drf_request, 'user', False) request.drf_request_user = getattr(drf_request, 'user', False)
except AuthenticationFailed: except AuthenticationFailed:
request.drf_request_user = None request.drf_request_user = None
except ParseError as exc: except (PermissionDenied, ParseError) as exc:
request.drf_request_user = None request.drf_request_user = None
self.__init_request_error__ = exc self.__init_request_error__ = exc
return drf_request return drf_request
@@ -210,6 +209,7 @@ class APIView(views.APIView):
if hasattr(self, '__init_request_error__'): if hasattr(self, '__init_request_error__'):
response = self.handle_exception(self.__init_request_error__) response = self.handle_exception(self.__init_request_error__)
if response.status_code == 401: if response.status_code == 401:
response.data['detail'] += ' To establish a login session, visit /api/login/.'
logger.info(status_msg) logger.info(status_msg)
else: else:
logger.warn(status_msg) logger.warn(status_msg)
@@ -228,31 +228,35 @@ class APIView(views.APIView):
return response return response
def get_authenticate_header(self, request): def get_authenticate_header(self, request):
""" # HTTP Basic auth is insecure by default, because the basic auth
Determine the WWW-Authenticate header to use for 401 responses. Try to # backend does not provide CSRF protection.
use the request header as an indication for which authentication method #
was attempted. # If you visit `/api/v2/job_templates/` and we return
""" # `WWW-Authenticate: Basic ...`, your browser will prompt you for an
if request.META.get('HTTP_X_REQUESTED_WITH') == 'XMLHttpRequest': # HTTP basic auth username+password and will store it _in the browser_
return 'Bearer realm=api' # for subsequent requests. Because basic auth does not require CSRF
for authenticator in self.get_authenticators(): # validation (because it's commonly used with e.g., tower-cli and other
try: # non-browser clients), browsers that save basic auth in this way are
resp_hdr = authenticator.authenticate_header(request) # vulnerable to cross-site request forgery:
if not resp_hdr: #
continue # 1. Visit `/api/v2/job_templates/` and specify a user+pass for basic auth.
except AttributeError: # 2. Visit a nefarious website and submit a
continue # `<form action='POST' method='https://tower.example.org/api/v2/job_templates/N/launch/'>`
req_hdr = get_authorization_header(request) # 3. The browser will use your persisted user+pass and your login
if not req_hdr: # session is effectively hijacked.
continue #
if resp_hdr.split()[0] and resp_hdr.split()[0] == req_hdr.split()[0]: # To prevent this, we will _no longer_ send `WWW-Authenticate: Basic ...`
return resp_hdr # headers in responses; this means that unauthenticated /api/v2/... requests
# If it can't be determined from the request, use the last # will now return HTTP 401 in-browser, rather than popping up an auth dialog.
# authenticator (should be Basic). #
try: # This means that people who wish to use the interactive API browser
return authenticator.authenticate_header(request) # must _first_ login in via `/api/login/` to establish a session (which
except NameError: # _does_ enforce CSRF).
pass #
# CLI users can _still_ specify basic auth credentials explicitly via
# a header or in the URL e.g.,
# `curl https://user:pass@tower.example.org/api/v2/job_templates/N/launch/`
return 'Bearer realm=api authorization_url=/api/o/authorize/'
def get_view_description(self, html=False): def get_view_description(self, html=False):
""" """

View File

@@ -4233,7 +4233,6 @@ class JobRelaunch(RetrieveAPIView):
data.pop('credential_passwords', None) data.pop('credential_passwords', None)
return data return data
@csrf_exempt
@transaction.non_atomic_requests @transaction.non_atomic_requests
def dispatch(self, *args, **kwargs): def dispatch(self, *args, **kwargs):
return super(JobRelaunch, self).dispatch(*args, **kwargs) return super(JobRelaunch, self).dispatch(*args, **kwargs)
@@ -4479,7 +4478,6 @@ class AdHocCommandList(ListCreateAPIView):
serializer_class = AdHocCommandListSerializer serializer_class = AdHocCommandListSerializer
always_allow_superuser = False always_allow_superuser = False
@csrf_exempt
@transaction.non_atomic_requests @transaction.non_atomic_requests
def dispatch(self, *args, **kwargs): def dispatch(self, *args, **kwargs):
return super(AdHocCommandList, self).dispatch(*args, **kwargs) return super(AdHocCommandList, self).dispatch(*args, **kwargs)
@@ -4577,7 +4575,6 @@ class AdHocCommandRelaunch(GenericAPIView):
# FIXME: Figure out why OPTIONS request still shows all fields. # FIXME: Figure out why OPTIONS request still shows all fields.
@csrf_exempt
@transaction.non_atomic_requests @transaction.non_atomic_requests
def dispatch(self, *args, **kwargs): def dispatch(self, *args, **kwargs):
return super(AdHocCommandRelaunch, self).dispatch(*args, **kwargs) return super(AdHocCommandRelaunch, self).dispatch(*args, **kwargs)