openedx / edx-drf-extensions

edX extensions for Django REST Framework
http://edx-drf-extensions.readthedocs.org
Apache License 2.0
14 stars 16 forks source link

Clean up JWT_VERIFY_AUDIENCE and AUDIENCE setting #328

Open robrap opened 1 year ago

robrap commented 1 year ago

JWT audience is validated if JWT_VERIFY_AUDIENCE is set to True. See https://github.com/openedx/edx-drf-extensions/blob/ae7416f200bb7595b0172eb345e669c7fbcbc903/edx_rest_framework_extensions/auth/jwt/decoder.py#L260.

However, since we don't have a strong stance on this, JWT_VERIFY_AUDIENCE is set to False in many places, including in edx.org settings. See https://github.com/search?q=(org%3Aopenedx%20OR%20org%3Aedx)%20JWT_VERIFY_AUDIENCE&type=code

Additionally, in many edx.org settings, the AUDIENCE setting is unnecessarily encrypted, which leads to further confusion.

feanil commented 1 year ago

@robrap what do you mean by "Clean up" here. Should this setting be removed or should we be updating everything so that it defaults to true? Something else?

Also it sounds like there is a second small task to not encrypt the Audience data in the JWT? How would that work, isn't the Audience a part of the payload?

robrap commented 1 year ago
  1. First, we should have some sort of stance. Maybe this is an org level decision, but I wish someone could explain why audience verification helps. This article states that you should verify all of these optimal claims, but I don’t get what it buys you if the signature either is ok or is not? See https://www.pingidentity.com/en/resources/blog/post/jwt-security-nobody-talks-about.html#:~:text=Similarly%2C%20their%20presence%20must%20be,functions%20to%20verify%20these%20claims
  2. Audience is not encrypted in the payload. It is just encrypted in some 2U config.