From 1fe29dc26666b7ab906a8008076ddb7a43c953ec Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Fri, 4 Oct 2024 17:39:14 -0700
Subject: [PATCH 1/9] fix: improve v2 library block permissions checks for
 read-only authors

---
 .../content_libraries/library_context.py      | 13 ++++++++
 .../content_libraries/permissions.py          | 15 +++++++--
 .../content_libraries/tests/base.py           |  9 ++++++
 .../tests/test_content_libraries.py           |  9 +++++-
 openedx/core/djangoapps/xblock/api.py         | 31 +++++++++++++++----
 .../learning_context/learning_context.py      |  8 +++++
 .../core/djangoapps/xblock/rest_api/views.py  |  8 +++--
 7 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py
index 6ff426e73560..888c4ca323c3 100644
--- a/openedx/core/djangoapps/content_libraries/library_context.py
+++ b/openedx/core/djangoapps/content_libraries/library_context.py
@@ -49,6 +49,19 @@ def can_edit_block(self, user, usage_key):
 
         return self.block_exists(usage_key)
 
+    def can_view_block_for_editing(self, user, usage_key):
+        """
+        Does the specified usage key exist in its context, and if so, does the
+        specified user have permission to view its fields and OLX details (but
+        not necessarily to make changes to it).
+        """
+        try:
+            api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
+        except (PermissionDenied, api.ContentLibraryNotFound):
+            return False
+
+        return self.block_exists(usage_key)
+
     def can_view_block(self, user, usage_key):
         """
         Does the specified usage key exist in its context, and if so, does the
diff --git a/openedx/core/djangoapps/content_libraries/permissions.py b/openedx/core/djangoapps/content_libraries/permissions.py
index c7da012c9fb4..0fe1c94728c6 100644
--- a/openedx/core/djangoapps/content_libraries/permissions.py
+++ b/openedx/core/djangoapps/content_libraries/permissions.py
@@ -2,7 +2,8 @@
 Permissions for Content Libraries (v2, Learning-Core-based)
 """
 from bridgekeeper import perms, rules
-from bridgekeeper.rules import Attribute, ManyRelation, Relation, in_current_groups
+from bridgekeeper.rules import Attribute, ManyRelation, Relation, blanket_rule, in_current_groups
+from django.conf import settings
 
 from openedx.core.djangoapps.content_libraries.models import ContentLibraryPermission
 
@@ -41,6 +42,12 @@
 )
 
 
+# Are we in Studio? (Is there a better or more contextual way to define this, e.g. get from learning context?)
+@blanket_rule
+def is_studio_request(_):
+    return settings.ROOT_URLCONF == "cms.urls"
+
+
 ########################### Permissions ###########################
 
 # Is the user allowed to view XBlocks from the specified content library
@@ -51,10 +58,12 @@
 perms[CAN_LEARN_FROM_THIS_CONTENT_LIBRARY] = (
     # Global staff can learn from any library:
     is_global_staff |
-    # Regular users can learn if the library allows public learning:
+    # Regular and even anonymous users can learn if the library allows public learning:
     Attribute('allow_public_learning', True) |
     # Users/groups who are explicitly granted permission can learn from the library:
-    (is_user_active & has_explicit_read_permission_for_library)
+    (is_user_active & has_explicit_read_permission_for_library) |
+    # Or, in Studio (but not the LMS) any users can access libraries with "public read" permissions:
+    (is_studio_request & is_user_active & Attribute('allow_public_read', True))
 )
 
 # Is the user allowed to create content libraries?
diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py
index 987133255f58..0ed47f995f50 100644
--- a/openedx/core/djangoapps/content_libraries/tests/base.py
+++ b/openedx/core/djangoapps/content_libraries/tests/base.py
@@ -308,3 +308,12 @@ def _get_block_handler_url(self, block_key, handler_name):
         """
         url = URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name)
         return self._api('get', url, None, expect_response=200)["handler_url"]
+
+    def _get_library_block_fields(self, block_key, expect_response=200):
+        """ Get the fields of a specific block in the library. This API is only used by the MFE editors. """
+        result = self._api('get', URL_BLOCK_FIELDS_URL.format(block_key=block_key), None, expect_response)
+        return result
+
+    def _set_library_block_fields(self, block_key, new_fields, expect_response=200):
+        """ Set the fields of a specific block in the library. This API is only used by the MFE editors. """
+        return self._api('post', URL_BLOCK_FIELDS_URL.format(block_key=block_key), new_fields, expect_response)
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
index d995a2c79683..e7ef98d2b703 100644
--- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
@@ -635,21 +635,27 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
         # A random user cannot read OLX nor assets (this library has allow_public_read False):
         with self.as_user(random_user):
             self._get_library_block_olx(block3_key, expect_response=403)
+            self._get_library_block_fields(block3_key, expect_response=404)
             self._get_library_block_assets(block3_key, expect_response=403)
             self._get_library_block_asset(block3_key, file_name="whatever.png", expect_response=403)
+            # Nor can they preview the block:
+            self._render_block_view(block3_key, view_name="student_view", expect_response=404)
         # But if we grant allow_public_read, then they can:
         with self.as_user(admin):
             self._update_library(lib_id, allow_public_read=True)
             # self._set_library_block_asset(block3_key, "whatever.png", b"data")
         with self.as_user(random_user):
             self._get_library_block_olx(block3_key)
+            self._render_block_view(block3_key, view_name="student_view")
+            f = self._get_library_block_fields(block3_key)
             # self._get_library_block_assets(block3_key)
             # self._get_library_block_asset(block3_key, file_name="whatever.png")
 
-        # Users without authoring permission cannot edit nor delete XBlocks (this library has allow_public_read False):
+        # Users without authoring permission cannot edit nor delete XBlocks:
         for user in [reader, random_user]:
             with self.as_user(user):
                 self._set_library_block_olx(block3_key, "<problem/>", expect_response=403)
+                self._set_library_block_fields(block3_key, {"data": "<problem />", "metadata": {}}, expect_response=404)
                 # self._set_library_block_asset(block3_key, "test.txt", b"data", expect_response=403)
                 self._delete_library_block(block3_key, expect_response=403)
                 self._commit_library_changes(lib_id, expect_response=403)
@@ -659,6 +665,7 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
         with self.as_user(author_group_member):
             olx = self._get_library_block_olx(block3_key)
             self._set_library_block_olx(block3_key, olx)
+            self._set_library_block_fields(block3_key, {"data": olx, "metadata": {}})
             # self._get_library_block_assets(block3_key)
             # self._set_library_block_asset(block3_key, "test.txt", b"data")
             # self._get_library_block_asset(block3_key, file_name="test.txt")
diff --git a/openedx/core/djangoapps/xblock/api.py b/openedx/core/djangoapps/xblock/api.py
index 80a4bd57eab8..2c5e5ad9ae5e 100644
--- a/openedx/core/djangoapps/xblock/api.py
+++ b/openedx/core/djangoapps/xblock/api.py
@@ -8,7 +8,7 @@
 Studio APIs cover use cases like adding/deleting/editing blocks.
 """
 # pylint: disable=unused-import
-
+from enum import Enum
 from datetime import datetime
 import logging
 import threading
@@ -43,6 +43,16 @@
 log = logging.getLogger(__name__)
 
 
+class CheckPerm(Enum):
+    """ Options for the default permission check done by load_block() """
+    # can view the published block and call handlers etc. but not necessarily view its OLX source nor field data
+    CAN_LEARN = 1
+    # read-only studio view: can see the block (draft or published), see its OLX, see its field data, etc.
+    CAN_READ_AS_AUTHOR = 2
+    # can view everything and make changes to the block
+    CAN_EDIT = 3
+
+
 def get_runtime_system():
     """
     Return a new XBlockRuntimeSystem.
@@ -74,7 +84,7 @@ def get_runtime_system():
     return runtime
 
 
-def load_block(usage_key, user):
+def load_block(usage_key, user, *, check_permission: CheckPerm | None = CheckPerm.CAN_LEARN):
     """
     Load the specified XBlock for the given user.
 
@@ -94,10 +104,19 @@ def load_block(usage_key, user):
 
     # Now, check if the block exists in this context and if the user has
     # permission to render this XBlock view:
-    if user is not None and not context_impl.can_view_block(user, usage_key):
-        # We do not know if the block was not found or if the user doesn't have
-        # permission, but we want to return the same result in either case:
-        raise NotFound(f"XBlock {usage_key} does not exist, or you don't have permission to view it.")
+    if check_permission and user is not None:
+        if check_permission == CheckPerm.CAN_EDIT:
+            has_perm = context_impl.can_edit_block(user, usage_key)
+        elif check_permission == CheckPerm.CAN_READ_AS_AUTHOR:
+            has_perm = context_impl.can_view_block_for_editing(user, usage_key)
+        elif check_permission == CheckPerm.CAN_LEARN:
+            has_perm = context_impl.can_view_block(user, usage_key)
+        else:
+            has_perm = False
+        if not has_perm:
+            # We do not know if the block was not found or if the user doesn't have
+            # permission, but we want to return the same result in either case:
+            raise NotFound(f"XBlock {usage_key} does not exist, or you don't have permission to view it.")
 
     # TODO: load field overrides from the context
     # e.g. a course might specify that all 'problem' XBlocks have 'max_attempts'
diff --git a/openedx/core/djangoapps/xblock/learning_context/learning_context.py b/openedx/core/djangoapps/xblock/learning_context/learning_context.py
index 2dc5155dc4e2..baf841311a69 100644
--- a/openedx/core/djangoapps/xblock/learning_context/learning_context.py
+++ b/openedx/core/djangoapps/xblock/learning_context/learning_context.py
@@ -37,6 +37,14 @@ def can_edit_block(self, user, usage_key):  # pylint: disable=unused-argument
         """
         return False
 
+    def can_view_block_for_editing(self, user, usage_key):
+        """
+        Does the specified usage key exist in its context, and if so, does the
+        specified user have permission to view its fields and OLX details (but
+        not necessarily to make changes to it).
+        """
+        return self.can_edit_block(user, usage_key)
+
     def can_view_block(self, user, usage_key):  # pylint: disable=unused-argument
         """
         Does the specified usage key exist in its context, and if so, does the
diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py
index 7934e24bd2db..20e6a6dde0b2 100644
--- a/openedx/core/djangoapps/xblock/rest_api/views.py
+++ b/openedx/core/djangoapps/xblock/rest_api/views.py
@@ -29,6 +29,7 @@
 from openedx.core.djangoapps.xblock.learning_context.manager import get_learning_context_impl
 from openedx.core.lib.api.view_utils import view_auth_classes
 from ..api import (
+    CheckPerm,
     get_block_metadata,
     get_block_display_name,
     get_handler_url as _get_handler_url,
@@ -108,7 +109,7 @@ def embed_block_view(request, usage_key_str, view_name):
         raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e
 
     try:
-        block = load_block(usage_key, request.user)
+        block = load_block(usage_key, request.user, check_permission=CheckPerm.CAN_LEARN)
     except NoSuchUsage as exc:
         raise NotFound(f"{usage_key} not found") from exc
 
@@ -246,7 +247,8 @@ def get(self, request, usage_key_str):
         except InvalidKeyError as e:
             raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e
 
-        block = load_block(usage_key, request.user)
+        # 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)
         block_dict = {
             "display_name": get_block_display_name(block),  # potentially duplicated from metadata
             "data": block.data,
@@ -265,7 +267,7 @@ def post(self, request, usage_key_str):
             raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e
 
         user = request.user
-        block = load_block(usage_key, user)
+        block = load_block(usage_key, user, check_permission=CheckPerm.CAN_EDIT)
         data = request.data.get("data")
         metadata = request.data.get("metadata")
 

From 4776de31afd4c6cf241ef558c54c3b0e2a7b5fc7 Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Sat, 5 Oct 2024 08:18:55 -0700
Subject: [PATCH 2/9] fix: Allow accessing the v2 xblock "embed block" view
 from Studio

---
 cms/templates/xblock_v2/xblock_iframe.html       | 1 +
 openedx/core/djangoapps/xblock/rest_api/views.py | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 120000 cms/templates/xblock_v2/xblock_iframe.html

diff --git a/cms/templates/xblock_v2/xblock_iframe.html b/cms/templates/xblock_v2/xblock_iframe.html
new file mode 120000
index 000000000000..361225fd9088
--- /dev/null
+++ b/cms/templates/xblock_v2/xblock_iframe.html
@@ -0,0 +1 @@
+../../../lms/templates/xblock_v2/xblock_iframe.html
\ No newline at end of file
diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py
index 20e6a6dde0b2..b864eefaa4ac 100644
--- a/openedx/core/djangoapps/xblock/rest_api/views.py
+++ b/openedx/core/djangoapps/xblock/rest_api/views.py
@@ -125,7 +125,7 @@ def embed_block_view(request, usage_key_str, view_name):
         'lms_root_url': lms_root_url,
         'is_development': settings.DEBUG,
     }
-    response = render(request, 'xblock_v2/xblock_iframe.html', context, content_type='text/html')
+    response = render(request, 'xblock_v2/xblock_iframe.html', context, content_type='text/html', using='django')
 
     # Only allow this iframe be embedded if the parent is in the CORS_ORIGIN_WHITELIST
     cors_origin_whitelist = configuration_helpers.get_value('CORS_ORIGIN_WHITELIST', settings.CORS_ORIGIN_WHITELIST)

From c975aafcf798911fbcc9004ecd3692d19fc4605f Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Sat, 5 Oct 2024 08:19:13 -0700
Subject: [PATCH 3/9] fix: use white background for v2 runtime's "embed block"
 view

---
 cms/templates/content_libraries/xblock_iframe.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cms/templates/content_libraries/xblock_iframe.html b/cms/templates/content_libraries/xblock_iframe.html
index b6e455f78515..8b733373bd82 100644
--- a/cms/templates/content_libraries/xblock_iframe.html
+++ b/cms/templates/content_libraries/xblock_iframe.html
@@ -156,7 +156,7 @@
 <!-- The default stylesheet will set the body min-height to 100% (a common strategy to allow for background
   images to fill the viewport), but this has the undesireable side-effect of causing an infinite loop via the onResize
   event listeners below, in certain situations.  Resetting it to the default "auto" skirts the problem.-->
-<body style="min-height: auto">
+<body style="min-height: auto; background-color: white;">
   <!-- fragment body -->
   {{ fragment.body_html | safe }}
   <!-- fragment foot -->

From 3882fe397cc81d8016b01741b64162ee6c6a9a3a Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Mon, 7 Oct 2024 10:47:36 -0700
Subject: [PATCH 4/9] fix: Allow accessing the v2 xblock "embed block" view
 from Studio (take 2)

---
 cms/templates/xblock_v2/xblock_iframe.html                      | 1 -
 .../templates/xblock_v2}/xblock_iframe.html                     | 0
 lms/templates/xblock_v2/xblock_iframe.html                      | 1 -
 openedx/core/djangoapps/content_libraries/views.py              | 2 +-
 openedx/core/djangoapps/xblock/rest_api/views.py                | 2 +-
 5 files changed, 2 insertions(+), 4 deletions(-)
 delete mode 120000 cms/templates/xblock_v2/xblock_iframe.html
 rename {cms/templates/content_libraries => common/templates/xblock_v2}/xblock_iframe.html (100%)
 delete mode 120000 lms/templates/xblock_v2/xblock_iframe.html

