From eb694528e75e381300f10cd1862aad821813c7dc Mon Sep 17 00:00:00 2001
From: Manjinder Singh <49171515+jinder1s@users.noreply.github.com>
Date: Thu, 30 Jan 2020 09:13:51 -0500
Subject: [PATCH] Adding metrics to oauth2authentication class (#22970)
Currently, we are working on removing the rest_framework_auth library from edx-platform. For this push, we need to remove the oauth2Authentication class. This PR creates a new class oauth2AuthenticationDeprecated that adds additional new relic metrics. The metrics would allow us to see how often this class is used and its success rate. The hope is that this information will help us with transitioning to a different authentication class.
---
.../djangoapps/third_party_auth/api/views.py | 5 ++-
lms/djangoapps/bulk_enroll/views.py | 4 +--
lms/djangoapps/commerce/api/v1/views.py | 6 ++--
lms/djangoapps/teams/views.py | 14 ++++-----
.../core/djangoapps/api_admin/api/v1/views.py | 4 +--
openedx/core/djangoapps/bookmarks/views.py | 6 ++--
openedx/core/djangoapps/credit/views.py | 4 +--
.../user_api/verification_api/views.py | 4 +--
openedx/core/lib/api/authentication.py | 31 +++++++++++++++----
9 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/common/djangoapps/third_party_auth/api/views.py b/common/djangoapps/third_party_auth/api/views.py
index 9f3d799bcf1..8f83908c551 100644
--- a/common/djangoapps/third_party_auth/api/views.py
+++ b/common/djangoapps/third_party_auth/api/views.py
@@ -16,10 +16,9 @@ from rest_framework import exceptions, permissions, status, throttling
from rest_framework.generics import ListAPIView
from rest_framework.response import Response
from rest_framework.views import APIView
-from rest_framework_oauth.authentication import OAuth2Authentication
from social_django.models import UserSocialAuth
-from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated, OAuth2AuthenticationAllowInactiveUser
from openedx.core.lib.api.permissions import ApiKeyHeaderPermission
from third_party_auth import pipeline
from third_party_auth.api import serializers
@@ -333,7 +332,7 @@ class UserMappingView(ListAPIView):
* remote_id: The Id from third party auth provider
"""
authentication_classes = (
- OAuth2Authentication,
+ OAuth2AuthenticationDeprecated,
)
serializer_class = serializers.UserMappingSerializer
diff --git a/lms/djangoapps/bulk_enroll/views.py b/lms/djangoapps/bulk_enroll/views.py
index ee4ea64f000..072f92c2aba 100644
--- a/lms/djangoapps/bulk_enroll/views.py
+++ b/lms/djangoapps/bulk_enroll/views.py
@@ -18,7 +18,7 @@ from lms.djangoapps.instructor.views.api import students_update_enrollment
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, get_cohort_by_name
from openedx.core.djangoapps.course_groups.models import CourseUserGroup
from openedx.core.djangoapps.enrollments.views import EnrollmentUserThrottle
-from openedx.core.lib.api.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from openedx.core.lib.api.permissions import IsStaff
from util.disable_rate_limit import can_disable_rate_limit
@@ -68,7 +68,7 @@ class BulkEnrollView(APIView):
to the 'before' and 'after' states.
"""
- authentication_classes = JwtAuthentication, OAuth2Authentication
+ authentication_classes = (JwtAuthentication, OAuth2AuthenticationDeprecated,)
permission_classes = (IsStaff,)
throttle_classes = (EnrollmentUserThrottle,)
diff --git a/lms/djangoapps/commerce/api/v1/views.py b/lms/djangoapps/commerce/api/v1/views.py
index 39a5c6e06be..79868545658 100644
--- a/lms/djangoapps/commerce/api/v1/views.py
+++ b/lms/djangoapps/commerce/api/v1/views.py
@@ -13,7 +13,7 @@ from rest_framework.authentication import SessionAuthentication
from rest_framework.generics import ListAPIView, RetrieveUpdateAPIView
from rest_framework.permissions import IsAuthenticated
from rest_framework.views import APIView
-from rest_framework_oauth.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from course_modes.models import CourseMode
from openedx.core.djangoapps.commerce.utils import ecommerce_api_client
@@ -30,7 +30,7 @@ log = logging.getLogger(__name__)
class CourseListView(ListAPIView):
""" List courses and modes. """
- authentication_classes = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,)
+ authentication_classes = (JwtAuthentication, OAuth2AuthenticationDeprecated, SessionAuthentication,)
permission_classes = (IsAuthenticated,)
serializer_class = CourseSerializer
pagination_class = None
@@ -44,7 +44,7 @@ class CourseRetrieveUpdateView(PutAsCreateMixin, RetrieveUpdateAPIView):
lookup_field = 'id'
lookup_url_kwarg = 'course_id'
model = CourseMode
- authentication_classes = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,)
+ authentication_classes = (JwtAuthentication, OAuth2AuthenticationDeprecated, SessionAuthentication,)
permission_classes = (ApiKeyOrModelPermission,)
serializer_class = CourseSerializer
diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py
index 9b6b83eb807..9a1b8827bb2 100644
--- a/lms/djangoapps/teams/views.py
+++ b/lms/djangoapps/teams/views.py
@@ -27,7 +27,7 @@ from rest_framework.generics import GenericAPIView
from rest_framework.response import Response
from rest_framework.reverse import reverse
from rest_framework.views import APIView
-from rest_framework_oauth.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from lms.djangoapps.courseware.courses import get_course_with_access, has_access
from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
@@ -368,7 +368,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
"""
# OAuth2Authentication must come first to return a 401 for unauthenticated users
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated,)
serializer_class = CourseTeamSerializer
@@ -696,7 +696,7 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView):
If the user is logged in and the team does not exist, a 404 is returned.
"""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated, IsStaffOrPrivilegedOrReadOnly, IsEnrolledOrIsStaff,)
lookup_field = 'team_id'
serializer_class = CourseTeamSerializer
@@ -791,7 +791,7 @@ class TopicListView(GenericAPIView):
those teams whose members are outside of institutions affliation.
"""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated,)
pagination_class = TopicsPagination
@@ -922,7 +922,7 @@ class TopicDetailView(APIView):
those teams whose members are outside of institutions affliation.
"""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated,)
def get(self, request, topic_id, course_id):
@@ -1082,7 +1082,7 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView):
another user to a team.
"""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated,)
serializer_class = MembershipSerializer
@@ -1295,7 +1295,7 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView):
If the membership does not exist, a 404 error is returned.
"""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated,)
serializer_class = MembershipSerializer
diff --git a/openedx/core/djangoapps/api_admin/api/v1/views.py b/openedx/core/djangoapps/api_admin/api/v1/views.py
index ff476ee650d..75ea98f2322 100644
--- a/openedx/core/djangoapps/api_admin/api/v1/views.py
+++ b/openedx/core/djangoapps/api_admin/api/v1/views.py
@@ -8,7 +8,7 @@ from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthenticat
from rest_framework.authentication import SessionAuthentication
from rest_framework.permissions import IsAuthenticated
from rest_framework.generics import ListAPIView
-from rest_framework_oauth.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from openedx.core.djangoapps.api_admin.api.v1 import serializers as api_access_serializers
from openedx.core.djangoapps.api_admin.models import ApiAccessRequest
@@ -50,7 +50,7 @@ class ApiAccessRequestView(ListAPIView):
"previous": null
}
"""
- authentication_classes = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,)
+ authentication_classes = (JwtAuthentication, OAuth2AuthenticationDeprecated, SessionAuthentication,)
permission_classes = (IsAuthenticated, )
serializer_class = api_access_serializers.ApiAccessRequestSerializer
filter_backends = (IsOwnerOrStaffFilterBackend, DjangoFilterBackend)
diff --git a/openedx/core/djangoapps/bookmarks/views.py b/openedx/core/djangoapps/bookmarks/views.py
index 8f4fb3978c7..490f1c4ad43 100644
--- a/openedx/core/djangoapps/bookmarks/views.py
+++ b/openedx/core/djangoapps/bookmarks/views.py
@@ -22,7 +22,7 @@ from rest_framework.authentication import SessionAuthentication
from rest_framework.generics import ListCreateAPIView
from rest_framework.response import Response
from rest_framework.views import APIView
-from rest_framework_oauth.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from openedx.core.djangoapps.bookmarks.api import BookmarksLimitReachedError
from openedx.core.lib.api.permissions import IsUserInUrl
@@ -99,7 +99,7 @@ class BookmarksViewMixin(object):
class BookmarksListView(ListCreateAPIView, BookmarksViewMixin):
"""REST endpoints for lists of bookmarks."""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
pagination_class = BookmarksPagination
permission_classes = (permissions.IsAuthenticated,)
serializer_class = BookmarkSerializer
@@ -290,7 +290,7 @@ class BookmarksDetailView(APIView, BookmarksViewMixin):
to a requesting user's bookmark a 404 is returned. 404 will also be returned
if the bookmark does not exist.
"""
- authentication_classes = (OAuth2Authentication, SessionAuthentication)
+ authentication_classes = (OAuth2AuthenticationDeprecated, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated, IsUserInUrl)
serializer_class = BookmarkSerializer
diff --git a/openedx/core/djangoapps/credit/views.py b/openedx/core/djangoapps/credit/views.py
index b3b295280f0..dd273df2652 100644
--- a/openedx/core/djangoapps/credit/views.py
+++ b/openedx/core/djangoapps/credit/views.py
@@ -18,7 +18,7 @@ from rest_framework import generics, mixins, permissions, views, viewsets
from rest_framework.authentication import SessionAuthentication
from rest_framework.exceptions import ValidationError
from rest_framework.response import Response
-from rest_framework_oauth.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from six import text_type
from openedx.core.djangoapps.credit.api import create_credit_request
@@ -45,7 +45,7 @@ from openedx.core.lib.api.mixins import PutAsCreateMixin
from openedx.core.lib.api.permissions import IsStaffOrOwner
log = logging.getLogger(__name__)
-AUTHENTICATION_CLASSES = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,)
+AUTHENTICATION_CLASSES = (JwtAuthentication, OAuth2AuthenticationDeprecated, SessionAuthentication,)
class CreditProviderViewSet(viewsets.ReadOnlyModelViewSet):
diff --git a/openedx/core/djangoapps/user_api/verification_api/views.py b/openedx/core/djangoapps/user_api/verification_api/views.py
index 0a9810593b3..d943b0c49a0 100644
--- a/openedx/core/djangoapps/user_api/verification_api/views.py
+++ b/openedx/core/djangoapps/user_api/verification_api/views.py
@@ -5,7 +5,7 @@ from django.http import Http404
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from rest_framework.authentication import SessionAuthentication
from rest_framework.generics import RetrieveAPIView
-from rest_framework_oauth.authentication import OAuth2Authentication
+from openedx.core.lib.api.authentication import OAuth2AuthenticationDeprecated
from lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification
from lms.djangoapps.verify_student.utils import most_recent_verification
@@ -19,7 +19,7 @@ from openedx.core.lib.api.permissions import IsStaffOrOwner
class IDVerificationStatusView(RetrieveAPIView):
""" IDVerificationStatus detail endpoint. """
- authentication_classes = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,)
+ authentication_classes = (JwtAuthentication, OAuth2AuthenticationDeprecated, SessionAuthentication,)
permission_classes = (IsStaffOrOwner,)
def get_serializer(self, *args, **kwargs):
diff --git a/openedx/core/lib/api/authentication.py b/openedx/core/lib/api/authentication.py
index 50ed39f6612..81c192ffa87 100644
--- a/openedx/core/lib/api/authentication.py
+++ b/openedx/core/lib/api/authentication.py
@@ -8,6 +8,7 @@ from oauth2_provider import models as dot_models
from provider.oauth2 import models as dop_models
from rest_framework.exceptions import AuthenticationFailed
from rest_framework_oauth.authentication import OAuth2Authentication
+from edx_django_utils.monitoring import set_custom_metric
OAUTH2_TOKEN_ERROR = u'token_error'
@@ -20,22 +21,41 @@ OAUTH2_TOKEN_ERROR_NOT_PROVIDED = u'token_not_provided'
log = logging.getLogger(__name__)
-class OAuth2AuthenticationAllowInactiveUser(OAuth2Authentication):
+class OAuth2AuthenticationDeprecated(OAuth2Authentication):
+ """
+ This child class was added to add new_relic metrics to OAuth2Authentication. This should be very temporary.
+ """
+
+ def authenticate(self, request):
+ """
+ Returns two-tuple of (user, token) if access token authentication
+ succeeds, None if the user did not try to authenticate using an access
+ token, or raises an AuthenticationFailed (HTTP 401) if authentication
+ fails.
+ """
+ set_custom_metric("OAuth2AuthenticationDeprecated", "Failed")
+ output = super(OAuth2AuthenticationDeprecated, self).authenticate(request)
+ if output is None:
+ set_custom_metric("OAuth2AuthenticationDeprecated", "None")
+ else:
+ set_custom_metric("OAuth2AuthenticationDeprecated", "Success")
+ return output
+
+
+class OAuth2AuthenticationAllowInactiveUser(OAuth2AuthenticationDeprecated):
"""
This is a temporary workaround while the is_active field on the user is coupled
with whether or not the user has verified ownership of their claimed email address.
Once is_active is decoupled from verified_email, we will no longer need this
class override.
-
But until then, this authentication class ensures that the user is logged in,
but does not require that their account "is_active".
-
This class can be used for an OAuth2-accessible endpoint that allows users to access
that endpoint without having their email verified. For example, this is used
for mobile endpoints.
"""
- def authenticate(self, *args, **kwargs):
+ def authenticate(self, request):
"""
Returns two-tuple of (user, token) if access token authentication
succeeds, raises an AuthenticationFailed (HTTP 401) if authentication
@@ -44,7 +64,7 @@ class OAuth2AuthenticationAllowInactiveUser(OAuth2Authentication):
"""
try:
- return super(OAuth2AuthenticationAllowInactiveUser, self).authenticate(*args, **kwargs)
+ return super(OAuth2AuthenticationAllowInactiveUser, self).authenticate(request)
except AuthenticationFailed as exc:
if isinstance(exc.detail, dict):
developer_message = exc.detail['developer_message']
@@ -65,7 +85,6 @@ class OAuth2AuthenticationAllowInactiveUser(OAuth2Authentication):
def authenticate_credentials(self, request, access_token):
"""
Authenticate the request, given the access token.
-
Overrides base class implementation to discard failure if user is
inactive.
"""
--
GitLab