Skip to content

Commit

Permalink
refactor: simplify REST API handling of usage keys
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Oct 18, 2024
1 parent dc8fd74 commit 8d10909
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1101,10 +1101,6 @@ def test_invalid_key(self, endpoint, endpoint_parameters):
endpoint.format(**endpoint_parameters),
)
self.assertEqual(response.status_code, 404)
msg = f"XBlock {endpoint_parameters.get('block_key')} does not exist, or you don't have permission to view it."
self.assertEqual(response.json(), {
'detail': msg,
})

def test_xblock_handler_invalid_key(self):
"""This endpoint is tested separately from the previous ones as it's not a DRF endpoint."""
Expand Down
5 changes: 2 additions & 3 deletions openedx/core/djangoapps/xblock/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ def get_handler_url(
This view does not check/care if the XBlock actually exists.
"""
usage_key_str = str(usage_key)
site_root_url = get_xblock_app_config().get_site_root_url()
if not user:
raise TypeError("Cannot get handler URLs without specifying a specific user ID.")
Expand All @@ -291,10 +290,10 @@ def get_handler_url(
raise ValueError("Invalid user value")
# Now generate a token-secured URL for this handler, specific to this user
# and this XBlock:
secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str)
secure_token = get_secure_token_for_xblock_handler(user_id, str(usage_key))
# Now generate the URL to that handler:
kwargs = {
'usage_key_str': usage_key_str,
'usage_key': usage_key,
'user_id': user_id,
'secure_token': secure_token,
'handler_name': handler_name,
Expand Down
23 changes: 23 additions & 0 deletions openedx/core/djangoapps/xblock/rest_api/url_converters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
"""
URL pattern converters
https://docs.djangoproject.com/en/5.1/topics/http/urls/#registering-custom-path-converters
"""
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKeyV2


class UsageKeyV2Converter:
"""
Converter that matches V2 usage keys like:
lb:Org:LIB:drag-and-drop-v2:91c2b1d5
"""
regex = r'[\w-]+(:[\w\-.]+)+'

def to_python(self, value):
try:
return UsageKeyV2.from_string(value)
except InvalidKeyError as exc:
raise ValueError from exc

def to_url(self, value):
return str(value)
16 changes: 9 additions & 7 deletions openedx/core/djangoapps/xblock/rest_api/urls.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
"""
URL configuration for the new XBlock API
"""
from django.urls import include, path, re_path
from django.urls import include, path, re_path, register_converter
from .url_converters import UsageKeyV2Converter
from . import views

# Note that the exact same API URLs are used in Studio and the LMS, but the API
# may act a bit differently in each (e.g. Studio stores user state ephemerally).
# If necessary at some point in the future, these URLs could be duplicated into
# urls_studio and urls_lms, and/or the views could be likewise duplicated.
app_name = 'openedx.core.djangoapps.xblock.rest_api'

register_converter(UsageKeyV2Converter, "usage_v2")

block_endpoints = [
# get metadata about an XBlock:
path('', views.block_metadata),
# get/post full json fields of an XBlock:
path('fields/', views.BlockFieldsView.as_view()),
# render one of this XBlock's views (e.g. student_view)
re_path(r'^view/(?P<view_name>[\w\-]+)/$', views.render_block_view),
path('view/<str:view_name>/', views.render_block_view),
# get the URL needed to call this XBlock's handlers
re_path(r'^handler_url/(?P<handler_name>[\w\-]+)/$', views.get_handler_url),
path('handler_url/<str:handler_name>/', views.get_handler_url),
# call one of this block's handlers
re_path(
r'^handler/(?P<user_id>\w+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
Expand All @@ -30,12 +33,11 @@

urlpatterns = [
path('api/xblock/v2/', include([
# This pattern with the version must come first so '@' isn't included in usage_key_str below
path(r'xblocks/<str:usage_key_str>@<str:version>/', include(block_endpoints)),
path(r'xblocks/<str:usage_key_str>/', include(block_endpoints)),
path(r'xblocks/<usage_v2:usage_key>/', include(block_endpoints)),
path(r'xblocks/<usage_v2:usage_key>@<str:version>/', include(block_endpoints)),
])),
# Non-API views (these return HTML, not JSON):
path('xblocks/v2/<str:usage_key_str>/', include([
path('xblocks/v2/<usage_v2:usage_key>/', include([
# render one of this XBlock's views (e.g. student_view) for embedding in an iframe
# NOTE: this endpoint is **unstable** and subject to changes after Sumac
path('embed/<str:view_name>/', views.embed_block_view),
Expand Down
53 changes: 14 additions & 39 deletions openedx/core/djangoapps/xblock/rest_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def parse_version_request(version_str: str | None) -> LatestVersion | int:
@api_view(['GET'])
@view_auth_classes(is_authenticated=False)
@permission_classes((permissions.AllowAny, )) # Permissions are handled at a lower level, by the learning context
def block_metadata(request, usage_key_str):
def block_metadata(request, usage_key, version=None):
"""
Get metadata about the specified block.
Expand All @@ -74,10 +74,8 @@ def block_metadata(request, usage_key_str):
* "include": a comma-separated list of keys to include.
Valid keys are "index_dictionary" and "student_view_data".
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e
if version:
raise NotImplementedError()

block = load_block(usage_key, request.user)
includes = request.GET.get("include", "").split(",")
Expand All @@ -92,14 +90,12 @@ def block_metadata(request, usage_key_str):
@api_view(['GET'])
@view_auth_classes(is_authenticated=False)
@permission_classes((permissions.AllowAny, )) # Permissions are handled at a lower level, by the learning context
def render_block_view(request, usage_key_str, view_name):
def render_block_view(request, usage_key, view_name, version=None):
"""
Get the HTML, JS, and CSS needed to render the given XBlock.
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e
if version:
raise NotImplementedError()

try:
block = load_block(usage_key, request.user)
Expand All @@ -116,17 +112,12 @@ def render_block_view(request, usage_key_str, view_name):
@view_auth_classes(is_authenticated=False)
@permission_classes((permissions.AllowAny, )) # Permissions are handled at a lower level, by the learning context
@xframe_options_exempt
def embed_block_view(request, usage_key_str, view_name):
def embed_block_view(request, usage_key, view_name):
"""
Render the given XBlock in an <iframe>
Unstable - may change after Sumac
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

# Check if a specific version has been requested
version = parse_version_request(request.GET.get("version"))

Expand Down Expand Up @@ -162,17 +153,15 @@ def embed_block_view(request, usage_key_str, view_name):

@api_view(['GET'])
@view_auth_classes(is_authenticated=False)
def get_handler_url(request, usage_key_str, handler_name):
def get_handler_url(request, usage_key, handler_name, version=None):
"""
Get an absolute URL which can be used (without any authentication) to call
the given XBlock handler.
The URL will expire but is guaranteed to be valid for a minimum of 2 days.
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e
if version:
raise NotImplementedError()

handler_url = _get_handler_url(usage_key, handler_name, request.user)
return Response({"handler_url": handler_url})
Expand All @@ -184,24 +173,19 @@ def get_handler_url(request, usage_key_str, handler_name):
# and https://github.com/openedx/XBlock/pull/383 for context.
@csrf_exempt
@xframe_options_exempt
def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name, suffix=None, version=None):
def xblock_handler(request, user_id, secure_token, usage_key, handler_name, suffix=None, version=None):
"""
Run an XBlock's handler and return the result
This endpoint has a unique authentication scheme that involves a temporary
auth token included in the URL (see below). As a result it can be exempt
from CSRF, session auth, and JWT/OAuth.
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise Http404 from e

# To support sandboxed XBlocks, custom frontends, and other use cases, we
# authenticate requests using a secure token in the URL. see
# openedx.core.djangoapps.xblock.utils.get_secure_hash_for_xblock_handler
# for details and rationale.
if not validate_secure_token_for_xblock_handler(user_id, usage_key_str, secure_token):
if not validate_secure_token_for_xblock_handler(user_id, str(usage_key), secure_token):
raise PermissionDenied("Invalid/expired auth token.")
if request.user.is_authenticated:
# The user authenticated twice, e.g. with session auth and the token.
Expand Down Expand Up @@ -265,14 +249,10 @@ class BlockFieldsView(APIView):
"""

@atomic
def get(self, request, usage_key_str):
def get(self, request, usage_key):
"""
retrieves the xblock, returning display_name, data, and metadata
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

# The "fields" view requires "read as author" permissions because the fields can contain answers, etc.
block = load_block(usage_key, request.user, check_permission=CheckPerm.CAN_READ_AS_AUTHOR)
Expand All @@ -288,15 +268,10 @@ def get(self, request, usage_key_str):
return Response(block_dict)

@atomic
def post(self, request, usage_key_str):
def post(self, request, usage_key):
"""
edits the xblock, saving changes to data and metadata only (display_name included in metadata)
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

user = request.user
block = load_block(usage_key, user, check_permission=CheckPerm.CAN_EDIT)
data = request.data.get("data")
Expand Down

0 comments on commit 8d10909

Please sign in to comment.