diff --git a/cms/templates/xblock_v2/xblock_iframe.html b/cms/templates/xblock_v2/xblock_iframe.html
deleted file mode 120000
index 361225fd9088..000000000000
--- a/cms/templates/xblock_v2/xblock_iframe.html
+++ /dev/null
@@ -1 +0,0 @@
-../../../lms/templates/xblock_v2/xblock_iframe.html
\ No newline at end of file
diff --git a/cms/templates/content_libraries/xblock_iframe.html b/common/templates/xblock_v2/xblock_iframe.html
similarity index 100%
rename from cms/templates/content_libraries/xblock_iframe.html
rename to common/templates/xblock_v2/xblock_iframe.html
diff --git a/lms/templates/xblock_v2/xblock_iframe.html b/lms/templates/xblock_v2/xblock_iframe.html
deleted file mode 120000
index 7264c253466d..000000000000
--- a/lms/templates/xblock_v2/xblock_iframe.html
+++ /dev/null
@@ -1 +0,0 @@
-../../../cms/templates/content_libraries/xblock_iframe.html
\ No newline at end of file
diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py
index 835a5de1f1f2..3712af6e597f 100644
--- a/openedx/core/djangoapps/content_libraries/views.py
+++ b/openedx/core/djangoapps/content_libraries/views.py
@@ -919,7 +919,7 @@ class LtiToolLaunchView(TemplateResponseMixin, LtiToolView):
     LTI platform.  Other features and resouces are ignored.
     """
 
-    template_name = 'content_libraries/xblock_iframe.html'
+    template_name = 'xblock_v2/xblock_iframe.html'
 
     @property
     def launch_data(self):
diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py
index b864eefaa4ac..20e6a6dde0b2 100644
--- a/openedx/core/djangoapps/xblock/rest_api/views.py
+++ b/openedx/core/djangoapps/xblock/rest_api/views.py
@@ -125,7 +125,7 @@ def embed_block_view(request, usage_key_str, view_name):
         'lms_root_url': lms_root_url,
         'is_development': settings.DEBUG,
     }
-    response = render(request, 'xblock_v2/xblock_iframe.html', context, content_type='text/html', using='django')
+    response = render(request, 'xblock_v2/xblock_iframe.html', context, content_type='text/html')
 
     # Only allow this iframe be embedded if the parent is in the CORS_ORIGIN_WHITELIST
     cors_origin_whitelist = configuration_helpers.get_value('CORS_ORIGIN_WHITELIST', settings.CORS_ORIGIN_WHITELIST)

From 59e0ebd3673f60d21eb3b52de9d2026bd74d8c51 Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Mon, 7 Oct 2024 10:48:54 -0700
Subject: [PATCH 5/9] refactor: use SERVICE_VARIANT instead of ROOT_URLCONF

---
 openedx/core/djangoapps/content_libraries/permissions.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/openedx/core/djangoapps/content_libraries/permissions.py b/openedx/core/djangoapps/content_libraries/permissions.py
index 0fe1c94728c6..17671b5659f8 100644
--- a/openedx/core/djangoapps/content_libraries/permissions.py
+++ b/openedx/core/djangoapps/content_libraries/permissions.py
@@ -45,7 +45,7 @@
 # Are we in Studio? (Is there a better or more contextual way to define this, e.g. get from learning context?)
 @blanket_rule
 def is_studio_request(_):
-    return settings.ROOT_URLCONF == "cms.urls"
+    return settings.SERVICE_VARIANT == "cms"
 
 
 ########################### Permissions ###########################

From 8ef9e0c39dde6cd23303b7d1a275357a138ac24d Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Mon, 7 Oct 2024 12:02:23 -0700
Subject: [PATCH 6/9] fix: improve v2 library block permissions checks for
 read-only authors

---
 .../content_libraries/library_context.py      | 65 ++++++++++---------
 .../tests/test_content_libraries.py           | 39 +++++++----
 openedx/core/djangoapps/xblock/api.py         | 17 +++--
 .../learning_context/learning_context.py      | 23 ++++---
 4 files changed, 83 insertions(+), 61 deletions(-)

diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py
index 888c4ca323c3..d62cedb645a5 100644
--- a/openedx/core/djangoapps/content_libraries/library_context.py
+++ b/openedx/core/djangoapps/content_libraries/library_context.py
@@ -1,19 +1,20 @@
 """
 Definition of "Library" as a learning context.
 """
-
 import logging
 
 from django.core.exceptions import PermissionDenied
+from rest_framework.exceptions import NotFound
 
 from openedx_events.content_authoring.data import LibraryBlockData
 from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED
+from opaque_keys.edx.keys import UsageKeyV2
+from openedx_learning.api import authoring as authoring_api
 
 from openedx.core.djangoapps.content_libraries import api, permissions
 from openedx.core.djangoapps.content_libraries.models import ContentLibrary
 from openedx.core.djangoapps.xblock.api import LearningContext
-
-from openedx_learning.api import authoring as authoring_api
+from openedx.core.types import User as UserType
 
 log = logging.getLogger(__name__)
 
@@ -30,60 +31,60 @@ def __init__(self, **kwargs):
         super().__init__(**kwargs)
         self.use_draft = kwargs.get('use_draft', None)
 
-    def can_edit_block(self, user, usage_key):
+    def can_edit_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
-        Does the specified usage key exist in its context, and if so, does the
-        specified user have permission to edit it (make changes to the authored
-        data store)?
-
-        user: a Django User object (may be an AnonymousUser)
-
-        usage_key: the UsageKeyV2 subclass used for this learning context
+        Assuming a block with the specified ID (usage_key) exists, does the
+        specified user have permission to edit it (make changes to the
+        fields / authored data store)?
 
-        Must return a boolean.
+        May raise ContentLibraryNotFound if the library does not exist.
         """
         try:
             api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
