From c596d1e21555656fe2be5c4e8065446f71fb1b23 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Sat, 21 Feb 2026 12:25:47 +0530 Subject: [PATCH] fix: misc permission issues --- frontend/src/components/CourseReviews.vue | 8 +-- lms/hooks.py | 17 +++--- lms/lms/api.py | 7 ++- lms/lms/doctype/lms_badge/lms_badge.json | 4 +- .../lms_badge_assignment.py | 56 ++++++++++--------- lms/lms/doctype/lms_batch/lms_batch.py | 25 +++++++++ .../lms_certificate/lms_certificate.json | 32 ++++------- .../lms_certificate/lms_certificate.py | 54 ++++++++++++++++++ .../lms_course_review/lms_course_review.json | 4 +- .../doctype/lms_live_class/lms_live_class.py | 12 ++++ lms/lms/doctype/lms_program/lms_program.py | 23 ++++++++ .../lms_programming_exercise_submission.json | 3 +- 12 files changed, 177 insertions(+), 68 deletions(-) diff --git a/frontend/src/components/CourseReviews.vue b/frontend/src/components/CourseReviews.vue index 04a39bf2..4ebb5c5a 100644 --- a/frontend/src/components/CourseReviews.vue +++ b/frontend/src/components/CourseReviews.vue @@ -12,7 +12,7 @@
-
+
+
+ {{ review.review }} +
-
- {{ review.review }} -
diff --git a/lms/hooks.py b/lms/hooks.py index 71d47d8d..9f99e09d 100644 --- a/lms/hooks.py +++ b/lms/hooks.py @@ -86,13 +86,16 @@ after_migrate = [ # ----------- # Permissions evaluated in scripted ways -# permission_query_conditions = { -# "Event": "frappe.desk.doctype.event.event.get_permission_query_conditions", -# } -# -# has_permission = { -# "Event": "frappe.desk.doctype.event.event.has_permission", -# } +permission_query_conditions = { + "LMS Certificate": "lms.lms.doctype.lms_certificate.lms_certificate.get_permission_query_conditions", +} + +has_permission = { + "LMS Live Class": "lms.lms.doctype.lms_live_class.lms_live_class.has_permission", + "LMS Batch": "lms.lms.doctype.lms_batch.lms_batch.has_permission", + "LMS Program": "lms.lms.doctype.lms_program.lms_program.has_permission", + "LMS Certificate": "lms.lms.doctype.lms_certificate.lms_certificate.has_permission", +} # DocType Class # --------------- diff --git a/lms/lms/api.py b/lms/lms/api.py index d60f6103..ba7255ea 100644 --- a/lms/lms/api.py +++ b/lms/lms/api.py @@ -1722,7 +1722,7 @@ def get_profile_details(username: str): as_dict=True, ) roles = frappe.get_roles(details.name) - if not has_lms_role(roles): + if not has_lms_role(): frappe.throw( _("User does not have permission to access this users profile details."), frappe.PermissionError ) @@ -1730,7 +1730,8 @@ def get_profile_details(username: str): return details -def has_lms_role(roles: list): +def has_lms_role(): + roles = frappe.get_roles() lms_roles = set(LMS_ROLES) user_roles = set(roles) return not lms_roles.isdisjoint(user_roles) @@ -2223,7 +2224,7 @@ def get_assessment_from_lesson(course: str, assessmentType: str): @frappe.whitelist() def get_badges(member: str): - if not has_lms_role(frappe.get_roles()): + if not has_lms_role(): frappe.throw(_("You do not have permission to access badges."), frappe.PermissionError) badges = frappe.get_all( diff --git a/lms/lms/doctype/lms_badge/lms_badge.json b/lms/lms/doctype/lms_badge/lms_badge.json index 5f4cda8f..0463f3e2 100644 --- a/lms/lms/doctype/lms_badge/lms_badge.json +++ b/lms/lms/doctype/lms_badge/lms_badge.json @@ -59,7 +59,7 @@ "fieldname": "condition", "fieldtype": "Code", "label": "Condition", - "mandatory_depends_on": "eval:doc.event == \"Manual Assignment\"" + "reqd": 1 }, { "depends_on": "eval:doc.event == 'Value Change'", @@ -100,7 +100,7 @@ "link_fieldname": "badge" } ], - "modified": "2026-02-19 15:05:49.719925", + "modified": "2026-02-20 17:58:25.924109", "modified_by": "sayali@frappe.io", "module": "LMS", "name": "LMS Badge", diff --git a/lms/lms/doctype/lms_badge_assignment/lms_badge_assignment.py b/lms/lms/doctype/lms_badge_assignment/lms_badge_assignment.py index 4c6f9965..182ca35b 100644 --- a/lms/lms/doctype/lms_badge_assignment/lms_badge_assignment.py +++ b/lms/lms/doctype/lms_badge_assignment/lms_badge_assignment.py @@ -10,17 +10,17 @@ from lms.lms.doctype.lms_badge.lms_badge import eval_condition class LMSBadgeAssignment(Document): def validate(self): - self.validate_owner() self.validate_duplicate_badge_assignment() self.validate_badge_criteria() + self.validate_owner() def validate_owner(self): - if self.owner == self.member: - return - - roles = frappe.get_roles(self.owner) - if "Moderator" not in roles: - frappe.throw(_("You must be a Moderator to assign badges to users.")) + event = frappe.db.get_value("LMS Badge", self.badge, "event") + if event == "Manual Assignment": + roles = frappe.get_roles(frappe.session.user) + admins = ["Moderator", "Course Creator", "Batch Evaluator"] + if not any(role in roles for role in admins): + frappe.throw(_("You must be an Admin to assign badges to users.")) def validate_duplicate_badge_assignment(self): grant_only_once = frappe.db.get_value("LMS Badge", self.badge, "grant_only_once") @@ -40,25 +40,27 @@ class LMSBadgeAssignment(Document): "LMS Badge", self.badge, ["reference_doctype", "user_field", "condition", "enabled"], as_dict=True ) - if badge_details: - if badge_details.reference_doctype and badge_details.user_field and badge_details.condition: - user_fieldname = frappe.db.get_value( - "DocField", - {"parent": badge_details.reference_doctype, "fieldname": badge_details.user_field}, - "fieldname", + if not badge_details: + return + + if badge_details.reference_doctype and badge_details.user_field and badge_details.condition: + user_fieldname = frappe.db.get_value( + "DocField", + {"parent": badge_details.reference_doctype, "fieldname": badge_details.user_field}, + "fieldname", + ) + + documents = frappe.get_all( + badge_details.reference_doctype, + {user_fieldname: self.member}, + ) + + for document in documents: + reference_value = eval_condition( + frappe.get_doc(badge_details.reference_doctype, document.name), + badge_details.condition, ) + if reference_value: + return - documents = frappe.get_all( - badge_details.reference_doctype, - {user_fieldname: self.member}, - ) - - for document in documents: - reference_value = eval_condition( - frappe.get_doc(badge_details.reference_doctype, document.name), - badge_details.condition, - ) - if reference_value: - return - - frappe.throw(_("Member does not meet the criteria for the badge {0}.").format(self.badge)) + frappe.throw(_("Member does not meet the criteria for the badge {0}.").format(self.badge)) diff --git a/lms/lms/doctype/lms_batch/lms_batch.py b/lms/lms/doctype/lms_batch/lms_batch.py index 70c79216..6f535cd7 100644 --- a/lms/lms/doctype/lms_batch/lms_batch.py +++ b/lms/lms/doctype/lms_batch/lms_batch.py @@ -20,6 +20,7 @@ from lms.lms.utils import ( get_lesson_url, get_lms_route, get_quiz_details, + guest_access_allowed, update_payment_record, ) @@ -213,6 +214,10 @@ def create_live_class( auto_recording: str, description: str = None, ): + roles = frappe.get_roles() + if not any(role in roles for role in ["Moderator", "Batch Evaluator"]): + frappe.throw(_("You do not have permission to create a live class.")) + payload = { "topic": title, "start_time": format_datetime(f"{date} {time}", "yyyy-MM-ddTHH:mm:ssZ"), @@ -391,3 +396,23 @@ def send_mail(batch, student): args=args, header=[_(f"Batch Start Reminder: {batch.title}"), "orange"], ) + + +def has_permission(doc, ptype="read", user=None): + user = user or frappe.session.user + if user == "Guest" and not guest_access_allowed(): + return False + + roles = frappe.get_roles(user) + if "Moderator" in roles or "Batch Evaluator" in roles: + return True + + is_enrolled = frappe.db.exists("LMS Batch Enrollment", {"batch": doc.name, "member": user}) + if is_enrolled: + return True + + is_batch_published = frappe.db.get_value("LMS Batch", doc.name, "published") + if is_batch_published: + return True + + return False diff --git a/lms/lms/doctype/lms_certificate/lms_certificate.json b/lms/lms/doctype/lms_certificate/lms_certificate.json index 3e3293d3..06f107fc 100644 --- a/lms/lms/doctype/lms_certificate/lms_certificate.json +++ b/lms/lms/doctype/lms_certificate/lms_certificate.json @@ -123,7 +123,7 @@ "grid_page_length": 50, "index_web_pages_for_search": 1, "links": [], - "modified": "2025-12-17 16:50:31.128747", + "modified": "2026-02-20 17:32:34.580862", "modified_by": "sayali@frappe.io", "module": "LMS", "name": "LMS Certificate", @@ -153,27 +153,6 @@ "share": 1, "write": 1 }, - { - "create": 1, - "email": 1, - "export": 1, - "if_owner": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "LMS Student", - "share": 1, - "write": 1 - }, - { - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "LMS Student", - "share": 1 - }, { "create": 1, "delete": 1, @@ -197,6 +176,15 @@ "role": "Course Creator", "share": 1, "write": 1 + }, + { + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "LMS Student", + "share": 1 } ], "row_format": "Dynamic", diff --git a/lms/lms/doctype/lms_certificate/lms_certificate.py b/lms/lms/doctype/lms_certificate/lms_certificate.py index ca35d11a..cfc93af8 100644 --- a/lms/lms/doctype/lms_certificate/lms_certificate.py +++ b/lms/lms/doctype/lms_certificate/lms_certificate.py @@ -12,6 +12,7 @@ from frappe.utils.telemetry import capture class LMSCertificate(Document): def validate(self): + self.validate_criteria() self.validate_duplicate_certificate() def autoname(self): @@ -54,6 +55,43 @@ class LMSCertificate(Document): header=[subject, "green"], ) + def validate_criteria(self): + self.validate_role_of_owner() + self.validate_batch_enrollment() + self.validate_course_enrollment() + + def validate_role_of_owner(self): + roles = frappe.get_roles() + is_admin = any(role in roles for role in ["Moderator", "Course Creator", "Batch Evaluator"]) + if not self.course and not self.batch_name and not is_admin: + frappe.throw(_("Course or Batch is required to issue a certificate.")) + + def validate_batch_enrollment(self): + if self.batch_name: + is_enrolled = frappe.db.exists( + "LMS Batch Enrollment", {"batch": self.batch_name, "member": self.member} + ) + if not is_enrolled: + frappe.throw(_("Certification cannot be issued as the member is not enrolled in this batch.")) + + def validate_course_enrollment(self): + if self.course: + is_enrolled = frappe.db.exists("LMS Enrollment", {"course": self.course, "member": self.member}) + if not is_enrolled: + frappe.throw( + _("Certification cannot be issued as the member is not enrolled in this course.") + ) + + completion_certificate = frappe.db.get_value("LMS Course", self.course, "enable_certification") + if completion_certificate: + progress = frappe.db.get_value( + "LMS Enrollment", {"course": self.course, "member": self.member}, "progress" + ) + if progress < 100: + frappe.throw( + _("Certification cannot be issued as the member has not completed the course.") + ) + def validate_duplicate_certificate(self): self.validate_course_duplicates() self.validate_batch_duplicates() @@ -177,3 +215,19 @@ def validate_certification_eligibility(course): ) if progress < 100: frappe.throw(_("You have not completed the course yet.")) + + +def has_permission(doc, ptype="read", user=None): + user = user or frappe.session.user + roles = frappe.get_roles(user) + if "Moderator" in roles or "Course Creator" in roles or "Batch Evaluator" in roles: + return True + return doc.published + + +def get_permission_query_conditions(user): + user = user or frappe.session.user + roles = frappe.get_roles(user) + if "Moderator" in roles or "Course Creator" in roles or "Batch Evaluator" in roles: + return None + return """(`tabLMS Certificate`.published = 1)""" diff --git a/lms/lms/doctype/lms_course_review/lms_course_review.json b/lms/lms/doctype/lms_course_review/lms_course_review.json index 4ac6be0c..64f0ac27 100644 --- a/lms/lms/doctype/lms_course_review/lms_course_review.json +++ b/lms/lms/doctype/lms_course_review/lms_course_review.json @@ -41,7 +41,7 @@ "grid_page_length": 50, "index_web_pages_for_search": 1, "links": [], - "modified": "2026-01-29 16:10:47.787285", + "modified": "2026-02-20 17:40:39.823017", "modified_by": "sayali@frappe.io", "module": "LMS", "name": "LMS Course Review", @@ -63,8 +63,8 @@ "create": 1, "email": 1, "export": 1, + "if_owner": 1, "print": 1, - "read": 1, "report": 1, "role": "LMS Student", "share": 1, diff --git a/lms/lms/doctype/lms_live_class/lms_live_class.py b/lms/lms/doctype/lms_live_class/lms_live_class.py index dc6dfa2f..9f9ff2cd 100644 --- a/lms/lms/doctype/lms_live_class/lms_live_class.py +++ b/lms/lms/doctype/lms_live_class/lms_live_class.py @@ -169,3 +169,15 @@ def get_minutes(duration_in_seconds): if duration_in_seconds: return int(duration_in_seconds) // 60 return 0 + + +def has_permission(doc, ptype="read", user=None): + user = user or frappe.session.user + roles = frappe.get_roles(user) + if "Moderator" in roles or "Batch Evaluator" in roles: + return True + + return frappe.db.exists( + "LMS Batch Enrollment", + {"batch": doc.batch_name, "member": user}, + ) diff --git a/lms/lms/doctype/lms_program/lms_program.py b/lms/lms/doctype/lms_program/lms_program.py index 9bcefd22..5d09c328 100644 --- a/lms/lms/doctype/lms_program/lms_program.py +++ b/lms/lms/doctype/lms_program/lms_program.py @@ -5,6 +5,8 @@ import frappe from frappe import _ from frappe.model.document import Document +from lms.lms.utils import guest_access_allowed + class LMSProgram(Document): def validate(self): @@ -41,3 +43,24 @@ class LMSProgram(Document): if self.member_count != member_count: self.member_count = member_count + + +def has_permission(doc, ptype="read", user=None): + user = user or frappe.session.user + + if user == "Guest" and not guest_access_allowed(): + return False + + roles = frappe.get_roles(user) + if "Moderator" in roles or "Course Creator" in roles: + return True + + is_enrolled = frappe.db.exists("LMS Program Member", {"parent": doc.name, "member": user}) + if is_enrolled: + return True + + is_program_published = frappe.db.get_value("LMS Program", doc.name, "published") + if is_program_published: + return True + + return False diff --git a/lms/lms/doctype/lms_programming_exercise_submission/lms_programming_exercise_submission.json b/lms/lms/doctype/lms_programming_exercise_submission/lms_programming_exercise_submission.json index 3b614b13..9e3532ef 100644 --- a/lms/lms/doctype/lms_programming_exercise_submission/lms_programming_exercise_submission.json +++ b/lms/lms/doctype/lms_programming_exercise_submission/lms_programming_exercise_submission.json @@ -88,7 +88,7 @@ "grid_page_length": 50, "index_web_pages_for_search": 1, "links": [], - "modified": "2025-06-24 14:42:08.288983", + "modified": "2026-02-20 14:43:56.587110", "modified_by": "sayali@frappe.io", "module": "LMS", "name": "LMS Programming Exercise Submission", @@ -146,6 +146,7 @@ "create": 1, "email": 1, "export": 1, + "if_owner": 1, "print": 1, "read": 1, "report": 1,