diff --git a/wiki/frappe_wiki/doctype/wiki_document/test_wiki_document.py b/wiki/frappe_wiki/doctype/wiki_document/test_wiki_document.py index f56dce8e..b64ac458 100644 --- a/wiki/frappe_wiki/doctype/wiki_document/test_wiki_document.py +++ b/wiki/frappe_wiki/doctype/wiki_document/test_wiki_document.py @@ -11,6 +11,7 @@ from frappe.utils import get_test_client from wiki.frappe_wiki.doctype.wiki_document.wiki_document import process_navbar_items +from wiki.test_api import create_test_user from wiki.wiki.markdown import render_markdown, render_markdown_with_toc # On IntegrationTestCase, the doctype test records and all @@ -174,6 +175,41 @@ def test_single_document_has_no_prev_or_next(self): self.assertIsNone(context["prev_doc"]) self.assertIsNone(context["next_doc"]) + def test_role_restricted_space_blocks_user_without_allowed_role(self): + doc = create_test_wiki_document(self, "Restricted Document") + user = create_test_user("restricted-no-role@example.com", roles=["Website Manager"]) + fake_space = SimpleNamespace(name="restricted-space", is_published=1, roles=[frappe._dict(role="Wiki User")]) + + with ( + patch.object(doc, "get_wiki_space", return_value=frappe._dict(name="restricted-space")), + patch( + "wiki.frappe_wiki.doctype.wiki_document.wiki_document.frappe.get_cached_doc", + return_value=fake_space, + ), + ): + frappe.set_user(user.name) + with self.assertRaises(frappe.PermissionError): + doc.check_guest_access() + + frappe.set_user("Administrator") + + def test_role_restricted_space_allows_user_with_allowed_role(self): + doc = create_test_wiki_document(self, "Allowed Document") + user = create_test_user("restricted-allowed@example.com", roles=["Wiki User"]) + fake_space = SimpleNamespace(name="restricted-space", is_published=1, roles=[frappe._dict(role="Wiki User")]) + + with ( + patch.object(doc, "get_wiki_space", return_value=frappe._dict(name="restricted-space")), + patch( + "wiki.frappe_wiki.doctype.wiki_document.wiki_document.frappe.get_cached_doc", + return_value=fake_space, + ), + ): + frappe.set_user(user.name) + doc.check_guest_access() + + frappe.set_user("Administrator") + def test_wiki_spaces_for_switcher_includes_current_space_even_if_not_published(self): """ Test that wiki_spaces_for_switcher includes the current space diff --git a/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py b/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py index 0fcd231a..53822ec7 100644 --- a/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py +++ b/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py @@ -10,6 +10,7 @@ from frappe.website.page_renderers.base_renderer import BaseRenderer from werkzeug.wrappers import Response +from wiki.utils import has_wiki_space_access from wiki.wiki.markdown import render_markdown_with_toc # Mapping of known service domains to icon identifiers @@ -243,6 +244,19 @@ def check_guest_access(self): frappe.PermissionError, ) + wiki_space = self.get_wiki_space() + if not wiki_space: + return + + space_doc = frappe.get_cached_doc("Wiki Space", wiki_space.name) + if not has_wiki_space_access(space_doc): + message = ( + frappe._("You must be logged in to view this page") + if frappe.session.user == "Guest" + else frappe._("You do not have access to view this page") + ) + frappe.throw(message, frappe.PermissionError) + def check_published(self): if not self.is_published: frappe.throw( diff --git a/wiki/utils.py b/wiki/utils.py index e3ff9591..42122645 100644 --- a/wiki/utils.py +++ b/wiki/utils.py @@ -1,5 +1,6 @@ import frappe from frappe.core.doctype.file.utils import get_content_hash +from frappe.utils import cint def get_tailwindcss_hash(): @@ -23,3 +24,65 @@ def check_app_permission(): def add_wiki_user_role(doc, event=None): doc.add_roles("Wiki User") + + +def get_wiki_space_for_route(route: str, published_only: bool = False): + """Return the wiki space whose route owns the given page route. + + Matches the longest wiki space route prefix, so a page route like + `team/start-here` resolves to the `team` wiki space. + """ + if not route: + return None + + normalized_route = route.strip("/") + spaces = frappe.get_all("Wiki Space", fields=["name", "route", "is_published"]) + + for space in sorted(spaces, key=lambda row: len((row.route or "").strip("/")), reverse=True): + space_route = (space.route or "").strip("/") + if not space_route: + continue + + if normalized_route != space_route and not normalized_route.startswith(f"{space_route}/"): + continue + + if published_only and not cint(space.is_published): + continue + + return frappe.get_cached_doc("Wiki Space", space.name) + + return None + + +def get_wiki_space_allowed_roles(wiki_space) -> set[str]: + """Extract the configured role allowlist from a wiki space, if any.""" + if not wiki_space: + return set() + + roles = wiki_space.get("roles") if hasattr(wiki_space, "get") else getattr(wiki_space, "roles", None) + allowed_roles = set() + + for row in roles or []: + role = row.get("role") if hasattr(row, "get") else getattr(row, "role", None) + if role: + allowed_roles.add(role) + + return allowed_roles + + +def has_wiki_space_access(wiki_space, user: str | None = None) -> bool: + """Check whether the user satisfies the wiki space role allowlist. + + If no roles are configured on the space, access falls back to the page-level + guest/login behaviour. + """ + user = user or frappe.session.user + + if user == "Administrator": + return True + + allowed_roles = get_wiki_space_allowed_roles(wiki_space) + if not allowed_roles: + return True + + return bool(allowed_roles.intersection(set(frappe.get_roles(user)))) diff --git a/wiki/wiki/doctype/wiki_page/test_wiki_page.py b/wiki/wiki/doctype/wiki_page/test_wiki_page.py index 58bd90fb..74c016fc 100644 --- a/wiki/wiki/doctype/wiki_page/test_wiki_page.py +++ b/wiki/wiki/doctype/wiki_page/test_wiki_page.py @@ -1,5 +1,56 @@ # Copyright (c) 2020, Frappe and Contributors # See license.txt -# Wiki Page doctype is deprecated. Tests have been removed. -# See Wiki Document for the new implementation. +from types import SimpleNamespace +from unittest.mock import patch + +import frappe +from frappe.tests import FrappeTestCase + + +class TestWikiPageWebsitePermission(FrappeTestCase): + def test_private_page_requires_login_when_space_has_no_role_allowlist(self): + page = frappe.new_doc("Wiki Page") + page.title = "Private Page" + page.route = "private/page" + page.published = 1 + page.allow_guest = 0 + + with patch( + "wiki.wiki.doctype.wiki_page.wiki_page.get_wiki_space_for_route", + return_value=SimpleNamespace(roles=[]), + ): + self.assertFalse(page.has_website_permission("read", "Guest")) + self.assertTrue(page.has_website_permission("read", "user@example.com")) + + def test_space_role_allowlist_blocks_logged_in_user_without_role(self): + page = frappe.new_doc("Wiki Page") + page.title = "Restricted Page" + page.route = "restricted/page" + page.published = 1 + page.allow_guest = 0 + + with ( + patch( + "wiki.wiki.doctype.wiki_page.wiki_page.get_wiki_space_for_route", + return_value=SimpleNamespace(roles=[frappe._dict(role="Wiki User")]), + ), + patch( + "wiki.wiki.doctype.wiki_page.wiki_page.has_wiki_space_access", + return_value=False, + ), + ): + self.assertFalse(page.has_website_permission("read", "user@example.com")) + + def test_published_public_page_allows_guest_when_space_roles_pass(self): + page = frappe.new_doc("Wiki Page") + page.title = "Public Page" + page.route = "public/page" + page.published = 1 + page.allow_guest = 1 + + with patch( + "wiki.wiki.doctype.wiki_page.wiki_page.get_wiki_space_for_route", + return_value=None, + ): + self.assertTrue(page.has_website_permission("read", "Guest")) diff --git a/wiki/wiki/doctype/wiki_page/wiki_page.json b/wiki/wiki/doctype/wiki_page/wiki_page.json index 6489194f..0f4c9a5d 100644 --- a/wiki/wiki/doctype/wiki_page/wiki_page.json +++ b/wiki/wiki/doctype/wiki_page/wiki_page.json @@ -1,6 +1,6 @@ { "actions": [], - "allow_guest_to_view": 1, + "allow_guest_to_view": 0, "allow_import": 1, "allow_rename": 1, "autoname": "hash", @@ -152,4 +152,4 @@ "title_field": "title", "track_changes": 1, "website_search_field": "content" -} \ No newline at end of file +} diff --git a/wiki/wiki/doctype/wiki_page/wiki_page.py b/wiki/wiki/doctype/wiki_page/wiki_page.py index 0d714f18..4d189b2e 100644 --- a/wiki/wiki/doctype/wiki_page/wiki_page.py +++ b/wiki/wiki/doctype/wiki_page/wiki_page.py @@ -3,6 +3,8 @@ import frappe from frappe.website.website_generator import WebsiteGenerator +from wiki.utils import get_wiki_space_for_route, has_wiki_space_access + class WikiPage(WebsiteGenerator): def validate(self): @@ -24,3 +26,16 @@ def _migrate_to_wiki_document(self): "is_private": not self.allow_guest, } ).insert() + + def has_website_permission(self, ptype, user, verbose=False): + if not self.published: + return False + + wiki_space = get_wiki_space_for_route(self.route) + if wiki_space and not has_wiki_space_access(wiki_space, user=user): + return False + + if self.allow_guest: + return True + + return user != "Guest" diff --git a/wiki/wiki/doctype/wiki_space/wiki_space.json b/wiki/wiki/doctype/wiki_space/wiki_space.json index b5bde80d..bade065a 100644 --- a/wiki/wiki/doctype/wiki_space/wiki_space.json +++ b/wiki/wiki/doctype/wiki_space/wiki_space.json @@ -11,6 +11,7 @@ "general_tab", "space_name", "route", + "roles", "switcher_order", "column_break_bynr", "app_switcher_logo", @@ -39,6 +40,12 @@ "reqd": 1, "unique": 1 }, + { + "fieldname": "roles", + "fieldtype": "Table", + "label": "Roles", + "options": "Has Role" + }, { "fieldname": "wiki_sidebars", "fieldtype": "Table", @@ -215,4 +222,4 @@ "sort_order": "DESC", "states": [], "title_field": "route" -} \ No newline at end of file +} diff --git a/wiki/wiki/doctype/wiki_space/wiki_space.py b/wiki/wiki/doctype/wiki_space/wiki_space.py index a55132f4..68735181 100644 --- a/wiki/wiki/doctype/wiki_space/wiki_space.py +++ b/wiki/wiki/doctype/wiki_space/wiki_space.py @@ -27,6 +27,7 @@ class WikiSpace(Document): if TYPE_CHECKING: from frappe.types import DF + from frappe.core.doctype.has_role.has_role import HasRole from frappe.website.doctype.top_bar_item.top_bar_item import TopBarItem from wiki.wiki.doctype.wiki_group_item.wiki_group_item import WikiGroupItem @@ -40,6 +41,7 @@ class WikiSpace(Document): navbar_items: DF.Table[TopBarItem] root_group: DF.Link | None route: DF.Data + roles: DF.Table[HasRole] show_in_switcher: DF.Check space_name: DF.Data | None switcher_order: DF.Int