-        except (PermissionDenied, api.ContentLibraryNotFound):
+            return True
+        except PermissionDenied:
             return False
+        except api.ContentLibraryNotFound:
+            # A 404 is probably what you want in this case, not a 500 error, so do that by default.
+            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
 
-        return self.block_exists(usage_key)
-
-    def can_view_block_for_editing(self, user, usage_key):
+    def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
-        Does the specified usage key exist in its context, and if so, does the
+        Assuming a block with the specified ID (usage_key) exists, does the
         specified user have permission to view its fields and OLX details (but
-        not necessarily to make changes to it).
+        not necessarily to make changes to it)?
+
+        May raise ContentLibraryNotFound if the library does not exist.
         """
         try:
             api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
-        except (PermissionDenied, api.ContentLibraryNotFound):
+            return True
+        except PermissionDenied:
             return False
+        except api.ContentLibraryNotFound:
+            # A 404 is probably what you want in this case, not a 500 error, so do that by default.
+            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
 
-        return self.block_exists(usage_key)
-
-    def can_view_block(self, user, usage_key):
+    def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
         Does the specified usage key exist in its context, and if so, does the
         specified user have permission to view it and interact with it (call
         handlers, save user state, etc.)?
 
-        user: a Django User object (may be an AnonymousUser)
-
-        usage_key: the UsageKeyV2 subclass used for this learning context
-
-        Must return a boolean.
+        May raise ContentLibraryNotFound if the library does not exist.
         """
         try:
             api.require_permission_for_library_key(
                 usage_key.lib_key, user, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY,
             )
-        except (PermissionDenied, api.ContentLibraryNotFound):
+            return True
+        except PermissionDenied:
             return False
+        except api.ContentLibraryNotFound:
+            # A 404 is probably what you want in this case, not a 500 error, so do that by default.
+            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
 
-        return self.block_exists(usage_key)
-
-    def block_exists(self, usage_key):
+    def block_exists(self, usage_key: UsageKeyV2):
         """
         Does the block for this usage_key exist in this Library?
 
@@ -110,7 +111,7 @@ def block_exists(self, usage_key):
             local_key=usage_key.block_id,
         )
 
-    def send_block_updated_event(self, usage_key):
+    def send_block_updated_event(self, usage_key: UsageKeyV2):
         """
         Send a "block updated" event for the library block with the given usage_key.
 
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
index e7ef98d2b703..a84f2924c63b 100644
--- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
@@ -502,6 +502,30 @@ def test_library_blocks_type_constrained(self, slug, library_type, block_type, e
         # Add a 'problem' XBlock to the library:
         self._add_block_to_library(lib_id, block_type, 'test-block', expect_response=expect_response)
 
+    def test_library_not_found(self):
+        """Test that requests fail with 404 when the library does not exist"""
+        valid_not_found_key = 'lb:valid:key:video:1'
+        response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=valid_not_found_key))
+        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.json(), {
+            'detail': f"Content Library 'lib:valid:key' does not exist",
+        })
+
+    def test_block_not_found(self):
+        """Test that requests fail with 404 when the library exists but the XBlock does not"""
+        lib = self._create_library(
+            slug="test_lib_block_event_delete",
+            title="Event Test Library",
+            description="Testing event in library"
+        )
+        library_key = LibraryLocatorV2.from_string(lib['id'])
+        non_existent_block_key = LibraryUsageLocatorV2(lib_key=library_key, block_type='video', usage_id='123')
+        response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=non_existent_block_key))
+        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.json(), {
+            'detail': f"The component '{non_existent_block_key}' does not exist.",
+        })
+
     # Test that permissions are enforced for content libraries
 
     def test_library_permissions(self):  # pylint: disable=too-many-statements
@@ -635,11 +659,11 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
         # A random user cannot read OLX nor assets (this library has allow_public_read False):
         with self.as_user(random_user):
             self._get_library_block_olx(block3_key, expect_response=403)
-            self._get_library_block_fields(block3_key, expect_response=404)
+            self._get_library_block_fields(block3_key, expect_response=403)
             self._get_library_block_assets(block3_key, expect_response=403)
             self._get_library_block_asset(block3_key, file_name="whatever.png", expect_response=403)
             # Nor can they preview the block:
