From 0f3e70f5148cff555c070ec6c4e8d200331d24ab Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 15:39:25 +0200 Subject: [PATCH 1/8] Modernize tooling & CI: ruff, hatch-vcs, Trusted Publishing Aligns the project's CI/release setup with the reference layout used by other BlueDynamics packages. Tooling - Derive the version from git tags via hatch-vcs; drop the static `version` from pyproject.toml. Releases are made by tagging. - Switch Python lint/format from black + isort to ruff (config mirrors the reference, with Zope/Plone-idiomatic ignores: A001/A002/A003, RUF012). Reformat the codebase accordingly. Two real fixes fell out: `raise ... from err` in the control panel (B904) and `contextlib.suppress` in the uninstall handler (SIM105). CI / release - Restructure GitHub Actions into a `CI` umbrella workflow (ci.yaml) that calls reusable `qa.yaml` (ruff) and `tests.yaml` (the existing Plone 6.0-6.2 / Python 3.10-3.14 matrix, now `workflow_call`). - Add `release.yaml`: build & inspect the package, publish in-dev builds to Test PyPI after CI passes on main, and publish tagged releases to PyPI -- both via OIDC Trusted Publishing (no API tokens). Docs - Update the README development workflow (ruff instead of black/isort) and the CI badge; note that .mo catalogs are compiled at build time. Full test suite green (240 passed); `ruff check .` and `ruff format --check .` clean; the build (hatch-vcs + babel hook) produces sdist + wheel containing the compiled en/es .mo catalogs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yaml | 20 ++ .github/workflows/qa.yaml | 22 ++ .github/workflows/release.yaml | 77 ++++++ .github/workflows/tests.yaml | 10 +- CHANGES.rst | 11 + README.rst | 28 ++- hatch_build.py | 1 + pyproject.toml | 57 ++++- src/pas/plugins/ldap/cache.py | 6 +- src/pas/plugins/ldap/defaults.py | 1 + src/pas/plugins/ldap/interfaces.py | 2 +- src/pas/plugins/ldap/locales/__main__.py | 15 +- src/pas/plugins/ldap/monkey.py | 13 +- .../plugins/ldap/plonecontrolpanel/cache.py | 1 + .../ldap/plonecontrolpanel/controlpanel.py | 4 +- .../ldap/plonecontrolpanel/exportimport.py | 8 +- .../ldap/plonecontrolpanel/inspector.py | 8 +- .../ldap/plonecontrolpanel/setuphandlers.py | 8 +- src/pas/plugins/ldap/plugin.py | 33 ++- src/pas/plugins/ldap/properties.py | 17 +- src/pas/plugins/ldap/setuphandlers.py | 1 + src/pas/plugins/ldap/sheet.py | 9 +- src/pas/plugins/ldap/zmi/manage_plugin.py | 2 +- tests/conftest.py | 11 +- tests/test_cache.py | 7 +- tests/test_doctests.py | 4 +- tests/test_monkey_unit.py | 158 ++++++++----- tests/test_plonecontrolpanel.py | 3 +- tests/test_plugin.py | 4 +- tests/test_plugin_unit.py | 88 ++++--- tests/test_properties_unit.py | 221 ++++++++++++------ tests/test_setuphandlers_unit.py | 28 ++- tests/test_sheet_unit.py | 93 +++++--- tests/testing.py | 27 ++- 34 files changed, 676 insertions(+), 322 deletions(-) create mode 100644 .github/workflows/ci.yaml create mode 100644 .github/workflows/qa.yaml create mode 100644 .github/workflows/release.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..fe111cf --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,20 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + qa: + uses: "./.github/workflows/qa.yaml" + + tests: + uses: "./.github/workflows/tests.yaml" diff --git a/.github/workflows/qa.yaml b/.github/workflows/qa.yaml new file mode 100644 index 0000000..40efddb --- /dev/null +++ b/.github/workflows/qa.yaml @@ -0,0 +1,22 @@ +name: QA + +on: + workflow_call: + +permissions: + contents: read + +jobs: + lint: + name: Ruff + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: astral-sh/setup-uv@v5 + + - name: Run ruff check + run: uvx ruff check . + + - name: Run ruff format check + run: uvx ruff format --check . diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml new file mode 100644 index 0000000..fd27fd5 --- /dev/null +++ b/.github/workflows/release.yaml @@ -0,0 +1,77 @@ +name: Build & upload PyPI package + +on: + # Build dev package after CI passes on main (no duplicate test runs) + workflow_run: + workflows: ["CI"] + types: [completed] + branches: [main] + release: + types: + - published + workflow_dispatch: + +concurrency: + group: release-${{ github.ref }} + cancel-in-progress: true + +jobs: + build-package: + name: Build & verify package + # Skip if triggered by workflow_run and CI failed + if: >- + github.event_name != 'workflow_run' || + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - uses: hynek/build-and-inspect-python-package@v2 + + release-test-pypi: + name: Publish in-dev package to test.pypi.org + environment: release-test-pypi + if: github.event_name == 'workflow_run' || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') + runs-on: ubuntu-latest + needs: + - build-package + permissions: + id-token: write + + steps: + - name: Download packages built by build-and-inspect-python-package + uses: actions/download-artifact@v4 + with: + name: Packages + path: dist + + - name: Upload package to Test PyPI + uses: pypa/gh-action-pypi-publish@release/v1 + with: + repository-url: https://test.pypi.org/legacy/ + + release-pypi: + name: Publish released package to pypi.org + environment: release-pypi + if: github.event.action == 'published' + runs-on: ubuntu-latest + needs: + - build-package + permissions: + id-token: write + + steps: + - name: Download packages built by build-and-inspect-python-package + uses: actions/download-artifact@v4 + with: + name: Packages + path: dist + + - name: Upload package to PyPI + uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index fa7a977..491ba5c 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -1,6 +1,7 @@ -name: Test the pas.plugins.ldap code +name: Tests + on: - push + workflow_call: jobs: build: @@ -16,9 +17,10 @@ jobs: - { python: "3.10", plone: "6.2.0" } - { python: "3.14", plone: "6.2.0" } - steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Install system packages run: | diff --git a/CHANGES.rst b/CHANGES.rst index 5ee1a06..2df5b96 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,17 @@ History 2.0.0 (unreleased) ------------------ +- Derive the package version from git tags via ``hatch-vcs`` (the static + ``version`` in ``pyproject.toml`` is gone). Releases are now made by tagging. + [jensens] + +- Switch Python linting and formatting from black/isort to ``ruff``. + [jensens] + +- Restructure CI into a ``CI`` umbrella workflow (QA + tests) and add a + PyPI/Test-PyPI release workflow using OIDC Trusted Publishing. + [jensens] + - Portrait traverser: raise a proper ``LocationError`` (404) when a user has no portrait on the property sheet, instead of an ``AttributeError`` from calling ``__of__`` on ``None``. diff --git a/README.rst b/README.rst index 9712f1c..25d8b5d 100644 --- a/README.rst +++ b/README.rst @@ -7,9 +7,9 @@ :target: https://pypi.python.org/pypi/pas.plugins.ldap :alt: Number of PyPI downloads -.. image:: https://github.com/collective/pas.plugins.ldap/actions/workflows/tests.yaml/badge.svg - :target: https://github.com/collective/pas.plugins.ldap/actions/workflows/tests.yaml - :alt: Test the pas.plugins.ldap code +.. image:: https://github.com/collective/pas.plugins.ldap/actions/workflows/ci.yaml/badge.svg + :target: https://github.com/collective/pas.plugins.ldap/actions/workflows/ci.yaml + :alt: CI .. image:: https://coveralls.io/repos/collective/pas.plugins.ldap/badge.svg?branch=master&service=github :target: https://coveralls.io/github/collective/pas.plugins.ldap?branch=master @@ -248,37 +248,35 @@ Development Workflow make zope-start -3. Zpretty format and lint code: +3. Zpretty format and lint XML/ZCML: .. code-block:: shell make zpretty-check && make zpretty-format && make zpretty-check -4. Isort format and lint code: +4. Lint and format Python code with `ruff `_ + (this is what CI enforces): .. code-block:: shell - make isort-check && make isort-format && make isort-check + uvx ruff check . && uvx ruff format . -5. Black format and lint code: - -.. code-block:: shell - - make black-check && make black-format && make black-check - -6. Extract i18n messages: +5. Extract i18n messages: .. code-block:: shell make gettext-create && make gettext-update && make gettext-compile -7. Run unit tests: + The compiled ``.mo`` catalogs are also generated automatically when the + package is built (sdist/wheel), so they always ship in a release. + +6. Run unit tests: .. code-block:: shell make test -8. Run coverage unit tests: +7. Run coverage unit tests: .. code-block:: shell diff --git a/hatch_build.py b/hatch_build.py index 685a611..61ea4d4 100644 --- a/hatch_build.py +++ b/hatch_build.py @@ -14,6 +14,7 @@ from hatchling.builders.hooks.plugin.interface import BuildHookInterface from pathlib import Path + LOCALES = Path("src/pas/plugins/ldap/locales") diff --git a/pyproject.toml b/pyproject.toml index 5eacab7..15be107 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "pas.plugins.ldap" -version = "2.0.0.dev0" +dynamic = ["version"] description = "LDAP/AD Plugin for Plone/Zope PluggableAuthService (users+groups)" readme = "README.rst" license = { text = "GPL 2.0" } @@ -70,9 +70,13 @@ Homepage = "https://github.com/collective/pas.plugins.ldap/" "Source Code" = "https://github.com/collective/pas.plugins.ldap" [build-system] -requires = ["hatchling", "babel"] +requires = ["hatchling", "hatch-vcs", "babel"] build-backend = "hatchling.build" +[tool.hatch.version] +source = "vcs" +raw-options = { local_scheme = "no-local-version" } + [tool.hatch.build.hooks.custom] path = "hatch_build.py" @@ -90,6 +94,51 @@ testpaths = [ "tests", ] -[tool.isort] -profile = "plone" +[tool.ruff] +target-version = "py310" + +[tool.ruff.lint] +select = [ + "A", + "B", + "C4", + "E", + "F", + "I", + "RUF", + "SIM", + "T20", + "UP", + "W", +] +ignore = [ + "E501", + "RUF001", + "RUF002", + # Zope/Plone idioms: builtins like ``id``/``type``/``property`` are + # pervasive as parameter and variable names in the PAS/CMF APIs. + "A001", + "A002", + "A003", + # Persistent / Acquisition classes use plain mutable class attributes + # (security declarations, property lists) and not ``typing.ClassVar``. + "RUF012", +] + +[tool.ruff.lint.isort] +force-single-line = true +from-first = true +lines-after-imports = 2 +lines-between-types = 1 +no-sections = true +order-by-type = false + +[tool.ruff.lint.pycodestyle] +max-line-length = 120 +max-doc-length = 120 + +[tool.ruff.lint.per-file-ignores] +# Tests use print() for diagnostics and deliberately nested ``with patch(...)`` +# blocks for readability of layered mocks. +"tests/*" = ["T20", "SIM117"] diff --git a/src/pas/plugins/ldap/cache.py b/src/pas/plugins/ldap/cache.py index 1fb0dc0..1c03782 100644 --- a/src/pas/plugins/ldap/cache.py +++ b/src/pas/plugins/ldap/cache.py @@ -12,7 +12,7 @@ from zope.globalrequest import getRequest from zope.interface import implementer -import re +import contextlib import threading import time @@ -200,7 +200,5 @@ def set(self, value): def invalidate(self): """Invalidate the cache by removing the cached value from the context.""" - try: + with contextlib.suppress(AttributeError): delattr(self.context, self._key) - except AttributeError: - pass diff --git a/src/pas/plugins/ldap/defaults.py b/src/pas/plugins/ldap/defaults.py index 4ac3ae8..2e4ea9d 100644 --- a/src/pas/plugins/ldap/defaults.py +++ b/src/pas/plugins/ldap/defaults.py @@ -2,6 +2,7 @@ from node.ext.ldap.scope import ONELEVEL + DEFAULTS = { "server.uri": "ldap://127.0.0.1:12345", "server.user": "cn=Manager,dc=my-domain,dc=com", diff --git a/src/pas/plugins/ldap/interfaces.py b/src/pas/plugins/ldap/interfaces.py index d7adaf9..6a7174b 100644 --- a/src/pas/plugins/ldap/interfaces.py +++ b/src/pas/plugins/ldap/interfaces.py @@ -13,7 +13,7 @@ class ICacheSettingsRecordProvider(Interface): """ -VALUE_NOT_CACHED = dict() +VALUE_NOT_CACHED = {} class IPluginCacheHandler(Interface): diff --git a/src/pas/plugins/ldap/locales/__main__.py b/src/pas/plugins/ldap/locales/__main__.py index b644f73..76bffd1 100644 --- a/src/pas/plugins/ldap/locales/__main__.py +++ b/src/pas/plugins/ldap/locales/__main__.py @@ -7,6 +7,7 @@ import re import subprocess + logger = logging.getLogger("i18n") logger.setLevel(logging.DEBUG) @@ -28,8 +29,8 @@ def i18n_script_setup(): """Setup the i18n scripts""" cmd_i18ndude = "uvx i18ndude" cmd_lingua = "uvx lingua" - subprocess.call(cmd_i18ndude, shell=True) # noQA: S602 - subprocess.call(cmd_lingua, shell=True) # noQA: S602 + subprocess.call(cmd_i18ndude, shell=True) + subprocess.call(cmd_lingua, shell=True) def locale_folder_setup(domain: str): @@ -51,7 +52,7 @@ def locale_folder_setup(domain: str): f"--input={locale_path}/{domain}.pot " f"--output={locale_path}/{lang}/LC_MESSAGES/{domain}.po" ) - subprocess.call(cmd, shell=True) # noQA: S602 + subprocess.call(cmd, shell=True) def _rebuild(domain: str): @@ -65,7 +66,7 @@ def _rebuild(domain: str): f"--exclude {excludes} " f"--create {domain} {target_path} {target_path}/plonecontrolpanel" ) - subprocess.call(cmd, shell=True) # noQA: S602 + subprocess.call(cmd, shell=True) def _rebuild_pot_to_merge(): @@ -74,7 +75,7 @@ def _rebuild_pot_to_merge(): f"{lingua} {target_path}/properties.yaml " f"--output {locale_path}/merge-lingua.pot" ) - subprocess.call(cmd, shell=True) # noQA: S602 + subprocess.call(cmd, shell=True) def _merge(domain: str): @@ -87,7 +88,7 @@ def _merge(domain: str): f"{i18ndude} merge --pot {locale_path}/{domain}.pot " f"--merge {locale_path}/merge-lingua.pot" ) - subprocess.call(cmd, shell=True) # noQA: S602 + subprocess.call(cmd, shell=True) def _sync(domain: str): @@ -100,7 +101,7 @@ def _sync(domain: str): f"{i18ndude} sync --pot {locale_path}/{domain}.pot " f"{locale_path}/*/LC_MESSAGES/{domain}.po" ) - subprocess.call(cmd, shell=True) # noQA: S602 + subprocess.call(cmd, shell=True) def main(): diff --git a/src/pas/plugins/ldap/monkey.py b/src/pas/plugins/ldap/monkey.py index 47b4684..da2552b 100644 --- a/src/pas/plugins/ldap/monkey.py +++ b/src/pas/plugins/ldap/monkey.py @@ -24,7 +24,7 @@ def getPhysicalPath(self): trav = f"++portrait++{self.id()}" if not hasattr(parent, "getPhysicalPath"): return ("", trav) - return tuple(list(parent.getPhysicalPath()) + [trav]) + return (*list(parent.getPhysicalPath()), trav) def getPortraitFromSheet(context, userid): @@ -89,10 +89,13 @@ def patched_getPersonalPortrait(self, id=None, verifyPermission=0): portrait = membertool._getPortrait(safe_id) if isinstance(portrait, str): portrait = None - if portrait is not None: - if verifyPermission and not _checkPermission("View", portrait): - # Don't return the portrait if the user can't get to it - portrait = None + if ( + portrait is not None + and verifyPermission + and not _checkPermission("View", portrait) + ): + # Don't return the portrait if the user can't get to it + portrait = None if portrait is None: portal = getToolByName(self, "portal_url").getPortalObject() portrait = getattr(portal, default_portrait, None) diff --git a/src/pas/plugins/ldap/plonecontrolpanel/cache.py b/src/pas/plugins/ldap/plonecontrolpanel/cache.py index 7015df3..449dadc 100644 --- a/src/pas/plugins/ldap/plonecontrolpanel/cache.py +++ b/src/pas/plugins/ldap/plonecontrolpanel/cache.py @@ -9,6 +9,7 @@ from zope.component import queryUtility from zope.interface import implementer + REGKEY = "pas.plugins.ldap.memcached" diff --git a/src/pas/plugins/ldap/plonecontrolpanel/controlpanel.py b/src/pas/plugins/ldap/plonecontrolpanel/controlpanel.py index 1f05804..cb8adff 100644 --- a/src/pas/plugins/ldap/plonecontrolpanel/controlpanel.py +++ b/src/pas/plugins/ldap/plonecontrolpanel/controlpanel.py @@ -33,11 +33,11 @@ def plugin(self): # "'RequestContainer' object has no attribute 'pasldap'" error. try: return aclu["pasldap"] - except KeyError: + except KeyError as err: raise AttributeError( "Plugin 'pasldap' not found in acl_users. " "Install pas.plugins.ldap via Plone Add-ons first." - ) + ) from err def save(self, widget, data): """Save the LDAP setting. diff --git a/src/pas/plugins/ldap/plonecontrolpanel/exportimport.py b/src/pas/plugins/ldap/plonecontrolpanel/exportimport.py index c018f3d..a31ad10 100644 --- a/src/pas/plugins/ldap/plonecontrolpanel/exportimport.py +++ b/src/pas/plugins/ldap/plonecontrolpanel/exportimport.py @@ -129,9 +129,7 @@ def _setDataAndType(self, data, node): node.setAttribute("type", "string") else: self._logger.warning( - "Invalid type {:s} found for key {:s} on export, skipped.".format( - type(data), data - ) + f"Invalid type {type(data):s} found for key {data:s} on export, skipped." ) return child = self._doc.createTextNode(data) @@ -141,14 +139,14 @@ def _getDataByType(self, node): """Get the data from an XML node based on its type attribute.""" vtype = node.getAttribute("type") if vtype == "list": - data = list() + data = [] for element in node.childNodes: if element.nodeName != "element": continue data.append(self._getDataByType(element)) return data if vtype == "dict": - data = dict() + data = {} for element in node.childNodes: if element.nodeName != "element": continue diff --git a/src/pas/plugins/ldap/plonecontrolpanel/inspector.py b/src/pas/plugins/ldap/plonecontrolpanel/inspector.py index 740e182..4557b77 100644 --- a/src/pas/plugins/ldap/plonecontrolpanel/inspector.py +++ b/src/pas/plugins/ldap/plonecontrolpanel/inspector.py @@ -50,15 +50,13 @@ def node_attributes(self): baseDN = groups.baseDN root = LDAPNode(baseDN, self.props) node = root.node_by_dn(safe_unicode(dn), strict=True) - ret = dict() + ret = {} for key, val in node.attrs.items(): try: if not node.attrs.is_binary(key): ret[safe_unicode(key)] = safe_unicode(val) else: - ret[safe_unicode(key)] = "(Binary Data with {} Bytes)".format( - len(val) - ) + ret[safe_unicode(key)] = f"(Binary Data with {len(val)} Bytes)" except UnicodeDecodeError: ret[safe_unicode(key)] = "! (UnicodeDecodeError)" except Exception: @@ -68,7 +66,7 @@ def node_attributes(self): def children(self, baseDN): """Get the children of the LDAP node with the given base DN.""" node = LDAPNode(baseDN, self.props) - ret = list() + ret = [] # XXX: related search filters for users and groups container? for dn in node.search(): ret.append({"dn": dn}) diff --git a/src/pas/plugins/ldap/plonecontrolpanel/setuphandlers.py b/src/pas/plugins/ldap/plonecontrolpanel/setuphandlers.py index 117491f..9b55e90 100644 --- a/src/pas/plugins/ldap/plonecontrolpanel/setuphandlers.py +++ b/src/pas/plugins/ldap/plonecontrolpanel/setuphandlers.py @@ -4,8 +4,10 @@ from pas.plugins.ldap.plugin import LDAPPlugin from zope.component.hooks import getSite +import contextlib import logging + logger = logging.getLogger(PACKAGE_NAME) @@ -29,11 +31,9 @@ def _removePlugin(pas, PLUGIN_ID="pasldap"): interface = info["interface"] if not interface.providedBy(plugin): continue - try: + # the plugin may not be active + with contextlib.suppress(KeyError): pas.plugins.deactivatePlugin(interface, plugin.getId()) - except KeyError: - # the plugin was not active - pass pas._delObject(PLUGIN_ID) logger.info("Removed LDAPPlugin %s from acl_users.", PLUGIN_ID) diff --git a/src/pas/plugins/ldap/plugin.py b/src/pas/plugins/ldap/plugin.py index 7f9674c..227fc12 100644 --- a/src/pas/plugins/ldap/plugin.py +++ b/src/pas/plugins/ldap/plugin.py @@ -25,6 +25,7 @@ import os import time + logger = logging.getLogger("pas.plugins.ldap") zmidir = os.path.join(os.path.dirname(__file__), "zmi") @@ -72,12 +73,7 @@ def _wrapper(self, *args, **kwargs): waiting = time.time() - self._v_ldaperror_timeout if waiting < LDAP_ERROR_LOG_TIMEOUT: logger.debug( - "{}: retry wait {:0.5f} of {:0.0f}s -> {}".format( - prefix, - waiting, - LDAP_ERROR_LOG_TIMEOUT, - self._v_ldaperror_msg, - ) + f"{prefix}: retry wait {waiting:0.5f} of {LDAP_ERROR_LOG_TIMEOUT:0.0f}s -> {self._v_ldaperror_msg}" ) return default try: @@ -132,7 +128,8 @@ class LDAPPlugin(BasePlugin): meta_type = "LDAP Plugin" manage_options = ( {"label": "LDAP Settings", "action": "manage_ldapplugin"}, - ) + BasePlugin.manage_options + *BasePlugin.manage_options, + ) # Tell PAS not to swallow our exceptions _dont_swallow_my_exceptions = False @@ -203,7 +200,7 @@ def ldaperror(self): if hasattr(self, "_v_ldaperror_msg"): waiting = time.time() - self._v_ldaperror_timeout if waiting < LDAP_ERROR_LOG_TIMEOUT: - return self._v_ldaperror_msg + " (for %0.2fs)" % waiting + return self._v_ldaperror_msg + f" (for {waiting:0.2f}s)" return False @security.public # really public?? @@ -236,13 +233,13 @@ def authenticateCredentials(self, credentials): pw = credentials.get("password") if not (login and pw): return default - logger.debug("login: %s" % login) + logger.debug(f"login: {login}") users = self.users if not users: return default userid = users.authenticate(login, pw) if userid: - logger.info("logged in %s" % userid) + logger.info(f"logged in {userid}") return (userid, login) return default @@ -313,7 +310,7 @@ def enumerateGroups( if sort_by == "id": matches = sorted(matches) pluginid = self.getId() - ret = [dict(id=_id, pluginid=pluginid) for _id in matches] + ret = [{"id": _id, "pluginid": pluginid} for _id in matches] if max_results and len(ret) > max_results: ret = ret[:max_results] return ret @@ -331,7 +328,7 @@ def getGroupsForPrincipal(self, principal, request=None): o May assign groups based on values in the REQUEST object, if present """ - default = tuple() + default = () if not self.is_plugin_active(pas_interfaces.IGroupsPlugin): return default users = self.users @@ -355,7 +352,7 @@ def getGroupsForPrincipal(self, principal, request=None): # # Allow querying users by ID, and searching for users. # - @ldap_error_handler("enumerateUsers", default=tuple()) + @ldap_error_handler("enumerateUsers", default=()) @security.private def enumerateUsers( self, @@ -406,7 +403,7 @@ def enumerateUsers( o Insufficiently-specified criteria may have catastrophic scaling issues for some implementations. """ - default = tuple() + default = () if not self.is_plugin_active(pas_interfaces.IUserEnumerationPlugin): return default # XXX: sort_by in node.ext.ldap @@ -439,7 +436,7 @@ def enumerateUsers( except ValueError: return default pluginid = self.getId() - ret = list() + ret = [] for id_, attrs in matches: ret.append({"id": id_, "login": attrs["login"][0], "pluginid": pluginid}) if max_results and len(ret) > max_results: @@ -723,7 +720,9 @@ def getGroupById(self, group_id): # add subgroups group._addGroups(pas._getGroupsForPrincipal(group, None, plugins=plugins)) # add roles - for rolemaker_id, rolemaker in plugins.listPlugins(pas_interfaces.IRolesPlugin): + for _rolemaker_id, rolemaker in plugins.listPlugins( + pas_interfaces.IRolesPlugin + ): roles = rolemaker.getRolesForPrincipal(group, None) if not roles: continue @@ -748,7 +747,7 @@ def getGroupIds(self): default = [] if not self.is_plugin_active(plonepas_interfaces.group.IGroupIntrospection): return default - return self.groups and self.groups.ids or default + return (self.groups and self.groups.ids) or default @security.private def getGroupMembers(self, group_id): diff --git a/src/pas/plugins/ldap/properties.py b/src/pas/plugins/ldap/properties.py index 255ef6b..5dfb2d0 100644 --- a/src/pas/plugins/ldap/properties.py +++ b/src/pas/plugins/ldap/properties.py @@ -26,7 +26,8 @@ import ldap -_marker = dict() + +_marker = {} class BasePropertiesForm(YAMLBaseForm): @@ -46,8 +47,8 @@ class BasePropertiesForm(YAMLBaseForm): # for the expiration unit of user accounts. The values represent the number # of seconds in the respective unit. account_expiration_unit_vocab = [ - (int(0), _("Days since Epoch")), - (int(1), _("Seconds since epoch")), + (0, _("Days since Epoch")), + (1, _("Seconds since epoch")), ] static_attrs_users = ["rdn", "id", "login"] static_attrs_groups = ["rdn", "id"] @@ -128,7 +129,7 @@ def save(self, widget, data): groups = ILDAPGroupsConfig(self.plugin) def fetch(name, default=UNSET): - name = "ldapsettings.%s" % name + name = f"ldapsettings.{name}" __traceback_info__ = name val = data.fetch(name).extracted if default is UNSET: @@ -383,7 +384,7 @@ def __init__(self, plugin): self.plugin = plugin strict = False - defaults = dict() + defaults = {} baseDN = propproxy("users.baseDN") attrmap = propproxy("users.attrmap") scope = propproxy("users.scope") @@ -404,7 +405,7 @@ def expiresAttr(self): Returns: str: The expiration attribute. """ - return self.account_expiration and self._expiresAttr or None + return (self.account_expiration and self._expiresAttr) or None @property def expiresUnit(self): @@ -413,7 +414,7 @@ def expiresUnit(self): Returns: int: The expiration unit. """ - return self.account_expiration and self._expiresUnit or 0 + return (self.account_expiration and self._expiresUnit) or 0 @implementer(ILDAPGroupsConfig) @@ -425,7 +426,7 @@ def __init__(self, plugin): self.plugin = plugin strict = False - defaults = dict() + defaults = {} baseDN = propproxy("groups.baseDN") attrmap = propproxy("groups.attrmap") scope = propproxy("groups.scope") diff --git a/src/pas/plugins/ldap/setuphandlers.py b/src/pas/plugins/ldap/setuphandlers.py index 2a855a0..efcaac0 100644 --- a/src/pas/plugins/ldap/setuphandlers.py +++ b/src/pas/plugins/ldap/setuphandlers.py @@ -6,6 +6,7 @@ import logging + logger = logging.getLogger(PACKAGE_NAME) TITLE = f"LDAP plugin ({PACKAGE_NAME})" diff --git a/src/pas/plugins/ldap/sheet.py b/src/pas/plugins/ldap/sheet.py index e13a50b..664de89 100644 --- a/src/pas/plugins/ldap/sheet.py +++ b/src/pas/plugins/ldap/sheet.py @@ -10,6 +10,7 @@ import logging + logger = logging.getLogger("pas.plugins.ldap") @@ -30,8 +31,8 @@ def __init__(self, principal, plugin): """ # do not set any non-pickable attribute here, i.e. acquisition wrapped self._plugin = aq_base(plugin) - self._properties = dict() - self._attrmap = dict() + self._properties = {} + self._attrmap = {} self._ldapprincipal_id = principal.getId() if self._ldapprincipal_id in plugin.users: pcfg = ILDAPUsersConfig(plugin) @@ -79,7 +80,7 @@ def setProperty(self, obj, id, value): ldapprincipal.context() except Exception as e: # XXX: specific exception(s) - logger.error("LDAPUserPropertySheet.setProperty: %s" % str(e)) + logger.error(f"LDAPUserPropertySheet.setProperty: {e!s}") def setProperties(self, obj, mapping): """Set multiple property values.""" @@ -92,4 +93,4 @@ def setProperties(self, obj, mapping): ldapprincipal.context() except Exception as e: # XXX: specific exception(s) - logger.error("LDAPUserPropertySheet.setProperties: %s" % str(e)) + logger.error(f"LDAPUserPropertySheet.setProperties: {e!s}") diff --git a/src/pas/plugins/ldap/zmi/manage_plugin.py b/src/pas/plugins/ldap/zmi/manage_plugin.py index b6a7893..84043ad 100644 --- a/src/pas/plugins/ldap/zmi/manage_plugin.py +++ b/src/pas/plugins/ldap/zmi/manage_plugin.py @@ -7,4 +7,4 @@ def plugin(self): return self.context def next(self, request): - return "%s/manage_ldapplugin" % self.context.absolute_url() + return f"{self.context.absolute_url()}/manage_ldapplugin" diff --git a/tests/conftest.py b/tests/conftest.py index 198462d..9e3a665 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,15 +1,8 @@ -from testing import PASLDAP_FIXTURE from pytest_plone import fixtures_factory +from testing import PASLDAP_FIXTURE pytest_plugins = ["pytest_plone"] -globals().update( - fixtures_factory( - ( - (PASLDAP_FIXTURE, "ldap"), - - ) - ) -) \ No newline at end of file +globals().update(fixtures_factory(((PASLDAP_FIXTURE, "ldap"),))) diff --git a/tests/test_cache.py b/tests/test_cache.py index 336338f..f0c763e 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -1,8 +1,10 @@ """Unit tests for pas.plugins.ldap.cache module.""" +from unittest.mock import MagicMock +from unittest.mock import patch + import time import unittest -from unittest.mock import MagicMock, patch class TestCacheProviderFactoryServers(unittest.TestCase): @@ -43,7 +45,8 @@ class TestVolatilePluginCache(unittest.TestCase): def test_get_returns_not_cached_when_expired(self): """get() returns VALUE_NOT_CACHED when the cached entry has expired.""" - from pas.plugins.ldap.cache import VOLATILE_CACHE_MAXAGE, VolatilePluginCache + from pas.plugins.ldap.cache import VOLATILE_CACHE_MAXAGE + from pas.plugins.ldap.cache import VolatilePluginCache from pas.plugins.ldap.interfaces import VALUE_NOT_CACHED context = MagicMock() diff --git a/tests/test_doctests.py b/tests/test_doctests.py index 65fa19d..1a41967 100644 --- a/tests/test_doctests.py +++ b/tests/test_doctests.py @@ -1,13 +1,11 @@ """Test suite for pas.plugins.ldap doctests.""" -from testing import PASLDAPLayer from plone.testing import layered from plone.testing import zope +from testing import PASLDAPLayer import doctest import pprint -import re -import six import unittest diff --git a/tests/test_monkey_unit.py b/tests/test_monkey_unit.py index a92e660..ff67434 100644 --- a/tests/test_monkey_unit.py +++ b/tests/test_monkey_unit.py @@ -3,15 +3,14 @@ 100% coverage without Zope/LDAP layer. """ -import unittest -from unittest.mock import MagicMock, call, patch +from pas.plugins.ldap.monkey import getPortraitFromSheet +from pas.plugins.ldap.monkey import patched_getPersonalPortrait +from pas.plugins.ldap.monkey import PortraitImage +from pas.plugins.ldap.monkey import PortraitTraverser +from unittest.mock import MagicMock +from unittest.mock import patch -from pas.plugins.ldap.monkey import ( - PortraitImage, - PortraitTraverser, - getPortraitFromSheet, - patched_getPersonalPortrait, -) +import unittest # --------------------------------------------------------------------------- @@ -35,8 +34,10 @@ def test_parent_with_getphysicalpath_appends_traversal(self): portrait = self._make_portrait("uid0") mock_parent = MagicMock() mock_parent.getPhysicalPath.return_value = ("", "plone", "acl_users") - with patch("pas.plugins.ldap.monkey.aq_inner", return_value=portrait), \ - patch("pas.plugins.ldap.monkey.aq_parent", return_value=mock_parent): + with ( + patch("pas.plugins.ldap.monkey.aq_inner", return_value=portrait), + patch("pas.plugins.ldap.monkey.aq_parent", return_value=mock_parent), + ): result = portrait.getPhysicalPath() self.assertEqual(result, ("", "plone", "acl_users", "++portrait++uid0")) @@ -44,8 +45,10 @@ def test_parent_without_getphysicalpath_returns_root_trav(self): """When parent has no getPhysicalPath, returns ('', trav) (line 25).""" portrait = self._make_portrait("uid0") mock_parent = object() # plain object, no getPhysicalPath attr - with patch("pas.plugins.ldap.monkey.aq_inner", return_value=portrait), \ - patch("pas.plugins.ldap.monkey.aq_parent", return_value=mock_parent): + with ( + patch("pas.plugins.ldap.monkey.aq_inner", return_value=portrait), + patch("pas.plugins.ldap.monkey.aq_parent", return_value=mock_parent), + ): result = portrait.getPhysicalPath() self.assertEqual(result, ("", "++portrait++uid0")) @@ -64,7 +67,9 @@ def _member_not_found(self): mtool.getMemberById.return_value = None return mtool - def _member_with_sheet(self, property_ids, portrait_data=None, fullname="Test User"): + def _member_with_sheet( + self, property_ids, portrait_data=None, fullname="Test User" + ): """Return (mtool, mock_user) for a member with one property sheet.""" mock_sheet = MagicMock() mock_sheet.propertyIds.return_value = property_ids @@ -88,8 +93,10 @@ def _member_with_sheet(self, property_ids, portrait_data=None, fullname="Test Us def test_returns_none_when_member_not_found(self): """getMemberById returns falsy → return None (line 34).""" context = MagicMock() - with patch("pas.plugins.ldap.monkey.getToolByName", - return_value=self._member_not_found()): + with patch( + "pas.plugins.ldap.monkey.getToolByName", + return_value=self._member_not_found(), + ): result = getPortraitFromSheet(context, "uid0") self.assertIsNone(result) @@ -127,25 +134,30 @@ def test_returns_portrait_image_when_portrait_in_sheet(self): fullname="Full Name", ) mock_portrait_img = MagicMock() - with patch("pas.plugins.ldap.monkey.getToolByName", return_value=mtool), \ - patch("pas.plugins.ldap.monkey.PortraitImage", - return_value=mock_portrait_img), \ - patch("pas.plugins.ldap.monkey.BytesIO"): + with ( + patch("pas.plugins.ldap.monkey.getToolByName", return_value=mtool), + patch( + "pas.plugins.ldap.monkey.PortraitImage", return_value=mock_portrait_img + ), + patch("pas.plugins.ldap.monkey.BytesIO"), + ): result = getPortraitFromSheet(context, "uid0") self.assertIs(result, mock_portrait_img) def test_portrait_image_created_with_correct_args(self): """PortraitImage is constructed with userid, fullname, sio, content_type.""" context = MagicMock() - mtool, mock_user = self._member_with_sheet( + mtool, _mock_user = self._member_with_sheet( property_ids=["portrait"], portrait_data=b"DATA", fullname="Jane Doe", ) mock_sio = MagicMock() - with patch("pas.plugins.ldap.monkey.getToolByName", return_value=mtool), \ - patch("pas.plugins.ldap.monkey.PortraitImage") as MockPI, \ - patch("pas.plugins.ldap.monkey.BytesIO", return_value=mock_sio): + with ( + patch("pas.plugins.ldap.monkey.getToolByName", return_value=mtool), + patch("pas.plugins.ldap.monkey.PortraitImage") as MockPI, + patch("pas.plugins.ldap.monkey.BytesIO", return_value=mock_sio), + ): getPortraitFromSheet(context, "testuser") MockPI.assert_called_once_with("testuser", "Jane Doe", mock_sio, "image/jpeg") @@ -207,12 +219,14 @@ def test_traverse_raises_when_no_portrait(self): ctx = MagicMock() traverser = PortraitTraverser(ctx) - with patch( - "pas.plugins.ldap.monkey.getPortraitFromSheet", - return_value=None, + with ( + patch( + "pas.plugins.ldap.monkey.getPortraitFromSheet", + return_value=None, + ), + self.assertRaises(LocationError), ): - with self.assertRaises(LocationError): - traverser.traverse("uid0", []) + traverser.traverse("uid0", []) # --------------------------------------------------------------------------- @@ -279,9 +293,12 @@ def test_falls_back_to_memberdata_when_no_sheet_portrait(self): mock_membertool = MagicMock() mock_membertool._getPortrait.return_value = mock_portrait_obj - with patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), \ - patch("pas.plugins.ldap.monkey.getToolByName", - return_value=mock_membertool): + with ( + patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), + patch( + "pas.plugins.ldap.monkey.getToolByName", return_value=mock_membertool + ), + ): result = patched_getPersonalPortrait(mock_self, id="uid0") self.assertIs(result, mock_portrait_obj) @@ -297,13 +314,15 @@ def test_string_portrait_becomes_none_and_falls_back_to_portal(self): mock_portal_url_tool = MagicMock() mock_portal_url_tool.getPortalObject.return_value = mock_portal - with patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), \ - patch( - "pas.plugins.ldap.monkey.getToolByName", - side_effect=[mock_membertool, mock_portal_url_tool], - ), \ - patch("pas.plugins.ldap.monkey.default_portrait", "defaultUser.png"): - result = patched_getPersonalPortrait(mock_self, id="uid0") + with ( + patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), + patch( + "pas.plugins.ldap.monkey.getToolByName", + side_effect=[mock_membertool, mock_portal_url_tool], + ), + patch("pas.plugins.ldap.monkey.default_portrait", "defaultUser.png"), + ): + patched_getPersonalPortrait(mock_self, id="uid0") mock_portal_url_tool.getPortalObject.assert_called_once() @@ -316,12 +335,16 @@ def test_verify_permission_zero_skips_permission_check(self): mock_membertool = MagicMock() mock_membertool._getPortrait.return_value = mock_portrait_obj - with patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), \ - patch("pas.plugins.ldap.monkey.getToolByName", - return_value=mock_membertool), \ - patch("pas.plugins.ldap.monkey._checkPermission") as mock_check: - result = patched_getPersonalPortrait(mock_self, id="uid0", - verifyPermission=0) + with ( + patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), + patch( + "pas.plugins.ldap.monkey.getToolByName", return_value=mock_membertool + ), + patch("pas.plugins.ldap.monkey._checkPermission") as mock_check, + ): + result = patched_getPersonalPortrait( + mock_self, id="uid0", verifyPermission=0 + ) mock_check.assert_not_called() self.assertIs(result, mock_portrait_obj) @@ -336,14 +359,15 @@ def test_verify_permission_denied_hides_portrait(self): mock_portal_url_tool = MagicMock() mock_portal_url_tool.getPortalObject.return_value = mock_portal - with patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), \ - patch( - "pas.plugins.ldap.monkey.getToolByName", - side_effect=[mock_membertool, mock_portal_url_tool], - ), \ - patch("pas.plugins.ldap.monkey._checkPermission", return_value=False): - result = patched_getPersonalPortrait(mock_self, id="uid0", - verifyPermission=1) + with ( + patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), + patch( + "pas.plugins.ldap.monkey.getToolByName", + side_effect=[mock_membertool, mock_portal_url_tool], + ), + patch("pas.plugins.ldap.monkey._checkPermission", return_value=False), + ): + patched_getPersonalPortrait(mock_self, id="uid0", verifyPermission=1) mock_portal_url_tool.getPortalObject.assert_called_once() @@ -354,12 +378,16 @@ def test_verify_permission_granted_returns_portrait(self): mock_membertool = MagicMock() mock_membertool._getPortrait.return_value = mock_portrait_obj - with patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), \ - patch("pas.plugins.ldap.monkey.getToolByName", - return_value=mock_membertool), \ - patch("pas.plugins.ldap.monkey._checkPermission", return_value=True): - result = patched_getPersonalPortrait(mock_self, id="uid0", - verifyPermission=1) + with ( + patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), + patch( + "pas.plugins.ldap.monkey.getToolByName", return_value=mock_membertool + ), + patch("pas.plugins.ldap.monkey._checkPermission", return_value=True), + ): + result = patched_getPersonalPortrait( + mock_self, id="uid0", verifyPermission=1 + ) self.assertIs(result, mock_portrait_obj) @@ -376,12 +404,14 @@ def test_fallback_to_portal_default_when_portrait_is_none(self): mock_portal_url_tool = MagicMock() mock_portal_url_tool.getPortalObject.return_value = mock_portal - with patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), \ - patch( - "pas.plugins.ldap.monkey.getToolByName", - side_effect=[mock_membertool, mock_portal_url_tool], - ), \ - patch("pas.plugins.ldap.monkey.default_portrait", "defaultUser.png"): + with ( + patch("pas.plugins.ldap.monkey.getPortraitFromSheet", return_value=None), + patch( + "pas.plugins.ldap.monkey.getToolByName", + side_effect=[mock_membertool, mock_portal_url_tool], + ), + patch("pas.plugins.ldap.monkey.default_portrait", "defaultUser.png"), + ): result = patched_getPersonalPortrait(mock_self, id="uid0") mock_portal_url_tool.getPortalObject.assert_called_once() diff --git a/tests/test_plonecontrolpanel.py b/tests/test_plonecontrolpanel.py index 9ee432f..8094760 100644 --- a/tests/test_plonecontrolpanel.py +++ b/tests/test_plonecontrolpanel.py @@ -1,8 +1,9 @@ """Unit tests for pas.plugins.ldap.plonecontrolpanel modules.""" -import unittest from unittest.mock import patch +import unittest + class TestHiddenProfiles(unittest.TestCase): """Tests for HiddenProfiles — uncovered branches (return statements).""" diff --git a/tests/test_plugin.py b/tests/test_plugin.py index a674800..09d6d9d 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1,7 +1,7 @@ """Unit tests for pas.plugins.ldap.""" -from testing import PASLDAP_FIXTURE from Products.PlonePAS.plugins.ufactory import PloneUser +from testing import PASLDAP_FIXTURE from unittest.mock import MagicMock import unittest @@ -25,6 +25,7 @@ def test_initialize_registers_plugin(self): class TestPluginInit(unittest.TestCase): """Tests for the initialization of the LDAPPlugin.""" + layer = PASLDAP_FIXTURE @property @@ -50,6 +51,7 @@ def test_create(self): class TestPluginFeatures(unittest.TestCase): """Tests for the features of the LDAPPlugin.""" + layer = PASLDAP_FIXTURE @property diff --git a/tests/test_plugin_unit.py b/tests/test_plugin_unit.py index b992423..3c494a6 100644 --- a/tests/test_plugin_unit.py +++ b/tests/test_plugin_unit.py @@ -4,28 +4,26 @@ tests, using unittest.mock so no real LDAP server or Zope layer is needed. """ -import time -import unittest -from unittest.mock import MagicMock, patch, PropertyMock +from pas.plugins.ldap.interfaces import VALUE_NOT_CACHED +from pas.plugins.ldap.plugin import ldap_error_handler +from pas.plugins.ldap.plugin import LDAP_ERROR_LOG_TIMEOUT +from pas.plugins.ldap.plugin import LDAP_LONG_RUNNING_LOG_THRESHOLD +from pas.plugins.ldap.plugin import LDAPPlugin +from pas.plugins.ldap.plugin import manage_addLDAPPlugin +from unittest.mock import MagicMock +from unittest.mock import patch +from unittest.mock import PropertyMock import ldap - -from pas.plugins.ldap.plugin import ( - LDAP_ERROR_LOG_TIMEOUT, - LDAP_LONG_RUNNING_LOG_THRESHOLD, - LDAPPlugin, - ldap_error_handler, - manage_addLDAPPlugin, -) -from pas.plugins.ldap.interfaces import VALUE_NOT_CACHED -from Products.PluggableAuthService.interfaces import plugins as pas_interfaces -from Products.PlonePAS import interfaces as plonepas_interfaces +import time +import unittest # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- + def _make_plugin(plugin_id="testplugin", title="Test Plugin"): """Return a minimal LDAPPlugin bypassing Zope/LDAP initialization.""" from BTrees import OOBTree @@ -46,6 +44,7 @@ class _Dummy: # manage_addLDAPPlugin (lines 52-55) # --------------------------------------------------------------------------- + class TestManageAddLDAPPlugin(unittest.TestCase): """Tests for the module-level manage_addLDAPPlugin factory function.""" @@ -80,6 +79,7 @@ def test_no_redirect_when_response_is_none(self): # ldap_error_handler (lines 93, 102-106) # --------------------------------------------------------------------------- + class TestLdapErrorHandler(unittest.TestCase): """Tests for the ldap_error_handler decorator factory.""" @@ -134,7 +134,7 @@ def good_op(self): return "result" obj = _Dummy() - obj._v_ldaperror_timeout = time.time() # very recent error + obj._v_ldaperror_timeout = time.time() # very recent error obj._v_ldaperror_msg = "previous failure" with patch("pas.plugins.ldap.plugin.logger"): result = good_op(obj) @@ -145,6 +145,7 @@ def good_op(self): # groups_enabled / users_enabled (lines 161, 167) # --------------------------------------------------------------------------- + class TestGroupsUsersEnabled(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -178,6 +179,7 @@ def test_users_enabled_false_when_users_is_none(self): # ldaperror property (lines 204-208) # --------------------------------------------------------------------------- + class TestLdaperror(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -185,7 +187,7 @@ def setUp(self): def test_ldaperror_returns_message_when_recent_error(self): """Returns formatted message string when error is within timeout (204-207).""" self.plugin._v_ldaperror_msg = "connection refused" - self.plugin._v_ldaperror_timeout = time.time() # very recent + self.plugin._v_ldaperror_timeout = time.time() # very recent result = self.plugin.ldaperror self.assertIn("connection refused", result) self.assertIn("for", result) @@ -207,17 +209,19 @@ def test_ldaperror_returns_false_when_error_expired(self): # reset (line 214) # --------------------------------------------------------------------------- + class TestReset(unittest.TestCase): def test_reset_executes_without_error(self): """reset() passes silently (line 214).""" plugin = _make_plugin() - plugin.reset() # should not raise + plugin.reset() # should not raise # --------------------------------------------------------------------------- # authenticateCredentials (lines 235, 239, 243, 245-248) # --------------------------------------------------------------------------- + class TestAuthenticateCredentials(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -273,6 +277,7 @@ def test_returns_none_on_failed_authentication(self): # enumerateGroups (lines 300, 303, 307, 313-320) # --------------------------------------------------------------------------- + class TestEnumerateGroups(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -348,6 +353,7 @@ def test_returns_pluginid_in_each_item(self): # getGroupsForPrincipal (lines 337, 341-352) # --------------------------------------------------------------------------- + class TestGetGroupsForPrincipal(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -409,6 +415,7 @@ def test_returns_group_ids_on_success(self): # enumerateUsers (lines 412, 417, 421, 425, 429, 441-448) # --------------------------------------------------------------------------- + class TestEnumerateUsers(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -499,6 +506,7 @@ def test_limits_results_by_max_results(self): # getRolesForPrincipal (lines 466, 468) # --------------------------------------------------------------------------- + class TestGetRolesForPrincipal(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -548,6 +556,7 @@ def test_returns_roles_when_user_exists(self): # updateUser / updateEveryLoginName (lines 485, 501) # --------------------------------------------------------------------------- + class TestUpdateMethods(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -569,6 +578,7 @@ def test_update_every_login_name_returns_none(self): # deleteUser (line 615) # --------------------------------------------------------------------------- + class TestPropertiesMethods(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -638,6 +648,7 @@ def test_deleteUser_does_nothing(self): # doAddUser / doChangeUser / doDeleteUser (lines 629, 641-643, 654) # --------------------------------------------------------------------------- + class TestUserManagementMethods(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -653,9 +664,11 @@ def test_doChangeUser_raises_runtime_error_when_user_not_found(self): mock_users.passwd.side_effect = KeyError("uid_unknown") with patch.object(type(self.plugin), "users", new_callable=PropertyMock) as pu: pu.return_value = mock_users - with patch("pas.plugins.ldap.plugin.logger"): - with self.assertRaises(RuntimeError) as ctx: - self.plugin.doChangeUser("uid_unknown", "newpass") + with ( + patch("pas.plugins.ldap.plugin.logger"), + self.assertRaises(RuntimeError) as ctx, + ): + self.plugin.doChangeUser("uid_unknown", "newpass") self.assertIn("uid_unknown", str(ctx.exception)) def test_doDeleteUser_returns_false(self): @@ -668,6 +681,7 @@ def test_doDeleteUser_returns_false(self): # getGroupById (lines 700, 702, 704, 707-729) # --------------------------------------------------------------------------- + class TestGetGroupById(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -730,7 +744,7 @@ def test_returns_plone_group_when_found(self): with patch("pas.plugins.ldap.plugin.PloneGroup") as MockPloneGroup: mock_group = MagicMock() MockPloneGroup.return_value.__of__ = MagicMock(return_value=mock_group) - result = self.plugin.getGroupById("grp1") + self.plugin.getGroupById("grp1") # Should have tried to create a PloneGroup MockPloneGroup.assert_called_once_with("grp1", "Group One") @@ -753,7 +767,7 @@ def test_adds_properties_to_group(self): # Return one propfinder and no role finder mock_plugins.listPlugins.side_effect = [ [("propfinder1", mock_propfinder)], # IPropertiesPlugin - [], # IRolesPlugin + [], # IRolesPlugin ] mock_pas.plugins = mock_plugins mock_pas._getGroupsForPrincipal.return_value = [] @@ -765,7 +779,9 @@ def test_adds_properties_to_group(self): mock_group = MagicMock() MockPloneGroup.return_value.__of__ = MagicMock(return_value=mock_group) self.plugin.getGroupById("grp1") - mock_group.addPropertysheet.assert_called_once_with("propfinder1", mock_propdata) + mock_group.addPropertysheet.assert_called_once_with( + "propfinder1", mock_propdata + ) def test_adds_roles_to_group(self): """Iterates role makers and adds non-empty roles (line 728).""" @@ -783,8 +799,8 @@ def test_adds_roles_to_group(self): mock_pas = MagicMock() mock_plugins = MagicMock() mock_plugins.listPlugins.side_effect = [ - [], # IPropertiesPlugin - [("rolemaker1", mock_rolemaker)], # IRolesPlugin + [], # IPropertiesPlugin + [("rolemaker1", mock_rolemaker)], # IRolesPlugin ] mock_pas.plugins = mock_plugins mock_pas._getGroupsForPrincipal.return_value = [] @@ -803,6 +819,7 @@ def test_adds_roles_to_group(self): # getGroupIds (line 748) # --------------------------------------------------------------------------- + class TestGetGroupIds(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -829,6 +846,7 @@ def test_returns_group_ids_when_active(self): # getGroupMembers (lines 758, 762-763) # --------------------------------------------------------------------------- + class TestGetGroupMembers(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -866,6 +884,7 @@ def test_returns_member_ids_tuple(self): # allowPasswordSet (lines 774, 777, 779) # --------------------------------------------------------------------------- + class TestAllowPasswordSet(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -909,6 +928,7 @@ def test_returns_false_on_value_error(self): # is_plugin_active real body (lines 153-155) # --------------------------------------------------------------------------- + class TestIsPluginActiveRealBody(unittest.TestCase): def test_returns_true_when_plugin_id_in_list(self): """Real method: returns True when getId() is in listPluginIds (153-155).""" @@ -935,6 +955,7 @@ def test_returns_false_when_plugin_id_not_in_list(self): # _ldap_props property (line 172) # --------------------------------------------------------------------------- + class TestLdapPropsProperty(unittest.TestCase): def test_returns_ildapprops_adapter(self): """_ldap_props calls ILDAPProps(self) and returns the result (line 172).""" @@ -949,6 +970,7 @@ def test_returns_ildapprops_adapter(self): # _ugm() method (lines 176-184) # --------------------------------------------------------------------------- + class TestUgmMethod(unittest.TestCase): def test_ugm_builds_and_caches_ugm_on_cache_miss(self): """Cache miss: builds Ugm, stores in cache, returns it (lines 176-184).""" @@ -972,7 +994,9 @@ def test_ugm_returns_cached_value_on_cache_hit(self): plugin = _make_plugin() cached_ugm = MagicMock() mock_cache = MagicMock() - mock_cache.get.return_value = cached_ugm # something other than VALUE_NOT_CACHED + mock_cache.get.return_value = ( + cached_ugm # something other than VALUE_NOT_CACHED + ) with patch("pas.plugins.ldap.plugin.get_plugin_cache", return_value=mock_cache): result = plugin._ugm() @@ -985,6 +1009,7 @@ def test_ugm_returns_cached_value_on_cache_hit(self): # groups / users real property bodies (lines 191, 198) # --------------------------------------------------------------------------- + class TestGroupsUsersPropertyBodies(unittest.TestCase): def test_groups_property_calls_ugm_groups(self): """Real groups property body returns self._ugm().groups (line 191).""" @@ -1009,6 +1034,7 @@ def test_users_property_calls_ugm_users(self): # getRolesForPrincipal "user not found" branch (line 469) # --------------------------------------------------------------------------- + class TestGetRolesForPrincipalNotFound(unittest.TestCase): def test_returns_empty_when_enumerateusers_finds_nothing(self): """Returns () when users is truthy but enumerateUsers returns () (line 469).""" @@ -1029,6 +1055,7 @@ def test_returns_empty_when_enumerateusers_finds_nothing(self): # Group management stubs (lines 513, 522, 531, 542, 551, 560) # --------------------------------------------------------------------------- + class TestGroupManagementStubs(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -1062,6 +1089,7 @@ def test_removePrincipalFromGroup_returns_false(self): # Capability allow methods (lines 664, 677, 686) # --------------------------------------------------------------------------- + class TestCapabilityAllowMethods(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -1083,6 +1111,7 @@ def test_allowGroupRemove_returns_false(self): # getGroupById continue branches (lines 719, 727) # --------------------------------------------------------------------------- + class TestGetGroupByIdContinueBranches(unittest.TestCase): def setUp(self): self.plugin = _make_plugin() @@ -1107,7 +1136,7 @@ def test_skips_propfinder_with_empty_data(self): mock_plugins = MagicMock() mock_plugins.listPlugins.side_effect = [ [("propfinder1", mock_propfinder)], # IPropertiesPlugin - [], # IRolesPlugin + [], # IRolesPlugin ] mock_pas.plugins = mock_plugins mock_pas._getGroupsForPrincipal.return_value = [] @@ -1131,8 +1160,8 @@ def test_skips_rolemaker_with_empty_roles(self): mock_pas = MagicMock() mock_plugins = MagicMock() mock_plugins.listPlugins.side_effect = [ - [], # IPropertiesPlugin - [("rolemaker1", mock_rolemaker)], # IRolesPlugin + [], # IPropertiesPlugin + [("rolemaker1", mock_rolemaker)], # IRolesPlugin ] mock_pas.plugins = mock_plugins mock_pas._getGroupsForPrincipal.return_value = [] @@ -1152,6 +1181,7 @@ def test_skips_rolemaker_with_empty_roles(self): # getGroups (line 739) # --------------------------------------------------------------------------- + class TestGetGroups(unittest.TestCase): def test_getGroups_returns_list_via_getGroupById(self): """getGroups maps getGroupById over getGroupIds (line 739).""" diff --git a/tests/test_properties_unit.py b/tests/test_properties_unit.py index 78f66de..b0ec752 100644 --- a/tests/test_properties_unit.py +++ b/tests/test_properties_unit.py @@ -4,22 +4,20 @@ no real LDAP server, Zope layer, or Plone infrastructure is needed. """ -import unittest -from unittest.mock import MagicMock, patch - -import ldap +from pas.plugins.ldap.defaults import DEFAULTS +from pas.plugins.ldap.properties import BasePropertiesForm +from pas.plugins.ldap.properties import GroupsConfig +from pas.plugins.ldap.properties import LDAPProps +from pas.plugins.ldap.properties import propproxy +from pas.plugins.ldap.properties import UsersConfig +from unittest.mock import MagicMock +from unittest.mock import patch from yafowil.base import ExtractionError from yafowil.base import UNSET from yafowil.plone.form import YAMLBaseForm -from pas.plugins.ldap.defaults import DEFAULTS -from pas.plugins.ldap.properties import ( - BasePropertiesForm, - GroupsConfig, - LDAPProps, - UsersConfig, - propproxy, -) +import ldap +import unittest # --------------------------------------------------------------------------- @@ -76,7 +74,7 @@ def _make_data(field_values): data = MagicMock() def _fetch_side_effect(name): - key = name[len("ldapsettings."):] # strip "ldapsettings." prefix + key = name[len("ldapsettings.") :] # strip "ldapsettings." prefix result = MagicMock() result.extracted = field_values.get(key, UNSET) return result @@ -222,10 +220,17 @@ def test_prepare_happy_path_sets_all_attrs(self): form = _make_form() mock_props, mock_users, mock_groups = self._mock_adapters(user="admin") - with patch("pas.plugins.ldap.properties.ILDAPProps", return_value=mock_props), \ - patch("pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users), \ - patch("pas.plugins.ldap.properties.ILDAPGroupsConfig", return_value=mock_groups), \ - patch.object(YAMLBaseForm, "prepare"): + with ( + patch("pas.plugins.ldap.properties.ILDAPProps", return_value=mock_props), + patch( + "pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users + ), + patch( + "pas.plugins.ldap.properties.ILDAPGroupsConfig", + return_value=mock_groups, + ), + patch.object(YAMLBaseForm, "prepare"), + ): form.prepare() self.assertIs(form.props, mock_props) @@ -253,10 +258,17 @@ def test_prepare_sets_anonymous_when_user_empty(self): form = _make_form() mock_props, mock_users, mock_groups = self._mock_adapters(user="") - with patch("pas.plugins.ldap.properties.ILDAPProps", return_value=mock_props), \ - patch("pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users), \ - patch("pas.plugins.ldap.properties.ILDAPGroupsConfig", return_value=mock_groups), \ - patch.object(YAMLBaseForm, "prepare"): + with ( + patch("pas.plugins.ldap.properties.ILDAPProps", return_value=mock_props), + patch( + "pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users + ), + patch( + "pas.plugins.ldap.properties.ILDAPGroupsConfig", + return_value=mock_groups, + ), + patch.object(YAMLBaseForm, "prepare"), + ): form.prepare() self.assertTrue(form.anonymous) @@ -274,11 +286,18 @@ def _ildapprops(plugin): raise TypeError("adapter failure on first call") return mock_props - with patch("pas.plugins.ldap.properties.ILDAPProps", side_effect=_ildapprops), \ - patch("pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users), \ - patch("pas.plugins.ldap.properties.ILDAPGroupsConfig", return_value=mock_groups), \ - patch.object(YAMLBaseForm, "prepare"), \ - patch("pas.plugins.ldap.properties.logger"): + with ( + patch("pas.plugins.ldap.properties.ILDAPProps", side_effect=_ildapprops), + patch( + "pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users + ), + patch( + "pas.plugins.ldap.properties.ILDAPGroupsConfig", + return_value=mock_groups, + ), + patch.object(YAMLBaseForm, "prepare"), + patch("pas.plugins.ldap.properties.logger"), + ): form.prepare() # init_settings() should have been called once on the plugin @@ -303,8 +322,12 @@ def test_render_form_returns_rendered_when_no_next(self): mock_controller.next = None mock_controller.rendered = "form html" - with patch.object(form, "prepare"), \ - patch("pas.plugins.ldap.properties.Controller", return_value=mock_controller): + with ( + patch.object(form, "prepare"), + patch( + "pas.plugins.ldap.properties.Controller", return_value=mock_controller + ), + ): result = form.render_form() self.assertEqual(result, "form html") @@ -315,8 +338,12 @@ def test_render_form_redirects_and_returns_empty_on_next(self): mock_controller = MagicMock() mock_controller.next = "/redirect-target" - with patch.object(form, "prepare"), \ - patch("pas.plugins.ldap.properties.Controller", return_value=mock_controller): + with ( + patch.object(form, "prepare"), + patch( + "pas.plugins.ldap.properties.Controller", return_value=mock_controller + ), + ): result = form.render_form() form.request.RESPONSE.redirect.assert_called_once_with("/redirect-target") @@ -331,7 +358,9 @@ def test_render_form_redirects_and_returns_empty_on_next(self): class TestSave(unittest.TestCase): """Tests for BasePropertiesForm.save().""" - def _run_save(self, field_values, mock_props=None, mock_users=None, mock_groups=None): + def _run_save( + self, field_values, mock_props=None, mock_users=None, mock_groups=None + ): """Helper: run save() with patched adapters and the given field values.""" form = _make_form() if mock_props is None: @@ -342,9 +371,16 @@ def _run_save(self, field_values, mock_props=None, mock_users=None, mock_groups= mock_groups = MagicMock() data = _make_data(field_values) - with patch("pas.plugins.ldap.properties.ILDAPProps", return_value=mock_props), \ - patch("pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users), \ - patch("pas.plugins.ldap.properties.ILDAPGroupsConfig", return_value=mock_groups): + with ( + patch("pas.plugins.ldap.properties.ILDAPProps", return_value=mock_props), + patch( + "pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=mock_users + ), + patch( + "pas.plugins.ldap.properties.ILDAPGroupsConfig", + return_value=mock_groups, + ), + ): form.save(MagicMock(), data) return mock_props, mock_users, mock_groups @@ -500,9 +536,7 @@ def test_returns_early_when_not_extracted(self): def test_returns_early_when_anonymous(self): """Returns data.extracted when anonymous=True (line 219 second branch).""" form = _make_form() - data, _, _ = self._make_data_mock( - extracted="some-data", anonymous=True - ) + data, _, _ = self._make_data_mock(extracted="some-data", anonymous=True) result = form.userpassanon_extractor(MagicMock(), data) self.assertEqual(result, "some-data") @@ -510,7 +544,11 @@ def test_user_empty_appends_error_and_raises(self): """Empty user → error appended and ExtractionError raised (lines 222-225).""" form = _make_form() data, user_mock, _ = self._make_data_mock( - extracted="data", anonymous=False, user="", password="secret", pw_value="secret" + extracted="data", + anonymous=False, + user="", + password="secret", + pw_value="secret", ) with self.assertRaises(ExtractionError): form.userpassanon_extractor(MagicMock(), data) @@ -579,10 +617,13 @@ def _patch_adapters(self, props=None, users=None, groups=None): def test_ildapprops_exception_returns_false(self): """Returns (False, msg) when ILDAPProps raises (lines 246-249).""" form = _make_form() - with patch( - "pas.plugins.ldap.properties.ILDAPProps", - side_effect=RuntimeError("props-fail"), - ), patch("pas.plugins.ldap.properties.logger"): + with ( + patch( + "pas.plugins.ldap.properties.ILDAPProps", + side_effect=RuntimeError("props-fail"), + ), + patch("pas.plugins.ldap.properties.logger"), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -591,12 +632,14 @@ def test_ildapprops_exception_returns_false(self): def test_ildapusersconfig_exception_returns_false(self): """Returns (False, msg) when ILDAPUsersConfig raises (lines 251-254).""" form = _make_form() - with patch("pas.plugins.ldap.properties.ILDAPProps", return_value=MagicMock()), \ - patch( - "pas.plugins.ldap.properties.ILDAPUsersConfig", - side_effect=RuntimeError("users-fail"), - ), \ - patch("pas.plugins.ldap.properties.logger"): + with ( + patch("pas.plugins.ldap.properties.ILDAPProps", return_value=MagicMock()), + patch( + "pas.plugins.ldap.properties.ILDAPUsersConfig", + side_effect=RuntimeError("users-fail"), + ), + patch("pas.plugins.ldap.properties.logger"), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -605,13 +648,17 @@ def test_ildapusersconfig_exception_returns_false(self): def test_ildapgroupsconfig_exception_returns_false(self): """Returns (False, msg) when ILDAPGroupsConfig raises (lines 256-259).""" form = _make_form() - with patch("pas.plugins.ldap.properties.ILDAPProps", return_value=MagicMock()), \ - patch("pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=MagicMock()), \ - patch( - "pas.plugins.ldap.properties.ILDAPGroupsConfig", - side_effect=RuntimeError("groups-fail"), - ), \ - patch("pas.plugins.ldap.properties.logger"): + with ( + patch("pas.plugins.ldap.properties.ILDAPProps", return_value=MagicMock()), + patch( + "pas.plugins.ldap.properties.ILDAPUsersConfig", return_value=MagicMock() + ), + patch( + "pas.plugins.ldap.properties.ILDAPGroupsConfig", + side_effect=RuntimeError("groups-fail"), + ), + patch("pas.plugins.ldap.properties.logger"), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -624,8 +671,12 @@ def test_server_down_returns_false(self): mock_ugm.users.authenticate.side_effect = ldap.SERVER_DOWN p1, p2, p3 = self._patch_adapters() - with p1, p2, p3, \ - patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm): + with ( + p1, + p2, + p3, + patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -638,8 +689,12 @@ def test_ldap_error_in_users_returns_false(self): mock_ugm.users.authenticate.side_effect = ldap.LDAPError("users ldap error") p1, p2, p3 = self._patch_adapters() - with p1, p2, p3, \ - patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm): + with ( + p1, + p2, + p3, + patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -652,9 +707,13 @@ def test_generic_exception_in_users_returns_false(self): mock_ugm.users.authenticate.side_effect = RuntimeError("generic users error") p1, p2, p3 = self._patch_adapters() - with p1, p2, p3, \ - patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), \ - patch("pas.plugins.ldap.properties.logger"): + with ( + p1, + p2, + p3, + patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), + patch("pas.plugins.ldap.properties.logger"), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -668,8 +727,12 @@ def test_ldap_error_in_groups_returns_false(self): mock_ugm.groups.keys.side_effect = _FakeLDAPGroupsError("groups-ldap-error") p1, p2, p3 = self._patch_adapters() - with p1, p2, p3, \ - patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm): + with ( + p1, + p2, + p3, + patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -683,9 +746,13 @@ def test_generic_exception_in_groups_returns_false(self): mock_ugm.groups.keys.side_effect = RuntimeError("generic groups error") p1, p2, p3 = self._patch_adapters() - with p1, p2, p3, \ - patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), \ - patch("pas.plugins.ldap.properties.logger"): + with ( + p1, + p2, + p3, + patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), + patch("pas.plugins.ldap.properties.logger"), + ): ok, msg = form.connection_test() self.assertFalse(ok) @@ -699,8 +766,12 @@ def test_connection_success_returns_true(self): mock_ugm.groups.keys.return_value = [] p1, p2, p3 = self._patch_adapters() - with p1, p2, p3, \ - patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm): + with ( + p1, + p2, + p3, + patch("pas.plugins.ldap.properties.Ugm", return_value=mock_ugm), + ): ok, msg = form.connection_test() self.assertTrue(ok) @@ -768,7 +839,9 @@ def test_memcached_getter_with_record_provider_returns_value(self): mock_record.value = "memcache-host:11211" mock_provider = MagicMock(return_value=mock_record) - with patch("pas.plugins.ldap.properties.queryUtility", return_value=mock_provider): + with patch( + "pas.plugins.ldap.properties.queryUtility", return_value=mock_provider + ): result = props.memcached self.assertEqual(result, "memcache-host:11211") @@ -788,7 +861,9 @@ def test_memcached_setter_with_record_provider_sets_value(self): mock_record = MagicMock() mock_provider = MagicMock(return_value=mock_record) - with patch("pas.plugins.ldap.properties.queryUtility", return_value=mock_provider): + with patch( + "pas.plugins.ldap.properties.queryUtility", return_value=mock_provider + ): props.memcached = "new-host:11211" self.assertEqual(mock_record.value, "new-host:11211") @@ -828,7 +903,9 @@ def _make_users_config( def test_expires_attr_returns_attr_when_expiration_enabled(self): """expiresAttr returns the attribute name when account_expiration is True (line 407).""" - config = self._make_users_config(account_expiration=True, expires_attr="shadowExpire") + config = self._make_users_config( + account_expiration=True, expires_attr="shadowExpire" + ) self.assertEqual(config.expiresAttr, "shadowExpire") def test_expires_attr_returns_none_when_expiration_disabled(self): diff --git a/tests/test_setuphandlers_unit.py b/tests/test_setuphandlers_unit.py index 5d4e7a9..aeaf793 100644 --- a/tests/test_setuphandlers_unit.py +++ b/tests/test_setuphandlers_unit.py @@ -4,21 +4,21 @@ so no real LDAP server, Zope layer, or GenericSetup context is needed. """ -import unittest -from unittest.mock import MagicMock, patch, call +from pas.plugins.ldap.setuphandlers import _addPlugin +from pas.plugins.ldap.setuphandlers import post_install +from pas.plugins.ldap.setuphandlers import remove_persistent_import_step +from pas.plugins.ldap.setuphandlers import TITLE +from unittest.mock import MagicMock +from unittest.mock import patch -from pas.plugins.ldap.setuphandlers import ( - TITLE, - _addPlugin, - post_install, - remove_persistent_import_step, -) +import unittest # --------------------------------------------------------------------------- # remove_persistent_import_step (lines 28-31) # --------------------------------------------------------------------------- + class TestRemovePersistentImportStep(unittest.TestCase): """Tests for remove_persistent_import_step.""" @@ -56,6 +56,7 @@ def test_calls_getImportStepRegistry(self): # _addPlugin (lines 34-50) # --------------------------------------------------------------------------- + class TestAddPlugin(unittest.TestCase): """Tests for _addPlugin helper.""" @@ -143,6 +144,7 @@ def test_skips_interface_not_provided_by_plugin(self): # post_install (lines 53-57) # --------------------------------------------------------------------------- + class TestPostInstall(unittest.TestCase): """Tests for post_install.""" @@ -162,8 +164,12 @@ def test_uses_getSite_to_get_site(self): """post_install calls getSite() (line 55).""" mock_site = MagicMock() - with patch("pas.plugins.ldap.setuphandlers.getSite", return_value=mock_site) as mock_get_site: - with patch("pas.plugins.ldap.setuphandlers._addPlugin"): - post_install(MagicMock()) + with ( + patch( + "pas.plugins.ldap.setuphandlers.getSite", return_value=mock_site + ) as mock_get_site, + patch("pas.plugins.ldap.setuphandlers._addPlugin"), + ): + post_install(MagicMock()) mock_get_site.assert_called_once() diff --git a/tests/test_sheet_unit.py b/tests/test_sheet_unit.py index dd33346..a17a28e 100644 --- a/tests/test_sheet_unit.py +++ b/tests/test_sheet_unit.py @@ -4,15 +4,20 @@ real LDAP server, Zope layer, or acquisition context is needed. """ +from unittest.mock import MagicMock +from unittest.mock import patch + import unittest -from unittest.mock import MagicMock, patch, call # --------------------------------------------------------------------------- # Helper builders # --------------------------------------------------------------------------- -def _make_bare_sheet(properties=None, principal_type="users", principal_id="uid0", context_raises=False): + +def _make_bare_sheet( + properties=None, principal_type="users", principal_id="uid0", context_raises=False +): """Create a LDAPUserPropertySheet bypassing __init__, with full state set.""" from pas.plugins.ldap.sheet import LDAPUserPropertySheet @@ -60,11 +65,13 @@ def _build_init_sheet(user_in_users=True, attrmap=None, request=None): plugin.users.__getitem__.return_value = mock_ldap_principal plugin.groups.__getitem__.return_value = mock_ldap_principal - with patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), \ - patch("pas.plugins.ldap.sheet.ILDAPUsersConfig", return_value=mock_pcfg), \ - patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig", return_value=mock_pcfg), \ - patch("pas.plugins.ldap.sheet.getRequest", return_value=request), \ - patch("pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None): + with ( + patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), + patch("pas.plugins.ldap.sheet.ILDAPUsersConfig", return_value=mock_pcfg), + patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig", return_value=mock_pcfg), + patch("pas.plugins.ldap.sheet.getRequest", return_value=request), + patch("pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None), + ): sheet = LDAPUserPropertySheet(principal, plugin) return sheet, plugin, mock_ldap_principal, mock_pcfg @@ -74,6 +81,7 @@ def _build_init_sheet(user_in_users=True, attrmap=None, request=None): # __init__ (lines 32-58) # --------------------------------------------------------------------------- + class TestLDAPUserPropertySheetInit(unittest.TestCase): """Tests for LDAPUserPropertySheet.__init__.""" @@ -91,12 +99,17 @@ def test_init_group_branch_sets_type_to_groups(self): def test_init_uses_ILDAPUsersConfig_for_user(self): """ILDAPUsersConfig(plugin) is called for user principals (line 37).""" - with patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), \ - patch("pas.plugins.ldap.sheet.ILDAPUsersConfig") as mock_cfg, \ - patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig"), \ - patch("pas.plugins.ldap.sheet.getRequest", return_value=None), \ - patch("pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None): + with ( + patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), + patch("pas.plugins.ldap.sheet.ILDAPUsersConfig") as mock_cfg, + patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig"), + patch("pas.plugins.ldap.sheet.getRequest", return_value=None), + patch( + "pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None + ), + ): from pas.plugins.ldap.sheet import LDAPUserPropertySheet + principal = MagicMock() principal.getId.return_value = "uid0" plugin = MagicMock() @@ -111,12 +124,17 @@ def test_init_uses_ILDAPUsersConfig_for_user(self): def test_init_uses_ILDAPGroupsConfig_for_group(self): """ILDAPGroupsConfig(plugin) is called for group principals (line 40).""" - with patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), \ - patch("pas.plugins.ldap.sheet.ILDAPUsersConfig"), \ - patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig") as mock_cfg, \ - patch("pas.plugins.ldap.sheet.getRequest", return_value=None), \ - patch("pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None): + with ( + patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), + patch("pas.plugins.ldap.sheet.ILDAPUsersConfig"), + patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig") as mock_cfg, + patch("pas.plugins.ldap.sheet.getRequest", return_value=None), + patch( + "pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None + ), + ): from pas.plugins.ldap.sheet import LDAPUserPropertySheet + principal = MagicMock() principal.getId.return_value = "uid0" plugin = MagicMock() @@ -151,7 +169,7 @@ def test_init_attrmap_includes_other_keys(self): def test_init_no_request_calls_context_load(self): """With request=None, attrs.context.load() is called (lines 50-51).""" - sheet, _, mock_ldap_principal, _ = _build_init_sheet( + _sheet, _, mock_ldap_principal, _ = _build_init_sheet( request=None, attrmap={"mail": "mail"} ) mock_ldap_principal.attrs.context.load.assert_called_once() @@ -159,14 +177,14 @@ def test_init_no_request_calls_context_load(self): def test_init_no_request_does_not_set_reload_flag(self): """With request=None, _ldap_props_reloaded is NOT set (line 52 False branch).""" request = None - sheet, _, _, _ = _build_init_sheet(request=request, attrmap={"mail": "mail"}) + _sheet, _, _, _ = _build_init_sheet(request=request, attrmap={"mail": "mail"}) # No exception means the code didn't try to set the item on None def test_init_request_not_reloaded_sets_flag(self): """With a fresh request, loads attrs and sets _ldap_props_reloaded (lines 50-53).""" request = MagicMock() request.get.return_value = None # not yet reloaded → falsy - sheet, _, mock_ldap_principal, _ = _build_init_sheet( + _sheet, _, mock_ldap_principal, _ = _build_init_sheet( request=request, attrmap={"mail": "mail"} ) mock_ldap_principal.attrs.context.load.assert_called_once() @@ -176,7 +194,7 @@ def test_init_request_already_reloaded_skips_load(self): """With a reloaded request, skips attrs.context.load (line 50 False branch).""" request = MagicMock() request.get.return_value = 1 # already reloaded → truthy - sheet, _, mock_ldap_principal, _ = _build_init_sheet( + _sheet, _, mock_ldap_principal, _ = _build_init_sheet( request=request, attrmap={"mail": "mail"} ) mock_ldap_principal.attrs.context.load.assert_not_called() @@ -185,19 +203,24 @@ def test_init_request_already_reloaded_skips_load(self): def test_init_loads_properties_from_ldap_attrs(self): """Properties are loaded from ldapprincipal.attrs for each attrmap key (lines 54-55).""" - sheet, _, mock_ldap_principal, _ = _build_init_sheet( + _sheet, _, mock_ldap_principal, _ = _build_init_sheet( attrmap={"mail": "mail"}, request=None ) mock_ldap_principal.attrs.get.assert_called_with("mail", "") def test_init_calls_userpropertysheet_init(self): """UserPropertySheet.__init__ is called at the end of __init__ (line 56).""" - with patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), \ - patch("pas.plugins.ldap.sheet.ILDAPUsersConfig") as mock_cfg, \ - patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig"), \ - patch("pas.plugins.ldap.sheet.getRequest", return_value=None), \ - patch("pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None) as mock_up_init: + with ( + patch("pas.plugins.ldap.sheet.aq_base", side_effect=lambda x: x), + patch("pas.plugins.ldap.sheet.ILDAPUsersConfig") as mock_cfg, + patch("pas.plugins.ldap.sheet.ILDAPGroupsConfig"), + patch("pas.plugins.ldap.sheet.getRequest", return_value=None), + patch( + "pas.plugins.ldap.sheet.UserPropertySheet.__init__", return_value=None + ) as mock_up_init, + ): from pas.plugins.ldap.sheet import LDAPUserPropertySheet + principal = MagicMock() principal.getId.return_value = "uid0" plugin = MagicMock() @@ -228,6 +251,7 @@ def test_init_combined_rdn_and_other_keys(self): # _get_ldap_principal (lines 66-67) # --------------------------------------------------------------------------- + class TestGetLDAPPrincipal(unittest.TestCase): """Tests for _get_ldap_principal.""" @@ -250,6 +274,7 @@ def test_returns_groups_entry_for_group_type(self): # canWriteProperty (line 71) # --------------------------------------------------------------------------- + class TestCanWriteProperty(unittest.TestCase): """Tests for canWriteProperty.""" @@ -268,6 +293,7 @@ def test_returns_false_for_nonexistent_property(self): # setProperty (lines 73-82) # --------------------------------------------------------------------------- + class TestSetProperty(unittest.TestCase): """Tests for setProperty.""" @@ -279,13 +305,19 @@ def test_updates_properties_dict(self): def test_updates_ldap_attrs(self): """ldapprincipal.attrs[id] is updated (line 77).""" - sheet, mock_ldap_principal = _make_bare_sheet(properties={"mail": "old@example.com"}) + sheet, mock_ldap_principal = _make_bare_sheet( + properties={"mail": "old@example.com"} + ) sheet.setProperty(None, "mail", "new@example.com") - mock_ldap_principal.attrs.__setitem__.assert_called_with("mail", "new@example.com") + mock_ldap_principal.attrs.__setitem__.assert_called_with( + "mail", "new@example.com" + ) def test_calls_ldapprincipal_context(self): """ldapprincipal.context() is called to persist the change (line 79).""" - sheet, mock_ldap_principal = _make_bare_sheet(properties={"mail": "old@example.com"}) + sheet, mock_ldap_principal = _make_bare_sheet( + properties={"mail": "old@example.com"} + ) sheet.setProperty(None, "mail", "new@example.com") mock_ldap_principal.context.assert_called_once() @@ -311,6 +343,7 @@ def test_asserts_id_must_be_in_properties(self): # setProperties (lines 84-95) # --------------------------------------------------------------------------- + class TestSetProperties(unittest.TestCase): """Tests for setProperties.""" diff --git a/tests/testing.py b/tests/testing.py index 763b328..9585052 100644 --- a/tests/testing.py +++ b/tests/testing.py @@ -1,15 +1,14 @@ """Testing layer for pas.plugins.ldap.""" -from pas.plugins.ldap.cache import cacheProviderFactory -from pas.plugins.ldap.cache import cacheProviderFactory -from pas.plugins.ldap.interfaces import ICacheSettingsRecordProvider -from pas.plugins.ldap.plonecontrolpanel.cache import CacheSettingsRecordProvider -from pas.plugins.ldap.properties import LDAPProps from node.ext.ldap import testing as ldaptesting from node.ext.ldap.interfaces import ICacheProviderFactory from node.ext.ldap.interfaces import ILDAPGroupsConfig from node.ext.ldap.interfaces import ILDAPProps from node.ext.ldap.interfaces import ILDAPUsersConfig +from pas.plugins.ldap.cache import cacheProviderFactory +from pas.plugins.ldap.interfaces import ICacheSettingsRecordProvider +from pas.plugins.ldap.plonecontrolpanel.cache import CacheSettingsRecordProvider +from pas.plugins.ldap.properties import LDAPProps from plone.registry import Registry from plone.registry.interfaces import IRegistry from plone.testing import Layer @@ -25,6 +24,8 @@ from zope.interface import implementer from zope.interface import Interface +import contextlib + SITE_OWNER_NAME = SITE_OWNER_PASSWORD = "admin" @@ -69,11 +70,11 @@ class PASLDAPLayer(Layer): # Products that will be installed, plus options products = ( - ("Products.GenericSetup", {"loadZCML": True}), # noqa - ("Products.CMFCore", {"loadZCML": True}), # noqa - ("Products.PluggableAuthService", {"loadZCML": True}), # noqa - ("Products.PluginRegistry", {"loadZCML": True}), # noqa - ("Products.PlonePAS", {"loadZCML": True}), # noqa + ("Products.GenericSetup", {"loadZCML": True}), + ("Products.CMFCore", {"loadZCML": True}), + ("Products.PluggableAuthService", {"loadZCML": True}), + ("Products.PluginRegistry", {"loadZCML": True}), + ("Products.PlonePAS", {"loadZCML": True}), ) def setUp(self): @@ -104,12 +105,10 @@ def loadAll(filename): if not config["loadZCML"]: continue package = resolve(p) - try: + with contextlib.suppress(OSError): xmlconfig.file( filename, package, context=self["configurationContext"] ) - except OSError: - pass loadAll("meta.zcml") loadAll("configure.zcml") @@ -122,7 +121,7 @@ def setUpProducts(self): """Install all old-style products listed in the the ``products`` tuple of this class. """ - for prd, config in self.products: + for prd, _config in self.products: zope.installProduct(self["app"], prd) From cdfcb3a53800e1ddda797e342d6e9f54898fe421 Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 15:41:44 +0200 Subject: [PATCH 2/8] Add RELEASE.md documenting the tag-driven Trusted Publishing release process Co-Authored-By: Claude Opus 4.8 (1M context) --- README.rst | 4 ++- RELEASE.md | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 RELEASE.md diff --git a/README.rst b/README.rst index 25d8b5d..ea974f0 100644 --- a/README.rst +++ b/README.rst @@ -307,7 +307,9 @@ Maintainers are Robert Niederreiter, Jens Klein and the `BlueDynamics Alliance < Support ======= -We appreciate any contribution and if a release is needed to be done on pypi, please just contact one of us: +We appreciate any contribution. Releases to PyPI are automated via tagging and +Trusted Publishing; see `RELEASE.md `_ +for the process. If you need a release done, please contact one of us: `dev@bluedynamics dot com `_. - If you are having issues, please let us know at our `issue tracker `_. diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000..424bd61 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,93 @@ +# Releasing pas.plugins.ldap + +This package is released to [PyPI](https://pypi.org/project/pas.plugins.ldap/) +**by tagging** — the version is derived from the git tag via +[hatch-vcs](https://github.com/ofek/hatch-vcs), and publishing happens through +GitHub Actions using PyPI [Trusted Publishing](https://docs.pypi.org/trusted-publishers/) +(OIDC, no API tokens). + +There is **no** version number to bump in `pyproject.toml`. The tag *is* the +version. + +## Workflows + +`.github/workflows/release.yaml` defines three jobs: + +| Job | Trigger | Target | +| --- | --- | --- | +| `build-package` | CI passed on `main`, a published Release, or manual dispatch | builds & inspects the sdist + wheel | +| `release-test-pypi` | after CI passes on `main` (or manual dispatch on `main`) | **Test** PyPI (in-dev builds) | +| `release-pypi` | a GitHub **Release** is *published* | **PyPI** (the real release) | + +The build runs the `hatch_build.py` hook, so the compiled `.mo` translation +catalogs are always included in the artifacts. + +## One-time setup (maintainers / org admins) + +This only needs to be done once per project. + +1. **GitHub environments** — create two environments in the repo + (*Settings → Environments*): + - `release-pypi` + - `release-test-pypi` + + Optionally add protection rules (e.g. required reviewers) to + `release-pypi`. + +2. **PyPI trusted publisher** — on + add a GitHub publisher: + - Owner: `collective` + - Repository: `pas.plugins.ldap` + - Workflow name: `release.yaml` + - Environment: `release-pypi` + +3. **Test PyPI trusted publisher** — repeat on + with environment `release-test-pypi`. + + > For a brand-new project name you may need to create the project first via + > a "pending publisher" entry on (Test) PyPI. + +## Versioning / tags + +`hatch-vcs` derives the version from the latest tag: + +- A commit **on** tag `X.Y.Z` builds as exactly `X.Y.Z`. +- Commits **after** a tag build as a `.devN` version based on that tag. + +Because the last released tag is `1.8.4`, untagged builds are currently +versioned `1.8.4.devN`. To get **2.0.0-series** in-dev builds on Test PyPI, +push an early pre-release tag, e.g.: + +```shell +git tag 2.0.0a1 +git push origin 2.0.0a1 +``` + +Use [PEP 440](https://peps.python.org/pep-0440/) pre-release tags +(`2.0.0a1`, `2.0.0b1`, `2.0.0rc1`) for alphas/betas/release candidates. + +## Cutting a release + +1. Make sure `main` is green and `CHANGES.rst` lists everything under the + target version heading. Rename the `2.0.0 (unreleased)` heading to the + release date, e.g. `2.0.0 (2026-06-22)`, and commit/merge that to `main`. +2. Create and push the tag: + ```shell + git checkout main && git pull + git tag 2.0.0 + git push origin 2.0.0 + ``` +3. Create a **GitHub Release** for that tag (*Releases → Draft a new + release → choose tag `2.0.0` → Publish release*). Publishing the Release + triggers `release-pypi`, which builds and uploads to PyPI via Trusted + Publishing. +4. Verify the new version appears on + and that the Actions run is + green. + +## Pre-release checklist + +- [ ] `CHANGES.rst` is up to date and the heading carries the release date. +- [ ] CI is green on `main` (QA + the full Plone 6.0–6.2 / Python 3.10–3.14 matrix). +- [ ] Trusted publishers and GitHub environments are configured (one-time). +- [ ] Tag follows PEP 440 (`2.0.0`, `2.0.1`, `2.1.0a1`, …). From 9e194626f6f78ccd003a499b564eb0315d454998 Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 15:48:57 +0200 Subject: [PATCH 3/8] RELEASE.md: require release-pypi environment protection (collective repo) Co-Authored-By: Claude Opus 4.8 (1M context) --- RELEASE.md | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 424bd61..446bcd5 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -31,8 +31,21 @@ This only needs to be done once per project. - `release-pypi` - `release-test-pypi` - Optionally add protection rules (e.g. required reviewers) to - `release-pypi`. + **Protecting `release-pypi` is required, not optional.** This repository + lives in the `collective` org, which grants write access liberally — so + *anyone with repo write access could otherwise publish a GitHub Release and + push to PyPI*. With Trusted Publishing the upload runs under the workflow's + identity in the environment, so the environment's protection rules are the + real gate on who can release. On the `release-pypi` environment set: + - **Required reviewers** → the actual package maintainers (e.g. + `jensens`, `rnixx`). A PyPI upload then waits for one of them to approve + the deployment, even if someone else published the Release. + - **Deployment branches and tags** → restrict to the release tags (e.g. a + tag rule like `*`) and/or `main`, so the workflow can only publish from + intended refs. + + `release-test-pypi` can stay unprotected — in-dev builds to Test PyPI are + low-risk. 2. **PyPI trusted publisher** — on add a GitHub publisher: @@ -89,5 +102,7 @@ Use [PEP 440](https://peps.python.org/pep-0440/) pre-release tags - [ ] `CHANGES.rst` is up to date and the heading carries the release date. - [ ] CI is green on `main` (QA + the full Plone 6.0–6.2 / Python 3.10–3.14 matrix). -- [ ] Trusted publishers and GitHub environments are configured (one-time). +- [ ] Trusted publishers and GitHub environments are configured (one-time), + and `release-pypi` has **required reviewers** set (mandatory for this + collective repo). - [ ] Tag follows PEP 440 (`2.0.0`, `2.0.1`, `2.1.0a1`, …). From b8ffb976d0245fb1ca40c765d7a3f4f089a01633 Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 15:51:27 +0200 Subject: [PATCH 4/8] RELEASE.md: clarify release-pypi needs a tag deployment rule, not a branch Co-Authored-By: Claude Opus 4.8 (1M context) --- RELEASE.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 446bcd5..b3c2e36 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -40,9 +40,12 @@ This only needs to be done once per project. - **Required reviewers** → the actual package maintainers (e.g. `jensens`, `rnixx`). A PyPI upload then waits for one of them to approve the deployment, even if someone else published the Release. - - **Deployment branches and tags** → restrict to the release tags (e.g. a - tag rule like `*`) and/or `main`, so the workflow can only publish from - intended refs. + - **Deployment branches and tags** → add a **tag** rule (e.g. `*`, or a + stricter version pattern like `[0-9]*.[0-9]*.[0-9]*`). This is required: + `release-pypi` only ever runs when a GitHub **Release** is published, and + that workflow runs on the **tag** ref (`refs/tags/`), not on a + branch. A branch-only rule (e.g. just `main`) would *block* the real PyPI + publish. `main` is not needed for `release-pypi`. `release-test-pypi` can stay unprotected — in-dev builds to Test PyPI are low-risk. From 017306795f697c1992c5cdb01be6b869c5d73adc Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 17:56:43 +0200 Subject: [PATCH 5/8] RELEASE.md: add exact PyPI + Test PyPI trusted publisher field tables Co-Authored-By: Claude Opus 4.8 (1M context) --- RELEASE.md | 46 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index b3c2e36..28619f5 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -50,18 +50,40 @@ This only needs to be done once per project. `release-test-pypi` can stay unprotected — in-dev builds to Test PyPI are low-risk. -2. **PyPI trusted publisher** — on - add a GitHub publisher: - - Owner: `collective` - - Repository: `pas.plugins.ldap` - - Workflow name: `release.yaml` - - Environment: `release-pypi` - -3. **Test PyPI trusted publisher** — repeat on - with environment `release-test-pypi`. - - > For a brand-new project name you may need to create the project first via - > a "pending publisher" entry on (Test) PyPI. +2. **PyPI trusted publisher** — the project already exists, so add a GitHub + publisher on + + (you must be an owner/maintainer of the PyPI project): + + | Field | Value | + | --- | --- | + | Owner | `collective` | + | Repository name | `pas.plugins.ldap` | + | Workflow name | `release.yaml` | + | Environment name | `release-pypi` | + +3. **Test PyPI trusted publisher** — Test PyPI is a **separate site with a + separate account**. The project most likely does not exist there yet, so add + a *pending publisher* on + : + + | Field | Value | + | --- | --- | + | PyPI Project Name | `pas.plugins.ldap` | + | Owner | `collective` | + | Repository name | `pas.plugins.ldap` | + | Workflow name | `release.yaml` | + | Environment name | `release-test-pypi` | + + The project is created on Test PyPI on the first successful upload. If it + already exists, add the publisher under its project settings instead + (`https://test.pypi.org/manage/project/pas.plugins.ldap/settings/publishing/`, + without the *PyPI Project Name* field). + + > The only difference between the two entries is the **Environment name** + > (`release-pypi` vs `release-test-pypi`); owner, repository and workflow are + > identical. The values must match **exactly** or (Test) PyPI rejects the + > OIDC token. ## Versioning / tags From cf38223dff0f487634fea86fc2ea2cb19ba50bcf Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 18:03:55 +0200 Subject: [PATCH 6/8] Convert documentation from reStructuredText to Markdown Convert README, CHANGES, CONTRIBUTORS, LICENSE and TODO from .rst to .md (GitHub-Flavored Markdown), and update all references: - pyproject.toml: `readme = "README.md"` (now `text/markdown`), and the ChangeLog URL points to CHANGES.md on `main`. - RELEASE.md: refer to CHANGES.md. - Remove MANIFEST.in, which is unused under the hatchling build backend. The `tests/*.rst` doctest files are intentionally left as reStructuredText (they are executed by the doctest runner, not documentation). Verified: `python -m build` yields `Description-Content-Type: text/markdown`, .mo catalogs still ship, and the full suite (incl. doctests) passes (240). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGES.rst => CHANGES.md | 177 +++++++-------- CONTRIBUTORS.rst => CONTRIBUTORS.md | 19 +- LICENSE.rst => LICENSE.md | 18 +- MANIFEST.in | 5 - README.md | 296 +++++++++++++++++++++++++ README.rst | 329 ---------------------------- RELEASE.md | 4 +- TODO.rst => TODO.md | 10 +- pyproject.toml | 4 +- 9 files changed, 398 insertions(+), 464 deletions(-) rename CHANGES.rst => CHANGES.md (63%) rename CONTRIBUTORS.rst => CONTRIBUTORS.md (79%) rename LICENSE.rst => LICENSE.md (77%) delete mode 100644 MANIFEST.in create mode 100644 README.md delete mode 100644 README.rst rename TODO.rst => TODO.md (77%) diff --git a/CHANGES.rst b/CHANGES.md similarity index 63% rename from CHANGES.rst rename to CHANGES.md index 2df5b96..f89c7b7 100644 --- a/CHANGES.rst +++ b/CHANGES.md @@ -1,31 +1,33 @@ +# History -History -======= +## 2.0.0 (unreleased) -2.0.0 (unreleased) ------------------- - -- Derive the package version from git tags via ``hatch-vcs`` (the static - ``version`` in ``pyproject.toml`` is gone). Releases are now made by tagging. +- Derive the package version from git tags via `hatch-vcs` (the static + `version` in `pyproject.toml` is gone). Releases are now made by tagging. [jensens] -- Switch Python linting and formatting from black/isort to ``ruff``. +- Switch Python linting and formatting from black/isort to `ruff`. [jensens] -- Restructure CI into a ``CI`` umbrella workflow (QA + tests) and add a +- Restructure CI into a `CI` umbrella workflow (QA + tests) and add a PyPI/Test-PyPI release workflow using OIDC Trusted Publishing. [jensens] -- Portrait traverser: raise a proper ``LocationError`` (404) when a user has - no portrait on the property sheet, instead of an ``AttributeError`` from - calling ``__of__`` on ``None``. - Fixes `issue #68 `_. +- Convert the documentation files (`README`, `CHANGES`, `CONTRIBUTORS`, + `LICENSE`, `TODO`) from reStructuredText to Markdown and drop the unused + `MANIFEST.in`. (The `tests/*.rst` doctest files are unchanged.) + [jensens] + +- Portrait traverser: raise a proper `LocationError` (404) when a user has + no portrait on the property sheet, instead of an `AttributeError` from + calling `__of__` on `None`. + Fixes [issue #68](https://github.com/collective/pas.plugins.ldap/issues/68). [jensens] -- Compile the gettext ``.po`` catalogs to ``.mo`` automatically during the +- Compile the gettext `.po` catalogs to `.mo` automatically during the build (hatchling build hook) and ship them in the sdist and wheel, so the translations are active for installs from PyPI without a manual - ``make gettext-compile`` step. + `make gettext-compile` step. [jensens] - Fixed the "LDAP / Active Directory Configuration" controlpanel uses wrong permission. @@ -52,29 +54,29 @@ History - Refactor test setup to use pytest as runner. [jensens] -- Increase the default of the ``PAS_PLUGINS_LDAP_OPT_TIMEOUT`` overall +- Increase the default of the `PAS_PLUGINS_LDAP_OPT_TIMEOUT` overall operation timeout from 2.0 to 30.0 seconds. [jensens] -- Remove ``five.globalrequest`` dependency. - The package now only depends on ``zope.globalrequest``, which was the only +- Remove `five.globalrequest` dependency. + The package now only depends on `zope.globalrequest`, which was the only one actually imported. Removes the stale dependency declaration. [cillianderoiste, jensens] -- Drop the unused ``six`` dependency. - Fix the portrait property-sheet support to use ``io.BytesIO`` instead of the - text-only ``six.StringIO``, which raised a ``TypeError`` on binary image data. +- Drop the unused `six` dependency. + Fix the portrait property-sheet support to use `io.BytesIO` instead of the + text-only `six.StringIO`, which raised a `TypeError` on binary image data. [jensens] -- ``getRolesForPrincipal`` now honors the activation state of the - ``IRolesPlugin`` interface and is declared ``@security.private`` like the +- `getRolesForPrincipal` now honors the activation state of the + `IRolesPlugin` interface and is declared `@security.private` like the other PAS interface methods. Note: as part of the new default user roles feature, LDAP users receive the - configured roles (default ``Member``) only when the *Roles* plugin interface + configured roles (default `Member`) only when the *Roles* plugin interface is activated for this plugin. [jensens] -- Fix the LDAP inspector raising a ``TypeError`` (bytes dict key) when a node +- Fix the LDAP inspector raising a `TypeError` (bytes dict key) when a node attribute could not be decoded; the error is now reported gracefully again. [jensens] @@ -85,17 +87,15 @@ History [sauzher] -1.8.4 (2025-07-14) ------------------- +## 1.8.4 (2025-07-14) -- Remove the dependency from ``five.globalrequest``. +- Remove the dependency from `five.globalrequest`. This is needed to make this package work on Plone 6.1.2. - See `PR #134 `_. + See [PR #134](https://github.com/collective/pas.plugins.ldap/pull/134). [cillianderoiste] -1.8.3 (2024-11-13) ------------------- +## 1.8.3 (2024-11-13) - Add uninstall profile [dumitval] @@ -104,23 +104,20 @@ History [mamico] -1.8.2 (2022-10-31) ------------------- +## 1.8.2 (2022-10-31) - Add connection and operation timeout properties for LDAP server. - Fixes `issue #61 `_. + Fixes [issue #61](https://github.com/collective/pas.plugins.ldap/issues/61). [mamico] -1.8.1 (2021-10-09) ------------------- +## 1.8.1 (2021-10-09) - Fix imports for Zope 5 and Plone 6. [pbauer] -1.8.0 (2020-06-11) ------------------- +## 1.8.0 (2020-06-11) Features: @@ -131,30 +128,27 @@ Features: [jensens] -1.7.2 (2020-02-21) ------------------- +## 1.7.2 (2020-02-21) Bug fixes: - Import loader from YAFOWIL. - Fixes `issue #97 `_ - and `issue #92 `_. + Fixes [issue #97](https://github.com/collective/pas.plugins.ldap/issues/97) + and [issue #92](https://github.com/collective/pas.plugins.ldap/issues/92). [al45tair] -1.7.1 (2020-02-14) ------------------- +## 1.7.1 (2020-02-14) - Use the plugin ID as the property sheet ID instead of the user ID. - Fixes `issue #95 `_. + Fixes [issue #95](https://github.com/collective/pas.plugins.ldap/issues/95). [reinhardt] - Grant the Member role to all LDAP users. [reinhardt] -1.7.0 (2020-01-22) ------------------- +## 1.7.0 (2020-01-22) - Fixed error display for /plone_ldapcontrolpanel when a wrong value is provided for the "Groups container DN" field. @@ -166,19 +160,18 @@ Bug fixes: - Log LDAP-errors as level error, to get them i.e. into Sentry. [jensens] -- Make timeout of LDAP-errors logging configurable with environment variable ``PAS_PLUGINS_LDAP_ERROR_LOG_TIMEOUT``. +- Make timeout of LDAP-errors logging configurable with environment variable `PAS_PLUGINS_LDAP_ERROR_LOG_TIMEOUT`. [jensens] - Log long running LDAP/ pas.plugin.ldap operations as error. - Threshold can be controlled with environment variable ``PAS_PLUGINS_LDAP_LONG_RUNNING_LOG_THRESHOLD``. + Threshold can be controlled with environment variable `PAS_PLUGINS_LDAP_LONG_RUNNING_LOG_THRESHOLD`. [jensens] -1.6.2 (2019-09-12) ------------------- +## 1.6.2 (2019-09-12) - Remove broken old import step from base profile. - Fixes `issue #74 `_. + Fixes [issue #74](https://github.com/collective/pas.plugins.ldap/issues/74). [maurits] - Remove deprecation warning for removal of time.clock() which will break @@ -190,8 +183,7 @@ Bug fixes: [reinhardt] -1.6.1 (2019-05-07) ------------------- +## 1.6.1 (2019-05-07) - Pimp ZMI view to look better on Zope 4. [jensens] @@ -200,8 +192,7 @@ Bug fixes: [jensens] -1.6.0 (2019-05-07) ------------------- +## 1.6.0 (2019-05-07) - Fix inspector: In Python 3 JSON dumps does not accept bytes as keys. [jensens, 2silver] @@ -244,11 +235,10 @@ Bug fixes: [reinhardt] -1.5.3 (2017-12-15) ------------------- +## 1.5.3 (2017-12-15) -- Remove manual LDAP search pagination on UGM principal ``search`` calls. - This is done in downstream API as of ``node.ext.ldap`` 1.0b7. +- Remove manual LDAP search pagination on UGM principal `search` calls. + This is done in downstream API as of `node.ext.ldap` 1.0b7. [rnix] - Fix testing: register plugin type of PlonePAS. @@ -258,8 +248,7 @@ Bug fixes: [jensens] -1.5.2 (2017-10-20) ------------------- +## 1.5.2 (2017-10-20) - Set the memcached TTW setting in the form definition to unicode, so that you can save the controlpanel form if you change this field. @@ -269,23 +258,20 @@ Bug fixes: [svx] -1.5.1 (2016-10-18) ------------------- +## 1.5.1 (2016-10-18) -- Fix: TTW setting of ``page_size`` resulted in float value. +- Fix: TTW setting of `page_size` resulted in float value. Now set form datattype to integer. Thanks @datakurre for reporting! [jensens] -1.5 (2016-10-06) ----------------- +## 1.5 (2016-10-06) - No changes. -1.5b1 (2016-09-09) ------------------- +## 1.5b1 (2016-09-09) - GroupEnumeration paged. [jensens] @@ -312,16 +298,16 @@ Bug fixes: - Adopt LDAP instector to use DN instead of RDN for node identification. [rnix] -- Add dummy ``defaults`` setting to ``UsersConfig`` and ``GroupsConfig`` +- Add dummy `defaults` setting to `UsersConfig` and `GroupsConfig` adapters. These defaults are used to set child creation defaults, thus concrete implementation is postponed until user and group creation is supported through plone UI. [rnix] -- Add ``ignore_cert`` setting to ``LDAPProps`` adapter. +- Add `ignore_cert` setting to `LDAPProps` adapter. [rnix] -- Remove ``check_duplicates`` setting which is not available any more in +- Remove `check_duplicates` setting which is not available any more in node.ext.ldap. [rnix] @@ -338,8 +324,7 @@ Bug fixes: [jensens] -1.4.0 (2014-10-24) ------------------- +## 1.4.0 (2014-10-24) - Feature: Alternative volatile cache for UGM tree on plugin. [jensens] @@ -349,53 +334,50 @@ Bug fixes: - introduce pluggable caching mechanism on ugm-tree level, defaults to caching on request. Can be overruled by providing an adapter implementing - ``pas.plugins.ldap.interfaces.IPluginCacheHandler``. + `pas.plugins.ldap.interfaces.IPluginCacheHandler`. [jensens] - log how long it takes to build up a users or groups tree. [jensens] -1.3.2 (2014-09-10) ------------------- +## 1.3.2 (2014-09-10) - Small fixes in inspector. [rnix] -1.3.1 (2014-08-05) ------------------- +## 1.3.1 (2014-08-05) - Fix dependency versions. [rnix] -1.3.0 (2014-05-12) ------------------- +## 1.3.0 (2014-05-12) -- Raise ``RuntimeError`` instead of ``KeyError`` when password change method +- Raise `RuntimeError` instead of `KeyError` when password change method couldn't locate the user in LDAP tree. Maybe it's a local user and - ``Products.PlonePAS.pas.userSetPassword`` expects a ``RuntimeError`` to be + `Products.PlonePAS.pas.userSetPassword` expects a `RuntimeError` to be raised in this case. [saily] -1.2.0 (2014-03-13) ------------------- +## 1.2.0 (2014-03-13) -- add property ``check_duplicates``. Adds ability to disable duplicates check +- add property `check_duplicates`. Adds ability to disable duplicates check for keys in ldap in order to avoid failure if ldap strcuture is not perfect. - Add new property to disable duplicate primary/secondary key checking in LDAP trees. This allows pas.plugins.ldap to read LDAP tree and ignore - duplicated items instead of raising:: + duplicated items instead of raising: - Traceback (most recent call last): - ... - RuntimeError: Key not unique: =''. + ``` + Traceback (most recent call last): + ... + RuntimeError: Key not unique: =''. + ``` -1.1.0 (2014-03-03) ------------------- +## 1.1.0 (2014-03-03) - ldap errors dont block that much if ldap is not reachable, timeout blocked in past the whole zope. now default timeout for retry is @@ -412,23 +394,20 @@ Bug fixes: [saily] -1.0.2 ------ +## 1.0.2 - sometimes ldap returns an empty string as portrait. take this as no portrait. [jensens, 2013-09-11] -1.0.1 ------ +## 1.0.1 - because of passwordreset problem we figured out that pas searchUsers calls plugins search with both login and name, which was passed to ugm and returned always an empty result [benniboy] -1.0 ---- +## 1.0 - make it work. -- base work done so far in ``bda.pasldap`` and ``bda.plone.ldap`` was merged. +- base work done so far in `bda.pasldap` and `bda.plone.ldap` was merged. diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.md similarity index 79% rename from CONTRIBUTORS.rst rename to CONTRIBUTORS.md index 4d0783c..d47901d 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.md @@ -1,10 +1,9 @@ -Contributors -============ - -- Jens W. Klein -- Robert Niederrreiter -- Florian Friesdorf -- Daniel Widerin -- Johannes Raggam -- Luca Fabbri -- Leonardo J. Caballero G. +# Contributors + +- Jens W. Klein +- Robert Niederrreiter +- Florian Friesdorf +- Daniel Widerin +- Johannes Raggam +- Luca Fabbri +- Leonardo J. Caballero G. diff --git a/LICENSE.rst b/LICENSE.md similarity index 77% rename from LICENSE.rst rename to LICENSE.md index f0e3a6c..a30eb22 100644 --- a/LICENSE.rst +++ b/LICENSE.md @@ -1,6 +1,4 @@ - -License -======= +# License Copyright (c) 2010-2020, BlueDynamics Alliance, Austria, Germany, Switzerland All rights reserved. @@ -8,16 +6,16 @@ All rights reserved. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: -* Redistributions of source code must retain the above copyright notice, this +- Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. -* Redistributions in binary form must reproduce the above copyright notice, this - list of conditions and the following disclaimer in the documentation and/or +- Redistributions in binary form must reproduce the above copyright notice, this + list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. -* Neither the name of the BlueDynamics Alliance nor the names of its - contributors may be used to endorse or promote products derived from this +- Neither the name of the BlueDynamics Alliance nor the names of its + contributors may be used to endorse or promote products derived from this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY BlueDynamics Alliance ``AS IS`` AND ANY + +THIS SOFTWARE IS PROVIDED BY BlueDynamics Alliance `AS IS` AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL BlueDynamics Alliance BE LIABLE FOR ANY diff --git a/MANIFEST.in b/MANIFEST.in deleted file mode 100644 index 5af1496..0000000 --- a/MANIFEST.in +++ /dev/null @@ -1,5 +0,0 @@ -graft src -include *.cfg *.rst -global-exclude *.pyc -include *.txt -include .coveragerc diff --git a/README.md b/README.md new file mode 100644 index 0000000..f39f435 --- /dev/null +++ b/README.md @@ -0,0 +1,296 @@ +[![Latest PyPI version](https://img.shields.io/pypi/v/pas.plugins.ldap.svg)](https://pypi.python.org/pypi/pas.plugins.ldap) + +[![Number of PyPI downloads](https://img.shields.io/pypi/dm/pas.plugins.ldap.svg)](https://pypi.python.org/pypi/pas.plugins.ldap) + +[![CI](https://github.com/collective/pas.plugins.ldap/actions/workflows/ci.yaml/badge.svg)](https://github.com/collective/pas.plugins.ldap/actions/workflows/ci.yaml) + +[![coveralls](https://coveralls.io/repos/collective/pas.plugins.ldap/badge.svg?branch=master&service=github)](https://coveralls.io/github/collective/pas.plugins.ldap?branch=master) + +This is a [LDAP](https://en.wikipedia.org/wiki/Lightweight_Directory_Access_Protocol) Plugin for the [Zope](https://www.zope.dev/) [Pluggable Authentication Service (PAS)](https://pypi.org/project/Products.PluggableAuthService/). + +`pas.plugins.ldap` is **not** releated to the old [LDAPUserFolder](https://pypi.org/project/Products.LDAPUserFolder/) / [LDAPMultiPlugins](https://pypi.org/project/Products.LDAPMultiPlugins/) and the packages (i.e. [PloneLDAP](https://pypi.org/project/Products.PloneLDAP/)) stacked on top of it in any way. + +It is based on [node.ext.ldap](https://pypi.org/project/node.ext.ldap/), an almost framework independent LDAP stack. + +# Features + +- If [Plone](https://plone.org) is installed an integration layer with a `setup profile` and a `Plone Controlpanel` page is available. +- It works in a plain Zope even if it depends on [PlonePAS](https://pypi.org/project/Products.PlonePAS). +- LDAP authentication and authorization for users and groups. +- It provides users and/or groups from an LDAP directory. +- LDAP properties for users and groups, which can be used in the rest of the system as well. +- For now users and groups can't be added or deleted. Properties on both are read/write. + +## TODO + +For a detailed list of TODO tasks to this project, please checkout the [TODO file](https://github.com/collective/pas.plugins.ldap/blob/main/TODO.md). + +# Screenshots + +After installation, you will find a new behavior available, go to `Site Setup` > `Users` > `LDAP / AD Support` as the following screenshot: + +[![pas.plugins.ldap Plone control panel](https://raw.githubusercontent.com/collective/pas.plugins.ldap/refs/heads/main/docs/images/ldap-settings.png)](https://github.com/collective/pas.plugins.ldap/) + +# Translations + +This product has been translated into + +- English +- Spanish + +# Installation + +This package supports Zope applications and Plone sites using Volto and Classic UI. + +## Dependencies + +This package depends on [python-ldap](https://pypi.org/project/python-ldap/) package. + +To build it correctly you need to have some development libraries included in your system. + +On a Debian-based installation use: + +```console +sudo apt install python-dev libldap2-dev libsasl2-dev libssl-dev +``` + +--- + +## Zope + +Install `pas.plugins.ldap` by adding it to the `instance` section of your `buildout`: + +```ini +eggs = + ... + pas.plugins.ldap + +zcml = + ... + pas.plugins.ldap +``` + +Run `buildout` and then restart the `Zope` instance. + +Then browse to your `acl_users` folder and add an `LDAP Plugin` object. + +Configure it using the `LDAP Settings` form and choose the functionality this +`LDAP Plugin` will perform with the `Activate` tab. + +- `Authentication` (`authenticateCredentials`) +- `Group_Enumeration` (`enumerateGroups`) +- `Group_Introspection` (`getGroupById`) +- `Group_Management` (`addGroup`) +- `Groups` (`getGroupsForPrincipal`) +- `Properties` (`getPropertiesForUser`) +- `Roles` (`getRolesForPrincipal`) +- `User_Adder` (`doAddUser`) +- `User_Enumeration` (`enumerateUsers`) +- `User_Management` (`doChangeUser`) + +--- + +## Plone + +To install `pas.plugins.ldap` in a Plone site, you need by adding it to the +`instance` section of your `buildout`: + +```ini +eggs = + ... + pas.plugins.ldap +``` + +Run `buildout` and then restart the `Plone` instance. + +Then go to the Plone control-panel, select `Addons` and install the `LDAP / Active Directory Support`. + +So, you can navigate to `Site Setup` > `Users` > `LDAP / AD Support` and click it and configure the plugin there. + +To use an own integration-profile, add to the profiles `metadata.xml` file: + +```xml +... + + ... + profile-pas.plugins.ldap.plonecontrolpanel:default + +... +``` + +Additionally the `LDAP Settings` can be exported and imported with `portal_setup` tool. +You can place the exported `ldapsettings.xml` file in your integration profile, so it will be imported with your next install again. + +**Warning:** + +**The LDAP-password is stored in there in plain text!** + +But anonymous bindings are possible. + +--- + +## Logging + +To get detailed output of all LDAP-operations and much more set the logging level to debug. +Attention, this is lots of output. + +LDAP as an external service might be down, non-responsive or slow. +This package logs such events to raise awareness. +There are two environment variables to control the logging of LDAP-errors: + +`PAS_PLUGINS_LDAP_ERROR_LOG_TIMEOUT` +: First LDAP-error is logged, further errors ignored until the given number of seconds have passed. + This supresses flooding logs if LDAP is down. + Default: 300.0 (time in seconds, float). + +`PAS_PLUGINS_LDAP_LONG_RUNNING_LOG_THRESHOLD` +: Log long running LDAP/PAS operations. + If a PAS operation takes longer than he given number of seconds, log it as error. + Default: 5 (time in seconds, float). + +--- + +## Timeouts + +Global LDAP timeouts are set and controlled by two environment variables: + +`PAS_PLUGINS_LDAP_OPT_NETWORK_TIMEOUT` +: Connection timeout. + Default: 1.0s + +`PAS_PLUGINS_LDAP_OPT_TIMEOUT` +: Overall timeout. + Default: 30.0s + +See details in [python-ldap](https://pypi.org/project/python-ldap/) documentation: OPT_NETWORK_TIMEOUT and OPT_TIMEOUT. + +--- + +## Caching + +**Without caching this module is slow** (as any other module talking to LDAP will be). + +By **default** the LDAP-queries are **not cached**. + +A **must have** for a production environment is having [memcached](https://memcached.org/) server configured as LDAP query cache. + +Cache at least for ~6 seconds, so a page load with all its resources is covered also in worst case. + +The UGM tree is cached by default on the request, that means its built up every request from (cached) ldap queries. + +There is an alternative adapter available which will cache the ugm tree as volatile attribute (`_v_...`) on the persistent plugin. + +Volatile attributes are not persisted in the ZODB. + +If the plugin object vanishes from ZODB cache the atrribute is gone. + +The volatile plugin cache can be activated by loading its zcml with ``_ Plugin for the `Zope `_ `Pluggable Authentication Service (PAS) `_. - -``pas.plugins.ldap`` is **not** releated to the old `LDAPUserFolder `_ / `LDAPMultiPlugins `_ and the packages (i.e. `PloneLDAP `_) stacked on top of it in any way. - -It is based on `node.ext.ldap `_, an almost framework independent LDAP stack. - - -Features -======== - -- If `Plone `_ is installed an integration layer with a `setup profile` and a `Plone Controlpanel` page is available. -- It works in a plain Zope even if it depends on `PlonePAS `_. -- LDAP authentication and authorization for users and groups. -- It provides users and/or groups from an LDAP directory. -- LDAP properties for users and groups, which can be used in the rest of the system as well. -- For now users and groups can't be added or deleted. Properties on both are read/write. - -TODO ----- - -For a detailed list of TODO tasks to this project, please checkout the `TODO file `_. - -Screenshots -=========== - -After installation, you will find a new behavior available, go to ``Site Setup`` > ``Users`` > ``LDAP / AD Support`` as the following screenshot: - - -.. image:: https://raw.githubusercontent.com/collective/pas.plugins.ldap/refs/heads/main/docs/images/ldap-settings.png - :target: https://github.com/collective/pas.plugins.ldap/ - :alt: pas.plugins.ldap Plone control panel - - -Translations -============ - -This product has been translated into - -- English -- Spanish - -Installation -============ - -This package supports Zope applications and Plone sites using Volto and Classic UI. - -Dependencies ------------- - -This package depends on `python-ldap `_ package. - -To build it correctly you need to have some development libraries included in your system. - -On a Debian-based installation use: - -.. code-block:: console - - sudo apt install python-dev libldap2-dev libsasl2-dev libssl-dev - ----- - -Zope ----- - -Install ``pas.plugins.ldap`` by adding it to the ``instance`` section of your ``buildout``: - -.. code-block:: ini - - eggs = - ... - pas.plugins.ldap - - zcml = - ... - pas.plugins.ldap - -Run ``buildout`` and then restart the ``Zope`` instance. - -Then browse to your ``acl_users`` folder and add an ``LDAP Plugin`` object. - -Configure it using the ``LDAP Settings`` form and choose the functionality this -``LDAP Plugin`` will perform with the ``Activate`` tab. - -- `Authentication` (``authenticateCredentials``) -- `Group_Enumeration` (``enumerateGroups``) -- `Group_Introspection` (``getGroupById``) -- `Group_Management` (``addGroup``) -- `Groups` (``getGroupsForPrincipal``) -- `Properties` (``getPropertiesForUser``) -- `Roles` (``getRolesForPrincipal``) -- `User_Adder` (``doAddUser``) -- `User_Enumeration` (``enumerateUsers``) -- `User_Management` (``doChangeUser``) - ----- - -Plone ------ - -To install ``pas.plugins.ldap`` in a Plone site, you need by adding it to the -``instance`` section of your ``buildout``: - -.. code-block:: ini - - eggs = - ... - pas.plugins.ldap - -Run ``buildout`` and then restart the ``Plone`` instance. - -Then go to the Plone control-panel, select ``Addons`` and install the ``LDAP / Active Directory Support``. - -So, you can navigate to ``Site Setup`` > ``Users`` > ``LDAP / AD Support`` and click it and configure the plugin there. - -To use an own integration-profile, add to the profiles ``metadata.xml`` file: - -.. code-block:: xml - - ... - - ... - profile-pas.plugins.ldap.plonecontrolpanel:default - - ... - -Additionally the ``LDAP Settings`` can be exported and imported with ``portal_setup`` tool. -You can place the exported ``ldapsettings.xml`` file in your integration profile, so it will be imported with your next install again. - -**Warning:** - -**The LDAP-password is stored in there in plain text!** - -But anonymous bindings are possible. - ----- - -Logging -------- - -To get detailed output of all LDAP-operations and much more set the logging level to debug. -Attention, this is lots of output. - -LDAP as an external service might be down, non-responsive or slow. -This package logs such events to raise awareness. -There are two environment variables to control the logging of LDAP-errors: - -``PAS_PLUGINS_LDAP_ERROR_LOG_TIMEOUT`` - First LDAP-error is logged, further errors ignored until the given number of seconds have passed. - This supresses flooding logs if LDAP is down. - Default: 300.0 (time in seconds, float). - -``PAS_PLUGINS_LDAP_LONG_RUNNING_LOG_THRESHOLD`` - Log long running LDAP/PAS operations. - If a PAS operation takes longer than he given number of seconds, log it as error. - Default: 5 (time in seconds, float). - ----- - -Timeouts --------- - -Global LDAP timeouts are set and controlled by two environment variables: - -``PAS_PLUGINS_LDAP_OPT_NETWORK_TIMEOUT`` - Connection timeout. - Default: 1.0s - -``PAS_PLUGINS_LDAP_OPT_TIMEOUT`` - Overall timeout. - Default: 30.0s - -See details in `python-ldap `_ documentation: OPT_NETWORK_TIMEOUT and OPT_TIMEOUT. - ----- - -Caching -------- - -**Without caching this module is slow** (as any other module talking to LDAP will be). - -By **default** the LDAP-queries are **not cached**. - -A **must have** for a production environment is having `memcached `_ server configured as LDAP query cache. - -Cache at least for ~6 seconds, so a page load with all its resources is covered also in worst case. - -The UGM tree is cached by default on the request, that means its built up every request from (cached) ldap queries. - -There is an alternative adapter available which will cache the ugm tree as volatile attribute (``_v_...``) on the persistent plugin. - -Volatile attributes are not persisted in the ZODB. - -If the plugin object vanishes from ZODB cache the atrribute is gone. - -The volatile plugin cache can be activated by loading its zcml with ```_ is installed properly and recognized by buildout. - -This package works fine for several 10000 users or groups, **unless you list users**. - -This is not that much a problem for small amount of users. -There is room for future optimization in the underlying `node.ext.ldap `_. - ----- - -Development Workflow -==================== - -1. Install requirements: - -.. code-block:: shell - - make install - -2. Start Zope instance: - -.. code-block:: shell - - make zope-start - -3. Zpretty format and lint XML/ZCML: - -.. code-block:: shell - - make zpretty-check && make zpretty-format && make zpretty-check - -4. Lint and format Python code with `ruff `_ - (this is what CI enforces): - -.. code-block:: shell - - uvx ruff check . && uvx ruff format . - -5. Extract i18n messages: - -.. code-block:: shell - - make gettext-create && make gettext-update && make gettext-compile - - The compiled ``.mo`` catalogs are also generated automatically when the - package is built (sdist/wheel), so they always ship in a release. - -6. Run unit tests: - -.. code-block:: shell - - make test - -7. Run coverage unit tests: - -.. code-block:: shell - - make coverage - ----- - -Source Code -=========== - -If you want to help with the development (improvement, update, bug-fixing, ...) of ``pas.plugins.ldap`` this is a great idea! - -- The code is located in the `GitHub Collective `_. - -- You can clone it or `get access to the GitHub Collective `_ and work directly on the project. - -Authors -------- - -This product was developed by `BlueDynamics Alliance `_ team. - -.. image:: https://bluedynamics.com/++theme++bda.theme/static/bda-media/bda-logo.svg - :target: https://bluedynamics.com/ - :alt: BlueDynamics Alliance - -Maintainers are Robert Niederreiter, Jens Klein and the `BlueDynamics Alliance `_ developer team. - -Support -======= - -We appreciate any contribution. Releases to PyPI are automated via tagging and -Trusted Publishing; see `RELEASE.md `_ -for the process. If you need a release done, please contact one of us: -`dev@bluedynamics dot com `_. - -- If you are having issues, please let us know at our `issue tracker `_. - -Contributors -============ - -For a list of all contributors to this project, please checkout the following resources: - -- The `CONTRIBUTORS file `_. - -- The `Contributors page on GitHub `_. - -License -======= - -The project is licensed under the GPLv2. diff --git a/RELEASE.md b/RELEASE.md index 28619f5..c9009d3 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -106,7 +106,7 @@ Use [PEP 440](https://peps.python.org/pep-0440/) pre-release tags ## Cutting a release -1. Make sure `main` is green and `CHANGES.rst` lists everything under the +1. Make sure `main` is green and `CHANGES.md` lists everything under the target version heading. Rename the `2.0.0 (unreleased)` heading to the release date, e.g. `2.0.0 (2026-06-22)`, and commit/merge that to `main`. 2. Create and push the tag: @@ -125,7 +125,7 @@ Use [PEP 440](https://peps.python.org/pep-0440/) pre-release tags ## Pre-release checklist -- [ ] `CHANGES.rst` is up to date and the heading carries the release date. +- [ ] `CHANGES.md` is up to date and the heading carries the release date. - [ ] CI is green on `main` (QA + the full Plone 6.0–6.2 / Python 3.10–3.14 matrix). - [ ] Trusted publishers and GitHub environments are configured (one-time), and `release-pypi` has **required reviewers** set (mandatory for this diff --git a/TODO.rst b/TODO.md similarity index 77% rename from TODO.rst rename to TODO.md index ef07018..fc54f78 100644 --- a/TODO.rst +++ b/TODO.md @@ -1,12 +1,8 @@ +# TODO -TODO -==== +See also [Issue-Tracker](https://github.com/collective/pas.plugins.ldap/issues) -See also `Issue-Tracker `_ - - -Milestone 3.0 -------------- +## Milestone 3.0 - remove portrait monkey patch - add/delete users diff --git a/pyproject.toml b/pyproject.toml index 15be107..ca8c248 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,7 +2,7 @@ name = "pas.plugins.ldap" dynamic = ["version"] description = "LDAP/AD Plugin for Plone/Zope PluggableAuthService (users+groups)" -readme = "README.rst" +readme = "README.md" license = { text = "GPL 2.0" } authors = [{ name = "BlueDynamics Alliance", email = "dev@bluedynamics.com" }] keywords = [ @@ -64,7 +64,7 @@ test = ["plone.testing", "pytest-plone"] target = "plone" [project.urls] -ChangeLog = "https://github.com/collective/pas.plugins.ldap/blob/master/CHANGES.rst" +ChangeLog = "https://github.com/collective/pas.plugins.ldap/blob/main/CHANGES.md" Homepage = "https://github.com/collective/pas.plugins.ldap/" "Issue Tracker" = "https://github.com/collective/pas.plugins.ldap/issues" "Source Code" = "https://github.com/collective/pas.plugins.ldap" From 9854b4f31d301a56ba7929bef8c4f02dc92775e2 Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 18:05:23 +0200 Subject: [PATCH 7/8] Drop unused setuptools runtime dependency No pkg_resources / setuptools usage anywhere in the code (implicit namespace packages); it was a leftover from the setup.py era. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGES.md | 3 +++ pyproject.toml | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index f89c7b7..a1c3437 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,9 @@ `MANIFEST.in`. (The `tests/*.rst` doctest files are unchanged.) [jensens] +- Drop the unused `setuptools` runtime dependency (no `pkg_resources` usage). + [jensens] + - Portrait traverser: raise a proper `LocationError` (404) when a user has no portrait on the property sheet, instead of an `AttributeError` from calling `__of__` on `None`. diff --git a/pyproject.toml b/pyproject.toml index ca8c248..e38c79c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,6 @@ dependencies = [ "Products.PluggableAuthService", "Products.statusmessages", "python-ldap>=3.4.0", - "setuptools", "yafowil.plone>=6.0.0", "yafowil.widget.array>=2.0.0", "yafowil.widget.dict>=2.0.0", From 5f871325db315abf756567792aa3c882d5f6c57f Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Mon, 22 Jun 2026 18:07:07 +0200 Subject: [PATCH 8/8] Remove dead coveralls badge Coveralls has not been fed since 2021 (no coverage upload step in CI) and the badge pointed at the old 'master' branch. Local coverage via 'make coverage' is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index f39f435..89104a0 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,6 @@ [![CI](https://github.com/collective/pas.plugins.ldap/actions/workflows/ci.yaml/badge.svg)](https://github.com/collective/pas.plugins.ldap/actions/workflows/ci.yaml) -[![coveralls](https://coveralls.io/repos/collective/pas.plugins.ldap/badge.svg?branch=master&service=github)](https://coveralls.io/github/collective/pas.plugins.ldap?branch=master) - This is a [LDAP](https://en.wikipedia.org/wiki/Lightweight_Directory_Access_Protocol) Plugin for the [Zope](https://www.zope.dev/) [Pluggable Authentication Service (PAS)](https://pypi.org/project/Products.PluggableAuthService/). `pas.plugins.ldap` is **not** releated to the old [LDAPUserFolder](https://pypi.org/project/Products.LDAPUserFolder/) / [LDAPMultiPlugins](https://pypi.org/project/Products.LDAPMultiPlugins/) and the packages (i.e. [PloneLDAP](https://pypi.org/project/Products.PloneLDAP/)) stacked on top of it in any way.