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.md similarity index 63% rename from CHANGES.rst rename to CHANGES.md index 5ee1a06..a1c3437 100644 --- a/CHANGES.rst +++ b/CHANGES.md @@ -1,20 +1,36 @@ +# 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. + [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] + +- 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] + +- 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``. - Fixes `issue #68 `_. +- 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. @@ -41,29 +57,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] @@ -74,17 +90,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] @@ -93,23 +107,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: @@ -120,30 +131,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. @@ -155,19 +163,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 @@ -179,8 +186,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] @@ -189,8 +195,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] @@ -233,11 +238,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. @@ -247,8 +251,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. @@ -258,23 +261,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] @@ -301,16 +301,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] @@ -327,8 +327,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] @@ -338,53 +337,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 @@ -401,23 +397,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..89104a0 --- /dev/null +++ b/README.md @@ -0,0 +1,294 @@ +[![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) + +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 code: - -.. code-block:: shell - - make zpretty-check && make zpretty-format && make zpretty-check - -4. Isort format and lint code: - -.. code-block:: shell - - make isort-check && make isort-format && make isort-check - -5. Black format and lint code: - -.. code-block:: shell - - make black-check && make black-format && make black-check - -6. Extract i18n messages: - -.. code-block:: shell - - make gettext-create && make gettext-update && make gettext-compile - -7. Run unit tests: - -.. code-block:: shell - - make test - -8. 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 and if a release is needed to be done on pypi, please just 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 new file mode 100644 index 0000000..c9009d3 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,133 @@ +# 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` + + **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** → 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. + +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 + +`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.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: + ```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.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 + collective repo). +- [ ] Tag follows PEP 440 (`2.0.0`, `2.0.1`, `2.1.0a1`, …). 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/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..e38c79c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,8 +1,8 @@ [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" +readme = "README.md" license = { text = "GPL 2.0" } authors = [{ name = "BlueDynamics Alliance", email = "dev@bluedynamics.com" }] keywords = [ @@ -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", @@ -64,15 +63,19 @@ 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" [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 +93,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)