Enhance logging and error handling in collection access methods and WSGI configuration
This commit is contained in:
+4
-1
@@ -1256,6 +1256,7 @@ class Series(SeriesBase):
|
|||||||
|
|
||||||
|
|
||||||
class CaseCollection(ExamOrCollectionGenericBase):
|
class CaseCollection(ExamOrCollectionGenericBase):
|
||||||
|
|
||||||
app_name = "atlas"
|
app_name = "atlas"
|
||||||
|
|
||||||
cases = models.ManyToManyField(Case, through="CaseDetail")
|
cases = models.ManyToManyField(Case, through="CaseDetail")
|
||||||
@@ -1518,7 +1519,9 @@ class CaseCollection(ExamOrCollectionGenericBase):
|
|||||||
Extend base check_user_can_take to also require completion of any
|
Extend base check_user_can_take to also require completion of any
|
||||||
prerequisite collections.
|
prerequisite collections.
|
||||||
"""
|
"""
|
||||||
# Perform the normal access checks first
|
logger.error("Checking if user can take collection with prerequisites")
|
||||||
|
logger.error(f"CID: {cid}, user: {user}, active_only: {active_only}")
|
||||||
|
#` Perform the normal access checks first
|
||||||
super().check_user_can_take(cid, passcode, user=user, active_only=active_only)
|
super().check_user_can_take(cid, passcode, user=user, active_only=active_only)
|
||||||
|
|
||||||
# If there are prerequisites, the user (or CID) must have completed them
|
# If there are prerequisites, the user (or CID) must have completed them
|
||||||
|
|||||||
@@ -4002,6 +4002,8 @@ def collection_take_start(request, pk, cid=None, passcode=None):
|
|||||||
valid_user = True
|
valid_user = True
|
||||||
except Http404:
|
except Http404:
|
||||||
valid_user = False
|
valid_user = False
|
||||||
|
except collection.InactiveException as e:
|
||||||
|
return HttpResponse(f"<h1>{str(e)}</h1>")
|
||||||
except PrerequisiteRequired as e:
|
except PrerequisiteRequired as e:
|
||||||
# Show a friendly page linking to the required collection rather than 404
|
# Show a friendly page linking to the required collection rather than 404
|
||||||
prereq = getattr(e, 'prereq', None)
|
prereq = getattr(e, 'prereq', None)
|
||||||
@@ -4034,6 +4036,8 @@ def collection_take_start(request, pk, cid=None, passcode=None):
|
|||||||
return render(
|
return render(
|
||||||
request, "atlas/collection_review_start.html", template_variables
|
request, "atlas/collection_review_start.html", template_variables
|
||||||
)
|
)
|
||||||
|
case _:
|
||||||
|
return HttpResponse(f"Unknown collection type {collection.collection_type}")
|
||||||
|
|
||||||
|
|
||||||
def _case_number_for_collection_and_caseid(collection_pk, case_id):
|
def _case_number_for_collection_and_caseid(collection_pk, case_id):
|
||||||
|
|||||||
+19
-5
@@ -654,6 +654,8 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin):
|
|||||||
e.g. user management
|
e.g. user management
|
||||||
|
|
||||||
"""
|
"""
|
||||||
|
class InactiveException(Exception):
|
||||||
|
pass
|
||||||
|
|
||||||
# Is this actually used? It is but should be replaced on the inherited class
|
# Is this actually used? It is but should be replaced on the inherited class
|
||||||
cid_users = GenericRelation("generic.CidUserExam")
|
cid_users = GenericRelation("generic.CidUserExam")
|
||||||
@@ -768,7 +770,9 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin):
|
|||||||
Raises:
|
Raises:
|
||||||
Http404: If user does not have access
|
Http404: If user does not have access
|
||||||
"""
|
"""
|
||||||
|
logger.debug(f"Checking user can take: {cid=}, passcode=****, user={user}, active_only={active_only}")
|
||||||
if user is not None and self.is_author(user):
|
if user is not None and self.is_author(user):
|
||||||
|
logger.debug("User is author, allowing access")
|
||||||
return
|
return
|
||||||
|
|
||||||
# If we are limiting by dates check that the current date is within the range
|
# If we are limiting by dates check that the current date is within the range
|
||||||
@@ -778,24 +782,32 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin):
|
|||||||
):
|
):
|
||||||
# Raise with an explanatory message so the frontend can
|
# Raise with an explanatory message so the frontend can
|
||||||
# surface the reason to the end user.
|
# surface the reason to the end user.
|
||||||
|
logger.debug("Exam is outside of allowed dates, denying access")
|
||||||
raise Http404("Exam not currently available (outside allowed dates)")
|
raise Http404("Exam not currently available (outside allowed dates)")
|
||||||
# If it is not active no one can take
|
# If it is not active no one can take
|
||||||
elif active_only and not self.active:
|
elif active_only and not self.active:
|
||||||
raise Http404("Exam currently inactive")
|
logger.debug("Exam is not active, denying access")
|
||||||
|
raise self.InactiveException("Exam currently inactive")
|
||||||
|
|
||||||
# If candidates only check they have access
|
# If candidates only check they have access
|
||||||
if self.candidates_only:
|
if self.candidates_only:
|
||||||
|
logger.debug("Exam is candidates only, checking CID/user access")
|
||||||
if not self.check_cid_user(cid, passcode, user):
|
if not self.check_cid_user(cid, passcode, user):
|
||||||
|
logger.debug("CID/user access check failed, denying access")
|
||||||
raise Http404("Error accessing exam")
|
raise Http404("Error accessing exam")
|
||||||
return
|
return
|
||||||
|
|
||||||
# Otherwise check that they are a valid user.
|
# Otherwise check that they are a valid user.
|
||||||
if user is None:
|
if user is None:
|
||||||
|
logger.debug("No user provided, denying access")
|
||||||
raise Http404("Error accessing exam")
|
raise Http404("Error accessing exam")
|
||||||
|
|
||||||
if not user.is_active:
|
if not user.is_active:
|
||||||
|
logger.debug("User is not active, denying access")
|
||||||
raise Http404("Error accessing exam")
|
raise Http404("Error accessing exam")
|
||||||
|
|
||||||
|
logger.debug("User passed all checks, allowing access")
|
||||||
|
|
||||||
return
|
return
|
||||||
|
|
||||||
def check_cid_user(
|
def check_cid_user(
|
||||||
@@ -808,7 +820,7 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin):
|
|||||||
allow_authors: bool = True,
|
allow_authors: bool = True,
|
||||||
):
|
):
|
||||||
"""Checks if a user (cid or otherwise) is allowed to access (and take) the exam"""
|
"""Checks if a user (cid or otherwise) is allowed to access (and take) the exam"""
|
||||||
print(f"Check cid user: {cid=}, {passcode=}, exam_id={self.pk}, {user=}, {user_id=}, {allow_authors=}")
|
logger.debug(f"Check cid user: {cid=}, {passcode=}, exam_id={self.pk}, {user=}, {user_id=}, {allow_authors=}")
|
||||||
if user is not None and user.is_superuser:
|
if user is not None and user.is_superuser:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
@@ -829,14 +841,16 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin):
|
|||||||
return True
|
return True
|
||||||
|
|
||||||
if not self.valid_cid_users.exists() and not self.valid_user_users.exists():
|
if not self.valid_cid_users.exists() and not self.valid_user_users.exists():
|
||||||
|
logger.debug("No valid CID or user users found")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
# Start by checking if the logged in user can access
|
# Start by checking if the logged in user can access
|
||||||
|
logger.debug(f"Checking user access for user_id={user_id}")
|
||||||
if user_id is not None:
|
if user_id is not None:
|
||||||
if self.valid_user_users.filter(pk=user_id).exists():
|
if self.valid_user_users.filter(pk=user_id).exists():
|
||||||
return True
|
return True
|
||||||
|
else:
|
||||||
|
logger.debug("No valid user users found for user_id={user_id}")
|
||||||
|
|
||||||
# Then test CID data
|
# Then test CID data
|
||||||
if self.valid_cid_users.exists():
|
if self.valid_cid_users.exists():
|
||||||
@@ -2284,4 +2298,4 @@ class FindingBase(models.Model):
|
|||||||
return get_pretty_json(json.loads(self.annotation_json))
|
return get_pretty_json(json.loads(self.annotation_json))
|
||||||
|
|
||||||
def get_pretty_viewport_json(self):
|
def get_pretty_viewport_json(self):
|
||||||
return get_pretty_json(json.loads(self.viewport_json))
|
return get_pretty_json(json.loads(self.viewport_json))
|
||||||
|
|||||||
+11
-2
@@ -22,9 +22,18 @@ try:
|
|||||||
# ensure directory exists
|
# ensure directory exists
|
||||||
log_dir = Path("/var/log/rad")
|
log_dir = Path("/var/log/rad")
|
||||||
log_dir.mkdir(parents=True, exist_ok=True)
|
log_dir.mkdir(parents=True, exist_ok=True)
|
||||||
|
# Determine stdout level: allow overriding with LOGURU_STDOUT_LEVEL env var,
|
||||||
|
# otherwise expose DEBUG when DEBUG env var is truthy, fall back to INFO.
|
||||||
|
raw_debug = os.environ.get("DEBUG", "0")
|
||||||
|
env_level = os.environ.get("LOGURU_STDOUT_LEVEL")
|
||||||
|
if env_level:
|
||||||
|
stdout_level = env_level
|
||||||
|
else:
|
||||||
|
stdout_level = "DEBUG" if raw_debug.lower() in ("1", "true", "yes") else "INFO"
|
||||||
|
|
||||||
# Add stdout sink (if not already added by other code)
|
# Add stdout sink (if not already added by other code)
|
||||||
logger.remove() # remove default handlers to avoid duplicate lines
|
logger.remove() # remove default handlers to avoid duplicate lines
|
||||||
logger.add(sys.stdout, level="INFO", enqueue=True, backtrace=False, diagnose=False)
|
logger.add(sys.stdout, level=stdout_level, enqueue=True, backtrace=False, diagnose=False)
|
||||||
# Add rotating file sink for persistence in container. Use JSON lines
|
# Add rotating file sink for persistence in container. Use JSON lines
|
||||||
# (serialize=True) to make logs easy to query in Loki/Grafana.
|
# (serialize=True) to make logs easy to query in Loki/Grafana.
|
||||||
logger.add(
|
logger.add(
|
||||||
@@ -35,7 +44,7 @@ try:
|
|||||||
enqueue=True,
|
enqueue=True,
|
||||||
serialize=True,
|
serialize=True,
|
||||||
)
|
)
|
||||||
logger.debug("Loguru logging configured with stdout and file sinks")
|
logger.debug("Loguru logging configured with stdout and file sinks (stdout=%s)", stdout_level)
|
||||||
|
|
||||||
logging.root.setLevel(logging.DEBUG)
|
logging.root.setLevel(logging.DEBUG)
|
||||||
except Exception:
|
except Exception:
|
||||||
|
|||||||
Reference in New Issue
Block a user