fix: gracefully handle ephemeral document permissions during GST validation

india_compliance/gst_india/overrides/company.py

@@ -2,6 +2,7 @@
 from erpnext.setup.setup_wizard.operations.taxes_setup import (
     from_detailed_data,
 )
+from frappe import _
 from frappe.utils import flt
 
 from india_compliance.gst_india.utils import get_data_file_path
@@ -64,7 +65,25 @@ def make_default_gst_expense_accounts(company):
 
 @frappe.whitelist()
 def make_default_tax_templates(company: str, gst_rate: float | None = None):
-    frappe.has_permission("Company", ptype="write", doc=company, throw=True)
+    # 1. Doctype-level permission check first (no doc lookup, safe from disclosure)
+    frappe.has_permission("Company", ptype="write", doc=None, throw=True)
+
+    # 2. Gracefully handle unsaved or missing documents.
+    # Case A: ephemeral UI name (new-*) that isn't a real saved record
+    # Case B: record deleted by another user or a stale/invalid name (race condition guard)
+    if not frappe.db.exists("Company", company):
+        frappe.throw(_("Please save the Company before creating tax templates."))
+
+    # 3. Record confirmed to exist — run full document-level permission check.
+    # The try/except guards against a TOCTOU race condition where the record is
+    # deleted between the existence check above and the get_doc() call inside
+    # has_permission — ensuring DoesNotExistError never surfaces to the client.
+    try:
+        frappe.has_permission("Company", ptype="write", doc=company, throw=True)
+    except frappe.DoesNotExistError:
+        frappe.throw(
+            _("Company {0} no longer exists. It may have been deleted.").format(frappe.bold(company))
+        )
 
     default_taxes = get_tax_defaults(gst_rate)
     from_detailed_data(company, default_taxes)

india_compliance/gst_india/overrides/test_company.py

@@ -1,7 +1,7 @@
 import frappe
 from frappe.tests import IntegrationTestCase
 
-from india_compliance.gst_india.overrides.company import get_tax_defaults
+from india_compliance.gst_india.overrides.company import get_tax_defaults, make_default_tax_templates
 
 
 class TestCompanyFixtures(IntegrationTestCase):
@@ -34,6 +34,20 @@ def test_tax_defaults_setup(self):
         # Check for tax category creations.
         self.assertTrue(frappe.db.exists("Tax Category", "Reverse Charge In-State"))
 
+    def test_make_default_tax_templates_with_ephemeral_company(self):
+        """make_default_tax_templates should throw a user-friendly error for unsaved companies."""
+        original_user = frappe.session.user
+        try:
+            frappe.set_user("Administrator")
+            self.assertRaisesRegex(
+                frappe.ValidationError,
+                "Please save the Company before creating tax templates.",
+                make_default_tax_templates,
+                "new-company-1",
+            )
+        finally:
+            frappe.set_user(original_user)
+
     def test_get_tax_defaults(self):
         gst_rate = 12
         default_taxes = get_tax_defaults(gst_rate)

india_compliance/gst_india/utils/__init__.py