-            self._render_block_view(block3_key, view_name="student_view", expect_response=404)
+            self._render_block_view(block3_key, view_name="student_view", expect_response=403)
         # But if we grant allow_public_read, then they can:
         with self.as_user(admin):
             self._update_library(lib_id, allow_public_read=True)
@@ -655,7 +679,7 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
         for user in [reader, random_user]:
             with self.as_user(user):
                 self._set_library_block_olx(block3_key, "<problem/>", expect_response=403)
-                self._set_library_block_fields(block3_key, {"data": "<problem />", "metadata": {}}, expect_response=404)
+                self._set_library_block_fields(block3_key, {"data": "<problem />", "metadata": {}}, expect_response=403)
                 # self._set_library_block_asset(block3_key, "test.txt", b"data", expect_response=403)
                 self._delete_library_block(block3_key, expect_response=403)
                 self._commit_library_changes(lib_id, expect_response=403)
@@ -1094,12 +1118,3 @@ def test_xblock_handler_invalid_key(self):
             secure_token='random',
         )))
         self.assertEqual(response.status_code, 404)
-
-    def test_not_found_fails_correctly(self):
-        """Test fails with 404 when xblock key is valid but not found."""
-        valid_not_found_key = 'lb:valid:key:video:1'
-        response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=valid_not_found_key))
-        self.assertEqual(response.status_code, 404)
-        self.assertEqual(response.json(), {
-            'detail': f"XBlock {valid_not_found_key} does not exist, or you don't have permission to view it.",
-        })
diff --git a/openedx/core/djangoapps/xblock/api.py b/openedx/core/djangoapps/xblock/api.py
index 2c5e5ad9ae5e..6635e014e3e2 100644
--- a/openedx/core/djangoapps/xblock/api.py
+++ b/openedx/core/djangoapps/xblock/api.py
@@ -13,6 +13,7 @@
 import logging
 import threading
 
+from django.core.exceptions import PermissionDenied
 from django.urls import reverse
 from django.utils.translation import gettext as _
 from openedx_learning.api import authoring as authoring_api
@@ -21,7 +22,7 @@
 from opaque_keys.edx.locator import BundleDefinitionLocator, LibraryUsageLocatorV2
 from rest_framework.exceptions import NotFound
 from xblock.core import XBlock
-from xblock.exceptions import NoSuchViewError
+from xblock.exceptions import NoSuchUsage, NoSuchViewError
 from xblock.plugin import PluginMissingError
 
 from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
