From 89f4a867bae0c61677a894e9ed11deded0f12c63 Mon Sep 17 00:00:00 2001
From: "Chen, Vivien" <Vivien.Chen+ecolab@ecolab.com>
Date: Thu, 5 Mar 2026 01:02:34 -0500
Subject: [PATCH 1/2] feat: admin-configurable access denied message (#557)

- Add access_denied_message to default_settings in functions_settings.py
- Persist access_denied_message from Admin Settings form in route_frontend_admin_settings.py
- Add Access Denied Message textarea to Admin Settings UI
- Render dynamic message on index.html for signed-in users lacking required roles

Fix Copilot PR #557 findings:
- Finding 1: default message in functions_settings.py now matches index.html fallback
  (both use 'Please contact an administrator for access.')
- Finding 2: replaced '| e | replace(\\n, <br>) | safe' filter chain with
  a proper nl2br Jinja filter registered in app.py, avoiding potential
  filter-order confusion flagged by Copilot review
---
 application/single_app/app.py                          | 10 ++++++++++
 application/single_app/config.py                       |  2 +-
 application/single_app/functions_settings.py           |  3 +++
 .../single_app/route_frontend_admin_settings.py        |  1 +
 application/single_app/templates/admin_settings.html   |  6 ++++++
 application/single_app/templates/index.html            |  3 +--
 6 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/application/single_app/app.py b/application/single_app/app.py
index 2354b1b5..d58eeed5 100644
--- a/application/single_app/app.py
+++ b/application/single_app/app.py
@@ -487,6 +487,16 @@ def markdown_filter(text):
 # Add the filter to the Jinja environment
 app.jinja_env.filters['markdown'] = markdown_filter
 
+# Register a custom Jinja filter for nl2br (newline to <br>)
+def nl2br_filter(value):
+    """Escape HTML then convert newline characters to <br> tags."""
+    from markupsafe import escape, Markup
+    if not value:
+        return Markup('')
+    return Markup(str(escape(value)).replace('\n', '<br>\n'))
+
+app.jinja_env.filters['nl2br'] = nl2br_filter
+
 # =================== Default Routes =====================
 @app.route('/')
 @swagger_route(security=get_auth_security())
diff --git a/application/single_app/config.py b/application/single_app/config.py
index 91288225..40d17d3c 100644
--- a/application/single_app/config.py
+++ b/application/single_app/config.py
@@ -94,7 +94,7 @@
 EXECUTOR_TYPE = 'thread'
 EXECUTOR_MAX_WORKERS = 30
 SESSION_TYPE = 'filesystem'
-VERSION = "0.239.002"
+VERSION = "0.239.003"
 
 SECRET_KEY = os.getenv('SECRET_KEY', 'dev-secret-key-change-in-production')
 
diff --git a/application/single_app/functions_settings.py b/application/single_app/functions_settings.py
index 8176939d..89367065 100644
--- a/application/single_app/functions_settings.py
+++ b/application/single_app/functions_settings.py
@@ -260,6 +260,9 @@ def get_settings(use_cosmos=False):
         'max_file_size_mb': 150,
         'conversation_history_limit': 10,
         'default_system_prompt': '',
+        # Access denied message shown on the home page for signed-in users who lack required roles.
+        # Default is hard-coded; admins can override via Admin Settings (persisted in Cosmos DB).
+        'access_denied_message': 'You are logged in but do not have the required permissions to access this application.\nPlease contact an administrator for access.',
         'enable_file_processing_logs': True,
         'file_processing_logs_timer_enabled': False,
         'file_timer_value': 1,
diff --git a/application/single_app/route_frontend_admin_settings.py b/application/single_app/route_frontend_admin_settings.py
index 578e1545..bf5c8077 100644
--- a/application/single_app/route_frontend_admin_settings.py
+++ b/application/single_app/route_frontend_admin_settings.py
@@ -869,6 +869,7 @@ def is_valid_url(url):
                 'max_file_size_mb': max_file_size_mb,
                 'conversation_history_limit': conversation_history_limit,
                 'default_system_prompt': form_data.get('default_system_prompt', '').strip(),
+                'access_denied_message': form_data.get('access_denied_message', '').strip(),
 
                 # Video file settings with Azure Video Indexer Settings
                 'video_indexer_endpoint': form_data.get('video_indexer_endpoint', video_indexer_endpoint).strip(),
diff --git a/application/single_app/templates/admin_settings.html b/application/single_app/templates/admin_settings.html
index 7d01f7da..f8c8b623 100644
--- a/application/single_app/templates/admin_settings.html
+++ b/application/single_app/templates/admin_settings.html
@@ -1428,6 +1428,12 @@ <h5>
                         <textarea class="form-control" id="default_system_prompt" name="default_system_prompt"
                             rows="5">{{ settings.default_system_prompt }}</textarea>
                     </div>
+                    <div class="mb-3">
+                        <label for="access_denied_message" class="form-label">Access Denied Message</label>
+                        <small class="text-muted d-block mb-1">Shown to signed-in users who lack the required roles. Use Enter for line breaks.</small>
+                        <textarea class="form-control" id="access_denied_message" name="access_denied_message"
+                            rows="3">{{ settings.access_denied_message }}</textarea>
+                    </div>
                 </div>
             </div>
 
diff --git a/application/single_app/templates/index.html b/application/single_app/templates/index.html
index 7a146e0d..4d415f4d 100644
--- a/application/single_app/templates/index.html
+++ b/application/single_app/templates/index.html
@@ -62,8 +62,7 @@
     {% else %}
         {% if session.get('user') %}
             <p class="lead">
-                You are logged in but do not have the required permissions to access this application.
-                Please submit a ticket to request access.
+                {{ (app_settings.access_denied_message or 'You are logged in but do not have the required permissions to access this application.\nPlease contact an administrator for access.') | nl2br }}
             </p>
         {% else %}
             <div>

From dc29406d2b536790e191d5442e447905e7757a9c Mon Sep 17 00:00:00 2001
From: "Chen, Vivien" <Vivien.Chen+ecolab@ecolab.com>
Date: Thu, 5 Mar 2026 13:14:11 -0500
Subject: [PATCH 2/2] fix: address Copilot PR review findings for access denied
 message feature

Finding 1 (index.html redundant fallback):
- Removed hardcoded 'or ...' fallback from access_denied_message rendering in
  templates/index.html; default lives exclusively in functions_settings.py and
  is guaranteed present after get_settings() deep-merge

Finding 2 (route silent data loss):
- Changed access_denied_message in route_frontend_admin_settings.py to fall back
  to settings.get('access_denied_message', '') instead of '' so an older/cached
  form submission that omits the field does not wipe the existing stored value

Finding 3 (missing functional regression test):
- Added functional_tests/test_access_denied_message_feature.py (4/4 passing):
  (1) admin_settings.html exposes textarea name=access_denied_message with label
  (2) route uses safe settings.get() fallback, not bare empty string
  (3) index.html renders via nl2br with no inline hardcoded fallback
  (4) functions_settings.py defines a non-empty default value
---
 .../route_frontend_admin_settings.py          |   2 +-
 application/single_app/templates/index.html   |   2 +-
 .../test_access_denied_message_feature.py     | 197 ++++++++++++++++++
 3 files changed, 199 insertions(+), 2 deletions(-)
 create mode 100644 functional_tests/test_access_denied_message_feature.py

diff --git a/application/single_app/route_frontend_admin_settings.py b/application/single_app/route_frontend_admin_settings.py
index bf5c8077..2fc5abc8 100644
--- a/application/single_app/route_frontend_admin_settings.py
+++ b/application/single_app/route_frontend_admin_settings.py
@@ -869,7 +869,7 @@ def is_valid_url(url):
                 'max_file_size_mb': max_file_size_mb,
                 'conversation_history_limit': conversation_history_limit,
                 'default_system_prompt': form_data.get('default_system_prompt', '').strip(),
-                'access_denied_message': form_data.get('access_denied_message', '').strip(),
+                'access_denied_message': form_data.get('access_denied_message', settings.get('access_denied_message', '')).strip(),
 
                 # Video file settings with Azure Video Indexer Settings
                 'video_indexer_endpoint': form_data.get('video_indexer_endpoint', video_indexer_endpoint).strip(),
diff --git a/application/single_app/templates/index.html b/application/single_app/templates/index.html
index 4d415f4d..c3c2abc6 100644
--- a/application/single_app/templates/index.html
+++ b/application/single_app/templates/index.html
@@ -62,7 +62,7 @@
     {% else %}
         {% if session.get('user') %}
             <p class="lead">
-                {{ (app_settings.access_denied_message or 'You are logged in but do not have the required permissions to access this application.\nPlease contact an administrator for access.') | nl2br }}
+                {{ app_settings.access_denied_message | nl2br }}
             </p>
         {% else %}
             <div>
diff --git a/functional_tests/test_access_denied_message_feature.py b/functional_tests/test_access_denied_message_feature.py
new file mode 100644
index 00000000..4a0a2194
--- /dev/null
+++ b/functional_tests/test_access_denied_message_feature.py
@@ -0,0 +1,197 @@
+#!/usr/bin/env python3
+# test_access_denied_message_feature.py
+"""
+Functional regression test for admin-configurable access denied message.
+
+Version: 0.239.002
+Implemented in: 0.239.002
+
+This test ensures that:
+1. The Admin Settings template exposes a textarea with name="access_denied_message".
+2. route_frontend_admin_settings.py reads the field from form_data and falls back
+   to the existing stored value (not '') when the field is absent -- preventing
+   silent data loss from cached/older form submissions.
+3. index.html renders app_settings.access_denied_message through the nl2br filter
+   without a redundant hardcoded fallback string.
+4. functions_settings.py defines a non-empty default for access_denied_message so
+   the field is always present after get_settings() deep-merges defaults.
+"""
+
+import sys
+import os
+import re
+
+# Resolve paths relative to repo root
+REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+
+ADMIN_TEMPLATE    = os.path.join(REPO_ROOT, "application", "single_app", "templates", "admin_settings.html")
+INDEX_TEMPLATE    = os.path.join(REPO_ROOT, "application", "single_app", "templates", "index.html")
+ROUTE_FILE        = os.path.join(REPO_ROOT, "application", "single_app", "route_frontend_admin_settings.py")
+SETTINGS_FILE     = os.path.join(REPO_ROOT, "application", "single_app", "functions_settings.py")
+
+
+# ---------------------------------------------------------------------------
+# Test 1 – Admin Settings template has the access_denied_message field
+# ---------------------------------------------------------------------------
+
+def test_admin_template_has_field():
+    """Admin Settings template must expose a textarea named access_denied_message."""
+    print("Testing admin_settings.html contains access_denied_message field...")
+    errors = []
+
+    with open(ADMIN_TEMPLATE, encoding="utf-8") as f:
+        content = f.read()
+
+    # textarea with correct name attribute
+    if 'name="access_denied_message"' not in content:
+        errors.append("No <textarea name=\"access_denied_message\"> found in admin_settings.html")
+
+    # label pointing to the field
+    if 'for="access_denied_message"' not in content:
+        errors.append("No <label for=\"access_denied_message\"> found in admin_settings.html")
+
+    # renders the current stored value
+    if 'settings.access_denied_message' not in content:
+        errors.append("Textarea does not render {{ settings.access_denied_message }}")
+
+    return _summarise(errors, "admin template field")
+
+
+# ---------------------------------------------------------------------------
+# Test 2 – Route persists access_denied_message with safe fallback
+# ---------------------------------------------------------------------------
+
+def test_route_persists_with_safe_fallback():
+    """route_frontend_admin_settings must fall back to existing stored value, not ''."""
+    print("\nTesting route_frontend_admin_settings.py persistence logic...")
+    errors = []
+
+    with open(ROUTE_FILE, encoding="utf-8") as f:
+        content = f.read()
+
+    # Key must be written into the settings dict
+    if "'access_denied_message'" not in content:
+        errors.append("'access_denied_message' key not found in route file")
+        return _summarise(errors, "route persistence")
+
+    # Must use form_data.get('access_denied_message', ...) - not a hard '' fallback
+    # Correct pattern: form_data.get('access_denied_message', settings.get(...))
+    safe_fallback_pattern = re.compile(
+        r"'access_denied_message'\s*:\s*form_data\.get\(\s*'access_denied_message'\s*,"
+        r"\s*settings\.get\("
+    )
+    if not safe_fallback_pattern.search(content):
+        errors.append(
+            "access_denied_message does not use settings.get() as fallback -- "
+            "form_data.get('access_denied_message', settings.get(...)) pattern not found"
+        )
+
+    # Must NOT be: form_data.get('access_denied_message', '')  (bare empty-string fallback)
+    bare_empty_pattern = re.compile(
+        r"'access_denied_message'\s*:\s*form_data\.get\(\s*'access_denied_message'\s*,\s*''\s*\)"
+    )
+    if bare_empty_pattern.search(content):
+        errors.append(
+            "access_denied_message still has bare '' fallback -- would wipe stored value "
+            "if field is absent from form submission"
+        )
+
+    return _summarise(errors, "route persistence")
+
+
+# ---------------------------------------------------------------------------
+# Test 3 – index.html renders via nl2br without a hardcoded fallback
+# ---------------------------------------------------------------------------
+
+def test_index_renders_via_nl2br_no_hardcoded_fallback():
+    """index.html must render access_denied_message | nl2br with no inline fallback."""
+    print("\nTesting index.html nl2br rendering...")
+    errors = []
+
+    with open(INDEX_TEMPLATE, encoding="utf-8") as f:
+        content = f.read()
+
+    # Must use the nl2br filter
+    if 'access_denied_message | nl2br' not in content:
+        errors.append("index.html does not render access_denied_message through nl2br filter")
+
+    # Must NOT contain a hardcoded fallback string inline
+    hardcoded_pattern = re.compile(
+        r"access_denied_message\s+or\s+'You are logged in"
+    )
+    if hardcoded_pattern.search(content):
+        errors.append(
+            "index.html still has a hardcoded fallback string -- "
+            "default should live only in functions_settings.py"
+        )
+
+    return _summarise(errors, "index nl2br rendering")
+
+
+# ---------------------------------------------------------------------------
+# Test 4 – functions_settings.py defines a non-empty default
+# ---------------------------------------------------------------------------
+
+def test_settings_default_is_defined():
+    """functions_settings.py must define a non-empty default for access_denied_message."""
+    print("\nTesting functions_settings.py default value...")
+    errors = []
+
+    with open(SETTINGS_FILE, encoding="utf-8") as f:
+        content = f.read()
+
+    pattern = re.compile(
+        r"'access_denied_message'\s*:\s*'(.+?)'"
+    )
+    match = pattern.search(content)
+    if not match:
+        errors.append("No non-empty default for 'access_denied_message' found in functions_settings.py")
+    else:
+        print(f"  Default value: \"{match.group(1)[:60]}...\"")
+
+    return _summarise(errors, "settings default")
+
+
+# ---------------------------------------------------------------------------
+# Helper
+# ---------------------------------------------------------------------------
+
+def _summarise(errors, label):
+    if errors:
+        for e in errors:
+            print(f"  FAIL: {e}")
+        return False
+    print(f"  All {label} checks passed!")
+    return True
+
+
+# ---------------------------------------------------------------------------
+# Entry point
+# ---------------------------------------------------------------------------
+
+if __name__ == "__main__":
+    tests = [
+        test_admin_template_has_field,
+        test_route_persists_with_safe_fallback,
+        test_index_renders_via_nl2br_no_hardcoded_fallback,
+        test_settings_default_is_defined,
+    ]
+    results = []
+    for t in tests:
+        print(f"\n{'='*60}")
+        print(f"Running {t.__name__}...")
+        print("="*60)
+        try:
+            results.append(t())
+        except Exception as exc:
+            import traceback
+            print(f"ERROR: {exc}")
+            traceback.print_exc()
+            results.append(False)
+
+    passed = sum(1 for r in results if r)
+    total = len(results)
+    print(f"\n{'='*60}")
+    print(f"Results: {passed}/{total} tests passed")
+    print("="*60)
+    sys.exit(0 if all(results) else 1)