@@ -100,7 +100,26 @@ def get_gstin_list(party: str, party_type: str = "Company", exclude_isd: bool =
     """
     Returns a list the party's GSTINs.
     """
-    frappe.has_permission(party_type, doc=party, throw=True)
+    # 1. Doctype-level permission check first (no doc lookup, safe from disclosure)
+    frappe.has_permission(party_type, ptype="read", doc=None, throw=True)
+
+    # 2. Gracefully handle unsaved or missing documents.
+    # Returning [] is an intentional architectural choice: the frontend calls this
+    # function in real-time as the user types, including before the document is saved.
+    # An empty list signals "no GSTINs yet" without blocking the UI or leaking existence info.
+    # Case A: ephemeral UI name (new-*) that isn't a real saved record
+    # Case B: record deleted by another user or a stale/invalid name (race condition guard)
+    if not frappe.db.exists(party_type, party):
+        return []
+
+    # 3. Record confirmed to exist — run full document-level permission check.
+    # The try/except guards against a TOCTOU race condition where the record is
+    # deleted between the existence check above and the get_doc() call inside
+    # has_permission — ensuring DoesNotExistError never surfaces to the client.
+    try:
+        frappe.has_permission(party_type, ptype="read", doc=party, throw=True)
+    except frappe.DoesNotExistError:
+        return []
 
     filters = {
         "link_doctype": party_type,

india_compliance/gst_india/utils/test_utils.py

@@ -10,6 +10,7 @@ class TestUtils(IntegrationTestCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
+        frappe.db.savepoint("before_test_utils")
 
         # create old fiscal years
         fiscal_year = frappe.new_doc("Fiscal Year")
@@ -30,6 +31,36 @@ def setUpClass(cls):
             }
         ).insert(ignore_if_duplicate=True)
 
+        # Restricted user for permission boundary tests
+        cls.restricted_user = "test_no_perms@example.com"
+        if not frappe.db.exists("User", cls.restricted_user):
+            frappe.get_doc(
+                {
+                    "doctype": "User",
+                    "email": cls.restricted_user,
+                    "first_name": "Test",
+                    "send_welcome_email": 0,
+                }
+            ).insert(ignore_permissions=True)
+
+        # User with Purchase Manager role (has Supplier read access)
+        cls.purchase_user = "test_purchase_mgr@example.com"
+        if not frappe.db.exists("User", cls.purchase_user):
+            frappe.get_doc(
+                {
+                    "doctype": "User",
+                    "email": cls.purchase_user,
+                    "first_name": "Purchase",
+                    "send_welcome_email": 0,
+                    "roles": [{"role": "Purchase Manager"}],
+                }
+            ).insert(ignore_permissions=True)
+
+    @classmethod
+    def tearDownClass(cls):
+        frappe.set_user("Administrator")
+        frappe.db.rollback(save_point="before_test_utils")
+
     @patch("india_compliance.gst_india.utils.getdate", return_value=getdate("2023-06-20"))
     def test_timespan_date_range(self, getdate_mock):
         from india_compliance.gst_india.utils import get_timespan_date_range
@@ -46,3 +77,50 @@ def test_timespan_date_range(self, getdate_mock):
 
             for i, expected_date in enumerate(expected_date_range):
                 self.assertEqual(expected_date, actual_date_range[i])
+
+    def test_get_gstin_list_with_ephemeral_document(self):
+        """get_gstin_list should return empty list for unsaved (ephemeral) documents."""
+        from india_compliance.gst_india.utils import get_gstin_list
+
+        original_user = frappe.session.user
+        try:
+            frappe.set_user("Administrator")
+            result = get_gstin_list("new-supplier-1", "Supplier")
+            self.assertEqual(result, [])
+        finally:
+            frappe.set_user(original_user)
+
+    def test_get_gstin_list_returns_list_for_saved_document(self):
+        """get_gstin_list should return a list (not raise) for a real saved Supplier."""
+        from india_compliance.gst_india.utils import get_gstin_list
+
+        original_user = frappe.session.user
+        try:
+            frappe.set_user(self.purchase_user)
+            # Uses a Supplier fixture defined in india_compliance/tests/test_records.json
+            result = get_gstin_list("_Test Registered Supplier", "Supplier")
+            self.assertIsInstance(result, list)
+        finally:
+            frappe.set_user(original_user)
+
+    def test_get_gstin_list_raises_permission_error_for_restricted_user(self):
+        """get_gstin_list should raise PermissionError for users without doctype access."""
+        from india_compliance.gst_india.utils import get_gstin_list
+
+        original_user = frappe.session.user
+        try:
+            frappe.set_user(self.restricted_user)
+            self.assertRaises(frappe.PermissionError, get_gstin_list, "new-supplier-1", "Supplier")
+        finally:
+            frappe.set_user(original_user)
+
+    def test_make_default_tax_templates_raises_permission_error_for_restricted_user(self):
+        """make_default_tax_templates should raise PermissionError for users without write access."""
+        from india_compliance.gst_india.overrides.company import make_default_tax_templates
+
+        original_user = frappe.session.user
+        try:
+            frappe.set_user(self.restricted_user)
+            self.assertRaises(frappe.PermissionError, make_default_tax_templates, "new-company-1")
+        finally:
+            frappe.set_user(original_user)