@@ -91,8 +92,8 @@ def load_block(usage_key, user, *, check_permission: CheckPerm | None = CheckPer
     Returns an instantiated XBlock.
 
     Exceptions:
-        NotFound - if the XBlock doesn't exist or if the user doesn't have the
-                   necessary permissions
+        NotFound - if the XBlock doesn't exist
+        PermissionDenied - if the user doesn't have the necessary permissions
 
     Args:
         usage_key(OpaqueKey): block identifier
@@ -114,9 +115,7 @@ def load_block(usage_key, user, *, check_permission: CheckPerm | None = CheckPer
         else:
             has_perm = False
         if not has_perm:
-            # We do not know if the block was not found or if the user doesn't have
-            # permission, but we want to return the same result in either case:
-            raise NotFound(f"XBlock {usage_key} does not exist, or you don't have permission to view it.")
+            raise PermissionDenied(f"You don't have permission to access the component '{usage_key}'.")
 
     # TODO: load field overrides from the context
     # e.g. a course might specify that all 'problem' XBlocks have 'max_attempts'
@@ -124,7 +123,11 @@ def load_block(usage_key, user, *, check_permission: CheckPerm | None = CheckPer
     # field_overrides = context_impl.get_field_overrides(usage_key)
     runtime = get_runtime_system().get_runtime(user=user)
 
-    return runtime.get_block(usage_key)
+    try:
+        return runtime.get_block(usage_key)
+    except NoSuchUsage:
+        # Convert NoSuchUsage to NotFound so we do the right thing (404 not 500) by default.
+        raise NotFound(f"The component '{usage_key}' does not exist.")
 
 
 def get_block_metadata(block, includes=()):
diff --git a/openedx/core/djangoapps/xblock/learning_context/learning_context.py b/openedx/core/djangoapps/xblock/learning_context/learning_context.py
index baf841311a69..b535e84ca7c1 100644
--- a/openedx/core/djangoapps/xblock/learning_context/learning_context.py
+++ b/openedx/core/djangoapps/xblock/learning_context/learning_context.py
@@ -2,6 +2,8 @@
 A "Learning Context" is a course, a library, a program, or some other collection
 of content where learning happens.
 """
+from openedx.core.types import User as UserType
+from opaque_keys.edx.keys import UsageKeyV2
 
 
 class LearningContext:
@@ -23,11 +25,11 @@ def __init__(self, **kwargs):
         parameters without changing the API.
         """
 
-    def can_edit_block(self, user, usage_key):  # pylint: disable=unused-argument
+    def can_edit_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:  # pylint: disable=unused-argument
         """
-        Does the specified usage key exist in its context, and if so, does the
-        specified user have permission to edit it (make changes to the authored
-        data store)?
+        Assuming a block with the specified ID (usage_key) exists, does the
+        specified user have permission to edit it (make changes to the
+        fields / authored data store)?
 
         user: a Django User object (may be an AnonymousUser)
 
@@ -37,19 +39,20 @@ def can_edit_block(self, user, usage_key):  # pylint: disable=unused-argument
         """
         return False
 
-    def can_view_block_for_editing(self, user, usage_key):
+    def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
-        Does the specified usage key exist in its context, and if so, does the
+        Assuming a block with the specified ID (usage_key) exists, does the
         specified user have permission to view its fields and OLX details (but
-        not necessarily to make changes to it).
+        not necessarily to make changes to it)?
         """
         return self.can_edit_block(user, usage_key)
 
-    def can_view_block(self, user, usage_key):  # pylint: disable=unused-argument
+    def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:  # pylint: disable=unused-argument
         """
-        Does the specified usage key exist in its context, and if so, does the
+        Assuming a block with the specified ID (usage_key) exists, does the
         specified user have permission to view it and interact with it (call
-        handlers, save user state, etc.)?
+        handlers, save user state, etc.)? This is also sometimes called the
+        "can_learn" permission.
 
         user: a Django User object (may be an AnonymousUser)
 

From 4aaf64ca981da571316fc9c364f1ce5db6801347 Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Mon, 7 Oct 2024 12:20:25 -0700
Subject: [PATCH 7/9] fix: type check warnings

---
 .../djangoapps/content_libraries/library_context.py   | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py
index d62cedb645a5..4e3178a90ad2 100644
--- a/openedx/core/djangoapps/content_libraries/library_context.py
+++ b/openedx/core/djangoapps/content_libraries/library_context.py
@@ -9,6 +9,7 @@
 from openedx_events.content_authoring.data import LibraryBlockData
 from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED
 from opaque_keys.edx.keys import UsageKeyV2
+from opaque_keys.edx.locator import LibraryUsageLocatorV2
 from openedx_learning.api import authoring as authoring_api
 
 from openedx.core.djangoapps.content_libraries import api, permissions
@@ -39,6 +40,7 @@ def can_edit_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
 
         May raise ContentLibraryNotFound if the library does not exist.
         """
+        assert isinstance(usage_key, LibraryUsageLocatorV2)
         try:
             api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
             return True
@@ -56,6 +58,7 @@ def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> b
 
         May raise ContentLibraryNotFound if the library does not exist.
         """
+        assert isinstance(usage_key, LibraryUsageLocatorV2)
         try:
             api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
             return True
@@ -73,6 +76,7 @@ def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
 
         May raise ContentLibraryNotFound if the library does not exist.
         """
+        assert isinstance(usage_key, LibraryUsageLocatorV2)
         try:
             api.require_permission_for_library_key(
                 usage_key.lib_key, user, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY,
@@ -84,7 +88,7 @@ def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
             # A 404 is probably what you want in this case, not a 500 error, so do that by default.
             raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
 
-    def block_exists(self, usage_key: UsageKeyV2):
+    def block_exists(self, usage_key: LibraryUsageLocatorV2):
         """
         Does the block for this usage_key exist in this Library?
 
@@ -96,7 +100,7 @@ def block_exists(self, usage_key: UsageKeyV2):
         version of it.
         """
         try:
-            content_lib = ContentLibrary.objects.get_by_key(usage_key.context_key)
+            content_lib = ContentLibrary.objects.get_by_key(usage_key.context_key)  # type: ignore[attr-defined]
         except ContentLibrary.DoesNotExist:
             return False
 
@@ -114,9 +118,8 @@ def block_exists(self, usage_key: UsageKeyV2):
     def send_block_updated_event(self, usage_key: UsageKeyV2):
         """
         Send a "block updated" event for the library block with the given usage_key.
-
-        usage_key: the UsageKeyV2 subclass used for this learning context
         """
+        assert isinstance(usage_key, LibraryUsageLocatorV2)
         LIBRARY_BLOCK_UPDATED.send_event(
             library_block=LibraryBlockData(
                 library_key=usage_key.lib_key,

From 3de76a58f491d1cec8f41ecad8c23f7d158572c4 Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Mon, 7 Oct 2024 13:28:16 -0700
Subject: [PATCH 8/9] chore: address lint warnings

---
 .../djangoapps/content_libraries/library_context.py  | 12 ++++++------
 .../tests/test_content_libraries.py                  |  2 +-
 openedx/core/djangoapps/xblock/api.py                |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py
index 4e3178a90ad2..3c9d09fb5666 100644
--- a/openedx/core/djangoapps/content_libraries/library_context.py
+++ b/openedx/core/djangoapps/content_libraries/library_context.py
@@ -46,9 +46,9 @@ def can_edit_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
             return True
         except PermissionDenied:
             return False
-        except api.ContentLibraryNotFound:
+        except api.ContentLibraryNotFound as exc:
             # A 404 is probably what you want in this case, not a 500 error, so do that by default.
-            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
+            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist") from exc
 
     def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
@@ -64,9 +64,9 @@ def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> b
             return True
         except PermissionDenied:
             return False
-        except api.ContentLibraryNotFound:
+        except api.ContentLibraryNotFound as exc:
             # A 404 is probably what you want in this case, not a 500 error, so do that by default.
-            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
+            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist") from exc
 
     def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
@@ -84,9 +84,9 @@ def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
             return True
         except PermissionDenied:
             return False
-        except api.ContentLibraryNotFound:
+        except api.ContentLibraryNotFound as exc:
             # A 404 is probably what you want in this case, not a 500 error, so do that by default.
-            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist")
+            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist") from exc
 
     def block_exists(self, usage_key: LibraryUsageLocatorV2):
         """
diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
index a84f2924c63b..56e258f8a1f1 100644
--- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
@@ -508,7 +508,7 @@ def test_library_not_found(self):
         response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=valid_not_found_key))
         self.assertEqual(response.status_code, 404)
         self.assertEqual(response.json(), {
-            'detail': f"Content Library 'lib:valid:key' does not exist",
+            'detail': "Content Library 'lib:valid:key' does not exist",
         })
 
     def test_block_not_found(self):
