From 5689fbb455619c446f52dad6602843eb06c4b42d Mon Sep 17 00:00:00 2001
From: Jannat Patel <pateljannat2308@gmail.com>
Date: Thu, 29 Jan 2026 20:32:02 +0530
Subject: [PATCH] fix: api permissions

---
 frontend/src/components/Controls/Uploader.vue |   2 +-
 frontend/src/components/CourseCardOverlay.vue |   1 +
 lms/hooks.py                                  |   1 -
 lms/lms/api.py                                |  27 +---
 lms/lms/utils.py                              | 117 ++++++++++++++----
 lms/lms/widgets/CourseCard.html               |   2 +-
 6 files changed, 100 insertions(+), 50 deletions(-)

diff --git a/frontend/src/components/Controls/Uploader.vue b/frontend/src/components/Controls/Uploader.vue
index 7876f0a6..5bcd81a2 100644
--- a/frontend/src/components/Controls/Uploader.vue
+++ b/frontend/src/components/Controls/Uploader.vue
@@ -38,7 +38,7 @@
 						'border object-cover',
 						shape === 'circle'
 							? 'w-20 h-20 rounded-full'
-							: 'w-44 h-auto rounded-md',
+							: 'w-44 h-auto min-h-20 rounded-md',
 					]"
 				/>
 				<video v-else controls class="border rounded-md w-44 h-auto">
diff --git a/frontend/src/components/CourseCardOverlay.vue b/frontend/src/components/CourseCardOverlay.vue
index 1fdd5ef1..3353d97d 100644
--- a/frontend/src/components/CourseCardOverlay.vue
+++ b/frontend/src/components/CourseCardOverlay.vue
@@ -59,6 +59,7 @@
 					v-else-if="course.data.disable_self_learning && !isAdmin"
 					theme="blue"
 					size="lg"
+					class="mb-4"
 				>
 					{{ __('Contact the Administrator to enroll for this course') }}
 				</Badge>
