Skip to content

Commit

Permalink
refactor: consider user role staff for masquerading (#27)
Browse files Browse the repository at this point in the history
  • Loading branch information
mariajgrimaldi authored Oct 2, 2023
1 parent ba978b5 commit 41e4cbe
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 38 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ Change Log
Unreleased
**********

0.8.4 - 2023-10-02
**********************************************

Changed
=======

* Consider checking both role staff and instructor

0.8.3 - 2023-10-02
**********************************************

Expand Down
2 changes: 1 addition & 1 deletion mindmap/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@

from .mindmap import MindMapXBlock

__version__ = '0.8.3'
__version__ = '0.8.4'
42 changes: 19 additions & 23 deletions mindmap/mindmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,22 @@ def get_current_user(self):
"""
return self.runtime.service(self, "user").get_current_user()

def is_course_staff(self, user) -> bool:
@property
def is_student(self) -> bool:
"""
Check whether the user has course staff permissions for this XBlock.
Check if the current user is a student.
"""
return user.opt_attrs.get("edx-platform.user_is_staff")
return self.get_current_user().opt_attrs.get("edx-platform.user_role") == "student"

def is_student(self, user) -> bool:
@property
def is_course_team(self) -> bool:
"""
Check if the user is a student.
Check if the user is part of the course team (instructor or staff).
"""
return user.opt_attrs.get("edx-platform.user_role") == "student"
user = self.get_current_user()
is_course_staff = user.opt_attrs.get("edx-platform.user_is_staff")
is_instructor = user.opt_attrs.get(ATTR_KEY_USER_ROLE) == "instructor"
return is_course_staff or is_instructor

def resource_string(self, path):
"""Handy helper for getting resources from our kit."""
Expand All @@ -199,7 +204,7 @@ def render_template(self, template_path, context=None) -> str:
template_path, context, i18n_service=self.runtime.service(self, 'i18n')
)

def get_context(self, user):
def get_context(self):
"""
Return the context for the student view.
Expand All @@ -209,7 +214,7 @@ def get_context(self, user):
Returns:
dict: The context for the student view
"""
in_student_view = self.is_student(user) or self.is_course_staff(user)
in_student_view = self.is_student or self.is_course_team
if self.is_static:
editable = False
else:
Expand Down Expand Up @@ -265,7 +270,7 @@ def student_view(self, _context=None) -> Fragment:
Fragment: The fragment to render
"""
user = self.get_current_user()
context = self.get_context(user)
context = self.get_context()
js_context = self.get_js_context(user, context)

if context["has_score"] and not context["can_submit_assignment"]:
Expand Down Expand Up @@ -293,7 +298,7 @@ def studio_view(self, context=None) -> Fragment:
Fragment: The fragment to render
"""
user = self.get_current_user()
context = self.get_context(user)
context = self.get_context()
js_context = self.get_js_context(user, context)

context.update({
Expand Down Expand Up @@ -350,16 +355,7 @@ def show_staff_grading_interface(self) -> bool:
bool: True if current user is instructor and not in studio.
"""
in_studio_preview = self.scope_ids.user_id is None
return self.is_instructor() and not in_studio_preview

def is_instructor(self) -> bool:
"""
Check if user role is instructor.
Returns:
bool: True if user role is instructor.
"""
return self.get_current_user().opt_attrs.get(ATTR_KEY_USER_ROLE) == "instructor"
return not in_studio_preview and self.is_course_team

@XBlock.json_handler
def studio_submit(self, data, _suffix="") -> None:
Expand Down Expand Up @@ -468,7 +464,7 @@ def get_instructor_grading_data(self, _, _suffix="") -> dict:
Returns:
dict: A dictionary containing student assignment information.
"""
require(self.is_instructor())
require(self.is_course_team)

def get_student_data() -> dict:
"""
Expand Down Expand Up @@ -554,7 +550,7 @@ def enter_grade(self, data, _suffix="") -> dict:
# Lazy import: import here to avoid app not ready errors
from submissions.api import set_score # pylint: disable=import-outside-toplevel

require(self.is_instructor())
require(self.is_course_team)

score = int(data.get("grade"))
uuid = data.get("submission_id")
Expand Down Expand Up @@ -588,7 +584,7 @@ def remove_grade(self, data, _suffix="") -> dict:
# Lazy import: import here to avoid app not ready errors
from submissions.api import reset_score # pylint: disable=import-outside-toplevel

require(self.is_instructor())
require(self.is_course_team)

student_id = data.get("student_id")
if not student_id:
Expand Down
40 changes: 26 additions & 14 deletions mindmap/tests/test_mindmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ def setUp(self) -> None:
self.mind_map = {"data": [{ "id": "root", "isroot": True, "topic": "Root" }]}
self.student = Mock(student_id="test-student-id", full_name="Test Student")
self.anonymous_user_id = "test-anonymous-user-id"
self.xblock.is_student = Mock()
self.xblock.is_course_staff = Mock()
self.xblock.get_current_user = Mock()
self.xblock.get_current_mind_map = Mock()
self.xblock.render_template = Mock(return_value="Test render")
self.xblock.resource_string = Mock()
Expand Down Expand Up @@ -64,7 +61,9 @@ def test_student_view_with_mind_map(self, initialize_js_mock: Mock):
- The student view is set up for the render with the student.
"""
self.xblock.is_static = False
self.xblock.is_student.return_value = True
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_role": "student",
}
self.xblock.get_current_user.return_value = self.student
self.xblock.get_current_mind_map.return_value = self.mind_map
expected_context = {
Expand Down Expand Up @@ -107,7 +106,9 @@ def test_student_view_empty_mind_map(self, initialize_js_mock: Mock):
- The student view is set up for the render with the student.
"""
self.xblock.is_static = False
self.xblock.is_student.return_value = True
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_role": "student",
}
self.xblock.get_current_mind_map.return_value = None
expected_context = {
"display_name": self.xblock.display_name,
Expand Down Expand Up @@ -148,7 +149,9 @@ def test_static_mind_map_in_student_view(self):
- The student view is set up for the render with the student.
"""
self.xblock.is_static = True
self.xblock.is_student.return_value = True
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_role": "student",
}
self.xblock.get_current_mind_map.return_value = self.mind_map
self.xblock.is_static = True
expected_context = {
Expand Down Expand Up @@ -180,8 +183,10 @@ def test_student_view_for_instructor(self):
- The student view is set up for the render with the instructor.
"""
self.xblock.is_static = False
self.xblock.is_student.return_value = False
self.xblock.is_course_staff.return_value = True
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_is_staff": True,
}
self.xblock.is_course_staff = True
self.xblock.get_current_mind_map.return_value = self.mind_map
self.xblock.show_staff_grading_interface.return_value = True
expected_context = {
Expand Down Expand Up @@ -213,8 +218,7 @@ def test_studio_view(self):
Expected result:
- The studio view is set up for the render.
"""
self.xblock.is_student.return_value = False
self.xblock.is_course_staff.return_value = False
self.xblock.get_current_user.return_value.opt_attrs = {}
self.xblock.fields = {
"display_name": "Test Mind Map",
"is_static": True,
Expand Down Expand Up @@ -264,7 +268,9 @@ def test_student_not_allowed_submission(self, initialize_js_mock: Mock):
"""
block_id = self.xblock.scope_ids.usage_id.block_id
self.xblock.is_static = False
self.xblock.is_student.return_value = True
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_role": "student",
}
self.xblock.get_current_mind_map.return_value = self.mind_map
self.xblock.submit_allowed.return_value = False
expected_context = {
Expand Down Expand Up @@ -419,7 +425,9 @@ def test_enter_grade(self, set_score_mock: Mock, get_student_module_mock: Mock):
"submission_id": self.submission_id
}
).encode("utf-8")
self.xblock.is_instructor = Mock(return_value=True)
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_is_staff": True,
}
get_student_module_mock.return_value = Mock(state='{"test-state": "mindmap"}')

response = self.xblock.enter_grade(self.request)
Expand All @@ -441,7 +449,9 @@ def test_remove_grade(self, reset_score_mock: Mock, get_student_module_mock: Moc
- The student view is rendered with the appropriate values.
"""
self.request.body = json.dumps({"student_id": self.student_id}).encode("utf-8")
self.xblock.is_instructor = Mock(return_value=True)
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_is_staff": True,
}
get_student_module_mock.return_value = Mock(state='{"test-state": "mindmap"}')

response = self.xblock.remove_grade(self.request)
Expand Down Expand Up @@ -484,7 +494,9 @@ def test_get_instructor_grading_data(
},
"created_at": current_datetime,
})
self.xblock.is_instructor = Mock(return_value=True)
self.xblock.get_current_user.return_value.opt_attrs = {
"edx-platform.user_is_staff": True,
}
user_by_anonymous_id_mock.return_value = Mock(username=self.student.student_id)
self.xblock.submission_status = "Submitted"
expected_result = {
Expand Down

0 comments on commit 41e4cbe

Please sign in to comment.