diff --git a/openedx/core/djangoapps/xblock/api.py b/openedx/core/djangoapps/xblock/api.py
index 6635e014e3e2..43ec3909bcd6 100644
--- a/openedx/core/djangoapps/xblock/api.py
+++ b/openedx/core/djangoapps/xblock/api.py
@@ -125,9 +125,9 @@ def load_block(usage_key, user, *, check_permission: CheckPerm | None = CheckPer
 
     try:
         return runtime.get_block(usage_key)
-    except NoSuchUsage:
+    except NoSuchUsage as exc:
         # Convert NoSuchUsage to NotFound so we do the right thing (404 not 500) by default.
-        raise NotFound(f"The component '{usage_key}' does not exist.")
+        raise NotFound(f"The component '{usage_key}' does not exist.") from exc
 
 
 def get_block_metadata(block, includes=()):

From f1737e15bedb95ea6d257412ea2675aa4f59862d Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Mon, 7 Oct 2024 14:07:00 -0700
Subject: [PATCH 9/9] refactor: make code more DRY

---
 .../content_libraries/library_context.py      | 30 ++++++-------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py
index 3c9d09fb5666..4bda10eb12fa 100644
--- a/openedx/core/djangoapps/content_libraries/library_context.py
+++ b/openedx/core/djangoapps/content_libraries/library_context.py
@@ -9,7 +9,7 @@
 from openedx_events.content_authoring.data import LibraryBlockData
 from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED
 from opaque_keys.edx.keys import UsageKeyV2
-from opaque_keys.edx.locator import LibraryUsageLocatorV2
+from opaque_keys.edx.locator import LibraryUsageLocatorV2, LibraryLocatorV2
 from openedx_learning.api import authoring as authoring_api
 
 from openedx.core.djangoapps.content_libraries import api, permissions
@@ -41,14 +41,7 @@ def can_edit_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         May raise ContentLibraryNotFound if the library does not exist.
         """
         assert isinstance(usage_key, LibraryUsageLocatorV2)
-        try:
-            api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
-            return True
-        except PermissionDenied:
-            return False
-        except api.ContentLibraryNotFound as exc:
-            # A 404 is probably what you want in this case, not a 500 error, so do that by default.
-            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist") from exc
+        return self._check_perm(user, usage_key.lib_key, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
 
     def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
@@ -59,14 +52,7 @@ def can_view_block_for_editing(self, user: UserType, usage_key: UsageKeyV2) -> b
         May raise ContentLibraryNotFound if the library does not exist.
         """
         assert isinstance(usage_key, LibraryUsageLocatorV2)
-        try:
-            api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
-            return True
-        except PermissionDenied:
-            return False
-        except api.ContentLibraryNotFound as exc:
-            # A 404 is probably what you want in this case, not a 500 error, so do that by default.
-            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist") from exc
+        return self._check_perm(user, usage_key.lib_key, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY)
 
     def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         """
@@ -77,16 +63,18 @@ def can_view_block(self, user: UserType, usage_key: UsageKeyV2) -> bool:
         May raise ContentLibraryNotFound if the library does not exist.
         """
         assert isinstance(usage_key, LibraryUsageLocatorV2)
+        return self._check_perm(user, usage_key.lib_key, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY)
+
+    def _check_perm(self, user: UserType, lib_key: LibraryLocatorV2, perm) -> bool:
+        """ Helper method to check a permission for the various can_ methods"""
         try:
-            api.require_permission_for_library_key(
-                usage_key.lib_key, user, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY,
-            )
+            api.require_permission_for_library_key(lib_key, user, perm)
             return True
         except PermissionDenied:
             return False
         except api.ContentLibraryNotFound as exc:
             # A 404 is probably what you want in this case, not a 500 error, so do that by default.
-            raise NotFound(f"Content Library '{usage_key.lib_key}' does not exist") from exc
+            raise NotFound(f"Content Library '{lib_key}' does not exist") from exc
 
     def block_exists(self, usage_key: LibraryUsageLocatorV2):
         """