diff --git a/lms/hooks.py b/lms/hooks.py
index 9c3a815b..8edb29e8 100644
--- a/lms/hooks.py
+++ b/lms/hooks.py
@@ -198,7 +198,6 @@ update_website_context = [
 
 jinja = {
 	"methods": [
-		"lms.lms.utils.get_tags",
 		"lms.lms.utils.get_lesson_count",
 		"lms.lms.utils.get_instructors",
 		"lms.lms.utils.get_lesson_index",
diff --git a/lms/lms/api.py b/lms/lms/api.py
index cd8413f0..39631838 100644
--- a/lms/lms/api.py
+++ b/lms/lms/api.py
@@ -31,6 +31,8 @@ from pypika import functions as fn
 
 from lms.lms.doctype.course_lesson.course_lesson import save_progress
 from lms.lms.utils import (
+	can_modify_batch,
+	can_modify_course,
 	get_average_rating,
 	get_batch_details,
 	get_course_details,
@@ -39,7 +41,6 @@ from lms.lms.utils import (
 	has_course_instructor_role,
 	has_evaluator_role,
 	has_moderator_role,
-	has_student_role,
 )
 
 
@@ -830,16 +831,6 @@ def get_announcements(batch):
 	return communications
 
 
-def can_modify_course(course):
-	is_instructor = frappe.db.exists(
-		"Course Instructor",
-		{"instructor": frappe.session.user, "parent": course, "parenttype": "LMS Course"},
-	)
-	if not (has_moderator_role() or is_instructor):
-		return False
-	return True
-
-
 @frappe.whitelist()
 def delete_course(course):
 	if not can_modify_course(course):
@@ -877,20 +868,6 @@ def delete_course(course):
 	frappe.delete_doc("LMS Course", course)
 
 
-def can_modify_batch(batch):
-	is_instructor = frappe.db.exists(
-		"Course Instructor",
-		{
-			"instructor": frappe.session.user,
-			"parent": batch,
-			"parenttype": "LMS Batch",
-		},
-	)
-	if not (has_moderator_role() or is_instructor):
-		return False
-	return True
-
-
 @frappe.whitelist()
 def delete_batch(batch):
 	if not can_modify_batch(batch):
diff --git a/lms/lms/utils.py b/lms/lms/utils.py
index 154b73fd..c768bcbd 100644
--- a/lms/lms/utils.py
+++ b/lms/lms/utils.py
@@ -202,13 +202,6 @@ def get_lesson_icon(body, content):
 	return "icon-list"
 
 
-@frappe.whitelist(allow_guest=True)
-@rate_limit(limit=500, seconds=60 * 60)
-def get_tags(course):
-	tags = frappe.db.get_value("LMS Course", course, "tags")
-	return tags.split(",") if tags else []
-
-
 def get_instructors(doctype, docname):
 	instructor_details = []
 	instructors = frappe.get_all(
@@ -254,7 +247,7 @@ def get_reviews(course):
 	for review in reviews:
 		review.rating = review.rating * out_of_ratings
 		review.owner_details = frappe.db.get_value(
-			"User", review.owner, ["name", "username", "full_name", "user_image"], as_dict=True
+			"User", review.owner, ["username", "full_name", "user_image"], as_dict=True
 		)
 		review.creation = pretty_date(review.creation)
 
@@ -654,9 +647,6 @@ def get_evaluator(course, batch=None):
 
 @frappe.whitelist()
 def get_upcoming_evals(courses=None, batch=None):
-	if frappe.session.user == "Guest":
-		return []
-
 	if not courses:
 		courses = []
 
@@ -759,11 +749,21 @@ def get_current_exchange_rate(source, target="USD"):
 	return details["rates"][target]
 
 
+def guest_access_allowed():
+	allow_guest_access = frappe.get_cached_value("LMS Settings", None, "allow_guest_access")
+	if frappe.session.user == "Guest" and not allow_guest_access:
+		return False
+	return True
+
+
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_courses(filters=None, start=0):
 	"""Returns the list of courses."""
 
+	if not guest_access_allowed():
+		return []
+
 	if not filters:
 		filters = {}
 
@@ -905,6 +905,9 @@ def get_course_fields():
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_course_details(course):
+	if not guest_access_allowed():
+		return {}
+
 	fields = get_course_fields()
 	course_details = frappe.db.get_value(
 		"LMS Course",
@@ -976,6 +979,10 @@ def get_categorized_courses(courses):
 @frappe.whitelist(allow_guest=True)
 def get_course_outline(course, progress=False):
 	"""Returns the course outline."""
+
+	if not guest_access_allowed():
+		return []
+
 	outline = []
 	chapters = frappe.get_all("Chapter Reference", {"parent": course}, ["chapter", "idx"], order_by="idx")
 	for chapter in chapters:
@@ -1003,6 +1010,9 @@ def get_course_outline(course, progress=False):
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_lesson(course, chapter, lesson):
+	if not guest_access_allowed():
+		return {}
+
 	chapter_name = frappe.db.get_value("Chapter Reference", {"parent": course, "idx": chapter}, "chapter")
 	lesson_name = frappe.db.get_value("Lesson Reference", {"parent": chapter_name, "idx": lesson}, "lesson")
 	lesson_details = frappe.db.get_value(
@@ -1114,8 +1124,10 @@ def get_neighbour_lesson(course, chapter, lesson):
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_batch_details(batch):
-	batch_students = frappe.get_all("LMS Batch Enrollment", {"batch": batch}, pluck="member")
+	if not guest_access_allowed():
+		return {}
 
+	batch_students = frappe.get_all("LMS Batch Enrollment", {"batch": batch}, pluck="member")
 	has_create_batch_role = can_create_batches()
 	is_course_published = frappe.db.get_value("LMS Batch", batch, "published")
 	is_student_enrolled = frappe.session.user in batch_students
@@ -1179,8 +1191,6 @@ def get_batch_details(batch):
 	if batch_details.seat_count:
 		batch_details.seats_left = batch_details.seat_count - len(batch_students)
 
-	print(batch_details.seat_count, len(batch_students))
-
 	return batch_details
 
 
@@ -1241,6 +1251,9 @@ def get_question_details(question):
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_batch_courses(batch):
+	if not guest_access_allowed():
+		return []
+
 	courses = []
 	course_list = frappe.get_all("Batch Course", {"parent": batch}, ["name", "course"])
 
@@ -1253,10 +1266,8 @@ def get_batch_courses(batch):
 
 
 @frappe.whitelist()
-def get_assessments(batch, member=None):
-	if not member:
-		member = frappe.session.user
-
+def get_assessments(batch):
+	member = frappe.session.user
 	assessments = frappe.get_all(
 		"LMS Assessment",
 		{"parent": batch},
@@ -1364,6 +1375,7 @@ def get_exercise_details(assessment, member):
 
 @frappe.whitelist()
 def get_batch_assessment_count(batch):
+	frappe.only_for(["Moderator", "Batch Evaluator"])
 	if not frappe.db.exists("LMS Batch", batch):
 		frappe.throw(_("The specified batch does not exist."))
 	return frappe.db.count("LMS Assessment", {"parent": batch})
@@ -1374,11 +1386,13 @@ def get_batch_students(filters, offset=0, limit_start=0, limit_page_length=None,
 	# limit_start and limit_page_length are used for backward compatibility
 	start = limit_start or offset
 	page_length = limit_page_length or limit
-
 	batch = filters.get("batch")
 	if not batch:
 		return []
 
+	if not can_modify_batch(batch):
+		frappe.throw(_("You are not authorized to view the students of this batch."))
+
 	students = []
 	students_list = frappe.get_all(
 		"LMS Batch Enrollment",
@@ -1480,6 +1494,8 @@ def get_quiz_pass_stats(batch):
 @frappe.whitelist()
 def get_batch_chart_data(batch):
 	"""Get completion counts per course and assessment"""
+	if not can_modify_batch(batch):
+		frappe.throw(_("You are not authorized to view the chart data of this batch."))
 	if not frappe.db.exists("LMS Batch", batch):
 		frappe.throw(_("The specified batch does not exist."))
 
@@ -1616,8 +1632,27 @@ def has_submitted_assessment(assessment, assessment_type, member=None):
 		)
 
 
+def can_access_topic(doctype, docname):
+	is_student = False
+	if doctype == "Course Lesson":
+		course = frappe.db.get_value("Course Lesson", docname, "course")
+		is_student = frappe.db.exists("LMS Enrollment", {"course": course, "member": frappe.session.user})
+		if not is_student and not can_modify_course(course):
+			return False
+	elif doctype == "LMS Batch":
+		is_student = frappe.db.exists(
+			"LMS Batch Enrollment", {"batch": docname, "member": frappe.session.user}
+		)
+		if not is_student and not can_modify_batch(docname):
+			return False
+	return True
+
+
 @frappe.whitelist()
 def get_discussion_topics(doctype, docname, single_thread):
+	if not can_access_topic(doctype, docname):
+		frappe.throw(_("You are not authorized to view the discussion topics for this item."))
+
 	if single_thread:
 		filters = {
 			"reference_doctype": doctype,
@@ -1659,6 +1694,10 @@ def create_discussion_topic(doctype, docname):
 
 @frappe.whitelist()
 def get_discussion_replies(topic):
+	doctype = frappe.db.get_value("Discussion Topic", topic, "reference_doctype")
+	if not can_access_topic(doctype, topic):
+		frappe.throw(_("You are not authorized to view the discussion replies for this topic."))
+
 	replies = frappe.get_all(
 		"Discussion Reply",
 		{
@@ -1811,6 +1850,7 @@ def calculate_discount_amount(base_amount, coupon):
 
 @frappe.whitelist()
 def get_lesson_creation_details(course, chapter, lesson):
+	frappe.only_for(["Moderator", "Course Creator"])
 	chapter_name = frappe.db.get_value("Chapter Reference", {"parent": course, "idx": chapter}, "chapter")
 	lesson_name = frappe.db.get_value("Lesson Reference", {"parent": chapter_name, "idx": lesson}, "lesson")
 
@@ -1995,6 +2035,9 @@ def update_certificate_purchase(course, payment_name):
 
 @frappe.whitelist()
 def get_programs():
+	if not guest_access_allowed():
+		frappe.throw(_("Please login to view programs."))
+
 	enrolled_programs = frappe.get_all(
 		"LMS Program Member", {"member": frappe.session.user}, ["parent as name", "progress"]
 	)
@@ -2027,6 +2070,9 @@ def get_programs():
 
 @frappe.whitelist()
 def get_program_details(program_name):
+	if not guest_access_allowed():
+		frappe.throw(_("Please login to view program details."))
+
 	program = frappe.db.get_value(
 		"LMS Program",
 		program_name,
@@ -2084,9 +2130,6 @@ def enroll_in_program(program):
 
 
 def validate_program_enrollment(program):
-	if frappe.session.user == "Guest":
-		frappe.throw(_("Please login to enroll in the program."))
-
 	published = frappe.db.get_value("LMS Program", program, "published")
 	if not published:
 		frappe.throw(_("You cannot enroll in an unpublished program."))
@@ -2095,6 +2138,9 @@ def validate_program_enrollment(program):
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_batches(filters=None, start=0, order_by="start_date"):
+	if not guest_access_allowed():
+		return []
+
 	if not filters:
 		filters = {}
 
@@ -2209,6 +2255,9 @@ def get_palette(full_name):
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=500, seconds=60 * 60)
 def get_related_courses(course):
+	if not guest_access_allowed():
+		return []
+
 	related_course_details = []
 	related_courses = frappe.get_all("Related Courses", {"parent": course}, order_by="idx", pluck="course")
 
@@ -2264,3 +2313,27 @@ def validate_batch_access(batch):
 	)
 	if not enrollment_exists:
 		frappe.throw(_("You do not have access to this batch."))
+
+
+def can_modify_course(course):
+	is_instructor = frappe.db.exists(
+		"Course Instructor",
+		{"instructor": frappe.session.user, "parent": course, "parenttype": "LMS Course"},
+	)
+	if not (has_moderator_role() or is_instructor):
+		return False
+	return True
+
+
+def can_modify_batch(batch):
+	is_instructor = frappe.db.exists(
+		"Course Instructor",
+		{
+			"instructor": frappe.session.user,
+			"parent": batch,
+			"parenttype": "LMS Batch",
+		},
+	)
+	if not (has_moderator_role() or is_instructor):
+		return False
+	return True
diff --git a/lms/lms/widgets/CourseCard.html b/lms/lms/widgets/CourseCard.html
index de74f2c0..083ebcf2 100644
--- a/lms/lms/widgets/CourseCard.html
+++ b/lms/lms/widgets/CourseCard.html
@@ -14,7 +14,7 @@
     <div class="course-image {% if not course.image %} default-image {% endif %}"
     {% if course.image %} style="background-image: url( {{ course.image | urlencode }} );" {% endif %}>
         <div class="course-tags">
-        {% for tag in get_tags(course.name) %}
+        {% for tag in frappe.db.get_value("LMS Course", course.name, "tags").split(",") %}
             <div class="course-card-pills">{{ tag }}</div>
         {% endfor %}
         </div>