fix: reorder JWT decoders (#3941)

Reordered the JWT decoders to first use the standard
library version, and then use the custom ecommerce
decoder which uses multiple issuers. In this way, we
can see if any JWTs cannot be decoded by that standard
library version, and when and if we are ready to retire
the custom JWT decoding code.

See DEPR openedx/public-engineering#83

ecommerce/extensions/api/handlers.py

@@ -5,8 +5,6 @@
 custom jwt_decode_handler.
 
 """
-
-
 import logging
 
 import jwt
@@ -18,8 +16,6 @@
 
 logger = logging.getLogger(__name__)
 
-JWT_DECODE_HANDLER_METRIC_KEY = 'ecom_jwt_decode_handler'
-
 
 def _ecommerce_jwt_decode_handler_multiple_issuers(token):
     """
@@ -94,26 +90,31 @@ def jwt_decode_handler(token):
 
     """
 
-    # First, try ecommerce decoder that handles multiple issuers.
-    # See ARCH-276 for details of removing additional issuers and retiring this
-    # custom jwt_decode_handler.
-    try:
-        jwt_payload = _ecommerce_jwt_decode_handler_multiple_issuers(token)
-        monitoring_utils.set_custom_metric(JWT_DECODE_HANDLER_METRIC_KEY, 'ecommerce-multiple-issuers')
-        return jwt_payload
-    except Exception:  # pylint: disable=broad-except
-        if waffle.switch_is_active('jwt_decode_handler.log_exception.ecommerce-multiple-issuers'):
-            logger.info('Failed to use ecommerce multiple issuer jwt_decode_handler.', exc_info=True)
-
-    # Next, try jwt_decode_handler from edx_drf_extensions
+    # First, try jwt_decode_handler from edx_drf_extensions
     # Note: this jwt_decode_handler can handle asymmetric keys, but only a
     #   single issuer. Therefore, the LMS must be the first configured issuer.
     try:
         jwt_payload = edx_drf_extensions_jwt_decode_handler(token)
-        monitoring_utils.set_custom_metric(JWT_DECODE_HANDLER_METRIC_KEY, 'edx-drf-extensions')
+        # Note: use increment in case there are multiple calls for different JWTs in the same transaction.
+        monitoring_utils.increment("ecom_jwt_decode_standard")
         return jwt_payload
     except Exception:  # pylint: disable=broad-except
         # continue and try again
         if waffle.switch_is_active('jwt_decode_handler.log_exception.edx-drf-extensions'):
             logger.info('Failed to use edx-drf-extensions jwt_decode_handler.', exc_info=True)
+
+    # Next, try ecommerce decoder that handles multiple issuers.
+    # See ARCH-276 for details of removing additional issuers and retiring this
+    # custom jwt_decode_handler.
+    try:
+        jwt_payload = _ecommerce_jwt_decode_handler_multiple_issuers(token)
+        # Ultimately we want there to be no JWTs that aren't handled by the first case.
+        #   This enables monitoring to see if there are still cases where the custom JWT
+        #   decoder is in use.
+        monitoring_utils.increment("ecom_jwt_decode_custom")
+        return jwt_payload
+    except Exception:  # pylint: disable=broad-except
+        if waffle.switch_is_active('jwt_decode_handler.log_exception.ecommerce-multiple-issuers'):
+            logger.info('Failed to use ecommerce multiple issuer jwt_decode_handler.', exc_info=True)
+        monitoring_utils.increment("ecom_jwt_decode_failed")
         raise

ecommerce/extensions/api/tests/test_handlers.py

@@ -49,9 +49,9 @@ def setUp(self):
             'JWT_VERIFY_AUDIENCE': False,
         }
     )
-    @mock.patch('edx_django_utils.monitoring.set_custom_metric')
+    @mock.patch('edx_django_utils.monitoring.increment')
     @mock.patch('ecommerce.extensions.api.handlers._ecommerce_jwt_decode_handler_multiple_issuers')
-    def test_decode_success_edx_drf_extensions(self, mock_multiple_issuer_decoder, mock_set_custom_metric):
+    def test_decode_success_edx_drf_extensions(self, mock_multiple_issuer_decoder, mock_increment):
         """
         Should pass using the edx-drf-extensions jwt_decode_handler.
 
@@ -63,7 +63,7 @@ def test_decode_success_edx_drf_extensions(self, mock_multiple_issuer_decoder, m
         payload = generate_jwt_payload(self.user, issuer_name=first_issuer['ISSUER'])
         token = generate_jwt_token(payload, first_issuer['SECRET_KEY'])
         self.assertDictContainsSubset(payload, jwt_decode_handler(token))
-        mock_set_custom_metric.assert_called_with('ecom_jwt_decode_handler', 'edx-drf-extensions')
+        mock_increment.assert_called_with('ecom_jwt_decode_standard')
 
     @override_settings(
         JWT_AUTH={
@@ -87,9 +87,9 @@ def test_decode_success_edx_drf_extensions(self, mock_multiple_issuer_decoder, m
             'JWT_VERIFY_AUDIENCE': False,
         }
     )
-    @mock.patch('edx_django_utils.monitoring.set_custom_metric')
+    @mock.patch('edx_django_utils.monitoring.increment')
     @mock.patch('ecommerce.extensions.api.handlers.logger')
-    def test_decode_success_multiple_issuers(self, mock_logger, mock_set_custom_metric):
+    def test_decode_success_multiple_issuers(self, mock_logger, mock_increment):
         """
         Should pass using ``_ecommerce_jwt_decode_handler_multiple_issuers``.
 
@@ -101,7 +101,7 @@ def test_decode_success_multiple_issuers(self, mock_logger, mock_set_custom_metr
         payload = generate_jwt_payload(self.user, issuer_name=non_first_issuer['ISSUER'])
         token = generate_jwt_token(payload, non_first_issuer['SECRET_KEY'])
         self.assertDictContainsSubset(payload, jwt_decode_handler(token))
-        mock_set_custom_metric.assert_called_with('ecom_jwt_decode_handler', 'ecommerce-multiple-issuers')
+        mock_increment.assert_called_with('ecom_jwt_decode_custom')
         mock_logger.exception.assert_not_called()
         mock_logger.warning.assert_not_called()
         mock_logger.error.assert_not_called()
@@ -141,6 +141,6 @@ def test_decode_error_invalid_token(self, mock_logger):
 
         mock_logger.exception.assert_called_with('Custom config JWT decode failed!')
         mock_logger.info.assert_has_calls(calls=[
-            mock.call('Failed to use ecommerce multiple issuer jwt_decode_handler.', exc_info=True),
             mock.call('Failed to use edx-drf-extensions jwt_decode_handler.', exc_info=True),
+            mock.call('Failed to use ecommerce multiple issuer jwt_decode_handler.', exc_info=True),
         ])